[PATCH v2 0/2] Fixes for stmmac driver

[PATCH v2 0/2] 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 bad cleaning when error during stmmac_mdio_register.

V2: - put Reviewed-by from Russell for second commit.
    - update first commit with Russell'remark.
    - Dropped the third patch from v1. It requires additional work on the
      pinctrl framework to add a new state, and I don’t want this to
      block the other two patches.
      I do not intend to drop this change; I will follow up on it in a
      separate thread together with Antonio Borneo.

Christophe Roullier (2):
  net: stmmac: fix pinctrl management during suspend/resume
  net: stmmac: manage error case during stmmac_dvr_probe

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

-- 
2.43.0

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

[Christophe Roullier]

In the deepest low-power modes, the pinctrl configuration is lost
and is never restored if the interface is down.
This commit ensures that the pinctrl state is set in all cases.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 11150bddd8726..26ac1cdc561c2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -8150,8 +8150,11 @@ int stmmac_suspend(struct device *dev)
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	u8 chan;
 
-	if (!ndev || !netif_running(ndev))
+	if (!ndev || !netif_running(ndev)) {
+		/* Select sleep pin state */
+		pinctrl_pm_select_sleep_state(dev);
 		goto suspend_bsp;
+	}
 
 	mutex_lock(&priv->lock);
 
@@ -8252,8 +8255,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

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

[Russell King (Oracle)]

On Fri, Mar 13, 2026 at 11:57:16AM +0100, Christophe Roullier wrote:
> In the deepest low-power modes, the pinctrl configuration is lost
> and is never restored if the interface is down.
> This commit ensures that the pinctrl state is set in all cases.

Shouldn't the pin state be restored by the pinctrl layer?

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

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

[Linus Walleij]

On Fri, Mar 13, 2026 at 12:08 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Fri, Mar 13, 2026 at 11:57:16AM +0100, Christophe Roullier wrote:
> > In the deepest low-power modes, the pinctrl configuration is lost
> > and is never restored if the interface is down.
> > This commit ensures that the pinctrl state is set in all cases.
>
> Shouldn't the pin state be restored by the pinctrl layer?

What we have in the device core only applies "init" and "default"
states, and provides these handles for transitioning to "sleep"
and "default" again (like a state machine).

I would love to handle this in the PM core, but devices are a
bit sensitive to where this is applied wrt e.g. some register
writes that need to happen before/after this pin transition etc
so it's called explicitly like this from each drivers suspend()
and resume() hooks.

So FWIW:
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

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

[Russell King (Oracle)]

On Sat, Mar 14, 2026 at 12:44:56AM +0100, Linus Walleij wrote:
> On Fri, Mar 13, 2026 at 12:08 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> > On Fri, Mar 13, 2026 at 11:57:16AM +0100, Christophe Roullier wrote:
> > > In the deepest low-power modes, the pinctrl configuration is lost
> > > and is never restored if the interface is down.
> > > This commit ensures that the pinctrl state is set in all cases.
> >
> > Shouldn't the pin state be restored by the pinctrl layer?
> 
> What we have in the device core only applies "init" and "default"
> states, and provides these handles for transitioning to "sleep"
> and "default" again (like a state machine).

What I was meaning is that - for a driver using the "default" state,
if the hardware loses the pinctrl state during sleep, isn't it the
responsibility of the pinctrl driver to restore the state rather
than leaving it in whatever states it happens to be when the SoC
comes back from suspend?

If that is not the case, then don't we have a major issue where
drivers using pinctrl but do not issue any pinctrl calls in the
resume function are buggy?

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

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

[Russell King (Oracle)]

