2012-10-31 07:00:49

by Yuanhan Liu

[permalink] [raw]
Subject: [PATCH 1/2] gpio: move those interface depends on GPIOLIB into proper section

There are 2 issues with current code:

1. redefinition of 'gpio_cansleep':
include/linux/gpio.h:60:19: error: redefinition of 'gpio_cansleep'
include/asm-generic/gpio.h:212:19: note: previous definition of 'gpio_cansleep' was here

The root cause is include/linux/gpio.h has a defintion of it. And if
CONFIG_GPIOLIB is not set, include/asm-generic/gpio.h has another
definition.

2. include/linux/gpio.h:62:2: error: implicit declaration of function '__gpio_cansleep'

Only happens when CONFIG_GPIOLIB is not set, too. Since it called
gpiolib functions without putting something like: #ifdef CONFIG_GPIOLIB

So, move those functions into proper section at
include/asm-generic/gpio.h would resolve 2 above issues.

Cc: Grant Likely <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Fengguang Wu <[email protected]>
Signed-off-by: Yuanhan Liu <[email protected]>
---
include/asm-generic/gpio.h | 39 +++++++++++++++++++++++++++++++++++++++
include/linux/gpio.h | 22 +---------------------
2 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index a9432fc..a73272d 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -199,14 +199,53 @@ extern void gpio_unexport(unsigned gpio);

#endif /* CONFIG_GPIO_SYSFS */

+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);
+}
+
+
#else /* !CONFIG_GPIOLIB */

+static inline int gpio_get_value(unsigned int gpio)
+{
+ WARN_ON(1);
+ return 0;
+}
+
+static inline void gpio_set_value(unsigned int gpio, int value)
+{
+ WARN_ON(1);
+}
+
static inline bool gpio_is_valid(int number)
{
/* only non-negative numbers are valid */
return number >= 0;
}

+static inline int gpio_to_irq(unsigned int gpio)
+{
+ WARN_ON(1);
+ return -EINVAL;
+}
+
+
/* platforms that don't directly support access to GPIOs through I2C, SPI,
* or other blocking infrastructure can use these wrappers.
*/
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 2e31e8b..eb6e6ac 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -47,26 +47,6 @@ struct gpio {

#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);
-}
-
-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;
@@ -74,7 +54,7 @@ static inline int irq_to_gpio(unsigned int irq)

#endif

-#else
+#else /* !CONFIG_GENERIC_GPIO */

#include <linux/kernel.h>
#include <linux/types.h>
--
1.7.7.6


2012-10-31 07:00:51

by Yuanhan Liu

[permalink] [raw]
Subject: [PATCH 2/2] gpio: do not call __gpio_xxx under !CONFIG_GPIOLIB

Those functions are availabe only when CONFIG_GPIOLIB is set. So, we
should not call them under !CONFIG_GPIOLIB block.

This would fix following build errros:
include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
include/asm-generic/gpio.h:220:2: error: implicit declaration of function '__gpio_get_value'
include/asm-generic/gpio.h: In function 'gpio_set_value_cansleep':
nclude/asm-generic/gpio.h:226:2: error: implicit declaration of function '__gpio_set_value'

Cc: Grant Likely <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: Fengguang Wu <[email protected]>
Signed-off-by: Yuanhan Liu <[email protected]>
---
include/asm-generic/gpio.h | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index a73272d..704bf5d 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -257,14 +257,13 @@ static inline int gpio_cansleep(unsigned gpio)

static inline int gpio_get_value_cansleep(unsigned gpio)
{
- might_sleep();
- return __gpio_get_value(gpio);
+ WARN_ON(1);
+ return 0;
}

static inline void gpio_set_value_cansleep(unsigned gpio, int value)
{
- might_sleep();
- __gpio_set_value(gpio, value);
+ WARN_ON(1);
}

#endif /* !CONFIG_GPIOLIB */
--
1.7.7.6

2012-10-31 07:12:42

by Fengguang Wu

[permalink] [raw]
Subject: Re: [PATCH 1/2] gpio: move those interface depends on GPIOLIB into proper section

On Wed, Oct 31, 2012 at 03:00:54PM +0800, Yuanhan Liu wrote:
> There are 2 issues with current code:
>
> 1. redefinition of 'gpio_cansleep':
> include/linux/gpio.h:60:19: error: redefinition of 'gpio_cansleep'
> include/asm-generic/gpio.h:212:19: note: previous definition of 'gpio_cansleep' was here
>
> The root cause is include/linux/gpio.h has a defintion of it. And if
> CONFIG_GPIOLIB is not set, include/asm-generic/gpio.h has another
> definition.
>
> 2. include/linux/gpio.h:62:2: error: implicit declaration of function '__gpio_cansleep'
>
> Only happens when CONFIG_GPIOLIB is not set, too. Since it called
> gpiolib functions without putting something like: #ifdef CONFIG_GPIOLIB
>
> So, move those functions into proper section at
> include/asm-generic/gpio.h would resolve 2 above issues.

The patch introduces new build errors:

yuanhan:gpio-fixes [2/2] db1a5a0 blackfin-BF526-EZBRD_defconfig

[error]
arch/blackfin/include/asm/gpio.h:197:19: error: redefinition of 'gpio_get_value'
arch/blackfin/include/asm/gpio.h:205:20: error: redefinition of 'gpio_set_value'
arch/blackfin/include/asm/gpio.h:213:19: error: redefinition of 'gpio_cansleep'
arch/blackfin/include/asm/gpio.h:218:19: error: redefinition of 'gpio_to_irq'

yuanhan:gpio-fixes [2/2] db1a5a0 blackfin-TCM-BF537_defconfig

[error]
include/asm-generic/gpio.h:225:19: error: redefinition of 'gpio_get_value'
include/asm-generic/gpio.h:231:20: error: redefinition of 'gpio_set_value'
include/asm-generic/gpio.h:242:19: error: redefinition of 'gpio_to_irq'

yuanhan:gpio-fixes [2/2] db1a5a0 sh-rsk7269_defconfig

[error]
arch/sh/include/asm/gpio.h:27:19: error: redefinition of 'gpio_get_value'
arch/sh/include/asm/gpio.h:32:20: error: redefinition of 'gpio_set_value'
arch/sh/include/asm/gpio.h:37:19: error: redefinition of 'gpio_cansleep'
arch/sh/include/asm/gpio.h:42:19: error: redefinition of 'gpio_to_irq'

yuanhan:gpio-fixes [2/2] db1a5a0 avr32-atngw100_defconfig

[error]
arch/avr32/mach-at32ap/include/mach/gpio.h:18: error: redefinition of 'gpio_get_value'
include/asm-generic/gpio.h:203: error: previous definition of 'gpio_get_value' was here
arch/avr32/mach-at32ap/include/mach/gpio.h:23: error: redefinition of 'gpio_set_value'
include/asm-generic/gpio.h:208: error: previous definition of 'gpio_set_value' was here
arch/avr32/mach-at32ap/include/mach/gpio.h:28: error: redefinition of 'gpio_cansleep'
include/asm-generic/gpio.h:213: error: previous definition of 'gpio_cansleep' was here
arch/avr32/mach-at32ap/include/mach/gpio.h:34: error: redefinition of 'gpio_to_irq'
include/asm-generic/gpio.h:218: error: previous definition of 'gpio_to_irq' was here

Thanks,
Fengguang

2012-10-31 07:19:17

by Yuanhan Liu

[permalink] [raw]
Subject: Re: [PATCH 1/2] gpio: move those interface depends on GPIOLIB into proper section

On Wed, Oct 31, 2012 at 03:12:36PM +0800, Fengguang Wu wrote:
> On Wed, Oct 31, 2012 at 03:00:54PM +0800, Yuanhan Liu wrote:
> > There are 2 issues with current code:
> >
> > 1. redefinition of 'gpio_cansleep':
> > include/linux/gpio.h:60:19: error: redefinition of 'gpio_cansleep'
> > include/asm-generic/gpio.h:212:19: note: previous definition of 'gpio_cansleep' was here
> >
> > The root cause is include/linux/gpio.h has a defintion of it. And if
> > CONFIG_GPIOLIB is not set, include/asm-generic/gpio.h has another
> > definition.
> >
> > 2. include/linux/gpio.h:62:2: error: implicit declaration of function '__gpio_cansleep'
> >
> > Only happens when CONFIG_GPIOLIB is not set, too. Since it called
> > gpiolib functions without putting something like: #ifdef CONFIG_GPIOLIB
> >
> > So, move those functions into proper section at
> > include/asm-generic/gpio.h would resolve 2 above issues.
>
> The patch introduces new build errors:

Ooops. I checked the log at build server and posted it after finding no
errors. Seems that the test hasn't been finished?

Sorry for the nosiy. Will send v2 then.

Thanks!

