[PATCH RFC] x86: Add user configurable GPIO-lib support

[PATCH RFC] x86: Add user configurable GPIO-lib support

[Michael Buesch]

So this adds user-configurable GPIO support through gpiolib
on subarchitectures that do not implement a GPIO implementation, yet.
Currently that's everything except X86_RDC321X.

The advantage of this is to make it possible to use generic PCI (or
other bus) GPIO extention cards in standard PCs through the standard
GPIO API.

If another subarch implements its own GPIO, it needs to add
itself as an inverted dependency to GPIO_USERSELECTION to make
sure the user does not enable two GPIO API implementations.

About the asm-x86/gpio.h:
I'm not sure what this <gpio.h> include currently is.
Can somebody explain that to me? Where is this supposed
to include a gpio.h file from?


What's your opinion on this?


Index: wireless-testing/arch/x86/Kconfig
===================================================================
--- wireless-testing.orig/arch/x86/Kconfig	2008-06-30 23:00:29.000000000 +0200
+++ wireless-testing/arch/x86/Kconfig	2008-06-30 23:07:04.000000000 +0200
@@ -1675,6 +1675,19 @@
 
 source "drivers/pci/hotplug/Kconfig"
 
+config GPIO_USERSELECTION
+	bool "Enable generic GPIO support through gpiolib"
+	depends on !X86_RDC321X
+	select GENERIC_GPIO
+	select HAVE_GPIO_LIB
+	help
+	  This enables GPIO support through the generic GPIO library.
+	  You only need to enable this, if you also want to enable
+	  one or more of the GPIO expansion card drivers in
+	  "Device Drivers/GPIO Support".
+
+	  If unsure, say N.
+
 endmenu
 
 
Index: wireless-testing/include/asm-x86/gpio.h
===================================================================
--- wireless-testing.orig/include/asm-x86/gpio.h	2008-06-30 23:28:55.000000000 +0200
+++ wireless-testing/include/asm-x86/gpio.h	2008-06-30 23:36:20.000000000 +0200
@@ -1,6 +1,62 @@
+/*
+ * Generic GPIO API implementation for x86.
+ *
+ * Derived from the generic GPIO API for powerpc:
+ *
+ * Copyright (c) 2007-2008  MontaVista Software, Inc.
+ *
+ * Author: Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
 #ifndef _ASM_I386_GPIO_H
 #define _ASM_I386_GPIO_H
 
+#ifdef CONFIG_X86_RDC321X
 #include <gpio.h>
+#else /* CONFIG_X86_RDC321X */
+
+#include <asm-generic/gpio.h>
+
+#ifdef CONFIG_HAVE_GPIO_LIB
+
+/*
+ * Just call gpiolib.
+ */
+static inline int gpio_get_value(unsigned int gpio)
+{
+	return __gpio_get_value(gpio);
+}
+
+static inline void gpio_set_value(unsigned int gpio, int value)
+{
+	__gpio_set_value(gpio, value);
+}
+
+static inline int gpio_cansleep(unsigned int gpio)
+{
+	return __gpio_cansleep(gpio);
+}
+
+/*
+ * Not implemented, yet.
+ */
+static inline int gpio_to_irq(unsigned int gpio)
+{
+	return -ENOSYS;
+}
+
+static inline int irq_to_gpio(unsigned int irq)
+{
+	return -EINVAL;
+}
+
+#endif /* CONFIG_HAVE_GPIO_LIB */
+
+#endif /* CONFIG_X86_RDC321X */
 
 #endif /* _ASM_I386_GPIO_H */

-- 
Greetings Michael.

Re: [PATCH RFC] x86: Add user configurable GPIO-lib support

[Ingo Molnar]

* Michael Buesch <mb@bu3sch.de> wrote:

> So this adds user-configurable GPIO support through gpiolib on 
> subarchitectures that do not implement a GPIO implementation, yet. 
> Currently that's everything except X86_RDC321X.
> 
> The advantage of this is to make it possible to use generic PCI (or 
> other bus) GPIO extention cards in standard PCs through the standard 
> GPIO API.
> 
> If another subarch implements its own GPIO, it needs to add itself as 
> an inverted dependency to GPIO_USERSELECTION to make sure the user 
> does not enable two GPIO API implementations.
> 
> About the asm-x86/gpio.h:
> I'm not sure what this <gpio.h> include currently is.
> Can somebody explain that to me? Where is this supposed
> to include a gpio.h file from?
> 
> What's your opinion on this?

