[PATCH v2 00/03] clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits

[PATCH v2 00/03] clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits

[Magnus Damm]

clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits (v2)

[PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI
[PATCH v2 02/03] ARM: shmobile: Remove CMT, TMU and STI Kconfig entries
[PATCH v2 03/03] sh: Remove Kconfig entries for TMU, CMT and MTU2

This is V2 of the SH and ARM mach-shmobile clocksource Kconfig
consolidation series.

By sharing Kconfig entires in drivers/clocksource/Kconfig we
can get rid of duplicated bits that do not belong under arch/.

With this series applied it is possible to build the timer
drivers for CMT, MTU2, TMU and STI in case of SH or ARM
or when COMPILE_TEST is selected.

Since last time this has been broken out into 3 separate patches,
where [PATCH v2 01/03] needs to go first, and the other patches
can be merged any time after without any build or configuration
issues.

Thanks to Olof and Simon for feedback! If I interpreted your
ideas wrong please let me know and I will update.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 Written against renesas.git tag renesas-devel-v3.12-rc7-20131101

 arch/arm/mach-shmobile/Kconfig |   21 ---------------------
 arch/sh/Kconfig                |   21 ---------------------
 drivers/clocksource/Kconfig    |   30 ++++++++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 42 deletions(-)

[PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Add Kconfig entries for CMT, MTU2, TMU and STI to
drivers/clocksource/Kconfig. This will allow us to
get rid of duplicated entires in architecture code
such as arch/sh and arch/arm/mach-shmobile.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 drivers/clocksource/Kconfig |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

--- 0001/drivers/clocksource/Kconfig
+++ work/drivers/clocksource/Kconfig	2013-11-05 19:49:47.000000000 +0900
@@ -110,3 +110,33 @@ config VF_PIT_TIMER
 	bool
 	help
 	  Support for Period Interrupt Timer on Freescale Vybrid Family SoCs.
+
+if COMPILE_TEST || ARM || SUPERH
+menu "Timer drivers"
+
+config SH_TIMER_CMT
+	bool "CMT timer driver"
+	default y if ARM || (SUPERH && SYS_SUPPORTS_CMT)
+	help
+	  This enables build of the CMT timer driver.
+
+config SH_TIMER_MTU2
+	bool "MTU2 timer driver"
+	default y if ARM || (SUPERH && SYS_SUPPORTS_MTU2)
+	help
+	  This enables build of the MTU2 timer driver.
+
+config SH_TIMER_TMU
+	bool "TMU timer driver"
+	default y if ARM || (SUPERH && SYS_SUPPORTS_TMU)
+	help
+	  This enables build of the TMU timer driver.
+
+config EM_TIMER_STI
+	bool "STI timer driver"
+	default y if ARM
+	help
+	  This enables build of the STI timer driver.
+
+endmenu
+endif

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Daniel Lezcano]

On 11/06/2013 12:05 PM, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
>
> Add Kconfig entries for CMT, MTU2, TMU and STI to
> drivers/clocksource/Kconfig. This will allow us to
> get rid of duplicated entires in architecture code
> such as arch/sh and arch/arm/mach-shmobile.
>
> Signed-off-by: Magnus Damm <damm@opensource.se>

Hi Magnus,

we are preventing to have selectable timer from this Kconfig.

You should select the timers from the arch Kconfig option.

Regards
   -- Daniel

> ---
>
>   drivers/clocksource/Kconfig |   30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
>
> --- 0001/drivers/clocksource/Kconfig
> +++ work/drivers/clocksource/Kconfig	2013-11-05 19:49:47.000000000 +0900
> @@ -110,3 +110,33 @@ config VF_PIT_TIMER
>   	bool
>   	help
>   	  Support for Period Interrupt Timer on Freescale Vybrid Family SoCs.
> +
> +if COMPILE_TEST || ARM || SUPERH
> +menu "Timer drivers"
> +
> +config SH_TIMER_CMT
> +	bool "CMT timer driver"
> +	default y if ARM || (SUPERH && SYS_SUPPORTS_CMT)
> +	help
> +	  This enables build of the CMT timer driver.
> +
> +config SH_TIMER_MTU2
> +	bool "MTU2 timer driver"
> +	default y if ARM || (SUPERH && SYS_SUPPORTS_MTU2)
> +	help
> +	  This enables build of the MTU2 timer driver.
> +
> +config SH_TIMER_TMU
> +	bool "TMU timer driver"
> +	default y if ARM || (SUPERH && SYS_SUPPORTS_TMU)
> +	help
> +	  This enables build of the TMU timer driver.
> +
> +config EM_TIMER_STI
> +	bool "STI timer driver"
> +	default y if ARM
> +	help
> +	  This enables build of the STI timer driver.
> +
> +endmenu
> +endif
>


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

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Magnus Damm]

On Thu, Nov 7, 2013 at 8:27 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> On 11/06/2013 12:05 PM, Magnus Damm wrote:
>>
>> From: Magnus Damm <damm@opensource.se>
>>
>> Add Kconfig entries for CMT, MTU2, TMU and STI to
>> drivers/clocksource/Kconfig. This will allow us to
>> get rid of duplicated entires in architecture code
>> such as arch/sh and arch/arm/mach-shmobile.
>>
>> Signed-off-by: Magnus Damm <damm@opensource.se>
>
>
> Hi Magnus,
>
> we are preventing to have selectable timer from this Kconfig.
>
> You should select the timers from the arch Kconfig option.

Thanks for your reply. In the case of ARM multiplatform this sounds
like a good match, but I wonder what to do in the case of single
platform ARM and the SH architecture. It would be nice to share the
same Kconfig entries for both SH and all ARM variants.

As you can see, in this series the Kconfig is shared between SH and
ARM mach-shmobile. Actually, on those platforms we don't treat the
timers any differently than any other device driver and traditionally
we have been allowing the user to select which timer to use as system
timer via the kernel configuration. What is the reason behind your
suggestion with the arch Kconfig selection? It seems to me that timers
are being treated differently than any other device driver, but I'm
not sure why that is the case.

Changing SH and ARM single/multi to select via the architecture
Kconfig seems like a functional regression for single platforms to me.
It is also not really in line with TWD and architected timers that are
today selectable via the ARM Kconfig. Say that I would like to develop
code for a certain timer on a SoC with multiple timers, how can I make
sure the right timer is used? Perhaps you propose relying on the
clocksource and clockevent ratings?

Cheers,

/ magnus

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[John Stultz]

On 11/08/2013 12:23 AM, Magnus Damm wrote:
> On Thu, Nov 7, 2013 at 8:27 PM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>> On 11/06/2013 12:05 PM, Magnus Damm wrote:
>>> From: Magnus Damm <damm@opensource.se>
>>>
>>> Add Kconfig entries for CMT, MTU2, TMU and STI to
>>> drivers/clocksource/Kconfig. This will allow us to
>>> get rid of duplicated entires in architecture code
>>> such as arch/sh and arch/arm/mach-shmobile.
>>>
>>> Signed-off-by: Magnus Damm <damm@opensource.se>
>>
>> Hi Magnus,
>>
>> we are preventing to have selectable timer from this Kconfig.
>>
>> You should select the timers from the arch Kconfig option.
> Thanks for your reply. In the case of ARM multiplatform this sounds
> like a good match, but I wonder what to do in the case of single
> platform ARM and the SH architecture. It would be nice to share the
> same Kconfig entries for both SH and all ARM variants.
>
> As you can see, in this series the Kconfig is shared between SH and
> ARM mach-shmobile. Actually, on those platforms we don't treat the
> timers any differently than any other device driver and traditionally
> we have been allowing the user to select which timer to use as system
> timer via the kernel configuration. What is the reason behind your
> suggestion with the arch Kconfig selection? It seems to me that timers
> are being treated differently than any other device driver, but I'm
> not sure why that is the case.

