Re: [RFC PATCH v2 0/3] gpio: Add gpio-delay support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 15, 2022 at 3:26 PM Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Dec 15, 2022 at 12:21:33PM -0600, Rob Herring wrote:
> > On Thu, Dec 15, 2022 at 7:16 AM Linus Walleij wrote:
> > > On Wed, Dec 14, 2022 at 10:53 AM Alexander Stein wrote:
> > >
> > > > thanks for the feedback I've received. This is the reworked RFC for
> > > > adressing a platform specific ramp-up/ramp-down delay on GPIO outputs.
> > > > Now the delays are neither specified as gpio-controller nor
> > > > consumer-specific properties.
> > > >
> > > > v2 is a different approach than v1 in that it adds a new driver which will
> > > > simply forward setting the GPIO output of specified GPIOs in OF node.
> > > > The ramp-up/ramp-down delay can now be actually defined on consumer side,
> > > > see Patch 1 or 3 for examples.
> > >
> > > I really like this approach, it looks better than I imagined.
> >
> > It seems over-engineered to me. So far no comments on my 3 suggestions either...
>
> I like the idea of handling this on the consumer's side, possibly with
> standard foo-gpios-ramp-{up,down}-delay-us (name to be bikeshedded)
> properties as you mentioned in the review of v1.
>
> > One is to just use some GPIO flag bits. Say 4-bits of GPIO flags
> > encoded as power of 2 ramp delay. We have to pick the units. For
> > example, 100us*2^N, which gives you 200us-3.2s of delay.
>
> This could probably work too.
>
> > Anything less is short enough to just hard code in a driver.
>
> In which driver though ? The whole point is that we should avoid
> handling this in particular drivers.

Okay, make the range 100us-1.63s and the minimum delay is 100us. Or
50us-819ms? What's a small enough minimum that no one will care about
the extra delay?

One thing we don't want is DT authors putting a device's delay needs
in here. Then we'll get coupling to the OS implementation or double
delays. Something like this should be clear:

#define GPIO_THIS_IS_ONLY_THE_SIGNAL_RC_RAMP_TIME_100us

;)

Rob



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux