RE: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()

Biju Das
2026-04-09T10:52:39Z

Hi Russell King,

Thanks for the feedback.

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: 09 April 2026 11:30
> Subject: Re: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
>
> On Thu, Apr 09, 2026 at 10:13:10AM +0000, Biju Das wrote:
> > Hi Ovidu,
> >
> > Thanks for the patch.
> >
> > > -----Original Message-----
> > > From: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> > > Sent: 09 April 2026 10:57
> > > Subject: [PATCH net v2 2/2] net: phy: micrel: remove
> > > ksz9131_resume()
> > >
> > > ksz9131_resume() was added to restore RGMII delays on resume for
> > > platforms where the PHY loses power during suspend to RAM. However, for s2idle, the PHY stays in
> Software Power-Down (SPD) during resume.
> > > In that case,
> > > ksz9131_config_rgmii_delay() accesses MMD registers before
> > > kszphy_resume() clears BMCR_PDOWN. The
> > > KSZ9131 datasheet states that during SPD, access to the MMD registers is restricted:
> > >
> > > - 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.
> > >
> > > Additionally, only RGMII delays were restored, while other settings
> > > from ksz9131_config_init() were not.
> > >
> > > Now that the preceding commit ("net: phylink: call phy_init_hw() in
> > > phylink resume path") performs a
> > > phy_init_hw() during phylink resume,
> > > ksz9131_resume() is no longer needed.
> > >
> > > Remove it and use kszphy_resume() directly.
> >
> > How to avoid code duplication in this case?
> >
> > For eg: phy_init_hw() makes the phy out of SPD state
> >
> > and kszphy_resume() unconditionally makes the phy out of SPD state again.
> > ¬ kszphy_generic_resume
> > ¬ genphy_resume
>
> My question would be... if we mandate that phy_init_hw() must be called before phy_resume() by MAC
> drivers, then how much of kszphy_resume() becomes redundant?
>
> Given that populating drv->soft_reset() with genphy_soft_reset() means the PDOWN bit will be cleared,
> genphy_resume() becomes redundant.
>
> phy_init_hw() will also call drv->config_intr(), so that doesn't need to be done either.
>
> It will also call drv->config_init(), which will call kszphy_config_reset().
>
> So most of kszphy_resume() becomes unnecessary. I think the only thing that remains would be the call
> to kszphy_enable_clk() - and is it fine to call that after phy_init_hw() ?

It just needs kszphy_enable_clk() and phydev->drv->config_intr() to enable PHY interrupts for
suspend-to-RAM to work on RZ/G3E SMARC EVK.

Cheers,
Biju