Re: [RFC PATCH 0/3] gpiolib: ramp-up delay support

[Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>]

Hi Rob,

On Tue, Dec 13, 2022 at 08:20:57AM -0600, Rob Herring wrote:
> On Mon, Dec 12, 2022 at 4:35 AM Alexander Stein wrote:
> >
> > Hi all,
> >
> > this series is an RFC for a general approach to solve the issue at [1]. While
> > a device specific property works as well, a more generic approach is preferred.
> > In short: When enabling a GPIO the actual ramp-up time might be (much) bigger
> > than what software usually assume, in my case >100ms. Adding a delay to each
> > driver is cumbersome.
> 
> At least for DT, I think this belongs (if at all) in the consumers,
> rather than a producer property. The options there are
> 'foo-gpios-ramp-us' for 'foo-gpios' or add some delay bits to GPIO
> flags.

My main requirement is to handle these properties in a central place,
without having to patch individual drivers, so this would work for me.

> We already have some of the former for various 'generic' power
> sequencing related delays. Of course, there's no real pattern to them
> as they all get added as we go without much foresight. In this case
> even, there are 4 possible delays: pre and post ramp up and down.
> 
> > Instead the (optional) ramp-up delay is added to each gpio_desc. The delays can
> > be specified per gpio-controller, similar to 'gpio-line-names'. Actually the
> > parsing code is almost a 1:1 copy of devprop_gpiochip_set_names(). Due to
> > (temporary) memory allocation, I opted for a separate function, there is code
> > duplication, but handling both properties in a single function seemed too
> > tedious, let alone the to be added ramp-down delays.
> >
> > This feature could also be added as a callback in gpio_chip, but the callbacks
> > have to be added to each driver then. I would prefer a single one-fits-all
> > implementation and another indirection in the GPIO call chain.
> >
> > Laurent suggest to add a GPIO delay node in DT. IMHO this increased the DT
> > complexity unnecessarily. But comments are welcome.
> >
> > The following 3 patches are a proof-of-concept on my platform, consisting of:
> > Patch 1 is the proposed bindings and straight forward.
> > Patch 2 is the current implementation
> > Patch 3 is an actual usage example for specifying the delays
> >
> > TODO:
> > 1. Adding ramp-down delays (Just the inverse copy of ramp-up delay)
> > 2. Should these delays take active low flags into account?
> > 3. How to deal with setting multiple GPIOs at once?
> >
> > I skipped 1. for now, because this is just a copy with ramp-up being replaced
> > with ramp-down.
> >
> > I'm not that well versed in gpiolib code, so I'm not sure if I got all placed
> > where GPIOs are set. So patch 2 might be incomplete.
> >
> > For now I skipped setting multiple GPIOs at once completely, so to get some
> > feedback on this approach. A possible solution is to check for the bigest delay
> > in the set and use that for all afterwards. But I'm not sure about the overhead
> > in this case.
> >
> > I hope there is some feedback. While thinking about this issue appears to be
> > more widespread than I expected.
> 
> Many/most GPIO controllers can read the actual state of an output
> (IIRC, i.MX ctrlr can). Perhaps that capability could be used to delay
> until the state of the signal matches the set state. And you'd
> probably want to measure how long that took and then add some more
> time based on it. This of course gets into the electricals of at what
> levels a low or high state will register. If you can't read the state,
> then you would be stuck with some maximum timeout.

-- 
Regards,

Laurent Pinchart