Skip to contentSkip to conversation

au1000: convert to using gpiolib

PATCH Applied Florian Fainelli 3 participants 12 messages Jan 15, 2009 – Feb 26, 2009 MIPS

lore
Share & tools
lore

Florian Fainelli

author
#0 lore raw

PATCH 1/1 au1000: convert to using gpiolib

This patch converts the GPIO board code to use gpiolib.

Signed-off-by Florian Fainelli <[email protected]>

---

arch/mips/alchemy/Kconfig +1 -0
index 7f8ef13..2fc5c13 100644
@@ -135,3 +135,4 @@ config SOC_AU1X00
 	select SYS_SUPPORTS_32BIT_KERNEL
 	select SYS_SUPPORTS_APM_EMULATION
 	select GENERIC_HARDIRQS_NO__DO_IRQ
+	select ARCH_REQUIRE_GPIOLIB
arch/mips/alchemy/common/gpio.c +128 -75

255 lines not on this page · raw message

arch/mips/include/asm/mach-au1x00/gpio.h +17 -53

82 lines not on this page · raw message

#1 lore raw

Hi Florian,

Quoted 1 line · Florian Fainelli

This patch converts the GPIO board code to use gpiolib

Very nice; a few minor things though:

Quoted 7 lines
arch/mips/alchemy/common/gpio.c +2 -2
-static void au1xxx_gpio2_write(unsigned gpio, int value)
+static void au1000_gpio2_set(struct gpio_chip *chip,
+				unsigned offset, int value)
 {
-	gpio -= AU1XXX_GPIO_BASE;

arch/mips/alchemy/common/gpio.c line 65

Quoted 10 lines
arch/mips/alchemy/common/gpio.c +8 -1
+	u32 mask = ((GPIO2_OUT_EN_MASK << offset) | (!!value << offset));
+	struct au1000_gpio_chip *gpch;
+	unsigned long flags;
+
+	gpch = container_of(chip, struct au1000_gpio_chip, chip);
 
-	gpio2->output = (GPIO2_OUTPUT_ENABLE_MASK << gpio) | ((!!value) << gpio);
+	local_irq_save(flags);
+	writel(mask, gpch->regbase + AU1000_GPIO2_OUT);
+	local_irq_restore(flags);

Because of the 'enable | value' scheme I believe you don't require any locking here.

Quoted 32 lines
arch/mips/alchemy/common/gpio.c +27 -4
+struct au1000_gpio_chip au1000_gpio_chip[] = {
+	[0] = {
+		.regbase			= (void __iomem *)SYS_BASE,
+		.chip = {
+			.label			= "au1000-gpio1",
+			.direction_input	= au1000_gpio1_direction_input,
+			.direction_output	= au1000_gpio1_direction_output,
+			.get			= au1000_gpio1_get,
+			.set			= au1000_gpio1_set,
+			.base			= 0,
+			.ngpio			= 32,
+		},
+	},
+#if !defined(CONFIG_SOC_AU1000)
+	[1] = {
+		.regbase                        = (void __iomem *)GPIO2_BASE,
+		.chip = {
+			.label                  = "au1000-gpio2",
+			.direction_input        = au1000_gpio2_direction_input,
+			.direction_output       = au1000_gpio2_direction_output,
+			.get                    = au1000_gpio2_get,
+			.set                    = au1000_gpio2_set,
+			.base                   = AU1XXX_GPIO_BASE,
+			.ngpio                  = 32,
+		},
+	},
 #endif
-	else
-		return au1xxx_gpio1_read(gpio);
-}
-EXPORT_SYMBOL(au1xxx_gpio_get_value);
+};

[...]

