Re: [PATCH v1 0/2] pinctrl / 8250_dw: Allow drivers to keep init pinctrl state until first open

Hi,

On Mon, Aug 10, 2026 at 11:34 PM Linus Walleij <[email protected]> wrote:
>
> Hi Michal,
>
> thanks for your patches!
>
> On Mon, Aug 10, 2026 at 3:06 PM Michał Kardaś <[email protected]> wrote:
>
> > During device probe, pinctrl_bind_pins() binds pins to their "init" state
> > if specified in Device Tree. When probe finishes, pinctrl_init_done()
> > automatically transitions the pins from "init" to "default" state.
> (...)
> > 1. Patch 1 (pinctrl core):
> >    Adds pinctrl_keep_init_state(dev). When called during probe,
> >    pinctrl_init_done() opts out of the automatic "init" -> "default"
> >    transition, allowing the driver to keep pins in the safe "init" state
> >    upon probe completion. Updates Documentation/driver-api/pin-control.rst.
> >    Board configurations that do not define an "init" state are completely
> >    unaffected.
>
> I think this is the wrong approach to this problem.
>
> You are changing the transition from "init" to "default" for all devices
> on the entire system.

I don't _think_ that's what his patches are doing, though? It's only
changing the transition for drivers that actually call
pinctrl_keep_init_state(). For any drivers that don't call
pinctrl_keep_init_state() then the behavior is unchanged.

Furthermore, this would only affect hardware that specifically has an
"init" state defined.

When brainstorming this with Michał, my thought was that for a certain
class of drivers we could unconditionally declare: if an "init" state
is defined for the hardware, the correct time to transition away from
the "init" state is upon the first device open, not upon completion of
probe. Specifically, I was thinking that for "bus" drivers (UART, SPI,
I2C) that the end of probe doesn't really mean that everything is set
up and pins can be configured normally. It's only after the actual
endpoint driver (the one using the bus) probes that you can really
transition off the "init" state.


> What if this is not good for all devices?

At the moment, I couldn't find any device that the above rules would
break. Certainly I could have missed something.


> I think the right approach is to create a new custom pin control
> state for 8250dw, something like "active" or "online", and then
> actively retrieve this state when the device goes online.
> The "init" state can just be the exact same as "default",
> or you can just define "default" to be what "init" is and skip
> "init" altogether.
>
> Pin control supports any kind of arbitrarily named custom
> states.
>
> In wherever the UART is actually opened:
>
> #include <linux/pinctrl/consumer.h>
>
> probe():
> mydev->p = devm_pinctrl_get(dev);
> mydev->pins_online_state = = pinctrl_lookup_state(dev->pins->p, "online");
>
> open():
> ret = pinctrl_select_state(dev->pins->p, mydev->pins_online_state);
>
> close():
> ret = pinctrl_pm_select_default_state(dev);
>
> + all error handling and stuff, see e.g. drivers/base/pinctrl.c for
> good coding practice.

Definitely the things Michał wants to achieve can be done without
changing the pinctrl core by using named pinctrl states and
transitioning at the right times. One of the solutions we looked at
together was functionally equivalent to what he proposed here: he
defined a state "unopened" that was a complete copy of "init". At the
end of probe he transitioned to "unopened" and then later transitioned
to "default" upon the first open, never to use "unopened" again.
Personally I didn't love this because it required duplicating the
"unopened" and "init" states when really we just wanted to keep the
"init" state longer. ...and I couldn't think of any time when a UART
driver would have an "init" state when it _shouldn't_ persist until
the first open. :-P That being said, it worked and wasn't too ugly or
anything.

FWIW, that is a _slightly_ different solution to what you've proposed.
In normal pinctrl usage the "init" state is a temporary state used
just at system startup time before all the pieces are together. In the
solution above "unopened" had this same property as being just a
temporary state during init time.  As I understand, the idea is that
the "init" state isn't necessarily the same as the "closed" or "sleep"
state but can also be a "safe" state to make sure lines don't glitch
until we're certain everything has been initialized properly. In your
proposal, the initial state needs to be equivalent to the "closed"
state, right? That may be fine for some cases, but maybe not
everything?


-Doug