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

[Ovidiu Panait]

Hi Geert,

> 
> Hi Ovidiu,
> 
> On Fri, 3 Apr 2026 at 13:18, Ovidiu Panait <ovidiu.panait.rb@xxxxxxxxxxx>
> wrote:
> > 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.
> >
> > Additionally, on platforms where the PHY loses power during suspend
> > (e.g. RZ/G3E), all settings from ksz9131_config_init(), not just the
> > RGMII delays, are lost and need to be restored. When the MAC driver
> > sets mac_managed_pm (e.g. stmmac), mdio_bus_phy_resume() is skipped,
> > so phy_init_hw() (which calls config_init to restore all PHY settings)
> > is never invoked during resume.
> >
> > Fix this by replacing the RGMII delay restoration with a call to
> > phy_init_hw(), which takes the PHY out of SPD and performs full
> > reinitialization.
> >
> > Fixes: f25a7eaa897f ("net: phy: micrel: Add ksz9131_resume()")
> > Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@xxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -6016,8 +6016,13 @@ static int lan8841_suspend(struct phy_device
> *phydev)
> >
> >  static int ksz9131_resume(struct phy_device *phydev)
> >  {
> > -       if (phydev->suspended && phy_interface_is_rgmii(phydev))
> > -               ksz9131_config_rgmii_delay(phydev);
> > +       int ret;
> > +
> > +       if (phydev->suspended) {
> > +               ret = phy_init_hw(phydev);
> > +               if (ret)
> > +                       return ret;
> > +       }
> >
> >         return kszphy_resume(phydev);
> >  }
> 
> This function is now no longer KSZ9131-specific.
> I am wondering if this should be done for other Micrel PHYs, too,
> e.g. by moving the phy_init_hw() call into kszphy_resume()?
> 
> Ethernet after resume has always been flaky on Salvator-X with KSZ9031
> and R-Car M3-W ES1.0 (this seems to be specific to R-Car M3-W, as
> boards with R-Car H3 or M3-N do not seem to suffer from this; don't
> ask me why).
> 
> I have just tried:
> 
> -       .resume         = kszphy_resume,
> +       .resume         = ksz9131_resume,
> 
> in the KSZ9031 entry, and ... surprise! Ethernet on R-Car M3-W now
> works much better after resume!
> 

I checked the dts for the Salvator-X board and it seems that it uses
the RAVB MAC. The driver for RAVB sets the mac_managed_pm flag, which
means that the MAC driver is handling the suspend/resume of the PHY.

In this case, as Russell pointed out in [1], the MAC driver should be
the one issuing phy_init_hw() before calling phy_resume(). I sent a
fix for the phylink resume path ([2]), to align it with the MDIO bus
resume path, but the RAVB driver doesn't seem to be using phylink,
it calls phy_start() directly.

Based on this, I think the fix should be to add a phy_init_hw() call
in the RAVB driver, on the resume path, rather than in the PHY driver.

[1] https://lore.kernel.org/all/ac_Udvtrj0Bl-6wl@xxxxxxxxxxxxxxxxxxxxx/
[2] https://lore.kernel.org/all/20260409095633.70973-2-ovidiu.panait.rb@xxxxxxxxxxx/

Thanks,
Ovidiu