diff mbox

[v2,03/12] ARM: hisi: add ARCH_HISI

Message ID 1396944052-9887-4-git-send-email-haojian.zhuang@linaro.org
State Superseded
Headers show

Commit Message

Haojian Zhuang April 8, 2014, 8 a.m. UTC
Since multiple ARCH configuration will be appended into mach-hisi
directory, add ARCH_HISI as common configuration for different ARCH in
mach-hisi.

Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 arch/arm/Makefile          |  2 +-
 arch/arm/mach-hisi/Kconfig | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

Comments

Arnd Bergmann April 8, 2014, 11:02 a.m. UTC | #1
On Tuesday 08 April 2014 16:00:43 Haojian Zhuang wrote:
> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
> index feee4db..da16efd 100644
> --- a/arch/arm/mach-hisi/Kconfig
> +++ b/arch/arm/mach-hisi/Kconfig
> @@ -1,8 +1,16 @@
> -config ARCH_HI3xxx
> -       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
> +config ARCH_HISI
> +       bool "Hisilicon SoC Support"
> +       depends on ARCH_MULTIPLATFORM
>         select ARM_AMBA
>         select ARM_GIC
>         select ARM_TIMER_SP804
> +
> +if ARCH_HISI
> +
> +menu "Hisilicon platform type"
> +
> +config ARCH_HI3xxx
> +       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
>         select CACHE_L2X0
>         select HAVE_ARM_SCU if SMP
>         select HAVE_ARM_TWD if SMP
> @@ -10,3 +18,7 @@ config ARCH_HI3xxx
>         select PINCTRL_SINGLE
>         help
>           Support for Hisilicon Hi36xx/Hi37xx processor family
> +
> +endmenu
> +
> +endif

I'd rather keep them as separate top-level options. The two platforms
are not really related technically, other than being designed by the
same company. Also, I'd prefer the use of 'menuconfig' here, like:

menuconfig ARCH_HI3xxx
	bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7

if ARCH_HI3xxx

...

endif


menuconfig ARCH_HIP04
	bool "Hisilicon HiP04 family" if ARCH_MULTI_V7_LPAE

if ARCH_HIP04

...

endif


	Arnd
Arnd Bergmann April 8, 2014, 11:13 a.m. UTC | #2
On Tuesday 08 April 2014 16:00:43 Haojian Zhuang wrote:
> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
> index feee4db..da16efd 100644
> --- a/arch/arm/mach-hisi/Kconfig
> +++ b/arch/arm/mach-hisi/Kconfig
> @@ -1,8 +1,16 @@
> -config ARCH_HI3xxx
> -       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
> +config ARCH_HISI
> +       bool "Hisilicon SoC Support"
> +       depends on ARCH_MULTIPLATFORM
>         select ARM_AMBA
>         select ARM_GIC
>         select ARM_TIMER_SP804
> +
> +if ARCH_HISI
> +
> +menu "Hisilicon platform type"
> +
> +config ARCH_HI3xxx
> +       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
>         select CACHE_L2X0
>         select HAVE_ARM_SCU if SMP
>         select HAVE_ARM_TWD if SMP
> @@ -10,3 +18,7 @@ config ARCH_HI3xxx
>         select PINCTRL_SINGLE
>         help
>           Support for Hisilicon Hi36xx/Hi37xx processor family
> +
> +endmenu

On second thought: You don't actually have any sub-options under
the main SoC specific option, so using 'menuconfig' as I suggested
first makes no sense.

Just leave ARCH_HI3xxx and ARCH_HIP04 as top-level options and
drop this patch. There is no real need for a new menu here.
That also means you can drop patch 9.

	Arnd
Haojian Zhuang April 14, 2014, 7:57 a.m. UTC | #3
On 8 April 2014 19:13, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 08 April 2014 16:00:43 Haojian Zhuang wrote:
>> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
>> index feee4db..da16efd 100644
>> --- a/arch/arm/mach-hisi/Kconfig
>> +++ b/arch/arm/mach-hisi/Kconfig
>> @@ -1,8 +1,16 @@
>> -config ARCH_HI3xxx
>> -       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
>> +config ARCH_HISI
>> +       bool "Hisilicon SoC Support"
>> +       depends on ARCH_MULTIPLATFORM
>>         select ARM_AMBA
>>         select ARM_GIC
>>         select ARM_TIMER_SP804
>> +
>> +if ARCH_HISI
>> +
>> +menu "Hisilicon platform type"
>> +
>> +config ARCH_HI3xxx
>> +       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
>>         select CACHE_L2X0
>>         select HAVE_ARM_SCU if SMP
>>         select HAVE_ARM_TWD if SMP
>> @@ -10,3 +18,7 @@ config ARCH_HI3xxx
>>         select PINCTRL_SINGLE
>>         help
>>           Support for Hisilicon Hi36xx/Hi37xx processor family
>> +
>> +endmenu
>
> On second thought: You don't actually have any sub-options under
> the main SoC specific option, so using 'menuconfig' as I suggested
> first makes no sense.
>
> Just leave ARCH_HI3xxx and ARCH_HIP04 as top-level options and
> drop this patch. There is no real need for a new menu here.
> That also means you can drop patch 9.
>
>         Arnd