So to me clocksources/clockevents are different from any other device
driver, because they are *almost* always baked into the system, rather
then being like pci or usb devices, which cannot be assumed to exist. In
almost all cases the kernel builder has to enable some sort of board
specific config option which implies which clocksource/clockevent driver
should be enabled, so having a secondary config option (somewhere deeply
nested in the drivers menus, that really just frustrates folks trying to
get a board running) is unnecessary.

I know for some SoC maintainers, there's a desire to have config options
be this abstract "buckets of parts", which folks can put together some
random configuration and out will pop a kernel that supports some new
board wihtout any code change. And while I think this is easier to
manage from a developers point of view, I see it as really painful to
the folks trying to build kernels.

The vast majority of clocksource/clockevent hardware are architecture
specific, and this is why I originally objected to moving all the
clocksources (and then clockevents) out of arch/arm and into into
drivers/clocksource. And what I'd really like to avoid is having
something like the drivers/rtc directory, where it approaches something
like a collection of rtc-<sha1>.c files, and the kernel builder has to
go through a menu page of configuration options where almost none of
them apply to the architecture they're using.

So while I've given up sub-maintainership of the drivers/clocksource
directory to Daniel, I tried to impress that there should really be an
*outstanding* reason for any user-selectable clocksource/clockevent
configuration options that pop up under the drivers directory.


> Changing SH and ARM single/multi to select via the architecture
> Kconfig seems like a functional regression for single platforms to me.

Not sure I understand how this is a regression. Could you expand on this?

> It is also not really in line with TWD and architected timers that are
> today selectable via the ARM Kconfig. Say that I would like to develop
> code for a certain timer on a SoC with multiple timers, how can I make
> sure the right timer is used? Perhaps you propose relying on the
> clocksource and clockevent ratings?

So if there is a SoC with multiple timers, and there isn't a obviously
preferred one (which can be selected either statically or via the
ratings values), then I think it may be reasonable to have a
user-selected config, but I'd much prefer that config to live in the
architecture Kconfig, close to the other SoC specific options, where the
tradeoffs of the choice can be properly documented so the user has maybe
a chance to be able to make the right choice for their needs (rather
then telling them to dig through nested driver config menus and having
them select the right one of seven poorly documented options, where
really only 2 actually apply to the board).

This is starting to sound like a rant, and its all my personal opinion,
and I'm no longer maintining the directory, so I should probably stop
here. :)

But am I making any sense? Maybe there's a more clear way to express the
goal of the policy?

thanks
-john

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Magnus Damm]

On Sat, Nov 9, 2013 at 3:34 AM, John Stultz <john.stultz@linaro.org> wrote:
> On 11/08/2013 12:23 AM, Magnus Damm wrote:
>> On Thu, Nov 7, 2013 at 8:27 PM, Daniel Lezcano
>> <daniel.lezcano@linaro.org> wrote:
>>> On 11/06/2013 12:05 PM, Magnus Damm wrote:
>>>> From: Magnus Damm <damm@opensource.se>
>>>>
>>>> Add Kconfig entries for CMT, MTU2, TMU and STI to
>>>> drivers/clocksource/Kconfig. This will allow us to
>>>> get rid of duplicated entires in architecture code
>>>> such as arch/sh and arch/arm/mach-shmobile.
>>>>
>>>> Signed-off-by: Magnus Damm <damm@opensource.se>
>>>
>>> Hi Magnus,
>>>
>>> we are preventing to have selectable timer from this Kconfig.
>>>
>>> You should select the timers from the arch Kconfig option.
>> Thanks for your reply. In the case of ARM multiplatform this sounds
>> like a good match, but I wonder what to do in the case of single
>> platform ARM and the SH architecture. It would be nice to share the
>> same Kconfig entries for both SH and all ARM variants.
>>
>> As you can see, in this series the Kconfig is shared between SH and
>> ARM mach-shmobile. Actually, on those platforms we don't treat the
>> timers any differently than any other device driver and traditionally
>> we have been allowing the user to select which timer to use as system
>> timer via the kernel configuration. What is the reason behind your
>> suggestion with the arch Kconfig selection? It seems to me that timers
>> are being treated differently than any other device driver, but I'm
>> not sure why that is the case.
>
> So to me clocksources/clockevents are different from any other device
> driver, because they are *almost* always baked into the system, rather
> then being like pci or usb devices, which cannot be assumed to exist. In
> almost all cases the kernel builder has to enable some sort of board
> specific config option which implies which clocksource/clockevent driver
> should be enabled, so having a secondary config option (somewhere deeply
> nested in the drivers menus, that really just frustrates folks trying to
> get a board running) is unnecessary.

Hi Jonh,

Thanks for your reply, nice to hear your point of view!

I can see where you are coming from and I agree that it is a good idea
to keep frustration at a minimum. I'm not sure if I agree that having
nested menus with settings is necessarily a bad thing. Most drivers
are kept like that. IMO, what is bad is if the defconfig doesn't work.

> I know for some SoC maintainers, there's a desire to have config options
> be this abstract "buckets of parts", which folks can put together some
> random configuration and out will pop a kernel that supports some new
> board wihtout any code change. And while I think this is easier to
> manage from a developers point of view, I see it as really painful to
> the folks trying to build kernels.

I can somehow imagine that people may do that, but that's not really
what I'm trying to here. I too would like a single defconfig that will
work across a wide range of SoCs and their boards. At the same time I
want to give people the ability to chose what they want to enable.

> The vast majority of clocksource/clockevent hardware are architecture
> specific, and this is why I originally objected to moving all the
> clocksources (and then clockevents) out of arch/arm and into into
> drivers/clocksource. And what I'd really like to avoid is having
> something like the drivers/rtc directory, where it approaches something
> like a collection of rtc-<sha1>.c files, and the kernel builder has to
> go through a menu page of configuration options where almost none of
> them apply to the architecture they're using.

Regarding making timers into something that is really difficult for
kernel builders, I too would like to avoid that. But having a
reasonable defconfig will solve that, no?

My experience is that many kinds of timer hardware is far from
architecture specific. Most of the Renesas timer drivers can be used
on both SH and ARM. I've seen similar things between 68k and PPC. I
suspect that the x86 PIT is used pretty much everywhere. Those are all
rather simple timers, when it comes to local timers for SMP then it of
course starts getting more complicated.. In such case you probably
want in in the arch directory, yes..

> So while I've given up sub-maintainership of the drivers/clocksource
> directory to Daniel, I tried to impress that there should really be an
> *outstanding* reason for any user-selectable clocksource/clockevent
> configuration options that pop up under the drivers directory.

I see. So I don't mind that much either way, I was asked to
consolidate the kconfig and that's what I'm doing. I felt it was kind
of natural to share the Kconfig entries for SH and ARM since the same
drivers are used.

One thing that I mind is that I'd like the timer configuration to be
consistent. I'd like to know what should be selected by the SoC and
what is optional.

Let me get back to the kernel configuration. Of course, it would be
really nice if the kernel configuration was 100% fool proof, but what
happens if the user doesn't compile-in certain parts? That hardware
won't be used. What happens if wrong console device is passed on the
kernel command line? The friendly answer is usually "don't do that".

So in case of the serial console, no driver - no output. You can still
use the network. If you have no timer then there won't be any timer
ticks. You can still get to user space though, but don't try to rely
on the timer. This CONFIG_TIMER=y/n case is pretty clear, but isn't
there a grey zone too?

