public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2 0/2] net: phylink: fix PHY reinitialization on resume
@ 2026-04-09  9:56 Ovidiu Panait
  2026-04-09  9:56 ` [PATCH net v2 1/2] net: phylink: call phy_init_hw() in phylink resume path Ovidiu Panait
  2026-04-09  9:56 ` [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume() Ovidiu Panait
  0 siblings, 2 replies; 10+ messages in thread
From: Ovidiu Panait @ 2026-04-09  9:56 UTC (permalink / raw)
  To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
	biju.das.jz
  Cc: netdev, linux-kernel, linux-renesas-soc, Ovidiu Panait

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.

This series adds phy_init_hw() to the phylink resume path to ensure
consistent PHY state regardless of whether mac_managed_pm is set,
and removes a now-redundant workaround in the KSZ9131 PHY driver.

v2:
- Moved phy_init_hw() from ksz9131_resume() to phylink resume path, as
  suggested by Russell.

v1: https://lore.kernel.org/all/20260403111738.37749-1-ovidiu.panait.rb@renesas.com/#t

Ovidiu Panait (2):
  net: phylink: call phy_init_hw() in phylink resume path
  net: phy: micrel: remove ksz9131_resume()

 drivers/net/phy/micrel.c  | 10 +---------
 drivers/net/phy/phylink.c |  9 ++++++++-
 2 files changed, 9 insertions(+), 10 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH net v2 1/2] net: phylink: call phy_init_hw() in phylink resume path
  2026-04-09  9:56 [PATCH net v2 0/2] net: phylink: fix PHY reinitialization on resume Ovidiu Panait
@ 2026-04-09  9:56 ` Ovidiu Panait
  2026-04-09 10:42   ` Russell King (Oracle)
  2026-04-09  9:56 ` [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume() Ovidiu Panait
  1 sibling, 1 reply; 10+ messages in thread
From: Ovidiu Panait @ 2026-04-09  9:56 UTC (permalink / raw)
  To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
	biju.das.jz
  Cc: netdev, linux-kernel, linux-renesas-soc, Ovidiu Panait

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);
+	}
 }
 EXPORT_SYMBOL_GPL(phylink_prepare_resume);
 
@@ -2683,6 +2685,8 @@ EXPORT_SYMBOL_GPL(phylink_prepare_resume);
  */
 void phylink_resume(struct phylink *pl)
 {
+	struct phy_device *phydev = pl->phydev;
+
 	ASSERT_RTNL();
 
 	if (phylink_phy_pm_speed_ctrl(pl))
@@ -2712,6 +2716,9 @@ void phylink_resume(struct phylink *pl)
 		/* Re-enable and re-resolve the link parameters */
 		phylink_enable_and_run_resolve(pl, PHYLINK_DISABLE_MAC_WOL);
 	} else {
+		if (phydev && phydev->suspended)
+			phy_init_hw(phydev);
+
 		phylink_start(pl);
 	}
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
  2026-04-09  9:56 [PATCH net v2 0/2] net: phylink: fix PHY reinitialization on resume Ovidiu Panait
  2026-04-09  9:56 ` [PATCH net v2 1/2] net: phylink: call phy_init_hw() in phylink resume path Ovidiu Panait
@ 2026-04-09  9:56 ` Ovidiu Panait
  2026-04-09 10:13   ` Biju Das
  1 sibling, 1 reply; 10+ messages in thread
From: Ovidiu Panait @ 2026-04-09  9:56 UTC (permalink / raw)
  To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni,
	biju.das.jz
  Cc: netdev, linux-kernel, linux-renesas-soc, Ovidiu Panait

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.

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
  2026-04-09  9:56 ` [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume() Ovidiu Panait
@ 2026-04-09 10:13   ` Biju Das
  2026-04-09 10:29     ` Russell King (Oracle)
  0 siblings, 1 reply; 10+ messages in thread
From: Biju Das @ 2026-04-09 10:13 UTC (permalink / raw)
  To: Ovidiu Panait, andrew@lunn.ch, hkallweit1@gmail.com,
	linux@armlinux.org.uk, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, Ovidiu Panait

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
  2026-04-09 10:13   ` Biju Das
@ 2026-04-09 10:29     ` Russell King (Oracle)
  2026-04-09 10:52       ` Biju Das
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King (Oracle) @ 2026-04-09 10:29 UTC (permalink / raw)
  To: Biju Das
  Cc: Ovidiu Panait, andrew@lunn.ch, hkallweit1@gmail.com,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org

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() ?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2 1/2] net: phylink: call phy_init_hw() in phylink resume path
  2026-04-09  9:56 ` [PATCH net v2 1/2] net: phylink: call phy_init_hw() in phylink resume path Ovidiu Panait
@ 2026-04-09 10:42   ` Russell King (Oracle)
  0 siblings, 0 replies; 10+ messages in thread
