ep93xx: implement gpiolib set_debounce for built-in GPIOs

PATCH New H Hartley Sweeten 2 participants 4 messages Jan 26, 2011 – Jan 27, 2011

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm/Cirrus logic EP93XX Arm architecture

lore

Share & tools

lore

H Hartley Sweeten

author

2011-01-26 23:22 UTC #0 lore raw

PATCH 1/1 ep93xx: implement gpiolib set_debounce for built-in GPIOs

GPIO Ports A, B, and F on the ep93xx provide interrupt capability. It is possible to debounce the input signal on these ports when interrupts are enabled.

Support for this debounce capability was provided with an ep93xx internal function. Now that gpiolib knows about gpio debounce, use the gpiolib method and do not export the internal function.

Signed-off-by H Hartley Sweeten <[email protected]>

Cc Ryan Mallon <[email protected]>

---

arch/arm/mach-ep93xx/gpio.c +29 -4

index bec34b8..a889fa7 100644
@@ -61,7 +61,7 @@ static inline void ep93xx_gpio_int_mask(unsigned line)
 	gpio_int_unmasked[line >> 3] &= ~(1 << (line & 7));
 }
 
-void ep93xx_gpio_int_debounce(unsigned int irq, int enable)
+static void ep93xx_gpio_int_debounce(unsigned int irq, bool enable)
 {
 	int line = irq_to_gpio(irq);
 	int port = line >> 3;

@@ -75,7 +75,6 @@ void ep93xx_gpio_int_debounce(unsigned int irq, int enable)
 	__raw_writeb(gpio_int_debounce[port],
 		EP93XX_GPIO_REG(int_debounce_register_offset[port]));
 }
-EXPORT_SYMBOL(ep93xx_gpio_int_debounce);
 
 static void ep93xx_gpio_ab_irq_handler(unsigned int irq, struct irq_desc *desc)
 {

@@ -335,6 +334,20 @@ static void ep93xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 	local_irq_restore(flags);
 }
 
+static int ep93xx_gpio_set_debounce(struct gpio_chip *chip,
+				    unsigned offset, unsigned debounce)
+{
+	int gpio = chip->base + offset;
+	int irq = gpio_to_irq(gpio);
+
+	if (irq < 0)
+		return -EINVAL;
+
+	ep93xx_gpio_int_debounce(irq, debounce ? true : false);
+
+	return 0;
+}
+
 static void ep93xx_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 {
 	struct ep93xx_gpio_chip *ep93xx_chip = to_ep93xx_gpio_chip(chip);

@@ -434,6 +447,18 @@ void __init ep93xx_gpio_init(void)
 				 EP93XX_SYSCON_DEVCFG_GONIDE |
 				 EP93XX_SYSCON_DEVCFG_HONIDE);
 
-	for (i = 0; i < ARRAY_SIZE(ep93xx_gpio_banks); i++)
-		gpiochip_add(&ep93xx_gpio_banks[i].chip);
+	for (i = 0; i < ARRAY_SIZE(ep93xx_gpio_banks); i++) {
+		struct gpio_chip *chip = &ep93xx_gpio_banks[i].chip;
+
+		/*
+		 * Ports A, B, and F support input debouncing when
+		 * used as interrupts.
+		 */
+		if (!strcmp(chip->label, "A") ||
+		    !strcmp(chip->label, "B") ||
+		    !strcmp(chip->label, "F"))
+			chip->set_debounce = ep93xx_gpio_set_debounce;
+
+		gpiochip_add(chip);
+	}
 }

arch/arm/mach-ep93xx/include/mach/gpio.h +0 -2

index c991b14..c57152c 100644
@@ -99,8 +99,6 @@
 /* maximum value for irq capable line identifiers */
 #define EP93XX_GPIO_LINE_MAX_IRQ	EP93XX_GPIO_LINE_F(7)
 
-extern void ep93xx_gpio_int_debounce(unsigned int irq, int enable);
-
 /* new generic GPIO API - see Documentation/gpio.txt */
 
 #include <asm-generic/gpio.h>

Ryan Mallon

2011-01-26 23:47 UTC #1 lore raw

PATCH 1/1 ep93xx: implement gpiolib set_debounce for built-in GPIOs

Quoted 38 lines · H Hartley Sweeten

GPIO Ports A, B, and F on the ep93xx provide interrupt capability. It is possible to debounce the input signal on these ports when interrupts are enabled.
Support for this debounce capability was provided with an ep93xx internal function. Now that gpiolib knows about gpio debounce, use the gpiolib method and do not export the internal function.
Signed-off-by H Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
---
arch/arm/mach-ep93xx/gpio.c +1 -2
index bec34b8..a889fa7 100644
@@ -61,7 +61,7 @@ static inline void ep93xx_gpio_int_mask(unsigned line)
 	gpio_int_unmasked[line >> 3] &= ~(1 << (line & 7));
 }
 
