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

From: "Russell King (Oracle)" <[email-masked]>
Date: Thu, 9 Apr 2026 11:29:38 +0100

On Thu, Apr 09, 2026 at 10:13:10AM +0000, Biju Das wrote:
> Hi Ovidu,
> 
> Thanks for the patch.
> 
> > -----Original Message-----
> > From: Ovidiu Panait <[email-masked]>
> > 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() ?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!