| Message ID | 20180307162430.2664523-2-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] ARM: npcm: add CONFIG_ARCH_MULTI_V7 dependency | expand |
On Thu, Mar 8, 2018 at 2:54 AM, Arnd Bergmann <arnd@arndb.de> wrote: > While looking at the build regression, I noticed that the > platform selects a lot of other Kconfig symbols that it really > should not: > > CPU_V7, ARM_GIC, HAVE_SMP, COMMON_CLK, GENERIC_CLOCKEVENTS, > and CLKDEV_LOOKUP are all implied by ARCH_MULTI_V7, so they > can be dropped. > > CACHE_L2X0, SMP and USB are meant to be user-selectable, we > want to be able to turn those off for testing purposes. > > CPU_USE_DOMAINS looks completely misplaced here, we should not > select that for an ARMv7 platform. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> I had a similar patch queued for sending out. Thanks Arnd. Acked-by: Joel Stanley <joel@jms.id.au> Cheers, Joel > --- > arch/arm/mach-npcm/Kconfig | 18 ------------------ > 1 file changed, 18 deletions(-) > > diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig > index 2bc6697c8d97..c6a16230e8ef 100644 > --- a/arch/arm/mach-npcm/Kconfig > +++ b/arch/arm/mach-npcm/Kconfig > @@ -12,12 +12,6 @@ comment "NPCM7XX CPU type" > config ARCH_NPCM750 > depends on ARCH_NPCM > bool "Support for NPCM750 BMC CPU (Poleg)" > - select CACHE_L2X0 > - select CPU_V7 > - select ARM_GIC > - select HAVE_SMP > - select SMP > - select SMP_ON_UP > select HAVE_ARM_SCU > select HAVE_ARM_TWD if SMP > select ARM_ERRATA_720789 > @@ -26,18 +20,6 @@ config ARCH_NPCM750 > select ARM_ERRATA_794072 > select PL310_ERRATA_588369 > select PL310_ERRATA_727915 > - select USB_EHCI_ROOT_HUB_TT > - select USB_ARCH_HAS_HCD > - select USB_ARCH_HAS_EHCI > - select USB_EHCI_HCD > - select USB_ARCH_HAS_OHCI > - select USB_OHCI_HCD > - select USB > - select FIQ > - select CPU_USE_DOMAINS > - select GENERIC_CLOCKEVENTS > - select CLKDEV_LOOKUP > - select COMMON_CLK if OF > select NPCM750_TIMER > select MFD_SYSCON > help > -- > 2.9.0 >
So at least replace CACHE_L2X0 with MIGHT_HAVE_CACHE_L2X0 then CACHE_L2X0 will be selected by default and user can still remove it. On Thu, Mar 8, 2018 at 4:12 AM, Joel Stanley <joel@jms.id.au> wrote: > On Thu, Mar 8, 2018 at 2:54 AM, Arnd Bergmann <arnd@arndb.de> wrote: >> While looking at the build regression, I noticed that the >> platform selects a lot of other Kconfig symbols that it really >> should not: >> >> CPU_V7, ARM_GIC, HAVE_SMP, COMMON_CLK, GENERIC_CLOCKEVENTS, >> and CLKDEV_LOOKUP are all implied by ARCH_MULTI_V7, so they >> can be dropped. >> >> CACHE_L2X0, SMP and USB are meant to be user-selectable, we >> want to be able to turn those off for testing purposes. >> >> CPU_USE_DOMAINS looks completely misplaced here, we should not >> select that for an ARMv7 platform. >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > I had a similar patch queued for sending out. Thanks Arnd. > > Acked-by: Joel Stanley <joel@jms.id.au> > > Cheers, > > Joel > >> --- >> arch/arm/mach-npcm/Kconfig | 18 ------------------ >> 1 file changed, 18 deletions(-) >> >> diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig >> index 2bc6697c8d97..c6a16230e8ef 100644 >> --- a/arch/arm/mach-npcm/Kconfig >> +++ b/arch/arm/mach-npcm/Kconfig >> @@ -12,12 +12,6 @@ comment "NPCM7XX CPU type" >> config ARCH_NPCM750 >> depends on ARCH_NPCM >> bool "Support for NPCM750 BMC CPU (Poleg)" >> - select CACHE_L2X0 >> - select CPU_V7 >> - select ARM_GIC >> - select HAVE_SMP >> - select SMP >> - select SMP_ON_UP >> select HAVE_ARM_SCU >> select HAVE_ARM_TWD if SMP >> select ARM_ERRATA_720789 >> @@ -26,18 +20,6 @@ config ARCH_NPCM750 >> select ARM_ERRATA_794072 >> select PL310_ERRATA_588369 >> select PL310_ERRATA_727915 >> - select USB_EHCI_ROOT_HUB_TT >> - select USB_ARCH_HAS_HCD >> - select USB_ARCH_HAS_EHCI >> - select USB_EHCI_HCD >> - select USB_ARCH_HAS_OHCI >> - select USB_OHCI_HCD >> - select USB >> - select FIQ >> - select CPU_USE_DOMAINS >> - select GENERIC_CLOCKEVENTS >> - select CLKDEV_LOOKUP >> - select COMMON_CLK if OF >> select NPCM750_TIMER >> select MFD_SYSCON >> help >> -- >> 2.9.0 >>
On Thu, Mar 8, 2018 at 11:46 AM, Avi Fishman <avifishman70@gmail.com> wrote: > So at least replace CACHE_L2X0 with MIGHT_HAVE_CACHE_L2X0 then > CACHE_L2X0 will be selected by default and user can still remove it. I looked at the other platforms for this one again, and found that most simply select CONFIG_CACHE_L2X0 directly, so we probably want the same here after all. The ones that use MIGHT_HAVE_CACHE_L2X0 are the platforms that have multiple SoCs in one family, and only some of them have an L2X0. Is that the case here as well? Usually, the L2X0 is used with Cortex-A9 and A5, rarely on older cores, and never on newer ones. Arnd
Hi Amd,
Sorry for the delayed reply,
the NPCM7xx is a family of BMC's that include NPCM750, NPCM730 and more
All of the NPCM7xx BMC's have the same basic modules like Cortex-A9,
WDT, timers and more
can we reflacte the NPCM7xx as follow in the Kconfig:
menuconfig ARCH_NPCM
bool "Nuvoton NPCM Architecture"
depends on ARCH_MULTI_V7
select USE_OF
select PINCTRL
if ARCH_NPCM
config ARCH_NPCM7XX
bool "Support for NPCM7xx BMC (Poleg)"
depends on ARCH_MULTI_V7
select HAVE_ARM_SCU
select PINCTRL_NPCM7XX
select NPCM7XX_TIMER
select ARCH_REQUIRE_GPIOLIB
select ARM_GIC
select ARM_ERRATA_720789
select ARM_ERRATA_754322
select ARM_ERRATA_794072
select PL310_ERRATA_588369
select PL310_ERRATA_727915
select MFD_SYSCON
help
General support for NPCM7xx BMC (Poleg).
Nuvoton NPCM7xx BMC based on the Cortex A9.
if ARCH_NPCM7XX
config ARCH_NPCM750
bool "NPCM750 BMC support with Device Tree"
select HAVE_ARM_TWD if SMP
help
General support for NPCM750 BMC (Poleg).
Nuvoton NPCM750 BMC based on the Cortex A9.
config ARCH_NPCM730
bool "NPCM730 BMC support with Device Tree"
select HAVE_ARM_TWD if SMP
select ARM_ERRATA_764369 if SMP
help
General support for NPCM730 BMC (Poleg).
Nuvoton NPCM730 BMC based on the Cortex A9.
endif #ARCH_NPCM7XX
endif
On 7 March 2018 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
> While looking at the build regression, I noticed that the
> platform selects a lot of other Kconfig symbols that it really
> should not:
>
> CPU_V7, ARM_GIC, HAVE_SMP, COMMON_CLK, GENERIC_CLOCKEVENTS,
> and CLKDEV_LOOKUP are all implied by ARCH_MULTI_V7, so they
> can be dropped.
>
> CACHE_L2X0, SMP and USB are meant to be user-selectable, we
> want to be able to turn those off for testing purposes.
>
> CPU_USE_DOMAINS looks completely misplaced here, we should not
> select that for an ARMv7 platform.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/mach-npcm/Kconfig | 18 ------------------
> 1 file changed, 18 deletions(-)
>
> diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig
> index 2bc6697c8d97..c6a16230e8ef 100644
> --- a/arch/arm/mach-npcm/Kconfig
> +++ b/arch/arm/mach-npcm/Kconfig
> @@ -12,12 +12,6 @@ comment "NPCM7XX CPU type"
> config ARCH_NPCM750
> depends on ARCH_NPCM
> bool "Support for NPCM750 BMC CPU (Poleg)"
> - select CACHE_L2X0
> - select CPU_V7
> - select ARM_GIC
> - select HAVE_SMP
> - select SMP
> - select SMP_ON_UP
> select HAVE_ARM_SCU
> select HAVE_ARM_TWD if SMP
> select ARM_ERRATA_720789
> @@ -26,18 +20,6 @@ config ARCH_NPCM750
> select ARM_ERRATA_794072
> select PL310_ERRATA_588369
> select PL310_ERRATA_727915
> - select USB_EHCI_ROOT_HUB_TT
> - select USB_ARCH_HAS_HCD
> - select USB_ARCH_HAS_EHCI
> - select USB_EHCI_HCD
> - select USB_ARCH_HAS_OHCI
> - select USB_OHCI_HCD
> - select USB
> - select FIQ
> - select CPU_USE_DOMAINS
> - select GENERIC_CLOCKEVENTS
> - select CLKDEV_LOOKUP
> - select COMMON_CLK if OF
> select NPCM750_TIMER
> select MFD_SYSCON
> help
> --
> 2.9.0
>
Hi Arnd,
Please discard my previous mail, (I have more things to ask... please
read this mail)
Sorry for the delayed reply,
Before entering the long mail (sorry about it) just a technical thing
I think we should add ARM_GIC configuration, It is not implied in ARCH_MULTI_V7.
Now for the real story :-)
The NPCM7xx is a family of BMC's that include NPCM750, NPCM730 and more
All of the NPCM7xx BMC's have the same basic modules like Cortex-A9,
WDT, timers, etc - must included shared modules, and there are a
optional modules that can be added (for example the USB is not in
NPCM730)
so the additional optional modules give us the differences between the
NPCM7xx BMC's
I will like to use the same method that SPEAR using
(arch/arm/mach-spear/Kconfig)
can we reflacte the NPCM7xx as follow in the Kconfig:
menuconfig ARCH_NPCM
bool "Nuvoton NPCM Architecture"
depends on ARCH_MULTI_V7
select USE_OF
select PINCTRL
if ARCH_NPCM
config ARCH_NPCM7XX
bool "Support for NPCM7xx BMC (Poleg)"
depends on ARCH_MULTI_V7
select CACHE_L2X0
select PINCTRL_NPCM7XX
select NPCM7XX_TIMER
select ARCH_REQUIRE_GPIOLIB
select ARM_GIC
select ARM_ERRATA_720789
select ARM_ERRATA_754322
select ARM_ERRATA_794072
select PL310_ERRATA_588369
select PL310_ERRATA_727915
select MFD_SYSCON
help
General support for NPCM7xx BMC (Poleg).
Nuvoton NPCM7xx BMC based on the Cortex A9.
if ARCH_NPCM7XX
config ARCH_NPCM750
bool "NPCM750 BMC support with Device Tree"
select HAVE_ARM_TWD if SMP
select HAVE_ARM_SCU if SMP
select ARM_ERRATA_764369 if SMP
help
General support for NPCM750 BMC (Poleg).
Nuvoton NPCM750 BMC based on the Cortex A9.
config ARCH_NPCM730
bool "NPCM730 BMC support with Device Tree"
select HAVE_ARM_TWD if SMP
select HAVE_ARM_SCU if SMP
select ARM_ERRATA_764369 if SMP
help
General support for NPCM730 BMC (Poleg).
Nuvoton NPCM730 BMC based on the Cortex A9.
endif #ARCH_NPCM7XX
endif
if we use the above approach can we unite the two sections into one
(since they are the same) as follows:
config ARCH_NPCM750_NPCM730
bool "NPCM750 or NPCM730 BMC support with Device Tree"
select HAVE_ARM_TWD if SMP
select HAVE_ARM_SCU if SMP
select ARM_ERRATA_764369 if SMP
help
General support for NPCM750 or NPCM730 BMC (Poleg).
Nuvoton NPCM750 or NPCM730 BMC based on the Cortex A9.
or even better include it in ARCH_NPCM7XX and remove the ARCH_NPCM750
and ARCH_NPCM730?
Regarding another matter, defconfig file:
We would like to consult how to describe the various chips (e.g.
NPCM750, NPCM730) in the defconfig file.
One option is to include all modules (as included in NPCM750 chip,
which is a superset chip) and all the chips.
The we call it npcm7xx_defconfig (same as spear13xx_deconfig) .
In this option customers of NPCM730 will need to manually undef
non-existing modules.
Another option is to create different defconfig files for each chip of
this NPCM7xx family, e.g. npcm750_defconfig and npcm730_defconfig.
Which approach should be adopt ? We will appreciate your advise.
Thanks a lot,
Tomer
On 12 March 2018 at 12:16, Tomer Maimon <tmaimon77@gmail.com> wrote:
> Hi Amd,
>
> Sorry for the delayed reply,
>
> the NPCM7xx is a family of BMC's that include NPCM750, NPCM730 and more
>
> All of the NPCM7xx BMC's have the same basic modules like Cortex-A9,
> WDT, timers and more
>
> can we reflacte the NPCM7xx as follow in the Kconfig:
>
> menuconfig ARCH_NPCM
> bool "Nuvoton NPCM Architecture"
> depends on ARCH_MULTI_V7
> select USE_OF
> select PINCTRL
>
> if ARCH_NPCM
>
> config ARCH_NPCM7XX
> bool "Support for NPCM7xx BMC (Poleg)"
> depends on ARCH_MULTI_V7
> select HAVE_ARM_SCU
> select PINCTRL_NPCM7XX
> select NPCM7XX_TIMER
> select ARCH_REQUIRE_GPIOLIB
> select ARM_GIC
> select ARM_ERRATA_720789
> select ARM_ERRATA_754322
> select ARM_ERRATA_794072
> select PL310_ERRATA_588369
> select PL310_ERRATA_727915
> select MFD_SYSCON
> help
> General support for NPCM7xx BMC (Poleg).
>
> Nuvoton NPCM7xx BMC based on the Cortex A9.
>
> if ARCH_NPCM7XX
>
> config ARCH_NPCM750
> bool "NPCM750 BMC support with Device Tree"
> select HAVE_ARM_TWD if SMP
> help
> General support for NPCM750 BMC (Poleg).
>
> Nuvoton NPCM750 BMC based on the Cortex A9.
>
> config ARCH_NPCM730
> bool "NPCM730 BMC support with Device Tree"
> select HAVE_ARM_TWD if SMP
> select ARM_ERRATA_764369 if SMP
> help
> General support for NPCM730 BMC (Poleg).
>
> Nuvoton NPCM730 BMC based on the Cortex A9.
>
> endif #ARCH_NPCM7XX
>
> endif
>
> On 7 March 2018 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
>> While looking at the build regression, I noticed that the
>> platform selects a lot of other Kconfig symbols that it really
>> should not:
>>
>> CPU_V7, ARM_GIC, HAVE_SMP, COMMON_CLK, GENERIC_CLOCKEVENTS,
>> and CLKDEV_LOOKUP are all implied by ARCH_MULTI_V7, so they
>> can be dropped.
>>
>> CACHE_L2X0, SMP and USB are meant to be user-selectable, we
>> want to be able to turn those off for testing purposes.
>>
>> CPU_USE_DOMAINS looks completely misplaced here, we should not
>> select that for an ARMv7 platform.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> arch/arm/mach-npcm/Kconfig | 18 ------------------
>> 1 file changed, 18 deletions(-)
>>
>> diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig
>> index 2bc6697c8d97..c6a16230e8ef 100644
>> --- a/arch/arm/mach-npcm/Kconfig
>> +++ b/arch/arm/mach-npcm/Kconfig
>> @@ -12,12 +12,6 @@ comment "NPCM7XX CPU type"
>> config ARCH_NPCM750
>> depends on ARCH_NPCM
>> bool "Support for NPCM750 BMC CPU (Poleg)"
>> - select CACHE_L2X0
>> - select CPU_V7
>> - select ARM_GIC
>> - select HAVE_SMP
>> - select SMP
>> - select SMP_ON_UP
>> select HAVE_ARM_SCU
>> select HAVE_ARM_TWD if SMP
>> select ARM_ERRATA_720789
>> @@ -26,18 +20,6 @@ config ARCH_NPCM750
>> select ARM_ERRATA_794072
>> select PL310_ERRATA_588369
>> select PL310_ERRATA_727915
>> - select USB_EHCI_ROOT_HUB_TT
>> - select USB_ARCH_HAS_HCD
>> - select USB_ARCH_HAS_EHCI
>> - select USB_EHCI_HCD
>> - select USB_ARCH_HAS_OHCI
>> - select USB_OHCI_HCD
>> - select USB
>> - select FIQ
>> - select CPU_USE_DOMAINS
>> - select GENERIC_CLOCKEVENTS
>> - select CLKDEV_LOOKUP
>> - select COMMON_CLK if OF
>> select NPCM750_TIMER
>> select MFD_SYSCON
>> help
>> --
>> 2.9.0
>>
On Mon, Mar 12, 2018 at 12:43 PM, Tomer Maimon <tmaimon77@gmail.com> wrote: > Before entering the long mail (sorry about it) just a technical thing > I think we should add ARM_GIC configuration, It is not implied in ARCH_MULTI_V7. Yes, you are right. > Now for the real story :-) > > The NPCM7xx is a family of BMC's that include NPCM750, NPCM730 and more > > All of the NPCM7xx BMC's have the same basic modules like Cortex-A9, > WDT, timers, etc - must included shared modules, and there are a > optional modules that can be added (for example the USB is not in > NPCM730) > so the additional optional modules give us the differences between the > NPCM7xx BMC's > > I will like to use the same method that SPEAR using > (arch/arm/mach-spear/Kconfig) > > can we reflacte the NPCM7xx as follow in the Kconfig: > > menuconfig ARCH_NPCM > bool "Nuvoton NPCM Architecture" > depends on ARCH_MULTI_V7 > select USE_OF > select PINCTRL selecting pinctrl is fine, USE_OF is implied. > if ARCH_NPCM > > config ARCH_NPCM7XX > bool "Support for NPCM7xx BMC (Poleg)" > depends on ARCH_MULTI_V7 > select CACHE_L2X0 > select PINCTRL_NPCM7XX > select NPCM7XX_TIMER > select ARCH_REQUIRE_GPIOLIB > select ARM_GIC > select ARM_ERRATA_720789 > select ARM_ERRATA_754322 > select ARM_ERRATA_794072 > select PL310_ERRATA_588369 > select PL310_ERRATA_727915 > select MFD_SYSCON > help > General support for NPCM7xx BMC (Poleg). > > Nuvoton NPCM7xx BMC based on the Cortex A9. This seems fine. > config ARCH_NPCM750_NPCM730 > bool "NPCM750 or NPCM730 BMC support with Device Tree" > select HAVE_ARM_TWD if SMP > select HAVE_ARM_SCU if SMP > select ARM_ERRATA_764369 if SMP > help > General support for NPCM750 or NPCM730 BMC (Poleg). > > Nuvoton NPCM750 or NPCM730 BMC based on the Cortex A9. > > or even better include it in ARCH_NPCM7XX and remove the ARCH_NPCM750 > and ARCH_NPCM730? Yes, I think that would be best. > Regarding another matter, defconfig file: > We would like to consult how to describe the various chips (e.g. > NPCM750, NPCM730) in the defconfig file. > One option is to include all modules (as included in NPCM750 chip, > which is a superset chip) and all the chips. > The we call it npcm7xx_defconfig (same as spear13xx_deconfig) . > In this option customers of NPCM730 will need to manually undef > non-existing modules. > > Another option is to create different defconfig files for each chip of > this NPCM7xx family, e.g. npcm750_defconfig and npcm730_defconfig. > > Which approach should be adopt ? We will appreciate your advise. We don't want one defconfig per chip. Some platform maintainers don't have any defconfig for their platforms at all, but simply rely on multi_v7_defconfig for testing, which is fine as well. If you have an extra defconfig file, make sure to always update both your own file and the multi_v7_defconfig together when you add a new driver that should be enabled. For multi_v7_defconfig, make all non-essential drivers loadable modules. Arnd
diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig index 2bc6697c8d97..c6a16230e8ef 100644 --- a/arch/arm/mach-npcm/Kconfig +++ b/arch/arm/mach-npcm/Kconfig @@ -12,12 +12,6 @@ comment "NPCM7XX CPU type" config ARCH_NPCM750 depends on ARCH_NPCM bool "Support for NPCM750 BMC CPU (Poleg)" - select CACHE_L2X0 - select CPU_V7 - select ARM_GIC - select HAVE_SMP - select SMP - select SMP_ON_UP select HAVE_ARM_SCU select HAVE_ARM_TWD if SMP select ARM_ERRATA_720789 @@ -26,18 +20,6 @@ config ARCH_NPCM750 select ARM_ERRATA_794072 select PL310_ERRATA_588369 select PL310_ERRATA_727915 - select USB_EHCI_ROOT_HUB_TT - select USB_ARCH_HAS_HCD - select USB_ARCH_HAS_EHCI - select USB_EHCI_HCD - select USB_ARCH_HAS_OHCI - select USB_OHCI_HCD - select USB - select FIQ - select CPU_USE_DOMAINS - select GENERIC_CLOCKEVENTS - select CLKDEV_LOOKUP - select COMMON_CLK if OF select NPCM750_TIMER select MFD_SYSCON help
While looking at the build regression, I noticed that the platform selects a lot of other Kconfig symbols that it really should not: CPU_V7, ARM_GIC, HAVE_SMP, COMMON_CLK, GENERIC_CLOCKEVENTS, and CLKDEV_LOOKUP are all implied by ARCH_MULTI_V7, so they can be dropped. CACHE_L2X0, SMP and USB are meant to be user-selectable, we want to be able to turn those off for testing purposes. CPU_USE_DOMAINS looks completely misplaced here, we should not select that for an ARMv7 platform. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/arm/mach-npcm/Kconfig | 18 ------------------ 1 file changed, 18 deletions(-) -- 2.9.0