Re: [PATCH] gpiolib: add GPIO_SET_DEBOUNCE_IOCTL

[Kent Gibson]

On Thu, Apr 30, 2020 at 01:32:22PM +0000, Bujanda, Hector wrote:
> Thanks all for your guidance!
> 
> First saying that this patch request was sent having our platforms in k4.14 in the way of upgrading to k5.4.
> In those versions the commit e588bb1eae31be73fbec2b731be986a7c09635a4 "gpio: add new SET_CONFIG ioctl() to gpio chardev" by Kent Gibson was not available.
> 
> I see that you clearly understand the necessity of having a way of configuring debounce from the userspace.
> Our platforms make use of hardware debouncing filtering. Up to now we were using the sysfilesystem to let the user handle gpios (including debounce configuration).
> We wanted now to get rid of sysfilesystem and start using gpiolib/libgpiod.... but configuring debounce is blocking us.
> 
> Now I clearly see (as pointed by Bartosz Golaszewski) that my suggested GPIO_SET_DEBOUNCE_IOCTL is wrong as it hits the chip file descriptor while 'Modifying any config settings can only happen on lines previously requested too in user-space'.
> 
> I agree with all that a flag is needed to allow configuring debounce to '0' which has always meant disabling it.
> 
> Also agree with 'Kent Gibson' suggestion of  'You might want to add a flag to the GPIOLINE_FLAGs to indicate if debounce is set'.
> 
> I have my doubts if it is compulsory to extend debounce configuration to the gpioevent_requests since the debounce value configured by a user is normally linked to a hardware noise in a line; and that does not change from one gpioevent_requests to another. So I think this configuration would be useful but not compulsory.
> 

Just to clarify on this point, the reason the SET_CONFIG would have to
be extended to events is not to alter the debounce on the fly but to set
it at all.  Lines are requested as either handles (for outputs or polled inputs) 
or events (for asynchronous edge events on inputs). We cannot extend
either the handle or event request ioctls themselves as there is no provision 
in their data structures for future expansion.  There is in the
SET_CONFIG ioctl - but that doesn't apply to event requests yet...


> I agree with Linus Walleij that 'there is a serious user-facing problem here though, because not all GPIO controllers supports debounce'.
> Our platforms have native freescale/NXP gpiochips not supporting hardware debounce and our own gpiochips having hardware debounce.
> We have also noticed that 'drivers/input/keyboard/gpio_keys.c contains generic debounce code using kernel timers if the GPIO driver cannot provide debouncing'. That feature is not of our interest (because of having hardware debounce filters) but it would clearly be a very good overall functionality.
> 
> Having said all above, I wonder how you want to proceed.
> Our current development in k5.4 and libgpiod1.4.1 is much behind master... what makes collaboration (and reusability) a bit more complex.
> Also I see the implementation requires a bigger picture than I initially expected.
> So I wonder if you want me to do the initial steps of the development (what I foresee will require some back and forth) or you prefer implementing all pieces.
> 

I totally agree with you on the widening scope.

Bart - how do you want to go forward with this?  I'm available to work
on it, in part or full.

Cheers,
Kent.