[PATCH] FRV: Hook up gpiolib support

[PATCH] FRV: Hook up gpiolib support

[Mark Brown]

Allow people to use gpiolib on FRV, mostly for build coverage as it
seems more useful to standardise the API than handle making it optional.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 arch/frv/Kconfig            |    4 +++
 arch/frv/include/asm/gpio.h |   55 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 0 deletions(-)
 create mode 100644 arch/frv/include/asm/gpio.h

diff --git a/arch/frv/Kconfig b/arch/frv/Kconfig
index cb884e4..8e828fc 100644
--- a/arch/frv/Kconfig
+++ b/arch/frv/Kconfig
@@ -7,6 +7,7 @@ config FRV
 	select HAVE_PERF_EVENTS
 	select HAVE_GENERIC_HARDIRQS
 	select GENERIC_IRQ_SHOW
+	select ARCH_WANT_OPTIONAL_GPIO
 
 config ZONE_DMA
 	bool
@@ -27,6 +28,9 @@ config GENERIC_CALIBRATE_DELAY
 	bool
 	default n
 
+config GENERIC_GPIO
+	def_bool y
+
 config TIME_LOW_RES
 	bool
 	default y
diff --git a/arch/frv/include/asm/gpio.h b/arch/frv/include/asm/gpio.h
new file mode 100644
index 0000000..8c3377f
--- /dev/null
+++ b/arch/frv/include/asm/gpio.h
@@ -0,0 +1,55 @@
+/*
+ * Generic GPIO API implementation for FRV.
+ *
+ * A stright copy of that for PowerPC which was:
+ *
+ * Copyright (c) 2007-2008  MontaVista Software, Inc.
+ *
+ * Author: Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _ASM_FRV_GPIO_H
+#define _ASM_FRV_GPIO_H
+
+#include <linux/errno.h>
+#include <asm-generic/gpio.h>
+
+#ifdef CONFIG_GPIOLIB
+
+/*
+ * We don't (yet) implement inlined/rapid versions for on-chip gpios.
+ * Just call gpiolib.
+ */
+static inline int gpio_get_value(unsigned int gpio)
+{
+	return __gpio_get_value(gpio);
+}
+
+static inline void gpio_set_value(unsigned int gpio, int value)
+{
+	__gpio_set_value(gpio, value);
+}
+
+static inline int gpio_cansleep(unsigned int gpio)
+{
+	return __gpio_cansleep(gpio);
+}
+
+static inline int gpio_to_irq(unsigned int gpio)
+{
+	return __gpio_to_irq(gpio);
+}
+
+static inline int irq_to_gpio(unsigned int irq)
+{
+	return -EINVAL;
+}
+
+#endif /* CONFIG_GPIOLIB */
+
+#endif /* _ASM_FRV_GPIO_H */
-- 
1.7.5.3

Re: [PATCH] FRV: Hook up gpiolib support

[Grant Likely]

On Wed, Jun 1, 2011 at 3:49 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> Allow people to use gpiolib on FRV, mostly for build coverage as it
> seems more useful to standardise the API than handle making it optional.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

g.

Re: [PATCH] FRV: Hook up gpiolib support

[David Howells]

Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> Allow people to use gpiolib on FRV, mostly for build coverage as it
> seems more useful to standardise the API than handle making it optional.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Nack.

There's an ordering problem.  The #inclusion of asm-generic/gpio.h must come
both before *and* after the functions in asm/gpio.h.  Take gpio_get_value()
for instance, it requires __gpio_get_value() - which is defined by the generic
header - but is used by gpio_get_value_cansleep() in the generic header.

You must split asm-generic/gpio.h.

David

Re: [PATCH] FRV: Hook up gpiolib support

[Mark Brown]

On Tue, Jun 14, 2011 at 07:08:32PM +0100, David Howells wrote:

> There's an ordering problem.  The #inclusion of asm-generic/gpio.h must come
> both before *and* after the functions in asm/gpio.h.  Take gpio_get_value()
> for instance, it requires __gpio_get_value() - which is defined by the generic
> header - but is used by gpio_get_value_cansleep() in the generic header.

Could you be more specific about the issue you see here?  The arch
header includes the generic header which appears to prototype all the
functions before it uses them.  I do also note that this is a verbatim
copy of the header that's been used on PowerPC for some considerable
time; is this some FRV specific limitation that wouldn't affect other
architectures?

> You must split asm-generic/gpio.h.

I must?

Re: [PATCH] FRV: Hook up gpiolib support

[David Howells]

Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> Could you be more specific about the issue you see here?

I was specific:

	Take gpio_get_value() for instance, it requires __gpio_get_value() -
	which is defined by the asm-generic header - but is used by
	gpio_get_value_cansleep() in the asm-generic header.

gpio_get_value() is defined in the arch header.

David

Re: [PATCH] FRV: Hook up gpiolib support

[Mark Brown]

On Thu, Jun 16, 2011 at 07:36:31AM +0100, David Howells wrote:
> Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > Could you be more specific about the issue you see here?

> I was specific:

> 	Take gpio_get_value() for instance, it requires __gpio_get_value() -
> 	which is defined by the asm-generic header - but is used by
> 	gpio_get_value_cansleep() in the asm-generic header.

> gpio_get_value() is defined in the arch header.

And to repeat what I said: the generic header prototypes everything it
needs before it uses it so there should be no problem here, it's not
been a problem on the other architectures using this code verbatim.
Please be more specific.

If there's some problem none of the tools notice or some FRV specific
issue you're going to have to tell us what it is.