-void ep93xx_gpio_int_debounce(unsigned int irq, int enable)
+static void ep93xx_gpio_int_debounce(unsigned int irq, bool enable)
 {
 	int line = irq_to_gpio(irq);
 	int port = line >> 3;
@@ -75,7 +75,6 @@ void ep93xx_gpio_int_debounce(unsigned int irq, int enable)
 	__raw_writeb(gpio_int_debounce[port],
 		EP93XX_GPIO_REG(int_debounce_register_offset[port]));
 }
-EXPORT_SYMBOL(ep93xx_gpio_int_debounce);
 
 static void ep93xx_gpio_ab_irq_handler(unsigned int irq, struct irq_desc *desc)
 {
@@ -335,6 +334,20 @@ static void ep93xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 	local_irq_restore(flags);
 }
 

arch/arm/mach-ep93xx/gpio.c static void ep93xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val) · line 346

Quoted 10 lines

arch/arm/mach-ep93xx/gpio.c +10 -0

+static int ep93xx_gpio_set_debounce(struct gpio_chip *chip,
+				    unsigned offset, unsigned debounce)
+{
+	int gpio = chip->base + offset;
+	int irq = gpio_to_irq(gpio);
+
+	if (irq < 0)
+		return -EINVAL;
+
+	ep93xx_gpio_int_debounce(irq, debounce ? true : false);

We should just move the code from ep93xx_gpio_int_debounce here. Why have two functions to do one thing?

Quoted 13 lines

arch/arm/mach-ep93xx/gpio.c +4 -2

+
+	return 0;
+}
+
 static void ep93xx_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 {
 	struct ep93xx_gpio_chip *ep93xx_chip = to_ep93xx_gpio_chip(chip);

@@ -434,6 +447,18 @@ void __init ep93xx_gpio_init(void)
 				 EP93XX_SYSCON_DEVCFG_GONIDE |
 				 EP93XX_SYSCON_DEVCFG_HONIDE);
 
-	for (i = 0; i < ARRAY_SIZE(ep93xx_gpio_banks); i++)
-		gpiochip_add(&ep93xx_gpio_banks[i].chip);

arch/arm/mach-ep93xx/gpio.c void __init ep93xx_gpio_init(void) · line 459

Quoted 10 lines

arch/arm/mach-ep93xx/gpio.c +10 -0

+	for (i = 0; i < ARRAY_SIZE(ep93xx_gpio_banks); i++) {
+		struct gpio_chip *chip = &ep93xx_gpio_banks[i].chip;
+
+		/*
+		 * Ports A, B, and F support input debouncing when
+		 * used as interrupts.
+		 */
+		if (!strcmp(chip->label, "A") ||
+		    !strcmp(chip->label, "B") ||
+		    !strcmp(chip->label, "F"))

This could just be if *chip->label == 'A'. I don't really mind either way.

Quoted 10 lines

arch/arm/mach-ep93xx/gpio.c +4 -0
+			chip->set_debounce = ep93xx_gpio_set_debounce;
+
+		gpiochip_add(chip);
+	}
 }
arch/arm/mach-ep93xx/include/mach/gpio.h +0 -0
index c991b14..c57152c 100644

arch/arm/mach-ep93xx/include/mach/gpio.h line 104

Quoted 8 lines

arch/arm/mach-ep93xx/include/mach/gpio.h +0 -2

@@ -99,8 +99,6 @@
 /* maximum value for irq capable line identifiers */
 #define EP93XX_GPIO_LINE_MAX_IRQ	EP93XX_GPIO_LINE_F(7)
 
-extern void ep93xx_gpio_int_debounce(unsigned int irq, int enable);
-
 /* new generic GPIO API - see Documentation/gpio.txt */
 
 #include <asm-generic/gpio.h>

There don't appear to be any users of ep93xx_gpio_int_debounce anyway. But with the above mentioned changes:

Acked-by Ryan Mallon <[email protected]>

Signature · 7 lines

Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

H Hartley Sweeten

author

2011-01-26 23:59 UTC #2 lore raw

PATCH 1/1 ep93xx: implement gpiolib set_debounce for built-in GPIOs

Quoted 39 lines · Ryan Mallon

Quoted 38 lines · H Hartley Sweeten
GPIO Ports A, B, and F on the ep93xx provide interrupt capability. It is possible to debounce the input signal on these ports when interrupts are enabled.
Support for this debounce capability was provided with an ep93xx internal function. Now that gpiolib knows about gpio debounce, use the gpiolib method and do not export the internal function.
Signed-off-by H Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
---
arch/arm/mach-ep93xx/gpio.c +1 -2
index bec34b8..a889fa7 100644
@@ -61,7 +61,7 @@ static inline void ep93xx_gpio_int_mask(unsigned line)
 	gpio_int_unmasked[line >> 3] &= ~(1 << (line & 7));
 }
 
