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

Hi,

On Tue, Aug 11, 2026 at 1:15 PM Doug Anderson <[email protected]> wrote:
>
> > > 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.
> >
> > I think you should step back and thing about what state names
> > are the most relevant for a UART.
> >
> > Why would you want "unopened"? Just use "default".
> > The states are just state transitions such as any finite
> > state machine.
> >
> > These are perfectly fine semantics:
> >
> >           open()         close()            open()
> > "default" ------> "open" -------> "default" ------> "open"
> >
> > The device core doesn't touch the states after probe()
> > so the "default" state can be reused without any problems.
> > No need to have any "unopened" state.
>
> The idea in my mind, at least, was that the state of the system isn't
> all that well known at boot time. The bootloader may not have left
> pins / peripherals in a consistent state. Until all system components
> finish probing and finding each other, it may be hard to know the
> correct pin state. It is during this unstable period of system boot
> that I envision we'd want this "init" / "unopened" state. After we get
> out of the uncertainty of boot, we'd never need to go back to this
> "init" state. Said another way, the "init" state might account for
> some uncertainty about the state of the device that the pins are
> connected to. On the other hand, there should be no uncertainty in the
> "closed" state. This is why I was thinking that they could/should be
> different states.
>
> This all being said, you're right that we may need to take a step back
> and look again at all the options. In the specific case Michał is
> trying to solve, the "init" state and the "closed" state probably can
> be the same. Thus using a solution like you proposed can make sense.
>
> One thing I guess we'd have to work out is whether we'd really want to
> use "default" and "open". I usually think of "default" as the active
> state and then the two common states "sleep" and "idle" as the
> inactive states. I guess looking at all the states, maybe you'd
> suggest:
>
> * At probe time, select "default" state since the port is closed.
> * At port open time, select "open" state if it exists.
> * If a port runtime suspends while open, select "idle" state if it exists.
> * If a port runtime resumes while open, try to select "open" if it
> exists, or fallback to "default".
> * If a port does system suspend while open, select "sleep" state if it exists.
> * If a port does system resume while open, try to select "open" if it
> exists, or fallback to "default".
> * If any kind of suspend/resume happens while a port is closed, it
> stays in "default"
>
> The above assumes that if a port is closed that it doesn't need /
> shouldn't transition to "sleep" / "idle" states. That's probably true
> (?) for any port that defines an "open" state, implying that "default"
> means "closed". If someone only defines "default" and "sleep" or
> "idle", though, maybe they'd expect transitions to "sleep" / "idle" at
> suspend time?
>
> Does that sound roughly like what you're thinking?

FWIW, we are in basically the same situation for SPI on the same
board. Specifically, this happens:

1. Remote side starts unpowered.

2. SPI bus driver probes and pinctrl sets SPI pins to their default
state. Importantly, the SPI chip select (CS_N) is high, which
backpowers the device (illegal).

What we need is we need to _assert_ chip select (make CS_N low) until
we know that the other side is powered.

...we can try to use the "init" state that exists today, but by the
time the SPI's probe function finishes, there is no guarantee that the
SPI client's probe function has run. The module might not have been
loaded yet. This means that "init" state (as currently defined)
doesn't help.

...we can't use the "opened" we've been talking about in UART because
there's no real idea of "open"ing a SPI bus. It's transaction
oriented.

...we can't use the normal "runtime pm" concept in SPI and start the
SPI bus in suspended state because the "runtime suspended" state of
the bus would want the chip select deasserted (CS_N high).

Using an extended "init" state (as in Michał's patch) and
transitioning to "default" upon the first SPI transfer would work,
since we'd know that the client is powered by then. After that point
in time, if the client wanted to power the device off it could
manually assert "chip select" before powering off.

Of course, I'll admit that using the extended "init" state like this
is still a bit ugly, whether doing it like Michał's patch does or by
doing something like the "unopened" solution that I talked about.

Other options for SPI could be:

1. Hack something into the SPI bus driver to turn on the regulator for
the device. This is pretty ugly, but would work. Probably not
upstreamable.

2. Invent a "pwrseq" solution for SPI where we run a sequence of steps
(turning the regulator on) before probing the SPI bus, like other
"pwrseq" drivers. Of course, most of those are for "discoverable"
busses, but the idea of powering up a peripheral before probing the
bus it's on is similar. This seems an extreme amount of work.

3. Hack the "CS GPIO" to be controlled by the client. It looks like we
could fully move the GPIO (including the pinctrl in the DT) to the
client. Then the client could call spi_set_csgpiod() after it's turned
on the regulator. The client could have an "init" state for the GPIO
that keeps it low and then transition to "default" right before
calling spi_set_csgpiod().

Of everything, solution #3 doesn't seem terrible. I also still don't
totally hate the idea of extending the "init" state... Of course,
there's also some chance we can figure out other ways to get this
regulator turned on sooner.

-Doug