Re: [RFC PATCH 1/3] dt-bindings: gpio: Add optional ramp-up delay property

[Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>]

Hi Laurent,

Am Dienstag, 20. Dezember 2022, 00:01:23 CET schrieb Laurent Pinchart:
> On Thu, Dec 15, 2022 at 11:56:57AM +0100, Linus Walleij wrote:
> > Hi Laurent,
> > 
> > thanks for the detailed brief!
> > 
> > On Tue, Dec 13, 2022 at 12:45 PM Laurent Pinchart
> > 
> > <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
> > > The circuit we're looking at is
> > > 
> > >   +----------+           +-----------+
> > >   
> > >   | SoC      |           |    VCC    |
> > >   | 
> > >   |          |           |     _     |
> > >   |          |           |     
> > >   |          |           |    | | R  |
> > >   |          |           |    |
> > >   |          |           |    |_|    |
> > >   |      
> > >   |      [IOx|-----+-----|EN]--+     |
> > >   |      
> > >   |          |     |     | SN65DSI83 |
> > >   
> > >   +----------+    --- C  +-----------+
> > >   
> > >                   ---
> > >                   
> > >                    -
> > >                   
> > >                   GND
> > > 
> > > The IOx pin is an open-drain output, the board has a 470nF capacitor to
> > > ground, and the SN65DSI83 has an internal pull-up off 200kΩ. This gives
> > > an RC time constant of 94ms, far from being negligible.
> > > 
> > > The delay is caused by the combination of the open-drain nature of the
> > > output (an intrinsic property of the GPIO controller), the pull-up
> > > resistor (an intrinsic property of the SN65DSI83) and the capacitor on
> > > the line (a property of the board). DT is notoriously bad at modelling
> > > this kind of setup.
> > 
> > Yeah :/
> > 
> > It's not like we don't model discrete electronics, we do that a lot,
> > but as you say, it is really hard to know where to draw the line
> > in cases like this.
> > 
> > > The alternative I proposed, adding a "GPIO delay" DT node to model this,
> > > would also offer a centralized solution to the problem, but with
> > > additional complexity both at probe time and runtime.
> > 
> > I have a slight preference for this, as it will be very explicit in the
> > device tree and we can just put all the code inside its own file and
> > depend on GPIO_OF so other HW description systems do not
> > need to include it.
> > 
> > At the same time it feels a bit overengineered, so maybe just adding
> > this delay as in the patch with some strings attached like comments
> > and docs is yet the best. It feels like we need some more input to
> > reach consensus.
> > 
> > > The regulator delays model the intrinsic delays when enabling or
> > > disabling a regulator, and they should stay. They address a different
> > > problem.
> > 
> > OK right. But someone not knowing exactly what they are doing
> > will end up abusing the delay property on the delay line
> > also for this delay. The risk of that is lesser with a separate
> > delay box.
> 
> That may be true, but I think we can also try to catch abuses in
> reviews. I would be a bit sad if we made life more difficult (and less
> efficient at runtime too) for legitimate users just because we are
> worried about abuses.

What is a legitimate user for you? Given the example in v2 of this series it's 
clear that this feature is an opt-in, both for the DT node as well as for 
specifying a delay.
Another benefit of using a dedicated driver: It also automatically handles 
things like setting multiple GPIOs at once.

> Another thing I've been thinking about is that we may not always want to
> wait for the GPIO delay. Some consumers may not care when the GPIO line
> reaches the desired state as long as it eventually does, or maybe they
> need to perform multiple operations (such as enabling/disabling
> regulators and/or clocks) and only need a synchronization point for a
> group of operations. All that would be pretty hard to handle, and maybe
> it's a problem we'll look at only when needed (and hopefully never).

If you don't care about rising time, do not use gpio-delay for that GPIO, or 
just don't specify a ramp-up delay in the gpio-cells, aka setting to 0.
The more complex synchronisation example you mentioned probably needs a 
similar dedicated driver for grouping those resources.

Best regards,
Alexander