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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 8 Jan 2018 16:26:30 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     Greentime Hu <green.hu@...il.com>,
        Greentime <greentime@...estech.com>,
        Rick Chen <rickchen36@...il.com>,
        Rick Chen <rick@...estech.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <marc.zyngier@....com>,
        Rob Herring <robh+dt@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        Vincent Chen <deanbo422@...il.com>,
        DTML <devicetree@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        David Howells <dhowells@...hat.com>,
        Will Deacon <will.deacon@....com>, linux-serial@...r.kernel.org
Subject: Re: [PATCH v5 1/3] clocksource/drivers/atcpit100: Add andestech
 atcpit100 timer

On Fri, Jan 5, 2018 at 10:31 AM, Daniel Lezcano
<daniel.lezcano@...aro.org> 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.

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.

      Arnd