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

[Biju Das]

Hi Ovidu,

Thanks for the patch.

> -----Original Message-----
> From: Ovidiu Panait <ovidiu.panait.rb@xxxxxxxxxxx>
> 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@xxxxxxxxxxx>
> ---
>  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