On Sat, Mar 14, 2026 at 12:37:19AM +0000, Russell King (Oracle) wrote:
> On Sat, Mar 14, 2026 at 12:44:56AM +0100, Linus Walleij wrote:
> > On Fri, Mar 13, 2026 at 12:08 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > > On Fri, Mar 13, 2026 at 11:57:16AM +0100, Christophe Roullier wrote:
> > > > In the deepest low-power modes, the pinctrl configuration is lost
> > > > and is never restored if the interface is down.
> > > > This commit ensures that the pinctrl state is set in all cases.
> > >
> > > Shouldn't the pin state be restored by the pinctrl layer?
> > 
> > What we have in the device core only applies "init" and "default"
> > states, and provides these handles for transitioning to "sleep"
> > and "default" again (like a state machine).
> 
> What I was meaning is that - for a driver using the "default" state,
> if the hardware loses the pinctrl state during sleep, isn't it the
> responsibility of the pinctrl driver to restore the state rather
> than leaving it in whatever states it happens to be when the SoC
> comes back from suspend?
> 
> If that is not the case, then don't we have a major issue where
> drivers using pinctrl but do not issue any pinctrl calls in the
> resume function are buggy?

I would like an answer on this before this patch is merged, because
even with your reviewed-by, I don't think this patch is correct.

For example, if pinctrl loses the pinmux state across suspend/resume,
then this patch only solves the case where the NIC is down when
suspending.

It does not address the case where the NIC is up but WoL is disabled.
Also, what happens when WoL is enabled at the MAC, when we expect the
NIC to still be functional - which means that the pinmux state must
remain active over suspend.

This is in addition to a more general concern that almost every driver
in the kernel is likely broken if we need to switch pinmux modes on
resume to ensure that the "default" pinmux state is restored upon
resume, which seems to be what you're saying by giving a r-b for this
patch.

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

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

[Christophe ROULLIER]

Hi Russell, Linus, All,

Le 16/03/2026 à 10:03, Russell King (Oracle) a écrit :
> On Sat, Mar 14, 2026 at 12:37:19AM +0000, Russell King (Oracle) wrote:
>> On Sat, Mar 14, 2026 at 12:44:56AM +0100, Linus Walleij wrote:
>>> On Fri, Mar 13, 2026 at 12:08 PM Russell King (Oracle)
>>> <linux@armlinux.org.uk> wrote:
>>>> On Fri, Mar 13, 2026 at 11:57:16AM +0100, Christophe Roullier wrote:
>>>>> In the deepest low-power modes, the pinctrl configuration is lost
>>>>> and is never restored if the interface is down.
>>>>> This commit ensures that the pinctrl state is set in all cases.
>>>> Shouldn't the pin state be restored by the pinctrl layer?
>>> What we have in the device core only applies "init" and "default"
>>> states, and provides these handles for transitioning to "sleep"
>>> and "default" again (like a state machine).
>> What I was meaning is that - for a driver using the "default" state,
>> if the hardware loses the pinctrl state during sleep, isn't it the
>> responsibility of the pinctrl driver to restore the state rather
>> than leaving it in whatever states it happens to be when the SoC
>> comes back from suspend?
>>
>> If that is not the case, then don't we have a major issue where
>> drivers using pinctrl but do not issue any pinctrl calls in the
>> resume function are buggy?
> I would like an answer on this before this patch is merged, because
> even with your reviewed-by, I don't think this patch is correct.
>
> For example, if pinctrl loses the pinmux state across suspend/resume,
> then this patch only solves the case where the NIC is down when
> suspending.
>
> It does not address the case where the NIC is up but WoL is disabled.
> Also, what happens when WoL is enabled at the MAC, when we expect the
> NIC to still be functional - which means that the pinmux state must
> remain active over suspend.

For me this case (when NIC is up) is already managed by the driver and 
function suspend/resume:

On stmmac_suspend :

==>      /* Enable Power down mode by programming the PMT regs */
      if (priv->wolopts) {
          stmmac_pmt(priv, priv->hw, priv->wolopts);
          priv->irq_wake = 1;
      } else {
          stmmac_mac_set(priv, priv->ioaddr, false);
*         pinctrl_pm_select_sleep_state(priv->device);*
      }

On stmmac_resume :

==>     if (priv->wolopts) {
          mutex_lock(&priv->lock);
          stmmac_pmt(priv, priv->hw, 0);
          mutex_unlock(&priv->lock);
          priv->irq_wake = 0;
      } else {
*         pinctrl_pm_select_default_state(priv->device);*
          /* reset the phy so that it's ready */
          if (priv->mii)
              stmmac_mdio_reset(priv->mii);
      }

BR

Christophe.

