diff mbox

arm64 defconfig breakage (gpio) in next-20130218

Message ID 20130221093104.GB23442@S2101-09.ap.freescale.net
State New
Headers show

Commit Message

Shawn Guo Feb. 21, 2013, 9:31 a.m. UTC 
On Mon, Feb 18, 2013 at 11:14:10AM +0000, Mark Rutland wrote:
> Hello,
> 
> When I attempt to build a defconfig arm64 kernel from next-20130218, it fails
> with the following complaints from gcc:
> 
> In file included from include/linux/gpio.h:48:0,
>                  from drivers/gpio/devres.c:18:
> 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]
> include/asm-generic/gpio.h: In function ‘gpio_set_value_cansleep’:
> include/asm-generic/gpio.h:276:2: error: implicit declaration of function ‘__gpio_set_value’ [-Werror=implicit-function-declaration]
> In file included from drivers/gpio/devres.c:18:0:
> include/linux/gpio.h: At top level:
> include/linux/gpio.h:60:19: error: redefinition of ‘gpio_cansleep’
> In file included from include/linux/gpio.h:48:0,
>                  from drivers/gpio/devres.c:18:
> include/asm-generic/gpio.h:262:19: note: previous definition of ‘gpio_cansleep’ was here
> In file included from drivers/gpio/devres.c:18:0:
> include/linux/gpio.h: In function ‘gpio_cansleep’:
> include/linux/gpio.h:62:2: error: implicit declaration of function ‘__gpio_cansleep’ [-Werror=implicit-function-declaration]
> include/linux/gpio.h: In function ‘gpio_to_irq’:
> include/linux/gpio.h:67:2: error: implicit declaration of function ‘__gpio_to_irq’ [-Werror=implicit-function-declaration]
> drivers/gpio/devres.c: In function ‘devm_gpio_release’:
> drivers/gpio/devres.c:26:2: error: implicit declaration of function ‘gpio_free’ [-Werror=implicit-function-declaration]
> drivers/gpio/devres.c: In function ‘devm_gpio_request’:
> drivers/gpio/devres.c:60:2: error: implicit declaration of function ‘gpio_request’ [-Werror=implicit-function-declaration]
> drivers/gpio/devres.c: In function ‘devm_gpio_request_one’:
> drivers/gpio/devres.c:90:2: error: implicit declaration of function ‘gpio_request_one’ [-Werror=implicit-function-declaration]
> cc1: some warnings being treated as errors
> make[2]: *** [drivers/gpio/devres.o] Error 1
> make[1]: *** [drivers/gpio] Error 2
> 
> I've bisected this down to 0fa2fd9a0d: "Merge branch 'linusw/devel' of
> git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git into
> gpio/next". Both parent commits build fine.
> 
> I've played around a bit, and selecting ARCH_REQUIRE_GPIOLIB in the arm64
> Kconfig fixes the issue, but I'm unfamilar with gpio{,lib} and I'm not sure
> that's the best/correct way of solving this.

Kconfig GENERIC_GPIO is designed as an option defined at architecture
level and select by platform who knows how gpio driver/API is
implemented.  The following change should fix the error.

Shawn

Comments

Grant Likely Feb. 21, 2013, 10:36 a.m. UTC | #1 
On Thu, Feb 21, 2013 at 10:02 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 21 February 2013 17:31:07 Shawn Guo wrote:
>> >
>> > I've bisected this down to 0fa2fd9a0d: "Merge branch 'linusw/devel' of
>> > git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git into
>> > gpio/next". Both parent commits build fine.
>> >
>> > I've played around a bit, and selecting ARCH_REQUIRE_GPIOLIB in the arm64
>> > Kconfig fixes the issue, but I'm unfamilar with gpio{,lib} and I'm not sure
>> > that's the best/correct way of solving this.
>>
>> Kconfig GENERIC_GPIO is designed as an option defined at architecture
>> level and select by platform who knows how gpio driver/API is
>> implemented.  The following change should fix the error.
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index f532ce5..aca4a25 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -93,7 +93,7 @@ config IOMMU_HELPER
>>         def_bool SWIOTLB
>>
>>  config GENERIC_GPIO
>> -       def_bool y
>> +       bool
>>
>>  source "init/Kconfig"
>
> But we have no arm64 platforms that would pick GENERIC_GPIO, it's expected
> to come from GPIOLIB anyway. I think the right fix is this:
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index ab4aa54..3fab0db 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -3,6 +3,7 @@ config ARM64
>         select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>         select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
>         select ARCH_WANT_FRAME_POINTERS
> +       select ARCH_WANT_OPTIONAL_GPIOLIB
>         select ARM_AMBA
>         select ARM_ARCH_TIMER
>         select CLONE_BACKWARDS
> @@ -92,9 +93,6 @@ config SWIOTLB
>  config IOMMU_HELPER
>         def_bool SWIOTLB
>
> -config GENERIC_GPIO
> -       def_bool y
> -
>  source "init/Kconfig"
>
>  source "kernel/Kconfig.freezer"

