[v2,04/13] gpio: Introduce CONFIG_ONLY_GENERIC_GPIO to cleanup #ifdefs

Message ID 161861627412.298230.5111171339392037845.stgit@localhost
State New
Headers show
Series
  • arm64: synquacer: Add SynQuacer/DeveloperBox support
Related show

Commit Message

Masami Hiramatsu April 16, 2021, 11:37 p.m.
Many architecture do not have specific asm/arch/gpio.h, so instead
of adding !defined(CONFIG_ARCH_xxx), introduce CONFIG_ONLY_GENERIC_GPIO
and select it.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

---
 arch/arm/Kconfig             |   17 +++++++++++++++++
 arch/arm/include/asm/gpio.h  |    8 +-------
 board/cortina/common/Kconfig |    1 +
 3 files changed, 19 insertions(+), 7 deletions(-)

Comments

Simon Glass April 29, 2021, 4:09 p.m. | #1
Hi Masami,

On Fri, 16 Apr 2021 at 16:38, Masami Hiramatsu
<masami.hiramatsu@linaro.org> wrote:
>

> Many architecture do not have specific asm/arch/gpio.h, so instead

> of adding !defined(CONFIG_ARCH_xxx), introduce CONFIG_ONLY_GENERIC_GPIO


This seems OK, but I think GPIO_GENERIC_ONLY is a better name, since
it uses the GPIO prefix.

I would also prefer to have a 'positive' option, but I suspect that
might be a pain to do?

> and select it.

>

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

> ---

>  arch/arm/Kconfig             |   17 +++++++++++++++++

>  arch/arm/include/asm/gpio.h  |    8 +-------

>  board/cortina/common/Kconfig |    1 +

>  3 files changed, 19 insertions(+), 7 deletions(-)

>


[..]

Regards,
Simon
Masami Hiramatsu April 30, 2021, 2:03 a.m. | #2
Hi Simon,

2021年4月30日(金) 1:10 Simon Glass <sjg@chromium.org>:
>

> Hi Masami,

>

> On Fri, 16 Apr 2021 at 16:38, Masami Hiramatsu

> <masami.hiramatsu@linaro.org> wrote:

> >

> > Many architecture do not have specific asm/arch/gpio.h, so instead

> > of adding !defined(CONFIG_ARCH_xxx), introduce CONFIG_ONLY_GENERIC_GPIO

>

> This seems OK, but I think GPIO_GENERIC_ONLY is a better name, since

> it uses the GPIO prefix.

>

> I would also prefer to have a 'positive' option, but I suspect that

> might be a pain to do?


Would you mean making it something like CONFIG_GPIO_EXTRA_HEADER ?

I think it is also possible. My concern is if I missed any arch which
should say y that :P.

Thank you,

>

> > and select it.

> >

> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>

> > ---

> >  arch/arm/Kconfig             |   17 +++++++++++++++++

> >  arch/arm/include/asm/gpio.h  |    8 +-------

> >  board/cortina/common/Kconfig |    1 +

> >  3 files changed, 19 insertions(+), 7 deletions(-)

> >

>

> [..]

>

> Regards,

> Simon




--
Masami Hiramatsu
Simon Glass April 30, 2021, 6:13 p.m. | #3
Hi Masami,

On Thu, 29 Apr 2021 at 20:03, Masami Hiramatsu
<masami.hiramatsu@linaro.org> wrote:
>

> Hi Simon,

>

> 2021年4月30日(金) 1:10 Simon Glass <sjg@chromium.org>:

> >

> > Hi Masami,

> >

> > On Fri, 16 Apr 2021 at 16:38, Masami Hiramatsu

> > <masami.hiramatsu@linaro.org> wrote:

> > >

> > > Many architecture do not have specific asm/arch/gpio.h, so instead

> > > of adding !defined(CONFIG_ARCH_xxx), introduce CONFIG_ONLY_GENERIC_GPIO

> >

> > This seems OK, but I think GPIO_GENERIC_ONLY is a better name, since

> > it uses the GPIO prefix.

> >

> > I would also prefer to have a 'positive' option, but I suspect that

> > might be a pain to do?

>

> Would you mean making it something like CONFIG_GPIO_EXTRA_HEADER ?

>

> I think it is also possible. My concern is if I missed any arch which

> should say y that :P.


Yes...it's just that negative configs generally cause us pain at some point.