From: Russell King (Oracle) @ 2026-04-09 10:42 UTC (permalink / raw)
  To: Ovidiu Panait
  Cc: andrew, hkallweit1, davem, edumazet, kuba, pabeni, biju.das.jz,
	netdev, linux-kernel, linux-renesas-soc

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!

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
  2026-04-09 10:29     ` Russell King (Oracle)
@ 2026-04-09 10:52       ` Biju Das
  2026-04-09 11:05         ` Russell King (Oracle)
  0 siblings, 1 reply; 10+ messages in thread
From: Biju Das @ 2026-04-09 10:52 UTC (permalink / raw)
  To: Russell King
  Cc: Ovidiu Panait, andrew@lunn.ch, hkallweit1@gmail.com,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
  2026-04-09 10:52       ` Biju Das
@ 2026-04-09 11:05         ` Russell King (Oracle)
  2026-04-09 11:19           ` Biju Das
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King (Oracle) @ 2026-04-09 11:05 UTC (permalink / raw)
  To: Biju Das
  Cc: Ovidiu Panait, andrew@lunn.ch, hkallweit1@gmail.com,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org

On Thu, Apr 09, 2026 at 10:52:35AM +0000, Biju Das wrote:
> 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()
> >
> > 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.

I think you mean WoL rather than suspend-to-RAM, although I don't see
anything in micrel.c that hints that WoL is supported, so please explain
why and how the PHY interrupt impacts suspend-to-RAM.

Note that a particular interrupt should not wake the system unless
enable_irq_wake() has been called for that specific interrupt.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
  2026-04-09 11:05         ` Russell King (Oracle)
@ 2026-04-09 11:19           ` Biju Das
  2026-04-09 11:30             ` Russell King (Oracle)
  0 siblings, 1 reply; 10+ messages in thread
From: Biju Das @ 2026-04-09 11:19 UTC (permalink / raw)
  To: Russell King
  Cc: Ovidiu Panait, andrew@lunn.ch, hkallweit1@gmail.com,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org

Hi Russell King,

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: 09 April 2026 12:05
> Subject: Re: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
> 
> On Thu, Apr 09, 2026 at 10:52:35AM +0000, Biju Das wrote:
> > 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()
> > >
> > > 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.
> 
> I think you mean WoL rather than suspend-to-RAM, although I don't see anything in micrel.c that hints
> that WoL is supported, so please explain why and how the PHY interrupt impacts suspend-to-RAM.

This is not WoL. During Suspend-to-RAM, the DDR goes into retention mode while
the CPU, SoC, and PHY power is cut off.

During resume, TF-A detects WARM_RESET, brings DDR out of retention, and jumps to
the PSCI resume path.

> 
> Note that a particular interrupt should not wake the system unless
> enable_irq_wake() has been called for that specific interrupt.

If PHY interrupts are not configured during resume, no link interrupt is received and the message:
"renesas-gbeth 11c30000.ethernet end0: Link is Up - 1Gbps/Full - flow control rx/tx"
is not seen, as shown in [1].

Cheers,
Biju

[1]
root@smarc-rzg3l:~# echo mem > /sys/power/state
[  184.611719] PM: suspend entry (deep)
[  184.616854] Filesystems sync: 0.000 seconds
[  184.629390] Freezing user space processes
[  184.637539] Freezing user space processes completed (elapsed 0.003 seconds)
[  184.644541] OOM killer disabled.
[  184.647758] Freezing remaining freezable tasks
[  184.653520] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
[  184.660941] printk: Suspending console(s) (use no_console_suspend to debug)
NOTICE:  BL2: v2.10.5(release):2.10.5/rz_soc_dev-383-g15a06c881
NOTICE:  BL2: Built : 12:13:18, Apr  2 2026
INFO:    BL2: Doing platform setup
INFO:    Configuring TrustZone Controller
INFO:    Total 3 regions set.
INFO:    Configuring TrustZone Controller
INFO:    Total 1 regions set.
INFO:    Configuring TrustZone Controller
INFO:    Total 1 regions set.
INFO:    eMMC boot from partition 1
INFO:    Loading image id=39 at address 0x44428
INFO:    emmcdrv_block_len: len: 0x00001000
INFO:    Load dst=0x44428 src=(p:1)0x260000(4864) len=0x1000(8)
INFO:    Image id=39 loaded: 0x44428 - 0x45428
INFO:    DDR: Retention Exit (Rev. 02.05)
NOTICE:  BL2: SYS_LSI_MODE: 0x12051
NOTICE:  BL2: SYS_LSI_DEVID: 0x87d9447
INFO:    BL2: Skip loading image id 3
INFO:    BL2: Skip loading image id 5
NOTICE:  BL2: Booting BL31
INFO:    Entry point address = 0x44000000
INFO:    SPSR = 0x3cd
[  184.670380] renesas-gbeth 11c30000.ethernet end0: Link is Down
[  184.674006] Disabling non-boot CPUs ...
[  184.675870] psci: CPU3 killed (polled 4 ms)
[  184.679357] psci: CPU2 killed (polled 0 ms)
[  184.683525] psci: CPU1 killed (polled 0 ms)
[  184.685755] Enabling non-boot CPUs ...
[  184.686014] Detected VIPT I-cache on CPU1
[  184.686070] GICv3: CPU1: found redistributor 100 region 0:0x0000000012460000
[  184.686119] CPU1: Booted secondary processor 0x0000000100 [0x412fd050]
[  184.687190] CPU1 is up
[  184.687348] Detected VIPT I-cache on CPU2
[  184.687384] GICv3: CPU2: found redistributor 200 region 0:0x0000000012480000
[  184.687419] CPU2: Booted secondary processor 0x0000000200 [0x412fd050]
[  184.688357] CPU2 is up
[  184.688534] Detected VIPT I-cache on CPU3
[  184.688573] GICv3: CPU3: found redistributor 300 region 0:0x00000000124a0000
[  184.688615] CPU3: Booted secondary processor 0x0000000300 [0x412fd050]
[  184.689702] CPU3 is up
[  184.692965] da7213 3-001a: Unable to sync registers 0x23-0x23. -6
[  184.767008] dwmac4: Master AXI performs fixed burst length
[  184.767049] renesas-gbeth 11c30000.ethernet end0: No Safety Features support found
[  184.767090] renesas-gbeth 11c30000.ethernet end0: IEEE 1588-2008 Advanced Timestamp supported
[  184.769791] renesas-gbeth 11c30000.ethernet end0: configuring for phy/rgmii-id link mode
[  184.839754] dwmac4: Master AXI performs fixed burst length
[  184.839784] renesas-gbeth 11c40000.ethernet end1: No Safety Features support found
[  184.839814] renesas-gbeth 11c40000.ethernet end1: IEEE 1588-2008 Advanced Timestamp supported
[  184.840892] renesas-gbeth 11c40000.ethernet end1: configuring for phy/rgmii-id link mode
[  184.994774] OOM killer enabled.
[  184.997922] Restarting tasks: Starting
[  185.002227] Restarting tasks: Done
[  185.005781] random: crng reseeded on system resumption
[  185.011124] PM: suspend exit
root@smarc-rzg3l:~#
root@smarc-rzg3l:~# [  187.356951] renesas-gbeth 11c30000.ethernet end0: Link is Up - 1Gbps/Full - flow control rx/tx



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
  2026-04-09 11:19           ` Biju Das
