linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] include/asm-generic/gpio.h: remove the call for __gpio_get_value() and __gpio_set_value() when GPIOLIB disabled
@ 2013-08-26 10:08 Chen Gang
  2013-08-29 12:08 ` Linus Walleij
  0 siblings, 1 reply; 5+ messages in thread
From: Chen Gang @ 2013-08-26 10:08 UTC (permalink / raw)
  To: linus.walleij
  Cc: Arnd Bergmann, linux-gpio, Linux-Arch,
	linux-kernel@vger.kernel.org

When GPIOLIB disabled, __gpio_get_value() and __gpio_set_value() will
not implement, so need remove them, or compiling fails.

e.g. (allmodconfig for h8300)

    CC      arch/h8300/kernel/h8300_ksyms.o
  In file included from arch/h8300/include/generated/asm/gpio.h:1:0,
                   from arch/h8300/kernel/h8300_ksyms.c:17:
  include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
  include/asm-generic/gpio.h:270:2: error: implicit declaration of function '__gpio_get_value' [-Werror=implicit-function-declaration]
    return __gpio_get_value(gpio);
    ^

For __gpio_get_value(), according to its implementation, it is enough
to use "return 0" instead of, and for __gpio_set_value(), just remove
directly.


Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
 include/asm-generic/gpio.h |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index bde6469..10a2853 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -267,13 +267,12 @@ static inline int gpio_cansleep(unsigned gpio)
 static inline int gpio_get_value_cansleep(unsigned gpio)
 {
 	might_sleep();
-	return __gpio_get_value(gpio);
+	return 0;
 }
 
 static inline void gpio_set_value_cansleep(unsigned gpio, int value)
 {
 	might_sleep();
-	__gpio_set_value(gpio, value);
 }
 
 #endif /* !CONFIG_GPIOLIB */
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] include/asm-generic/gpio.h: remove the call for __gpio_get_value() and __gpio_set_value() when GPIOLIB disabled
  2013-08-26 10:08 [PATCH] include/asm-generic/gpio.h: remove the call for __gpio_get_value() and __gpio_set_value() when GPIOLIB disabled Chen Gang
@ 2013-08-29 12:08 ` Linus Walleij
  2013-08-29 12:08   ` Linus Walleij
  2013-08-30  1:50   ` Chen Gang
  0 siblings, 2 replies; 5+ messages in thread
From: Linus Walleij @ 2013-08-29 12:08 UTC (permalink / raw)
  To: Chen Gang, Grant Likely, Russell King - ARM Linux
  Cc: Arnd Bergmann, linux-gpio@vger.kernel.org, Linux-Arch,
	linux-kernel@vger.kernel.org

On Mon, Aug 26, 2013 at 12:08 PM, Chen Gang <gang.chen@asianux.com> wrote:

> When GPIOLIB disabled, __gpio_get_value() and __gpio_set_value() will
> not implement, so need remove them, or compiling fails.
>
> e.g. (allmodconfig for h8300)
>
>     CC      arch/h8300/kernel/h8300_ksyms.o
>   In file included from arch/h8300/include/generated/asm/gpio.h:1:0,
>                    from arch/h8300/kernel/h8300_ksyms.c:17:
>   include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
>   include/asm-generic/gpio.h:270:2: error: implicit declaration of function '__gpio_get_value' [-Werror=implicit-function-declaration]
>     return __gpio_get_value(gpio);
>     ^
>
> For __gpio_get_value(), according to its implementation, it is enough
> to use "return 0" instead of, and for __gpio_set_value(), just remove
> directly.
>
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>

NAK, this is not how you do it. This fallback path is for GENERIC_GPIO
without GPIOLIB. Including it from a file indicates that you *are*
using GENERIC_GPIO when GPIOLIB is not activated. It can not be
used to stub out gpiolib.

This is a better alternative: let h8300 select
ARCH_HAVE_CUSTOM_GPIO_H

This is quite common:
$ git grep ARCH_HAVE_CUSTOM_GPIO_H
arch/arm/Kconfig:       select ARCH_HAVE_CUSTOM_GPIO_H
arch/avr32/Kconfig:     select ARCH_HAVE_CUSTOM_GPIO_H
arch/blackfin/Kconfig:  select ARCH_HAVE_CUSTOM_GPIO_H
arch/m68k/Kconfig.cpu:  select ARCH_HAVE_CUSTOM_GPIO_H
arch/mips/Kconfig:      select ARCH_HAVE_CUSTOM_GPIO_H
arch/sh/Kconfig:        select ARCH_HAVE_CUSTOM_GPIO_H
arch/unicore32/Kconfig: select ARCH_HAVE_CUSTOM_GPIO_H

Then put your stubs for __gpio_get_value() etc in
arch/h8300/include/asm/gpio.h

This way the h8300 will have a stub implementation if GPIOLIB
is not selected.

Be sure to put a comment about this in that file.

Note: I'm still a bit rookie as GPIO maintainer so if Grant or
Russell tells me I'm telling you wrong things: listen to them.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] include/asm-generic/gpio.h: remove the call for __gpio_get_value() and __gpio_set_value() when GPIOLIB disabled
  2013-08-29 12:08 ` Linus Walleij
@ 2013-08-29 12:08   ` Linus Walleij
  2013-08-30  1:50   ` Chen Gang
  1 sibling, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2013-08-29 12:08 UTC (permalink / raw)
  To: Chen Gang, Grant Likely, Russell King - ARM Linux
  Cc: Arnd Bergmann, linux-gpio@vger.kernel.org, Linux-Arch,
	linux-kernel@vger.kernel.org

On Mon, Aug 26, 2013 at 12:08 PM, Chen Gang <gang.chen@asianux.com> wrote:

> When GPIOLIB disabled, __gpio_get_value() and __gpio_set_value() will
> not implement, so need remove them, or compiling fails.
>
> e.g. (allmodconfig for h8300)
>
>     CC      arch/h8300/kernel/h8300_ksyms.o
>   In file included from arch/h8300/include/generated/asm/gpio.h:1:0,
>                    from arch/h8300/kernel/h8300_ksyms.c:17:
>   include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
>   include/asm-generic/gpio.h:270:2: error: implicit declaration of function '__gpio_get_value' [-Werror=implicit-function-declaration]
>     return __gpio_get_value(gpio);
>     ^
>
> For __gpio_get_value(), according to its implementation, it is enough
> to use "return 0" instead of, and for __gpio_set_value(), just remove
> directly.
>
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>

NAK, this is not how you do it. This fallback path is for GENERIC_GPIO
without GPIOLIB. Including it from a file indicates that you *are*
using GENERIC_GPIO when GPIOLIB is not activated. It can not be
used to stub out gpiolib.

This is a better alternative: let h8300 select
ARCH_HAVE_CUSTOM_GPIO_H

This is quite common:
$ git grep ARCH_HAVE_CUSTOM_GPIO_H
arch/arm/Kconfig:       select ARCH_HAVE_CUSTOM_GPIO_H
arch/avr32/Kconfig:     select ARCH_HAVE_CUSTOM_GPIO_H
arch/blackfin/Kconfig:  select ARCH_HAVE_CUSTOM_GPIO_H
arch/m68k/Kconfig.cpu:  select ARCH_HAVE_CUSTOM_GPIO_H
arch/mips/Kconfig:      select ARCH_HAVE_CUSTOM_GPIO_H
arch/sh/Kconfig:        select ARCH_HAVE_CUSTOM_GPIO_H
arch/unicore32/Kconfig: select ARCH_HAVE_CUSTOM_GPIO_H

Then put your stubs for __gpio_get_value() etc in
arch/h8300/include/asm/gpio.h

This way the h8300 will have a stub implementation if GPIOLIB
is not selected.

Be sure to put a comment about this in that file.

Note: I'm still a bit rookie as GPIO maintainer so if Grant or
Russell tells me I'm telling you wrong things: listen to them.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] include/asm-generic/gpio.h: remove the call for __gpio_get_value() and __gpio_set_value() when GPIOLIB disabled
  2013-08-29 12:08 ` Linus Walleij
  2013-08-29 12:08   ` Linus Walleij
@ 2013-08-30  1:50   ` Chen Gang
  2013-08-30  1:50     ` Chen Gang
  1 sibling, 1 reply; 5+ messages in thread
From: Chen Gang @ 2013-08-30  1:50 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Grant Likely, Russell King - ARM Linux, Arnd Bergmann,
	linux-gpio@vger.kernel.org, Linux-Arch,
	linux-kernel@vger.kernel.org, Geert Uytterhoeven

On 08/29/2013 08:08 PM, Linus Walleij wrote:
> On Mon, Aug 26, 2013 at 12:08 PM, Chen Gang <gang.chen@asianux.com> wrote:
> 
>> When GPIOLIB disabled, __gpio_get_value() and __gpio_set_value() will
>> not implement, so need remove them, or compiling fails.
>>
>> e.g. (allmodconfig for h8300)
>>
>>     CC      arch/h8300/kernel/h8300_ksyms.o
>>   In file included from arch/h8300/include/generated/asm/gpio.h:1:0,
>>                    from arch/h8300/kernel/h8300_ksyms.c:17:
>>   include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
>>   include/asm-generic/gpio.h:270:2: error: implicit declaration of function '__gpio_get_value' [-Werror=implicit-function-declaration]
>>     return __gpio_get_value(gpio);
>>     ^
>>
>> For __gpio_get_value(), according to its implementation, it is enough
>> to use "return 0" instead of, and for __gpio_set_value(), just remove
>> directly.
>>
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> 
> NAK, this is not how you do it. This fallback path is for GENERIC_GPIO
> without GPIOLIB. Including it from a file indicates that you *are*
> using GENERIC_GPIO when GPIOLIB is not activated. It can not be
> used to stub out gpiolib.
> 

Hmm... what you said above sounds reasonable to me (at least, it can be
acceptable).


> This is a better alternative: let h8300 select
> ARCH_HAVE_CUSTOM_GPIO_H
> 
> This is quite common:
> $ git grep ARCH_HAVE_CUSTOM_GPIO_H
> arch/arm/Kconfig:       select ARCH_HAVE_CUSTOM_GPIO_H
> arch/avr32/Kconfig:     select ARCH_HAVE_CUSTOM_GPIO_H
> arch/blackfin/Kconfig:  select ARCH_HAVE_CUSTOM_GPIO_H
> arch/m68k/Kconfig.cpu:  select ARCH_HAVE_CUSTOM_GPIO_H
> arch/mips/Kconfig:      select ARCH_HAVE_CUSTOM_GPIO_H
> arch/sh/Kconfig:        select ARCH_HAVE_CUSTOM_GPIO_H
> arch/unicore32/Kconfig: select ARCH_HAVE_CUSTOM_GPIO_H
> 
> Then put your stubs for __gpio_get_value() etc in
> arch/h8300/include/asm/gpio.h
> 
> This way the h8300 will have a stub implementation if GPIOLIB
> is not selected.
> 
> Be sure to put a comment about this in that file.
> 

That sounds a standard way for it, thanks.

Hmm... but for h8300, it seems it may not include "gpio.h" which is just
discussing about it with Geert in another thread (it seems what he said
is correct, and now I am just proving it).

Thanks.


> Note: I'm still a bit rookie as GPIO maintainer so if Grant or
> Russell tells me I'm telling you wrong things: listen to them.
> 

Thank you for your modesty and honesty.

And welcome other members' suggestions or completions.


Thanks.

> Yours,
> Linus Walleij
> 
> 


-- 
Chen Gang

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] include/asm-generic/gpio.h: remove the call for __gpio_get_value() and __gpio_set_value() when GPIOLIB disabled
  2013-08-30  1:50   ` Chen Gang
