Re: [PATCH v5 00/20] gpio: cdev: add uAPI v2

[Kent Gibson]

On Thu, Aug 27, 2020 at 06:02:03PM +0200, Bartosz Golaszewski wrote:
> On Thu, Aug 27, 2020 at 5:53 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> >
> > On Thu, Aug 27, 2020 at 4:00 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> >
> > > This patchset defines and implements a new version of the
> > > GPIO CDEV uAPI to address existing 32/64-bit alignment issues, add
> > > support for debounce, event sequence numbers, and allow for requested
> > > lines with different configurations.
> > > It provides some future proofing by adding optional configuration fields
> > > and padding reserved for future use.
> > >
> > > The series can be partitioned into three blocks; the first two patches
> > > are minor fixes that impact later patches, the next eleven contain the
> > > v2 uAPI definition and implementation, and the final seven port the GPIO
> > > tools to the v2 uAPI and extend them to use new uAPI features.
> > >
> > > The more complicated patches include their own commentary where
> > > appropriate.
> >
> > I'm ready to queue this now. Certainly any remaining snags can be
> > fixed in-tree.
> >
> > It kind of keeps in tradition with proper software projects "plan to
> > throw one away" which is what we have traditionally done several
> > times: the first Bluetooh framework was tossed, JFFS was tossed
> > for JFFS2, Video4Linux was tossed for V4L2. So let's do this.
> >
> > Anyone against? I will put it on an immutable branch and then merge
> > that in for devel.
> >
> 
> Hi Linus,
> 
> please hold it maybe for one more week - I'd love to have some more
> people take a look at the user facing header at least. Andy is usually
> very thorough in his reviews so I'm Ccing him here.
> 
> I'll too skim through the series one more time.
> 

I'd be happier with more eyeballs over it before it goes in as well.

I'm also wondering if it is worthwhile dropping the restriction on
changing edge detection configuration, that is present in
gpio_v2_line_config_change_validate() in patch 10, so long as userspace
is made aware that changing it may result in lost interrupts as we
may need to release and re-request the interrupt to effect the change.

Similarly changing debounce while edge detection is enabled, that is
disallowed in patch 12.

My policy when writing the implementation was to disallow any operation
that may have unanticiapted side-effects, and forcing userspace to
release and re-request the line(s), but that may be overly restrictive?
The particular use case I am considering is one I had been asked about -
changing a requested line from input with edge detection to output, and
vice versa. Losing interrupts isn't really an issue for this use case -
it is expected.  Yet the current implementation requires a re-request.

Cheers,
Kent.