( i've Cc:-ed Florian on this who's maintaining the RDC R-321X bits. )

The longer-term goal is that we'd like to remove the explicit RDC 
sub-arch and just support such systems out of box on x86.

We are almost there - Florian already moved a few special drivers out of 
arch/x86, the only missing piece is this complication in 
include/asm-x86/timex.h:

 #ifdef CONFIG_X86_ELAN
 #  define PIT_TICK_RATE 1189200 /* AMD Elan has different frequency! */
 #elif defined(CONFIG_X86_RDC321X)
 #  define PIT_TICK_RATE 1041667 /* Underlying HZ for R8610 */
 #else
 #  define PIT_TICK_RATE 1193182 /* Underlying HZ */
 #endif

... once that is made dynamic/quirkable we are there.

And we could even remove the AMD Elan subarch that way: AFAICS it's a 
486 compatible with an A20 quirk and this PIT_TICK quirk.

... and if we do that we'll also remove these:

        select GENERIC_GPIO
        select LEDS_CLASS
        select LEDS_GPIO
        select NEW_LEDS

... and thus perhaps your GPIO_USERSELECTION patch should move into 
drivers/ and be generally accessible, not special to x86?

	Ingo

Re: [PATCH RFC] x86: Add user configurable GPIO-lib support

[Michael Buesch]

On Tuesday 01 July 2008 13:04:37 Ingo Molnar wrote:
> 
> * Michael Buesch <mb@bu3sch.de> wrote:
> 
> > So this adds user-configurable GPIO support through gpiolib on 
> > subarchitectures that do not implement a GPIO implementation, yet. 
> > Currently that's everything except X86_RDC321X.
> > 
> > The advantage of this is to make it possible to use generic PCI (or 
> > other bus) GPIO extention cards in standard PCs through the standard 
> > GPIO API.
> > 
> > If another subarch implements its own GPIO, it needs to add itself as 
> > an inverted dependency to GPIO_USERSELECTION to make sure the user 
> > does not enable two GPIO API implementations.
> > 
> > About the asm-x86/gpio.h:
> > I'm not sure what this <gpio.h> include currently is.
> > Can somebody explain that to me? Where is this supposed
> > to include a gpio.h file from?
> > 
> > What's your opinion on this?
> 
> ( i've Cc:-ed Florian on this who's maintaining the RDC R-321X bits. )
> 
> The longer-term goal is that we'd like to remove the explicit RDC 
> sub-arch and just support such systems out of box on x86.

nice :)

> ... and thus perhaps your GPIO_USERSELECTION patch should move into 
> drivers/ and be generally accessible, not special to x86?

Yes I'd really like to move it there, too. But currently that
clashes with architectures like MIPS, some PPC flavours and probably
others that implement their own GPIO API.
We should have an ARCH_IMPLEMENT_GPIO or whatever, but currently we
don't seem to have that.

