On 05/01/2018 09:45, Greentime Hu wrote:

Hi, Daniel:

[ ... ]

quoted

quoted

quoted

quoted

quoted

[ ... ]

quoted

+config CLKSRC_ATCPIT100
+     bool "Clocksource for AE3XX platform"
+     depends on NDS32 || COMPILE_TEST
+     depends on HAS_IOMEM
+     help
+       This option enables support for the Andestech AE3XX platform timers.

Hi Rick,

the general rule for the Kconfig is:

bool "Clocksource for AE3XX platform" if COMPILE_TEST

BTW, select TIMER_OF is missing.

We don't select here because we select TIMER_OF in arch/nds32/Kconfig
I am not sure if I still need to select TIMER_OF here?

Actually, I want the drivers/clocksource/Kconfig to be consistent across
all entries. As TIMER_OF is needed by the driver and nothing else, it
must be selected in the TIMER entry.

As there are a lot of timers and we do the changes little by little,
there are still entries with different format.

It should be something like that:

config ASM9260_TIMER
        bool "ASM9260 timer driver" if COMPILE_TEST
        select CLKSRC_MMIO
        select TIMER_OF
        help
          Enables support for the ASM9260 timer.

Move the select TIMER_OF to the timer option entry.

quoted

quoted

quoted

quoted

quoted

and no deps on the platform.

It is up to the platform Kconfig to select the option.

We want here a silent option but make it selectable in case of
compilation test coverage.

The way we like to use it is because
1. This timer is a basic component to boot an nds32 CPU and it should
be able to select without COMPILE_TEST for nds32 architecture.

Yes, so you don't need it to be selectable, you must select it from the
platform's Kconfig.

I am not sure that I get your point or not.
We don't have a CONFIG_PLAT_AE3XX.
Do you mean we should create one and select CLKSRC_ATCPIT100 under
CONFIG_PLAT_AE3XX?

No. Can't you add in arch/ndis32/Kconfig ?

+select TIMER_ATCPIT100

Like:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/Kconfig#n50

IMHO, it might be a little bit wierd if we select TIMER_ATCPIT100 in
arch/nds32/Kconfig because it is part of SoC instead of CPU.
If we change to another SoC with another timer, we need to select
another TIMER in arch/nds32/Kconfig and delete TIMER_ATCPIT100.
It seems more flexible to be selected in driver layer.

It seems to be the timer is part of the arch to be selected in arch's Kconfig.
arch/arc/Kconfig:       select ARC_TIMERS
arch/arc/Kconfig:       select ARC_TIMERS_64BIT
arch/arm/Kconfig:       select ARM_ARCH_TIMER
arch/arm64/Kconfig:     select ARM_ARCH_TIMER
arch/blackfin/Kconfig:  select BFIN_GPTIMERS

No, the timer must be selected from the arch/soc's or whatever Kconfig.
Not in the clocksource's Kconfig.

eg.

on ARM:

arch/arm/mach-vt8500/Kconfig:   select VT8500_TIMER
arch/arm/mach-bcm/Kconfig:      select BCM_KONA_TIMER
arch/arm/mach-actions/Kconfig:  select OWL_TIMER
arch/arm/mach-digicolor/Kconfig:        select DIGICOLOR_TIMER

etc ...

on ARM64:

arch/arm64/Kconfig.platforms:   select OWL_TIMER
arch/arm64/Kconfig.platforms:       select ARM_TIMER_SP804
arch/arm64/Kconfig.platforms:       select MTK_TIMER

etc ...

Thanks.

  -- Daniel


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog