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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxx>
> > ---
> >  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