gpiolib: mcp23s08 spi gpio expander support
Subsystems (8)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/9 gpiolib: add drivers/gpio directory
From: David Brownell <[email protected]>
Add an empty drivers/gpio directory for gpiolib infrastructure and GPIO expanders. It is populated by subsequent patches.
This won't be the only place to hold such gpio_chip code. Many external chips add a few GPIOs as secondary functionality (such as MFD drivers) and platform code frequently needs to closely integrate GPIO and IRQ support.
This is placed *early* in the build/link sequence since it's common for other drivers to depend on GPIOs to do their work, so they must be initialized early in the device_initcall() sequence.
--- arch/arm/Kconfig | 2 ++ drivers/Kconfig | 2 ++ drivers/Makefile | 1 + drivers/gpio/Kconfig | 32 ++++++++++++++++++++++++++++++++ drivers/gpio/Makefile | 6 ++++++ 5 files changed, 43 insertions(+)
arch/arm/Kconfig +2 -0
@@ -1040,6 +1040,8 @@ source "drivers/i2c/Kconfig" source "drivers/spi/Kconfig" +source "drivers/gpio/Kconfig" + source "drivers/w1/Kconfig" source "drivers/power/Kconfig"
drivers/Kconfig +2 -0
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig" source "drivers/spi/Kconfig" +source "drivers/gpio/Kconfig" + source "drivers/w1/Kconfig" source "drivers/power/Kconfig"
drivers/Makefile +1 -0
@@ -5,6 +5,7 @@ # Rewritten to use lists instead of if-statements. # +obj-$(CONFIG_GPIO_LIB) += gpio/ obj-$(CONFIG_PCI) += pci/ obj-$(CONFIG_PARISC) += parisc/ obj-$(CONFIG_RAPIDIO) += rapidio/
drivers/gpio/Kconfig new file +32 -0
@@ -0,0 +1,32 @@ +# +# GPIO infrastructure and expanders +# + +config GPIO_LIB + bool + help + Platforms select gpiolib if they use this infrastructure + for all their GPIOs, usually starting with ones integrated + into SOC processors. + +menu "GPIO Support" + depends on GPIO_LIB + +config DEBUG_GPIO + bool "Debug GPIO calls" + depends on DEBUG_KERNEL + help + Say Y here to add some extra checks and diagnostics to GPIO calls. + The checks help ensure that GPIOs have been properly initialized + before they are used and that sleeping calls aren not made from + nonsleeping contexts. They can make bitbanged serial protocols + slower. The diagnostics help catch the type of setup errors + that are most common when setting up new platforms or boards. + +# put expanders in the right section, in alphabetical order + +comment "I2C GPIO expanders:" + +comment "SPI GPIO expanders:" + +endmenu
drivers/gpio/Makefile new file +6 -0
@@ -0,0 +1,6 @@ +# gpio support: dedicated expander chips, etc + +ifeq ($(CONFIG_DEBUG_GPIO),y) +EXTRA_CFLAGS += -DDEBUG +endif +
Re: [patch 2.6.24-rc6-mm 1/9] gpiolib: add drivers/gpio directory
Hi David.
Quoted 14 lines
drivers/Makefile +1
-0@@ -5,6 +5,7 @@ # Rewritten to use lists instead of if-statements. # +obj-$(CONFIG_GPIO_LIB) += gpio/ obj-$(CONFIG_PCI) += pci/ obj-$(CONFIG_PARISC) += parisc/ obj-$(CONFIG_RAPIDIO) += rapidio/drivers/gpio/Kconfig new file +1
-0@@ -0,0 +1,32 @@ +#
drivers/gpio/Kconfig line 11
Quoted 10 lines
drivers/gpio/Kconfig new file +10
-0+# GPIO infrastructure and expanders +# + +config GPIO_LIB + bool + help + Platforms select gpiolib if they use this infrastructure + for all their GPIOs, usually starting with ones integrated + into SOC processors. +
kconfig symbols that are "select" targets should be named "HAVE_" so in this case you could use HAVE_GPIO_LIB.
This is by convention only but introduced to make it visible that this is a config symbol supposed to be selected.
Then on top of that the HAVE_ symbols should not have any dependencies so we avoid that "select" selects a symbol where the dependencies are not fulfilled.
In your case you could just use
obj-y += gpio/
in the Makefile and then there is no need to actually reference the kconfig symbol.
drivers/gpio/Makefile line 5
Quoted 8 lines
drivers/gpio/Makefile new file +5
-0@@ -0,0 +1,6 @@ +# gpio support: dedicated expander chips, etc + +ifeq ($(CONFIG_DEBUG_GPIO),y) +EXTRA_CFLAGS += -DDEBUG +endif
Use of EXTRA_CFLAGS are deprecated. Please use:
ccflags-$(CONFIG_DEBUG_GPIO) := -DDEBUG
Sam
Re: [patch 2.6.24-rc6-mm 1/9] gpiolib: add drivers/gpio directory
Quoted 4 lines · Sam Ravnborg
kconfig symbols that are "select" targets should be named "
HAVE_" so in this case you could useHAVE_GPIO_LIB.
This is by convention only but introduced to make it visible that this is a config symbol supposed to be selected.
Not widely adopted yet, but of course such things take time.
Quoted 3 lines
Then on top of that the
HAVE_symbols should not have any dependencies so we avoid that "select" selects a symbol where the dependencies are not fulfilled.
That limitation of "select" still seems more buglike to me than anything else, FWIW. Not that it matters in this case.
Quoted 5 lines
In your case you could just use
obj-y += gpio/in the
Makefileand then there is no need to actually reference the kconfig symbol.
No, that just shifts the reference into the drivers/gpio Makefilee.
Quoted 1 line
ccflags-$(CONFIG_DEBUG_GPIO) := -DDEBUG
Another new convention. :)
Pretty much like the folowing, then?
- Dave
============== CUT HERE
Kbuild/Kconfig feedback from Sam Ravnborg, adopting new conventions:
- HAVE_* for Kconfig symbols that platforms will select
- Descend into drivers/gpio unconditionally, letting the Makefile sort out the differences
- Use "ccflags-$(CONGIF_DEBUG_GPIO) += -DDEBUG" instead of those ugly (but traditional) conditionals.
--- arch/arm/Kconfig | 4 +- arch/avr32/Kconfig | 2 +- drivers/Makefile | 2 +- drivers/gpio/Kconfig | 4 ++-- drivers/gpio/Makefile | 10 ++++------ include/asm-generic/gpio.h | 2 +- 6 files changed, 10 insertions(+), 12 deletions(-)
arch/arm/Kconfig +1 -1
@@ -346,7 +346,7 @@ config ARCH_PXA select GENERIC_GPIO select GENERIC_TIME select GENERIC_CLOCKEVENTS - select GPIO_LIB + select HAVE_GPIO_LIB help Support for Intel/Marvell's PXA2xx/PXA3xx processor line.
arch/avr32/Kconfig +1 -1
@@ -80,7 +80,7 @@ config PLATFORM_AT32AP select SUBARCH_AVR32B select MMU select PERFORMANCE_COUNTERS - select GPIO_LIB + select HAVE_GPIO_LIB choice prompt "AVR32 CPU type"
drivers/Makefile +1 -1
@@ -5,7 +5,7 @@ # Rewritten to use lists instead of if-statements. # -obj-$(CONFIG_GPIO_LIB) += gpio/ +obj-y += gpio/ obj-$(CONFIG_PCI) += pci/ obj-$(CONFIG_PARISC) += parisc/ obj-$(CONFIG_RAPIDIO) += rapidio/
drivers/gpio/Kconfig +2 -2
@@ -2,7 +2,7 @@ # GPIO infrastructure and expanders # -config GPIO_LIB +config HAVE_GPIO_LIB bool help Platforms select gpiolib if they use this infrastructure
@@ -10,7 +10,7 @@ config GPIO_LIB into SOC processors. menu "GPIO Support" - depends on GPIO_LIB + depends on HAVE_GPIO_LIB config DEBUG_GPIO bool "Debug GPIO calls"
drivers/gpio/Makefile +4 -6
@@ -1,12 +1,10 @@ # gpio support: dedicated expander chips, etc +ccflags-$(CONGIF_DEBUG_GPIO) += -DDEBUG + +obj-$(CONFIG_HAVE_GPIO_LIB) += gpiolib.o + obj-$(CONFIG_GPIO_MCP23S08) += mcp23s08.o obj-$(CONFIG_GPIO_PCA9539) += pca9539.o obj-$(CONFIG_GPIO_PCF857X) += pcf857x.o -ifeq ($(CONFIG_DEBUG_GPIO),y) -EXTRA_CFLAGS += -DDEBUG -endif - -obj-$(CONFIG_GPIO_LIB) += gpiolib.o -
include/asm-generic/gpio.h +1 -1
@@ -1,7 +1,7 @@#ifndef _ASM_GENERIC_GPIO_H#define _ASM_GENERIC_GPIO_H -#ifdef CONFIG_GPIO_LIB +#ifdef CONFIG_HAVE_GPIO_LIB /* Platforms may implement their GPIO interface with library code, * at a small performance cost for non-inlined operations and some
Re: [patch 2.6.24-rc6-mm 1/9] gpiolib: add drivers/gpio directory
Quoted 7 lines · David Brownell
Quoted 4 lines · Sam Ravnborg
kconfig symbols that are "select" targets should be named "
HAVE_" so in this case you could useHAVE_GPIO_LIB.
This is by convention only but introduced to make it visible that this is a config symbol supposed to be selected.Not widely adopted yet, but of course such things take time.
Yes - which is why I am a bit more careful to note it.
Quoted 8 lines
Quoted 5 lines
Then on top of that the
HAVE_symbols should not have any dependencies so we avoid that "select" selects a symbol where the dependencies are not fulfilled.That limitation of "select" still seems more buglike to me than anything else, FWIW. Not that it matters in this case.
But I had not yet figured out how to fix it - and limiting select to symbols without dependencies avoid the worst offendes/abusers.
Quoted 4 lines
Pretty much like the folowing, then?
- Use "ccflags-$(CONGIF_DEBUG_GPIO) += -DDEBUG" instead of those ugly (but traditional) conditionals.
s/CONGIF/CONFIG/
Review
Quoted 3 lines
(quoted diff) +2
-0+ccflags-$(CONGIF_DEBUG_GPIO) += -DDEBUG +
s/CONGIF/CONFIG/
Otherwise OK.
Sam
Re: [patch 2.6.24-rc6-mm 1/9] gpiolib: add drivers/gpio directory
In reply to David BrownellFrom: David Brownell <[email protected]>
Add an empty drivers/gpio directory for gpiolib infrastructure and GPIO expanders. It will be populated by later patches.
This won't be the only place to hold such gpio_chip code. Many external chips add a few GPIOs as secondary functionality (such as MFD drivers) and platform code frequently needs to closely integrate GPIO and IRQ support.
This is placed *early* in the build/link sequence since it's common for other drivers to depend on GPIOs to do their work, so they must be initialized early in the device_initcall() sequence.
---
Incorporates cleanup as suggested by Sam Ravnborg: use HAVE_* and a new idiom for adding "-DDEBUG".
arch/arm/Kconfig | 2 ++ drivers/Kconfig | 2 ++ drivers/Makefile | 1 + drivers/gpio/Kconfig | 32 ++++++++++++++++++++++++++++++++ drivers/gpio/Makefile | 4 ++++ 5 files changed, 41 insertions(+)
arch/arm/Kconfig +2 -0
@@ -1040,6 +1040,8 @@ source "drivers/i2c/Kconfig" source "drivers/spi/Kconfig" +source "drivers/gpio/Kconfig" + source "drivers/w1/Kconfig" source "drivers/power/Kconfig"
drivers/Kconfig +2 -0
@@ -52,6 +52,8 @@ source "drivers/i2c/Kconfig" source "drivers/spi/Kconfig" +source "drivers/gpio/Kconfig" + source "drivers/w1/Kconfig" source "drivers/power/Kconfig"
drivers/Makefile +1 -0
@@ -5,6 +5,7 @@ # Rewritten to use lists instead of if-statements. # +obj-$(CONFIG_HAVE_GPIO_LIB) += gpio/ obj-$(CONFIG_PCI) += pci/ obj-$(CONFIG_PARISC) += parisc/ obj-$(CONFIG_RAPIDIO) += rapidio/
drivers/gpio/Kconfig new file +32 -0
@@ -0,0 +1,32 @@ +# +# GPIO infrastructure and expanders +# + +config HAVE_GPIO_LIB + bool + help + Platforms select gpiolib if they use this infrastructure + for all their GPIOs, usually starting with ones integrated + into SOC processors. + +menu "GPIO Support" + depends on HAVE_GPIO_LIB + +config DEBUG_GPIO + bool "Debug GPIO calls" + depends on DEBUG_KERNEL + help + Say Y here to add some extra checks and diagnostics to GPIO calls. + The checks help ensure that GPIOs have been properly initialized + before they are used and that sleeping calls aren not made from + nonsleeping contexts. They can make bitbanged serial protocols + slower. The diagnostics help catch the type of setup errors + that are most common when setting up new platforms or boards. + +# put expanders in the right section, in alphabetical order + +comment "I2C GPIO expanders:" + +comment "SPI GPIO expanders:" + +endmenu
drivers/gpio/Makefile new file +4 -0
@@ -0,0 +1,4 @@ +# gpio support: dedicated expander chips, etc + +ccflags-$(CONFIG_DEBUG_GPIO) += -DDEBUG +
PATCH 2/9 gpiolib: add gpio provider infrastructure
In reply to a message not in the archiveFrom: David Brownell <[email protected]>
Provide new implementation infrastructure that platforms may choose to use to support the GPIO programming interface. Platforms can update current GPIO support to use this. In many cases the incremental cost to access a non-inlined GPIO should be less than a dozen instructions, with the memory cost being about a page (total) of extra data and code. The upside is:
* Providing two features which were "want to have (but OK to defer)" when GPIO interfaces were first discussed in November 2006:
- A "struct gpio_chip" to plug in GPIOs that aren't directly supported
by SOC platforms, but come from FPGAs or other multifunction devices using conventional device registers (like UCB-1x00 or SM501 GPIOs, and southbridges in PCs with more open specs than usual).
- Full support for message-based GPIO expanders, where registers are
accessed through sleeping I/O calls. Previous support for these "cansleep" calls was just stubs. (One example: the widely used pcf8574 I2C chips, with 8 GPIOs each.)
* Including a non-stub implementation of the gpio_{request,free}() calls, making those calls much more useful. The diagnostic labels are also recorded given DEBUG_FS, so /sys/kernel/debug/gpio can show a snapshot of all GPIOs known to this infrastructure.
The driver programming interfaces introduced in 2.6.21 do not change at all; this infrastructure is entirely below those covers.
--- drivers/gpio/Makefile | 2 drivers/gpio/gpiolib.c | 567 +++++++++++++++++++++++++++++++++++++++++++++ include/asm-generic/gpio.h | 98 +++++++ 3 files changed, 667 insertions(+)
drivers/gpio/Makefile +2 -0
@@ -4,3 +4,5 @@ ifeq ($(CONFIG_DEBUG_GPIO),y) EXTRA_CFLAGS += -DDEBUG endif +obj-$(CONFIG_GPIO_LIB) += gpiolib.o +
drivers/gpio/gpiolib.c new file +567 -0
567 lines not on this page · raw message
include/asm-generic/gpio.h +98 -0
108 lines not on this page · raw message
Re: [patch 2.6.24-rc6-mm 2/9] gpiolib: add gpio provider infrastructure
Hi David.
Quoted 4 lines
drivers/gpio/gpiolib.c new file +4
-0+/* Warn when drivers omit gpio_request() calls -- legal but ill-advised +* when setting direction, and otherwise illegal. Until board setup code +* and drivers use explicit requests everywhere (which won't happen when +* those calls have no teeth) we can't avoid autorequesting. This nag
drivers/gpio/gpiolib.c line 67
Quoted 10 lines
drivers/gpio/gpiolib.c new file +10
-0+* message should motivate switching to explicit requests... +*/ +static void gpio_ensure_requested (struct gpio_desc *desc) +{ +if (test_and_set_bit (FLAG_REQUESTED , &desc->flags) ==0 ) { +pr_warning ("GPIO-%d autorequested\n" , (int )(desc - gpio_desc)); +#ifdef CONFIG_DEBUG_FS + desc->label ="[auto]" ; +#endif + }
For this an the other setters of desc->label a small helper function would be better. The helper function could then contain the necessary ifdef in only one place.
include/asm-generic/gpio.h line 72
Quoted 9 lines
include/asm-generic/gpio.h +7
-0+ +extern const char *gpiochip_is_requested (struct gpio_chip *chip, +unsigned offset); + +/* add/remove chips */ +extern int gpiochip_add (struct gpio_chip *chip); +extern int __must_check gpiochip_remove (struct gpio_chip *chip);
The use of "extern" is not needed in .h files for function prototypes.
Sam
Re: [patch 2.6.24-rc6-mm 2/9] gpiolib: add gpio provider infrastructure
Quoted 3 lines · Sam Ravnborg
For this an the other setters of
desc->labela small helper function would be better. The helper function could then contain the necessary ifdef in only one place.
Good point; see the appended.
Quoted 5 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+/* add/remove chips */ +extern int gpiochip_add(struct gpio_chip *chip); +extern int __must_check gpiochip_remove(struct gpio_chip *chip);The use of "extern" is not needed in .h files for function prototypes.
But it's widely used nonetheless, and many people prefer to see those annotations. Heck, /usr/include is full of them...
- Dave
============ CUT HERE
Minor gpiolib.c cleanup: remove most #ifdefs for CONFIG_FS.
--- drivers/gpio/gpiolib.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
drivers/gpio/gpiolib.c +11 -11
@@ -50,6 +50,12 @@ struct gpio_desc { };static struct gpio_desc gpio_desc[ARCH_NR_GPIOS ]; +static inline void desc_set_label (struct gpio_desc *d,const char *label) +{ +#ifdef CONFIG_DEBUG_FS + d->label = label; +#endif +}/* Warn when drivers omit gpio_request() calls -- legal but ill-advised * when setting direction, and otherwise illegal. Until board setup code
@@ -61,9 +67,7 @@ static void gpio_ensure_requested(struct {if (test_and_set_bit (FLAG_REQUESTED , &desc->flags) ==0 ) {pr_warning ("GPIO-%d autorequested\n" , (int )(desc - gpio_desc)); -#ifdef CONFIG_DEBUG_FS - desc->label ="[auto]" ; -#endif +desc_set_label (desc,"[auto]" ); } }
@@ -178,9 +182,7 @@ int gpio_request(unsigned gpio, const ch*/ if (test_and_set_bit (FLAG_REQUESTED , &desc->flags) ==0 ) { -#ifdef CONFIG_DEBUG_FS - desc->label = (label ==NULL ) ?"?" : label; -#endif +desc_set_label (desc, label ? :"?" ); status =0 ; }else status = -EBUSY ;
@@ -207,11 +209,9 @@ void gpio_free(unsigned gpio)spin_lock_irqsave (&gpio_lock, flags); desc = &gpio_desc[gpio]; -if (desc->chip &&test_and_clear_bit (FLAG_REQUESTED , &desc->flags)) { -#ifdef CONFIG_DEBUG_FS - desc->label =NULL ; -#endif - }else +if (desc->chip &&test_and_clear_bit (FLAG_REQUESTED , &desc->flags)) +desc_set_label (desc,NULL ); +else WARN_ON (extra_checks);spin_unlock_irqrestore (&gpio_lock, flags);
Re: [patch 2.6.24-rc6-mm 2/9] gpiolib: add gpio provider infrastructure
In reply to David BrownellFrom: David Brownell <[email protected]>
Provide new implementation infrastructure that platforms may choose to use when implementing the GPIO programming interface. Platforms can update their GPIO support to use this. In many cases the incremental cost to access a non-inlined GPIO should be less than a dozen instructions, with the memory cost being about a page (total) of extra data and code. The upside is:
* Providing two features which were "want to have (but OK to defer)" when GPIO interfaces were first discussed in November 2006:
- A "struct gpio_chip" to plug in GPIOs that aren't directly supported
by SOC platforms, but come from FPGAs or other multifunction devices using conventional device registers (like UCB-1x00 or SM501 GPIOs, and southbridges in PCs with more open specs than usual).
- Full support for message-based GPIO expanders, where registers are
accessed through sleeping I/O calls. Previous support for these "cansleep" calls was just stubs. (One example: the widely used pcf8574 I2C chips, with 8 GPIOs each.)
* Including a non-stub implementation of the gpio_{request,free}() calls, making those calls much more useful. The diagnostic labels are also recorded given DEBUG_FS, so /sys/kernel/debug/gpio can show a snapshot of all GPIOs known to this infrastructure.
The driver programming interfaces introduced in 2.6.21 do not change at all; this infrastructure is entirely below those covers.
---
Incorporates cleanup as suggested by Sam Ravnborg.
drivers/gpio/Makefile | 2 drivers/gpio/gpiolib.c | 567 +++++++++++++++++++++++++++++++++++++++++++++ include/asm-generic/gpio.h | 98 +++++++ 3 files changed, 667 insertions(+)
drivers/gpio/Makefile +2 -0
@@ -2,3 +2,5 @@ ccflags-$(CONFIG_DEBUG_GPIO) += -DDEBUG +obj-$(CONFIG_HAVE_GPIO_LIB) += gpiolib.o +
drivers/gpio/gpiolib.c new file +567 -0
567 lines not on this page · raw message
include/asm-generic/gpio.h +98 -0
108 lines not on this page · raw message
PATCH 3/9 gpiolib: update Documentation/gpio.txt
In reply to a message not in the archiveFrom: David Brownell <[email protected]>
Update Documentation/gpio.txt, primarily to include the new optional "gpiolib" implementation infrastructure.
--- Documentation/gpio.txt | 133 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 121 insertions(+), 12 deletions(-)
Documentation/gpio.txt +121 -12
@@ -32,7 +32,7 @@ The exact capabilities of GPIOs vary bet - Input values are likewise readable (1, 0). Some chips support readback of pins configured as "output", which is very useful in such "wire-OR" cases (to support bidirectional signaling). GPIO controllers may have - input de-glitch logic, sometimes with software controls. + input de-glitch/debounce logic, sometimes with software controls. - Inputs can often be used as IRQ signals, often edge triggered but sometimes level triggered. Such IRQs may be configurable as system
@@ -60,10 +60,13 @@ used on a board that's wired differently functionality can be very portable. Other features are platform-specific, and that can be critical for glue logic. -Plus, this doesn't define an implementation framework, just an interface. +Plus, this doesn't require any implementation framework, just an interface. One platform might implement it as simple inline functions accessing chip registers; another might implement it by delegating through abstractions -used for several very different kinds of GPIO controller. +used for several very different kinds of GPIO controller. (There is some +optional code supporting such an implementation strategy, described later +in this document, but drivers acting as clients to the GPIO interface must +not care how it's implemented.) That said, if the convention is supported on their platform, drivers should use it when possible. Platforms should declare GENERIC_GPIO support in
@@ -121,6 +124,11 @@ before tasking is enabled, as part of ea For output GPIOs, the value provided becomes the initial output value. This helps avoid signal glitching during system startup. +For compatibility with legacy interfaces to GPIOs, setting the direction +of a GPIO implicitly requests that GPIO (see below) if it has not been +requested already. That compatibility may be removed in the future; +explicitly requesting GPIOs is strongly preferred. + Setting the direction can fail if the GPIO number is invalid, or when that particular GPIO can't be used in that mode. It's generally a bad idea to rely on boot firmware to have set the direction correctly, since
@@ -133,6 +141,7 @@ Spinlock-Safe GPIO access ------------------------- Most GPIO controllers can be accessed with memory read/write instructions. That doesn't need to sleep, and can safely be done from inside IRQ handlers. +(That includes hardirq contexts on RT kernels.) Use these calls to access such GPIOs:
@@ -145,7 +154,7 @@ Use these calls to access such GPIOs: The values are boolean, zero for low, nonzero for high. When reading the value of an output pin, the value returned should be what's seen on the pin ... that won't always match the specified output value, because of -issues including wire-OR and output latencies. +issues including open-drain signaling and output latencies. The get/set calls have no error returns because "invalid GPIO" should have been reported earlier from gpio_direction_*(). However, note that not all
@@ -170,7 +179,8 @@ get to the head of a queue to transmit a This requires sleeping, which can't be done from inside IRQ handlers. Platforms that support this type of GPIO distinguish them from other GPIOs -by returning nonzero from this call: +by returning nonzero from this call (which requires a valid GPIO number, +either explicitly or implicitly requested): int gpio_cansleep(unsigned gpio);
@@ -209,8 +219,11 @@ before tasking is enabled, as part of ea These calls serve two basic purposes. One is marking the signals which are actually in use as GPIOs, for better diagnostics; systems may have several hundred potential GPIOs, but often only a dozen are used on any -given board. Another is to catch conflicts between drivers, reporting -errors when drivers wrongly think they have exclusive use of that signal. +given board. Another is to catch conflicts, identifying errors when +(a) two or more drivers wrongly think they have exclusive use of that +signal, or (b) something wrongly believes it's safe to remove drivers +needed to manage a signal that's in active use. That is, requesting a +GPIO can serve as a kind of lock. These two calls are optional because not not all current Linux platforms offer such functionality in their GPIO support; a valid implementation
@@ -223,6 +236,9 @@ Note that requesting a GPIO does NOT cau way; it just marks that GPIO as in use. Separate code must handle any pin setup (e.g. controlling which pin the GPIO uses, pullup/pulldown). +Also note that it's your responsibility to have stopped using a GPIO +before you free it. + GPIOs mapped to IRQs --------------------
@@ -238,7 +254,7 @@ map between them using calls like: Those return either the corresponding number in the other namespace, or else a negative errno code if the mapping can't be done. (For example, -some GPIOs can't used as IRQs.) It is an unchecked error to use a GPIO +some GPIOs can't be used as IRQs.) It is an unchecked error to use a GPIO number that wasn't set up as an input using gpio_direction_input(), or to use an IRQ number that didn't originally come from gpio_to_irq().
@@ -299,17 +315,110 @@ Related to multiplexing is configuration pulldowns integrated on some platforms. Not all platforms support them, or support them in the same way; and any given board might use external pullups (or pulldowns) so that the on-chip ones should not be used. +(When a circuit needs 5 kOhm, on-chip 100 kOhm resistors won't do.) There are other system-specific mechanisms that are not specified here, like the aforementioned options for input de-glitching and wire-OR output. Hardware may support reading or writing GPIOs in gangs, but that's usually configuration dependent: for GPIOs sharing the same bank. (GPIOs are commonly grouped in banks of 16 or 32, with a given SOC having several such -banks.) Some systems can trigger IRQs from output GPIOs. Code relying on -such mechanisms will necessarily be nonportable. +banks.) Some systems can trigger IRQs from output GPIOs, or read values +from pins not managed as GPIOs. Code relying on such mechanisms will +necessarily be nonportable. -Dynamic definition of GPIOs is not currently supported; for example, as +Dynamic definition of GPIOs is not currently standard; for example, as a side effect of configuring an add-on board with some GPIO expanders. These calls are purely for kernel space, but a userspace API could be built -on top of it. +on top of them. + + +GPIO implementor's framework (OPTIONAL) +======================================= +As noted earlier, there is an optional implementation framework making it +easier for platforms to support different kinds of GPIO controller using +the same programming interface. + +As a debugging aid, if debugfs is available a /sys/kernel/debug/gpio file +will be found there. That will list all the controllers registered through +this framework, and the state of the GPIOs currently in use. + + +Controller Drivers: gpio_chip +----------------------------- +In this framework each GPIO controller is packaged as a "struct gpio_chip" +with information common to each controller of that type: + + - methods to establish GPIO direction + - methods used to access GPIO values + - flag saying whether calls to its methods may sleep + - optional debugfs dump method (showing extra state like pullup config) + - label for diagnostics + +There is also per-instance data, which may come from device.platform_data: +the number of its first GPIO, and how many GPIOs it exposes. + +The code implementing a gpio_chip should support multiple instances of the +controller, possibly using the driver model. That code will configure each +gpio_chip and issue gpiochip_add(). Removing a GPIO controller should be +rare; use gpiochip_remove() when it is unavoidable. + +Most often a gpio_chip is part of an instance-specific structure with state +not exposed by the GPIO interfaces, such as addressing, power management, +and more. Chips such as codecs will have complex non-GPIO state, + +Any debugfs dump method should normally ignore signals which haven't been +requested as GPIOs. They can use gpiochip_is_requested(), which returns +either NULL or the label associated with that GPIO when it was requested. + + +Platform Support +---------------- +To support this framework, a platform's Kconfig will "select GPIO_LIB" and +arrange that its <asm/gpio.h> then include <asm-generic/gpio.h> and define +three functions: gpio_get_value(), gpio_set_value(), and gpio_cansleep(). +They may also want to provide a custom value for ARCH_NR_GPIOS. + +Trivial implementations of those functions can directly use framework +code, which always dispatches through the gpio_chip: + + #define gpio_get_value __gpio_get_value + #define gpio_set_value __gpio_set_value + #define gpio_cansleep __gpio_cansleep + +Fancier implementations could instead define those as inline functions with +logic optimizing access to specific SOC-based GPIOs. For example, if the +referenced GPIO is the constant "12", getting or setting its value could +cost as little as two or three instructions, never sleeping. When such an +optimization is not possible those calls must delegate to the framework +code, costing at least a few dozen instructions. For bitbanged I/O, such +instruction savings can be significant. + +For SOCs, platform-specific code defines and registers gpio_chip instances +for each bank of on-chip GPIOs. Those GPIOs should be numbered/labeled to +match chip vendor documentation, and directly match board schematics. They +may well start at zero and go up to a platform-specific limit. Such GPIOs +are normally integrated into platform initialization to make them always be +available, from arch_initcall() or earlier; they can often serve as IRQs. + + +Board Support +------------- +For external GPIO controllers -- such as I2C or SPI expanders, ASICs, multi +function devices, FPGAs or CPLDs -- most often board-specific code handles +registering controller devices and ensures that their drivers know what GPIO +numbers to use with gpiochip_add(). Their numbers often start right after +platform-specific GPIOs. + +For example, board setup code could create structures identifying the range +of GPIOs that chip will expose, and passes them to each GPIO expander chip +using platform_data. Then the chip driver's probe() routine could pass that +data to gpiochip_add(). + +Initialization order can be important. For example, when a device relies on +an I2C-based GPIO, its probe() routine should only be called after that GPIO +becomes available. That may mean the device should not be registered until +calls for that GPIO can work. One way to address such dependencies is for +such gpio_chip controllers to provide setup() and teardown() callbacks to +board specific code; those board specific callbacks would register devices +once all the necessary resources are available.
Re: [patch 2.6.24-rc6-mm 3/9] gpiolib: update Documentation/gpio.txt
From: David Brownell <[email protected]>
Update Documentation/gpio.txt, primarily to include the new "gpiolib" infrastructure.
---
Incorporates cleanup as suggested by Sam Ravnborg.
Documentation/gpio.txt | 133 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 121 insertions(+), 12 deletions(-)
Documentation/gpio.txt +121 -12
@@ -32,7 +32,7 @@ The exact capabilities of GPIOs vary bet - Input values are likewise readable (1, 0). Some chips support readback of pins configured as "output", which is very useful in such "wire-OR" cases (to support bidirectional signaling). GPIO controllers may have - input de-glitch logic, sometimes with software controls. + input de-glitch/debounce logic, sometimes with software controls. - Inputs can often be used as IRQ signals, often edge triggered but sometimes level triggered. Such IRQs may be configurable as system
@@ -60,10 +60,13 @@ used on a board that's wired differently functionality can be very portable. Other features are platform-specific, and that can be critical for glue logic. -Plus, this doesn't define an implementation framework, just an interface. +Plus, this doesn't require any implementation framework, just an interface. One platform might implement it as simple inline functions accessing chip registers; another might implement it by delegating through abstractions -used for several very different kinds of GPIO controller. +used for several very different kinds of GPIO controller. (There is some +optional code supporting such an implementation strategy, described later +in this document, but drivers acting as clients to the GPIO interface must +not care how it's implemented.) That said, if the convention is supported on their platform, drivers should use it when possible. Platforms should declare GENERIC_GPIO support in
@@ -121,6 +124,11 @@ before tasking is enabled, as part of ea For output GPIOs, the value provided becomes the initial output value. This helps avoid signal glitching during system startup. +For compatibility with legacy interfaces to GPIOs, setting the direction +of a GPIO implicitly requests that GPIO (see below) if it has not been +requested already. That compatibility may be removed in the future; +explicitly requesting GPIOs is strongly preferred. + Setting the direction can fail if the GPIO number is invalid, or when that particular GPIO can't be used in that mode. It's generally a bad idea to rely on boot firmware to have set the direction correctly, since
@@ -133,6 +141,7 @@ Spinlock-Safe GPIO access ------------------------- Most GPIO controllers can be accessed with memory read/write instructions. That doesn't need to sleep, and can safely be done from inside IRQ handlers. +(That includes hardirq contexts on RT kernels.) Use these calls to access such GPIOs:
@@ -145,7 +154,7 @@ Use these calls to access such GPIOs: The values are boolean, zero for low, nonzero for high. When reading the value of an output pin, the value returned should be what's seen on the pin ... that won't always match the specified output value, because of -issues including wire-OR and output latencies. +issues including open-drain signaling and output latencies. The get/set calls have no error returns because "invalid GPIO" should have been reported earlier from gpio_direction_*(). However, note that not all
@@ -170,7 +179,8 @@ get to the head of a queue to transmit a This requires sleeping, which can't be done from inside IRQ handlers. Platforms that support this type of GPIO distinguish them from other GPIOs -by returning nonzero from this call: +by returning nonzero from this call (which requires a valid GPIO number, +either explicitly or implicitly requested): int gpio_cansleep(unsigned gpio);
@@ -209,8 +219,11 @@ before tasking is enabled, as part of ea These calls serve two basic purposes. One is marking the signals which are actually in use as GPIOs, for better diagnostics; systems may have several hundred potential GPIOs, but often only a dozen are used on any -given board. Another is to catch conflicts between drivers, reporting -errors when drivers wrongly think they have exclusive use of that signal. +given board. Another is to catch conflicts, identifying errors when +(a) two or more drivers wrongly think they have exclusive use of that +signal, or (b) something wrongly believes it's safe to remove drivers +needed to manage a signal that's in active use. That is, requesting a +GPIO can serve as a kind of lock. These two calls are optional because not not all current Linux platforms offer such functionality in their GPIO support; a valid implementation
@@ -223,6 +236,9 @@ Note that requesting a GPIO does NOT cau way; it just marks that GPIO as in use. Separate code must handle any pin setup (e.g. controlling which pin the GPIO uses, pullup/pulldown). +Also note that it's your responsibility to have stopped using a GPIO +before you free it. + GPIOs mapped to IRQs --------------------
@@ -238,7 +254,7 @@ map between them using calls like: Those return either the corresponding number in the other namespace, or else a negative errno code if the mapping can't be done. (For example, -some GPIOs can't used as IRQs.) It is an unchecked error to use a GPIO +some GPIOs can't be used as IRQs.) It is an unchecked error to use a GPIO number that wasn't set up as an input using gpio_direction_input(), or to use an IRQ number that didn't originally come from gpio_to_irq().
@@ -299,17 +315,110 @@ Related to multiplexing is configuration pulldowns integrated on some platforms. Not all platforms support them, or support them in the same way; and any given board might use external pullups (or pulldowns) so that the on-chip ones should not be used. +(When a circuit needs 5 kOhm, on-chip 100 kOhm resistors won't do.) There are other system-specific mechanisms that are not specified here, like the aforementioned options for input de-glitching and wire-OR output. Hardware may support reading or writing GPIOs in gangs, but that's usually configuration dependent: for GPIOs sharing the same bank. (GPIOs are commonly grouped in banks of 16 or 32, with a given SOC having several such -banks.) Some systems can trigger IRQs from output GPIOs. Code relying on -such mechanisms will necessarily be nonportable. +banks.) Some systems can trigger IRQs from output GPIOs, or read values +from pins not managed as GPIOs. Code relying on such mechanisms will +necessarily be nonportable. -Dynamic definition of GPIOs is not currently supported; for example, as +Dynamic definition of GPIOs is not currently standard; for example, as a side effect of configuring an add-on board with some GPIO expanders. These calls are purely for kernel space, but a userspace API could be built -on top of it. +on top of them. + + +GPIO implementor's framework (OPTIONAL) +======================================= +As noted earlier, there is an optional implementation framework making it +easier for platforms to support different kinds of GPIO controller using +the same programming interface. + +As a debugging aid, if debugfs is available a /sys/kernel/debug/gpio file +will be found there. That will list all the controllers registered through +this framework, and the state of the GPIOs currently in use. + + +Controller Drivers: gpio_chip +----------------------------- +In this framework each GPIO controller is packaged as a "struct gpio_chip" +with information common to each controller of that type: + + - methods to establish GPIO direction + - methods used to access GPIO values + - flag saying whether calls to its methods may sleep + - optional debugfs dump method (showing extra state like pullup config) + - label for diagnostics + +There is also per-instance data, which may come from device.platform_data: +the number of its first GPIO, and how many GPIOs it exposes. + +The code implementing a gpio_chip should support multiple instances of the +controller, possibly using the driver model. That code will configure each +gpio_chip and issue gpiochip_add(). Removing a GPIO controller should be +rare; use gpiochip_remove() when it is unavoidable. + +Most often a gpio_chip is part of an instance-specific structure with state +not exposed by the GPIO interfaces, such as addressing, power management, +and more. Chips such as codecs will have complex non-GPIO state, + +Any debugfs dump method should normally ignore signals which haven't been +requested as GPIOs. They can use gpiochip_is_requested(), which returns +either NULL or the label associated with that GPIO when it was requested. + + +Platform Support +---------------- +To support this framework, a platform's Kconfig will "select HAVE_GPIO_LIB" +and arrange that its <asm/gpio.h> includes <asm-generic/gpio.h> and defines +three functions: gpio_get_value(), gpio_set_value(), and gpio_cansleep(). +They may also want to provide a custom value for ARCH_NR_GPIOS. + +Trivial implementations of those functions can directly use framework +code, which always dispatches through the gpio_chip: + + #define gpio_get_value __gpio_get_value + #define gpio_set_value __gpio_set_value + #define gpio_cansleep __gpio_cansleep + +Fancier implementations could instead define those as inline functions with +logic optimizing access to specific SOC-based GPIOs. For example, if the +referenced GPIO is the constant "12", getting or setting its value could +cost as little as two or three instructions, never sleeping. When such an +optimization is not possible those calls must delegate to the framework +code, costing at least a few dozen instructions. For bitbanged I/O, such +instruction savings can be significant. + +For SOCs, platform-specific code defines and registers gpio_chip instances +for each bank of on-chip GPIOs. Those GPIOs should be numbered/labeled to +match chip vendor documentation, and directly match board schematics. They +may well start at zero and go up to a platform-specific limit. Such GPIOs +are normally integrated into platform initialization to make them always be +available, from arch_initcall() or earlier; they can often serve as IRQs. + + +Board Support +------------- +For external GPIO controllers -- such as I2C or SPI expanders, ASICs, multi +function devices, FPGAs or CPLDs -- most often board-specific code handles +registering controller devices and ensures that their drivers know what GPIO +numbers to use with gpiochip_add(). Their numbers often start right after +platform-specific GPIOs. + +For example, board setup code could create structures identifying the range +of GPIOs that chip will expose, and passes them to each GPIO expander chip +using platform_data. Then the chip driver's probe() routine could pass that +data to gpiochip_add(). + +Initialization order can be important. For example, when a device relies on +an I2C-based GPIO, its probe() routine should only be called after that GPIO +becomes available. That may mean the device should not be registered until +calls for that GPIO can work. One way to address such dependencies is for +such gpio_chip controllers to provide setup() and teardown() callbacks to +board specific code; those board specific callbacks would register devices +once all the necessary resources are available.
PATCH 4/9 gpiolib: avr32 at32ap platform support
In reply to a message not in the archiveFrom: David Brownell <[email protected]>
Teach AVR32 to use the "GPIO Library" when exposing its GPIOs, so that signals on external chips (like GPIO expanders) can easily be used.
This mostly just reorganizes some existing logic, with two minor changes in behavior:
- The PSR registers are used instead of the previous "gpio_mask" values, matching AT91 behavior and removing some duplication between that role and that of "pinmux_mask".
- NR_IRQs grew to acommodate a bank of external GPIOs. Eventually this number should probably become a board-specific config option.
There's a debugfs dump of status for the built-in GPIOs, showing which pins have deglitching, pullups, or open drain drive enabled, as well as the ID string used when requesting each IRQ.
--- arch/avr32/Kconfig | 1 arch/avr32/mach-at32ap/pio.c | 172 +++++++++++++++-------------- arch/avr32/mach-at32ap/pio.h | 2 include/asm-avr32/arch-at32ap/at32ap7000.h | 2 include/asm-avr32/arch-at32ap/gpio.h | 36 ++++-- include/asm-avr32/arch-at32ap/irq.h | 4 6 files changed, 122 insertions(+), 95 deletions(-)
arch/avr32/Kconfig +1 -0
@@ -80,6 +80,7 @@ config PLATFORM_AT32AP select SUBARCH_AVR32B select MMU select PERFORMANCE_COUNTERS + select GPIO_LIB choice prompt "AVR32 CPU type"
arch/avr32/mach-at32ap/pio.c +92 -80
248 lines not on this page · raw message
arch/avr32/mach-at32ap/pio.h +1 -1
@@ -19,7 +19,7 @@#define PIO_OSR 0x0018#define PIO_IFER 0x0020#define PIO_IFDR 0x0024 -#define PIO_ISFR 0x0028 +#define PIO_IFSR 0x0028#define PIO_SODR 0x0030#define PIO_CODR 0x0034#define PIO_ODSR 0x0038
include/asm-avr32/arch-at32ap/at32ap7000.h +0 -2
@@ -13,8 +13,6 @@#define GPIO_PERIPH_A 0#define GPIO_PERIPH_B 1 -#define NR_GPIO_CONTROLLERS 4 -/* * Pin numbers identifying specific GPIO pins on the chip. They can * also be converted to IRQ numbers by passing them through
include/asm-avr32/arch-at32ap/gpio.h +26 -10
@@ -5,20 +5,36 @@#include <asm/irq.h> -/* Arch-neutral GPIO API */ -int __must_check gpio_request (unsigned int gpio,const char *label); -void gpio_free (unsigned int gpio); - -int gpio_direction_input (unsigned int gpio); -int gpio_direction_output (unsigned int gpio,int value); -int gpio_get_value (unsigned int gpio); -void gpio_set_value (unsigned int gpio,int value); +/* Some GPIO chips can manage IRQs; some can't. The exact numbers can +* be changed if needed, but for the moment they're not configurable. +*/ +#define ARCH_NR_GPIOS (NR_GPIO_IRQS + 2 * 32) + + +/* Arch-neutral GPIO API, supporting both "native" and external GPIOs. */ +#include <asm-generic/gpio.h> + +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); +} -#include <asm-generic/gpio.h> /* cansleep wrappers */ static inline int gpio_to_irq (unsigned int gpio) { -return gpio +GPIO_IRQ_BASE ; +if (gpio <NR_GPIO_IRQS ) +return gpio +GPIO_IRQ_BASE ; +return -EINVAL ; }static inline int irq_to_gpio (unsigned int irq)
include/asm-avr32/arch-at32ap/irq.h +2 -2
@@ -3,11 +3,11 @@#define EIM_IRQ_BASE NR_INTERNAL_IRQS#define NR_EIM_IRQS 32 -#define AT32_EXTINT (n) (EIM_IRQ_BASE + (n))#define GPIO_IRQ_BASE (EIM_IRQ_BASE + NR_EIM_IRQS) -#define NR_GPIO_IRQS (5 * 32) +#define NR_GPIO_CTLR (5 /*internal*/ + 1 /*external*/) +#define NR_GPIO_IRQS (NR_GPIO_CTLR * 32)#define NR_IRQS (GPIO_IRQ_BASE + NR_GPIO_IRQS)
Re: [patch 2.6.24-rc6-mm 4/9] gpiolib: avr32 at32ap platform support
From: David Brownell <[email protected]>
Teach AVR32 to use the "GPIO Library" when exposing its GPIOs, so that signals on external chips (like GPIO expanders) can easily be used.
This mostly reorganizes some existing logic, with two minor changes in behavior:
- The PSR registers are used instead of the previous "gpio_mask" values, matching AT91 behavior and removing some duplication between that role and that of "pinmux_mask".
- NR_IRQs grew to acommodate a bank of external GPIOs. Eventually this number should probably become a board-specific config option.
There's a debugfs dump of status for the built-in GPIOs, showing which pins have deglitching, pullups, or open drain drive enabled, as well as the ID string used when requesting each IRQ.
---
Incorporates cleanup as suggested by Sam Ravnborg.
arch/avr32/Kconfig | 1 arch/avr32/mach-at32ap/pio.c | 172 +++++++++++++++-------------- arch/avr32/mach-at32ap/pio.h | 2 include/asm-avr32/arch-at32ap/at32ap7000.h | 2 include/asm-avr32/arch-at32ap/gpio.h | 36 ++++-- include/asm-avr32/arch-at32ap/irq.h | 4 6 files changed, 122 insertions(+), 95 deletions(-)
arch/avr32/Kconfig +1 -0
@@ -80,6 +80,7 @@ config PLATFORM_AT32AP select SUBARCH_AVR32B select MMU select PERFORMANCE_COUNTERS + select HAVE_GPIO_LIB choice prompt "AVR32 CPU type"
arch/avr32/mach-at32ap/pio.c +92 -80
248 lines not on this page · raw message
arch/avr32/mach-at32ap/pio.h +1 -1
@@ -19,7 +19,7 @@#define PIO_OSR 0x0018#define PIO_IFER 0x0020#define PIO_IFDR 0x0024 -#define PIO_ISFR 0x0028 +#define PIO_IFSR 0x0028#define PIO_SODR 0x0030#define PIO_CODR 0x0034#define PIO_ODSR 0x0038
include/asm-avr32/arch-at32ap/at32ap7000.h +0 -2
@@ -13,8 +13,6 @@#define GPIO_PERIPH_A 0#define GPIO_PERIPH_B 1 -#define NR_GPIO_CONTROLLERS 4 -/* * Pin numbers identifying specific GPIO pins on the chip. They can * also be converted to IRQ numbers by passing them through
include/asm-avr32/arch-at32ap/gpio.h +26 -10
@@ -5,20 +5,36 @@#include <asm/irq.h> -/* Arch-neutral GPIO API */ -int __must_check gpio_request (unsigned int gpio,const char *label); -void gpio_free (unsigned int gpio); - -int gpio_direction_input (unsigned int gpio); -int gpio_direction_output (unsigned int gpio,int value); -int gpio_get_value (unsigned int gpio); -void gpio_set_value (unsigned int gpio,int value); +/* Some GPIO chips can manage IRQs; some can't. The exact numbers can +* be changed if needed, but for the moment they're not configurable. +*/ +#define ARCH_NR_GPIOS (NR_GPIO_IRQS + 2 * 32) + + +/* Arch-neutral GPIO API, supporting both "native" and external GPIOs. */ +#include <asm-generic/gpio.h> + +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); +} -#include <asm-generic/gpio.h> /* cansleep wrappers */ static inline int gpio_to_irq (unsigned int gpio) { -return gpio +GPIO_IRQ_BASE ; +if (gpio <NR_GPIO_IRQS ) +return gpio +GPIO_IRQ_BASE ; +return -EINVAL ; }static inline int irq_to_gpio (unsigned int irq)
include/asm-avr32/arch-at32ap/irq.h +2 -2
@@ -3,11 +3,11 @@#define EIM_IRQ_BASE NR_INTERNAL_IRQS#define NR_EIM_IRQS 32 -#define AT32_EXTINT (n) (EIM_IRQ_BASE + (n))#define GPIO_IRQ_BASE (EIM_IRQ_BASE + NR_EIM_IRQS) -#define NR_GPIO_IRQS (5 * 32) +#define NR_GPIO_CTLR (5 /*internal*/ + 1 /*external*/) +#define NR_GPIO_IRQS (NR_GPIO_CTLR * 32)#define NR_IRQS (GPIO_IRQ_BASE + NR_GPIO_IRQS)
Haavard Skinnemoen
Acked-byRe: [patch 2.6.24-rc6-mm 4/9] gpiolib: avr32 at32ap platform support
Quoted 15 lines · On Sat, 5 Jan 2008 12:10:56 -0800 David Brownell <[email protected]> wrote:
From: David Brownell <[email protected]>
Teach AVR32 to use the "GPIO Library" when exposing its GPIOs, so that signals on external chips (like GPIO expanders) can easily be used.
This mostly reorganizes some existing logic, with two minor changes in behavior:
- The PSR registers are used instead of the previous "
gpio_mask"
values, matching AT91 behavior and removing some duplication between that role and that of "pinmux_mask".-
NR_IRQsgrew to acommodate a bank of external GPIOs. Eventually
this number should probably become a board-specific config option.
Quoted 6 lines
There's a debugfs dump of status for the built-in GPIOs, showing which pins have deglitching, pullups, or open drain drive enabled, as well as the ID string used when requesting each IRQ.
Signed-off-by David Brownell <[email protected]>Cc Haavard Skinnemoen <[email protected]>
I'm not going to merge it since the rest of gpiolib isn't in mainline yet.
Haavard
Re: [patch 2.6.24-rc6-mm 4/9] gpiolib: avr32 at32ap platform support
Quoted 11 lines · Haavard Skinnemoen
Quoted 6 lines · On Sat, 5 Jan 2008 12:10:56 -0800 David Brownell <[email protected]> wrote:
From: David Brownell <[email protected]>
Teach AVR32 to use the "GPIO Library" when exposing its GPIOs, so that signals on external chips (like GPIO expanders) can easily be used.
This mostly reorganizes some existing logic ...
Thanks ... an ack is better than that informal "OK". ;)
Quoted 2 lines
I'm not going to merge it since the rest of gpiolib isn't in mainline yet.
Right. That's why the PXA support is going through Andrew too.
PATCH 5/9 gpiolib: pxa platform support
In reply to a message not in the archiveFrom: Philipp Zabel <[email protected]>
This adds gpiolib support for the PXA architecture:
- move all GPIO API functions from generic.c into gpio.c
- convert the gpio_get/set_value macros into inline functions
This makes it easier to hook up GPIOs provided by external chips like ASICs and CPLDs, as used on various product and developer boards.
--- arch/arm/Kconfig | 1 arch/arm/mach-pxa/Makefile | 2 arch/arm/mach-pxa/generic.c | 93 ---------------- arch/arm/mach-pxa/generic.h | 1 arch/arm/mach-pxa/gpio.c | 197 ++++++++++++++++++++++++++++++++++++ arch/arm/mach-pxa/irq.c | 2 include/asm-arm/arch-pxa/gpio.h | 48 +++----- include/asm-arm/arch-pxa/pxa-regs.h | 13 ++ 8 files changed, 235 insertions(+), 122 deletions(-) create mode 100644 arch/arm/mach-pxa/gpio.c
arch/arm/Kconfig +1 -0
@@ -345,6 +345,7 @@ config ARCH_PXA select GENERIC_GPIO select GENERIC_TIME select GENERIC_CLOCKEVENTS + select GPIO_LIB help Support for Intel/Marvell's PXA2xx/PXA3xx processor line.
arch/arm/mach-pxa/Makefile +1 -1
@@ -3,7 +3,7 @@ # # Common support (must be linked before board specific support) -obj-y += clock.o generic.o irq.o dma.o time.o +obj-y += clock.o generic.o gpio.o irq.o dma.o time.o obj-$(CONFIG_PXA25x) += pxa25x.o obj-$(CONFIG_PXA27x) += pxa27x.o obj-$(CONFIG_PXA3xx) += pxa3xx.o mfp.o
arch/arm/mach-pxa/generic.c +0 -93
111 lines not on this page · raw message
arch/arm/mach-pxa/generic.h +1 -0
@@ -16,6 +16,7 @@ extern void __init pxa_init_irq_low(voidextern void __init pxa_init_irq_high (void );extern void __init pxa_init_irq_gpio (int gpio_nr);extern void __init pxa_init_irq_set_wake (int (*set_wake)(unsigned int ,unsigned int )); +extern void __init pxa_init_gpio (int gpio_nr);extern void __init pxa25x_init_irq (void );extern void __init pxa27x_init_irq (void );extern void __init pxa3xx_init_irq (void );
arch/arm/mach-pxa/gpio.c new file +197 -0
197 lines not on this page · raw message
arch/arm/mach-pxa/irq.c +2 -0
@@ -310,6 +310,8 @@ void __init pxa_init_irq_gpio(int gpio_n/* Install handler for GPIO>=2 edge detect interrupts */ set_irq_chip (IRQ_GPIO_2_x, &pxa_internal_chip_low);set_irq_chained_handler (IRQ_GPIO_2_x, pxa_gpio_demux_handler); + +pxa_init_gpio (gpio_nr); }void __init pxa_init_irq_set_wake (int (*set_wake)(unsigned int ,unsigned int ))
include/asm-arm/arch-pxa/gpio.h +20 -28
63 lines not on this page · raw message
include/asm-arm/arch-pxa/pxa-regs.h +13 -0
@@ -1131,6 +1131,19 @@* General Purpose I/O */ +#define GPIO0_BASE ((void __iomem *)io_p2v(0x40E00000)) +#define GPIO1_BASE ((void __iomem *)io_p2v(0x40E00004)) +#define GPIO2_BASE ((void __iomem *)io_p2v(0x40E00008)) +#define GPIO3_BASE ((void __iomem *)io_p2v(0x40E00100)) + +#define GPLR_OFFSET 0x00 +#define GPDR_OFFSET 0x0C +#define GPSR_OFFSET 0x18 +#define GPCR_OFFSET 0x24 +#define GRER_OFFSET 0x30 +#define GFER_OFFSET 0x3C +#define GEDR_OFFSET 0x48 +#define GPLR0 __REG(0x40E00000)/* GPIO Pin-Level Register GPIO<31:0> */ #define GPLR1 __REG(0x40E00004)/* GPIO Pin-Level Register GPIO<63:32> */ #define GPLR2 __REG(0x40E00008)/* GPIO Pin-Level Register GPIO<80:64> */
Re: [patch 2.6.24-rc6-mm 5/9] gpiolib: pxa platform support
Subject: [PATCH] gpiolib support for the PXA architecture
From: Philipp Zabel <[email protected]>
This adds gpiolib support for the PXA architecture:
- move all GPIO API functions from generic.c into gpio.c
- convert the gpio_get/set_value macros into inline functions
This makes it easier to hook up GPIOs provided by external chips like ASICs and CPLDs.
---
Incorporates minor cleanup suggested by Sam Ravnborg.
arch/arm/Kconfig | 1 arch/arm/mach-pxa/Makefile | 2 arch/arm/mach-pxa/generic.c | 93 ---------------- arch/arm/mach-pxa/generic.h | 1 arch/arm/mach-pxa/gpio.c | 197 ++++++++++++++++++++++++++++++++++++ arch/arm/mach-pxa/irq.c | 2 include/asm-arm/arch-pxa/gpio.h | 48 +++----- include/asm-arm/arch-pxa/pxa-regs.h | 13 ++ 8 files changed, 235 insertions(+), 122 deletions(-) create mode 100644 arch/arm/mach-pxa/gpio.c
arch/arm/Kconfig +1 -0
@@ -345,6 +345,7 @@ config ARCH_PXA select GENERIC_GPIO select GENERIC_TIME select GENERIC_CLOCKEVENTS + select HAVE_GPIO_LIB help Support for Intel/Marvell's PXA2xx/PXA3xx processor line.
arch/arm/mach-pxa/Makefile +1 -1
@@ -3,7 +3,7 @@ # # Common support (must be linked before board specific support) -obj-y += clock.o generic.o irq.o dma.o time.o +obj-y += clock.o generic.o gpio.o irq.o dma.o time.o obj-$(CONFIG_PXA25x) += pxa25x.o obj-$(CONFIG_PXA27x) += pxa27x.o obj-$(CONFIG_PXA3xx) += pxa3xx.o mfp.o
arch/arm/mach-pxa/generic.c +0 -93
111 lines not on this page · raw message
arch/arm/mach-pxa/generic.h +1 -0
@@ -16,6 +16,7 @@ extern void __init pxa_init_irq_low(voidextern void __init pxa_init_irq_high (void );extern void __init pxa_init_irq_gpio (int gpio_nr);extern void __init pxa_init_irq_set_wake (int (*set_wake)(unsigned int ,unsigned int )); +extern void __init pxa_init_gpio (int gpio_nr);extern void __init pxa25x_init_irq (void );extern void __init pxa27x_init_irq (void );extern void __init pxa3xx_init_irq (void );
arch/arm/mach-pxa/gpio.c new file +197 -0
197 lines not on this page · raw message
arch/arm/mach-pxa/irq.c +2 -0
@@ -310,6 +310,8 @@ void __init pxa_init_irq_gpio(int gpio_n/* Install handler for GPIO>=2 edge detect interrupts */ set_irq_chip (IRQ_GPIO_2_x, &pxa_internal_chip_low);set_irq_chained_handler (IRQ_GPIO_2_x, pxa_gpio_demux_handler); + +pxa_init_gpio (gpio_nr); }void __init pxa_init_irq_set_wake (int (*set_wake)(unsigned int ,unsigned int ))
include/asm-arm/arch-pxa/gpio.h +20 -28
63 lines not on this page · raw message
include/asm-arm/arch-pxa/pxa-regs.h +13 -0
@@ -1131,6 +1131,19 @@* General Purpose I/O */ +#define GPIO0_BASE ((void __iomem *)io_p2v(0x40E00000)) +#define GPIO1_BASE ((void __iomem *)io_p2v(0x40E00004)) +#define GPIO2_BASE ((void __iomem *)io_p2v(0x40E00008)) +#define GPIO3_BASE ((void __iomem *)io_p2v(0x40E00100)) + +#define GPLR_OFFSET 0x00 +#define GPDR_OFFSET 0x0C +#define GPSR_OFFSET 0x18 +#define GPCR_OFFSET 0x24 +#define GRER_OFFSET 0x30 +#define GFER_OFFSET 0x3C +#define GEDR_OFFSET 0x48 +#define GPLR0 __REG(0x40E00000)/* GPIO Pin-Level Register GPIO<31:0> */ #define GPLR1 __REG(0x40E00004)/* GPIO Pin-Level Register GPIO<63:32> */ #define GPLR2 __REG(0x40E00008)/* GPIO Pin-Level Register GPIO<80:64> */
PATCH 6/9 gpiolib: pcf857x i2c gpio expander support
In reply to a message not in the archiveFrom: David Brownell <[email protected]>
This is a new-style I2C driver for most common 8 and 16 bit I2C based "quasi-bidirectional" GPIO expanders: pcf8574 or pcf8575, and several compatible models (mostly faster, supporting I2C at up to 1 MHz).
The driver exposes the GPIO signals using the platform-neutral GPIO programming interface, so they are easily accessed by other kernel code. The lack of such a flexible kernel API has been a big factor in the proliferation of board-specific drivers for these chips... stuff that rarely makes it upstream since it's so ugly. This driver will let such boards use standard calls.
Since it's a new-style driver, these devices must be configured as part of board-specific init. That eliminates the need for error-prone manual configuration of module parameters, and makes compatibility with legacy drivers (pcf8574.c, pc8575.c) for these chips easier (there's a clear either/or disjunction).
--- drivers/gpio/Kconfig | 23 +++ drivers/gpio/Makefile | 2 drivers/gpio/pcf857x.c | 330 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/i2c/pcf857x.h | 45 ++++++ 4 files changed, 400 insertions(+)
drivers/gpio/Kconfig +23 -0
@@ -27,6 +27,29 @@ config DEBUG_GPIO comment "I2C GPIO expanders:" +config GPIO_PCF857X + tristate "PCF857x, PCA857x, and PCA967x I2C GPIO expanders" + depends on I2C + help + Say yes here to provide access to most "quasi-bidirectional" I2C + GPIO expanders used for additional digital outputs or inputs. + Most of these parts are from NXP, though TI is a second source for + some of them. Compatible models include: + + 8 bits: pcf8574, pcf8574a, pca8574, pca8574a, + pca9670, pca9672, pca9674, pca9674a + + 16 bits: pcf8575, pcf8575c, pca8575, + pca9671, pca9673, pca9675 + + Your board setup code will need to declare the expanders in + use, and assign numbers to the GPIOs they expose. Those GPIOs + can then be used from drivers and other kernel code, just like + other GPIOs, but only accessible from task contexts. + + This driver provides an in-kernel interface to those GPIOs using + platform-neutral GPIO calls. + comment "SPI GPIO expanders:" endmenu
drivers/gpio/Makefile +2 -0
@@ -1,5 +1,7 @@ # gpio support: dedicated expander chips, etc +obj-$(CONFIG_GPIO_PCF857X) += pcf857x.o + ifeq ($(CONFIG_DEBUG_GPIO),y) EXTRA_CFLAGS += -DDEBUG endif
drivers/gpio/pcf857x.c new file +330 -0
330 lines not on this page · raw message
include/linux/i2c/pcf857x.h new file +45 -0
45 lines not on this page · raw message
Re: [patch 2.6.24-rc6-mm 6/9] gpiolib: pcf857x i2c gpio expander support
From: David Brownell <[email protected]>
This is a new-style I2C driver for most common 8 and 16 bit I2C based "quasi-bidirectional" GPIO expanders: pcf8574 or pcf8575, and several compatible models (mostly faster, supporting I2C at up to 1 MHz).
The driver exposes the GPIO signals using the platform-neutral GPIO programming interface, so they are easily accessed by other kernel code. The lack of such a flexible kernel API has been a big factor in the proliferation of board-specific drivers for these chips... stuff that rarely makes it upstream since it's so ugly. This driver will let such boards use standard calls.
Since it's a new-style driver, these devices must be configured as part of board-specific init. That eliminates the need for error-prone manual configuration of module parameters, and makes compatibility with legacy drivers (pcf8574.c, pc8575.c) for these chips easier (there's a clear either/or disjunction).
---
Needed refresh after cleanup as suggested by Sam Ravnborg.
drivers/gpio/Kconfig | 23 +++ drivers/gpio/Makefile | 1 drivers/gpio/pcf857x.c | 330 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/i2c/pcf857x.h | 45 ++++++ 4 files changed, 399 insertions(+)
drivers/gpio/Kconfig +23 -0
@@ -27,6 +27,29 @@ config DEBUG_GPIO comment "I2C GPIO expanders:" +config GPIO_PCF857X + tristate "PCF857x, PCA857x, and PCA967x I2C GPIO expanders" + depends on I2C + help + Say yes here to provide access to most "quasi-bidirectional" I2C + GPIO expanders used for additional digital outputs or inputs. + Most of these parts are from NXP, though TI is a second source for + some of them. Compatible models include: + + 8 bits: pcf8574, pcf8574a, pca8574, pca8574a, + pca9670, pca9672, pca9674, pca9674a + + 16 bits: pcf8575, pcf8575c, pca8575, + pca9671, pca9673, pca9675 + + Your board setup code will need to declare the expanders in + use, and assign numbers to the GPIOs they expose. Those GPIOs + can then be used from drivers and other kernel code, just like + other GPIOs, but only accessible from task contexts. + + This driver provides an in-kernel interface to those GPIOs using + platform-neutral GPIO calls. + comment "SPI GPIO expanders:" endmenu
drivers/gpio/Makefile +1 -0
@@ -4,3 +4,4 @@ ccflags-$(CONFIG_DEBUG_GPIO) += -DDEBUG obj-$(CONFIG_HAVE_GPIO_LIB) += gpiolib.o +obj-$(CONFIG_GPIO_PCF857X) += pcf857x.o
drivers/gpio/pcf857x.c new file +330 -0
330 lines not on this page · raw message
include/linux/i2c/pcf857x.h new file +45 -0
45 lines not on this page · raw message
PATCH 7/9 gpiolib: mcp23s08 spi gpio expander support
In reply to a message not in the archiveFrom: David Brownell <[email protected]>
Basic driver for 8-bit SPI based MCP23S08 GPIO expander, without support for IRQs or the shared chipselect mechanism.
--- drivers/gpio/Kconfig | 7 drivers/gpio/Makefile | 1 drivers/gpio/mcp23s08.c | 357 +++++++++++++++++++++++++++++++++++++++++++ include/linux/spi/mcp23s08.h | 24 ++ 4 files changed, 389 insertions(+)
drivers/gpio/Kconfig +7 -0
@@ -52,4 +52,11 @@ config GPIO_PCF857X comment "SPI GPIO expanders:" +config GPIO_MCP23S08 + tristate "Microchip MCP23S08 I/O expander" + depends on SPI_MASTER + help + SPI driver for Microchip MCP23S08 I/O expander. This provides + a GPIO interface supporting inputs and outputs. + endmenu
drivers/gpio/Makefile +1 -0
@@ -1,5 +1,6 @@ # gpio support: dedicated expander chips, etc +obj-$(CONFIG_GPIO_MCP23S08) += mcp23s08.o obj-$(CONFIG_GPIO_PCF857X) += pcf857x.o ifeq ($(CONFIG_DEBUG_GPIO),y)
drivers/gpio/mcp23s08.c new file +357 -0
357 lines not on this page · raw message
include/linux/spi/mcp23s08.h new file +24 -0
@@ -0,0 +1,24 @@ + +/* FIXME driver should be able to handle all four slaves that +* can be hooked up to each chipselect, as well as IRQs... +*/ + +struct mcp23s08_platform_data { +/* four slaves can share one SPI chipselect */ +u8 slave; + +/* number assigned to the first GPIO */ +unsigned base; + +/* pins with pullups */ +u8 pullups; + +void *context;/* param to setup/teardown */ + +int (*setup)(struct spi_device *spi, +int gpio,unsigned ngpio, +void *context); +int (*teardown)(struct spi_device *spi, +int gpio,unsigned ngpio, +void *context); +};
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
PATCH 8/9 gpiolib: pca9539 i2c gpio expander support
In reply to a message not in the archiveFrom: eric miao <[email protected]>
This adds a new-style I2C driver with basic support for the sixteen bit PCA9539 GPIO expanders. These chips have multiple registers, push-pull output drivers, and (not supported by this patch) pin change interrupts.
Board-specific code must provide "pca9539_platform_data" with each chip's "i2c_board_info". That provides the GPIO numbers to be used by that chip, and callbacks for board-specific setup/teardown logic.
Derived from drivers/i2c/chips/pca9539.c (which has no current known users). This is faster and simpler; it uses 16-bit register access, and caches the OUTPUT and DIRECTION registers for fast access.
--- drivers/gpio/Kconfig | 10 + drivers/gpio/Makefile | 1 drivers/gpio/pca9539.c | 264 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/i2c/pca9539.h | 18 +++ 4 files changed, 293 insertions(+)
drivers/gpio/Kconfig +10 -0
@@ -27,6 +27,16 @@ config DEBUG_GPIO comment "I2C GPIO expanders:" +config GPIO_PCA9539 + tristate "PCA9539 16-bit I/O port" + depends on I2C + help + Say yes here to support the PCA9539 16-bit I/O port. These + parts are made by NXP and TI. + + This driver can also be built as a module. If so, the module + will be called pca9539. + config GPIO_PCF857X tristate "PCF857x, PCA857x, and PCA967x I2C GPIO expanders" depends on I2C
drivers/gpio/Makefile +1 -0
@@ -1,6 +1,7 @@ # gpio support: dedicated expander chips, etc obj-$(CONFIG_GPIO_MCP23S08) += mcp23s08.o +obj-$(CONFIG_GPIO_PCA9539) += pca9539.o obj-$(CONFIG_GPIO_PCF857X) += pcf857x.o ifeq ($(CONFIG_DEBUG_GPIO),y)
drivers/gpio/pca9539.c new file +264 -0
264 lines not on this page · raw message
include/linux/i2c/pca9539.h new file +18 -0
@@ -0,0 +1,18 @@ +/* platform data for the PCA9539 16-bit I/O expander driver */ + +struct pca9539_platform_data { +/* number of the first GPIO */ +unsigned gpio_base; + +/* initial polarity inversion setting */ +uint16_t invert; + +void *context;/* param to setup/teardown */ + +int (*setup)(struct i2c_client *client, +unsigned gpio,unsigned ngpio, +void *context); +int (*teardown)(struct i2c_client *client, +unsigned gpio,unsigned ngpio, +void *context); +};
Re: [patch 2.6.24-rc6-mm 8/9] gpiolib: pca9539 i2c gpio expander support
Hi David, hi Eric,
Quoted 15 lines · David Brownell
From: eric miao <[email protected]>
This adds a new-style I2C driver with basic support for the sixteen bit PCA9539 GPIO expanders. These chips have multiple registers, push-pull output drivers, and (not supported by this patch) pin change interrupts.
Board-specific code must provide "
pca9539_platform_data" with each chip's "i2c_board_info". That provides the GPIO numbers to be used by that chip, and callbacks for board-specific setup/teardown logic.Derived from
drivers/i2c/chips/pca9539.c(which has no current known users). This is faster and simpler; it uses 16-bit register access, and caches the OUTPUT and DIRECTION registers for fast access.
Quoted 8 lines
Signed-off-by eric miao <[email protected]>Signed-off-by David Brownell <[email protected]>---
drivers/gpio/Kconfig | 10 + drivers/gpio/Makefile | 1 drivers/gpio/pca9539.c | 264 ++++++++++++++++++++++++++ include/linux/i2c/pca9539.h | 18 +++ 4 files changed, 293 insertions(+)
Random comments:
Quoted 5 lines
drivers/gpio/pca9539.c new file +5
-0+static int pca9539_gpio_get_value (struct gpio_chip *gc,unsigned off) +{ +struct pca9539_chip *chip; +uint16_t reg_val; +int ret;
drivers/gpio/pca9539.c line 125
Quoted 10 lines
drivers/gpio/pca9539.c new file +10
-0+ + chip = container_of(gc, struct pca9539_chip, gpio_chip); + + ret =pca9539_read_reg (chip,PCA9539_INPUT , ®_val); +if (ret <0 ) { +/* NOTE: diagnostic already omitted; that's the +* best we can do here. +*/ +return 0 ; + }
I guess that you really mean "emitted" here, not "omitted"?
More importantly, I don't agree that it's the best we can do here. Maybe it was already discussed before and there's a good reason to not report errors from "get" functions at the gpio-core level, but I can't see it. Whether a read error should be considered as "0" or "1" (or neither) should be a decision left to the user of the GPIO chip, rather than to each GPIO driver.
drivers/gpio/pca9539.c line 208
Quoted 12 lines
drivers/gpio/pca9539.c new file +11
-0+ +return (reg_val & (1u << off)) ?1 :0 ; +} +static int __devinit pca9539_probe (struct i2c_client *client) +{ + (...) +if (pdata->setup) { + ret = pdata->setup (client, chip->gpio_chip.base, + chip->gpio_chip.ngpio, pdata->context); +if (ret <0 ) +dev_dbg (&client->dev,"setup failed, %d\n" , ret);
Should be at least dev_warn() and maybe even dev_err().
drivers/gpio/pca9539.c line 229
Quoted 12 lines
drivers/gpio/pca9539.c new file +12
-0+ } + (...) +} + +static int pca9539_remove (struct i2c_client *client) +{ + (...) +if (pdata->teardown) { + ret = pdata->teardown (client, chip->gpio_chip.base, + chip->gpio_chip.ngpio, pdata->context); +if (ret <0 ) +dev_dbg (&client->dev,"teardown failed, %d\n" , ret);
Same thing here.
drivers/gpio/pca9539.c line 234
Quoted 5 lines
drivers/gpio/pca9539.c new file +5
-0+ } + + ret =gpiochip_remove (&chip->gpio_chip); +if (ret) { +dev_err (&client->dev,"failed remove gpio_chip\n" );
This error message could certainly be reworded to sound better. Also, for consistency you should include the value of "ret" in the message.
drivers/gpio/pca9539.c line 240
Quoted 6 lines
drivers/gpio/pca9539.c new file +6
-0+return ret; + } + +kfree (chip); +return 0 ; +}
--
Jean Delvare
Re: [patch 2.6.24-rc6-mm 8/9] gpiolib: pca9539 i2c gpio expander support
Quoted 13 lines · Jean Delvare
Hi David, hi Eric,
Quoted 6 lines · David Brownell
From: eric miao <[email protected]>
This adds a new-style I2C driver with basic support for the sixteen bit PCA9539 GPIO expanders.
...Random comments:
Quoted 13 lines
Review
Quoted 11 lines
(quoted diff) +11
-0+static int pca9539_gpio_get_value(struct gpio_chip *gc, unsigned off) +{ + ... + + ret = pca9539_read_reg(chip, PCA9539_INPUT, ®_val); + if (ret < 0) { + /* NOTE: diagnostic already omitted; that's the + * best we can do here. + */ + return 0; + }I guess that you really mean "emitted" here, not "omitted"?
Yeah, typo.
Quoted 3 lines
More importantly, I don't agree that it's the best we can do here. Maybe it was already discussed before and there's a good reason to not report errors from "get" functions at the gpio-core level,
Yes there is. It's by explicit request. Expecting drivers to cope with per-bit errors is at best unrealistic. This was decided well over a year ago ... nobody wants to see bit-banging code that spends more time trying to figure out how to recover from "can't happen" errors than getting real work done. (None of the SOC-specific GPIO interfaces being replaced by this generic one returned errors either.)
That said, with things like I2C there actually *could* be errors; which are impossible with valid parameters to SOC-level GPIOs. That might argue for gpio_{get,set}_value_cansleep() calls being able to return fault codes that would be nonsense on the more widely used gpio_{get,set}_value() alls.
But such a change would be for a different set of patches. This set does not change *any* driver programming interface. At all.
Quoted 10 lines
Review
Quoted 8 lines
(quoted diff) +8
-0+static int __devinit pca9539_probe(struct i2c_client *client) +{ + (...) + if (pdata->setup) { + ret = pdata->setup(client, chip->gpio_chip.base, + chip->gpio_chip.ngpio, pdata->context); + if (ret < 0) + dev_dbg(&client->dev, "setup failed, %d\n", ret);Should be at least
dev_warn()and maybe evendev_err().
It's not treated as an error (i.e. abort the probe); warning is right.
Hmm, I thought both this issue and the previous one had been fixed already ... oh, it was the pcf857x driver that fixed that. Never mind. ;)
Quoted 6 lines
Quoted 6 lines
(quoted diff) +6
-0+ } + (...) +} + +static int pca9539_remove(struct i2c_client *client) +{
Quoted 8 lines
Review
Quoted 6 lines
+ (...)
(quoted diff) +5
-0+ if (pdata->teardown) { + ret = pdata->teardown(client, chip->gpio_chip.base, + chip->gpio_chip.ngpio, pdata->context); + if (ret < 0) + dev_dbg(&client->dev, "teardown failed, %d\n", ret);Same thing here.
That was supposed to be dev_err() then "return ret" !
Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ } + + ret = gpiochip_remove(&chip->gpio_chip); + if (ret) { + dev_err(&client->dev, "failed remove gpio_chip\n");This error message could certainly be reworded to sound better. Also, for consistency you should include the value of "ret" in the message.
Right. So, pretty much like the appended. (Which I'll merge into refreshed version of this patch.)
drivers/gpio/pca9539.c +9 -5
@@ -118,7 +118,7 @@ static int pca9539_gpio_get_value(struct ret =pca9539_read_reg (chip,PCA9539_INPUT , ®_val);if (ret <0 ) { -/* NOTE: diagnostic already omitted; that's the +/* NOTE: diagnostic already emitted; that's the * best we can do here. */ return 0 ;
@@ -205,7 +205,7 @@ static int __devinit pca9539_probe(struc ret = pdata->setup (client, chip->gpio_chip.base, chip->gpio_chip.ngpio, pdata->context);if (ret <0 ) -dev_dbg (&client->dev,"setup failed, %d\n" , ret); +dev_warn (&client->dev,"setup failed, %d\n" , ret); }i2c_set_clientdata (client, chip);
@@ -225,13 +225,17 @@ static int pca9539_remove(struct i2c_cliif (pdata->teardown) { ret = pdata->teardown (client, chip->gpio_chip.base, chip->gpio_chip.ngpio, pdata->context); -if (ret <0 ) -dev_dbg (&client->dev,"teardown failed, %d\n" , ret); +if (ret <0 ) { +dev_err (&client->dev,"%s failed, %d\n" , +"teardown" , ret); +return ret; + } } ret =gpiochip_remove (&chip->gpio_chip);if (ret) { -dev_err (&client->dev,"failed remove gpio_chip\n" ); +dev_err (&client->dev,"%s failed, %d\n" , +"gpiochip_remove()" , ret);return ret; }
Re: [patch 2.6.24-rc6-mm 8/9] gpiolib: pca9539 i2c gpio expander support
In reply to David BrownellFrom: eric miao <[email protected]>
Subject: [PATCH] gpiolib: support PCA9539 GPIO expander
This adds a new-style I2C driver with basic support for the sixteen bit PCA9539 GPIO expanders. These chips have multiple registers, push-pull output drivers, and (not supported in this patch) pin change interrupts.
Board-specific code must provide "pca9539_platform_data" with each chip's "i2c_board_info". That provides the GPIO numbers to be used by that chip, and callbacks for board-specific setup/teardown logic.
Derived from drivers/i2c/chips/pca9539.c (which has no current known users). This is faster and simpler; it uses 16-bit register access, and cache the OUTPUT and DIRECTION registers for fast access
---
Incorporates cleanups noted by Jean Delvare.
drivers/gpio/Kconfig | 10 + drivers/gpio/Makefile | 1 drivers/gpio/pca9539.c | 271 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/i2c/pca9539.h | 18 ++ 4 files changed, 300 insertions(+)
drivers/gpio/Kconfig +10 -0
@@ -27,6 +27,16 @@ config DEBUG_GPIO comment "I2C GPIO expanders:" +config GPIO_PCA9539 + tristate "PCA9539 16-bit I/O port" + depends on I2C + help + Say yes here to support the PCA9539 16-bit I/O port. These + parts are made by NXP and TI. + + This driver can also be built as a module. If so, the module + will be called pca9539. + config GPIO_PCF857X tristate "PCF857x, PCA857x, and PCA967x I2C GPIO expanders" depends on I2C
drivers/gpio/Makefile +1 -0
@@ -1,6 +1,7 @@ # gpio support: dedicated expander chips, etc obj-$(CONFIG_GPIO_MCP23S08) += mcp23s08.o +obj-$(CONFIG_GPIO_PCA9539) += pca9539.o obj-$(CONFIG_GPIO_PCF857X) += pcf857x.o ifeq ($(CONFIG_DEBUG_GPIO),y)
drivers/gpio/pca9539.c new file +271 -0
271 lines not on this page · raw message
include/linux/i2c/pca9539.h new file +18 -0
@@ -0,0 +1,18 @@ +/* platform data for the PCA9539 16-bit I/O expander driver */ + +struct pca9539_platform_data { +/* number of the first GPIO */ +unsigned gpio_base; + +/* initial polarity inversion setting */ +uint16_t invert; + +void *context;/* param to setup/teardown */ + +int (*setup)(struct i2c_client *client, +unsigned gpio,unsigned ngpio, +void *context); +int (*teardown)(struct i2c_client *client, +unsigned gpio,unsigned ngpio, +void *context); +};
Jean Delvare
Acked-byRe: [patch 2.6.24-rc6-mm 8/9] gpiolib: pca9539 i2c gpio expander support
Quoted 18 lines · David Brownell
From: eric miao <[email protected]>
Subject: [PATCH] gpiolib: support PCA9539 GPIO expanderThis adds a new-style I2C driver with basic support for the sixteen bit PCA9539 GPIO expanders. These chips have multiple registers, push-pull output drivers, and (not supported in this patch) pin change interrupts.
Board-specific code must provide "
pca9539_platform_data" with each chip's "i2c_board_info". That provides the GPIO numbers to be used by that chip, and callbacks for board-specific setup/teardown logic.Derived from
drivers/i2c/chips/pca9539.c(which has no current known users). This is faster and simpler; it uses 16-bit register access, and cache the OUTPUT and DIRECTION registers for fast accessSigned-off-by eric miao <[email protected]>Signed-off-by David Brownell <[email protected]>
Quoted 8 lines
---
Incorporates cleanups noted by Jean Delvare.drivers/gpio/Kconfig | 10 + drivers/gpio/Makefile | 1 drivers/gpio/pca9539.c | 271 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/i2c/pca9539.h | 18 ++ 4 files changed, 300 insertions(+)
Signature · 1 line
Jean Delvare
PATCH 9/9 gpiolib: deprecate obsolete pca9539 driver
In reply to a message not in the archiveFrom: eric miao <[email protected]>
Use drivers/gpio/pca9539.c instead.
--- Documentation/i2c/chips/pca9539 | 3 +++ drivers/i2c/chips/Kconfig | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-)
Documentation/i2c/chips/pca9539 +3 -0
@@ -1,6 +1,9 @@ Kernel driver pca9539 ===================== +NOTE: this driver is deprecated and will be dropped soon, use +drivers/gpio/pca9539.c instead. + Supported chips: * Philips PCA9539 Prefix: 'pca9539'
drivers/i2c/chips/Kconfig +5 -2
@@ -92,8 +92,8 @@ config SENSORS_PCF8574 hardware. If unsure, say N. config SENSORS_PCA9539 - tristate "Philips PCA9539 16-bit I/O port" - depends on EXPERIMENTAL + tristate "Philips PCA9539 16-bit I/O port (DEPRECATED)" + depends on EXPERIMENTAL && GPIO_PCA9539 = "n" help If you say yes here you get support for the Philips PCA9539 16-bit I/O port.
@@ -101,6 +101,9 @@ config SENSORS_PCA9539 This driver can also be built as a module. If so, the module will be called pca9539. + This driver is deprecated and will be dropped soon. Use + drivers/gpio/pca9539.c instead. + config SENSORS_PCF8591 tristate "Philips PCF8591" depends on EXPERIMENTAL