Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface
       [not found] <message-id-of-your-v1-mail>
@ 2026-07-07 19:54 ` Stefan Agner
  2026-07-07 21:13   ` Jakub Raczynski
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Stefan Agner @ 2026-07-07 19:54 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn
  Cc: Russell King (Oracle), Maxime Chevallier, Ovidiu Panait,
	Maxime Coquelin, Alexandre Torgue, netdev, linux-stm32,
	linux-arm-kernel, regressions, Stefan Agner

Since the referenced commit, changing the MTU on a running interface no
longer disconnects and reconnects the PHY; __stmmac_release() merely
stops phylink, which also suspends the PHY (BMCR power-down) when WoL
is not enabled. __stmmac_open() then performs the DMA software reset in
stmmac_hw_setup() before phylink_start() resumes the PHY again.

IEEE 802.3 22.2.4.1.5 allows a PHY to stop its receive clock while
powered down, and stmmac requires a running receive clock for the DMA
software reset to complete (the phylink config sets mac_requires_rxc).
On such setups, e.g. the RK3566-based Home Assistant Green with an
RTL8211F-VD PHY in RGMII mode, any runtime MTU change now times out and
leaves the interface dead:

  rk_gmac-dwmac fe010000.ethernet end0: Failed to reset the dma
  rk_gmac-dwmac fe010000.ethernet end0: stmmac_hw_setup: DMA engine initialization failed
  rk_gmac-dwmac fe010000.ethernet end0: __stmmac_open: Hw setup failed
  rk_gmac-dwmac fe010000.ethernet end0: failed reopening the interface after MTU change

In the field this is triggered by NetworkManager applying an MTU while
activating the connection, breaking networking entirely.

Resume the PHY in __stmmac_open() before the hardware setup, making it
the counterpart of the phylink_stop() in __stmmac_release(), like
stmmac_resume() already does for the same reason. phylink_start() also
resumes the PHY, but only after stmmac_hw_setup(), and it cannot be
moved before the hardware setup since it may bring the link up
immediately from a workqueue, racing with the initialization (see the
comment in stmmac_resume()). For the regular ndo_open path the PHY has
just been attached and is not suspended, in which case
phylink_prepare_resume() does nothing.

Fixes: db299a0c09e9 ("net: stmmac: move PHY handling out of __stmmac_open()/release()")
Link: https://github.com/home-assistant/operating-system/issues/4858
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Changes in v2:
- Move the PHY resume from stmmac_change_mtu() into __stmmac_open() so
  that it also counters the PHY suspend caused by __stmmac_release()
  (suggested by Andrew Lunn), placed before stmmac_reset_queues_param()
  to match the ordering used in stmmac_resume()

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4147,6 +4147,15 @@
 			dma_conf->tx_queue[i].tbs = priv->dma_conf.tx_queue[i].tbs;
 	memcpy(&priv->dma_conf, dma_conf, sizeof(*dma_conf));
 
+	/* The PHY is suspended when the interface is reopened without
+	 * disconnecting the PHY, e.g. on MTU change. IEEE 802.3 allows PHYs
+	 * to stop their receive clock while powered down, but the DMA
+	 * software reset in stmmac_hw_setup() requires a running receive
+	 * clock, and phylink_start() below resumes the PHY only after the
+	 * hardware setup. Resume a suspended PHY here first.
+	 */
+	phylink_prepare_resume(priv->phylink);
+
 	stmmac_reset_queues_param(priv);
 
 	ret = stmmac_hw_setup(dev);
-- 
2.49.0


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

