[RFC/PATCH 0/4] teach gpiolib about gpio debouncing

[RFC/PATCH 0/4] teach gpiolib about gpio debouncing

[Felipe Balbi]

Hi all,

the following *UNTESTED* patches teach gpiolib about
gpio_debouncing. If you have the board, please give it
a shot and report to this mailing.

Dave, do you have any comments regarding these ??

Felipe Balbi (4):
  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

 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                  |   85 ++++++++++------------------
 drivers/gpio/gpiolib.c                     |   39 +++++++++++++
 include/asm-generic/gpio.h                 |    5 ++
 include/linux/gpio.h                       |    5 ++
 9 files changed, 84 insertions(+), 68 deletions(-)

[RFC/PATCH 1/4] gpiolib: introduce set_debounce method

[Felipe Balbi]

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     |   39 +++++++++++++++++++++++++++++++++++++++
 include/asm-generic/gpio.h |    5 +++++
 include/linux/gpio.h       |    5 +++++
 3 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 6d1b866..27f4034 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1445,6 +1445,45 @@ fail:
 }
 EXPORT_SYMBOL_GPL(gpio_direction_output);
 
+int gpio_set_debounce(unsigned gpio, int value)
+{
+	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, value);
+
+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..0c39a79 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, int value);
+
 	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, int value);
+
 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.0.rc0.33.g7c3932

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

[Felipe Balbi]

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 |   46 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 76a347b..f9cd64c 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -612,6 +612,38 @@ do {	\
 	__raw_writel(l, base + reg); \
 } while(0)
 
+static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, int value)
+{
+	void __iomem		*reg = bank->base;
+	u32			val;
+	u32			l;
+
+	value &= 0xff;
+	l = 1 << get_gpio_index(gpio);
+
+	if (cpu_is_omap44xx())
+		reg += OMAP4_GPIO_DEBOUNCINGTIME;
+	else
+		reg += OMAP24XX_GPIO_DEBOUNCE_VAL;
+
+	__raw_writel(value, reg);
+
+	reg = bank->base;
+	if (cpu_is_omap44xx())
+		reg += OMAP4_GPIO_DEBOUNCENABLE;
+	else
+		reg += OMAP24XX_GPIO_DEBOUNCE_EN;
+
+	val = __raw_readl(reg);
+
+	if (value)
+		val |= l;
+	else
+		val &= ~l;
+
+	__raw_writel(val, reg);
+}
+
 void omap_set_gpio_debounce(int gpio, int enable)
 {
 	struct gpio_bank *bank;
@@ -1608,6 +1640,19 @@ static int gpio_output(struct gpio_chip *chip, unsigned offset, int value)
 	return 0;
 }
 
+static int gpio_debounce(struct gpio_chip *chip, unsigned offset, int value)
+{
+	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, value);
+	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 +1905,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.0.rc0.33.g7c3932

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

[Felipe Balbi]

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..7525d0d 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, 0x0a);
 }
 
 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..270c589 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, 0x0a);
 }
 
 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..bcce319 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, 0x0a);
 }
 
 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..6826786 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, 0x0a);
 }
 
 static struct ads7846_platform_data ads7846_config = {
-- 
1.7.0.rc0.33.g7c3932

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

[Felipe Balbi]

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 f9cd64c..5945be6 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -644,79 +644,6 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, int value)
 	__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.0.rc0.33.g7c3932

Re: [RFC/PATCH 2/4] arm: omap: gpio: implement set_debounce method

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 283 bytes --]

On Wed, Mar 31, 2010 at 02:27:13PM +0200, Balbi Felipe (Nokia-D/Helsinki) wrote:
>OMAP support debouncing of gpio lines, implement
>the method using gpiolib.
>
>Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>

this one was missing dbck handling. New version attached

-- 
balbi