Quoted 16 lines
arch/mips/alchemy/common/gpio.c +4 -10
+static int __init au1000_gpio_init(void)
 {
-	if (gpio >= AU1XXX_GPIO_BASE)
-#if defined(CONFIG_SOC_AU1000)
-		;
-#else
-		au1xxx_gpio2_write(gpio, value);
-#endif
-	else
-		au1xxx_gpio1_write(gpio, value);
-}
-EXPORT_SYMBOL(au1xxx_gpio_set_value);
+	gpiochip_add(&au1000_gpio_chip[0].chip);
+#if !defined(CONFIG_SOC_AU1000)
+	gpiochip_add(&au1000_gpio_chip[1].chip);

[...]

Quoted 1 line

+arch_initcall(au1000_gpio_init);

Can you please make the gpiolib registration dependent on a CONFIG symbol? I.e. make the au1000_gpio{,2}_direction() and friends calls globally visible but let the individual boards decide whether they want to use the gpio numbering imposed by this patch.

Long explanation: I maintain a number of modules with a common connector interface, based on different architectures (sh, mips and arm so far). I also maintain a few baseboards which can carry theese modules. Modules provide 16 gpios numbered 0-15, which are used by the baseboards. Since I need to keep the baseboard code free of arch-specific hacks, every module provides its own gpio-chip which distributes the gpio-lib calls to various on- and off-chip peripherals. On my alchemy board in particular, those 16 gpios are provided by a mixture of gpio1, gpio2 and FPGA based pins (yes I repeatedly moanoed to the hw guys about this but pin multiplexing and required features make a sane implementation difficult; but at least I was allowed to write the VHDL for the fpga-based gpios).

If this explanation doesn't make sense I'll gladly whip up an addon patch.

arch/mips/include/asm/mach-au1x00/gpio.h line 6

Quoted 7 lines
arch/mips/include/asm/mach-au1x00/gpio.h +0 -0
index 2dc61e0..34d9b72 100644
@@ -5,65 +5,29 @@
 
 #define AU1XXX_GPIO_BASE	200

please change this to AU1XXX_GPIO2_BASE (it's the base number of the GPIO2 block pins after all)

Thanks!
Manuel Lauss

author
#2 lore raw

Hi Manuel,

2009/1/15 Manuel Lauss <[email protected]>

Quoted 7 lines

Hi Florian,

Quoted 1 line · Florian Fainelli

This patch converts the GPIO board code to use gpiolib

Because of the 'enable | value' scheme I believe you don't require any locking here.

Right, thanks.

Quoted 58 lines

arch/mips/alchemy/common/gpio.c

Quoted 9 lines
arch/mips/alchemy/common/gpio.c +6 -0
+struct au1000_gpio_chip au1000_gpio_chip[] = {
+     [0] = {
+             .regbase                        = (void __iomem *)SYS_BASE,
+             .chip = {
+                     .label                  = "au1000-gpio1",
+                     .direction_input        =

au1000_gpio1_direction_input,

Quoted 1 line

+ .direction_output =

au1000_gpio1_direction_output,

arch/mips/alchemy/common/gpio.c

Quoted 9 lines
arch/mips/alchemy/common/gpio.c +9 -0
+                     .get                    = au1000_gpio1_get,
+                     .set                    = au1000_gpio1_set,
+                     .base                   = 0,
+                     .ngpio                  = 32,
+             },
+     },
+#if !defined(CONFIG_SOC_AU1000)
+     [1] = {
+             .regbase                        = (void __iomem

*)GPIO2_BASE,

arch/mips/alchemy/common/gpio.c

Quoted 3 lines
arch/mips/alchemy/common/gpio.c +3 -0
+             .chip = {
+                     .label                  = "au1000-gpio2",
+                     .direction_input        =

au1000_gpio2_direction_input,

Quoted 1 line

+ .direction_output =

au1000_gpio2_direction_output,

Quoted 12 lines
arch/mips/alchemy/common/gpio.c +7 -4
+                     .get                    = au1000_gpio2_get,
+                     .set                    = au1000_gpio2_set,
+                     .base                   = AU1XXX_GPIO_BASE,
+                     .ngpio                  = 32,
+             },
+     },
 #endif
-     else
-             return au1xxx_gpio1_read(gpio);
-}
-EXPORT_SYMBOL(au1xxx_gpio_get_value);
+};

