Re: [PATCH v1 0/2] pinctrl / 8250_dw: Allow drivers to keep init pinctrl state until first open
Doug Anderson <[email protected]>
| Newsgroups | org.kernel.vger.linux-gpio,org.kernel.vger.linux-kernel,org.kernel.vger.linux-serial |
|---|---|
| Message-ID | <CAD=FV=UoZ=CoUdku5aaJr9uihxr0Q-Yj4UQeQtYeODMzku7LkA@mail.gmail.com> |
Hi, On Tue, Aug 11, 2026 at 11:47 AM Linus Walleij <[email protected]> wrote: > > Hi Doug, > > > > 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. > > No you're right, nothing breaks other than the that for one > instance of a driver the default/init semantics change. > Which isn't elegant IMO, but it works. > > > 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. > > I think we need to do that. This approach is a hack, it will be > hard for others to understand and lead to a lot of questions > about why this is done and so on. Better use a custom state. Sure, that's a fair argument. To me, it felt like adding the extra state was more of a hack / working around the pinctrl limitations, but I can certainly see both perspectives. > > 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? -Doug