[-- Attachment #2: 0002-arm-omap-gpio-implement-set_debounce-method.diff --]
[-- Type: text/x-diff, Size: 2439 bytes --]

>From e37f49e1006abde377cf56604077fb20340cca0a Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi@nokia.com>
Date: Wed, 31 Mar 2010 15:17:29 +0300
Subject: [RFC/PATCH 2/4] arm: omap: gpio: implement set_debounce method

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 |   51 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 76a347b..9df434e 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -612,6 +612,43 @@ do {	\
 	__raw_writel(l, base + reg); \
 } while(0)
 
+static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio, int value)
+{
+	void __iomem		*reg = bank->base;
+	u32			val;
+	u32			l;
+
+	value &= 0xff;
+	l = 1 << get_gpio_index(gpio);
+
+	if (cpu_is_omap44xx())
+		reg += OMAP4_GPIO_DEBOUNCINGTIME;
+	else
+		reg += OMAP24XX_GPIO_DEBOUNCE_VAL;
+
+	__raw_writel(value, reg);
+
+	reg = bank->base;
+	if (cpu_is_omap44xx())
+		reg += OMAP4_GPIO_DEBOUNCENABLE;
+	else
+		reg += OMAP24XX_GPIO_DEBOUNCE_EN;
+
+	val = __raw_readl(reg);
+
+	if (value) {
+		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 +1645,19 @@ static int gpio_output(struct gpio_chip *chip, unsigned offset, int value)
 	return 0;
 }
 
+static int gpio_debounce(struct gpio_chip *chip, unsigned offset, int value)
+{
+	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, value);
+	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 +1910,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.0.rc0.33.g7c3932

Re: [RFC/PATCH 1/4] gpiolib: introduce set_debounce method

[Mark Brown]

On Wed, Mar 31, 2010 at 03:27:12PM +0300, Felipe Balbi wrote:
> Few architectures, like OMAP, allow you to set
> a debouncing time for the gpio before generating
> the IRQ. Teach gpiolib about that.

More than a few, I suspect, and certainly some off-CPU GPIO controllers.

> +int gpio_set_debounce(unsigned gpio, int value)
> +{

What are the intended semantics of value here?

Re: [RFC/PATCH 1/4] gpiolib: introduce set_debounce method

[Felipe Balbi]

On Wed, Mar 31, 2010 at 02:46:28PM +0100, Mark Brown wrote:
> > +int gpio_set_debounce(unsigned gpio, int value)
> > +{
> 
> What are the intended semantics of value here?

debounce time. I just used value because other functions also call it
value.

-- 
balbi

Re: [RFC/PATCH 1/4] gpiolib: introduce set_debounce method

[Felipe Balbi]

On Wed, Mar 31, 2010 at 04:50:59PM +0200, ext Felipe Balbi wrote:
>On Wed, Mar 31, 2010 at 02:46:28PM +0100, Mark Brown wrote:
>> > +int gpio_set_debounce(unsigned gpio, int value)
>> > +{
>>
>> What are the intended semantics of value here?
>
>debounce time. I just used value because other functions also call it

oh yeah, in miliseconds.

>value.

maybe we could call it debounce. And switch to unsigned. Calling 
gpio_set_debounce() with a 0 debounce time should disable it, btw.

-- 
balbi

Re: [RFC/PATCH 1/4] gpiolib: introduce set_debounce method

[Mark Brown]

On Wed, Mar 31, 2010 at 05:59:19PM +0300, Felipe Balbi wrote:
> On Wed, Mar 31, 2010 at 04:50:59PM +0200, ext Felipe Balbi wrote:

> >debounce time. I just used value because other functions also call it

> oh yeah, in miliseconds.

> >value.

> maybe we could call it debounce. And switch to unsigned. Calling
> gpio_set_debounce() with a 0 debounce time should disable it, btw.

Sounds reasonable, a comment would help also.

Re: [RFC/PATCH 1/4] gpiolib: introduce set_debounce method

[Felipe Balbi]

On Wed, Mar 31, 2010 at 05:04:03PM +0200, ext Mark Brown wrote:
>On Wed, Mar 31, 2010 at 05:59:19PM +0300, Felipe Balbi wrote:
>> On Wed, Mar 31, 2010 at 04:50:59PM +0200, ext Felipe Balbi wrote:
>
>> >debounce time. I just used value because other functions also call it
>
>> oh yeah, in miliseconds.
>
>> >value.
>
>> maybe we could call it debounce. And switch to unsigned. Calling
>> gpio_set_debounce() with a 0 debounce time should disable it, btw.
>
>Sounds reasonable, a comment would help also.

I'm adding that. And now that I think about it, it's better to pass 
debounce time i microseconds. What do you think ?

-- 
balbi

Re: [RFC/PATCH 1/4] gpiolib: introduce set_debounce method

[Mark Brown]

On Wed, Mar 31, 2010 at 06:06:44PM +0300, Felipe Balbi wrote:
> On Wed, Mar 31, 2010 at 05:04:03PM +0200, ext Mark Brown wrote:

> >Sounds reasonable, a comment would help also.

> I'm adding that. And now that I think about it, it's better to pass
> debounce time i microseconds. What do you think ?

Microseconds is certainly safer, it's much easier to multiply everything
by 1000 than run out of room.

Re: [RFC/PATCH 1/4] gpiolib: introduce set_debounce method

[Felipe Balbi]

On Wed, Mar 31, 2010 at 05:11:26PM +0200, ext Mark Brown wrote:
>On Wed, Mar 31, 2010 at 06:06:44PM +0300, Felipe Balbi wrote:
>> On Wed, Mar 31, 2010 at 05:04:03PM +0200, ext Mark Brown wrote:
>
>> >Sounds reasonable, a comment would help also.
>
>> I'm adding that. And now that I think about it, it's better to pass
>> debounce time i microseconds. What do you think ?
>
>Microseconds is certainly safer, it's much easier to multiply everything
>by 1000 than run out of room.

ok, changing that too :-) new version coming soon.

-- 
balbi

[RFC/PATCHv2 0/4] teach gpiolib about debouncing

[Felipe Balbi]

Changes since v1:
	- add a comment to gpio_set_debounce() describing the
	  arguments

Hi all,

the following patches teach gpiolib about debouncing
time and implement support for OMAP.

Felipe Balbi (4):
  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

 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                  |  103 +++++++++++++---------------
 drivers/gpio/gpiolib.c                     |   43 ++++++++++++
 include/asm-generic/gpio.h                 |    5 ++
 include/linux/gpio.h                       |    5 ++
 9 files changed, 108 insertions(+), 66 deletions(-)

[RFC/PATCHv2 1/4] gpiolib: introduce set_debounce method

[Felipe Balbi]

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 6d1b866..da2e20a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1445,6 +1445,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.0.rc0.33.g7c3932

[RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Felipe Balbi]

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 76a347b..345ed2c 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, value);
+	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.0.rc0.33.g7c3932

[RFC/PATCHv2 3/4] arm: omap: switch over to gpio_set_debounce

[Felipe Balbi]

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.0.rc0.33.g7c3932

[RFC/PATCHv2 4/4] arm: omap: remove the unused omap_gpio_set_debounce methods

[Felipe Balbi]

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 345ed2c..74d5062 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.0.rc0.33.g7c3932

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Mark Brown]

On Wed, Mar 31, 2010 at 06:35:10PM +0300, Felipe Balbi wrote:
> +	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);
> +	}