So well. If it's desired to put the user selection into drivers/gpio
(which I'd really prefer), I can try to make a patch that adds
ARCH_IMPLEMENT_GPIO to every arch that implements their own GPIO API
and make GPIO_USERSELECTION depend on !ARCH_IMPLEMENT_GPIO.

-- 
Greetings Michael.

Re: [PATCH RFC] x86: Add user configurable GPIO-lib support

[Ingo Molnar]

* Michael Buesch <mb@bu3sch.de> wrote:

> > ... and thus perhaps your GPIO_USERSELECTION patch should move into 
> > drivers/ and be generally accessible, not special to x86?
> 
> Yes I'd really like to move it there, too. But currently that clashes 
> with architectures like MIPS, some PPC flavours and probably others 
> that implement their own GPIO API. We should have an 
> ARCH_IMPLEMENT_GPIO or whatever, but currently we don't seem to have 
> that.
> 
> So well. If it's desired to put the user selection into drivers/gpio 
> (which I'd really prefer), I can try to make a patch that adds 
> ARCH_IMPLEMENT_GPIO to every arch that implements their own GPIO API 
> and make GPIO_USERSELECTION depend on !ARCH_IMPLEMENT_GPIO.

or we could try it the other way around: stick 
ARCH_ALLOWS_TRULY_GENERAL_PURPOSE_INPUT_OUTPUT (note: please use a 
better name ;-) into x86, add the user text to drivers/ and let other 
architectures enable it too?

That way your commits would still be pretty focused (you'd avoid having 
to touch a lot of architectures) and it would still work all across the 
spectrum.

hm?

	Ingo

Re: [PATCH RFC] x86: Add user configurable GPIO-lib support

[Michael Buesch]

On Tuesday 01 July 2008 13:19:18 Ingo Molnar wrote:
> 
> * Michael Buesch <mb@bu3sch.de> wrote:
> 
> > > ... and thus perhaps your GPIO_USERSELECTION patch should move into 
> > > drivers/ and be generally accessible, not special to x86?
> > 
> > Yes I'd really like to move it there, too. But currently that clashes 
> > with architectures like MIPS, some PPC flavours and probably others 
> > that implement their own GPIO API. We should have an 
> > ARCH_IMPLEMENT_GPIO or whatever, but currently we don't seem to have 
> > that.
> > 
> > So well. If it's desired to put the user selection into drivers/gpio 
> > (which I'd really prefer), I can try to make a patch that adds 
> > ARCH_IMPLEMENT_GPIO to every arch that implements their own GPIO API 
> > and make GPIO_USERSELECTION depend on !ARCH_IMPLEMENT_GPIO.
> 
> or we could try it the other way around: stick 
> ARCH_ALLOWS_TRULY_GENERAL_PURPOSE_INPUT_OUTPUT (note: please use a 
> better name ;-) into x86, add the user text to drivers/ and let other 
> architectures enable it too?
> 
> That way your commits would still be pretty focused (you'd avoid having 
> to touch a lot of architectures) and it would still work all across the 
> spectrum.
> 
> hm?

Yeah great idea :)
I'll make a patch later.

-- 
Greetings Michael.

Re: [PATCH RFC] x86: Add user configurable GPIO-lib support

[Florian Fainelli]

Hello Michael, Ingo,

Le Tuesday 01 July 2008 13:04:37 Ingo Molnar, vous avez écrit :
> * Michael Buesch <mb@bu3sch.de> wrote:
> > So this adds user-configurable GPIO support through gpiolib on
> > subarchitectures that do not implement a GPIO implementation, yet.
> > Currently that's everything except X86_RDC321X.

The only reason RDC321x is not yet converted to using gpiolib is the small 
number of actually accessible GPIOs, anyway, I will do the conversion soon.

> >
> > The advantage of this is to make it possible to use generic PCI (or
> > other bus) GPIO extention cards in standard PCs through the standard
> > GPIO API.
> >
> > If another subarch implements its own GPIO, it needs to add itself as
> > an inverted dependency to GPIO_USERSELECTION to make sure the user
> > does not enable two GPIO API implementations.
> >
> > About the asm-x86/gpio.h:
> > I'm not sure what this <gpio.h> include currently is.
> > Can somebody explain that to me? Where is this supposed
> > to include a gpio.h file from?
> >

This code is inherited from MIPS/ARM before GPIO lib, where generic assembly 
headers provide prototypes for the usual GPIO functions. Boards override 
those functions with their own copy of these GPIO functions.

>  #ifdef CONFIG_X86_ELAN
>  #  define PIT_TICK_RATE 1189200 /* AMD Elan has different frequency! */
>  #elif defined(CONFIG_X86_RDC321X)
>  #  define PIT_TICK_RATE 1041667 /* Underlying HZ for R8610 */
>  #else
>  #  define PIT_TICK_RATE 1193182 /* Underlying HZ */
>  #endif
>
> ... once that is made dynamic/quirkable we are there.

Yes, I did not have time to investigate this, and am still wondering if we can 
have dynamic tick rate settings in early board setup code.
-- 
Cordialement, Florian Fainelli
------------------------------

Re: [PATCH RFC] x86: Add user configurable GPIO-lib support

[Ingo Molnar]

* Florian Fainelli <florian.fainelli@telecomint.eu> wrote:

> > ... once that is made dynamic/quirkable we are there.
> 
> Yes, I did not have time to investigate this, and am still wondering 
> if we can have dynamic tick rate settings in early board setup code.

well, does the system have any early boot detectable property? Such as a 
DMI signature? That way there would not be anything truly dynamic about 
the PIT rate - we'd just set a new pit_tick_rate variable to a 
non-default value very early during bootup, on an RDC system.

On usual PCs it would stay unmodified, at the default 1193182 value.

	Ingo

Re: [PATCH RFC] x86: Add user configurable GPIO-lib support

[Michael Buesch]

On Tuesday 01 July 2008 13:19:18 Ingo Molnar wrote:
> 
> * Michael Buesch <mb@bu3sch.de> wrote:
> 
> > > ... and thus perhaps your GPIO_USERSELECTION patch should move into 
> > > drivers/ and be generally accessible, not special to x86?
> > 
> > Yes I'd really like to move it there, too. But currently that clashes 
> > with architectures like MIPS, some PPC flavours and probably others 
> > that implement their own GPIO API. We should have an 
> > ARCH_IMPLEMENT_GPIO or whatever, but currently we don't seem to have 
> > that.
> > 
> > So well. If it's desired to put the user selection into drivers/gpio 
> > (which I'd really prefer), I can try to make a patch that adds 
> > ARCH_IMPLEMENT_GPIO to every arch that implements their own GPIO API 
> > and make GPIO_USERSELECTION depend on !ARCH_IMPLEMENT_GPIO.
> 
> or we could try it the other way around: stick 
> ARCH_ALLOWS_TRULY_GENERAL_PURPOSE_INPUT_OUTPUT (note: please use a 
> better name ;-) into x86, add the user text to drivers/ and let other 
> architectures enable it too?
> 
> That way your commits would still be pretty focused (you'd avoid having 
> to touch a lot of architectures) and it would still work all across the 
> spectrum.

Something like this, perhaps. Not 100% finished, yet (see the FIXME).


Index: linux-2.6/arch/x86/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/Kconfig	2008-07-01 12:45:23.000000000 +0200
+++ linux-2.6/arch/x86/Kconfig	2008-07-02 00:21:05.000000000 +0200
@@ -25,6 +25,7 @@ config X86
 	select HAVE_KRETPROBES
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
 	select HAVE_ARCH_KGDB if !X86_VOYAGER
+	select ARCH_WANT_OPTIONAL_GPIOLIB if !X86_RDC321X
 
 config ARCH_DEFCONFIG
 	string
Index: linux-2.6/include/asm-x86/gpio.h
===================================================================
--- linux-2.6.orig/include/asm-x86/gpio.h	2008-07-01 12:45:23.000000000 +0200
+++ linux-2.6/include/asm-x86/gpio.h	2008-07-02 00:00:34.000000000 +0200
@@ -1,6 +1,62 @@
+/*
+ * Generic GPIO API implementation for x86.
+ *
+ * Derived from the generic GPIO API for powerpc:
+ *
+ * Copyright (c) 2007-2008  MontaVista Software, Inc.
+ *
+ * Author: Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
 #ifndef _ASM_I386_GPIO_H
 #define _ASM_I386_GPIO_H
 
+#ifdef CONFIG_X86_RDC321X
 #include <gpio.h>
+#else /* CONFIG_X86_RDC321X */
+
+#include <asm-generic/gpio.h>
+
+#ifdef CONFIG_HAVE_GPIO_LIB
+
+/*
+ * Just call gpiolib.
+ */
+static inline int gpio_get_value(unsigned int gpio)
+{
+	return __gpio_get_value(gpio);
+}
+
+static inline void gpio_set_value(unsigned int gpio, int value)
+{
+	__gpio_set_value(gpio, value);
+}
+
+static inline int gpio_cansleep(unsigned int gpio)
+{
+	return __gpio_cansleep(gpio);
+}
+
+/*
+ * Not implemented, yet.
+ */
+static inline int gpio_to_irq(unsigned int gpio)
+{
+	return -ENOSYS;
+}
+
+static inline int irq_to_gpio(unsigned int irq)
+{
+	return -EINVAL;
+}
+
+#endif /* CONFIG_HAVE_GPIO_LIB */
+
+#endif /* CONFIG_X86_RDC321X */
 
 #endif /* _ASM_I386_GPIO_H */
Index: linux-2.6/arch/powerpc/Kconfig
===================================================================
--- linux-2.6.orig/arch/powerpc/Kconfig	2008-07-01 12:45:23.000000000 +0200
+++ linux-2.6/arch/powerpc/Kconfig	2008-07-02 01:01:26.000000000 +0200
@@ -110,6 +110,7 @@ config PPC
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_LMB
+	select ARCH_WANT_OPTIONAL_GPIOLIB
 
 config EARLY_PRINTK
 	bool
Index: linux-2.6/drivers/gpio/Kconfig
===================================================================
--- linux-2.6.orig/drivers/gpio/Kconfig	2008-07-01 12:45:23.000000000 +0200
+++ linux-2.6/drivers/gpio/Kconfig	2008-07-02 01:00:16.000000000 +0200
@@ -2,15 +2,39 @@
 # GPIO infrastructure and expanders
 #
 
+config ARCH_WANT_OPTIONAL_GPIOLIB
+	bool
+	help
+	  Select this config option from the architecture Kconfig, if
+	  it is possible to use gpiolib on the architecture, but let the
+	  user decide whether to actually build it or not.
+	  Select this instead of HAVE_GPIO_LIB, if your architecture does
+	  not depend on GPIOs being available, but rather let the user
+	  decide whether he needs it or not.
+
+#FIXME: Probably rename the following to ARCH_REQUIRE_GPIOLIB
 config HAVE_GPIO_LIB
 	bool
+	select GPIOLIB
 	help
 	  Platforms select gpiolib if they use this infrastructure
 	  for all their GPIOs, usually starting with ones integrated
 	  into SOC processors.
+	  Selecting this from the architecture code will cause the gpiolib
+	  code to always get built in.
+
+menuconfig GPIOLIB
+	bool "GPIO Support"
+	depends on ARCH_WANT_OPTIONAL_GPIOLIB || HAVE_GPIO_LIB
+	select GENERIC_GPIO
+	help
+	  This enables GPIO support through the generic GPIO library.
+	  You only need to enable this, if you also want to enable
+	  one or more of the GPIO expansion card drivers below.
+
+	  If unsure, say N.
 
-menu "GPIO Support"
-	depends on HAVE_GPIO_LIB
+if GPIOLIB
 
 config DEBUG_GPIO
 	bool "Debug GPIO calls"
@@ -70,4 +94,4 @@ config GPIO_MCP23S08
 	  SPI driver for Microchip MCP23S08 I/O expander.  This provides
 	  a GPIO interface supporting inputs and outputs.
 
-endmenu
+endif
Index: linux-2.6/drivers/gpio/Makefile
===================================================================
--- linux-2.6.orig/drivers/gpio/Makefile	2008-07-01 12:45:23.000000000 +0200
+++ linux-2.6/drivers/gpio/Makefile	2008-07-02 00:45:11.000000000 +0200
@@ -2,7 +2,7 @@
 
 ccflags-$(CONFIG_DEBUG_GPIO)	+= -DDEBUG
 
-obj-$(CONFIG_HAVE_GPIO_LIB)	+= gpiolib.o
+obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 
 obj-$(CONFIG_GPIO_MCP23S08)	+= mcp23s08.o
 obj-$(CONFIG_GPIO_PCA953X)	+= pca953x.o



-- 
Greetings Michael.

Re: [PATCH RFC] x86: Add user configurable GPIO-lib support

[David Brownell]

More food for thought ... 

This makes the rdc321x support switch over to using gpiolib,
along with all of x86.  Quick'n'dirty.

It's perhaps best done in conjunction with something to
make GPIOLIB be user-configurable on systems where it's not
already selected by platform code.  The rdc321x stuff needs
more working, but this is a start.

- Dave

---
 arch/x86/Kconfig                            |    6 +
 arch/x86/mach-rdc321x/gpio.c                |  107 ++++++++--------------------
 arch/x86/mach-rdc321x/platform.c            |    3 
 include/asm-x86/gpio.h                      |   22 +++++
 include/asm-x86/mach-rdc321x/gpio.h         |   52 -------------
 include/asm-x86/mach-rdc321x/rdc321x_defs.h |    2 
 6 files changed, 62 insertions(+), 130 deletions(-)

--- a/arch/x86/Kconfig	2008-07-01 16:46:11.000000000 -0700
+++ b/arch/x86/Kconfig	2008-07-01 16:46:13.000000000 -0700
@@ -87,7 +87,10 @@ config GENERIC_HWEIGHT
 	def_bool y
 
 config GENERIC_GPIO
-	def_bool n
+	def_bool y
+
+config HAVE_GPIO_LIB
+	def_bool y
 
 config ARCH_MAY_HAVE_PC_FDC
 	def_bool y
@@ -325,7 +328,6 @@ config X86_RDC321X
 	depends on X86_32
 	select M486
 	select X86_REBOOTFIXUPS
-	select GENERIC_GPIO
 	select LEDS_CLASS
 	select LEDS_GPIO
 	select NEW_LEDS
--- a/arch/x86/mach-rdc321x/gpio.c	2008-07-01 16:46:11.000000000 -0700
+++ b/arch/x86/mach-rdc321x/gpio.c	2008-07-01 17:06:48.000000000 -0700
@@ -25,8 +25,9 @@
 #include <linux/io.h>
 #include <linux/types.h>
 #include <linux/module.h>
+#include <linux/gpio.h>
 
-#include <asm/gpio.h>
+#include <asm/mach-rdc321x/gpio.h>
 #include <asm/mach-rdc321x/rdc321x_defs.h>
 
 
@@ -38,8 +39,6 @@ static DEFINE_SPINLOCK(gpio_lock);
 static u32 gpio_data_reg1;
 static u32 gpio_data_reg2;
 
-static u32 gpio_request_data[2];
-
 
 static inline void rdc321x_conf_write(unsigned addr, u32 value)
 {
@@ -73,61 +72,8 @@ static void rdc321x_configure_gpio(unsig
 	spin_unlock_irqrestore(&gpio_lock, flags);
 }
 
-/* initially setup the 2 copies of the gpio data registers.
-   This function must be called by the platform setup code. */
-void __init rdc321x_gpio_setup()
-{
-	/* this might not be, what others (BIOS, bootloader, etc.)
-	   wrote to these registers before, but it's a good guess. Still
-	   better than just using 0xffffffff. */
-
-	gpio_data_reg1 = rdc321x_conf_read(RDC321X_GPIO_DATA_REG1);
-	gpio_data_reg2 = rdc321x_conf_read(RDC321X_GPIO_DATA_REG2);
-}
-
-/* determine, if gpio number is valid */
-static inline int rdc321x_is_gpio(unsigned gpio)
-{
-	return gpio <= RDC321X_MAX_GPIO;
-}
-
-/* request GPIO */
-int rdc_gpio_request(unsigned gpio, const char *label)
-{
-	unsigned long flags;
-
-	if (!rdc321x_is_gpio(gpio))
-		return -EINVAL;
-
-	spin_lock_irqsave(&gpio_lock, flags);
-	if (gpio_request_data[(gpio & 0x20) ? 1 : 0] & (1 << (gpio & 0x1f)))
-		goto inuse;
-	gpio_request_data[(gpio & 0x20) ? 1 : 0] |= (1 << (gpio & 0x1f));
-	spin_unlock_irqrestore(&gpio_lock, flags);
-
-	return 0;
-inuse:
-	spin_unlock_irqrestore(&gpio_lock, flags);
-	return -EINVAL;
-}
-EXPORT_SYMBOL(rdc_gpio_request);
-
-/* release previously-claimed GPIO */
-void rdc_gpio_free(unsigned gpio)
-{
-	unsigned long flags;
-
-	if (!rdc321x_is_gpio(gpio))
-		return;
-
-	spin_lock_irqsave(&gpio_lock, flags);
-	gpio_request_data[(gpio & 0x20) ? 1 : 0] &= ~(1 << (gpio & 0x1f));
-	spin_unlock_irqrestore(&gpio_lock, flags);
-}
-EXPORT_SYMBOL(rdc_gpio_free);
-
 /* read GPIO pin */
-int rdc_gpio_get_value(unsigned gpio)
+static int rdc_gpio_get_value(struct gpio_chip *g, unsigned gpio)
 {
 	u32 reg;
 	unsigned long flags;
@@ -139,56 +85,69 @@ int rdc_gpio_get_value(unsigned gpio)
 
 	return (1 << (gpio & 0x1f)) & reg ? 1 : 0;
 }
-EXPORT_SYMBOL(rdc_gpio_get_value);
 
 /* set GPIO pin to value */
-void rdc_gpio_set_value(unsigned gpio, int value)
+static void rdc_gpio_set_value(struct gpio_chip *g, unsigned gpio, int value)
 {
 	unsigned long flags;
 	u32 reg;
 
 	reg = 1 << (gpio & 0x1f);
+	spin_lock_irqsave(&gpio_lock, flags);
 	if (gpio < 32) {
-		spin_lock_irqsave(&gpio_lock, flags);
 		if (value)
 			gpio_data_reg1 |= reg;
 		else
 			gpio_data_reg1 &= ~reg;
 		rdc321x_conf_write(RDC321X_GPIO_DATA_REG1, gpio_data_reg1);
-		spin_unlock_irqrestore(&gpio_lock, flags);
 	} else {
-		spin_lock_irqsave(&gpio_lock, flags);
 		if (value)
 			gpio_data_reg2 |= reg;
 		else
 			gpio_data_reg2 &= ~reg;
 		rdc321x_conf_write(RDC321X_GPIO_DATA_REG2, gpio_data_reg2);
-		spin_unlock_irqrestore(&gpio_lock, flags);
 	}
+	spin_unlock_irqrestore(&gpio_lock, flags);
 }
-EXPORT_SYMBOL(rdc_gpio_set_value);
 
 /* configure GPIO pin as input */
-int rdc_gpio_direction_input(unsigned gpio)
+static int rdc_gpio_direction_input(struct gpio_chip *g, unsigned gpio)
 {
-	if (!rdc321x_is_gpio(gpio))
-		return -EINVAL;
-
 	rdc321x_configure_gpio(gpio);
 
 	return 0;
 }
-EXPORT_SYMBOL(rdc_gpio_direction_input);
 
 /* configure GPIO pin as output and set value */
-int rdc_gpio_direction_output(unsigned gpio, int value)
+static int rdc_gpio_direction_output(struct gpio_chip *g, unsigned gpio, int value)
 {
-	if (!rdc321x_is_gpio(gpio))
-		return -EINVAL;
-
 	gpio_set_value(gpio, value);
 	rdc321x_configure_gpio(gpio);
 
 	return 0;
 }
-EXPORT_SYMBOL(rdc_gpio_direction_output);
+
+static struct gpio_chip rdc_gpio_chip = {
+	.label			= "rdc321x",
+	.direction_input	= rdc_gpio_direction_input,
+	.get			= rdc_gpio_get_value,
+	.direction_output	= rdc_gpio_direction_output,
+	.set			= rdc_gpio_set_value,
+	.base			= 0,
+	.ngpio			= RDC321X_MAX_GPIO + 1,
+};
+
+/* initially setup the 2 copies of the gpio data registers.
+   This function must be called by the platform setup code. */
+void __init rdc321x_gpio_setup()
+{
+	/* this might not be, what others (BIOS, bootloader, etc.)
+	   wrote to these registers before, but it's a good guess. Still
+	   better than just using 0xffffffff. */
+
+	gpio_data_reg1 = rdc321x_conf_read(RDC321X_GPIO_DATA_REG1);
+	gpio_data_reg2 = rdc321x_conf_read(RDC321X_GPIO_DATA_REG2);
+
+	gpiochip_add(&rdc_gpio_chip);
+}
+
--- a/arch/x86/mach-rdc321x/platform.c	2008-07-01 16:46:11.000000000 -0700
+++ b/arch/x86/mach-rdc321x/platform.c	2008-07-01 16:46:13.000000000 -0700
@@ -27,8 +27,7 @@
 #include <linux/platform_device.h>
 #include <linux/version.h>
 #include <linux/leds.h>
-
-#include <asm/gpio.h>
+#include <linux/gpio.h>
 
 /* LEDS */
 static struct gpio_led default_leds[] = {
--- a/include/asm-x86/gpio.h	2008-07-01 16:46:11.000000000 -0700
+++ b/include/asm-x86/gpio.h	2008-07-01 16:46:13.000000000 -0700
@@ -1,6 +1,26 @@
 #ifndef _ASM_I386_GPIO_H
 #define _ASM_I386_GPIO_H
 
-#include <gpio.h>
+#include <linux/errno.h>
+
+#ifdef CONFIG_X86_RDC321X
+#include <asm/mach-rdc321x/gpio.h>
+#endif
+
+#include <asm-generic/gpio.h>
+
+#define gpio_get_value	__gpio_get_value
+#define gpio_set_value	__gpio_set_value
+#define gpio_cansleep	__gpio_cansleep
+
+static inline int gpio_to_irq(unsigned gpio)
+{
+	return -ENOSYS;
+}
+
+static inline int irq_to_gpio(unsigned irq)
+{
+	return -EINVAL;
+}
 
 #endif /* _ASM_I386_GPIO_H */
--- a/include/asm-x86/mach-rdc321x/gpio.h	2008-07-01 16:46:11.000000000 -0700
+++ b/include/asm-x86/mach-rdc321x/gpio.h	2008-07-01 17:08:38.000000000 -0700
@@ -1,57 +1,11 @@
 #ifndef _RDC321X_GPIO_H
 #define _RDC321X_GPIO_H
 
-extern int rdc_gpio_get_value(unsigned gpio);
-extern void rdc_gpio_set_value(unsigned gpio, int value);
-extern int rdc_gpio_direction_input(unsigned gpio);
-extern int rdc_gpio_direction_output(unsigned gpio, int value);
-extern int rdc_gpio_request(unsigned gpio, const char *label);
-extern void rdc_gpio_free(unsigned gpio);
 extern void __init rdc321x_gpio_setup(void);
 
-/* Wrappers for the arch-neutral GPIO API */
-
-static inline int gpio_request(unsigned gpio, const char *label)
-{
-	return rdc_gpio_request(gpio, label);
-}
-
-static inline void gpio_free(unsigned gpio)
-{
-	rdc_gpio_free(gpio);
-}
-
-static inline int gpio_direction_input(unsigned gpio)
-{
-	return rdc_gpio_direction_input(gpio);
-}
-
-static inline int gpio_direction_output(unsigned gpio, int value)
-{
-	return rdc_gpio_direction_output(gpio, value);
-}
-
-static inline int gpio_get_value(unsigned gpio)
-{
-	return rdc_gpio_get_value(gpio);
-}
-
-static inline void gpio_set_value(unsigned gpio, int value)
-{
-	rdc_gpio_set_value(gpio, value);
-}
-
-static inline int gpio_to_irq(unsigned gpio)
-{
-	return gpio;
-}
-
-static inline int irq_to_gpio(unsigned irq)
-{
-	return irq;
-}
+#define RDC321X_MAX_GPIO	58
 
-/* For cansleep */
-#include <asm-generic/gpio.h>
+/* allow for a few external GPIO expanders */
+#define	ARCH_NR_GPIOS		(RDC321X_MAX_GPIO + 64)
 
 #endif /* _RDC321X_GPIO_H_ */
--- a/include/asm-x86/mach-rdc321x/rdc321x_defs.h	2008-07-01 16:46:11.000000000 -0700
+++ b/include/asm-x86/mach-rdc321x/rdc321x_defs.h	2008-07-01 16:46:13.000000000 -0700
@@ -8,5 +8,3 @@
 #define RDC321X_GPIO_CTRL_REG2	0x84
 #define RDC321X_GPIO_DATA_REG1	0x4c
 #define RDC321X_GPIO_DATA_REG2	0x88
-
-#define RDC321X_MAX_GPIO	58

Re: [PATCH RFC] x86: Add user configurable GPIO-lib support

[Michael Buesch]

On Wednesday 02 July 2008 04:57:41 David Brownell wrote:
> More food for thought ... 
> 
> This makes the rdc321x support switch over to using gpiolib,
> along with all of x86.  Quick'n'dirty.
> 
> It's perhaps best done in conjunction with something to
> make GPIOLIB be user-configurable on systems where it's not
> already selected by platform code.  The rdc321x stuff needs
> more working, but this is a start.
> 
> - Dave
> 
> ---
>  arch/x86/Kconfig                            |    6 +
>  arch/x86/mach-rdc321x/gpio.c                |  107 ++++++++--------------------
>  arch/x86/mach-rdc321x/platform.c            |    3 
>  include/asm-x86/gpio.h                      |   22 +++++
>  include/asm-x86/mach-rdc321x/gpio.h         |   52 -------------
>  include/asm-x86/mach-rdc321x/rdc321x_defs.h |    2 
>  6 files changed, 62 insertions(+), 130 deletions(-)
> 
> --- a/arch/x86/Kconfig	2008-07-01 16:46:11.000000000 -0700
> +++ b/arch/x86/Kconfig	2008-07-01 16:46:13.000000000 -0700
> @@ -87,7 +87,10 @@ config GENERIC_HWEIGHT
>  	def_bool y
>  
>  config GENERIC_GPIO
> -	def_bool n
> +	def_bool y
> +
> +config HAVE_GPIO_LIB
> +	def_bool y

I think we should not unconditionally compile in gpiolib.
See my other patch that converts the stuff to be user selectable.

(Too bad all my mail bounces at Daves mailfilter and this won't reach him... :/ )


-- 
Greetings Michael.