Re: [PATCH 1/5] gpiolib: introduce set_debounce method

[Alan Cox]

> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index cd85fd1..ed1ed74 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1461,9 +1461,14 @@ int gpio_set_debounce(unsigned gpio, unsigned debounce)
>   
>          spin_lock_irqsave(&gpio_lock, flags);
>   
> +       chip = desc->chip;
> +       if (!(chip->flags & GPIO_FLAG_DEBOUNCE)) {
> +               spin_unlock_irqrestore(&gpio_lock, flags);
> +               return 0;
> +       }
> +

If you add the feature check then presumably someone trying to do
debounce on a port without the feature isn't paying attention so this
should WARN at the very least. Also it shouldn't be a return 0.

This however seems a bit excessive and inconsistent. Every other function
simply returns -EINVAL if the request is unsupported. So not only does it
complicate the code it makes the code inconsistent with its existing
regular behaviour. The initial patch is consistent, regular and follows
expected gpiolib behaviour in all respects.

> that could be used later for adding debounce emulation for chips that 
> doesn't support hw debouncing.

You don't need flags for this - the request will just start working if
someone adds the feature.

GPIO is almost always fairly tightly platform bound so features only
existing on certain ports is fine. The platform vendor will have made
sure they relevant ports have suitable debounce facilities.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/