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:13:10 +0000 |
| |
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
Cheers, Biju
> > Fixes: f25a7eaa897f ("net: phy: micrel: Add ksz9131_resume()") > Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com> > --- > drivers/net/phy/micrel.c | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index 2aa1dedd21b8..f2513109865a > 100644 > --- a/drivers/net/phy/micrel.c > +++ b/drivers/net/phy/micrel.c > @@ -6014,14 +6014,6 @@ static int lan8841_suspend(struct phy_device *phydev) > return kszphy_generic_suspend(phydev); } > > -static int ksz9131_resume(struct phy_device *phydev) -{ > - if (phydev->suspended && phy_interface_is_rgmii(phydev)) > - ksz9131_config_rgmii_delay(phydev); > - > - return kszphy_resume(phydev); > -} > - > #define LAN8842_PTP_GPIO_NUM 16 > > static int lan8842_ptp_probe_once(struct phy_device *phydev) @@ -6929,7 +6921,7 @@ static struct > phy_driver ksphy_driver[] = { > .get_strings = kszphy_get_strings, > .get_stats = kszphy_get_stats, > .suspend = kszphy_suspend, > - .resume = ksz9131_resume, > + .resume = kszphy_resume, > .cable_test_start = ksz9x31_cable_test_start, > .cable_test_get_status = ksz9x31_cable_test_get_status, > .get_features = ksz9477_get_features, > -- > 2.34.1
|  |