[PATCH 0/3] Fixes for stmmac driver

[PATCH 0/3] Fixes for stmmac driver

[Christophe Roullier]

First commit fix error when :
1 - Deactivated eth0 (ip link set eth0 down)
2 - Perform low power procedure
3 - Reactivate eth0 (ip link set eth0 up)
=> eth0: stmmac_hw_setup: DMA engine initialization failed
         stm32-dwmac 5800a000.ethernet eth0: stmmac_open: Hw setup failed

Second commit fix error when ethernet driver is unbind,
ethernet pinouts always set.

Third commit fix bad cleaning when error during stmmac_mdio_register.

Christophe Roullier (3):
  net: stmmac: fix pinctrl management during suspend/resume
  net: stmmac: restore pinctrl when driver remove.
  net: stmmac: manage error case during stmmac_dvr_probe

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH 1/3] net: stmmac: fix pinctrl management during suspend/resume

[Christophe Roullier]

In the deepest low power modes the pinctrl are lost and never restored
if the interface is down.
This commit makes sure that pinctrl modes are set in all the cases.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c63099a77cc0c..067b17f03cd09 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -8133,6 +8133,8 @@ int stmmac_suspend(struct device *dev)
 		ethtool_mmsv_stop(&priv->fpe_cfg.mmsv);
 
 suspend_bsp:
+	/* Select sleep pin state */
+	pinctrl_pm_select_sleep_state(dev);
 	if (priv->plat->suspend)
 		return priv->plat->suspend(dev, priv->plat->bsp_priv);
 
@@ -8194,8 +8196,11 @@ int stmmac_resume(struct device *dev)
 			return ret;
 	}
 
-	if (!netif_running(ndev))
+	if (!netif_running(ndev)) {
+		/* Select default pin state */
+		pinctrl_pm_select_default_state(priv->device);
 		return 0;
+	}
 
 	/* Power Down bit, into the PM register, is cleared
 	 * automatically as soon as a magic packet or a Wake-up frame
-- 
2.43.0

[PATCH 2/3] net: stmmac: restore pinctrl when driver remove.

[Christophe Roullier]

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

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 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);
+
 	pm_runtime_disable(dev);
 	pm_runtime_put_noidle(dev);
 
-- 
2.43.0

[PATCH 3/3] net: stmmac: manage error case during stmmac_dvr_probe

[Christophe Roullier]

In case of error during stmmac_mdio_register, pm_runtime is not
cleaning before exit probe.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3d4f0e4cb53fb..d8eff228f3971 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7999,6 +7999,8 @@ static int __stmmac_dvr_probe(struct device *device,
 error_pcs_setup:
 	stmmac_mdio_unregister(ndev);
 error_mdio_register:
+	pm_runtime_put_sync(device);
+	pm_runtime_disable(device);
 	stmmac_napi_del(ndev);
 error_hw_init:
 	destroy_workqueue(priv->wq);
-- 
2.43.0

Re: [PATCH 1/3] net: stmmac: fix pinctrl management during suspend/resume

[Russell King (Oracle)]

On Wed, Feb 18, 2026 at 09:36:16AM +0100, Christophe Roullier wrote:
> In the deepest low power modes the pinctrl are lost and never restored
> if the interface is down.
> This commit makes sure that pinctrl modes are set in all the cases.
> 
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index c63099a77cc0c..067b17f03cd09 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -8133,6 +8133,8 @@ int stmmac_suspend(struct device *dev)
>  		ethtool_mmsv_stop(&priv->fpe_cfg.mmsv);
>  
>  suspend_bsp:
> +	/* Select sleep pin state */
> +	pinctrl_pm_select_sleep_state(dev);

This is wrong. Look above, this, and you will notice that
pinctrl_pm_select_sleep_state() is called _conditionally_, depending
on whether the MAC is doing WoL or not. If WoL is enabled, the MAC
needs to stay functional, which means its pins need to stay properly
configured.

So, we can't unconditionally call this. I suggest moving this to:

-	if (!ndev || !netif_running(ndev)) {
+	if (!ndev || !netif_running(ndev)) {
+		/* Select sleep pin state */
+		pinctrl_pm_select_sleep_state(dev);
                goto suspend_bsp;
+	}

>  	if (priv->plat->suspend)
>  		return priv->plat->suspend(dev, priv->plat->bsp_priv);
>  
> @@ -8194,8 +8196,11 @@ int stmmac_resume(struct device *dev)
>  			return ret;
>  	}
>  
> -	if (!netif_running(ndev))
> +	if (!netif_running(ndev)) {
> +		/* Select default pin state */
> +		pinctrl_pm_select_default_state(priv->device);
>  		return 0;
> +	}

This looks fine to me.

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

Re: [PATCH 3/3] net: stmmac: manage error case during stmmac_dvr_probe

[Russell King (Oracle)]

On Wed, Feb 18, 2026 at 09:36:18AM +0100, Christophe Roullier wrote:
> In case of error during stmmac_mdio_register, pm_runtime is not
> cleaning before exit probe.
> 
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

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

Re: [PATCH 2/3] net: stmmac: restore pinctrl when driver remove.

[Russell King (Oracle)]

On Wed, Feb 18, 2026 at 09:36:17AM +0100, Christophe Roullier wrote:
> when system suspend or unbind, need to set pins
> to low power state to save IO power consumption.
> 
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
> ---
>  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.

Adding LinusW for comment.

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

Re: [PATCH 2/3] net: stmmac: restore pinctrl when driver remove.

[Linus Walleij]

On Wed, Feb 18, 2026 at 9:55 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Wed, Feb 18, 2026 at 09:36:17AM +0100, Christophe Roullier wrote:
> > when system suspend or unbind, need to set pins
> > to low power state to save IO power consumption.
> >
> > Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
> > ---
> >  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.

Yours,
Linus Walleij

Re: [PATCH 2/3] net: stmmac: restore pinctrl when driver remove.

[Russell King (Oracle)]

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)
> <linux@armlinux.org.uk> wrote:
> > On Wed, Feb 18, 2026 at 09:36:17AM +0100, Christophe Roullier wrote:
> > > when system suspend or unbind, need to set pins
> > > to low power state to save IO power consumption.
> > >
> > > Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
> > > ---
> > >  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!

Re: [PATCH 2/3] net: stmmac: restore pinctrl when driver remove.

[Linus Walleij]

On Thu, Feb 19, 2026 at 8:25 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Thu, Feb 19, 2026 at 08:11:13PM +0100, Linus Walleij wrote:

> > 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.

Indeed.

> 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?

I think you're right. Just very few device trees define the "sleep"
state (the infamous "so far so good...") so it has not been adressed.

> 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.

That's a good point.

It's perfectly legal to define any random state for the driver, such
as "relaxed" or "unused". It had the only downside that the device
driver core will not help out in looking up the state by default.

I'd be happy to take a patch that e.g. looks for an "unused"
state and put the pins into that state if probe fails, or when
remove() is called, so we get something everyone can use for
this.

Yours,
Linus Walleij