Regards,
Simon
Tom Rini May 3, 2021, 11:51 a.m. | #4
On Fri, Apr 30, 2021 at 11:13:45AM -0700, Simon Glass wrote:
> Hi Masami,

> 

> On Thu, 29 Apr 2021 at 20:03, Masami Hiramatsu

> <masami.hiramatsu@linaro.org> wrote:

> >

> > Hi Simon,

> >

> > 2021年4月30日(金) 1:10 Simon Glass <sjg@chromium.org>:

> > >

> > > Hi Masami,

> > >

> > > On Fri, 16 Apr 2021 at 16:38, Masami Hiramatsu

> > > <masami.hiramatsu@linaro.org> wrote:

> > > >

> > > > Many architecture do not have specific asm/arch/gpio.h, so instead

> > > > of adding !defined(CONFIG_ARCH_xxx), introduce CONFIG_ONLY_GENERIC_GPIO

> > >

> > > This seems OK, but I think GPIO_GENERIC_ONLY is a better name, since

> > > it uses the GPIO prefix.

> > >

> > > I would also prefer to have a 'positive' option, but I suspect that

> > > might be a pain to do?

> >

> > Would you mean making it something like CONFIG_GPIO_EXTRA_HEADER ?

> >

> > I think it is also possible. My concern is if I missed any arch which

> > should say y that :P.

> 

> Yes...it's just that negative configs generally cause us pain at some point.


And in terms of catching all of the platforms, you can submit a pull
request in GitHub which will trigger a CI world build and tell you
what's broken :)

-- 
Tom
Masami Hiramatsu May 6, 2021, 2:28 a.m. | #5
Hi Tom,

2021年5月3日(月) 20:51 Tom Rini <trini@konsulko.com>:
>

> On Fri, Apr 30, 2021 at 11:13:45AM -0700, Simon Glass wrote:

> > Hi Masami,

> >

> > On Thu, 29 Apr 2021 at 20:03, Masami Hiramatsu

> > <masami.hiramatsu@linaro.org> wrote:

> > >

> > > Hi Simon,

> > >

> > > 2021年4月30日(金) 1:10 Simon Glass <sjg@chromium.org>:

> > > >

> > > > Hi Masami,

> > > >

> > > > On Fri, 16 Apr 2021 at 16:38, Masami Hiramatsu

> > > > <masami.hiramatsu@linaro.org> wrote:

> > > > >

> > > > > Many architecture do not have specific asm/arch/gpio.h, so instead

> > > > > of adding !defined(CONFIG_ARCH_xxx), introduce CONFIG_ONLY_GENERIC_GPIO

> > > >

> > > > This seems OK, but I think GPIO_GENERIC_ONLY is a better name, since

> > > > it uses the GPIO prefix.

> > > >

> > > > I would also prefer to have a 'positive' option, but I suspect that

> > > > might be a pain to do?

> > >

> > > Would you mean making it something like CONFIG_GPIO_EXTRA_HEADER ?

> > >

> > > I think it is also possible. My concern is if I missed any arch which

> > > should say y that :P.

> >

> > Yes...it's just that negative configs generally cause us pain at some point.

>

> And in terms of catching all of the platforms, you can submit a pull

> request in GitHub which will trigger a CI world build and tell you

> what's broken :)


OK, let me try it.

Thank you!

>

> --

> Tom




-- 
Masami Hiramatsu

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 3307f2b3fc..fba28323cd 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -89,6 +89,11 @@  config HAS_VBAR
 config HAS_THUMB2
 	bool
 
+config ONLY_GENERIC_GPIO
+	bool
+	help
+	  The target has no arch-xxxx/gpio.h and use only asm-generic/gpio.h.
+
 # Used for compatibility with asm files copied from the kernel
 config ARM_ASM_UNIFIED
 	bool
@@ -634,18 +639,21 @@  config ARCH_BCM283X
 
 config ARCH_BCM63158
 	bool "Broadcom BCM63158 family"
+	select ONLY_GENERIC_GPIO
 	select DM
 	select OF_CONTROL
 	imply CMD_DM
 
 config ARCH_BCM68360
 	bool "Broadcom BCM68360 family"
+	select ONLY_GENERIC_GPIO
 	select DM
 	select OF_CONTROL
 	imply CMD_DM
 
 config ARCH_BCM6858
 	bool "Broadcom BCM6858 family"
+	select ONLY_GENERIC_GPIO
 	select DM
 	select OF_CONTROL
 	imply CMD_DM
