Re: [PATCH v4 12/20] gpiolib: cdev: support setting debounce

From: Kent Gibson
Date: Tue Aug 18 2020 - 10:38:03 EST


On Mon, Aug 17, 2020 at 08:21:58PM +0200, Bartosz Golaszewski wrote:
> On Fri, Aug 14, 2020 at 5:05 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> >
> > Add support for setting debounce on a line via the GPIO uAPI.
> > Where debounce is not supported by hardware, a software debounce is
> > provided.
> >
> > Signed-off-by: Kent Gibson <warthog618@xxxxxxxxx>
> > ---

[snip]

> > + debounce_period = READ_ONCE(desc->debounce_period);
> > + if (debounce_period) {
> > + info->attrs[num_attrs].id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE;
> > + info->attrs[num_attrs].debounce_period = debounce_period;
> > + num_attrs++;
> > + }
> > + info->num_attrs = num_attrs;
>
> AFAICT this (reading it in gpio_desc_to_lineinfo) is the only reason
> to store the debounce period in struct gpio_desc. I'm wondering if we
> can avoid extending this struct only for such uncommon case and store
> it elsewhere. In all other cases where you read or write to it - you
> have access to the underlying edge detector. Would the single-line
> struct line I suggested elsewhere be a good place? On the other hand
> I'm not sure how to get it having only the desc. I need to think about
> it more.
>

Yeah, it is stored there so it can be returned by lineinfo_get() for the
GPIO_V2_GET_LINEINFO_IOCTL and GPIO_V2_GET_LINEINFO_WATCH_IOCTL.
And the same applies to any future config fields.
I would also like to not pollute the desc, or anything else in gpiolib,
but wasn't sure where else to put it.

I'm open to suggestions.

Cheers,
Kent.