Regarding this grey timer area, for example have a look at the flag
CLOCK_EVT_FEAT_C3STOP. The arch timer driver sets this flag, which
means that with only arch timer the system can function as expected
for most of the time. But if we try to use the high resolution timer
with cyclictest then we get the error "WARNING: High resolution timers
not available" unless another timer is installed as well. So for some
parts of the kernel we need more than one timer. Where do you draw the
line with select?

Also, like I mentioned before, some shared timers can already be
controlled via the kernel configuration today. For instance, it is
possible to control arch timer and TWD via the kernel configuration.
How about them? I'd like to be able to control them via the kernel
configuration since this makes it possible to test all parts of the
system.

>> Changing SH and ARM single/multi to select via the architecture
>> Kconfig seems like a functional regression for single platforms to me.
>
> Not sure I understand how this is a regression. Could you expand on this?

On SH we have platform devices pointing out several different timer
channels using different drivers. Basically different timer channels
that may be used are listed for each SoC. Zero or more of these timer
drivers can be selected via kernel configuration. If you prefer low
power operation then select a timer that operates on a 32Khz timer. If
not, go with a MHz-range timer. How can this be selected without the
kernel configuration?

>> It is also not really in line with TWD and architected timers that are
>> today selectable via the ARM Kconfig. Say that I would like to develop
>> code for a certain timer on a SoC with multiple timers, how can I make
>> sure the right timer is used? Perhaps you propose relying on the
>> clocksource and clockevent ratings?
>
> So if there is a SoC with multiple timers, and there isn't a obviously
> preferred one (which can be selected either statically or via the
> ratings values), then I think it may be reasonable to have a
> user-selected config, but I'd much prefer that config to live in the
> architecture Kconfig, close to the other SoC specific options, where the
> tradeoffs of the choice can be properly documented so the user has maybe
> a chance to be able to make the right choice for their needs (rather
> then telling them to dig through nested driver config menus and having
> them select the right one of seven poorly documented options, where
> really only 2 actually apply to the board).

I think one underlying problem why we're not expressing all
dependencies in Kconfig lingo is that it would be too verbose and
heavy to point out in detail exactly which config options that apply
on each platform. Now if there was some kind of tool available that
could generate the kernel configuration based on one or more DT
files.. Or generate dependencies for Kconfig based on DT.

If I understand you correctly then your preferred choice here would be
for us to move in the direction of using select for timers in case on
ARM mach-shmobile. I think being standard is important, so following
common practise in case of ARM makes sense.

> This is starting to sound like a rant, and its all my personal opinion,
> and I'm no longer maintining the directory, so I should probably stop
> here. :)

Well, without opinions we can't find a sensible direction, can we?

> But am I making any sense? Maybe there's a more clear way to express the
> goal of the policy?

I think all your goals make sense, and I would like to reach the same
place from a usability point of view. I would however like to allow
existing power users to select whatever they want enabled on their
platform. Ideally I also would like to share Kconfig bits between
multiple architectures where appropriate, but it's just a few lines of
code so I don't care that much.

So I guess it boils down to a question if the turn-key experience
should be controlled via Kconfig dependencies or the defconfig.
Perhaps it is sane for an architecture to default to yes but allow
power users to opt-out?

Thanks,

/ magnus

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[John Stultz]

On Tue, Nov 12, 2013 at 4:26 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> On Sat, Nov 9, 2013 at 3:34 AM, John Stultz <john.stultz@linaro.org> wrote:
>> I know for some SoC maintainers, there's a desire to have config options
>> be this abstract "buckets of parts", which folks can put together some
>> random configuration and out will pop a kernel that supports some new
>> board wihtout any code change. And while I think this is easier to
>> manage from a developers point of view, I see it as really painful to
>> the folks trying to build kernels.
>
> I can somehow imagine that people may do that, but that's not really
> what I'm trying to here. I too would like a single defconfig that will
> work across a wide range of SoCs and their boards. At the same time I
> want to give people the ability to chose what they want to enable.

And your case may be one of those outstanding cases, which is fine. I
just want to make sure the policy is clear and enforced so we don't
just start adding config options for everything.

>> The vast majority of clocksource/clockevent hardware are architecture
>> specific, and this is why I originally objected to moving all the
>> clocksources (and then clockevents) out of arch/arm and into into
>> drivers/clocksource. And what I'd really like to avoid is having
>> something like the drivers/rtc directory, where it approaches something
>> like a collection of rtc-<sha1>.c files, and the kernel builder has to
>> go through a menu page of configuration options where almost none of
>> them apply to the architecture they're using.
>
> Regarding making timers into something that is really difficult for
> kernel builders, I too would like to avoid that. But having a
> reasonable defconfig will solve that, no?

I've had enough bad experiences with defconfigs that I don't trust
them.  And really, I see defconfigs as a really poor solution to the
problem. We have this expressive language in Kconfig to state
dependencies between configs and provide help documentation for each
option. But since folks tend to not to look at the bigger picture and
use those dependencies, they just add options for their code and call
it a day. So then folks start to rely on uncommented defconfigs to
provide those implicit dependencies.

To me It seems like the same rational could be made for providing a
directory of patches that users apply to the kernel tree depending on
which board they have. :) Why bother with configs and ifdefs at all!?

Hyperbole aside, I do get that for many cases, encoding the full
config dependency logic explicitly is really hard and would make for
very ugly Kconfigs. And I also understand defconfigs are a much easier
way to get by while things aren't perfect.  But at least for the
clocksource/clockevent directory, I'd push that we try to be better
and push to have patch submitters really put the effort in to do the
right thing.


> My experience is that many kinds of timer hardware is far from
> architecture specific. Most of the Renesas timer drivers can be used
> on both SH and ARM. I've seen similar things between 68k and PPC. I
> suspect that the x86 PIT is used pretty much everywhere. Those are all
> rather simple timers, when it comes to local timers for SMP then it of
> course starts getting more complicated.. In such case you probably
> want in in the arch directory, yes..

So for drivers that are *actually* shared between architectures, I
have less of a problem. That's why there was a generic
drivers/clocksource directory in the first place. The issue is that
folks are adding things that maybe-theoretically could be shared, but
actually aren't.

But again, even if the hardware is shared between arches, unless there
really is a compelling reason to make the user select an option, I
think it should be automatically selected by a platform config. (for
example, the ACPI PM timer is i368, x86_64, ia64..  but users
shouldn't have to chose it if they have already chosen to enable ACPI
on those arches - so no config option is needed).

>> So while I've given up sub-maintainership of the drivers/clocksource
>> directory to Daniel, I tried to impress that there should really be an
>> *outstanding* reason for any user-selectable clocksource/clockevent
>> configuration options that pop up under the drivers directory.
>
> I see. So I don't mind that much either way, I was asked to
> consolidate the kconfig and that's what I'm doing. I felt it was kind
> of natural to share the Kconfig entries for SH and ARM since the same
> drivers are used.

Again, no problem with the Kconfig entries being shared. I just want
to make sure users don't have to select what are in effect redundant
options. This can be done via platform configs selecting appropriate
clocksource/clockevent options, or by the clocksource/clockevent
options having clear dependencies and defaulting to y w/o a user
prompt.

> One thing that I mind is that I'd like the timer configuration to be
> consistent. I'd like to know what should be selected by the SoC and
> what is optional.
>
> Let me get back to the kernel configuration. Of course, it would be
> really nice if the kernel configuration was 100% fool proof, but what
> happens if the user doesn't compile-in certain parts? That hardware
> won't be used. What happens if wrong console device is passed on the
> kernel command line? The friendly answer is usually "don't do that".
>
> So in case of the serial console, no driver - no output. You can still
> use the network. If you have no timer then there won't be any timer
> ticks. You can still get to user space though, but don't try to rely
> on the timer. This CONFIG_TIMER=y/n case is pretty clear, but isn't
> there a grey zone too?

