<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Hi Russell, Linus, All,</p>
    <div class="moz-cite-prefix">Le 16/03/2026 à 10:03, Russell King
      (Oracle) a écrit :<br>
    </div>
    <blockquote type="cite"
      cite="mid:abfHUrOpHnjv3GGW@shell.armlinux.org.uk">
      <pre wrap="" class="moz-quote-pre">On Sat, Mar 14, 2026 at 12:37:19AM +0000, Russell King (Oracle) wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="" class="moz-quote-pre">On Sat, Mar 14, 2026 at 12:44:56AM +0100, Linus Walleij wrote:
</pre>
        <blockquote type="cite">
          <pre wrap="" class="moz-quote-pre">On Fri, Mar 13, 2026 at 12:08 PM Russell King (Oracle)
<a class="moz-txt-link-rfc2396E" href="mailto:linux@armlinux.org.uk"><linux@armlinux.org.uk></a> wrote:
</pre>
          <blockquote type="cite">
            <pre wrap="" class="moz-quote-pre">On Fri, Mar 13, 2026 at 11:57:16AM +0100, Christophe Roullier wrote:
</pre>
            <blockquote type="cite">
              <pre wrap="" class="moz-quote-pre">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.
</pre>
            </blockquote>
            <pre wrap="" class="moz-quote-pre">
Shouldn't the pin state be restored by the pinctrl layer?
</pre>
          </blockquote>
          <pre wrap="" class="moz-quote-pre">
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).
</pre>
        </blockquote>
        <pre wrap="" class="moz-quote-pre">
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?
</pre>
      </blockquote>
      <pre wrap="" class="moz-quote-pre">
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.</pre>
    </blockquote>
    <p>For me this case (when NIC is up) is already managed by the
      driver and function suspend/resume:</p>
    <p>On stmmac_suspend :</p>
    <p>==>      /* Enable Power down mode by programming the PMT regs
      */<br>
           if (priv->wolopts) {<br>
               stmmac_pmt(priv, priv->hw, priv->wolopts);<br>
               priv->irq_wake = 1;<br>
           } else {<br>
               stmmac_mac_set(priv, priv->ioaddr, false);<br>
      <b>         pinctrl_pm_select_sleep_state(priv->device);</b><br>
           }</p>
    <p>On stmmac_resume :</p>
    <p>==>     if (priv->wolopts) {<br>
               mutex_lock(&priv->lock);<br>
               stmmac_pmt(priv, priv->hw, 0);<br>
               mutex_unlock(&priv->lock);<br>
               priv->irq_wake = 0;<br>
           } else {<br>
      <b>         pinctrl_pm_select_default_state(priv->device);</b><br>
               /* reset the phy so that it's ready */<br>
               if (priv->mii)<br>
                   stmmac_mdio_reset(priv->mii);<br>
           }<br>
       </p>
    <p>BR</p>
    <p>Christophe.</p>
    <blockquote type="cite"
      cite="mid:abfHUrOpHnjv3GGW@shell.armlinux.org.uk">
      <pre wrap="" class="moz-quote-pre">

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.

</pre>
    </blockquote>
  </body>
</html>