Re: [PATCH] FRV: Hook up gpiolib support

[David Howells]

Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> And to repeat what I said: the generic header prototypes everything it
> needs before it uses it so there should be no problem here, it's not
> been a problem on the other architectures using this code verbatim.
> Please be more specific.

Okay:

  CC      drivers/mtd/maps/gpio-addr-flash.o
In file included from /data/frv/linux-2.6-frv/arch/frv/include/asm/gpio.h:21,
                 from include/linux/gpio.h:8,
                 from drivers/mtd/maps/gpio-addr-flash.c:17:
include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
include/asm-generic/gpio.h:233: error: implicit declaration of function 'gpio_get_value'
include/asm-generic/gpio.h: In function 'gpio_set_value_cansleep':
include/asm-generic/gpio.h:239: error: implicit declaration of function 'gpio_set_value'
drivers/mtd/maps/gpio-addr-flash.c: In function 'gpio_flash_probe':
drivers/mtd/maps/gpio-addr-flash.c:234: error: implicit declaration of function 'gpio_request'
drivers/mtd/maps/gpio-addr-flash.c:238: error: implicit declaration of function 'gpio_free'
drivers/mtd/maps/gpio-addr-flash.c:242: error: implicit declaration of function 'gpio_direction_output'


Let me expand on my explanation earlier:

 (1) asm-generic/gpio.h has an _inline_ function:

	static inline int gpio_get_value_cansleep(unsigned gpio)
	{
		might_sleep();
		return gpio_get_value(gpio);
	}

 (2) gpio_get_value() is implemented in asm/gpio.h.  It is not declared in
     asm-generic/gpio.h.

 (3) Therefore, the #inclusion of asm-generic/gpio.h must come _after_ the
     implementation of gpio_get_value() in asm/gpio.h.

 (4) asm/gpio.h implements the aforementioned inline function:

	static inline int gpio_get_value(unsigned int gpio)
	{
		return __gpio_get_value(gpio);
	}

 (5) __gpio_get_value() is declared in asm-generic/gpio.h.  It is not declared
     in asm/gpio.h.

 (6) Therefore, the #inclusion of asm/gpio.h must come _before_ the
     implementation of gpio_get_value() in asm/gpio.h.

Thus (3) and (6) contradict.

David

Re: [PATCH] FRV: Hook up gpiolib support

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2409 bytes --]

On Fri, Jun 17, 2011 at 11:50:59AM +0100, David Howells wrote:
> Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> 
> > And to repeat what I said: the generic header prototypes everything it
> > needs before it uses it so there should be no problem here, it's not
> > been a problem on the other architectures using this code verbatim.
> > Please be more specific.
> 
> Okay:
> 
>   CC      drivers/mtd/maps/gpio-addr-flash.o
> In file included from /data/frv/linux-2.6-frv/arch/frv/include/asm/gpio.h:21,
>                  from include/linux/gpio.h:8,
>                  from drivers/mtd/maps/gpio-addr-flash.c:17:
> include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
> include/asm-generic/gpio.h:233: error: implicit declaration of function 'gpio_get_value'
> include/asm-generic/gpio.h: In function 'gpio_set_value_cansleep':
> include/asm-generic/gpio.h:239: error: implicit declaration of function 'gpio_set_value'
> drivers/mtd/maps/gpio-addr-flash.c: In function 'gpio_flash_probe':
> drivers/mtd/maps/gpio-addr-flash.c:234: error: implicit declaration of function 'gpio_request'
> drivers/mtd/maps/gpio-addr-flash.c:238: error: implicit declaration of function 'gpio_free'
> drivers/mtd/maps/gpio-addr-flash.c:242: error: implicit declaration of function 'gpio_direction_output'
> 
> 
> Let me expand on my explanation earlier:
> 
>  (1) asm-generic/gpio.h has an _inline_ function:
> 
> 	static inline int gpio_get_value_cansleep(unsigned gpio)
> 	{
> 		might_sleep();
> 		return gpio_get_value(gpio);
> 	}
> 
>  (2) gpio_get_value() is implemented in asm/gpio.h.  It is not declared in
>      asm-generic/gpio.h.
> 
>  (3) Therefore, the #inclusion of asm-generic/gpio.h must come _after_ the
>      implementation of gpio_get_value() in asm/gpio.h.
> 
>  (4) asm/gpio.h implements the aforementioned inline function:
> 
> 	static inline int gpio_get_value(unsigned int gpio)
> 	{
> 		return __gpio_get_value(gpio);
> 	}
> 
>  (5) __gpio_get_value() is declared in asm-generic/gpio.h.  It is not declared
>      in asm/gpio.h.
> 
>  (6) Therefore, the #inclusion of asm/gpio.h must come _before_ the
>      implementation of gpio_get_value() in asm/gpio.h.
> 
> Thus (3) and (6) contradict.

sounds like it's time to move all that mess to one header alone ?!?
<linux/gpio.h> should be self-sufficient ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH] FRV: Hook up gpiolib support

[Mark Brown]

On Fri, Jun 17, 2011 at 11:50:59AM +0100, David Howells wrote:
> Okay:

Thanks, this is a lot more constructive.  I'm now wondering how anyone
has managed to use this header (which isn't exactly new or only used on
obscure architectures) without noticing and fixing this.

It's unlikely that I'm going to have the bandwidth to work on the cross
tree issues that sorting this would create in the immediate future, my
interest here is in stopping people bugging me about architectures that
don't keep up with the standard GPIO interface but it'd be good if
someone could do so.