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

[Ovidiu Panait]

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.

> But in Suspend to RAM, PHY loses power and the reset value of Power Down
> bit 0 (ie normal mode),
> there is no restriction for accessing MMD registers in this mode.
> 

If the PHY loses power, it loses all the configuration that was done by
ksz9131_config_init(). Right now, only the RGMII delays are restored, which
is a subset of the configurations done by ksz9131_config_init().

Calling phy_init_hw() fixes both of these issues.

Ovidiu


> Am I missing anything here??
> 
> 
> Cheers,
> Biju