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

[Biju Das]

> -----Original Message-----
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Sent: 03 April 2026 17:29
> Subject: RE: [PATCH net] net: phy: micrel: Fix MMD register access during SPD in ksz9131_resume()
> 
> 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.


Please ignore the above, as I am not an expert in PHY sub system.

Cheers,
Biju