Will these do the right thing with the clocks for noop transitions or
tweaks to the debounce time?  I'm not familiar with the OMAP clock
framework, it could handle this.

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Felipe Balbi]

Hi,

On Wed, Mar 31, 2010 at 06:21:58PM +0200, ext Mark Brown wrote:
>On Wed, Mar 31, 2010 at 06:35:10PM +0300, Felipe Balbi wrote:
>> +	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);
>> +	}
>
>Will these do the right thing with the clocks for noop transitions or
>tweaks to the debounce time?  I'm not familiar with the OMAP clock
>framework, it could handle this.

good catch. We need to be a bit smarter here. I'll leave that for 
tomorrow. already 19:30 :-p

maybe adding a flag to gpio_bank that gets set when we first enable the 
debounce clock and then we:

if (debounce) {
	if (!bank->dbck_enabled && (cpu_is_omap34xx() || cpu_is_omap44xx())) {
		clk_enable(bank->dbck);
		bank->dbck_enabled = true;
	}
} else {
	if (bank->dbck_enabled && (cpu_is_omap34xx() || cpu_is_omap44xx())) {
		clk_enable(bank->dbck);
		bank->dbck_enabled = false;
	}
}

-- 
balbi

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Hi,

On Wed, Mar 31, 2010 at 06:29:09PM +0200, Balbi Felipe (Nokia-D/Helsinki) wrote:
>On Wed, Mar 31, 2010 at 06:21:58PM +0200, ext Mark Brown wrote:
>>On Wed, Mar 31, 2010 at 06:35:10PM +0300, Felipe Balbi wrote:
>>> +	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);
>>> +	}
>>
>>Will these do the right thing with the clocks for noop transitions or
>>tweaks to the debounce time?  I'm not familiar with the OMAP clock
>>framework, it could handle this.
>
>good catch. We need to be a bit smarter here. I'll leave that for
>tomorrow. already 19:30 :-p

