Skip to contentSkip to conversation

gpio: always enable GPIO_OMAP on ARCH_OMAP

PATCH New Arnd Bergmann 3 participants 7 messages Apr 28, 2014 – May 9, 2014 GPIO

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 gpio: always enable GPIO_OMAP on ARCH_OMAP

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.

drivers/gpio/Kconfig +3 -2
index c58b828..c8c42be 100644
@@ -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.
#1 lore raw

PATCH 1/1 gpio: always enable GPIO_OMAP on ARCH_OMAP

Hello Arnd,

Thanks a lot for the patch.

Quoted 5 lines · Arnd Bergmann

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:

Quoted 8 lines

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.

Quoted 12 lines

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.

drivers/gpio/Kconfig +0 -0
index c58b828..c8c42be 100644

drivers/gpio/Kconfig config GPIO_OCTEON · line 247

Quoted 5 lines
drivers/gpio/Kconfig +1 -1
@@ -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

drivers/gpio/Kconfig config GPIO_OCTEON · line 253

Quoted 8 lines
drivers/gpio/Kconfig +2 -1
 	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:

Best regards,
Javier

#2 lore raw

PATCH 1/1 gpio: always enable GPIO_OMAP on ARCH_OMAP

Quoted 19 lines · 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.

Quoted 14 lines

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.

drivers/gpio/Kconfig +0 -0
index c58b828..c8c42be 100644
@@ -244,9 +244,10 @@ config GPIO_OCTEON
        family of SOCs.
 
Quoted 9 lines

drivers/gpio/Kconfig config GPIO_OCTEON · line 247

Quoted 3 lines
drivers/gpio/Kconfig +1 -1
 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

#3 lore raw

PATCH 1/1 gpio: always enable GPIO_OMAP on ARCH_OMAP

Hello Arnd,

Quoted 30 lines · Arnd Bergmann
Quoted 19 lines · 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.

Quoted 14 lines

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.

drivers/gpio/Kconfig +0 -0
index c58b828..c8c42be 100644
@@ -244,9 +244,10 @@ config GPIO_OCTEON
        family of SOCs.
 
Quoted 9 lines

drivers/gpio/Kconfig config GPIO_OCTEON · line 247

Quoted 3 lines
drivers/gpio/Kconfig +1 -1
 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

Quoted 7 lines

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

#4 lore raw

PATCH 1/1 gpio: always enable GPIO_OMAP on ARCH_OMAP

Quoted 39 lines · On Tuesday 29 April 2014 12:53:45 Javier Martinez Canillas wrote:

Hello Arnd,

Quoted 26 lines · Arnd Bergmann
Quoted 25 lines · 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.

Quoted 12 lines

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.

drivers/gpio/Kconfig +0 -0
index c58b828..c8c42be 100644

drivers/gpio/Kconfig config GPIO_OCTEON · line 247

Quoted 5 lines
drivers/gpio/Kconfig +1 -1
@@ -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:

Quoted 9 lines
Quoted 3 lines

+ 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.

Quoted 3 lines

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.

Quoted 3 lines

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!

Quoted 4 lines

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

#5 lore raw

PATCH 1/1 gpio: always enable GPIO_OMAP on ARCH_OMAP

In reply to Arnd Bergmann
Quoted 13 lines · Arnd Bergmann

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.

Quoted 6 lines

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.

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

#6 lore raw

PATCH 1/1 gpio: always enable GPIO_OMAP on ARCH_OMAP

Quoted 20 lines · On Friday 09 May 2014 09:48:00 Linus Walleij wrote:
Quoted 18 lines · Arnd Bergmann

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.

Quoted 4 lines
Quoted 1 line

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