>
> This is in addition to a more general concern that almost every driver
> in the kernel is likely broken if we need to switch pinmux modes on
> resume to ensure that the "default" pinmux state is restored upon
> resume, which seems to be what you're saying by giving a r-b for this
> patch.
>

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

[Russell King (Oracle)]

On Mon, Mar 16, 2026 at 11:01:02AM +0100, Christophe ROULLIER wrote:
> Hi Russell, Linus, All,
> 
> Le 16/03/2026 à 10:03, Russell King (Oracle) a écrit :
> > On Sat, Mar 14, 2026 at 12:37:19AM +0000, Russell King (Oracle) wrote:
> > > On Sat, Mar 14, 2026 at 12:44:56AM +0100, Linus Walleij wrote:
> > > > On Fri, Mar 13, 2026 at 12:08 PM Russell King (Oracle)
> > > > <linux@armlinux.org.uk> wrote:
> > > > > On Fri, Mar 13, 2026 at 11:57:16AM +0100, Christophe Roullier wrote:
> > > > > > In the deepest low-power modes, the pinctrl configuration is lost
> > > > > > and is never restored if the interface is down.
> > > > > > This commit ensures that the pinctrl state is set in all cases.
> > > > > Shouldn't the pin state be restored by the pinctrl layer?
> > > > What we have in the device core only applies "init" and "default"
> > > > states, and provides these handles for transitioning to "sleep"
> > > > and "default" again (like a state machine).
> > > What I was meaning is that - for a driver using the "default" state,
> > > if the hardware loses the pinctrl state during sleep, isn't it the
> > > responsibility of the pinctrl driver to restore the state rather
> > > than leaving it in whatever states it happens to be when the SoC
> > > comes back from suspend?
> > > 
> > > If that is not the case, then don't we have a major issue where
> > > drivers using pinctrl but do not issue any pinctrl calls in the
> > > resume function are buggy?
> > I would like an answer on this before this patch is merged, because
> > even with your reviewed-by, I don't think this patch is correct.
> > 
> > For example, if pinctrl loses the pinmux state across suspend/resume,
> > then this patch only solves the case where the NIC is down when
> > suspending.
> > 
> > It does not address the case where the NIC is up but WoL is disabled.
> > Also, what happens when WoL is enabled at the MAC, when we expect the
> > NIC to still be functional - which means that the pinmux state must
> > remain active over suspend.
> 
> For me this case (when NIC is up) is already managed by the driver and
> function suspend/resume:
> 
> On stmmac_suspend :
> 
> ==>      /* Enable Power down mode by programming the PMT regs */
>      if (priv->wolopts) {
>          stmmac_pmt(priv, priv->hw, priv->wolopts);
>          priv->irq_wake = 1;
>      } else {
>          stmmac_mac_set(priv, priv->ioaddr, false);
> *         pinctrl_pm_select_sleep_state(priv->device);*
>      }
> 
> On stmmac_resume :
> 
> ==>     if (priv->wolopts) {
>          mutex_lock(&priv->lock);
>          stmmac_pmt(priv, priv->hw, 0);
>          mutex_unlock(&priv->lock);
>          priv->irq_wake = 0;
>      } else {
> *         pinctrl_pm_select_default_state(priv->device);*
>          /* reset the phy so that it's ready */
>          if (priv->mii)
>              stmmac_mdio_reset(priv->mii);
>      }
> 

I don't know if there's a difference between your two seperate replies
that appear to be saying the same thing.

I also didn't realise that we're already changing the pinctrl state
on suspend/resume when the interface is up - thanks for pointing that
out.

I think it may be better to move the pinctrl changes after the
priv->plat->suspend() call:

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 428b2e5bb4c4..596f9e2349cb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -8174,6 +8174,7 @@ int stmmac_suspend(struct device *dev)
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	u8 chan;
+	int ret;
 
 	if (!ndev || !netif_running(ndev))
 		goto suspend_bsp;
@@ -8203,7 +8204,6 @@ int stmmac_suspend(struct device *dev)
 		priv->irq_wake = 1;
 	} else {
 		stmmac_mac_set(priv, priv->ioaddr, false);
-		pinctrl_pm_select_sleep_state(priv->device);
 	}
 
 	mutex_unlock(&priv->lock);