attached is a new version of this patch.

-- 
balbi

[-- Attachment #2: 0001-arm-omap-gpio-implement-set_debounce-method.diff --]
[-- Type: text/x-diff, Size: 3224 bytes --]

>From 23b09f437d8bacebab676deaf52631b7eb0e8f15 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi@nokia.com>
Date: Wed, 31 Mar 2010 15:17:29 +0300
Subject: [PATCHv3] arm: omap: gpio: implement set_debounce method

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

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---

Changes since v2:
	- enable/disable debounce clock only once

 arch/arm/plat-omap/gpio.c |   75 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index 76a347b..163d88b 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -194,6 +194,7 @@ struct gpio_bank {
 	spinlock_t lock;
 	struct gpio_chip chip;
 	struct clk *dbck;
+	unsigned dbck_enabled:1;
 	u32 mod_usage;
 };
 
@@ -612,6 +613,65 @@ 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 (!bank->dbck_enabled &&
+				(cpu_is_omap34xx() || cpu_is_omap44xx())) {
+			clk_enable(bank->dbck);
+			bank->dbck_enabled = true;
+		}
+	} else {
+		val &= ~l;
+		if (bank->dbck_enabled &&
+				(cpu_is_omap34xx() || cpu_is_omap44xx())) {
+			clk_disable(bank->dbck);
+			bank->dbck_enabled = false;
+		}
+	}
+
+	__raw_writel(val, reg);
+}
+
 void omap_set_gpio_debounce(int gpio, int enable)
 {
 	struct gpio_bank *bank;
@@ -1608,6 +1668,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 +1934,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.0.rc0.33.g7c3932

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Grazvydas Ignotas]

On Wed, Mar 31, 2010 at 6:35 PM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> 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 76a347b..345ed2c 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);
> +       }

Hmh, dbck is shared by the whole GPIO bank, so what happens if someone
calls _set_gpio_debounce(bank, 1, 310) and then
_set_gpio_debounce(bank, 2, 0)? This should leave debounce enabled for
GPIO1, but you'll disable dbck on second call. GPIOs 0-31 share the
same bank.

There is also an issue if somebody calls _set_gpio_debounce(bank, 1,
310) and _set_gpio_debounce(bank, 2, 620), the second call will
override debounce setting of GPIO1 (as it's shared by the whole bank).
This might be not what the user intended, would be useful to detect
this and warn the user.
--
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: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Felipe Balbi]

On Thu, Apr 01, 2010 at 11:29:16AM +0200, ext Grazvydas Ignotas wrote:
>Hmh, dbck is shared by the whole GPIO bank, so what happens if someone
>calls _set_gpio_debounce(bank, 1, 310) and then
>_set_gpio_debounce(bank, 2, 0)? This should leave debounce enabled for
>GPIO1, but you'll disable dbck on second call. GPIOs 0-31 share the
>same bank.

but why would you call _set_gpio_debounce(bank, 2 0); without setting a 
real debounce value before ?

>There is also an issue if somebody calls _set_gpio_debounce(bank, 1,
>310) and _set_gpio_debounce(bank, 2, 620), the second call will
>override debounce setting of GPIO1 (as it's shared by the whole bank).
>This might be not what the user intended, would be useful to detect
>this and warn the user.

good point. As this is RFC, I'll wait until everybody comments.

-- 
balbi

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Grazvydas Ignotas]

On Thu, Apr 1, 2010 at 12:32 PM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> On Thu, Apr 01, 2010 at 11:29:16AM +0200, ext Grazvydas Ignotas wrote:
>>
>> Hmh, dbck is shared by the whole GPIO bank, so what happens if someone
>> calls _set_gpio_debounce(bank, 1, 310) and then
>> _set_gpio_debounce(bank, 2, 0)? This should leave debounce enabled for
>> GPIO1, but you'll disable dbck on second call. GPIOs 0-31 share the
>> same bank.
>
> but why would you call _set_gpio_debounce(bank, 2 0); without setting a real
> debounce value before ?

ok then you could call
  _set_gpio_debounce(bank, 1, 310);
  _set_gpio_debounce(bank, 2, 310);
  _set_gpio_debounce(bank, 2, 0);

