Thread (14 messages) 14 messages, 4 authors, 2018-01-08

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

From: Arnd Bergmann <arnd@arndb.de>
Date: 2018-01-08 16:30:19
Also in: linux-arch, linux-devicetree, linux-serial, lkml

On Mon, Jan 8, 2018 at 5:08 PM, Daniel Lezcano
[off-list ref] wrote:
On 08/01/2018 16:26, Arnd Bergmann wrote:
quoted
On Fri, Jan 5, 2018 at 10:31 AM, Daniel Lezcano
[off-list ref] wrote:
quoted
quoted
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.
The point is that there is no platform specific Kconfig that could select
it, as the concept doesn't make a lot of sense here.

If you require the driver to always be selected by the
architecture code, it adds a little bit of bloat on systems
that don't need it. This is possible, but I think it's preferable
to give users a way of tuning a kernel for a particular chip.
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.
I'm not worried about the driver bloating the kernel here when it
isn't disabled, this is no different from enabling the USB controller
by default for a board that doesn't have a USB connector, or
enabling ten different pinctrl drivers for all members of a chip
family even though you know which particular chip you are
running on.
quoted
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.
Having a dependency on CONFIG_EXPERT means that a more
users will end up having to set that for a shipping system. It's
something we can do (even without a special Kconfig keyword),
but it should be used carefully for stuff that 99% of the users want
to enable.

Why not just:

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

That way, it's simply enabled on NDS32 by default, but just as easy
to disable for systems that don't need it.

      Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help