@@ -716,6 +724,7 @@  config TARGET_BCMNS2
 config TARGET_BCMNS3
 	bool "Support Broadcom NS3"
 	select ARM64
+	select ONLY_GENERIC_GPIO
 	select BOARD_LATE_INIT
 	help
 	  Support for Broadcom Northstar 3 SoCs. NS3 is a octo-core 64-bit
@@ -784,6 +793,7 @@  config ARCH_KEYSTONE
 
 config ARCH_K3
 	bool "Texas Instruments' K3 Architecture"
+	select ONLY_GENERIC_GPIO
 	select SPL
 	select SUPPORT_SPL
 	select FIT
@@ -929,6 +939,7 @@  config ARCH_OWL
 
 config ARCH_QEMU
 	bool "QEMU Virtual Platform"
+	select ONLY_GENERIC_GPIO
 	select DM
 	select DM_SERIAL
 	select OF_CONTROL
@@ -1061,6 +1072,7 @@  config ARCH_SUNXI
 config ARCH_U8500
 	bool "ST-Ericsson U8500 Series"
 	select CPU_V7A
+	select ONLY_GENERIC_GPIO
 	select DM
 	select DM_GPIO
 	select DM_MMC if MMC
@@ -1203,6 +1215,7 @@  config TARGET_VEXPRESS64_JUNO
 config TARGET_TOTAL_COMPUTE
 	bool "Support Total Compute Platform"
 	select ARM64
+	select ONLY_GENERIC_GPIO
 	select PL01X_SERIAL
 	select DM
 	select DM_SERIAL
@@ -1633,6 +1646,7 @@  config TARGET_COLIBRI_PXA270
 
 config ARCH_UNIPHIER
 	bool "Socionext UniPhier SoCs"
+	select ONLY_GENERIC_GPIO
 	select BOARD_LATE_INIT
 	select DM
 	select DM_ETH
@@ -1670,6 +1684,7 @@  config ARCH_STM32
 
 config ARCH_STI
 	bool "Support STMicrolectronics SoCs"
+	select ONLY_GENERIC_GPIO
 	select BLK
 	select CPU_V7A
 	select DM
@@ -1717,6 +1732,7 @@  config ARCH_STM32MP
 
 config ARCH_ROCKCHIP
 	bool "Support Rockchip SoCs"
+	select ONLY_GENERIC_GPIO
 	select BLK
 	select BINMAN if SPL_OPTEE
 	select DM
@@ -1778,6 +1794,7 @@  config TARGET_THUNDERX_88XX
 
 config ARCH_ASPEED
 	bool "Support Aspeed SoCs"
+	select ONLY_GENERIC_GPIO
 	select DM
 	select OF_CONTROL
 	imply CMD_DM
diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
index 7609367884..bf1f239f81 100644
--- a/arch/arm/include/asm/gpio.h
+++ b/arch/arm/include/asm/gpio.h
@@ -1,10 +1,4 @@ 
-#if !defined(CONFIG_ARCH_UNIPHIER) && !defined(CONFIG_ARCH_STI) && \
-	!defined(CONFIG_ARCH_K3) && !defined(CONFIG_ARCH_BCM68360) && \
-	!defined(CONFIG_ARCH_BCM6858) && !defined(CONFIG_ARCH_BCM63158) && \
-	!defined(CONFIG_ARCH_ROCKCHIP) && !defined(CONFIG_ARCH_ASPEED) && \
-	!defined(CONFIG_ARCH_U8500) && !defined(CONFIG_CORTINA_PLATFORM) && \
-	!defined(CONFIG_TARGET_BCMNS3) && !defined(CONFIG_TARGET_TOTAL_COMPUTE) && \
-	!defined(CONFIG_ARCH_QEMU)
+#if !defined(CONFIG_ONLY_GENERIC_GPIO)
 #include <asm/arch/gpio.h>
 #endif
 #include <asm-generic/gpio.h>
diff --git a/board/cortina/common/Kconfig b/board/cortina/common/Kconfig
index 00c709e70f..3f1be84d21 100644
--- a/board/cortina/common/Kconfig
+++ b/board/cortina/common/Kconfig
@@ -1,6 +1,7 @@ 
 config CORTINA_PLATFORM
        bool "Cortina-Access Platform"
        default y
+       select ONLY_GENERIC_GPIO
        help
          Select this option for Cortina-Access platforms
 	 to enables selection of CAxxxx drivers