Since they're two archs. In arch/arm/Kconfig, I have to write as below.

machine-$(CONFIG_ARCH_HI3xxx)            += mach-hisi
machine-$(CONFIG_ARCH_HIP04)             += mach-hisi

Then mach-hisi is duplicated in Kconfig. I'll get build error in below.

Makefile:877: target `arch/arm/mach-hisi' given more than once in the same rule.

Regards
Haojian
Arnd Bergmann April 14, 2014, 9:10 a.m. UTC | #4
On Monday 14 April 2014 15:57:08 Haojian Zhuang wrote:
> On 8 April 2014 19:13, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 08 April 2014 16:00:43 Haojian Zhuang wrote:
> >> diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
> >> index feee4db..da16efd 100644
> >> --- a/arch/arm/mach-hisi/Kconfig
> >> +++ b/arch/arm/mach-hisi/Kconfig
> >> @@ -1,8 +1,16 @@
> >> -config ARCH_HI3xxx
> >> -       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
> >> +config ARCH_HISI
> >> +       bool "Hisilicon SoC Support"
> >> +       depends on ARCH_MULTIPLATFORM
> >>         select ARM_AMBA
> >>         select ARM_GIC
> >>         select ARM_TIMER_SP804
> >> +
> >> +if ARCH_HISI
> >> +
> >> +menu "Hisilicon platform type"
> >> +
> >> +config ARCH_HI3xxx
> >> +       bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
> >>         select CACHE_L2X0
> >>         select HAVE_ARM_SCU if SMP
> >>         select HAVE_ARM_TWD if SMP
> >> @@ -10,3 +18,7 @@ config ARCH_HI3xxx
> >>         select PINCTRL_SINGLE
> >>         help
> >>           Support for Hisilicon Hi36xx/Hi37xx processor family
> >> +
> >> +endmenu
> >
> > On second thought: You don't actually have any sub-options under
> > the main SoC specific option, so using 'menuconfig' as I suggested
> > first makes no sense.
> >
> > Just leave ARCH_HI3xxx and ARCH_HIP04 as top-level options and
> > drop this patch. There is no real need for a new menu here.
> > That also means you can drop patch 9.
> >
> >         Arnd
> 
> Since they're two archs. In arch/arm/Kconfig, I have to write as below.
> 
> machine-$(CONFIG_ARCH_HI3xxx)            += mach-hisi
> machine-$(CONFIG_ARCH_HIP04)             += mach-hisi
> 
> Then mach-hisi is duplicated in Kconfig. I'll get build error in below.
> 
> Makefile:877: target `arch/arm/mach-hisi' given more than once in the same rule.

Ah, you are right.

You can use a top-level option then, but please use 'menuconfig for it'
Alternatively, you could make ARCH_HISI a hidden symbol like

config ARCH_HISI
	def_bool ARCH_HI3xxx || ARCH_HIP04

and use that one in the Makefile.

	Arnd
diff mbox

Patch

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 41c1931..4c2798a 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -154,7 +154,7 @@  machine-$(CONFIG_ARCH_EP93XX)		+= ep93xx
 machine-$(CONFIG_ARCH_EXYNOS)		+= exynos
 machine-$(CONFIG_ARCH_GEMINI)		+= gemini
 machine-$(CONFIG_ARCH_HIGHBANK)		+= highbank
-machine-$(CONFIG_ARCH_HI3xxx)		+= hisi
+machine-$(CONFIG_ARCH_HISI)		+= hisi
 machine-$(CONFIG_ARCH_INTEGRATOR)	+= integrator
 machine-$(CONFIG_ARCH_IOP13XX)		+= iop13xx
 machine-$(CONFIG_ARCH_IOP32X)		+= iop32x
diff --git a/arch/arm/mach-hisi/Kconfig b/arch/arm/mach-hisi/Kconfig
index feee4db..da16efd 100644
--- a/arch/arm/mach-hisi/Kconfig
+++ b/arch/arm/mach-hisi/Kconfig
@@ -1,8 +1,16 @@ 
-config ARCH_HI3xxx
-	bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
+config ARCH_HISI
+	bool "Hisilicon SoC Support"
+	depends on ARCH_MULTIPLATFORM
 	select ARM_AMBA
 	select ARM_GIC
 	select ARM_TIMER_SP804
+
+if ARCH_HISI
+
+menu "Hisilicon platform type"
+
+config ARCH_HI3xxx
+	bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7
 	select CACHE_L2X0
 	select HAVE_ARM_SCU if SMP
 	select HAVE_ARM_TWD if SMP
@@ -10,3 +18,7 @@  config ARCH_HI3xxx
 	select PINCTRL_SINGLE
 	help
 	  Support for Hisilicon Hi36xx/Hi37xx processor family
+
+endmenu
+
+endif