And on every new board, I have to fumble around with exactly those
sorts of no-serial output issues. Its never something I consider a
great use of my time :)

And your example is a little flawed have no timer ticks, you're not
getting to userspace. The system won't boot.

I'll admit there may be cases where there are gray zones w/ timers.
Much there are by far more black and white cases, so I want to policy
to be clear for those cases so the driver writers who best know the
black/white division can make the right call, and we don't end up with
a bunch of drivers that just seem to be in the grey zone because folks
didn't think about it.

Black and white case:
* hardware won't boot without the clocksource/clockevent driver, and
your hardware has a platform level config option
* Platform might boot w/ jiffies clocksource, but there is a highres
capable clocksource in the cpu design that every user would obviously
want.

Grey cases:
* Hardware supports clocksource/clockevent hardware from a PCI card or
other optional bus.

* Hardware has multiple clocksources, that run at vastly different
frequencies and wrap fairly quickly. Thus to extend low-power idle
times, the slow clocksource should be used, but if fine grained timing
is needed the faster clocksource should be used.

But even that grey case, I suspect having both drivers built in is the
right approach, allowing userspace to select the right clocksource for
the needs without having to rebuild a kernel.


> Regarding this grey timer area, for example have a look at the flag
> CLOCK_EVT_FEAT_C3STOP. The arch timer driver sets this flag, which
> means that with only arch timer the system can function as expected
> for most of the time. But if we try to use the high resolution timer
> with cyclictest then we get the error "WARNING: High resolution timers
> not available" unless another timer is installed as well. So for some
> parts of the kernel we need more than one timer. Where do you draw the
> line with select?

Well, my understanding is C3STOP means you have to have a secondary
broadcast timer to handle wakeups when you hit deep idle. So on that
platform you need to build two timers to get what I consider expected
behavior.

In general, It seems that enabling all the hardware on the system
(within reason) would be best if you weren't concerned with code size
issues (and that can be a reasonable concern), and most clockevent
drivers are fairly small.  But I think code size issues would be a
grey area where an option could make sense, but it needs to be clearly
explained that that is the reason and the code size delta is X kb and
obviously worth it.


> Also, like I mentioned before, some shared timers can already be
> controlled via the kernel configuration today. For instance, it is
> possible to control arch timer and TWD via the kernel configuration.
> How about them? I'd like to be able to control them via the kernel
> configuration since this makes it possible to test all parts of the
> system.

Testing is probably one of the arguments I don't have a good response
to. Arnd brought this up as well, since he wants to be able to do test
builds of all the ARM clocksource drivers from an x86 config (TEST ALL
OF THE BUCKETS OF PARTS!!! ;). Non-user prompt-able options could
effectively hide build issues, making it hard for folks who are doing
their testing on a limited set of systems to be able to catch compile
issues they might create on other arches.  Here we have to weigh the
cost to the new user against the utility to the all-yes-config testing
developer. Better cross tools, the zero-day build robot and other
approaches also help here. And maybe there can be some special Kconfig
option that lets the module be built without requiring a Kconfig user
prompt?


>>> Changing SH and ARM single/multi to select via the architecture
>>> Kconfig seems like a functional regression for single platforms to me.
>>
>> Not sure I understand how this is a regression. Could you expand on this?
>
> On SH we have platform devices pointing out several different timer
> channels using different drivers. Basically different timer channels
> that may be used are listed for each SoC. Zero or more of these timer
> drivers can be selected via kernel configuration. If you prefer low
> power operation then select a timer that operates on a 32Khz timer. If
> not, go with a MHz-range timer. How can this be selected without the
> kernel configuration?

This may be one of the few grey areas.

Some questions to help sort this out:
1) Is this best selected at build time? Would boot-time or even
runtime switching (as we can do w/ clocksources) be a better choice?
What is the actual cost of building the reasonable options together?

2) Is the power vs accuracy decision something that is shared between
all users of the driver, or just particular hardware?

3) Is the decision best made by a user under the Drivers -> Timers (or
whatever) submenu? Or closer to the platform selection?


>> So if there is a SoC with multiple timers, and there isn't a obviously
>> preferred one (which can be selected either statically or via the
>> ratings values), then I think it may be reasonable to have a
>> user-selected config, but I'd much prefer that config to live in the
>> architecture Kconfig, close to the other SoC specific options, where the
>> tradeoffs of the choice can be properly documented so the user has maybe
>> a chance to be able to make the right choice for their needs (rather
>> then telling them to dig through nested driver config menus and having
>> them select the right one of seven poorly documented options, where
>> really only 2 actually apply to the board).
>
> I think one underlying problem why we're not expressing all
> dependencies in Kconfig lingo is that it would be too verbose and
> heavy to point out in detail exactly which config options that apply
> on each platform. Now if there was some kind of tool available that
> could generate the kernel configuration based on one or more DT
> files.. Or generate dependencies for Kconfig based on DT.

I agree its not always easy to sort out the best solution with
Kconfig. But I think always handing users buckets of parts for them to
assemble because we couldn't be bothered to think about it a bit isn't
the right solution either. In many ways this mirrors the discussion
about userspace policy. Its easy to reflexively punt to userspace to
choose what to do, but quite often the kernel could pick a better sane
default. We just have to push for people to think about it a bit.

And there will always be exceptional cases where config options are
just the only reasonable way to support all that folks want to do. But
I want to make sure we don't just default to the "give the user the
option" case, when the option could logically conflict with other
choices already made.


> If I understand you correctly then your preferred choice here would be
> for us to move in the direction of using select for timers in case on
> ARM mach-shmobile. I think being standard is important, so following
> common practise in case of ARM makes sense.

Yea. And to some extent its a standard for clocksources/clockevent
drivers, since they are usually non-optional functionality.

>> This is starting to sound like a rant, and its all my personal opinion,
>> and I'm no longer maintining the directory, so I should probably stop
>> here. :)
>
> Well, without opinions we can't find a sensible direction, can we?
>
>> But am I making any sense? Maybe there's a more clear way to express the
>> goal of the policy?
>
> I think all your goals make sense, and I would like to reach the same
> place from a usability point of view. I would however like to allow
> existing power users to select whatever they want enabled on their
> platform. Ideally I also would like to share Kconfig bits between
> multiple architectures where appropriate, but it's just a few lines of
> code so I don't care that much.

And as long as the options for the power-users actually make sense,
that all sounds fine. But I want to make sure we aren't needlessly
causing pain to folks building kernels all to save a few lines of
Kconfig logic.

And again, this is just my pet peeve, I'm not the directory
submaintainer any more, so Daniel and Thomas are the ones to convince.
:)

thanks
-john

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Daniel Lezcano]

On 11/12/2013 09:47 PM, John Stultz wrote:

[ ... ]

>> I think all your goals make sense, and I would like to reach the same
>> place from a usability point of view. I would however like to allow
>> existing power users to select whatever they want enabled on their
>> platform. Ideally I also would like to share Kconfig bits between
>> multiple architectures where appropriate, but it's just a few lines of
>> code so I don't care that much.
>
> And as long as the options for the power-users actually make sense,
> that all sounds fine. But I want to make sure we aren't needlessly
> causing pain to folks building kernels all to save a few lines of
> Kconfig logic.
>
> And again, this is just my pet peeve, I'm not the directory
> submaintainer any more, so Daniel and Thomas are the ones to convince.
> :)

So to summarize:

1. We want to prevent to manually select the drivers, this is painful to 
have the right config. We assume the SoC config will choose the right 
driver config option.

2. We want to disable some drivers because they could conflict. Or for 
kernel builders, it is easier to hack around the options.

3. We want to select a driver as a module because the timer could reside 
on a PCI board.

4. Code size could be an issue if everything is selected.

IMO, John's approach makes totally sense.

