All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Marek Vasut <marex@nabladev.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	Christophe Roullier <christophe.roullier@st.com>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	kernel@dh-electronics.com, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com
Subject: Re: [net-next,PATCH] net: stmmac: stm32: Do not suspend downed interface
Date: Wed, 14 Jan 2026 17:12:01 +0000	[thread overview]
Message-ID: <aWfOYf_YmJFUakvP@shell.armlinux.org.uk> (raw)
In-Reply-To: <aWfEXX1iMHy3V5sK@shell.armlinux.org.uk>

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);
 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


  reply	other threads:[~2026-01-14 17:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-14  8:17 [net-next,PATCH] net: stmmac: stm32: Do not suspend downed interface Marek Vasut
2026-01-14 15:49 ` Andrew Lunn
2026-01-14 16:29 ` Russell King (Oracle)
2026-01-14 17:12   ` Russell King (Oracle) [this message]
2026-01-14 23:27     ` Marek Vasut
2026-01-30 15:52       ` Russell King (Oracle)
2026-02-01 18:20         ` Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aWfOYf_YmJFUakvP@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=christophe.roullier@st.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kernel@dh-electronics.com \
    --cc=krzk@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=marex@nabladev.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.