On Thu, Feb 19, 2026 at 08:11:13PM +0100, Linus Walleij wrote:

On Wed, Feb 18, 2026 at 9:55 AM Russell King (Oracle)
[off-list ref] wrote:

quoted

On Wed, Feb 18, 2026 at 09:36:17AM +0100, Christophe Roullier wrote:

quoted

when system suspend or unbind, need to set pins
to low power state to save IO power consumption.

Signed-off-by: Christophe Roullier <redacted>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 067b17f03cd09..3d4f0e4cb53fb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

@@ -8071,6 +8071,9 @@ void stmmac_dvr_remove(struct device *dev)
      mutex_destroy(&priv->lock);
      bitmap_free(priv->af_xdp_zc_qps);

+     /* Select sleep pin state */
+     pinctrl_pm_select_sleep_state(dev);
+

I'm not convinced this is correct, there's nothing to match it in the
probe function, except what the driver model core does. However, the
driver model core also doesn't clean up the state if probe fails.

I think it looks right, if this state is indeed for the sleep state of the
device pins, and this is what you want to happen at remove().

The non-cleanup of the pin states is a (maybe ugly) feature: there
is an "init" and a "default" state. If the "init" state does not exist the
"default" state is selected in the dd.c call.

These states are just some values in dev->pins, allocated with
devm_kzalloc(), and devm_pinctrl_get() for e.g. dev->pins->p,
so IIUC this will befree:ed on driver detach,
also if the probe() fails, at least that is what the original devres
design document says
Documentation/driver-api/driver-model/devres.rst

There is really no other cleanup that can happen: there is no
before-default-or-init state we can revert to (that would be the
power-on values), so there are just these states in some pointers
that could be accessed by e.g. pinctrl_pm_select_sleep_state()
that get free:ed up, and muxing and pin config that happened
in the pin control hardware just stays around.

What we seem to end up with is that on probe failure, we set the pins
to init or default state. However, on remove, it's up to the driver to
decide whether to put the pins into sleep state or not.

Shouldn't a driver be consistent, and place the pins into the same
state in both these scenarios?

Conversely, I'm slightly worried that putting pins into sleep state
when we're not powering down may be bad if they're allowed to then
float, which could cause the PHY to interfere on the network.

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