Re: [PATCH 1/4 resend] [x86] Add generic GPIO support to x86

Previous thread: [PATCH 2/4 resend] pci.h : Add PCI identifiers for the RDC devices by Florian Fainelli on Thursday, October 18, 2007 - 6:51 am. (2 messages)

Next thread: [PATCH 3/4 resend] [x86] : Add support for the RDC R-321x SoC by Florian Fainelli on Thursday, October 18, 2007 - 6:51 am. (1 message)
From: Florian Fainelli
Date: Thursday, October 18, 2007 - 6:51 am

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 <florian.fainelli@telecomint.eu>
---
 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 */
-

From: Andres Salomon
Date: Thursday, October 18, 2007 - 2:38 pm

On Thu, 18 Oct 2007 15:51:24 +0200

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.


-

From: Florian Fainelli
Date: Friday, October 19, 2007 - 5:01 am

Hi Andres,


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

From: Andres Salomon
Date: Monday, October 22, 2007 - 12:18 pm

On Fri, 19 Oct 2007 14:01:56 +0200

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

From: Florian Fainelli
Date: Monday, October 22, 2007 - 12:59 pm

Hi Andres,


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

From: Thomas Gleixner
Date: Thursday, October 25, 2007 - 1:18 am

> > Le jeudi 18 octobre 2007, Andres Salomon a 
From: Florian Fainelli
Date: Thursday, October 25, 2007 - 5:30 am

Hi,


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

From: Thomas Gleixner
Date: Thursday, October 25, 2007 - 8:24 am

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


-

From: Andrew Morton
Date: Wednesday, February 13, 2008 - 3:02 pm

On Thu, 18 Oct 2007 15:51:24 +0200

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

From: David Brownell
Date: Wednesday, February 13, 2008 - 4:25 pm

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 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 */

--

From: David Brownell
Date: Wednesday, February 13, 2008 - 6:55 pm

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 <dbrownell@users.sourceforge.net>
---
 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 ...
From: Anton Vorontsov
Date: Friday, February 22, 2008 - 4:56 pm

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: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
--

From: David Brownell
Date: Friday, February 22, 2008 - 5:51 pm

Sure; that'll be gpio_request() and the gpio_direction_*() calls only
though, since those are the initial setup calls drivers make.

- Dave

--

Previous thread: [PATCH 2/4 resend] pci.h : Add PCI identifiers for the RDC devices by Florian Fainelli on Thursday, October 18, 2007 - 6:51 am. (2 messages)

Next thread: [PATCH 3/4 resend] [x86] : Add support for the RDC R-321x SoC by Florian Fainelli on Thursday, October 18, 2007 - 6:51 am. (1 message)