I've not asked Linus to pull the GPIO tree yet because of this issue.
Can someone please test this on aarch64 and send me a properly
formatted patch?

g.
Catalin Marinas Feb. 21, 2013, 10:37 a.m. UTC | #2 
On Thu, Feb 21, 2013 at 10:02:15AM +0000, Arnd Bergmann wrote:
> On Thursday 21 February 2013 17:31:07 Shawn Guo wrote:
> > > 
> > > I've bisected this down to 0fa2fd9a0d: "Merge branch 'linusw/devel' of
> > > git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git into
> > > gpio/next". Both parent commits build fine.
> > > 
> > > I've played around a bit, and selecting ARCH_REQUIRE_GPIOLIB in the arm64
> > > Kconfig fixes the issue, but I'm unfamilar with gpio{,lib} and I'm not sure
> > > that's the best/correct way of solving this.
> > 
> > Kconfig GENERIC_GPIO is designed as an option defined at architecture
> > level and select by platform who knows how gpio driver/API is
> > implemented.  The following change should fix the error.
> > 
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index f532ce5..aca4a25 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -93,7 +93,7 @@ config IOMMU_HELPER
> >         def_bool SWIOTLB
> > 
> >  config GENERIC_GPIO
> > -       def_bool y
> > +       bool
> > 
> >  source "init/Kconfig"
> 
> But we have no arm64 platforms that would pick GENERIC_GPIO, it's expected
> to come from GPIOLIB anyway. I think the right fix is this:
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index ab4aa54..3fab0db 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -3,6 +3,7 @@ config ARM64
>  	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>  	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
>  	select ARCH_WANT_FRAME_POINTERS
> +	select ARCH_WANT_OPTIONAL_GPIOLIB
>  	select ARM_AMBA
>  	select ARM_ARCH_TIMER
>  	select CLONE_BACKWARDS
> @@ -92,9 +93,6 @@ config SWIOTLB
>  config IOMMU_HELPER
>  	def_bool SWIOTLB
>  
> -config GENERIC_GPIO
> -	def_bool y
> -

In my soc-armv8-model branch, the ARCH_VEXPRESS selects
ARCH_REQUIRE_GPIOLIB which in turn selects GENERIC_GPIO but this seems
to be defined in the arch Kconfig.
Catalin Marinas Feb. 21, 2013, 10:46 a.m. UTC | #3 
On Thu, Feb 21, 2013 at 10:42:57AM +0000, Arnd Bergmann wrote:
> An architecture should not unconditionally enable 'GENERIC_GPIO'
> without providing an implementation. In case of arm64, selecting
> ARCH_WANT_OPTIONAL_GPIOLIB is the right solution, because it
> lets us enable GPIOLIB when configuring the kernel, and that
> implicitly turns on GENERIC_GPIO.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ----
> On Thursday 21 February 2013 10:36:41 Grant Likely wrote:
> > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > index ab4aa54..3fab0db 100644
> > > --- a/arch/arm64/Kconfig
> > > +++ b/arch/arm64/Kconfig
> > > @@ -3,6 +3,7 @@ config ARM64
> > >         select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
> > >         select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
> > >         select ARCH_WANT_FRAME_POINTERS
> > > +       select ARCH_WANT_OPTIONAL_GPIOLIB
> > >         select ARM_AMBA
> > >         select ARM_ARCH_TIMER
> > >         select CLONE_BACKWARDS
> > > @@ -92,9 +93,6 @@ config SWIOTLB
> > >  config IOMMU_HELPER
> > >         def_bool SWIOTLB
> > >
> > > -config GENERIC_GPIO
> > > -       def_bool y
> > > -
> > >  source "init/Kconfig"
> > >
> > >  source "kernel/Kconfig.freezer"
> > 
> > I've not asked Linus to pull the GPIO tree yet because of this issue.
> > Can someone please test this on aarch64 and send me a properly
> > formatted patch?
> 
> It was wrong, the new version below is good though. Tested with defconfig
> and allyesconfig on arm64. I found a few more trivial issues with arm64
> allyesconfig that I fixed up as well. I'll send separate patches
> for those.