>
> yuanhan:gpio-fixes [2/2] db1a5a0 blackfin-BF526-EZBRD_defconfig
>
> [error]
> arch/blackfin/include/asm/gpio.h:197:19: error: redefinition of 'gpio_get_value'
> arch/blackfin/include/asm/gpio.h:205:20: error: redefinition of 'gpio_set_value'
> arch/blackfin/include/asm/gpio.h:213:19: error: redefinition of 'gpio_cansleep'
> arch/blackfin/include/asm/gpio.h:218:19: error: redefinition of 'gpio_to_irq'
>
> yuanhan:gpio-fixes [2/2] db1a5a0 blackfin-TCM-BF537_defconfig
>
> [error]
> include/asm-generic/gpio.h:225:19: error: redefinition of 'gpio_get_value'
> include/asm-generic/gpio.h:231:20: error: redefinition of 'gpio_set_value'
> include/asm-generic/gpio.h:242:19: error: redefinition of 'gpio_to_irq'
>
> yuanhan:gpio-fixes [2/2] db1a5a0 sh-rsk7269_defconfig
>
> [error]
> arch/sh/include/asm/gpio.h:27:19: error: redefinition of 'gpio_get_value'
> arch/sh/include/asm/gpio.h:32:20: error: redefinition of 'gpio_set_value'
> arch/sh/include/asm/gpio.h:37:19: error: redefinition of 'gpio_cansleep'
> arch/sh/include/asm/gpio.h:42:19: error: redefinition of 'gpio_to_irq'
>
> yuanhan:gpio-fixes [2/2] db1a5a0 avr32-atngw100_defconfig
>
> [error]
> arch/avr32/mach-at32ap/include/mach/gpio.h:18: error: redefinition of 'gpio_get_value'
> include/asm-generic/gpio.h:203: error: previous definition of 'gpio_get_value' was here
> arch/avr32/mach-at32ap/include/mach/gpio.h:23: error: redefinition of 'gpio_set_value'
> include/asm-generic/gpio.h:208: error: previous definition of 'gpio_set_value' was here
> arch/avr32/mach-at32ap/include/mach/gpio.h:28: error: redefinition of 'gpio_cansleep'
> include/asm-generic/gpio.h:213: error: previous definition of 'gpio_cansleep' was here
> arch/avr32/mach-at32ap/include/mach/gpio.h:34: error: redefinition of 'gpio_to_irq'
> include/asm-generic/gpio.h:218: error: previous definition of 'gpio_to_irq' was here
>
> Thanks,
> Fengguang

2012-11-04 17:47:19

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 2/2] gpio: do not call __gpio_xxx under !CONFIG_GPIOLIB

On Wed, Oct 31, 2012 at 8:00 AM, Yuanhan Liu
<[email protected]> wrote:

> Those functions are availabe only when CONFIG_GPIOLIB is set. So, we
> should not call them under !CONFIG_GPIOLIB block.
>
> This would fix following build errros:
> include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
> include/asm-generic/gpio.h:220:2: error: implicit declaration of function '__gpio_get_value'
> include/asm-generic/gpio.h: In function 'gpio_set_value_cansleep':
> nclude/asm-generic/gpio.h:226:2: error: implicit declaration of function '__gpio_set_value'

OK...

> static inline int gpio_get_value_cansleep(unsigned gpio)
> {
> - might_sleep();

So why are you deleting this very useful might_sleep() runtime
semantic check?

> - return __gpio_get_value(gpio);
> + WARN_ON(1);
> + return 0;
> }
>
> static inline void gpio_set_value_cansleep(unsigned gpio, int value)
> {
> - might_sleep();
> - __gpio_set_value(gpio, value);
> + WARN_ON(1);
> }

Yours,
Linus Walleij

2012-11-06 05:27:21

by Yuanhan Liu

[permalink] [raw]
Subject: Re: [PATCH 2/2] gpio: do not call __gpio_xxx under !CONFIG_GPIOLIB

On Sun, Nov 04, 2012 at 06:47:12PM +0100, Linus Walleij wrote:
> On Wed, Oct 31, 2012 at 8:00 AM, Yuanhan Liu
> <[email protected]> wrote:
>
> > Those functions are availabe only when CONFIG_GPIOLIB is set. So, we
> > should not call them under !CONFIG_GPIOLIB block.
> >
> > This would fix following build errros:
> > include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
> > include/asm-generic/gpio.h:220:2: error: implicit declaration of function '__gpio_get_value'
> > include/asm-generic/gpio.h: In function 'gpio_set_value_cansleep':
> > nclude/asm-generic/gpio.h:226:2: error: implicit declaration of function '__gpio_set_value'
>
> OK...
>
> > static inline int gpio_get_value_cansleep(unsigned gpio)
> > {
> > - might_sleep();
Hi,

>
> So why are you deleting this very useful might_sleep() runtime
> semantic check?

Yes, I should keep it. I did it because I saw the definition of
gpio_get_value at include/linux/gpio.h didn't call that function.

Will keep it in the next version together with the previous patch.
And sorry that it may take some time, as I'm occupied by other things :(

Thanks.

--yliu
>
> > - return __gpio_get_value(gpio);
> > + WARN_ON(1);
> > + return 0;
> > }
> >
> > static inline void gpio_set_value_cansleep(unsigned gpio, int value)
> > {
> > - might_sleep();
> > - __gpio_set_value(gpio, value);
> > + WARN_ON(1);
> > }
>
> Yours,
> Linus Walleij