Messages in this thread |  | | From | Biju Das <> | | Subject | RE: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume() | | Date | Thu, 9 Apr 2026 10:52:35 +0000 |
| |
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
|  |