@ 2026-04-09 11:30             ` Russell King (Oracle)
  0 siblings, 0 replies; 10+ messages in thread
From: Russell King (Oracle) @ 2026-04-09 11:30 UTC (permalink / raw)
  To: Biju Das
  Cc: Ovidiu Panait, andrew@lunn.ch, hkallweit1@gmail.com,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org

On Thu, Apr 09, 2026 at 11:19:43AM +0000, Biju Das wrote:
> Hi Russell King,
> 
> > -----Original Message-----
> > From: Russell King <linux@armlinux.org.uk>
> > Sent: 09 April 2026 12:05
> > Subject: Re: [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume()
> > 
> > On Thu, Apr 09, 2026 at 10:52:35AM +0000, Biju Das wrote:
> > > 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()
> > > >
> > > > 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.
> > 
> > I think you mean WoL rather than suspend-to-RAM, although I don't see anything in micrel.c that hints
> > that WoL is supported, so please explain why and how the PHY interrupt impacts suspend-to-RAM.
> 
> This is not WoL. During Suspend-to-RAM, the DDR goes into retention mode while
> the CPU, SoC, and PHY power is cut off.
> 
> During resume, TF-A detects WARM_RESET, brings DDR out of retention, and jumps to
> the PSCI resume path.
> 
> > 
> > Note that a particular interrupt should not wake the system unless
> > enable_irq_wake() has been called for that specific interrupt.
> 
> If PHY interrupts are not configured during resume, no link interrupt is received and the message:
> "renesas-gbeth 11c30000.ethernet end0: Link is Up - 1Gbps/Full - flow control rx/tx"
> is not seen, as shown in [1].

... and why does that happen? Is it because the PHY has lost its
interrupt configuration and that needs to be reprogrammed?

If you don't disable the PHY interrupt in the suspend path, then
will the call to drv->config_intr() via phy_init_hw() before
phy_resume() be sufficient?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-04-09 11:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-09  9:56 [PATCH net v2 0/2] net: phylink: fix PHY reinitialization on resume Ovidiu Panait
2026-04-09  9:56 ` [PATCH net v2 1/2] net: phylink: call phy_init_hw() in phylink resume path Ovidiu Panait
2026-04-09 10:42   ` Russell King (Oracle)
2026-04-09  9:56 ` [PATCH net v2 2/2] net: phy: micrel: remove ksz9131_resume() Ovidiu Panait
2026-04-09 10:13   ` Biju Das
2026-04-09 10:29     ` Russell King (Oracle)
2026-04-09 10:52       ` Biju Das
2026-04-09 11:05         ` Russell King (Oracle)
2026-04-09 11:19           ` Biju Das
2026-04-09 11:30             ` Russell King (Oracle)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox