[PATCH 0/6] teach gpiolib about gpio debouncing

[PATCH 0/6] teach gpiolib about gpio debouncing

[felipe.balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

Hi all,

I'm resending this series since no-one has had any further
comments for quite some time.

Adding Andrew Morton to the loop also since David Brownell
didn't pick the patch neither comment to any version of it.

Felipe Balbi (5):
  gpiolib: introduce set_debounce method
  arm: omap: gpio: implement set_debounce method
  arm: omap: switch over to gpio_set_debounce
  arm: omap: remove the unused omap_gpio_set_debounce methods
  arm: omap: move all gpio defines to plat/gpio.h

 arch/arm/mach-omap2/board-3430sdp.c        |    4 +-
 arch/arm/mach-omap2/board-ldp.c            |    3 +-
 arch/arm/mach-omap2/board-omap3evm.c       |    4 +-
 arch/arm/mach-omap2/board-omap3pandora.c   |    4 +-
 arch/arm/mach-omap2/board-omap3touchbook.c |    3 +-
 arch/arm/plat-omap/gpio.c                  |  247 ++++++----------------------
 arch/arm/plat-omap/include/plat/gpio.h     |  143 ++++++++++++++++
 drivers/gpio/gpiolib.c                     |   43 +++++
 include/asm-generic/gpio.h                 |    5 +
 include/linux/gpio.h                       |    5 +
 10 files changed, 252 insertions(+), 209 deletions(-)

[PATCH 1/5] gpiolib: introduce set_debounce method

[felipe.balbi]

From: Felipe Balbi <felipe.balbi@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.balbi@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

[PATCH 2/5] arm: omap: gpio: implement set_debounce method

[felipe.balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

OMAP support debouncing of gpio lines, implement
the method using gpiolib.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/plat-omap/gpio.c |   68 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 45a225d..4a17dc4 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -612,6 +612,59 @@ do {	\
 	__raw_writel(l, base + reg); \
 } while(0)
 
+/**
+ * _set_gpio_debounce - low level gpio debounce time
+ * @bank: the gpio bank we're acting upon
+ * @gpio: the gpio number on this @gpio
+ * @debounce: debounce time to use
+ *
+ * OMAP's debounce time is in 31us steps so we need
+ * to convert and round up to the closest unit.
+ */
+static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio,
+		unsigned debounce)
+{
+	void __iomem		*reg = bank->base;
+	u32			val;
+	u32			l;
+
+	if (debounce < 32)
+		debounce = 0x01;
+	else if (debounce > 7936)
+		debounce = 0xff;
+	else
+		debounce = (debounce / 0x1f) - 1;
+
+	l = 1 << get_gpio_index(gpio);
+
+	if (cpu_is_omap44xx())
+		reg += OMAP4_GPIO_DEBOUNCINGTIME;
+	else
+		reg += OMAP24XX_GPIO_DEBOUNCE_VAL;
+
+	__raw_writel(debounce, reg);
+
+	reg = bank->base;
+	if (cpu_is_omap44xx())
+		reg += OMAP4_GPIO_DEBOUNCENABLE;
+	else
+		reg += OMAP24XX_GPIO_DEBOUNCE_EN;
+
+	val = __raw_readl(reg);
+
+	if (debounce) {
+		val |= l;
+		if (cpu_is_omap34xx() || cpu_is_omap44xx())
+			clk_enable(bank->dbck);
+	} else {
+		val &= ~l;
+		if (cpu_is_omap34xx() || cpu_is_omap44xx())
+			clk_disable(bank->dbck);
+	}
+
+	__raw_writel(val, reg);
+}
+
 void omap_set_gpio_debounce(int gpio, int enable)
 {
 	struct gpio_bank *bank;
@@ -1608,6 +1661,20 @@ static int gpio_output(struct gpio_chip *chip, unsigned offset, int value)
 	return 0;
 }
 
+static int gpio_debounce(struct gpio_chip *chip, unsigned offset,
+		unsigned debounce)
+{
+	struct gpio_bank *bank;
+	unsigned long flags;
+
+	bank = container_of(chip, struct gpio_bank, chip);
+	spin_lock_irqsave(&bank->lock, flags);
+	_set_gpio_debounce(bank, offset, debounce);
+	spin_unlock_irqrestore(&bank->lock, flags);
+
+	return 0;
+}
+
 static void gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 {
 	struct gpio_bank *bank;
@@ -1860,6 +1927,7 @@ static int __init _omap_gpio_init(void)
 		bank->chip.direction_input = gpio_input;
 		bank->chip.get = gpio_get;
 		bank->chip.direction_output = gpio_output;
+		bank->chip.set_debounce = gpio_debounce;
 		bank->chip.set = gpio_set;
 		bank->chip.to_irq = gpio_2irq;
 		if (bank_is_mpuio(bank)) {
-- 
1.7.1

[PATCH 3/5] arm: omap: switch over to gpio_set_debounce

[felipe.balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

stop using the omap-specific implementations for gpio
debouncing now that gpiolib provides its own support.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/mach-omap2/board-3430sdp.c        |    4 +---
 arch/arm/mach-omap2/board-ldp.c            |    3 +--
 arch/arm/mach-omap2/board-omap3evm.c       |    4 +---
 arch/arm/mach-omap2/board-omap3pandora.c   |    4 ++--
 arch/arm/mach-omap2/board-omap3touchbook.c |    3 +--
 5 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index 5822bcf..8678b19 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -137,9 +137,7 @@ static void ads7846_dev_init(void)
 	}
 
 	gpio_direction_input(ts_gpio);
-
-	omap_set_gpio_debounce(ts_gpio, 1);
-	omap_set_gpio_debounce_time(ts_gpio, 0xa);
+	gpio_set_debounce(ts_gpio, 310);
 }
 
 static int ads7846_get_pendown_state(void)
diff --git a/arch/arm/mach-omap2/board-ldp.c b/arch/arm/mach-omap2/board-ldp.c
index 5fcb52e..fefd7e6 100644
--- a/arch/arm/mach-omap2/board-ldp.c
+++ b/arch/arm/mach-omap2/board-ldp.c
@@ -209,8 +209,7 @@ static void ads7846_dev_init(void)
 	}
 
 	gpio_direction_input(ts_gpio);
-	omap_set_gpio_debounce(ts_gpio, 1);
-	omap_set_gpio_debounce_time(ts_gpio, 0xa);
+	gpio_set_debounce(ts_gpio, 310);
 }
 
 static int ads7846_get_pendown_state(void)
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index 017bb2f..e80f9da 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -579,9 +579,7 @@ static void ads7846_dev_init(void)
 		printk(KERN_ERR "can't get ads7846 pen down GPIO\n");
 
 	gpio_direction_input(OMAP3_EVM_TS_GPIO);
-
-	omap_set_gpio_debounce(OMAP3_EVM_TS_GPIO, 1);
-	omap_set_gpio_debounce_time(OMAP3_EVM_TS_GPIO, 0xa);
+	gpio_set_debounce(OMAP3_EVM_TS_GPIO, 310);
 }
 
 static int ads7846_get_pendown_state(void)
diff --git a/arch/arm/mach-omap2/board-omap3pandora.c b/arch/arm/mach-omap2/board-omap3pandora.c
index 395d049..db06dc9 100644
--- a/arch/arm/mach-omap2/board-omap3pandora.c
+++ b/arch/arm/mach-omap2/board-omap3pandora.c
@@ -130,8 +130,8 @@ static struct platform_device pandora_keys_gpio = {
 static void __init pandora_keys_gpio_init(void)
 {
 	/* set debounce time for GPIO banks 4 and 6 */
-	omap_set_gpio_debounce_time(32 * 3, GPIO_DEBOUNCE_TIME);
-	omap_set_gpio_debounce_time(32 * 5, GPIO_DEBOUNCE_TIME);
+	gpio_set_debounce(32 * 3, GPIO_DEBOUNCE_TIME);
+	gpio_set_debounce(32 * 5, GPIO_DEBOUNCE_TIME);
 }
 
 static int board_keymap[] = {
diff --git a/arch/arm/mach-omap2/board-omap3touchbook.c b/arch/arm/mach-omap2/board-omap3touchbook.c
index 2504d41..2f5f823 100644
--- a/arch/arm/mach-omap2/board-omap3touchbook.c
+++ b/arch/arm/mach-omap2/board-omap3touchbook.c
@@ -328,8 +328,7 @@ static void __init omap3_ads7846_init(void)
 	}
 
 	gpio_direction_input(OMAP3_TS_GPIO);
-	omap_set_gpio_debounce(OMAP3_TS_GPIO, 1);
-	omap_set_gpio_debounce_time(OMAP3_TS_GPIO, 0xa);
+	gpio_set_debounce(OMAP3_TS_GPIO, 310);
 }
 
 static struct ads7846_platform_data ads7846_config = {
-- 
1.7.1

[PATCH 4/5] arm: omap: remove the unused omap_gpio_set_debounce methods

[felipe.balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

nobody uses that anymore, so remove and expect drivers
to use the gpiolib implementation.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/plat-omap/gpio.c |   73 ---------------------------------------------
 1 files changed, 0 insertions(+), 73 deletions(-)

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 4a17dc4..95f92b0 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -665,79 +665,6 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio,
 	__raw_writel(val, reg);
 }
 
-void omap_set_gpio_debounce(int gpio, int enable)
-{
-	struct gpio_bank *bank;
-	void __iomem *reg;
-	unsigned long flags;
-	u32 val, l = 1 << get_gpio_index(gpio);
-
-	if (cpu_class_is_omap1())
-		return;
-
-	bank = get_gpio_bank(gpio);
-	reg = bank->base;
-
-	if (cpu_is_omap44xx())
-		reg += OMAP4_GPIO_DEBOUNCENABLE;
-	else
-		reg += OMAP24XX_GPIO_DEBOUNCE_EN;
-
-	if (!(bank->mod_usage & l)) {
-		printk(KERN_ERR "GPIO %d not requested\n", gpio);
-		return;
-	}
-
-	spin_lock_irqsave(&bank->lock, flags);
-	val = __raw_readl(reg);
-
-	if (enable && !(val & l))
-		val |= l;
-	else if (!enable && (val & l))
-		val &= ~l;
-	else
-		goto done;
-
-	if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
-		if (enable)
-			clk_enable(bank->dbck);
-		else
-			clk_disable(bank->dbck);
-	}
-
-	__raw_writel(val, reg);
-done:
-	spin_unlock_irqrestore(&bank->lock, flags);
-}
-EXPORT_SYMBOL(omap_set_gpio_debounce);
-
-void omap_set_gpio_debounce_time(int gpio, int enc_time)
-{
-	struct gpio_bank *bank;
-	void __iomem *reg;
-
-	if (cpu_class_is_omap1())
-		return;
-
-	bank = get_gpio_bank(gpio);
-	reg = bank->base;
-
-	if (!bank->mod_usage) {
-		printk(KERN_ERR "GPIO not requested\n");
-		return;
-	}
-
-	enc_time &= 0xff;
-
-	if (cpu_is_omap44xx())
-		reg += OMAP4_GPIO_DEBOUNCINGTIME;
-	else
-		reg += OMAP24XX_GPIO_DEBOUNCE_VAL;
-
-	__raw_writel(enc_time, reg);
-}
-EXPORT_SYMBOL(omap_set_gpio_debounce_time);
-
 #ifdef CONFIG_ARCH_OMAP2PLUS
 static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
 						int trigger)
-- 
1.7.1

[PATCH 5/5] arm: omap: move all gpio defines to plat/gpio.h

[felipe.balbi]

From: Felipe Balbi <felipe.balbi@nokia.com>

move everything to a common location. No functional
changes.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/plat-omap/gpio.c              |  143 --------------------------------
 arch/arm/plat-omap/include/plat/gpio.h |  143 ++++++++++++++++++++++++++++++++
 2 files changed, 143 insertions(+), 143 deletions(-)

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 95f92b0..b4ff1a4 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -28,149 +28,6 @@
 #include <mach/gpio.h>
 #include <asm/mach/irq.h>
 
-/*
- * OMAP1510 GPIO registers
- */
-#define OMAP1510_GPIO_BASE		0xfffce000
-#define OMAP1510_GPIO_DATA_INPUT	0x00
-#define OMAP1510_GPIO_DATA_OUTPUT	0x04
-#define OMAP1510_GPIO_DIR_CONTROL	0x08
-#define OMAP1510_GPIO_INT_CONTROL	0x0c
-#define OMAP1510_GPIO_INT_MASK		0x10
-#define OMAP1510_GPIO_INT_STATUS	0x14
-#define OMAP1510_GPIO_PIN_CONTROL	0x18
-
-#define OMAP1510_IH_GPIO_BASE		64
-
-/*
- * OMAP1610 specific GPIO registers
- */
-#define OMAP1610_GPIO1_BASE		0xfffbe400
-#define OMAP1610_GPIO2_BASE		0xfffbec00
-#define OMAP1610_GPIO3_BASE		0xfffbb400
-#define OMAP1610_GPIO4_BASE		0xfffbbc00
-#define OMAP1610_GPIO_REVISION		0x0000
-#define OMAP1610_GPIO_SYSCONFIG		0x0010
-#define OMAP1610_GPIO_SYSSTATUS		0x0014
-#define OMAP1610_GPIO_IRQSTATUS1	0x0018
-#define OMAP1610_GPIO_IRQENABLE1	0x001c
-#define OMAP1610_GPIO_WAKEUPENABLE	0x0028
-#define OMAP1610_GPIO_DATAIN		0x002c
-#define OMAP1610_GPIO_DATAOUT		0x0030
-#define OMAP1610_GPIO_DIRECTION		0x0034
-#define OMAP1610_GPIO_EDGE_CTRL1	0x0038
-#define OMAP1610_GPIO_EDGE_CTRL2	0x003c
-#define OMAP1610_GPIO_CLEAR_IRQENABLE1	0x009c
-#define OMAP1610_GPIO_CLEAR_WAKEUPENA	0x00a8
-#define OMAP1610_GPIO_CLEAR_DATAOUT	0x00b0
-#define OMAP1610_GPIO_SET_IRQENABLE1	0x00dc
-#define OMAP1610_GPIO_SET_WAKEUPENA	0x00e8
-#define OMAP1610_GPIO_SET_DATAOUT	0x00f0
-
-/*
- * OMAP7XX specific GPIO registers
- */
-#define OMAP7XX_GPIO1_BASE		0xfffbc000
-#define OMAP7XX_GPIO2_BASE		0xfffbc800
-#define OMAP7XX_GPIO3_BASE		0xfffbd000
-#define OMAP7XX_GPIO4_BASE		0xfffbd800
-#define OMAP7XX_GPIO5_BASE		0xfffbe000
-#define OMAP7XX_GPIO6_BASE		0xfffbe800
-#define OMAP7XX_GPIO_DATA_INPUT		0x00
-#define OMAP7XX_GPIO_DATA_OUTPUT	0x04
-#define OMAP7XX_GPIO_DIR_CONTROL	0x08
-#define OMAP7XX_GPIO_INT_CONTROL	0x0c
-#define OMAP7XX_GPIO_INT_MASK		0x10
-#define OMAP7XX_GPIO_INT_STATUS		0x14
-
-#define OMAP1_MPUIO_VBASE		OMAP1_MPUIO_BASE
-
-/*
- * omap24xx specific GPIO registers
- */
-#define OMAP242X_GPIO1_BASE		0x48018000
-#define OMAP242X_GPIO2_BASE		0x4801a000
-#define OMAP242X_GPIO3_BASE		0x4801c000
-#define OMAP242X_GPIO4_BASE		0x4801e000
-
-#define OMAP243X_GPIO1_BASE		0x4900C000
-#define OMAP243X_GPIO2_BASE		0x4900E000
-#define OMAP243X_GPIO3_BASE		0x49010000
-#define OMAP243X_GPIO4_BASE		0x49012000
-#define OMAP243X_GPIO5_BASE		0x480B6000
-
-#define OMAP24XX_GPIO_REVISION		0x0000
-#define OMAP24XX_GPIO_SYSCONFIG		0x0010
-#define OMAP24XX_GPIO_SYSSTATUS		0x0014
-#define OMAP24XX_GPIO_IRQSTATUS1	0x0018
-#define OMAP24XX_GPIO_IRQSTATUS2	0x0028
-#define OMAP24XX_GPIO_IRQENABLE2	0x002c
-#define OMAP24XX_GPIO_IRQENABLE1	0x001c
-#define OMAP24XX_GPIO_WAKE_EN		0x0020
-#define OMAP24XX_GPIO_CTRL		0x0030
-#define OMAP24XX_GPIO_OE		0x0034
-#define OMAP24XX_GPIO_DATAIN		0x0038
-#define OMAP24XX_GPIO_DATAOUT		0x003c
-#define OMAP24XX_GPIO_LEVELDETECT0	0x0040
-#define OMAP24XX_GPIO_LEVELDETECT1	0x0044
-#define OMAP24XX_GPIO_RISINGDETECT	0x0048
-#define OMAP24XX_GPIO_FALLINGDETECT	0x004c
-#define OMAP24XX_GPIO_DEBOUNCE_EN	0x0050
-#define OMAP24XX_GPIO_DEBOUNCE_VAL	0x0054
-#define OMAP24XX_GPIO_CLEARIRQENABLE1	0x0060
-#define OMAP24XX_GPIO_SETIRQENABLE1	0x0064
-#define OMAP24XX_GPIO_CLEARWKUENA	0x0080
-#define OMAP24XX_GPIO_SETWKUENA		0x0084
-#define OMAP24XX_GPIO_CLEARDATAOUT	0x0090
-#define OMAP24XX_GPIO_SETDATAOUT	0x0094
-
-#define OMAP4_GPIO_REVISION		0x0000
-#define OMAP4_GPIO_SYSCONFIG		0x0010
-#define OMAP4_GPIO_EOI			0x0020
-#define OMAP4_GPIO_IRQSTATUSRAW0	0x0024
-#define OMAP4_GPIO_IRQSTATUSRAW1	0x0028
-#define OMAP4_GPIO_IRQSTATUS0		0x002c
-#define OMAP4_GPIO_IRQSTATUS1		0x0030
-#define OMAP4_GPIO_IRQSTATUSSET0	0x0034
-#define OMAP4_GPIO_IRQSTATUSSET1	0x0038
-#define OMAP4_GPIO_IRQSTATUSCLR0	0x003c
-#define OMAP4_GPIO_IRQSTATUSCLR1	0x0040
-#define OMAP4_GPIO_IRQWAKEN0		0x0044
-#define OMAP4_GPIO_IRQWAKEN1		0x0048
-#define OMAP4_GPIO_SYSSTATUS		0x0104
-#define OMAP4_GPIO_CTRL			0x0130
-#define OMAP4_GPIO_OE			0x0134
-#define OMAP4_GPIO_DATAIN		0x0138
-#define OMAP4_GPIO_DATAOUT		0x013c
-#define OMAP4_GPIO_LEVELDETECT0		0x0140
-#define OMAP4_GPIO_LEVELDETECT1		0x0144
-#define OMAP4_GPIO_RISINGDETECT		0x0148
-#define OMAP4_GPIO_FALLINGDETECT	0x014c
-#define OMAP4_GPIO_DEBOUNCENABLE	0x0150
-#define OMAP4_GPIO_DEBOUNCINGTIME	0x0154
-#define OMAP4_GPIO_CLEARDATAOUT		0x0190
-#define OMAP4_GPIO_SETDATAOUT		0x0194
-/*
- * omap34xx specific GPIO registers
- */
-
-#define OMAP34XX_GPIO1_BASE		0x48310000
-#define OMAP34XX_GPIO2_BASE		0x49050000
-#define OMAP34XX_GPIO3_BASE		0x49052000
-#define OMAP34XX_GPIO4_BASE		0x49054000
-#define OMAP34XX_GPIO5_BASE		0x49056000
-#define OMAP34XX_GPIO6_BASE		0x49058000
-
-/*
- * OMAP44XX  specific GPIO registers
- */
-#define OMAP44XX_GPIO1_BASE             0x4a310000
-#define OMAP44XX_GPIO2_BASE             0x48055000
-#define OMAP44XX_GPIO3_BASE             0x48057000
-#define OMAP44XX_GPIO4_BASE             0x48059000
-#define OMAP44XX_GPIO5_BASE             0x4805B000
-#define OMAP44XX_GPIO6_BASE             0x4805D000
-
 struct gpio_bank {
 	unsigned long pbase;
 	void __iomem *base;
diff --git a/arch/arm/plat-omap/include/plat/gpio.h b/arch/arm/plat-omap/include/plat/gpio.h
index de7c547..b444f5f 100644
--- a/arch/arm/plat-omap/include/plat/gpio.h
+++ b/arch/arm/plat-omap/include/plat/gpio.h
@@ -71,6 +71,149 @@
 				 IH_MPUIO_BASE + ((nr) & 0x0f) : \
 				 IH_GPIO_BASE + (nr))
 
+/*
+ * OMAP1510 GPIO registers
+ */
+#define OMAP1510_GPIO_BASE		0xfffce000
+#define OMAP1510_GPIO_DATA_INPUT	0x00
+#define OMAP1510_GPIO_DATA_OUTPUT	0x04
+#define OMAP1510_GPIO_DIR_CONTROL	0x08
+#define OMAP1510_GPIO_INT_CONTROL	0x0c
+#define OMAP1510_GPIO_INT_MASK		0x10
+#define OMAP1510_GPIO_INT_STATUS	0x14
+#define OMAP1510_GPIO_PIN_CONTROL	0x18
+
+#define OMAP1510_IH_GPIO_BASE		64
+
+/*
+ * OMAP1610 specific GPIO registers
+ */
+#define OMAP1610_GPIO1_BASE		0xfffbe400
+#define OMAP1610_GPIO2_BASE		0xfffbec00
+#define OMAP1610_GPIO3_BASE		0xfffbb400
+#define OMAP1610_GPIO4_BASE		0xfffbbc00
+#define OMAP1610_GPIO_REVISION		0x0000
+#define OMAP1610_GPIO_SYSCONFIG		0x0010
+#define OMAP1610_GPIO_SYSSTATUS		0x0014
+#define OMAP1610_GPIO_IRQSTATUS1	0x0018
+#define OMAP1610_GPIO_IRQENABLE1	0x001c
+#define OMAP1610_GPIO_WAKEUPENABLE	0x0028
+#define OMAP1610_GPIO_DATAIN		0x002c
+#define OMAP1610_GPIO_DATAOUT		0x0030
+#define OMAP1610_GPIO_DIRECTION		0x0034
+#define OMAP1610_GPIO_EDGE_CTRL1	0x0038
+#define OMAP1610_GPIO_EDGE_CTRL2	0x003c
+#define OMAP1610_GPIO_CLEAR_IRQENABLE1	0x009c
+#define OMAP1610_GPIO_CLEAR_WAKEUPENA	0x00a8
+#define OMAP1610_GPIO_CLEAR_DATAOUT	0x00b0
+#define OMAP1610_GPIO_SET_IRQENABLE1	0x00dc
+#define OMAP1610_GPIO_SET_WAKEUPENA	0x00e8
+#define OMAP1610_GPIO_SET_DATAOUT	0x00f0
+
+/*
+ * OMAP7XX specific GPIO registers
+ */
+#define OMAP7XX_GPIO1_BASE		0xfffbc000
+#define OMAP7XX_GPIO2_BASE		0xfffbc800
+#define OMAP7XX_GPIO3_BASE		0xfffbd000
+#define OMAP7XX_GPIO4_BASE		0xfffbd800
+#define OMAP7XX_GPIO5_BASE		0xfffbe000
+#define OMAP7XX_GPIO6_BASE		0xfffbe800
+#define OMAP7XX_GPIO_DATA_INPUT		0x00
+#define OMAP7XX_GPIO_DATA_OUTPUT	0x04
+#define OMAP7XX_GPIO_DIR_CONTROL	0x08
+#define OMAP7XX_GPIO_INT_CONTROL	0x0c
+#define OMAP7XX_GPIO_INT_MASK		0x10
+#define OMAP7XX_GPIO_INT_STATUS		0x14
+
+#define OMAP1_MPUIO_VBASE		OMAP1_MPUIO_BASE
+
+/*
+ * omap24xx specific GPIO registers
+ */
+#define OMAP242X_GPIO1_BASE		0x48018000
+#define OMAP242X_GPIO2_BASE		0x4801a000
+#define OMAP242X_GPIO3_BASE		0x4801c000
+#define OMAP242X_GPIO4_BASE		0x4801e000
+
+#define OMAP243X_GPIO1_BASE		0x4900C000
+#define OMAP243X_GPIO2_BASE		0x4900E000
+#define OMAP243X_GPIO3_BASE		0x49010000
+#define OMAP243X_GPIO4_BASE		0x49012000
+#define OMAP243X_GPIO5_BASE		0x480B6000
+
+#define OMAP24XX_GPIO_REVISION		0x0000
+#define OMAP24XX_GPIO_SYSCONFIG		0x0010
+#define OMAP24XX_GPIO_SYSSTATUS		0x0014
+#define OMAP24XX_GPIO_IRQSTATUS1	0x0018
+#define OMAP24XX_GPIO_IRQSTATUS2	0x0028
+#define OMAP24XX_GPIO_IRQENABLE2	0x002c
+#define OMAP24XX_GPIO_IRQENABLE1	0x001c
+#define OMAP24XX_GPIO_WAKE_EN		0x0020
+#define OMAP24XX_GPIO_CTRL		0x0030
+#define OMAP24XX_GPIO_OE		0x0034
+#define OMAP24XX_GPIO_DATAIN		0x0038
+#define OMAP24XX_GPIO_DATAOUT		0x003c
+#define OMAP24XX_GPIO_LEVELDETECT0	0x0040
+#define OMAP24XX_GPIO_LEVELDETECT1	0x0044
+#define OMAP24XX_GPIO_RISINGDETECT	0x0048
+#define OMAP24XX_GPIO_FALLINGDETECT	0x004c
+#define OMAP24XX_GPIO_DEBOUNCE_EN	0x0050
+#define OMAP24XX_GPIO_DEBOUNCE_VAL	0x0054
+#define OMAP24XX_GPIO_CLEARIRQENABLE1	0x0060
+#define OMAP24XX_GPIO_SETIRQENABLE1	0x0064
+#define OMAP24XX_GPIO_CLEARWKUENA	0x0080
+#define OMAP24XX_GPIO_SETWKUENA		0x0084
+#define OMAP24XX_GPIO_CLEARDATAOUT	0x0090
+#define OMAP24XX_GPIO_SETDATAOUT	0x0094
+
+#define OMAP4_GPIO_REVISION		0x0000
+#define OMAP4_GPIO_SYSCONFIG		0x0010
+#define OMAP4_GPIO_EOI			0x0020
+#define OMAP4_GPIO_IRQSTATUSRAW0	0x0024
+#define OMAP4_GPIO_IRQSTATUSRAW1	0x0028
+#define OMAP4_GPIO_IRQSTATUS0		0x002c
+#define OMAP4_GPIO_IRQSTATUS1		0x0030
+#define OMAP4_GPIO_IRQSTATUSSET0	0x0034
+#define OMAP4_GPIO_IRQSTATUSSET1	0x0038
+#define OMAP4_GPIO_IRQSTATUSCLR0	0x003c
+#define OMAP4_GPIO_IRQSTATUSCLR1	0x0040
+#define OMAP4_GPIO_IRQWAKEN0		0x0044
+#define OMAP4_GPIO_IRQWAKEN1		0x0048
+#define OMAP4_GPIO_SYSSTATUS		0x0104
+#define OMAP4_GPIO_CTRL			0x0130
+#define OMAP4_GPIO_OE			0x0134
+#define OMAP4_GPIO_DATAIN		0x0138
+#define OMAP4_GPIO_DATAOUT		0x013c
+#define OMAP4_GPIO_LEVELDETECT0		0x0140
+#define OMAP4_GPIO_LEVELDETECT1		0x0144
+#define OMAP4_GPIO_RISINGDETECT		0x0148
+#define OMAP4_GPIO_FALLINGDETECT	0x014c
+#define OMAP4_GPIO_DEBOUNCENABLE	0x0150
+#define OMAP4_GPIO_DEBOUNCINGTIME	0x0154
+#define OMAP4_GPIO_CLEARDATAOUT		0x0190
+#define OMAP4_GPIO_SETDATAOUT		0x0194
+
+/*
+ * omap34xx specific GPIO registers
+ */
+#define OMAP34XX_GPIO1_BASE		0x48310000
+#define OMAP34XX_GPIO2_BASE		0x49050000
+#define OMAP34XX_GPIO3_BASE		0x49052000
+#define OMAP34XX_GPIO4_BASE		0x49054000
+#define OMAP34XX_GPIO5_BASE		0x49056000
+#define OMAP34XX_GPIO6_BASE		0x49058000
+
+/*
+ * OMAP44XX  specific GPIO registers
+ */
+#define OMAP44XX_GPIO1_BASE             0x4a310000
+#define OMAP44XX_GPIO2_BASE             0x48055000
+#define OMAP44XX_GPIO3_BASE             0x48057000
+#define OMAP44XX_GPIO4_BASE             0x48059000
+#define OMAP44XX_GPIO5_BASE             0x4805B000
+#define OMAP44XX_GPIO6_BASE             0x4805D000
+
 extern int omap_gpio_init(void);	/* Call from board init only */
 extern void omap2_gpio_prepare_for_retention(void);
 extern void omap2_gpio_resume_after_retention(void);
-- 
1.7.1

Re: [PATCH 5/5] arm: omap: move all gpio defines to plat/gpio.h

[Felipe Balbi]

On Mon, May 17, 2010 at 12:02:34PM +0200, Balbi Felipe (Nokia-D/Helsinki) wrote:
>From: Felipe Balbi <felipe.balbi@nokia.com>
>
>move everything to a common location. No functional
>changes.
>
>Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>

sorry, this patch shouldn't be here... it was a broken cherry-pick from 
myself. Please ignore this one.

-- 
balbi

DefectiveByDesign.org

Re: [PATCH 0/6] teach gpiolib about gpio debouncing

[Andrew Morton]

On Mon, 17 May 2010 13:02:29 +0300
felipe.balbi@nokia.com wrote:

> From: Felipe Balbi <felipe.balbi@nokia.com>
> 
> Hi all,
> 
> I'm resending this series since no-one has had any further
> comments for quite some time.
> 
> Adding Andrew Morton to the loop also since David Brownell
> didn't pick the patch neither comment to any version of it.

David's been very quiet lately - please always cc me on gpio patches.

When were these first sent?  Has there been any feedback or external
testing?

I'm not seeing anything in the changelogs which helps me to understand
how important this feature is and it's rather late to be merging
2.6.35 feature work..

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

[Andrew Morton]

On Mon, 17 May 2010 13:02:30 +0300
felipe.balbi@nokia.com wrote:

> From: Felipe Balbi <felipe.balbi@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 ;)

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

[Felipe Balbi]

Hi,

On Thu, May 20, 2010 at 11:06:00AM -0700, Andrew Morton wrote:
> 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 ;)

heh, I can post another version if you like, but this is pretty much
what other functions are doing, just following the 'standard'.

-- 
balbi

Re: [PATCH 0/6] teach gpiolib about gpio debouncing

[Felipe Balbi]

On Thu, May 20, 2010 at 11:04:31AM -0700, Andrew Morton wrote:
> On Mon, 17 May 2010 13:02:29 +0300
> felipe.balbi@nokia.com wrote:
> 
> > From: Felipe Balbi <felipe.balbi@nokia.com>
> > 
> > Hi all,
> > 
> > I'm resending this series since no-one has had any further
> > comments for quite some time.
> > 
> > Adding Andrew Morton to the loop also since David Brownell
> > didn't pick the patch neither comment to any version of it.
> 
> David's been very quiet lately - please always cc me on gpio patches.
> 
> When were these first sent?  Has there been any feedback or external
> testing?
> 
> I'm not seeing anything in the changelogs which helps me to understand
> how important this feature is and it's rather late to be merging
> 2.6.35 feature work..

I guess we could leave it to 2.6.36 since OMAP is currently the only
user and we have our own omap-specific ways of setting debounce time on
gpios. That would also give people more time to complain if they think
the api should change somehow.

-- 
balbi

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

[Alan Cox]

> > +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

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

[Andrew Morton]

On Thu, 20 May 2010 20:04:17 +0100
Alan Cox <alan@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 ;)

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

[Mark Brown]

On Mon, May 17, 2010 at 01:02:30PM +0300, felipe.balbi@nokia.com wrote:
> From: Felipe Balbi <felipe.balbi@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.balbi@nokia.com>

Reviewed-by: Mark Brown <broonie@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.

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

[Alan Cox]

> 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

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

[David Brownell]

> 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

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

[David Brownell]

> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Balbi]

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

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

[Alan Cox]

> 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

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

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

[David Brownell]

--- On Fri, 5/21/10, Alan Cox <alan@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.

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

[David Brownell]

> > 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 ... 

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

[David Brownell]

> > 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 ... 

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

[Alan Cox]

> 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

Re: [PATCH 3/5] arm: omap: switch over to gpio_set_debounce

[Grazvydas Ignotas]

On Mon, May 17, 2010 at 1:02 PM,  <felipe.balbi@nokia.com> wrote:
> From: Felipe Balbi <felipe.balbi@nokia.com>
>
> stop using the omap-specific implementations for gpio
> debouncing now that gpiolib provides its own support.

hmm, this gives me:
[    0.000000] WARNING: at drivers/gpio/gpiolib.c:104
gpio_ensure_requested+0x50/0xcc()
[    0.000000] autorequest GPIO-96
[    0.000000] Modules linked in:
[    0.000000] [<c00302bc>] (unwind_backtrace+0x0/0xec) from
[<c00510dc>] (warn_slowpath_common+0x4c/0x64)
[    0.000000] [<c00510dc>] (warn_slowpath_common+0x4c/0x64) from
[<c0051174>] (warn_slowpath_fmt+0x2c/0x3)
[    0.000000] [<c0051174>] (warn_slowpath_fmt+0x2c/0x3c) from
[<c0166c2c>] (gpio_ensure_requested+0x50/0x)
[    0.000000] [<c0166c2c>] (gpio_ensure_requested+0x50/0xcc) from
[<c0166d14>] (gpio_set_debounce+0x6c/0x)
[    0.000000] [<c0166d14>] (gpio_set_debounce+0x6c/0xb0) from
[<c000ee84>] (omap3pandora_init+0x94/0xcc)
[    0.000000] [<c000ee84>] (omap3pandora_init+0x94/0xcc) from
[<c000b300>] (customize_machine+0x18/0x24)

..and later gpio-keys is unable to request the same GPIO because of
above autorequest.

So how is this supposed to be used here, should I request all GPIOs I
want to have debouncing on, setup debounce time and release them for
gpio-keys to take? Or should I wait for this to get supported in
gpio-keys?

Re: [PATCH 3/5] arm: omap: switch over to gpio_set_debounce

[Felipe Balbi]

Hi,

On Wed, Jun 16, 2010 at 07:26:30PM +0200, ext Grazvydas Ignotas wrote:
>hmm, this gives me:

yeah, funny thing is that this patch was cooking in the mailing list for 
over a month since I said I didn't have the boards and they were 
compile-tested only. Anyways,

>[    0.000000] WARNING: at drivers/gpio/gpiolib.c:104
>gpio_ensure_requested+0x50/0xcc()
>[    0.000000] autorequest GPIO-96
>[    0.000000] Modules linked in:
>[    0.000000] [<c00302bc>] (unwind_backtrace+0x0/0xec) from
>[<c00510dc>] (warn_slowpath_common+0x4c/0x64)
>[    0.000000] [<c00510dc>] (warn_slowpath_common+0x4c/0x64) from
>[<c0051174>] (warn_slowpath_fmt+0x2c/0x3)
>[    0.000000] [<c0051174>] (warn_slowpath_fmt+0x2c/0x3c) from
>[<c0166c2c>] (gpio_ensure_requested+0x50/0x)
>[    0.000000] [<c0166c2c>] (gpio_ensure_requested+0x50/0xcc) from
>[<c0166d14>] (gpio_set_debounce+0x6c/0x)
>[    0.000000] [<c0166d14>] (gpio_set_debounce+0x6c/0xb0) from
>[<c000ee84>] (omap3pandora_init+0x94/0xcc)
>[    0.000000] [<c000ee84>] (omap3pandora_init+0x94/0xcc) from
>[<c000b300>] (customize_machine+0x18/0x24)
>
>..and later gpio-keys is unable to request the same GPIO because of
>above autorequest.
>
>So how is this supposed to be used here, should I request all GPIOs I
>want to have debouncing on, setup debounce time and release them for
>gpio-keys to take? Or should I wait for this to get supported in
>gpio-keys?

to me it looks like the *_keys_gpio_init() function should be passed 
down to gpio-keys as a ->setup() callback and you should probably also 
pass a ->teardown() which would clear debounce time when you remove the 
driver. Then only call those functions after gpio has been requested.

-- 
balbi

DefectiveByDesign.org

Re: [PATCH 3/5] arm: omap: switch over to gpio_set_debounce

[Grazvydas Ignotas]

On Thu, Jun 17, 2010 at 7:45 AM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> Hi,
>
> On Wed, Jun 16, 2010 at 07:26:30PM +0200, ext Grazvydas Ignotas wrote:
>> So how is this supposed to be used here, should I request all GPIOs I
>> want to have debouncing on, setup debounce time and release them for
>> gpio-keys to take? Or should I wait for this to get supported in
>> gpio-keys?
>
> to me it looks like the *_keys_gpio_init() function should be passed down to
> gpio-keys as a ->setup() callback and you should probably also pass a
> ->teardown() which would clear debounce time when you remove the driver.
> Then only call those functions after gpio has been requested.

gpio-keys platform data already has 'debounce_interval' member, which
is used to setup timer (if nonzero) in the driver. So I maybe it would
be better to modify gpio-keys to try to set debounce, and if that
fails make it fall back to it's current timer-based debounce
functionality?

Re: [PATCH 3/5] arm: omap: switch over to gpio_set_debounce

[Felipe Balbi]

On Sun, Jun 20, 2010 at 08:03:45PM +0300, Grazvydas Ignotas wrote:
> gpio-keys platform data already has 'debounce_interval' member, which
> is used to setup timer (if nonzero) in the driver. So I maybe it would
> be better to modify gpio-keys to try to set debounce, and if that
> fails make it fall back to it's current timer-based debounce
> functionality?

makes sense to me.

-- 
balbi