[...]

Quoted 16 lines
arch/mips/alchemy/common/gpio.c +4 -10
+static int __init au1000_gpio_init(void)
 {
-     if (gpio >= AU1XXX_GPIO_BASE)
-#if defined(CONFIG_SOC_AU1000)
-             ;
-#else
-             au1xxx_gpio2_write(gpio, value);
-#endif
-     else
-             au1xxx_gpio1_write(gpio, value);
-}
-EXPORT_SYMBOL(au1xxx_gpio_set_value);
+     gpiochip_add(&au1000_gpio_chip[0].chip);
+#if !defined(CONFIG_SOC_AU1000)
+     gpiochip_add(&au1000_gpio_chip[1].chip);

[...]

Quoted 7 lines
Quoted 1 line

+arch_initcall(au1000_gpio_init);

Can you please make the gpiolib registration dependent on a CONFIG symbol? I.e. make the au1000_gpio{,2}_direction() and friends calls globally visible but let the individual boards decide whether they want to use the gpio numbering imposed by this patch.

Would something like #ifdef CONFIG_AU1000_NON_STD_GPIOS be ok with you ? Or maybe we could get the base information from board-specific code ?

Quoted 13 lines

Long explanation: I maintain a number of modules with a common connector interface, based on different architectures (sh, mips and arm so far). I also maintain a few baseboards which can carry theese modules. Modules provide 16 gpios numbered 0-15, which are used by the baseboards. Since I need to keep the baseboard code free of arch-specific hacks, every module provides its own gpio-chip which distributes the gpio-lib calls to various on- and off-chip peripherals. On my alchemy board in particular, those 16 gpios are provided by a mixture of gpio1, gpio2 and FPGA based pins (yes I repeatedly moanoed to the hw guys about this but pin multiplexing and required features make a sane implementation difficult; but at least I was allowed to write the VHDL for the fpga-based gpios).

Your explanation makes perfect sense to me.

Quoted 12 lines

If this explanation doesn't make sense I'll gladly whip up an addon patch.

Review

Quoted 1 line
(quoted diff) +0 -0

b/arch/mips/include/asm/mach-au1x00/gpio.h

Quoted 5 lines

index 2dc61e0..34d9b72 100644

arch/mips/include/asm/mach-au1x00/gpio.h +0 -0
@@ -5,65 +5,29 @@
Quoted 4 lines

arch/mips/include/asm/mach-au1x00/gpio.h line 6

Quoted 1 line
arch/mips/include/asm/mach-au1x00/gpio.h +0 -0
 #define AU1XXX_GPIO_BASE     200

please change this to AU1XXX_GPIO2_BASE (it's the base number of the GPIO2 block pins after all)

Ok.

Thanks for your comments, I will respin with your comments.

#3 lore raw

Hi Florian,

Quoted 9 lines · Florian Fainelli
Quoted 5 lines

Can you please make the gpiolib registration dependent on a CONFIG symbol? I.e. make the au1000_gpio{,2}_direction() and friends calls globally visible but let the individual boards decide whether they want to use the gpio numbering imposed by this patch.

Would something like #ifdef CONFIG_AU1000_NON_STD_GPIOS be ok with you ? Or maybe we could get the base information from board-specific code ?

Well, we could move the core_initcall() to all boards' setup function, and stick a comment on top of it explaining why it's there.

Config symbol or the above, either is fine with me.

Thanks!
Manuel Lauss

author
#4 lore raw

Hi Manuel,

Le Friday 16 January 2009 18:47:53 Manuel Lauss, vous avez écrit :

Quoted 9 lines

Hi Florian,

Quoted 6 lines · Florian Fainelli
Quoted 5 lines

Can you please make the gpiolib registration dependent on a CONFIG symbol? I.e. make the au1000_gpio{,2}_direction() and friends calls globally visible but let the individual boards decide whether they want to use the gpio numbering imposed by this patch.

Quoted 7 lines
Quoted 2 lines

Would something like #ifdef CONFIG_AU1000_NON_STD_GPIOS be ok with you ? Or maybe we could get the base information from board-specific code ?

Well, we could move the core_initcall() to all boards' setup function, and stick a comment on top of it explaining why it's there.

Config symbol or the above, either is fine with me.

I would prefer the config symbol, there is only one file to change ;). Patch attached below:
--
From: Florian Fainelli <[email protected]>
Subject: [PATCH v2] au1000: convert to using gpiolib