@@ -8225,8 +8225,14 @@ int stmmac_suspend(struct device *dev)
 		ethtool_mmsv_stop(&priv->fpe_cfg.mmsv);
 
 suspend_bsp:
-	if (priv->plat->suspend)
-		return priv->plat->suspend(dev, priv->plat->bsp_priv);
+	if (priv->plat->suspend) {
+		ret = priv->plat->suspend(dev, priv->plat->bsp_priv);
+		if (ret)
+			return ret;
+	}
+
+	if (!priv->wolopts)
+		pinctrl_pm_select_sleep_state(priv->device);
 
 	return 0;
 }
@@ -8280,6 +8286,9 @@ int stmmac_resume(struct device *dev)
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	int ret;
 
+	if (!priv->wolopts)
+		pinctrl_pm_select_default_state(priv->device);
+
 	if (priv->plat->resume) {
 		ret = priv->plat->resume(dev, priv->plat->bsp_priv);
 		if (ret)
@@ -8301,7 +8310,6 @@ int stmmac_resume(struct device *dev)
 		mutex_unlock(&priv->lock);
 		priv->irq_wake = 0;
 	} else {
-		pinctrl_pm_select_default_state(priv->device);
 		/* reset the phy so that it's ready */
 		if (priv->mii)
 			stmmac_mdio_reset(priv->mii);

This means that we don't switch the pinctrl state until we've finished
suspending the device as necessary, which seems way more sane than
trying to do it part-way through suspending - for example, while
phylink and phylib is still active.

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

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

[Linus Walleij]

On Sat, Mar 14, 2026 at 1:37 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Sat, Mar 14, 2026 at 12:44:56AM +0100, Linus Walleij wrote:
> > On Fri, Mar 13, 2026 at 12:08 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > > On Fri, Mar 13, 2026 at 11:57:16AM +0100, Christophe Roullier wrote:
> > > > In the deepest low-power modes, the pinctrl configuration is lost
> > > > and is never restored if the interface is down.
> > > > This commit ensures that the pinctrl state is set in all cases.
> > >
> > > Shouldn't the pin state be restored by the pinctrl layer?
> >
> > What we have in the device core only applies "init" and "default"
> > states, and provides these handles for transitioning to "sleep"
> > and "default" again (like a state machine).
>
> What I was meaning is that - for a driver using the "default" state,
> if the hardware loses the pinctrl state during sleep, isn't it the
> responsibility of the pinctrl driver to restore the state rather
> than leaving it in whatever states it happens to be when the SoC
> comes back from suspend?

Aha I understand.

Some pin controllers are aware of the different states from an
electronic point of view, so different states are programmed into
the hardware for suspend/sleep and "active". This programming
can be both implicit (fixed in the pin controller hardware) or
programmable by the operating system. Then a hardware
line tells the pin controller when the system goes to sleep so it
autonomously apply some sleep mode settings to the pins
and reverse this back when waking up. These pin controllers
themselves are "always on", in a specific power domain that
never shuts down so they can keep track of this.

In other cases the pin controller lacks this autonomous ability
and then each driver need to transition its pin state to the
right one when going to sleep and coming back up.

In some further cases the pin controller does some power
management but it's sub-optimal such as leaving I2C bus lines
floating on pull-up when they should be pulled to VDD
to minimize leaks during sleep (for example). The default
is OK but with some pin control state intervention we can do
better, sometimes.

> If that is not the case, then don't we have a major issue where
> drivers using pinctrl but do not issue any pinctrl calls in the
> resume function are buggy?

Sadly, in my experience, the power management features of an
SoC is the last step or an "afterthought" when companies
upstream their changes to the Linux kernel, this comes last
and often never. The code is there in the downstream (vendor)
kernels. Running the mainline kernel works but eats power :(

I think what we see when we do:
$ git grep  pinctrl_pm_select_sleep_state |wc -l
107
are the companies that have actually gotten around to do this
properly and test it. There are some very generic hardware blocks
such as the designware USB controller in that list.

Some SoCs may benefit from autonomous power state control
in the hardware, all ACPI platforms for example, I *think* have
the ambition that firmware/BIOS deals with this and we need not
think about it. For devicetree I think it depends, it's up to the
SoC how much is autonomous and how much needs to be
initiated by the driver.

Yours,
Linus Walleij

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

[Russell King (Oracle)]

On Tue, Mar 17, 2026 at 01:13:14AM +0100, Linus Walleij wrote:
> On Sat, Mar 14, 2026 at 1:37 AM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> > On Sat, Mar 14, 2026 at 12:44:56AM +0100, Linus Walleij wrote:
> > > On Fri, Mar 13, 2026 at 12:08 PM Russell King (Oracle)
> > > <linux@armlinux.org.uk> wrote:
> > > > On Fri, Mar 13, 2026 at 11:57:16AM +0100, Christophe Roullier wrote:
> > > > > In the deepest low-power modes, the pinctrl configuration is lost
> > > > > and is never restored if the interface is down.
> > > > > This commit ensures that the pinctrl state is set in all cases.
> > > >
> > > > Shouldn't the pin state be restored by the pinctrl layer?
> > >
> > > What we have in the device core only applies "init" and "default"
> > > states, and provides these handles for transitioning to "sleep"
> > > and "default" again (like a state machine).
> >
> > What I was meaning is that - for a driver using the "default" state,
> > if the hardware loses the pinctrl state during sleep, isn't it the
> > responsibility of the pinctrl driver to restore the state rather
> > than leaving it in whatever states it happens to be when the SoC
> > comes back from suspend?
> 
> Aha I understand.

I'm not so sure. My point is that if a driver binds, and pinctrl
ends up using the default pinctrl settings, then should it not
be the case that after a suspend/resume cycle, the pinctrl
settings remain configured in the default state with no driver
intervention?

This is my point - a driver that is unaware of pinctrl should not
have to do anything special in its resume path to ensure that the
pinctrl state that was configured by generic code at probe time
is maintained after a resume - that state should not be lost.

I'm trying to get an answer on this, because the original patch
description here says:

| In the deepest low-power modes, the pinctrl configuration is
| lost and is never restored if the interface is down.

stmmac uses the "default" pinctrl state at probe time. This commit
says that is lost over suspend/resume - which to me sounds like
a pinctrl driver bug, because on resume, the pinctrl driver is not
ensuring that the pinctrl state is restored to whatever it was when
the suspend happened (whether the driver explicitly changed it or
not.)

To put it another way...

On entry to probe for a non-pinctrl driver, if DT describes a default
pinctrl state, that state will be selected by core code.

On suspend, the driver is free to select another state if it so wishes,
or do nothing (e.g. it's unaware of pinctrl.)

On resume, the pinctrl layer, what is expected to happen. Surely, it
is reasonable for a pinctrl unaware driver, or at least a driver which
has _not_ changed the pinctrl state to expect that the default pinctrl
state is still in effect when its resume function is called - and if
that is not the case, then there's a bug here.

Another way to put it...

We shouldn't be expecting device drivers to have to mess with pinctrl
e.g. switching to a sleep state and then back to a default state just
to have pinctrl settings restored to a functional state on resume.

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

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

[Linus Walleij]

On Tue, Mar 17, 2026 at 2:11 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> On Tue, Mar 17, 2026 at 01:13:14AM +0100, Linus Walleij wrote:

> > Aha I understand.
>
> I'm not so sure.

OK let's hash it out.

> I'm trying to get an answer on this, because the original patch
> description here says:
>
> | In the deepest low-power modes, the pinctrl configuration is
> | lost and is never restored if the interface is down.
>
> stmmac uses the "default" pinctrl state at probe time. This commit
> says that is lost over suspend/resume - which to me sounds like
> a pinctrl driver bug, because on resume, the pinctrl driver is not
> ensuring that the pinctrl state is restored to whatever it was when
> the suspend happened (whether the driver explicitly changed it or
> not.)
>
> To put it another way...
>
> On entry to probe for a non-pinctrl driver, if DT describes a default
> pinctrl state, that state will be selected by core code.
>
> On suspend, the driver is free to select another state if it so wishes,
> or do nothing (e.g. it's unaware of pinctrl.)
>
> On resume, the pinctrl layer, what is expected to happen. Surely, it
> is reasonable for a pinctrl unaware driver, or at least a driver which
> has _not_ changed the pinctrl state to expect that the default pinctrl
> state is still in effect when its resume function is called - and if
> that is not the case, then there's a bug here.
>
> Another way to put it...
>
> We shouldn't be expecting device drivers to have to mess with pinctrl
> e.g. switching to a sleep state and then back to a default state just
> to have pinctrl settings restored to a functional state on resume.

OK I see your point, the pinctrl hardware state should not change
behind the back of the driver, and if it does, and that is worked
around by these calls to restore the state using the pinctrl PM
helpers become a messy quirk.

- Selecting those states to reconfigure pins into special modes
  during sleep is OK.

- Selecting those states to restore the hardware state inside the
  pin controller itself is NOT OK.

If this is done for the latter reason, you're right of course, there is
a bug in the pin controller. Certainly it is expected to maintain its
own state over a suspend/resume cycle.

Yours,
Linus Walleij

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

[Russell King (Oracle)]

On Thu, Mar 19, 2026 at 10:43:25AM +0100, Linus Walleij wrote:
> On Tue, Mar 17, 2026 at 2:11 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> > I'm trying to get an answer on this, because the original patch
> > description here says:
> >
> > | In the deepest low-power modes, the pinctrl configuration is
> > | lost and is never restored if the interface is down.
> >
> > stmmac uses the "default" pinctrl state at probe time. This commit
> > says that is lost over suspend/resume - which to me sounds like
> > a pinctrl driver bug, because on resume, the pinctrl driver is not
> > ensuring that the pinctrl state is restored to whatever it was when
> > the suspend happened (whether the driver explicitly changed it or
> > not.)
> >
> > To put it another way...
> >
> > On entry to probe for a non-pinctrl driver, if DT describes a default
> > pinctrl state, that state will be selected by core code.
> >
> > On suspend, the driver is free to select another state if it so wishes,
> > or do nothing (e.g. it's unaware of pinctrl.)
> >
> > On resume, the pinctrl layer, what is expected to happen. Surely, it
> > is reasonable for a pinctrl unaware driver, or at least a driver which
> > has _not_ changed the pinctrl state to expect that the default pinctrl
> > state is still in effect when its resume function is called - and if
> > that is not the case, then there's a bug here.
> >
> > Another way to put it...
> >
> > We shouldn't be expecting device drivers to have to mess with pinctrl
> > e.g. switching to a sleep state and then back to a default state just
> > to have pinctrl settings restored to a functional state on resume.
> 
> OK I see your point, the pinctrl hardware state should not change
> behind the back of the driver, and if it does, and that is worked
> around by these calls to restore the state using the pinctrl PM
> helpers become a messy quirk.
> 
> - Selecting those states to reconfigure pins into special modes
>   during sleep is OK.
> 
> - Selecting those states to restore the hardware state inside the
>   pin controller itself is NOT OK.
> 
> If this is done for the latter reason, you're right of course, there is
> a bug in the pin controller. Certainly it is expected to maintain its
> own state over a suspend/resume cycle.

Thanks for the clarification. So, going back to the original patch:

"In the deepest low-power modes, the pinctrl configuration is lost
 and is never restored if the interface is down.
 This commit ensures that the pinctrl state is set in all cases."

This commit message seems to be suggesting that the switch to sleep
mode and back to default mode is to ensure that the pinctrl driver
for this platform correctly re-sets the pinctrl state back to the
default state - which suggests that this patch is the wrong approach
and there is a bug somewhere in the pin controller layer.

That said, for platforms that do provide a sleep state for stmmac, it
makes sense to switch the pinctrl to that state even when the netif
is down. So, I think there's actually two separate issues:

1. the pin controller not restoring the state that was set at suspend.

2. not switching to sleep state when supported when the netif is down.

and fixing (2) will mask (1) which will be bad. So, I think (1) needs
to be fixed first while we have an easy-to-test case, and my suggested
replacement patch is a better approach for (2).

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

[PATCH v2 2/2] 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>
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 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 26ac1cdc561c2..c3eaeffcb6aee 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -8057,6 +8057,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