linux-kernel - Re: [PATCH 1/5] gpiolib: introduce set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100521111136.53dba6c9@lxorguk.ukuu.org.uk>
Date:	Fri, 21 May 2010 11:11:36 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	felipe.balbi@...ia.com
Cc:	David Brownell <david-b@...bell.net>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Tony Lindgren <tony@...mide.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 1/5] gpiolib: introduce set_debounce method

> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/