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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux