Re: [PATCH v5 1/3] clocksource/drivers/atcpit100: Add andestech atcpit100 timer

[Daniel Lezcano]

On 08/01/2018 16:26, Arnd Bergmann wrote:
> On Fri, Jan 5, 2018 at 10:31 AM, Daniel Lezcano
> <daniel.lezcano@xxxxxxxxxx> wrote:
>>>> 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 ...
> 
> I'd actually prefer to not do it for ARM either: Most other subsystems
> don't do that, and I don't see a strong reason why clocksource should
> be special here.

The majority doing the opposite does not mean it is right.

Do you know which clock belongs to which board ? Who will unselect a
clock ? I'm pretty sure nobody. Everyone relies on the platform Kconfig
and expect it to select the right drivers.

We don't expect the hackers to have a deep knowledge of the hardware and
the driver dependencies. It is very convenient to not care about that
and let the platform's Kconfig to select the right drivers.

And that is the behavior I would like to keep.

> Selecting 'TIMER_OF' from the individual drivers that need it (as you
> suggest) makes sense, but I think for ARM we treat SoC families
> as a bit too special, in the end they are for the most part collections
> of individual hardware blocks that may or may not be present on
> some chip.
> 
> In case of risc-v and nds32, I expect that the separation will be
> even less visibile in the hardware, as a typical model here is
> that one company designs SoCs for multiple customers that each
> have different requirements. Some of them may have one
> timer and some have another timer or multiple timers, but there
> is no strict separation between SoC families as I understand.
> Here we'd be better off not having a per-SoC Kconfig option at
> all, just a generic defconfig that enables all the drivers that might
> be used, and integrators can have a defconfig file that only
> enables the stuff they actually use on a given chip.

Yes, the result is the same, the option is not showed in the menu.

However, I can understand it could be interesting to have the ability to
unselect a driver for experts.

I'm wondering if we can create a bool_expert which shows up only when
CONFIG_EXPERT is set.




-- 
 <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