[PATCH v4 2/6] Add GPIO ioctl decoding

[Kent Gibson]

On Mon, Jan 25, 2021 at 04:37:02AM +0300, Dmitry V. Levin wrote:
> On Mon, Jan 25, 2021 at 08:34:56AM +0800, Kent Gibson wrote:
> > On Sun, Jan 24, 2021 at 06:06:42PM +0300, Dmitry V. Levin wrote:
> > > On Sat, Jan 23, 2021 at 09:56:41AM +0800, Kent Gibson wrote:
> > > > Decode the GPIO character device ioctls first introduced in Linux v4.8,
> > > > as well as those added in subsequent kernel releases up to Linux v5.7.
> > > > 
> > > > Signed-off-by: Kent Gibson <warthog618 at gmail.com>
> > > 
> > > I've given this a wider testing and found some portability issues.
> > > 
> > > First of all, include/uapi/linux/gpio.h was first introduced a bit
> > > earlier: in Linux v4.6 it contained just 2 ioctl commands
> > > (GPIO_GET_CHIPINFO_IOCTL and GPIO_GET_LINEINFO_IOCTL) and just two
> > > definitions of structures (gpiochip_info and gpioline_info), so this code
> > > doesn't compile with kernel headers from Linux v4.6 and v4.7.
> > 
> > Sorry - didn't realise those preliminary versions of gpio.h made it into
> > the released kernel.  They actually define a different ioctl major, 'o'
> > instead of 0xb4, that conflicted with other ioctls.
> 
> Fortunately, no: the header was introduced by commit
> v4.6-rc1~108^2~104^2~3, and 'o' was changed later to 0xB4 by commit
> v4.6-rc1~108^2~6, so there were no released kernels with 'o'.
> 

That makes more sense.

> > So probably best to not use those versions of gpio.h from the kernel.
> > That rules out using HAVE_LINUX_GPIO_H.
> > 
> > > I see 3 alternative fixes for this portability issue:
> > > - check for structures available since v4.8 and bump requirements
> > >   appropriately;
> > > - add to types/gpio.h definitions of those 4 structures that were
> > >   introduced in Linux v4.8;
> > > - add to types/gpio.h definitions of all 6 structures defined since Linux
> > >   v4.8 and lift the HAVE_LINUX_GPIO_H requirement altogether.
> > > 
> > > I'm inclined to choose the last alternative, see the proposed patch below.
> > 
> > Of the three options, I agree that the third is the best - only use the
> > kernel gpio.h to check that the local type definitions match.
> > 
> > Is there a reason you couldn't just include the latest kernel gpio.h in
> > the strace build directly, rather than having to redefine everything in
> > types/gpio.h?
> 
> Yes, this is certainly possible, in fact, this is what I suggest to do in
> the test, but we need fallback definitions for older kernel headers, too,
> so there should be no material difference.
> 
> Also, we don't have automation for that redirection back to linux/gpio.h,
> and a multitude of #ifdef HAVE_STRUCT_GPIO*/#define/#endif doesn't look
> that nice.
> 
> I must admit that linux/gpio.h is good enough to allow any of these two
> approaches, there were no incompatible changes once it was included into
> released kernels.  Unfortunately, not all uapi headers are good enough in
> this respect.
> 

What I'm suggesting is keeping a copy of the current linux/gpio.h in the
strace tree so we always build against that - even when building on old
kernels.  The decoder and tests would match that specific header, but
build on any kernel.

Granted this doesn't work for any headers that may have ABI issues, but
that isn't the case for gpio.h - all the types have remained ABI
compatible since inception.  And that is why we needed to do a v2 as
those types were unable to be extended.

The biggest problem I have is where to put it, as the types directory
serves a different purpose.

Cheers,
Kent.