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