I am not worried about the code size because one day or another we will 
have to fix up the code size increasing with the single zImage for ARM, 
and we will probably end up to unload dynamically unneeded drivers from 
the memory after booting (I don't how. Perhaps by some magic with the 
init sections).

Disabling some drivers, or in other words, give more customization 
options to the kernel builders, makes also sense.

It isn't possible to select the driver as we do right now but let them 
optional from the Kconfig ? What if we invert the logic in the Kconfig, 
make each driver depends on a arch_option defaulting to 'yes', so it can 
be manually unselected (similar to drivers/cpuidle/Kconfig.arm).

In any case, consolidating SH and ARM Kconfig is ok but with a change 
which is consistent with the current Kconfig, that is following the 
policy of the Kconfig (on top of the current one or on top of a new one).

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

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[John Stultz]

On 11/13/2013 01:14 PM, Daniel Lezcano wrote:
> On 11/12/2013 09:47 PM, John Stultz wrote:
>
> [ ... ]
>
>>> I think all your goals make sense, and I would like to reach the same
>>> place from a usability point of view. I would however like to allow
>>> existing power users to select whatever they want enabled on their
>>> platform. Ideally I also would like to share Kconfig bits between
>>> multiple architectures where appropriate, but it's just a few lines of
>>> code so I don't care that much.
>>
>> And as long as the options for the power-users actually make sense,
>> that all sounds fine. But I want to make sure we aren't needlessly
>> causing pain to folks building kernels all to save a few lines of
>> Kconfig logic.
>>
>> And again, this is just my pet peeve, I'm not the directory
>> submaintainer any more, so Daniel and Thomas are the ones to convince.
>> :)
>
> So to summarize:
>
> 1. We want to prevent to manually select the drivers, this is painful
> to have the right config. We assume the SoC config will choose the
> right driver config option.
>
> 2. We want to disable some drivers because they could conflict. Or for
> kernel builders, it is easier to hack around the options.

This one I'm not sure I agree with completely. Basically I think
exceptions are reasonable, but we ought to keep the bar fairly high for
adding a user-visible config option.


> 3. We want to select a driver as a module because the timer could
> reside on a PCI board.
>
> 4. Code size could be an issue if everything is selected.
>
> IMO, John's approach makes totally sense.
>
> I am not worried about the code size because one day or another we
> will have to fix up the code size increasing with the single zImage
> for ARM, and we will probably end up to unload dynamically unneeded
> drivers from the memory after booting (I don't how. Perhaps by some
> magic with the init sections).
>
> Disabling some drivers, or in other words, give more customization
> options to the kernel builders, makes also sense.
>
> It isn't possible to select the driver as we do right now but let them
> optional from the Kconfig ? What if we invert the logic in the
> Kconfig, make each driver depends on a arch_option defaulting to
> 'yes', so it can be manually unselected (similar to
> drivers/cpuidle/Kconfig.arm).

Again, the main point for me is I don't even want the options to be
visible unless there is a real need. There may be reasonable exceptions,
but for the most part we shouldn't see these.

I'll go back to Magnus' original mail and reply with the sort of
questions I think we should answer before adding user visible configs.

thanks
-john

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Magnus Damm]

Hi John, everyone,

Let me get back to you all in a little while together with some code,
but before that let me just clarify this:

On Wed, Nov 13, 2013 at 5:47 AM, John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Nov 12, 2013 at 4:26 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>> On Sat, Nov 9, 2013 at 3:34 AM, John Stultz <john.stultz@linaro.org> wrote:

>> Let me get back to the kernel configuration. Of course, it would be
>> really nice if the kernel configuration was 100% fool proof, but what
>> happens if the user doesn't compile-in certain parts? That hardware
>> won't be used. What happens if wrong console device is passed on the
>> kernel command line? The friendly answer is usually "don't do that".
>>
>> So in case of the serial console, no driver - no output. You can still
>> use the network. If you have no timer then there won't be any timer
>> ticks. You can still get to user space though, but don't try to rely
>> on the timer. This CONFIG_TIMER=y/n case is pretty clear, but isn't
>> there a grey zone too?
>
> And on every new board, I have to fumble around with exactly those
> sorts of no-serial output issues. Its never something I consider a
> great use of my time :)
>
> And your example is a little flawed have no timer ticks, you're not
> getting to userspace. The system won't boot.

Correct me if I'm wrong here but I don't think so!

On some platforms that may very well be the case, but on mach-shmobile
we get to user space without any timers. If the timers are enabled
then they are regular platform devices these days. From my experience
the main blocker for going to user space without timer is on ARM
usually the udelay() calculation and/or the TWD delay, but those can
be handled with preset worst-case values for udelay() and getting the
rate via CCF for the TWD.

Also it of course depends on if some compiled-in driver needs to use
the timer. If we just enable the serial console then all our platforms
make it to initramfs-based user space without any clock source or
clock event.

I'd be happy to give you remote access to a board if you'd like to play. =)

Cheers,

/ magnus

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[John Stultz]

On 11/27/2013 09:42 PM, Magnus Damm wrote:
> Hi John, everyone,
>
> Let me get back to you all in a little while together with some code,
> but before that let me just clarify this:
>
> On Wed, Nov 13, 2013 at 5:47 AM, John Stultz <john.stultz@linaro.org> wrote:
>> On Tue, Nov 12, 2013 at 4:26 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>> On Sat, Nov 9, 2013 at 3:34 AM, John Stultz <john.stultz@linaro.org> wrote:
>>> Let me get back to the kernel configuration. Of course, it would be
>>> really nice if the kernel configuration was 100% fool proof, but what
>>> happens if the user doesn't compile-in certain parts? That hardware
>>> won't be used. What happens if wrong console device is passed on the
>>> kernel command line? The friendly answer is usually "don't do that".
>>>
>>> So in case of the serial console, no driver - no output. You can still
>>> use the network. If you have no timer then there won't be any timer
>>> ticks. You can still get to user space though, but don't try to rely
>>> on the timer. This CONFIG_TIMER=y/n case is pretty clear, but isn't
>>> there a grey zone too?
>> And on every new board, I have to fumble around with exactly those
>> sorts of no-serial output issues. Its never something I consider a
>> great use of my time :)
>>
>> And your example is a little flawed have no timer ticks, you're not
>> getting to userspace. The system won't boot.
> Correct me if I'm wrong here but I don't think so!
>
> On some platforms that may very well be the case, but on mach-shmobile
> we get to user space without any timers. If the timers are enabled
> then they are regular platform devices these days. From my experience
> the main blocker for going to user space without timer is on ARM
> usually the udelay() calculation and/or the TWD delay, but those can
> be handled with preset worst-case values for udelay() and getting the
> rate via CCF for the TWD.
>
> Also it of course depends on if some compiled-in driver needs to use
> the timer. If we just enable the serial console then all our platforms
> make it to initramfs-based user space without any clock source or
> clock event.
So something still triggers basic HZ timer functionality, and you get by
with the jiffies clocksource?

Ok, so thanks for the correction. But I still think booting in a
degraded mode isn't ideal when the user has already provided enough info
via config options that we could have sorted it out for them.

But yea, I look forward to your next patch so we can discuss things more
concretely, rather then in my more abstract handwaving rants ;)

thanks
-john

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Magnus Damm]

