diff mbox

gpio: always enable GPIO_OMAP on ARCH_OMAP

Message ID 5523458.JVZJJObMjC@wuerfel (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann April 28, 2014, 9:07 a.m. UTC 
Commit 4df42de9d3e "gpio: omap: add a GPIO_OMAP option instead of using
ARCH_OMAP" made it possible to build OMAP kernels without the GPIO driver,
which at least on OMAP2 and OMAP3 causes build errors because of functions
used by the platform power management code:

arch/arm/mach-omap2/built-in.o: In function `omap_sram_idle':
arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_prepare_for_idle'
arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_resume_after_idle'

We presumably always want the GPIO driver on OMAP, so this adds a slightly
broader dependency and only allows disabling the driver only when no
OMAP2PLUS platform is selected.

However, it seems entirely reasonable to include the driver in build tests
on other platforms, so we should also allow building it for COMPILE_TEST
builds and select the required GENERIC_IRQ_CHIP that may not already be
enabled on other platforms.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Javier Martinez Canillas April 28, 2014, 11:59 p.m. UTC | #1 
Hello Arnd,

Thanks a lot for the patch.

On 04/28/2014 11:07 AM, Arnd Bergmann wrote:
> Commit 4df42de9d3e "gpio: omap: add a GPIO_OMAP option instead of using
> ARCH_OMAP" made it possible to build OMAP kernels without the GPIO driver,
> which at least on OMAP2 and OMAP3 causes build errors because of functions
> used by the platform power management code:
> 
> arch/arm/mach-omap2/built-in.o: In function `omap_sram_idle':
> arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_prepare_for_idle'
> arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_resume_after_idle'
> 
> We presumably always want the GPIO driver on OMAP, so this adds a slightly
> broader dependency and only allows disabling the driver only when no
> OMAP2PLUS platform is selected.
>

This driver is also used by OMAP1. Even when disabling GPIO_OMAP on that
platform doesn't cause a build error since no function defined in the driver is
used directly by platform code, I think that we always want this driver on OMAP1
too.

> However, it seems entirely reasonable to include the driver in build tests
> on other platforms, so we should also allow building it for COMPILE_TEST
> builds and select the required GENERIC_IRQ_CHIP that may not already be
> enabled on other platforms.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index c58b828..c8c42be 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -244,9 +244,10 @@ config GPIO_OCTEON
>  	  family of SOCs.
>  
>  config GPIO_OMAP
> -	bool "TI OMAP GPIO support"
> +	bool "TI OMAP GPIO support" if COMPILE_TEST && !ARCH_OMAP2PLUS

So this should be:

+       bool "TI OMAP GPIO support" if COMPILE_TEST && !ARCH_OMAP2PLUS &&
!ARCH_OMAP1

>  	default y if ARCH_OMAP
> -	depends on ARM && ARCH_OMAP
> +	depends on ARM
> +	select GENERIC_IRQ_CHIP
>  	select GPIOLIB_IRQCHIP
>  	help
>  	  Say yes here to enable GPIO support for TI OMAP SoCs.
> 

With that change:

Acked-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann April 29, 2014, 10:26 a.m. UTC | #2 
On Tuesday 29 April 2014 01:59:20 Javier Martinez Canillas wrote:
> This driver is also used by OMAP1. Even when disabling GPIO_OMAP on that
> platform doesn't cause a build error since no function defined in the driver is
> used directly by platform code, I think that we always want this driver on OMAP1
> too.
> 
> > However, it seems entirely reasonable to include the driver in build tests
> > on other platforms, so we should also allow building it for COMPILE_TEST
> > builds and select the required GENERIC_IRQ_CHIP that may not already be
> > enabled on other platforms.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index c58b828..c8c42be 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -244,9 +244,10 @@ config GPIO_OCTEON
> >         family of SOCs.
> >  
> >  config GPIO_OMAP
> > -     bool "TI OMAP GPIO support"
> > +     bool "TI OMAP GPIO support" if COMPILE_TEST && !ARCH_OMAP2PLUS
> 
> So this should be:
> 
> +       bool "TI OMAP GPIO support" if COMPILE_TEST && !ARCH_OMAP2PLUS &&
> !ARCH_OMAP1
> 

Well, if COMPILE_TEST is disabled on OMAP1, the option is already
hidden and enabled in my version. It seems reasonable to me to 
allow compile-testing OMAP1 without the GPIO driver, while a kernel
running on OMAP1 should always have COMPILE_TEST disabled.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas April 29, 2014, 10:53 a.m. UTC | #3 
Hello Arnd,

On 04/29/2014 12:26 PM, Arnd Bergmann wrote:
> On Tuesday 29 April 2014 01:59:20 Javier Martinez Canillas wrote:
>> This driver is also used by OMAP1. Even when disabling GPIO_OMAP on that
>> platform doesn't cause a build error since no function defined in the driver is
>> used directly by platform code, I think that we always want this driver on OMAP1
>> too.
>> 
>> > However, it seems entirely reasonable to include the driver in build tests
>> > on other platforms, so we should also allow building it for COMPILE_TEST
>> > builds and select the required GENERIC_IRQ_CHIP that may not already be
>> > enabled on other platforms.
>> > 
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> > 
>> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> > index c58b828..c8c42be 100644
>> > --- a/drivers/gpio/Kconfig
>> > +++ b/drivers/gpio/Kconfig
>> > @@ -244,9 +244,10 @@ config GPIO_OCTEON
>> >         family of SOCs.
>> >  
>> >  config GPIO_OMAP
>> > -     bool "TI OMAP GPIO support"
>> > +     bool "TI OMAP GPIO support" if COMPILE_TEST && !ARCH_OMAP2PLUS
>> 
>> So this should be:
>> 
>> +       bool "TI OMAP GPIO support" if COMPILE_TEST && !ARCH_OMAP2PLUS &&
>> !ARCH_OMAP1
>> 
> 
> Well, if COMPILE_TEST is disabled on OMAP1, the option is already
> hidden and enabled in my version. It seems reasonable to me to 
> allow compile-testing OMAP1 without the GPIO driver, while a kernel
> running on OMAP1 should always have COMPILE_TEST disabled.
> 
> 	Arnd
> 

I understand your point. I thought you also wanted to be sure that the option
will be hidden in platforms where it make sense to always enable GPIO_OMAP even
if COMPILE_TEST is enabled.

If the idea of this patch is to only avoid build errors when GPIO_OMAP is
disabled and allowing build testing the driver in other platforms, then I'm ok
with your patch and feel free to add my Acked-by tag.

And you are right that COMPILE_TEST is disabled in omap1_defconfig and after all
there are so many drivers needed for a platform to be usable on boot. So one has
to take care of enabling all the needed options as long as there isn't a
combination that cause a build error like the one you are fixing here.

Thanks a lot and best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann April 29, 2014, 12:07 p.m. UTC | #4 
On Tuesday 29 April 2014 12:53:45 Javier Martinez Canillas wrote:
> Hello Arnd,
> 
> On 04/29/2014 12:26 PM, Arnd Bergmann wrote:
> > On Tuesday 29 April 2014 01:59:20 Javier Martinez Canillas wrote:
> >> This driver is also used by OMAP1. Even when disabling GPIO_OMAP on that
> >> platform doesn't cause a build error since no function defined in the driver is
> >> used directly by platform code, I think that we always want this driver on OMAP1
> >> too.
> >> 
> >> > However, it seems entirely reasonable to include the driver in build tests
> >> > on other platforms, so we should also allow building it for COMPILE_TEST
> >> > builds and select the required GENERIC_IRQ_CHIP that may not already be
> >> > enabled on other platforms.
> >> > 
> >> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> > 
> >> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> >> > index c58b828..c8c42be 100644
> >> > --- a/drivers/gpio/Kconfig
> >> > +++ b/drivers/gpio/Kconfig
> >> > @@ -244,9 +244,10 @@ config GPIO_OCTEON
> >> >         family of SOCs.
> >> >  
> >> >  config GPIO_OMAP
> >> > -     bool "TI OMAP GPIO support"
> >> > +     bool "TI OMAP GPIO support" if COMPILE_TEST && !ARCH_OMAP2PLUS
> >> 
> >> So this should be:
> >> 
> >> +       bool "TI OMAP GPIO support" if COMPILE_TEST && !ARCH_OMAP2PLUS &&
> >> !ARCH_OMAP1
> >> 
> > 
> > Well, if COMPILE_TEST is disabled on OMAP1, the option is already
> > hidden and enabled in my version. It seems reasonable to me to 
> > allow compile-testing OMAP1 without the GPIO driver, while a kernel
> > running on OMAP1 should always have COMPILE_TEST disabled.
> > 
> 
> I understand your point. I thought you also wanted to be sure that the option
> will be hidden in platforms where it make sense to always enable GPIO_OMAP even
> if COMPILE_TEST is enabled.

I thought about that, but didn't want to enforce it without a good reason
when your original patch had just made it possible to disable the option.

> If the idea of this patch is to only avoid build errors when GPIO_OMAP is
> disabled and allowing build testing the driver in other platforms, then I'm ok
> with your patch and feel free to add my Acked-by tag.

Ok, thanks!

> And you are right that COMPILE_TEST is disabled in omap1_defconfig and after all
> there are so many drivers needed for a platform to be usable on boot. So one has
> to take care of enabling all the needed options as long as there isn't a
> combination that cause a build error like the one you are fixing here.

Ok

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij May 9, 2014, 7:48 a.m. UTC | #5 
On Mon, Apr 28, 2014 at 11:07 AM, Arnd Bergmann <arnd@arndb.de> wrote:

> Commit 4df42de9d3e "gpio: omap: add a GPIO_OMAP option instead of using
> ARCH_OMAP" made it possible to build OMAP kernels without the GPIO driver,
> which at least on OMAP2 and OMAP3 causes build errors because of functions
> used by the platform power management code:
>
> arch/arm/mach-omap2/built-in.o: In function `omap_sram_idle':
> arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_prepare_for_idle'
> arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_resume_after_idle'
>
> We presumably always want the GPIO driver on OMAP, so this adds a slightly
> broader dependency and only allows disabling the driver only when no
> OMAP2PLUS platform is selected.
>
> However, it seems entirely reasonable to include the driver in build tests
> on other platforms, so we should also allow building it for COMPILE_TEST
> builds and select the required GENERIC_IRQ_CHIP that may not already be
> enabled on other platforms.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied to my devel branch, this is not a fix to Torvalds HEAD
AFAICT, but a fix to v3.16? Else poke me.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann May 9, 2014, 8:48 a.m. UTC | #6 
On Friday 09 May 2014 09:48:00 Linus Walleij wrote:
> On Mon, Apr 28, 2014 at 11:07 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > Commit 4df42de9d3e "gpio: omap: add a GPIO_OMAP option instead of using
> > ARCH_OMAP" made it possible to build OMAP kernels without the GPIO driver,
> > which at least on OMAP2 and OMAP3 causes build errors because of functions
> > used by the platform power management code:
> >
> > arch/arm/mach-omap2/built-in.o: In function `omap_sram_idle':
> > arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_prepare_for_idle'
> > arch/arm/mach-omap2/pm24xx.c:129: undefined reference to `omap2_gpio_resume_after_idle'
> >
> > We presumably always want the GPIO driver on OMAP, so this adds a slightly
> > broader dependency and only allows disabling the driver only when no
> > OMAP2PLUS platform is selected.
> >
> > However, it seems entirely reasonable to include the driver in build tests
> > on other platforms, so we should also allow building it for COMPILE_TEST
> > builds and select the required GENERIC_IRQ_CHIP that may not already be
> > enabled on other platforms.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Patch applied to my devel branch, this is not a fix to Torvalds HEAD
> AFAICT, but a fix to v3.16? Else poke me.

Yes, 3.16 is ok.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c58b828..c8c42be 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -244,9 +244,10 @@  config GPIO_OCTEON
 	  family of SOCs.
 
 config GPIO_OMAP
-	bool "TI OMAP GPIO support"
+	bool "TI OMAP GPIO support" if COMPILE_TEST && !ARCH_OMAP2PLUS
 	default y if ARCH_OMAP
-	depends on ARM && ARCH_OMAP
+	depends on ARM
+	select GENERIC_IRQ_CHIP
 	select GPIOLIB_IRQCHIP
 	help
 	  Say yes here to enable GPIO support for TI OMAP SoCs.