x86: Add user configurable GPIO-lib support

RFC PATCH In review Michael Buesch 3 participants 8 messages Jul 1, 2008

Subsystems (3)

x86 PowerPC GPIO

lore

Share & tools

lore

Michael Buesch

author

2008-07-01 10:51 UTC #0 lore raw

RFC PATCH 1/1 x86: Add user configurable GPIO-lib support

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?

arch/x86/Kconfig +13 -0

Index: wireless-testing/arch/x86/Kconfig
===================================================================
@@ -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

include/asm-x86/gpio.h +56 -0

62 lines not on this page · raw message

Signature · 1 line

Greetings Michael.

Ingo Molnar

2008-07-01 11:05 UTC #1 lore raw

Quoted 12 lines · 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.

Quoted 6 lines

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

Michael Buesch

author

2008-07-01 11:14 UTC #2 lore raw

Quoted 20 lines · On Tuesday 01 July 2008 13:04:37 Ingo Molnar wrote:

Quoted 17 lines · 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?

Quoted 6 lines

Quoted 1 line
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 :)

Quoted 2 lines

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

Signature · 1 line

Greetings Michael.

Ingo Molnar

2008-07-01 11:19 UTC #3 lore raw

Quoted 9 lines · Michael Buesch

Quoted 2 lines
... 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.

Quoted 4 lines

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

Michael Buesch

author

2008-07-01 11:28 UTC #4 lore raw

Quoted 22 lines · On Tuesday 01 July 2008 13:19:18 Ingo Molnar wrote:

Quoted 9 lines · Michael Buesch
Quoted 2 lines
... 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.
Quoted 4 lines
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?

Quoted 5 lines

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.

Signature · 1 line

Greetings Michael.

Michael Buesch

author

2008-07-01 23:07 UTC #5 lore raw

In reply to Ingo Molnar

Quoted 17 lines · On Tuesday 01 July 2008 13:19:18 Ingo Molnar wrote:

Quoted 9 lines · Michael Buesch
Quoted 2 lines
... 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.
Quoted 4 lines
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.

Quoted 8 lines

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

arch/x86/Kconfig +1 -0

Index: linux-2.6/arch/x86/Kconfig
===================================================================
@@ -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

include/asm-x86/gpio.h +56 -0

62 lines not on this page · raw message

arch/powerpc/Kconfig +1 -0

Index: linux-2.6/arch/powerpc/Kconfig
===================================================================
@@ -110,6 +110,7 @@ config PPC
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
 	select HAVE_LMB
+	select ARCH_WANT_OPTIONAL_GPIOLIB
 
 config EARLY_PRINTK
 	bool

drivers/gpio/Kconfig +27 -3

Index: linux-2.6/drivers/gpio/Kconfig
===================================================================
@@ -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

drivers/gpio/Makefile +1 -1

Index: linux-2.6/drivers/gpio/Makefile
===================================================================
@@ -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

Signature · 1 line

Greetings Michael.

Florian Fainelli

2008-07-01 12:56 UTC #6 lore raw

In reply to Ingo Molnar

Hello Michael, Ingo,

Le Tuesday 01 July 2008 13:04:37 Ingo Molnar, vous avez écrit :

Quoted 4 lines · Michael Buesch

Quoted 3 lines
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.

Quoted 9 lines

Quoted 9 lines
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.

Quoted 5 lines

Quoted 5 lines
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.

Quoted 9 lines

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

Signature · 2 lines

Cordialement, Florian Fainelli
------------------------------

Ingo Molnar

2008-07-01 14:27 UTC #7 lore raw

Quoted 4 lines · Florian Fainelli

Quoted 1 line
... 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