The problem here is that debounce is still active for GPIO1, but you
disable dbck for the whole bank.

>
>> There is also an issue if somebody calls _set_gpio_debounce(bank, 1,
>> 310) and _set_gpio_debounce(bank, 2, 620), the second call will
>> override debounce setting of GPIO1 (as it's shared by the whole bank).
>> This might be not what the user intended, would be useful to detect
>> this and warn the user.
>
> good point. As this is RFC, I'll wait until everybody comments.
>
> --
> balbi
>

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Felipe Balbi]

On Thu, Apr 01, 2010 at 11:37:16AM +0200, ext Grazvydas Ignotas wrote:
>On Thu, Apr 1, 2010 at 12:32 PM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
>> On Thu, Apr 01, 2010 at 11:29:16AM +0200, ext Grazvydas Ignotas wrote:
>>>
>>> Hmh, dbck is shared by the whole GPIO bank, so what happens if someone
>>> calls _set_gpio_debounce(bank, 1, 310) and then
>>> _set_gpio_debounce(bank, 2, 0)? This should leave debounce enabled for
>>> GPIO1, but you'll disable dbck on second call. GPIOs 0-31 share the
>>> same bank.
>>
>> but why would you call _set_gpio_debounce(bank, 2 0); without setting a real
>> debounce value before ?
>
>ok then you could call
>  _set_gpio_debounce(bank, 1, 310);
>  _set_gpio_debounce(bank, 2, 310);
>  _set_gpio_debounce(bank, 2, 0);
>
>The problem here is that debounce is still active for GPIO1, but you
>disable dbck for the whole bank.

but then you enabled the clock twice. There's refcounting for the clock.

-- 
balbi

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Grazvydas Ignotas]

On Thu, Apr 1, 2010 at 1:10 PM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> On Thu, Apr 01, 2010 at 11:37:16AM +0200, ext Grazvydas Ignotas wrote:
>>
>> On Thu, Apr 1, 2010 at 12:32 PM, Felipe Balbi <felipe.balbi@nokia.com>
>> wrote:
>>>
>>> On Thu, Apr 01, 2010 at 11:29:16AM +0200, ext Grazvydas Ignotas wrote:
>>>>
>>>> Hmh, dbck is shared by the whole GPIO bank, so what happens if someone
>>>> calls _set_gpio_debounce(bank, 1, 310) and then
>>>> _set_gpio_debounce(bank, 2, 0)? This should leave debounce enabled for
>>>> GPIO1, but you'll disable dbck on second call. GPIOs 0-31 share the
>>>> same bank.
>>>
>>> but why would you call _set_gpio_debounce(bank, 2 0); without setting a
>>> real
>>> debounce value before ?
>>
>> ok then you could call
>>  _set_gpio_debounce(bank, 1, 310);
>>  _set_gpio_debounce(bank, 2, 310);
>>  _set_gpio_debounce(bank, 2, 0);
>>
>> The problem here is that debounce is still active for GPIO1, but you
>> disable dbck for the whole bank.
>
> but then you enabled the clock twice. There's refcounting for the clock.

Oh, it's fine then, forgot about clock refcounting.
--
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: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Jani Nikula]

On Thu, Apr 1, 2010 at 12:32, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> On Thu, Apr 01, 2010 at 11:29:16AM +0200, ext Grazvydas Ignotas wrote:
>>
>> There is also an issue if somebody calls _set_gpio_debounce(bank, 1,
>> 310) and _set_gpio_debounce(bank, 2, 620), the second call will
>> override debounce setting of GPIO1 (as it's shared by the whole bank).
>> This might be not what the user intended, would be useful to detect
>> this and warn the user.
>
> good point. As this is RFC, I'll wait until everybody comments.

Hi Felipe -

You might want to have a look at [1] on irq debouncing. The hardware
support for debouncing varies (bank/gpio restrictions, debounce
timeouts, no support at all, what else?) so how can the users of this
interface rely on debouncing? What are the guarantees? AFAICS e.g.
gpio-keys would have to do software debouncing anyway.

[1] http://lkml.org/lkml/2008/9/24/325

BR,
Jani.

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Felipe Balbi]

