Re: [PATCH net v2 1/2] net: phylink: call phy_init_hw() in phylink resume path

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Thu, Apr 09, 2026 at 09:56:32AM +0000, Ovidiu Panait wrote:
> When mac_managed_pm flag is set, mdio_bus_phy_resume() is skipped,
> so phy_init_hw(), which performs soft_reset and config_init, is not
> called during resume.
> 
> This is inconsistent with the non-mac_managed_pm path, where
> mdio_bus_phy_resume() calls phy_init_hw() before phy_resume()
> on every resume.
> 
> Add phy_init_hw() calls in both phylink_prepare_resume() and
> phylink_resume(), to ensure that the PHY state is the same as
> when the PHY is resumed via the MDIO bus.
> 
> Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
> ---
>  drivers/net/phy/phylink.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 087ac63f9193..c302126009f6 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -2669,8 +2669,10 @@ void phylink_prepare_resume(struct phylink *pl)
>  	 * then resume the PHY. Note that 802.3 allows PHYs 500ms before
>  	 * the clock meets requirements. We do not implement this delay.
>  	 */
> -	if (pl->config->mac_requires_rxc && phydev && phydev->suspended)
> +	if (pl->config->mac_requires_rxc && phydev && phydev->suspended) {
> +		phy_init_hw(phydev);
>  		phy_resume(phydev);

I'm going to make an alternative suggestion - should we combine
phy_init_hw() and phy_resume() to ensure that all MAC drivers that
call phy_resume() correctly initialise the PHY first?

Looking at the callers of phy_resume():

- drivers/net/ethernet/nxp/lpc_eth.c - calls phy_resume() from
  lpc_eth_open() but no call to phy_init_hw(). Not used in
  suspend/resume paths, so presumably uses the built-in phylib
  handling that does call phy_init_hw() before phy_resume().

- drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c - suspends
  and then immediately resumes the PHY before enabling loopback. Seems
  like a PHY workaround that should've been handled in the PHY driver.
  Suspends the PHY when loopback is disabled (which looks buggy.)

- drivers/net/ethernet/hisilicon/hns/hns_ethtool.c - resumes the PHY
  when enabling loopback and suspends the PHY when loopback is
  disabled. (what if the netdev is already up? Also looks buggy to me.)

- drivers/net/ethernet/broadcom/genet/bcmgenet.c - bcmgenet_resume()
  calls phy_init_hw() before a conditional call to phy_resume(). I
  don't see a matching call to phy_suspend(). If the bcmgenet device
  may wakeup the system, then wouldn't the PHY configuration be
  preserved over suspend/resume making the call to phy_init_hw() also
  unnecessary if device_may_wakeup(d) returns true?

- drivers/net/ethernet/broadcom/bcmsysport.c - no call to phy_init_hw()
  before phy_resume().

- drivers/net/ethernet/realtek/r8169_main.c - calls phy_init_hw()
  immediately before phy_resume().

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