netdev - Re: [net-next,PATCH] net: stmmac: stm32: Do not suspend downed interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6c9cadc2-67b7-4bfe-9cf7-2b102a0a3c21@nabladev.com>
Date: Thu, 15 Jan 2026 00:27:05 +0100
From: Marek Vasut <marex@...ladev.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Andrew Lunn <andrew+netdev@...n.ch>,
 Christophe Roullier <christophe.roullier@...com>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Krzysztof Kozlowski <krzk@...nel.org>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>, Paolo Abeni
 <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 kernel@...electronics.com, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com
Subject: Re: [net-next,PATCH] net: stmmac: stm32: Do not suspend downed
 interface

On 1/14/26 6:12 PM, Russell King (Oracle) wrote:
> On Wed, Jan 14, 2026 at 04:29:17PM +0000, Russell King (Oracle) wrote:
>> On Wed, Jan 14, 2026 at 09:17:54AM +0100, Marek Vasut wrote:
>>> If an interface is down, the ETHnSTP clock are not running. Suspending
>>> such an interface will attempt to stop already stopped ETHnSTP clock,
>>> and produce a warning in the kernel log about this.
>>>
>>> STM32MP25xx that is booted from NFS root via its first ethernet MAC
>>> (also the consumer of ck_ker_eth1stp) and with its second ethernet
>>> MAC downed produces the following warnings during suspend resume
>>> cycle. This can be provoked even using pm_test:
>>>
>>> "
>>> $ echo devices > /sys/power/pm_test
>>> $ echo mem > /sys/power/state
>>> ...
>>> ck_ker_eth2stp already disabled
>>> ...
>>> ck_ker_eth2stp already unprepared
>>> ...
>>> "
>>>
>>> Fix this by not manipulating with the clock during suspend resume
>>> of interfaces which are downed.
>>
>> I don't think this is the correct fix. Looking back at my commits:
>> b51f34bc85e3 net: stmmac: platform: legacy hooks for suspend()/resume() methods
>> 07bbbfe7addf net: stmmac: add suspend()/resume() platform ops
>>
>> I think I changed the behaviour of the suspend/resume callbacks
>> unintentionally. Sorry, I don't have time to complete this email
>> (meeting.)
> 
> I think I'm going to start over, trying to figure out what happened.
> 
> c7308b2f3d0d net: stmmac: stm32: convert to suspend()/resume() methods
> 
> Did the conversion, and it always called stm32_dwmac_clk_disable() and
> where it exists, dwmac->ops->suspend() on suspend, provided
> stmmac_suspend() returns zero (which it will do, even if the interface
> is down. On resume, it always calls dwmac->ops->resume() and
> stm32_dwmac_init() before calling stmmac_resume().
> 
> The conversion added hooks into ny new ->suspend() and ->resume()
> methods to handle the stm32_dwmac_clk_disable(), dwmac->ops->suspend(),
> dwmac->ops->resume() and stm32_dwmac_init() steps.
> 
> However, in 07bbbfe7addf I failed to realise that, in order to keep
> things compatible with how stuff works, we need to call
> priv->plat->suspend() even if the interface is down. This is where
> the bug is, not in your glue driver.
> 
> Please try this:
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index a8a78fe7d01f..2acbb0107cd3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -8066,7 +8066,7 @@ int stmmac_suspend(struct device *dev)
>   	u32 chan;
>   
>   	if (!ndev || !netif_running(ndev))
> -		return 0;
> +		goto suspend_bsp;
>   
>   	mutex_lock(&priv->lock);
>   
> @@ -8106,6 +8106,7 @@ int stmmac_suspend(struct device *dev)
>   	if (stmmac_fpe_supported(priv))
>   		ethtool_mmsv_stop(&priv->fpe_cfg.mmsv);
>   
> +suspend_bsp:
>   	if (priv->plat->suspend)
>   		return priv->plat->suspend(dev, priv->plat->bsp_priv);
>   
This works too, thank you.

Will you send this fix ?