On Thu, Apr 01, 2010 at 04:11:27PM +0300, Jani Nikula wrote:
> On Thu, Apr 1, 2010 at 12:32, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> > On Thu, Apr 01, 2010 at 11:29:16AM +0200, ext Grazvydas Ignotas wrote:
> >>
> >> There is also an issue if somebody calls _set_gpio_debounce(bank, 1,
> >> 310) and _set_gpio_debounce(bank, 2, 620), the second call will
> >> override debounce setting of GPIO1 (as it's shared by the whole bank).
> >> This might be not what the user intended, would be useful to detect
> >> this and warn the user.
> >
> > good point. As this is RFC, I'll wait until everybody comments.
> 
> Hi Felipe -
> 
> You might want to have a look at [1] on irq debouncing. The hardware
> support for debouncing varies (bank/gpio restrictions, debounce
> timeouts, no support at all, what else?) so how can the users of this
> interface rely on debouncing? What are the guarantees? AFAICS e.g.
> gpio-keys would have to do software debouncing anyway.
> 
> [1] http://lkml.org/lkml/2008/9/24/325

I think we could provide a generic software debouncing mechanism, sure,
but if the hardware supports it, why not using ?

I believe Dave's approach is really good, this is just another way to do
it. The difference with this patch is that we have control over the
debouncing time.

-- 
balbi

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Mark Brown]

On Thu, Apr 01, 2010 at 07:42:50PM +0300, Felipe Balbi wrote:
> On Thu, Apr 01, 2010 at 04:11:27PM +0300, Jani Nikula wrote:

> > You might want to have a look at [1] on irq debouncing. The hardware
> > support for debouncing varies (bank/gpio restrictions, debounce
> > timeouts, no support at all, what else?) so how can the users of this
> > interface rely on debouncing? What are the guarantees? AFAICS e.g.
> > gpio-keys would have to do software debouncing anyway.

> I think we could provide a generic software debouncing mechanism, sure,
> but if the hardware supports it, why not using ?

I tend to agree here, especially for GPIOs on slow buses like I2C or
which may be used as wake sources.  I guess ideally we want something
like the LEDs do with blinking where we use the hardware feature if
present and suitable but fall back on software emulation transparently
if it's not available or can't be configured appropriately.

Re: [RFC/PATCHv2 2/4] arm: omap: gpio: implement set_debounce method

[Felipe Balbi]

On Thu, 1 Apr 2010 19:15:08 +0100, Mark Brown 
> I tend to agree here, especially for GPIOs on slow buses like I2C or
> which may be used as wake sources.  I guess ideally we want something
> like the LEDs do with blinking where we use the hardware feature if
> present and suitable but fall back on software emulation transparently
> if it's not available or can't be configured appropriately.

yes, and that's a matter of just putting together a little more code
on the default implementation of gpio_set_debounce() instead of
simply returning -ENOSYS.

-- 
balbi

Re: [RFC/PATCHv2 1/4] gpiolib: introduce set_debounce method

[Mark Brown]

On Wed, Mar 31, 2010 at 06:35:09PM +0300, Felipe Balbi wrote:
> 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>

This looks useful - for what it's worth

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [RFC/PATCHv2 1/4] gpiolib: introduce set_debounce method

[Tony Lindgren]

* Mark Brown <broonie@opensource.wolfsonmicro.com> [100402 05:30]:
> On Wed, Mar 31, 2010 at 06:35:09PM +0300, Felipe Balbi wrote:
> > 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>
> 
> This looks useful - for what it's worth
> 
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Ping, looks like this series is pretty close to being mergeable.
Felipe, care to repost one more time without RFC and with the
acks if you think you have updated version that's ready for
the upcoming merge window.

Regards,

Tony

Re: [RFC/PATCHv2 1/4] gpiolib: introduce set_debounce method

[Felipe Balbi]

On Tue, May 04, 2010 at 03:40:30PM -0700, Tony Lindgren wrote:
> * Mark Brown <broonie@opensource.wolfsonmicro.com> [100402 05:30]:
> > On Wed, Mar 31, 2010 at 06:35:09PM +0300, Felipe Balbi wrote:
> > > 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>
> > 
> > This looks useful - for what it's worth
> > 
> > Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> Ping, looks like this series is pretty close to being mergeable.
> Felipe, care to repost one more time without RFC and with the
> acks if you think you have updated version that's ready for
> the upcoming merge window.

will do it tomorrow, but it would be good to hear from Dave before.

-- 
balbi