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