* [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