On Tue, Dec 3, 2013 at 12:08 AM, John Stultz <john.stultz@linaro.org> wrote:
> On 11/27/2013 09:42 PM, Magnus Damm wrote:
>> Hi John, everyone,
>>
>> Let me get back to you all in a little while together with some code,
>> but before that let me just clarify this:
>>
>> On Wed, Nov 13, 2013 at 5:47 AM, John Stultz <john.stultz@linaro.org> wrote:
>>> On Tue, Nov 12, 2013 at 4:26 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
>>>> On Sat, Nov 9, 2013 at 3:34 AM, John Stultz <john.stultz@linaro.org> wrote:
>>>> Let me get back to the kernel configuration. Of course, it would be
>>>> really nice if the kernel configuration was 100% fool proof, but what
>>>> happens if the user doesn't compile-in certain parts? That hardware
>>>> won't be used. What happens if wrong console device is passed on the
>>>> kernel command line? The friendly answer is usually "don't do that".
>>>>
>>>> So in case of the serial console, no driver - no output. You can still
>>>> use the network. If you have no timer then there won't be any timer
>>>> ticks. You can still get to user space though, but don't try to rely
>>>> on the timer. This CONFIG_TIMER=y/n case is pretty clear, but isn't
>>>> there a grey zone too?
>>> And on every new board, I have to fumble around with exactly those
>>> sorts of no-serial output issues. Its never something I consider a
>>> great use of my time :)
>>>
>>> And your example is a little flawed have no timer ticks, you're not
>>> getting to userspace. The system won't boot.
>> Correct me if I'm wrong here but I don't think so!
>>
>> On some platforms that may very well be the case, but on mach-shmobile
>> we get to user space without any timers. If the timers are enabled
>> then they are regular platform devices these days. From my experience
>> the main blocker for going to user space without timer is on ARM
>> usually the udelay() calculation and/or the TWD delay, but those can
>> be handled with preset worst-case values for udelay() and getting the
>> rate via CCF for the TWD.
>>
>> Also it of course depends on if some compiled-in driver needs to use
>> the timer. If we just enable the serial console then all our platforms
>> make it to initramfs-based user space without any clock source or
>> clock event.
> So something still triggers basic HZ timer functionality, and you get by
> with the jiffies clocksource?

No clock event driver is registered, so no interrupts are coming. As
for clock source, simple jiffy is used selected by default I'm
mistaken

> Ok, so thanks for the correction. But I still think booting in a
> degraded mode isn't ideal when the user has already provided enough info
> via config options that we could have sorted it out for them.

The timer subsystem is complex, so if your goal is to make things
simpler then I'm all for it!

Booting in what you call degraded mode is only useful in one case IMO:
If the user has decided to configure the kernel with some drivers
disabled. Perhaps to save space. For instance, if the user doesn't
care about power management then only using arch timer is a valid way
to use the kernel IMO. In that case other timer drivers may be
deselected.

I'm not so sure why we want to set policy to forcibly compile in bits
that are not really mandatory for basic operation?

My view is that it is just natural for users to not compile in various
drivers. No serial console driver - no serial output, no timer driver
- no timer ticks. Having a working default is of course useful, but as
I mentioned earlier, I find that defconfigs is the way to go there. Of
course, how to deal with multiplatform in such case is a bit trickier,
so Kconfig may come in handy there.

> But yea, I look forward to your next patch so we can discuss things more
> concretely, rather then in my more abstract handwaving rants ;)

Thanks, hopefully the next version will be one step in the right direction!

Cheers,

/ magnus

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[John Stultz]

On Wed, Nov 6, 2013 at 3:05 AM, Magnus Damm <magnus.damm@gmail.com> wrote:
> From: Magnus Damm <damm@opensource.se>
>
> Add Kconfig entries for CMT, MTU2, TMU and STI to
> drivers/clocksource/Kconfig. This will allow us to
> get rid of duplicated entires in architecture code
> such as arch/sh and arch/arm/mach-shmobile.
>
> Signed-off-by: Magnus Damm <damm@opensource.se>
> ---
>
>  drivers/clocksource/Kconfig |   30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> --- 0001/drivers/clocksource/Kconfig
> +++ work/drivers/clocksource/Kconfig    2013-11-05 19:49:47.000000000 +0900
> @@ -110,3 +110,33 @@ config VF_PIT_TIMER
>         bool
>         help
>           Support for Period Interrupt Timer on Freescale Vybrid Family SoCs.
> +
> +if COMPILE_TEST || ARM || SUPERH
> +menu "Timer drivers"
> +
> +config SH_TIMER_CMT
> +       bool "CMT timer driver"
> +       default y if ARM || (SUPERH && SYS_SUPPORTS_CMT)
> +       help
> +         This enables build of the CMT timer driver.
> +
> +config SH_TIMER_MTU2
> +       bool "MTU2 timer driver"
> +       default y if ARM || (SUPERH && SYS_SUPPORTS_MTU2)
> +       help
> +         This enables build of the MTU2 timer driver.
> +
> +config SH_TIMER_TMU
> +       bool "TMU timer driver"
> +       default y if ARM || (SUPERH && SYS_SUPPORTS_TMU)
> +       help
> +         This enables build of the TMU timer driver.
> +
> +config EM_TIMER_STI
> +       bool "STI timer driver"
> +       default y if ARM
> +       help
> +         This enables build of the STI timer driver.


So since I do want to avoid adding user-selectable configs if
possible, here are some concrete thoughts on this patch, trying to
provide an example from my more abstract rants down thread. :)

1) Why does the user have to chose something here?  Can the option be
set based on other platform details?

2) The help text for these options are basically tautological and
could be better. Consider what is it that your really asking the user
to decide here? What tradeoffs do they have to consider?  What is the
actual hardware here? CMT/TMU/MTU2/STI? This is just acronym soup.
These should be included in the help text.

3) If all of these options are disabled, will the affected system boot?

4) Do you have a list of systems that actually use these drivers? Can
you be any more restrictive then config ARM?

thanks
-john

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Arnd Bergmann]

On Wednesday 13 November 2013, John Stultz wrote:
> > +config EM_TIMER_STI
> > +       bool "STI timer driver"
> > +       default y if ARM
> > +       help
> > +         This enables build of the STI timer driver.
> 
> 
> So since I do want to avoid adding user-selectable configs if
> possible, here are some concrete thoughts on this patch, trying to
> provide an example from my more abstract rants down thread. :)

I think we should be using the CONFIG_BUILD_TEST (or whatever
it's called) here to allow building the drivers everywhere but
at the same time be specific with the platform. How about this:

config EM_TIMER_STI
	bool "Renesas EMMA EV2 STI timer driver" if BUILD_TEST && !ARCH_EMEV2
	default ARCH_EMEV2
	help
	  ...

This makes it a silent option that is set correctly all the time
but lets you enable it everywhere when BUILD_TEST is set.

We could skip the "&& !ARCH_EMEV2" part to allow building an
EMEV2 kernel without this driver when build testing if that
is possible.

	Arnd

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Daniel Lezcano]