This patch converts the GPIO board code to use gpiolib. Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

Signed-off-by Florian Fainelli <[email protected]>

--

arch/mips/alchemy/Kconfig +1 -0
index 7f8ef13..2fc5c13 100644
@@ -135,3 +135,4 @@ config SOC_AU1X00
 	select SYS_SUPPORTS_32BIT_KERNEL
 	select SYS_SUPPORTS_APM_EMULATION
 	select GENERIC_HARDIRQS_NO__DO_IRQ
+	select ARCH_REQUIRE_GPIOLIB
arch/mips/alchemy/common/gpio.c +125 -75

252 lines not on this page · raw message

arch/mips/include/asm/mach-au1x00/gpio.h +18 -54

85 lines not on this page · raw message

Signature · 4 lines
Best regards, Florian Fainelli
Email : [email protected]
http://openwrt.org
-------------------------------
author
#5 lore raw

Hi Manuel,

Le Monday 19 January 2009 18:12:23 Florian Fainelli, vous avez écrit :

Quoted 4 lines

This patch converts the GPIO board code to use gpiolib. Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

I did not receive comments from you on this patch, can I consider it being ok ?

Signature · 4 lines
Best regards, Florian Fainelli
Email : [email protected]
http://openwrt.org
-------------------------------
#6 lore raw

Florian,

Quoted 8 lines

Le Monday 19 January 2009 18:12:23 Florian Fainelli, vous avez ?crit?:

Quoted 4 lines

This patch converts the GPIO board code to use gpiolib. Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

I did not receive comments from you on this patch, can I consider it being ok ?

Oh sorry. Please export all the au1000_gpioX_* functions and I'm happy.

Thanks!
Manuel Lauss

author
#7 lore raw

Le Tuesday 27 January 2009 07:58:27 Manuel Lauss, vous avez écrit :

Quoted 8 lines

Florian,

Quoted 6 lines

Le Monday 19 January 2009 18:12:23 Florian Fainelli, vous avez ?crit?:

Quoted 4 lines

This patch converts the GPIO board code to use gpiolib. Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

Quoted 7 lines
Quoted 2 lines

I did not receive comments from you on this patch, can I consider it being ok ?

Oh sorry. Please export all the au1000_gpioX_* functions and I'm happy.

Thanks!
Manuel Lauss

Here we go. Thanks !
--
From: Florian Fainelli <[email protected]>
Subject: [PATCH v3] Alchemy: convert to gpiolib

This patch converts Alchemy SoC GPIO code to use gpiolib instead of the old GENERIC_GPIO implementation.

Changes from v2:
- export all gpio accessors so that custom boards can use them

Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

Signed-off-by Florian Fainelli <[email protected]>

---

arch/mips/alchemy/Kconfig +1 -0
index 7f8ef13..2fc5c13 100644
@@ -135,3 +135,4 @@ config SOC_AU1X00
 	select SYS_SUPPORTS_32BIT_KERNEL
 	select SYS_SUPPORTS_APM_EMULATION
 	select GENERIC_HARDIRQS_NO__DO_IRQ
+	select ARCH_REQUIRE_GPIOLIB
arch/mips/alchemy/common/gpio.c +134 -76

261 lines not on this page · raw message

arch/mips/include/asm/mach-au1x00/gpio.h +18 -54

85 lines not on this page · raw message

#8 lore raw
Quoted 12 lines · Florian Fainelli

Le Tuesday 27 January 2009 07:58:27 Manuel Lauss, vous avez ?crit?:

