Re: [PATCH 1/5] gpiolib: introduce set_debounce method
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;
unsignedcan_sleep:1;
unsignedexported:1;
+ unsignedflags;
};
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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
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-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
--- On Fri, 5/21/10, Alan Cox a...@lxorguk.ukuu.org.uk wrote: GPIO is almost always fairly tightly platform bound Not entirely. SOC based ones, yes. But GPIOs on expanders and ancillary chips have more variable capabilities, and many SOC-based boards have a bunch of those. so features only existing on certain ports is fine. The platform vendor will have made sure they relevant ports have suitable debounce facilities. One of the earlier examples of why to want debouncing (in the original discussion, not this one) was for buttons (one signal per button press) and the hardware designers don't necessarily hook buttons up to only GPIOs with hardware debounce. So code to kick in debounce there needed to be sensitive to whether or not the capability was available ... and moreover, whether the debounce scale was adequate to compensate the contact chatter. When not, then software debounce was inescapable. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
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, Presense of a debounce method would suffice when gpiolib is in use (vs direct implementation of the API provided by gpiolib). or fallback to software emulation if chosen by the user. ISTR that if you look back to the original discussion about GPIO debouncing, you will find such a software implementation; and maybe such an integration ... -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
One of the earlier examples of why to want debouncing (in the original discussion, not this one) was for buttons (one signal per button press) and the hardware designers don't necessarily hook buttons up to only GPIOs with hardware debounce. So code to kick in debounce there needed to be sensitive to whether or not the capability was available ... and moreover, whether the debounce scale was adequate to compensate the contact chatter. When not, then software debounce was inescapable. And someone who needs it can contribute it. The proposed patch causes no problems in this area. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
On Mon, 17 May 2010 13:02:30 +0300
felipe.ba...@nokia.com wrote:
From: Felipe Balbi felipe.ba...@nokia.com
Few architectures, like OMAP, allow you to set
a debouncing time for the gpio before generating
the IRQ. Teach gpiolib about that.
...
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1447,6 +1447,49 @@ fail:
}
EXPORT_SYMBOL_GPL(gpio_direction_output);
+/**
+ * gpio_set_debounce - sets @debounce time for a @gpio
+ * @gpio: the gpio to set debounce time
+ * @debounce: debounce time is microseconds
+ */
+int gpio_set_debounce(unsigned gpio, unsigned debounce)
+{
+ unsigned long flags;
+ struct gpio_chip*chip;
+ struct gpio_desc*desc = gpio_desc[gpio];
+ int status = -EINVAL;
+
+ spin_lock_irqsave(gpio_lock, flags);
+
+ if (!gpio_is_valid(gpio))
+ goto fail;
+ chip = desc-chip;
+ if (!chip || !chip-set || !chip-set_debounce)
+ goto fail;
+ gpio -= chip-base;
+ if (gpio = chip-ngpio)
+ goto fail;
+ status = gpio_ensure_requested(desc, gpio);
+ if (status 0)
+ goto fail;
+
+ /* now we know the gpio is valid and chip won't vanish */
+
+ spin_unlock_irqrestore(gpio_lock, flags);
+
+ might_sleep_if(extra_checks chip-can_sleep);
+
+ return chip-set_debounce(chip, gpio, debounce);
+
+fail:
+ spin_unlock_irqrestore(gpio_lock, flags);
+ if (status)
+ pr_debug(%s: gpio-%d status %d\n,
+ __func__, gpio, status);
+
+ return status;
+}
+EXPORT_SYMBOL_GPL(gpio_set_debounce);
nitlet: I suspect this function is taking gpio_lock sooner than it
strictly needs to. Find-tuning that would decrease contention by an
insignificant amount ;)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
+EXPORT_SYMBOL_GPL(gpio_set_debounce); nitlet: I suspect this function is taking gpio_lock sooner than it strictly needs to. Find-tuning that would decrease contention by an insignificant amount ;) We need this for Intel MID boxes as well Andrew - so its a generic need. Alan -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
On Thu, 20 May 2010 20:04:17 +0100 Alan Cox a...@lxorguk.ukuu.org.uk wrote: +EXPORT_SYMBOL_GPL(gpio_set_debounce); nitlet: I suspect this function is taking gpio_lock sooner than it strictly needs to. Find-tuning that would decrease contention by an insignificant amount ;) We need this for Intel MID boxes as well Andrew - so its a generic need. Thanks. 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? etc ;) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
On Mon, May 17, 2010 at 01:02:30PM +0300, felipe.ba...@nokia.com wrote: From: Felipe Balbi felipe.ba...@nokia.com Few architectures, like OMAP, allow you to set a debouncing time for the gpio before generating the IRQ. Teach gpiolib about that. Signed-off-by: Felipe Balbi felipe.ba...@nokia.com Reviewed-by: Mark Brown broo...@opensource.wolfsonmicro.com 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. We've lived without it for quite some time, though. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
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-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] gpiolib: introduce set_debounce method
From: Felipe Balbi felipe.ba...@nokia.com
Few architectures, like OMAP, allow you to set
a debouncing time for the gpio before generating
the IRQ. Teach gpiolib about that.
Signed-off-by: Felipe Balbi felipe.ba...@nokia.com
---
drivers/gpio/gpiolib.c | 43 +++
include/asm-generic/gpio.h |5 +
include/linux/gpio.h |5 +
3 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index eb0c3fe..cd85fd1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1447,6 +1447,49 @@ fail:
}
EXPORT_SYMBOL_GPL(gpio_direction_output);
+/**
+ * gpio_set_debounce - sets @debounce time for a @gpio
+ * @gpio: the gpio to set debounce time
+ * @debounce: debounce time is microseconds
+ */
+int gpio_set_debounce(unsigned gpio, unsigned debounce)
+{
+ unsigned long flags;
+ struct gpio_chip*chip;
+ struct gpio_desc*desc = gpio_desc[gpio];
+ int status = -EINVAL;
+
+ spin_lock_irqsave(gpio_lock, flags);
+
+ if (!gpio_is_valid(gpio))
+ goto fail;
+ chip = desc-chip;
+ if (!chip || !chip-set || !chip-set_debounce)
+ goto fail;
+ gpio -= chip-base;
+ if (gpio = chip-ngpio)
+ goto fail;
+ status = gpio_ensure_requested(desc, gpio);
+ if (status 0)
+ goto fail;
+
+ /* now we know the gpio is valid and chip won't vanish */
+
+ spin_unlock_irqrestore(gpio_lock, flags);
+
+ might_sleep_if(extra_checks chip-can_sleep);
+
+ return chip-set_debounce(chip, gpio, debounce);
+
+fail:
+ spin_unlock_irqrestore(gpio_lock, flags);
+ if (status)
+ pr_debug(%s: gpio-%d status %d\n,
+ __func__, gpio, status);
+
+ return status;
+}
+EXPORT_SYMBOL_GPL(gpio_set_debounce);
/* I/O calls are only valid after configuration completed; the relevant
* is this a valid GPIO error checks should already have been done.
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 979c6a5..ce3eb87 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -88,6 +88,9 @@ struct gpio_chip {
unsigned offset);
int (*direction_output)(struct gpio_chip *chip,
unsigned offset, int value);
+ int (*set_debounce)(struct gpio_chip *chip,
+ unsigned offset, unsigned
debounce);
+
void(*set)(struct gpio_chip *chip,
unsigned offset, int value);
@@ -121,6 +124,8 @@ extern void gpio_free(unsigned gpio);
extern int gpio_direction_input(unsigned gpio);
extern int gpio_direction_output(unsigned gpio, int value);
+extern int gpio_set_debounce(unsigned gpio, unsigned debounce);
+
extern int gpio_get_value_cansleep(unsigned gpio);
extern void gpio_set_value_cansleep(unsigned gpio, int value);
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 4e949a5..03f616b 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -51,6 +51,11 @@ static inline int gpio_direction_output(unsigned gpio, int
value)
return -ENOSYS;
}
+static inline int gpio_set_debounce(unsigned gpio, unsigned debounce)
+{
+ return -ENOSYS;
+}
+
static inline int gpio_get_value(unsigned gpio)
{
/* GPIO can never have been requested or set as {in,out}put */
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