* Re: [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface
  2026-07-07 19:54 ` [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface Stefan Agner
@ 2026-07-07 21:13   ` Jakub Raczynski
  2026-07-07 21:45     ` Jakub Raczynski
  2026-07-08  8:00   ` Alexander Stein
  2026-07-08  9:16   ` Maxime Chevallier
  2 siblings, 1 reply; 5+ messages in thread
From: Jakub Raczynski @ 2026-07-07 21:13 UTC (permalink / raw)
  To: Stefan Agner
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn, Russell King (Oracle), Maxime Chevallier,
	Ovidiu Panait, Maxime Coquelin, Alexandre Torgue, netdev,
	linux-stm32, linux-arm-kernel, regressions

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]

Please read
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
in this case 'don’t repost your patches within one 24h period'

Because:
- you have two patches now processing and no changes requested yet,
  nor did you get AI review that is currently employed
  https://patchwork.kernel.org/project/netdevbpf/patch/20260707195425.405989-1-stefan@agner.ch/
  and
  https://patchwork.kernel.org/project/netdevbpf/patch/20260707162146.73823-1-stefan@agner.ch/
- This change is actually broken - in previous patch rtnl_lock() was asserted
  by dev_ioctl(), but in this version it is not asserted anywhere and will
  trigger WARN_ONCE().
- Full tests within an hour is possible, but no need to rush patches
- Please spend time reviewing patches, as I spent few minutes to make sure
  you (or rather Fable AI) is correct that rtnl_lock() was asserted, so my
  review would be accurate (previous patch was ok, this is not).
- Please respond yourself to comments, take time to understand codebase.
  I would prefer not getting copy-paste answer from AI,
  we have Sashiko review for that.
- Missing link to previous thread, shown below
- Moving code to other lines does not justify dropping "Assisted by AI"

I am not maintainer, just random reviewer. Some maintainer will give input
surely soon.

But rule of thumb, do not repost patches till your patch gets
'Changes requested' in patchwork or like 2 weeks have passed
(maybe during vacation even longer, as currently there are >600 patches
pending review).

On Tue, Jul 07, 2026 at 09:54:25PM +0200, Stefan Agner wrote:
> Changes in v2:
> - Move the PHY resume from stmmac_change_mtu() into __stmmac_open() so
>   that it also counters the PHY suspend caused by __stmmac_release()
>   (suggested by Andrew Lunn), placed before stmmac_reset_queues_param()
>   to match the ordering used in stmmac_resume()

Missing link to previous thread

> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>

BR
Jakub Raczynski 

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



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

* Re: [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface
  2026-07-07 21:13   ` Jakub Raczynski
@ 2026-07-07 21:45     ` Jakub Raczynski
  0 siblings, 0 replies; 5+ messages in thread
From: Jakub Raczynski @ 2026-07-07 21:45 UTC (permalink / raw)
  To: Stefan Agner
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn, Russell King (Oracle), Maxime Chevallier,
	Ovidiu Panait, Maxime Coquelin, Alexandre Torgue, netdev,
	linux-stm32, linux-arm-kernel, regressions

[-- Attachment #1: Type: text/plain, Size: 450 bytes --]

On Tue, Jul 07, 2026 at 11:13:46PM +0200, Jakub Raczynski wrote:
> - This change is actually broken - in previous patch rtnl_lock() was asserted
>   by dev_ioctl(), but in this version it is not asserted anywhere and will
>   trigger WARN_ONCE().

Ok, as I looked again, I am wrong again, ndo_open() does hold rtnl_lock
already. So from this point it is good. Relocking would create deadlock,
so I take that part of comment back.

BR
Jakub Raczynski

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



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

* Re: [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface
  2026-07-07 19:54 ` [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface Stefan Agner
  2026-07-07 21:13   ` Jakub Raczynski
@ 2026-07-08  8:00   ` Alexander Stein
  2026-07-08  9:16   ` Maxime Chevallier
  2 siblings, 0 replies; 5+ messages in thread
From: Alexander Stein @ 2026-07-08  8:00 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Andrew Lunn, Stefan Agner
  Cc: Russell King (Oracle), Maxime Chevallier, Ovidiu Panait,
	Maxime Coquelin, Alexandre Torgue, netdev, linux-stm32,
	linux-arm-kernel, regressions, Stefan Agner

Hi Stefan,

thanks for the patch.

Am Dienstag, 7. Juli 2026, 21:54:25 CEST schrieb Stefan Agner:
> Since the referenced commit, changing the MTU on a running interface no
> longer disconnects and reconnects the PHY; __stmmac_release() merely
> stops phylink, which also suspends the PHY (BMCR power-down) when WoL
> is not enabled. __stmmac_open() then performs the DMA software reset in
> stmmac_hw_setup() before phylink_start() resumes the PHY again.
> 
> IEEE 802.3 22.2.4.1.5 allows a PHY to stop its receive clock while
> powered down, and stmmac requires a running receive clock for the DMA
> software reset to complete (the phylink config sets mac_requires_rxc).
> On such setups, e.g. the RK3566-based Home Assistant Green with an
> RTL8211F-VD PHY in RGMII mode, any runtime MTU change now times out and
> leaves the interface dead:
> 
>   rk_gmac-dwmac fe010000.ethernet end0: Failed to reset the dma
>   rk_gmac-dwmac fe010000.ethernet end0: stmmac_hw_setup: DMA engine initialization failed
>   rk_gmac-dwmac fe010000.ethernet end0: __stmmac_open: Hw setup failed
>   rk_gmac-dwmac fe010000.ethernet end0: failed reopening the interface after MTU change
> 
> In the field this is triggered by NetworkManager applying an MTU while
> activating the connection, breaking networking entirely.
> 
> Resume the PHY in __stmmac_open() before the hardware setup, making it
> the counterpart of the phylink_stop() in __stmmac_release(), like
> stmmac_resume() already does for the same reason. phylink_start() also
> resumes the PHY, but only after stmmac_hw_setup(), and it cannot be
> moved before the hardware setup since it may bring the link up
> immediately from a workqueue, racing with the initialization (see the
> comment in stmmac_resume()). For the regular ndo_open path the PHY has
> just been attached and is not suspended, in which case
> phylink_prepare_resume() does nothing.
> 
> Fixes: db299a0c09e9 ("net: stmmac: move PHY handling out of __stmmac_open()/release()")
> Link: https://github.com/home-assistant/operating-system/issues/4858
> Signed-off-by: Stefan Agner <stefan@agner.ch>

FWIW imx8mp based platforms (TQMa8MPxL in my case) are also affected, see [1].

This patch fixes the problem here as well, so:
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>

[1] https://lore.kernel.org/all/8409022.LvFx2qVVIh@steina-w/

> ---
> Changes in v2:
> - Move the PHY resume from stmmac_change_mtu() into __stmmac_open() so
>   that it also counters the PHY suspend caused by __stmmac_release()
>   (suggested by Andrew Lunn), placed before stmmac_reset_queues_param()
>   to match the ordering used in stmmac_resume()
> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4147,6 +4147,15 @@
>  			dma_conf->tx_queue[i].tbs = priv->dma_conf.tx_queue[i].tbs;
>  	memcpy(&priv->dma_conf, dma_conf, sizeof(*dma_conf));
>  
> +	/* The PHY is suspended when the interface is reopened without
> +	 * disconnecting the PHY, e.g. on MTU change. IEEE 802.3 allows PHYs
> +	 * to stop their receive clock while powered down, but the DMA
> +	 * software reset in stmmac_hw_setup() requires a running receive
> +	 * clock, and phylink_start() below resumes the PHY only after the
> +	 * hardware setup. Resume a suspended PHY here first.
> +	 */
> +	phylink_prepare_resume(priv->phylink);
> +
>  	stmmac_reset_queues_param(priv);
>  
>  	ret = stmmac_hw_setup(dev);
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/




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

* Re: [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface
  2026-07-07 19:54 ` [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface Stefan Agner
  2026-07-07 21:13   ` Jakub Raczynski
  2026-07-08  8:00   ` Alexander Stein
@ 2026-07-08  9:16   ` Maxime Chevallier
  2 siblings, 0 replies; 5+ messages in thread
From: Maxime Chevallier @ 2026-07-08  9:16 UTC (permalink / raw)
  To: Stefan Agner, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn
  Cc: Russell King (Oracle), Ovidiu Panait, Maxime Coquelin,
	Alexandre Torgue, netdev, linux-stm32, linux-arm-kernel,
	regressions

Hi Stefan,

On 7/7/26 21:54, Stefan Agner wrote:
> Since the referenced commit, changing the MTU on a running interface no
> longer disconnects and reconnects the PHY; __stmmac_release() merely
> stops phylink, which also suspends the PHY (BMCR power-down) when WoL
> is not enabled. __stmmac_open() then performs the DMA software reset in
> stmmac_hw_setup() before phylink_start() resumes the PHY again.
> 
> IEEE 802.3 22.2.4.1.5 allows a PHY to stop its receive clock while
> powered down, and stmmac requires a running receive clock for the DMA
> software reset to complete (the phylink config sets mac_requires_rxc).
> On such setups, e.g. the RK3566-based Home Assistant Green with an
> RTL8211F-VD PHY in RGMII mode, any runtime MTU change now times out and
> leaves the interface dead:
> 
>   rk_gmac-dwmac fe010000.ethernet end0: Failed to reset the dma
>   rk_gmac-dwmac fe010000.ethernet end0: stmmac_hw_setup: DMA engine initialization failed
>   rk_gmac-dwmac fe010000.ethernet end0: __stmmac_open: Hw setup failed
>   rk_gmac-dwmac fe010000.ethernet end0: failed reopening the interface after MTU change
> 
> In the field this is triggered by NetworkManager applying an MTU while
> activating the connection, breaking networking entirely.
> 
> Resume the PHY in __stmmac_open() before the hardware setup, making it
> the counterpart of the phylink_stop() in __stmmac_release(), like
> stmmac_resume() already does for the same reason. phylink_start() also
> resumes the PHY, but only after stmmac_hw_setup(), and it cannot be
> moved before the hardware setup since it may bring the link up
> immediately from a workqueue, racing with the initialization (see the
> comment in stmmac_resume()). For the regular ndo_open path the PHY has
> just been attached and is not suspended, in which case
> phylink_prepare_resume() does nothing.
> 
> Fixes: db299a0c09e9 ("net: stmmac: move PHY handling out of __stmmac_open()/release()")
> Link: https://github.com/home-assistant/operating-system/issues/4858
> Signed-off-by: Stefan Agner <stefan@agner.ch>

I was able to reproduce the issue on imx8mp and socfpga. Adding this case to
my periodic test list...

Indeed the assymetry isn't very nice, OTOH there's not phylink counterpart
for phylink_prepare_resume(). This helper was added for the suspend/resume
case, and only for it as the doc states, it just happens to do exactly
what we need to fix the issue :

/**
 * phylink_prepare_resume() - prepare to resume a network device
 * @pl: a pointer to a &struct phylink returned from phylink_create()
 *
 * Optional, but if called must be called prior to phylink_resume().
 *
 * Prepare to resume a network device, preparing the PHY as necessary.
 */
void phylink_prepare_resume(struct phylink *pl)

I think this helper should be renamed and the doc updated, stmmac is the
sole user, and it's really about controlling that rxc and not about
suspend / resume. Resuming from suspend is just one of the cases where we
need that RXC early on.

So either something like 'phylink_prepare_start_or_resume' but it's long,
or maybe we can be more explicit about it and simply call it:

phylink_start_rxc(pl)

(without a corresponding stop)

Maxime



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

end of thread, other threads:[~2026-07-08  9:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <message-id-of-your-v1-mail>
2026-07-07 19:54 ` [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface Stefan Agner
2026-07-07 21:13   ` Jakub Raczynski
2026-07-07 21:45     ` Jakub Raczynski
2026-07-08  8:00   ` Alexander Stein
2026-07-08  9:16   ` Maxime Chevallier

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