Quoted 11 lines

Florian,

Quoted 8 lines

Le Monday 19 January 2009 18:12:23 Florian Fainelli, vous avez ?crit?:

Quoted 4 lines

This patch converts the GPIO board code to use gpiolib. Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

I did not receive comments from you on this patch, can I consider it being ok ?

Quoted 6 lines
Quoted 4 lines

Oh sorry. Please export all the au1000_gpioX_* functions and I'm happy.

Thanks!
Manuel Lauss

Here we go. Thanks !

One last nit: please also add function prototypes to the gpio header.

-- ml.

author
#9 lore raw

Le Tuesday 27 January 2009 10:47:16 Manuel Lauss, vous avez écrit :

Quoted 16 lines · Florian Fainelli
Quoted 15 lines

Le Tuesday 27 January 2009 07:58:27 Manuel Lauss, vous avez ?crit?:

Quoted 14 lines

Florian,

Quoted 8 lines

Le Monday 19 January 2009 18:12:23 Florian Fainelli, vous avez ?crit?:

Quoted 4 lines

This patch converts the GPIO board code to use gpiolib. Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

I did not receive comments from you on this patch, can I consider it being ok ?

Oh sorry. Please export all the au1000_gpioX_* functions and I'm happy.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Thanks!
Manuel Lauss

Here we go. Thanks !

One last nit: please also add function prototypes to the gpio header.

Right, sorry about that.
--
From: Florian Fainelli <[email protected]>
Subject: [PATCH v4] Alchemy: convert to gpiolib

This patch converts Alchemy SoC GPIO code to use gpiolib instead of the old GENERIC_GPIO implementation.

Changes from v3:
- add function prototypes in gpio header

Changes from v2:
- export all gpio accessors so that custom boards can use them

Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

Signed-off-by Florian Fainelli <[email protected]>

---

arch/mips/alchemy/Kconfig +1 -0
index 7f8ef13..2fc5c13 100644
@@ -135,3 +135,4 @@ config SOC_AU1X00
 	select SYS_SUPPORTS_32BIT_KERNEL
 	select SYS_SUPPORTS_APM_EMULATION
 	select GENERIC_HARDIRQS_NO__DO_IRQ
+	select ARCH_REQUIRE_GPIOLIB
arch/mips/alchemy/common/gpio.c +134 -76

261 lines not on this page · raw message

arch/mips/include/asm/mach-au1x00/gpio.h +28 -54

96 lines not on this page · raw message

Acked-by
#10 lore raw
Quoted 35 lines · Florian Fainelli

Le Tuesday 27 January 2009 10:47:16 Manuel Lauss, vous avez ?crit?:

Quoted 16 lines · Florian Fainelli
Quoted 15 lines

Le Tuesday 27 January 2009 07:58:27 Manuel Lauss, vous avez ?crit?:

Quoted 14 lines

Florian,

Quoted 8 lines

Le Monday 19 January 2009 18:12:23 Florian Fainelli, vous avez ?crit?:

Quoted 4 lines

This patch converts the GPIO board code to use gpiolib. Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

I did not receive comments from you on this patch, can I consider it being ok ?

Oh sorry. Please export all the au1000_gpioX_* functions and I'm happy.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

Thanks!
Manuel Lauss

Here we go. Thanks !

One last nit: please also add function prototypes to the gpio header.

Right, sorry about that.
--
From: Florian Fainelli <[email protected]>
Subject: [PATCH v4] Alchemy: convert to gpiolib

This patch converts Alchemy SoC GPIO code to use gpiolib instead of the old GENERIC_GPIO implementation.

Changes from v3:
- add function prototypes in gpio header

Quoted 8 lines

Changes from v2:
- export all gpio accessors so that custom boards can use them

Changes from v1:
- allow users not to use the default gpio accessors
- do not lock au1000_gpio2_set

Signed-off-by Florian Fainelli <[email protected]>
Acked-by Manuel Lauss <[email protected]>

Thank you!
Manuel Lauss