-void ep93xx_gpio_int_debounce(unsigned int irq, int enable)
+static void ep93xx_gpio_int_debounce(unsigned int irq, bool enable)
 {
 	int line = irq_to_gpio(irq);
 	int port = line >> 3;
@@ -75,7 +75,6 @@ void ep93xx_gpio_int_debounce(unsigned int irq, int enable)
 	__raw_writeb(gpio_int_debounce[port],
 		EP93XX_GPIO_REG(int_debounce_register_offset[port]));
 }
-EXPORT_SYMBOL(ep93xx_gpio_int_debounce);
 
 static void ep93xx_gpio_ab_irq_handler(unsigned int irq, struct irq_desc *desc)
 {
@@ -335,6 +334,20 @@ static void ep93xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 	local_irq_restore(flags);
 }
 

Quoted 13 lines

arch/arm/mach-ep93xx/gpio.c static void ep93xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val) · line 346
Quoted 10 lines
arch/arm/mach-ep93xx/gpio.c +10 -0
+static int ep93xx_gpio_set_debounce(struct gpio_chip *chip,
+				    unsigned offset, unsigned debounce)
+{
+	int gpio = chip->base + offset;
+	int irq = gpio_to_irq(gpio);
+
+	if (irq < 0)
+		return -EINVAL;
+
+	ep93xx_gpio_int_debounce(irq, debounce ? true : false);
We should just move the code from ep93xx_gpio_int_debounce here. Why have two functions to do one thing?

The reason I left the original function it that it messes with the cached gpio_int_debounce values. Also, the 'debounce' feature is interrupt specific. If the gpio pin is no enabled as an interrupt the debounce has no effect. As such I figured it was cleaner to leave it in place.

Would you still like me to combine the functions?

Regards,
Hartley

Ryan Mallon

2011-01-27 00:06 UTC #3 lore raw

PATCH 1/1 ep93xx: implement gpiolib set_debounce for built-in GPIOs

Quoted 54 lines · H Hartley Sweeten

Quoted 39 lines · Ryan Mallon
Quoted 38 lines · H Hartley Sweeten
GPIO Ports A, B, and F on the ep93xx provide interrupt capability. It is possible to debounce the input signal on these ports when interrupts are enabled.
Support for this debounce capability was provided with an ep93xx internal function. Now that gpiolib knows about gpio debounce, use the gpiolib method and do not export the internal function.
Signed-off-by H Hartley Sweeten <[email protected]>
Cc Ryan Mallon <[email protected]>
---
arch/arm/mach-ep93xx/gpio.c +1 -2
index bec34b8..a889fa7 100644
@@ -61,7 +61,7 @@ static inline void ep93xx_gpio_int_mask(unsigned line)
 	gpio_int_unmasked[line >> 3] &= ~(1 << (line & 7));
 }
 
-void ep93xx_gpio_int_debounce(unsigned int irq, int enable)
+static void ep93xx_gpio_int_debounce(unsigned int irq, bool enable)
 {
 	int line = irq_to_gpio(irq);
 	int port = line >> 3;
@@ -75,7 +75,6 @@ void ep93xx_gpio_int_debounce(unsigned int irq, int enable)
 	__raw_writeb(gpio_int_debounce[port],
 		EP93XX_GPIO_REG(int_debounce_register_offset[port]));
 }
-EXPORT_SYMBOL(ep93xx_gpio_int_debounce);
 
 static void ep93xx_gpio_ab_irq_handler(unsigned int irq, struct irq_desc *desc)
 {
@@ -335,6 +334,20 @@ static void ep93xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 	local_irq_restore(flags);
 }
 
Quoted 13 lines
arch/arm/mach-ep93xx/gpio.c static void ep93xx_gpio_set(struct gpio_chip *chip, unsigned offset, int val) · line 346
Quoted 10 lines
arch/arm/mach-ep93xx/gpio.c +10 -0
+static int ep93xx_gpio_set_debounce(struct gpio_chip *chip,
+				    unsigned offset, unsigned debounce)
+{
+	int gpio = chip->base + offset;
+	int irq = gpio_to_irq(gpio);
+
+	if (irq < 0)
+		return -EINVAL;
+
+	ep93xx_gpio_int_debounce(irq, debounce ? true : false);
We should just move the code from ep93xx_gpio_int_debounce here. Why have two functions to do one thing?

Quoted 4 lines

The reason I left the original function it that it messes with the cached gpio_int_debounce values. Also, the 'debounce' feature is interrupt specific. If the gpio pin is no enabled as an interrupt the debounce has no effect. As such I figured it was cleaner to leave it in place.

That's a valid reason. It is fine as is.

~Ryan

Signature · 7 lines

Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934