That's what I just tried, it looks ok to me:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Maybe Mark can try it with -next (I only tested with 3.8).

Thanks.
Linus Walleij Feb. 21, 2013, 9:35 p.m. UTC | #4 
On Thu, Feb 21, 2013 at 11:44 AM, Arnd Bergmann <arnd@arndb.de> wrote:

> An architecture should not unconditionally enable 'GENERIC_GPIO'
> without providing an implementation. In case of arm64, selecting
> ARCH_WANT_OPTIONAL_GPIOLIB is the right solution, because it
> lets us enable GPIOLIB when configuring the kernel, and that
> implicitly turns on GENERIC_GPIO.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

GENERIC_GPIO must die, it's just causing trouble like
this all the time :-(

Yours,
Linus Walleij
Mark Rutland Feb. 22, 2013, 9:39 a.m. UTC | #5 
On Thu, Feb 21, 2013 at 10:46:44AM +0000, Catalin Marinas wrote:
> On Thu, Feb 21, 2013 at 10:42:57AM +0000, Arnd Bergmann wrote:
> > An architecture should not unconditionally enable 'GENERIC_GPIO'
> > without providing an implementation. In case of arm64, selecting
> > ARCH_WANT_OPTIONAL_GPIOLIB is the right solution, because it
> > lets us enable GPIOLIB when configuring the kernel, and that
> > implicitly turns on GENERIC_GPIO.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ----
> > On Thursday 21 February 2013 10:36:41 Grant Likely wrote:
> > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > index ab4aa54..3fab0db 100644
> > > > --- a/arch/arm64/Kconfig
> > > > +++ b/arch/arm64/Kconfig
> > > > @@ -3,6 +3,7 @@ config ARM64
> > > >         select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
> > > >         select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
> > > >         select ARCH_WANT_FRAME_POINTERS
> > > > +       select ARCH_WANT_OPTIONAL_GPIOLIB
> > > >         select ARM_AMBA
> > > >         select ARM_ARCH_TIMER
> > > >         select CLONE_BACKWARDS
> > > > @@ -92,9 +93,6 @@ config SWIOTLB
> > > >  config IOMMU_HELPER
> > > >         def_bool SWIOTLB
> > > >
> > > > -config GENERIC_GPIO
> > > > -       def_bool y
> > > > -
> > > >  source "init/Kconfig"
> > > >
> > > >  source "kernel/Kconfig.freezer"
> > > 
> > > I've not asked Linus to pull the GPIO tree yet because of this issue.
> > > Can someone please test this on aarch64 and send me a properly
> > > formatted patch?
> > 
> > It was wrong, the new version below is good though. Tested with defconfig
> > and allyesconfig on arm64. I found a few more trivial issues with arm64
> > allyesconfig that I fixed up as well. I'll send separate patches
> > for those.
> 
> That's what I just tried, it looks ok to me:
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> 
> Maybe Mark can try it with -next (I only tested with 3.8).

I've just tried the updated patch on next-20130222; arm64 defconfig builds
successfully.

Thanks,
Mark.
Grant Likely Feb. 22, 2013, 2:29 p.m. UTC | #6 
On Thu, 21 Feb 2013 22:35:49 +0100, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Feb 21, 2013 at 11:44 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > An architecture should not unconditionally enable 'GENERIC_GPIO'
> > without providing an implementation. In case of arm64, selecting
> > ARCH_WANT_OPTIONAL_GPIOLIB is the right solution, because it
> > lets us enable GPIOLIB when configuring the kernel, and that
> > implicitly turns on GENERIC_GPIO.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Okay, I've put this into my gpio/next branch and I'll ask Linus to pull
shortly. Thanks everyone.

g.
diff mbox

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index f532ce5..aca4a25 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -93,7 +93,7 @@  config IOMMU_HELPER
        def_bool SWIOTLB

 config GENERIC_GPIO
-       def_bool y
+       bool

 source "init/Kconfig"