RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()

[Biju Das]

Hi Ovidiu Panait,

> -----Original Message-----
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > Subject: RE: [PATCH net] net: phy: micrel: Fix MMD register access
> > during SPD in ksz9131_resume()
> >
> >
> > > >
> > > > Hi,
> > > >
> > > > >
> > > > > Hi Ovidiu Panait,
> > > > >
> > > > > Thanks for the patch.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ovidiu Panait <ovidiu.panait.rb@xxxxxxxxxxx>
> > > > > > Sent: 03 April 2026 12:18
> > > > > > Subject: [PATCH net] net: phy: micrel: Fix MMD register access
> > > > > > during
> > > > > SPD in ksz9131_resume()
> > > > > >
> > > > > > During system suspend, phy_suspend() puts the PHY into
> > > > > > Software
> > > > > > Power-
> > > > > Down
> > > > > > (SPD) by setting the BMCR_PDOWN bit in MII_BMCR. According to
> > > > > > the
> > > > > KSZ9131 datasheet, MMD register
> > > > > > access is restricted during SPD:
> > > > > >
> > > > > >   - Only access to the standard registers (0 through 31) is
> > > supported.
> > > > > >   - Access to MMD address spaces other than MMD address space 1 is
> > > > > >     possible if the spd_clock_gate_override bit is set.
> > > > > >   - Access to MMD address space 1 is not possible.
> > > > > >
> > > > > > However, ksz9131_resume() calls ksz9131_config_rgmii_delay()
> > > > > > before
> > > > > > kszphy_resume() clears BMCR_PDOWN. This means MMD registers
> > > > > > are accessed
> > > > > while the PHY is still in SPD,
> > > > > > contrary to the datasheet.
> > > > >
> > > > > SPD mode: This mode is used to power down the device when it is
> > > > > not in use after power-up.
> > > > >           Previous register settings are maintained during and
> > > > > following the removal of SPD.
> > > > >
> > > > > Suspend to Idle case, it is in SPD mode and the MMD register
> > > > > values are retained.
> > > > >
> > > >
> > > > On resume from s2idle, ksz9131_resume() calls
> > > ksz9131_config_rgmii_delay() which does MMD accesses,
> > > > while the PHY is in SPD. According to the datasheet, it shouldn't
> > > happen. See commit e398822c4751
> > > > ("net: phy: micrel: populate .soft_reset for KSZ9131") which fixes
> > > > the
> > > same issue.
> > >
> > > On my board, while s2idle in SPD mode, it does not hang. The
> > > datasheet does not explain the behaviour when it is SPD mode. But it
> > > states that it retains all previous register values when it is out of SPD mode.
> > >
> >
> > According to the KSZ9131 datasheet ([1]):
> >
> > 4.17.3 SOFTWARE POWER-DOWN MODE (SPD)
> > ...
> > The following remain operational during SPD:
> > MII Management Interface
> > - Only access to the standard registers (0 through 31) is supported.
> > - Access to MMD address spaces other than MMD address space 1 is
> >   possible if the spd_clock_gate_override bit is set.
> > - Access to MMD address space 1 is not possible.
> >
> >
> > The spd_clock_gate_override bit is not used in the KSZ9131 driver.
> >
> > While the datasheet does not specify exactly what happens if registers
> > from an unsupported address space are accessed while the PHY is in
> > SPD, I think it is correct for the driver to not do it in the first place.
> >
> > [1]
> > https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/Product
> > Documents/DataSheets/00002841D.pd
> > f
> 
> For s2idlecase: ie, PHY is in software power down state you don't need to restore MMD register, as
> exiting software power down will restore those registers.
> 
> You need only restore MMD registers, when PHY loses power ie, suspend to RAM case.


I believe, You really don't need to call phy_init_hw() at all

SuspendtoRAM case: ###### ksz9131_resume 1140, software power down state is 0 (Normal mode)
SuspendtoIdle case: ###### ksz9131_resume 1940, software power down state  is 1 (SPD mode)

If (phydev->suspended && Normal mode)
  	Restore PHY specific MMD registers.

Cheers,
Biju