Netdev List
 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
  0 siblings, 1 reply; 3+ 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] 3+ 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
  0 siblings, 1 reply; 3+ 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] 3+ 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; 3+ 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] 3+ messages in thread

end of thread, other threads:[~2026-07-07 21:45 UTC | newest]

Thread overview: 3+ 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

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