This patch adds the generic GPIO support to the x86
architecture. We do the same as for MIPS, we let
the machine override the gpio callbacks and provide
defaults one in mach-generic.
Signed-off-by: Florian Fainelli <[email protected]>
---
arch/i386/Kconfig | 4 ++++
include/asm-x86/gpio.h | 6 ++++++
include/asm-x86/mach-generic/gpio.h | 15 +++++++++++++++
3 files changed, 25 insertions(+), 0 deletions(-)
create mode 100644 include/asm-x86/gpio.h
create mode 100644 include/asm-x86/mach-generic/gpio.h
---
diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
index bf9aafa..501fd6d 100644
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -79,6 +79,10 @@ config GENERIC_BUG
default y
depends on BUG
+config GENERIC_GPIO
+ bool
+ default n
+
config GENERIC_HWEIGHT
bool
default y
diff --git a/include/asm-x86/gpio.h b/include/asm-x86/gpio.h
new file mode 100644
index 0000000..ff87fca
--- /dev/null
+++ b/include/asm-x86/gpio.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_I386_GPIO_H
+#define _ASM_I386_GPIO_H
+
+#include <gpio.h>
+
+#endif /* _ASM_I386_GPIO_H */
diff --git a/include/asm-x86/mach-generic/gpio.h b/include/asm-x86/mach-generic/gpio.h
new file mode 100644
index 0000000..5305dcb
--- /dev/null
+++ b/include/asm-x86/mach-generic/gpio.h
@@ -0,0 +1,15 @@
+#ifndef __ASM_MACH_GENERIC_GPIO_H
+#define __ASM_MACH_GENERIC_GPIO_H
+
+int gpio_request(unsigned gpio, const char *label);
+void gpio_free(unsigned gpio);
+int gpio_direction_input(unsigned gpio);
+int gpio_direction_output(unsigned gpio, int value);
+int gpio_get_value(unsigned gpio);
+void gpio_set_value(unsigned gpio, int value);
+int gpio_to_irq(unsigned gpio);
+int irq_to_gpio(unsigned irq);
+
+#include <asm-generic/gpio.h> /* cansleep wrappers */
+
+#endif /* __ASM_MACH_GENERIC_GPIO_H */
On Thu, 18 Oct 2007 15:51:24 +0200
Florian Fainelli <[email protected]> wrote:
> This patch adds the generic GPIO support to the x86
> architecture. We do the same as for MIPS, we let
> the machine override the gpio callbacks and provide
> defaults one in mach-generic.
>
[...]
> +
> +int gpio_request(unsigned gpio, const char *label);
> +void gpio_free(unsigned gpio);
> +int gpio_direction_input(unsigned gpio);
> +int gpio_direction_output(unsigned gpio, int value);
> +int gpio_get_value(unsigned gpio);
> +void gpio_set_value(unsigned gpio, int value);
> +int gpio_to_irq(unsigned gpio);
> +int irq_to_gpio(unsigned irq);
While I certainly would like to see a generic GPIO API, this one isn't
really useful for geode GPIOs. It would be nice to have one that did
work for us as well. Unfortunately, I haven't had the chance to give
much thought to this problem yet.
Hi Andres,
Le jeudi 18 octobre 2007, Andres Salomon a ?crit?:
> While I certainly would like to see a generic GPIO API, this one isn't
> really useful for geode GPIOs. It would be nice to have one that did
> work for us as well. Unfortunately, I haven't had the chance to give
> much thought to this problem yet.
This one was discussed mostly on the ARM mailing-list and finally made his way
to the mainline kernel. Though it lacks some functions to change for instance
a entire GPIO line and not a single bit, it is used on ARM and MIPS systems
so I would conform with this one for now because it is used by at least two
or more architectures.
On Fri, 19 Oct 2007 14:01:56 +0200, Florian Fainelli wrote:
> Hi Andres,
>
> Le jeudi 18 octobre 2007, Andres Salomon a écrit :
>> While I certainly would like to see a generic GPIO API, this one isn't
>> really useful for geode GPIOs. It would be nice to have one that did
>> work for us as well. Unfortunately, I haven't had the chance to give
>> much thought to this problem yet.
>
> This one was discussed mostly on the ARM mailing-list and finally made his way
> to the mainline kernel. Though it lacks some functions to change for instance
> a entire GPIO line and not a single bit, it is used on ARM and MIPS systems
> so I would conform with this one for now because it is used by at least two
> or more architectures.
Its reasonable to expect that the API will expand over time as it is
thrust into new situations. There is nothing wrong with the existing API,
it does an admirable job of simple GPIO frobbing. But on the Geode, GPIOs
can do much more then just simple input and output. We can cause events,
use input filtering for debouncing, set various drain options, and more.
And these are not bullet points from the datasheet that we want to
implement for completeness; these functions are actually being used right
now in the kernel on real hardware. Just because other architectures
haven't found a need to expand the API doesn't mean that we shouldn't
expand it now. And bear in mind, the Geode isn't unique - I'll bet beers
that there are MIPS and ARM architectures that have these features too,
and they would use the API if it existed.
Jordan
On Fri, 19 Oct 2007 21:32:08 +0000 (UTC)
Jordan Crouse <[email protected]> wrote:
> > This one was discussed mostly on the ARM mailing-list and finally
> > made his way to the mainline kernel. Though it lacks some functions
> > to change for instance a entire GPIO line and not a single bit, it
> > is used on ARM and MIPS systems so I would conform with this one
> > for now because it is used by at least two or more architectures.
Make that four: AVR32 and Blackfin use the generic GPIO API too.
> Its reasonable to expect that the API will expand over time as it is
> thrust into new situations. There is nothing wrong with the existing
> API, it does an admirable job of simple GPIO frobbing. But on the
> Geode, GPIOs can do much more then just simple input and output. We
> can cause events, use input filtering for debouncing, set various
> drain options, and more.
There are many other architectures that can do things like that. But
since there are lots of variations in this kind of functionality from
architecture to architecture (and even chip to chip), it has been
excluded from the generic GPIO API.
> And these are not bullet points from the
> datasheet that we want to implement for completeness; these functions
> are actually being used right now in the kernel on real hardware.
How is it used? In many cases, you can get away with simply configuring
the port with the required features like debouncing, pullup, drain
options, etc. in the platform setup code where you can freely use
platform-specific port configuration calls.
> Just because other architectures haven't found a need to expand the
> API doesn't mean that we shouldn't expand it now. And bear in mind,
> the Geode isn't unique - I'll bet beers that there are MIPS and ARM
> architectures that have these features too, and they would use the
> API if it existed.
These features are certainly being utilized in the platform setup code
on various platforms. I guess the current API might be too simple for
drivers that need to reconfigure the port on the fly, but I haven't
really seen any such drivers so far.
For example, the generic i2c-gpio driver supports three different ways
of driving the i2c lines depending on how the port was configured by
the platform.
Håvard
On Fri, 19 Oct 2007 14:01:56 +0200
Florian Fainelli <[email protected]> wrote:
> Hi Andres,
>
> Le jeudi 18 octobre 2007, Andres Salomon a écrit :
> > While I certainly would like to see a generic GPIO API, this one
> > isn't really useful for geode GPIOs. It would be nice to have one
> > that did work for us as well. Unfortunately, I haven't had the
> > chance to give much thought to this problem yet.
>
> This one was discussed mostly on the ARM mailing-list and finally
> made his way to the mainline kernel. Though it lacks some functions
> to change for instance a entire GPIO line and not a single bit, it is
> used on ARM and MIPS systems so I would conform with this one for now
> because it is used by at least two or more architectures.
How does being used on MIPS and ARM make it suitable for x86? If
you're arguing that other x86 platforms conform to it, and it might be
useful for them; sure, I'll buy that. That doesn't seem to currently be
the case, though.
Hi Andres,
Le lundi 22 octobre 2007, Andres Salomon a écrit :
> How does being used on MIPS and ARM make it suitable for x86? If
> you're arguing that other x86 platforms conform to it, and it might be
> useful for them; sure, I'll buy that. That doesn't seem to currently be
> the case, though.
The purpose of the API, as defines itself is meant to be generic and
architecture neutral, and the few functions it defines are.
Since I needed some GPIO helpers for the RDC R-321x SoC, I picked up this one.
Now if you want to propose another one go ahead, and I will rebase my code on
top of yours.
--
Cordialement, Florian Fainelli
------------------------------
On Mon, 22 Oct 2007, Andres Salomon wrote:
> On Fri, 19 Oct 2007 14:01:56 +0200
> Florian Fainelli <[email protected]> wrote:
>
> > Hi Andres,
> >
> > Le jeudi 18 octobre 2007, Andres Salomon a écrit :
> > > While I certainly would like to see a generic GPIO API, this one
> > > isn't really useful for geode GPIOs. It would be nice to have one
> > > that did work for us as well. Unfortunately, I haven't had the
> > > chance to give much thought to this problem yet.
> >
> > This one was discussed mostly on the ARM mailing-list and finally
> > made his way to the mainline kernel. Though it lacks some functions
> > to change for instance a entire GPIO line and not a single bit, it is
> > used on ARM and MIPS systems so I would conform with this one for now
> > because it is used by at least two or more architectures.
>
> How does being used on MIPS and ARM make it suitable for x86? If
> you're arguing that other x86 platforms conform to it, and it might be
> useful for them; sure, I'll buy that. That doesn't seem to currently be
> the case, though.
It might be useful for everyone to check, whether the existing GPIO
functionality can be extended, reworked to match the needs of Geode as
well. Extending / reworking an existing interface is definitely
better than adding a new incompatible one.
tglx
Hi,
Le jeudi 25 octobre 2007, Thomas Gleixner a ?crit?:
> It might be useful for everyone to check, whether the existing GPIO
> functionality can be extended, reworked to match the needs of Geode as
> well. Extending / reworking an existing interface is definitely
> better than adding a new incompatible one.
Of course. Just because Geode is the only x86 potential user of such an API
does not mean it should impose its design. Remember that this patch is part
of a serie to add support for the RDC R-321x SoC, which becomes another
x86-family user.
Looking at what Geode can do, and what RDC can do, I guess there are only few
functions that are generic, all the drain, multiplexing, direction things are
only available on the Geode board.
------------------------------
On Thu, 25 Oct 2007, Florian Fainelli wrote:
> > It might be useful for everyone to check, whether the existing GPIO
> > functionality can be extended, reworked to match the needs of Geode as
> > well. Extending / reworking an existing interface is definitely
> > better than adding a new incompatible one.
>
> Of course. Just because Geode is the only x86 potential user of such an API
> does not mean it should impose its design. Remember that this patch is part
> of a serie to add support for the RDC R-321x SoC, which becomes another
> x86-family user.
>
> Looking at what Geode can do, and what RDC can do, I guess there are only few
> functions that are generic, all the drain, multiplexing, direction things are
> only available on the Geode board.
It does not matter whether Geode has 500 functions and RDC has 5. It
matters that we do _not_ want two different APIs which deal with the
same thing, where one can obviously be a subset of the other. So
either we can extend / rework the stuff that both parties are happy or
we are not going to merge either of them.
tglx
On Thu, 18 Oct 2007 15:51:24 +0200
Florian Fainelli <[email protected]> wrote:
> This patch adds the generic GPIO support to the x86
> architecture. We do the same as for MIPS, we let
> the machine override the gpio callbacks and provide
> defaults one in mach-generic.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> arch/i386/Kconfig | 4 ++++
> include/asm-x86/gpio.h | 6 ++++++
> include/asm-x86/mach-generic/gpio.h | 15 +++++++++++++++
> 3 files changed, 25 insertions(+), 0 deletions(-)
> create mode 100644 include/asm-x86/gpio.h
> create mode 100644 include/asm-x86/mach-generic/gpio.h
> ---
> diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
> index bf9aafa..501fd6d 100644
> --- a/arch/i386/Kconfig
> +++ b/arch/i386/Kconfig
> @@ -79,6 +79,10 @@ config GENERIC_BUG
> default y
> depends on BUG
>
> +config GENERIC_GPIO
> + bool
> + default n
> +
> config GENERIC_HWEIGHT
> bool
> default y
> diff --git a/include/asm-x86/gpio.h b/include/asm-x86/gpio.h
> new file mode 100644
> index 0000000..ff87fca
> --- /dev/null
> +++ b/include/asm-x86/gpio.h
> @@ -0,0 +1,6 @@
> +#ifndef _ASM_I386_GPIO_H
> +#define _ASM_I386_GPIO_H
> +
> +#include <gpio.h>
> +
> +#endif /* _ASM_I386_GPIO_H */
> diff --git a/include/asm-x86/mach-generic/gpio.h b/include/asm-x86/mach-generic/gpio.h
> new file mode 100644
> index 0000000..5305dcb
> --- /dev/null
> +++ b/include/asm-x86/mach-generic/gpio.h
> @@ -0,0 +1,15 @@
> +#ifndef __ASM_MACH_GENERIC_GPIO_H
> +#define __ASM_MACH_GENERIC_GPIO_H
> +
> +int gpio_request(unsigned gpio, const char *label);
> +void gpio_free(unsigned gpio);
> +int gpio_direction_input(unsigned gpio);
> +int gpio_direction_output(unsigned gpio, int value);
> +int gpio_get_value(unsigned gpio);
> +void gpio_set_value(unsigned gpio, int value);
> +int gpio_to_irq(unsigned gpio);
> +int irq_to_gpio(unsigned irq);
> +
> +#include <asm-generic/gpio.h> /* cansleep wrappers */
> +
> +#endif /* __ASM_MACH_GENERIC_GPIO_H */
There's a new driver in git-dvb which does a plain old
#include <asm/gpio.h>
and it explodes on i386 allmodconfig with:
In file included from drivers/media/video/mt9m001.c:20:
include/asm/gpio.h:4:18: error: gpio.h: No such file or directory
I don't even know how this was supposed to work. What does "#include
<gpio.h>" give us? Is the kbuild system supposed to have added
-Iinclude/asm/mach-generic? It obviously didn't.
Someone please fix.
It would be more modern to have a <linux/gpio.h> which takes care of
cruddy details, but it's getting too late for that.
On Wednesday 13 February 2008, Andrew Morton wrote:
> Someone please fix.
This is what I was using purely for test builds ... most x86 hardware
actually has no GPIOs, and I sure don't have any of the "unusual" x86
platforms here, so I'd not want to see this version merge. Someone
more clued in on how x86 handles what ARM does with e.g. <asm/arch/...>
should make a better patch.
(And my vote is to **NOT** use the "-I..." magic that PowerPC uses,
that's just confusing as all get-out. And not needed.)
> It would be more modern to have a <linux/gpio.h> which takes care of
> cruddy details, but it's getting too late for that.
That could be added eventually ... if the platform has GPIO support
it could include the <asm/gpio.h> else it could define all the calls
as error-returning inlines.
- Dave
========= CUT HERE
DEBUG ONLY -- make X86_PC use gpiolib.
It's not clear to me how the various x86-ish platforms should
be made to work here, since there seems to be no convention
that each platform type has its own <asm/arch/...> subdir.
---
arch/x86/Kconfig | 2 ++
include/asm-x86/gpio.h | 19 ++++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
--- g26.orig/arch/x86/Kconfig 2008-02-10 16:06:30.000000000 -0800
+++ g26/arch/x86/Kconfig 2008-02-10 16:09:44.000000000 -0800
@@ -228,6 +228,8 @@ choice
config X86_PC
bool "PC-compatible"
+ select GENERIC_GPIO
+ select HAVE_GPIO_LIB
help
Choose this option if your computer is a standard PC or compatible.
--- g26.orig/include/asm-x86/gpio.h 2008-02-10 16:06:30.000000000 -0800
+++ g26/include/asm-x86/gpio.h 2008-02-10 16:09:44.000000000 -0800
@@ -1,6 +1,23 @@
#ifndef _ASM_I386_GPIO_H
#define _ASM_I386_GPIO_H
-#include <gpio.h>
+// #include <gpio.h>
+
+#include <asm-generic/gpio.h> /* cansleep wrappers */
+
+#define gpio_get_value __gpio_get_value
+#define gpio_set_value __gpio_set_value
+#define gpio_cansleep __gpio_cansleep
+
+static inline int gpio_to_irq(unsigned gpio)
+{
+ return -ENOSYS;
+}
+
+static inline int irq_to_gpio(unsigned irq)
+{
+ return -EINVAL;
+}
+
#endif /* _ASM_I386_GPIO_H */
On Wednesday 13 February 2008, Andrew Morton wrote:
> It would be more modern to have a <linux/gpio.h> which takes care of
> cruddy details, but it's getting too late for that.
Sort of like this? For drivers that don't want to list themselves
in Kconfig as depending on GENERIC_GPIO (e.g. one NAND driver I heard
about), this lets them use <linux/gpio.h> ... existing code can't
break, and it won't hurt if new code uses this.
- Dave
============== CUT HERE
Add a <linux/gpio.h> defining fail/warn stubs for GPIO calls on
platforms that don't support the GPIO programming interface. It
includes the arch-specific interface glue otherwise.
Signed-off-by: David Brownell <[email protected]>
---
include/linux/gpio.h | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ g26/include/linux/gpio.h 2008-02-13 17:40:06.000000000 -0800
@@ -0,0 +1,93 @@
+#ifndef __LINUX_GPIO_H
+#define __LINUX_GPIO_H
+
+#ifdef CONFIG_GENERIC_GPIO
+#include <asm/gpio.h>
+
+#else
+
+/*
+ * Some platforms don't support the GPIO programming interface.
+ *
+ * In case some driver uses it anyway (it should normally have
+ * depended on GENERIC_GPIO), these routines help the compiler
+ * optimize out much GPIO-related code ... or trigger a runtime
+ * warning when something is wrongly called.
+ */
+
+static inline int gpio_is_valid(int number)
+{
+ return 0;
+}
+
+static inline int gpio_request(unsigned gpio, const char *label)
+{
+ return -EINVAL;
+}
+
+static inline void gpio_free(unsigned gpio)
+{
+ /* GPIO can never have been requested */
+ WARN_ON(1);
+}
+
+static inline int gpio_direction_input(unsigned gpio)
+{
+ return -EINVAL;
+}
+
+static inline int gpio_direction_output(unsigned gpio, int value)
+{
+ return -EINVAL;
+}
+
+static inline int gpio_get_value(unsigned gpio)
+{
+ /* GPIO can never have been requested or set as {in,out}put */
+ WARN_ON(1);
+ return 0;
+}
+
+static inline void gpio_set_value(unsigned gpio, int value)
+{
+ /* GPIO can never have been requested or set as output */
+ WARN_ON(1);
+}
+
+static int gpio_cansleep(unsigned gpio)
+{
+ /* GPIO can never have been requested or set as {in,out}put */
+ WARN_ON(1);
+ return 0;
+}
+
+static inline int gpio_get_value_cansleep(unsigned gpio)
+{
+ /* GPIO can never have been requested or set as {in,out}put */
+ WARN_ON(1);
+ return 0;
+}
+
+static inline void gpio_set_value_cansleep(unsigned gpio, int value)
+{
+ /* GPIO can never have been requested or set as output */
+ WARN_ON(1);
+}
+
+static inline int gpio_to_irq(unsigned gpio)
+{
+ /* GPIO can never have been requested or set as input */
+ WARN_ON(1);
+ return -EINVAL;
+}
+
+static inline int irq_to_gpio(unsigned irq)
+{
+ /* irq can never have been returned from gpio_to_irq() */
+ WARN_ON(1);
+ return -EINVAL;
+}
+
+#endif
+
+#endif /* __LINUX_GPIO_H */
On Wed, Feb 13, 2008 at 05:55:30PM -0800, David Brownell wrote:
> On Wednesday 13 February 2008, Andrew Morton wrote:
> > It would be more modern to have a <linux/gpio.h> which takes care of
> > cruddy details, but it's getting too late for that.
>
> Sort of like this? For drivers that don't want to list themselves
> in Kconfig as depending on GENERIC_GPIO (e.g. one NAND driver I heard
> about), this lets them use <linux/gpio.h> ... existing code can't
> break, and it won't hurt if new code uses this.
>
> - Dave
>
>
> ============== CUT HERE
> Add a <linux/gpio.h> defining fail/warn stubs for GPIO calls on
> platforms that don't support the GPIO programming interface. It
> includes the arch-specific interface glue otherwise.
>
> Signed-off-by: David Brownell <[email protected]>
> ---
> include/linux/gpio.h | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ g26/include/linux/gpio.h 2008-02-13 17:40:06.000000000 -0800
> @@ -0,0 +1,93 @@
> +#ifndef __LINUX_GPIO_H
> +#define __LINUX_GPIO_H
> +
> +#ifdef CONFIG_GENERIC_GPIO
> +#include <asm/gpio.h>
> +
> +#else
> +
> +/*
> + * Some platforms don't support the GPIO programming interface.
> + *
> + * In case some driver uses it anyway (it should normally have
> + * depended on GENERIC_GPIO), these routines help the compiler
> + * optimize out much GPIO-related code ... or trigger a runtime
> + * warning when something is wrongly called.
> + */
> +
> +static inline int gpio_is_valid(int number)
> +{
> + return 0;
> +}
> +
> +static inline int gpio_request(unsigned gpio, const char *label)
> +{
> + return -EINVAL;
Could we return -ENOSYS instead, so drivers will able to distinguish
between "something really failed" and "there is no gpio support,
fallback to other methods"? Without this, the notorious NAND driver
will hardly benefit from this change. ;-)
--
Anton Vorontsov
email: [email protected]
backup email: [email protected]
irc://irc.freenode.net/bd2
On Friday 22 February 2008, Anton Vorontsov wrote:
> > +static inline int gpio_request(unsigned gpio, const char *label)
> > +{
> > +?????return -EINVAL;
>
> Could we return -ENOSYS instead, so drivers will able to distinguish
> between "something really failed" and "there is no gpio support,
> fallback to other methods"? Without this, the notorious NAND driver
> will hardly benefit from this change. ;-)
Sure; that'll be gpio_request() and the gpio_direction_*() calls only
though, since those are the initial setup calls drivers make.
- Dave