On 11/14/2013 01:40 PM, Arnd Bergmann wrote:
> On Wednesday 13 November 2013, John Stultz wrote:
>>> +config EM_TIMER_STI
>>> +       bool "STI timer driver"
>>> +       default y if ARM
>>> +       help
>>> +         This enables build of the STI timer driver.
>>
>>
>> So since I do want to avoid adding user-selectable configs if
>> possible, here are some concrete thoughts on this patch, trying to
>> provide an example from my more abstract rants down thread. :)
>
> I think we should be using the CONFIG_BUILD_TEST (or whatever
> it's called) here to allow building the drivers everywhere but
> at the same time be specific with the platform. How about this:
>
> config EM_TIMER_STI
> 	bool "Renesas EMMA EV2 STI timer driver" if BUILD_TEST && !ARCH_EMEV2
> 	default ARCH_EMEV2
> 	help
> 	  ...
>
> This makes it a silent option that is set correctly all the time
> but lets you enable it everywhere when BUILD_TEST is set.

It sounds like a good compromise.

> We could skip the "&& !ARCH_EMEV2" part to allow building an
> EMEV2 kernel without this driver when build testing if that
> is possible.



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

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[John Stultz]

On 11/14/2013 04:40 AM, Arnd Bergmann wrote:
> On Wednesday 13 November 2013, John Stultz wrote:
>>> +config EM_TIMER_STI
>>> +       bool "STI timer driver"
>>> +       default y if ARM
>>> +       help
>>> +         This enables build of the STI timer driver.
>>
>> So since I do want to avoid adding user-selectable configs if
>> possible, here are some concrete thoughts on this patch, trying to
>> provide an example from my more abstract rants down thread. :)
> I think we should be using the CONFIG_BUILD_TEST (or whatever
> it's called) here to allow building the drivers everywhere but
> at the same time be specific with the platform. How about this:
>
> config EM_TIMER_STI
> 	bool "Renesas EMMA EV2 STI timer driver" if BUILD_TEST && !ARCH_EMEV2
> 	default ARCH_EMEV2
> 	help
> 	  ...
>
> This makes it a silent option that is set correctly all the time
> but lets you enable it everywhere when BUILD_TEST is set.
>
> We could skip the "&& !ARCH_EMEV2" part to allow building an
> EMEV2 kernel without this driver when build testing if that
> is possible.

Sorry, somehow this mail slipped by me.

So yea, as I mentioned earlier, the build testing is the one thing I
don't have a good argument against. But I still would rather not having
user prompts for these sorts of things.

Even so, it seems to me that it would be better to still avoid prompting
the user for the build test and instead just always build the drivers if
BUILD_TEST is enabled.

Is the driver by driver fine-granularity for build testing actually
worth having all the module prompts?  Or could we coarsen it a bit, and
have BUILT_TEST_TOPIC, so under kernel hacking or something you can
enable build testing and select the categories of items you want to
build (rather then having to go through them one by one?)

thanks
-john

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[Arnd Bergmann]

On Wednesday 20 November 2013, John Stultz wrote:
> On 11/14/2013 04:40 AM, Arnd Bergmann wrote:
> 
> Sorry, somehow this mail slipped by me.
> 
> So yea, as I mentioned earlier, the build testing is the one thing I
> don't have a good argument against. But I still would rather not having
> user prompts for these sorts of things.
> 
> Even so, it seems to me that it would be better to still avoid prompting
> the user for the build test and instead just always build the drivers if
> BUILD_TEST is enabled.
> 
> Is the driver by driver fine-granularity for build testing actually
> worth having all the module prompts?  Or could we coarsen it a bit, and
> have BUILT_TEST_TOPIC, so under kernel hacking or something you can
> enable build testing and select the categories of items you want to
> build (rather then having to go through them one by one?)

I would much prefer keeping things consistent across subsystems, which
means one symbol with prompt per driver. The only technical argument for
this is that it lets you disable some drivers when they do break in a
randconfig or allmodconfig build.

	Arnd

Re: [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI

[John Stultz]

On 11/20/2013 11:34 AM, Arnd Bergmann wrote:
> On Wednesday 20 November 2013, John Stultz wrote:
>> On 11/14/2013 04:40 AM, Arnd Bergmann wrote:
>>
>> Sorry, somehow this mail slipped by me.
>>
>> So yea, as I mentioned earlier, the build testing is the one thing I
>> don't have a good argument against. But I still would rather not having
>> user prompts for these sorts of things.
>>
>> Even so, it seems to me that it would be better to still avoid prompting
>> the user for the build test and instead just always build the drivers if
>> BUILD_TEST is enabled.
>>
>> Is the driver by driver fine-granularity for build testing actually
>> worth having all the module prompts?  Or could we coarsen it a bit, and
>> have BUILT_TEST_TOPIC, so under kernel hacking or something you can
>> enable build testing and select the categories of items you want to
>> build (rather then having to go through them one by one?)
> I would much prefer keeping things consistent across subsystems, which
> means one symbol with prompt per driver. 

My main issue though is that with clocksource/clockevents, these are not
classic drivers. They are much closer to platform drivers, and in the
majority of cases are much more tightly bound to specific systems.

They were abstracted out to be more classic driver like, because that
gave us more flexibility to support various systems at runtime. I just
hate to see that flexibility abused at the cost of users trying to build
kernels that will boot on their devices.

> The only technical argument for
> this is that it lets you disable some drivers when they do break in a
> randconfig or allmodconfig build.
If you have to add special Kconfig annotation anyway to support
BUILD_TEST, why not ensure the drivers build before adding that
annotation? That way any build regressions will be more easily found
(and not accidentally missed because it wasn't enabled via randconfig),
and we don't have to pester users with redundant config options.

thanks
-john

[PATCH v2 02/03] ARM: shmobile: Remove CMT, TMU and STI Kconfig entries

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Now when drivers/clocksource/Kconfig has been
updated with entires for CMT, TMU and STI
it is safe to remove these from mach-shmobile.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 arch/arm/mach-shmobile/Kconfig |   21 ---------------------
 1 file changed, 21 deletions(-)

--- 0004/arch/arm/mach-shmobile/Kconfig
+++ work/arch/arm/mach-shmobile/Kconfig	2013-11-05 19:55:01.000000000 +0900
@@ -283,7 +283,6 @@ source "drivers/sh/Kconfig"
 endif
 
 if ARCH_SHMOBILE || ARCH_SHMOBILE_MULTI
-
 menu "Timer and clock configuration"
 
 config SHMOBILE_TIMER_HZ
@@ -297,25 +296,5 @@ config SHMOBILE_TIMER_HZ
 	  SH-Mobile systems using a 32768 Hz RCLK for clock events may want
 	  to select a HZ value such as 128 that can evenly divide RCLK.
 	  A HZ value that does not divide evenly may cause timer drift.
-
-config SH_TIMER_CMT
-	bool "CMT timer driver"
-	default y
-	help
-	  This enables build of the CMT timer driver.
-
-config SH_TIMER_TMU
-	bool "TMU timer driver"
-	default y
-	help
-	  This enables build of the TMU timer driver.
-
-config EM_TIMER_STI
-	bool "STI timer driver"
-	default y
-	help
-	  This enables build of the STI timer driver.
-
 endmenu
-
 endif

[PATCH v2 03/03] sh: Remove Kconfig entries for TMU, CMT and MTU2

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Now when drivers/clocksource/Kconfig has been
updated with entires for CMT, TMU and MTU2
it is safe to remove these from SH.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 arch/sh/Kconfig |   21 ---------------------
 1 file changed, 21 deletions(-)

--- 0001/arch/sh/Kconfig
+++ work/arch/sh/Kconfig	2013-11-05 21:03:10.000000000 +0900
@@ -565,27 +565,6 @@ source "arch/sh/boards/Kconfig"
 
 menu "Timer and clock configuration"
 
-config SH_TIMER_TMU
-	bool "TMU timer driver"
-	depends on SYS_SUPPORTS_TMU
-	default y
-	help
-	  This enables the build of the TMU timer driver.
-
-config SH_TIMER_CMT
-	bool "CMT timer driver"
-	depends on SYS_SUPPORTS_CMT
-	default y
-	help
-	  This enables build of the CMT timer driver.
-
-config SH_TIMER_MTU2
-	bool "MTU2 timer driver"
-	depends on SYS_SUPPORTS_MTU2
-	default y
-	help
-	  This enables build of the MTU2 timer driver.
-
 config SH_PCLK_FREQ
 	int "Peripheral clock frequency (in Hz)"
 	depends on SH_CLK_CPG_LEGACY

Re: [PATCH v2 00/03] clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits

[Laurent Pinchart]

Hi Magnus,

Thanks for the patches.

On Wednesday 06 November 2013 20:05:08 Magnus Damm wrote:
> clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits (v2)
> 
> [PATCH v2 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI
> [PATCH v2 02/03] ARM: shmobile: Remove CMT, TMU and STI Kconfig entries
> [PATCH v2 03/03] sh: Remove Kconfig entries for TMU, CMT and MTU2
> 
> This is V2 of the SH and ARM mach-shmobile clocksource Kconfig
> consolidation series.
> 
> By sharing Kconfig entires in drivers/clocksource/Kconfig we
> can get rid of duplicated bits that do not belong under arch/.
> 
> With this series applied it is possible to build the timer
> drivers for CMT, MTU2, TMU and STI in case of SH or ARM
> or when COMPILE_TEST is selected.
> 
> Since last time this has been broken out into 3 separate patches,
> where [PATCH v2 01/03] needs to go first, and the other patches
> can be merged any time after without any build or configuration
> issues.
> 
> Thanks to Olof and Simon for feedback! If I interpreted your
> ideas wrong please let me know and I will update.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>

I like this approach.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
>  Written against renesas.git tag renesas-devel-v3.12-rc7-20131101
> 
>  arch/arm/mach-shmobile/Kconfig |   21 ---------------------
>  arch/sh/Kconfig                |   21 ---------------------
>  drivers/clocksource/Kconfig    |   30 ++++++++++++++++++++++++++++++
>  3 files changed, 30 insertions(+), 42 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v3 00/03] clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits

[Magnus Damm]

clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits (v3)

[PATCH v3 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI
[PATCH v3 02/03] ARM: shmobile: Remove CMT, TMU and STI Kconfig entries
[PATCH v2 03/03] sh: Remove Kconfig entries for TMU, CMT and MTU2

This is V3 of the SH and ARM mach-shmobile clocksource Kconfig
consolidation series.

By sharing Kconfig entires in drivers/clocksource/Kconfig we
can get rid of duplicated bits that do not belong under arch/.

With this series applied it is possible to build the timer
drivers for CMT, MTU2, TMU and STI in case of SH or ARM. The timer
drivers are automatically selected by default. It is also possible
to select and/or deselect freely when COMPILE_TEST has been selected.

Since last time this has been broken out into 3 separate patches,
where [PATCH v3 01/03] needs to go first, and the other patches
can be merged any time after without any build or configuration
issues.

Thanks to Daniel, John and Arnd for feedback!

This version tries to find a middle ground by:
 - Not allowing user selection of timers by default
 - Automaticlally select available drivers using SYS_SUPPORTS_xxx.
 - Allow manual override in case of COMPILE_TEST (select and deselect)
 - The system will boot regardless of these settigns
 - In this version the special case menu tand if have been dropped

Slightly different and/or not really handled in this version:
 - The acronym soup is kept as-is for now
 - Allow select _and_ deselect in case of COMPILE_TEST (not only select)
 - Follow same style as SH and select per-SoC SYS_SUPPORTS_xxx

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 Written against renesas.git tag renesas-devel-v3.13-rc2-20131202

 arch/arm/mach-shmobile/Kconfig |   35 +++++++++++++++++------------------
 arch/sh/Kconfig                |   21 ---------------------
 drivers/clocksource/Kconfig    |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 39 deletions(-)

[PATCH v2 03/03] sh: Remove Kconfig entries for TMU, CMT and MTU2

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Now when drivers/clocksource/Kconfig has been
updated with entires for CMT, TMU and MTU2
it is safe to remove these from SH.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 arch/sh/Kconfig |   21 ---------------------
 1 file changed, 21 deletions(-)

--- 0001/arch/sh/Kconfig
+++ work/arch/sh/Kconfig	2013-11-05 21:03:10.000000000 +0900
@@ -565,27 +565,6 @@ source "arch/sh/boards/Kconfig"
 
 menu "Timer and clock configuration"
 
-config SH_TIMER_TMU
-	bool "TMU timer driver"
-	depends on SYS_SUPPORTS_TMU
-	default y
-	help
-	  This enables the build of the TMU timer driver.
-
-config SH_TIMER_CMT
-	bool "CMT timer driver"
-	depends on SYS_SUPPORTS_CMT
-	default y
-	help
-	  This enables build of the CMT timer driver.
-
-config SH_TIMER_MTU2
-	bool "MTU2 timer driver"
-	depends on SYS_SUPPORTS_MTU2
-	default y
-	help
-	  This enables build of the MTU2 timer driver.
-
 config SH_PCLK_FREQ
 	int "Peripheral clock frequency (in Hz)"
 	depends on SH_CLK_CPG_LEGACY

Re: [PATCH v2 03/03] sh: Remove Kconfig entries for TMU, CMT and MTU2

[John Stultz]

On 12/04/2013 12:38 PM, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
>
> Now when drivers/clocksource/Kconfig has been
> updated with entires for CMT, TMU and MTU2
> it is safe to remove these from SH.
>
> Signed-off-by: Magnus Damm <damm@opensource.se>


Acked-by: John Stultz <john.stultz@linaro.org>

[PATCH v4 00/03] clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits

[Magnus Damm]

clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits (v4)

[PATCH v4 01/03] clocksource: Add Kconfig entries for CMT, MTU2, TMU and STI
[PATCH v4 02/03] ARM: shmobile: Remove CMT, TMU and STI Kconfig entries
[PATCH v2 03/03] sh: Remove Kconfig entries for TMU, CMT and MTU2

This is V4 of the SH and ARM mach-shmobile clocksource Kconfig
consolidation series.

By sharing Kconfig entires in drivers/clocksource/Kconfig we
can get rid of duplicated bits that do not belong under arch/.

With this series applied it is possible to build the timer
drivers for CMT, MTU2, TMU and STI in case of SH or ARM. The timer
drivers are automatically selected by default. It is also possible
to select and/or deselect freely when COMPILE_TEST has been selected.

Many thanks to John, Daniel and Arnd for feedback!

Changes since V3:
 - Updated patch 1/3 with more detailed help texts
 - Updated patch 2/3 to fit latest renesas-devel tag, see below
 - Included acks from John Stultz for patch 2/3 and 3/3

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 Written against renesas.git tag renesas-devel-v3.13-rc2-20131211

 arch/arm/mach-shmobile/Kconfig |   35 +++++++++++++++----------------
 arch/sh/Kconfig                |   21 -------------------
 drivers/clocksource/Kconfig    |   44 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 39 deletions(-)

[PATCH v2 03/03] sh: Remove Kconfig entries for TMU, CMT and MTU2

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

Now when drivers/clocksource/Kconfig has been
updated with entires for CMT, TMU and MTU2
it is safe to remove these from SH.

Signed-off-by: Magnus Damm <damm@opensource.se>
Acked-by: John Stultz <john.stultz@linaro.org>
---

 arch/sh/Kconfig |   21 ---------------------
 1 file changed, 21 deletions(-)

--- 0001/arch/sh/Kconfig
+++ work/arch/sh/Kconfig	2013-11-05 21:03:10.000000000 +0900
@@ -565,27 +565,6 @@ source "arch/sh/boards/Kconfig"
 
 menu "Timer and clock configuration"
 
-config SH_TIMER_TMU
-	bool "TMU timer driver"
-	depends on SYS_SUPPORTS_TMU
-	default y
-	help
-	  This enables the build of the TMU timer driver.
-
-config SH_TIMER_CMT
-	bool "CMT timer driver"
-	depends on SYS_SUPPORTS_CMT
-	default y
-	help
-	  This enables build of the CMT timer driver.
-
-config SH_TIMER_MTU2
-	bool "MTU2 timer driver"
-	depends on SYS_SUPPORTS_MTU2
-	default y
-	help
-	  This enables build of the MTU2 timer driver.
-
 config SH_PCLK_FREQ
 	int "Peripheral clock frequency (in Hz)"
 	depends on SH_CLK_CPG_LEGACY