On 05/01/2018 09:45, Greentime Hu wrote: > Hi, Daniel: [ ... ] >>>>>> [ ... ] >>>>>> >>>>>>> +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. >>>>>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html