Prev: teach gpiolib about gpio debouncing
Next: ipc: use shifts to extract seq/idx
From: Alan Cox on 20 May 2010 16:50
> I'm still wobbling on the 35-vs-36 line. Is that a big need or a
> little need? What user-visible problem does it solve, etc?
>
> Did anyone test it on the MID boxes?

Yes and no - our code is subtly different but we'll just adapt to the
Nokia patch.

The MID bits are heading for .36 I hope so I can't help on the "when"
choice other than to note that it doesn't touch seem to change any code
other than the new functions...

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Brownell on 20 May 2010 19:00

> This would be generally useful for embedded systems,
> especially where the interrupt concerned is a wake source.
>� It allows drivers to avoid
> spurious interrupts from noisy sources so if the hardware
> supports it
> the driver can avoid having to explicitly wait for the
> signal to become
> stable and software has to cope with fewer events.

True.

Not all GPIOs have hardware debounce though, so offering this
capability sort of begs the question of where/how to provide a
software debounce mechanism too...


> We've lived without it for quite some time, though.

I looked at adding debounce support to the generic GPIO calls
(and thus gpiolib) some time back, but decided against it. I
forget why at this time (check list archives) but it wasn't because
of lack of utility in certain contexts.

One thing to watch out for is just how variable the hardware
capabilities are. Atmel GPIOs have something like a fixed number
of 32K clock cycles for debounce, twl4030 had something odd, OMAPs
were more like the Atmel chips but with a different clock. In some
cases debouncing had to be ganged, not per-GPIO. And so forth.

- Dave


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Felipe Balbi on 21 May 2010 03:00
On Fri, May 21, 2010 at 12:50:41AM +0200, ext David Brownell wrote:
>
>> This would be generally useful for embedded systems,
>> especially where the interrupt concerned is a wake source.
>>� It allows drivers to avoid
>> spurious interrupts from noisy sources so if the hardware
>> supports it
>> the driver can avoid having to explicitly wait for the
>> signal to become
>> stable and software has to cope with fewer events.
>
>True.
>
>Not all GPIOs have hardware debounce though, so offering this
>capability sort of begs the question of where/how to provide a
>software debounce mechanism too...

how about adding a flag for supported features and if that hardware
doesn't support we simply return, or fallback to software emulation if
chosen by the user. Something like the feature flags for the i2c bus
drivers.

I mean something like:

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 95f92b0..ae03f6f 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -1855,6 +1855,7 @@ static int __init _omap_gpio_init(void)
bank->chip.get = gpio_get;
bank->chip.direction_output = gpio_output;
bank->chip.set_debounce = gpio_debounce;
+ bank->chip->flags = GPIO_FLAG_DEBOUNCE;
bank->chip.set = gpio_set;
bank->chip.to_irq = gpio_2irq;
if (bank_is_mpuio(bank)) {
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 (!gpio_is_valid(gpio))
goto fail;
- chip = desc->chip;
if (!chip || !chip->set || !chip->set_debounce)
goto fail;
gpio -= chip->base;
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index ce3eb87..26c0aa2 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -32,6 +32,8 @@ struct device;
struct seq_file;
struct module;

+#define GPIO_FLAG_DEBOUNCE (1 << 0)
+
/**
* struct gpio_chip - abstract a GPIO controller
* @label: for diagnostics
@@ -61,6 +63,7 @@ struct module;
* names for the GPIOs in this chip. Any entry in the array
* may be NULL if there is no alias for the GPIO, however the
* array must be @ngpio entries long.
+ * @flags: a bitmap for supported features by that particular gpio chip.
*
* A gpio_chip can help platforms abstract various sources of GPIOs so
* they can all be accessed through a common programing interface.
@@ -104,6 +107,7 @@ struct gpio_chip {
char **names;
unsigned can_sleep:1;
unsigned exported:1;
+ unsigned flags;
};

extern const char *gpiochip_is_requested(struct gpio_chip *chip,

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

--
balbi

DefectiveByDesign.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 21 May 2010 06:00
> stable and software has to cope with fewer events. We've lived without
> it for quite some time, though.

We lived without graphics for quite some time, without 64bit for quite
some time ;)

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on 21 May 2010 06:10

> 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(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3
Prev: teach gpiolib about gpio debouncing
Next: ipc: use shifts to extract seq/idx