@ 2013-08-30  1:50     ` Chen Gang
  0 siblings, 0 replies; 5+ messages in thread
From: Chen Gang @ 2013-08-30  1:50 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Grant Likely, Russell King - ARM Linux, Arnd Bergmann,
	linux-gpio@vger.kernel.org, Linux-Arch,
	linux-kernel@vger.kernel.org, Geert Uytterhoeven

On 08/29/2013 08:08 PM, Linus Walleij wrote:
> On Mon, Aug 26, 2013 at 12:08 PM, Chen Gang <gang.chen@asianux.com> wrote:
> 
>> When GPIOLIB disabled, __gpio_get_value() and __gpio_set_value() will
>> not implement, so need remove them, or compiling fails.
>>
>> e.g. (allmodconfig for h8300)
>>
>>     CC      arch/h8300/kernel/h8300_ksyms.o
>>   In file included from arch/h8300/include/generated/asm/gpio.h:1:0,
>>                    from arch/h8300/kernel/h8300_ksyms.c:17:
>>   include/asm-generic/gpio.h: In function 'gpio_get_value_cansleep':
>>   include/asm-generic/gpio.h:270:2: error: implicit declaration of function '__gpio_get_value' [-Werror=implicit-function-declaration]
>>     return __gpio_get_value(gpio);
>>     ^
>>
>> For __gpio_get_value(), according to its implementation, it is enough
>> to use "return 0" instead of, and for __gpio_set_value(), just remove
>> directly.
>>
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> 
> NAK, this is not how you do it. This fallback path is for GENERIC_GPIO
> without GPIOLIB. Including it from a file indicates that you *are*
> using GENERIC_GPIO when GPIOLIB is not activated. It can not be
> used to stub out gpiolib.
> 

Hmm... what you said above sounds reasonable to me (at least, it can be
acceptable).


> This is a better alternative: let h8300 select
> ARCH_HAVE_CUSTOM_GPIO_H
> 
> This is quite common:
> $ git grep ARCH_HAVE_CUSTOM_GPIO_H
> arch/arm/Kconfig:       select ARCH_HAVE_CUSTOM_GPIO_H
> arch/avr32/Kconfig:     select ARCH_HAVE_CUSTOM_GPIO_H
> arch/blackfin/Kconfig:  select ARCH_HAVE_CUSTOM_GPIO_H
> arch/m68k/Kconfig.cpu:  select ARCH_HAVE_CUSTOM_GPIO_H
> arch/mips/Kconfig:      select ARCH_HAVE_CUSTOM_GPIO_H
> arch/sh/Kconfig:        select ARCH_HAVE_CUSTOM_GPIO_H
> arch/unicore32/Kconfig: select ARCH_HAVE_CUSTOM_GPIO_H
> 
> Then put your stubs for __gpio_get_value() etc in
> arch/h8300/include/asm/gpio.h
> 
> This way the h8300 will have a stub implementation if GPIOLIB
> is not selected.
> 
> Be sure to put a comment about this in that file.
> 

That sounds a standard way for it, thanks.

Hmm... but for h8300, it seems it may not include "gpio.h" which is just
discussing about it with Geert in another thread (it seems what he said
is correct, and now I am just proving it).

Thanks.


> Note: I'm still a bit rookie as GPIO maintainer so if Grant or
> Russell tells me I'm telling you wrong things: listen to them.
> 

Thank you for your modesty and honesty.

And welcome other members' suggestions or completions.


Thanks.

> Yours,
> Linus Walleij
> 
> 


-- 
Chen Gang

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-08-30  1:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-26 10:08 [PATCH] include/asm-generic/gpio.h: remove the call for __gpio_get_value() and __gpio_set_value() when GPIOLIB disabled Chen Gang
2013-08-29 12:08 ` Linus Walleij
2013-08-29 12:08   ` Linus Walleij
2013-08-30  1:50   ` Chen Gang
2013-08-30  1:50     ` Chen Gang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).