[PATCH 0/3] clocksource/drivers: introduce DT based selection

[PATCH 0/3] clocksource/drivers: introduce DT based selection

[Alexandre Belloni]

Hi,

Currently, many drivers implement their own strategy when trying to find
which timer to use as the clocksource or the clockevent.

The main issue is that this selection is happen early in the boot
process and the kernel doesn't always have all the information to take
that decision.

So we end up with suboptimal solutions, especially in a multiplatform
kernel setting, as the MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK kernel
config option.

There is also the clocksource kernel parameter only implemented in
mach-omap2.

Other drivers are registering the first seen timer as a clockevent, the
other one as a clocksource.

Also, this will help in the goal of separating clocksource and
clockevent drivers (see 376bc27150f180d9f5eddec6a14117780177589d)

Patch 1 documents the binding, patch 2 implements the parsing of the
chosen node and finally, patch 3 makes use of the parsing in a driver to
give an overview of how it is working.

Alexandre Belloni (3):
  dt-bindings: chosen: Add clocksource and clockevent selection
  clocksource/drivers: timer-of: parse the chosen node
  clocksource/drivers: integrator-ap: parse the chosen node

 Documentation/devicetree/bindings/chosen.txt | 20 ++++++++++++++++++++
 drivers/clocksource/Kconfig                  |  1 +
 drivers/clocksource/timer-integrator-ap.c    | 11 +++++++++++
 drivers/clocksource/timer-of.c               | 22 ++++++++++++++++++++++
 drivers/clocksource/timer-of.h               |  3 +++
 5 files changed, 57 insertions(+)

-- 
2.15.1

[PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Alexandre Belloni]

The clocksource and clockevent timer are probed early in the boot process.
At that time it is difficult for linux to know whether a particular timer
can be used as the clocksource or the clockevent or by another driver,
especially when they are all identical or have similar features.

Until now, multiple strategies have been used to solve that:
 - use Kconfig option as MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK
 - use a kernel parameter as the "clocksource" early_param in mach-omap2
 - registering the first seen timer as a clockevent and the second one as
 a clocksource as in rk_timer_init or dw_apb_timer_init

Add a linux,clocksource and a linux,clockevent node in chosen with a timer
property pointing to the timer to use. Other properties, like the targeted
precision may be added later.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 Documentation/devicetree/bindings/chosen.txt | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
index e3b13ea7d2ae..c7ee3ecb5276 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -120,3 +120,23 @@ e.g.
 While this property does not represent a real hardware, the address
 and the size are expressed in #address-cells and #size-cells,
 respectively, of the root node.
+
+linux,clocksource and linux,clockevent
+--------------------------------------
+
+Those nodes have a timer property. This property is a phandle to the timer to be
+chosen as the clocksource or clockevent. This is only useful when the platform
+has multiple identical timers and it is not possible to let linux make the
+correct choice.
+
+/ {
+	chosen {
+		linux,clocksource {
+			timer = <&timer0>;
+		};
+
+		linux,clockevent {
+			timer = <&timer1>;
+		};
+	};
+};
-- 
2.15.1

[PATCH 2/3] clocksource/drivers: timer-of: parse the chosen node

[Alexandre Belloni]

Add a way for drivers to know whether the timer they are currently handling
is a clocksource or a clockevent.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/clocksource/timer-of.c | 22 ++++++++++++++++++++++
 drivers/clocksource/timer-of.h |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index a31990408153..71680eacd390 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -195,3 +195,25 @@ void __init timer_of_cleanup(struct timer_of *to)
 	if (to->flags & TIMER_OF_BASE)
 		timer_base_exit(&to->of_base);
 }
+
+int __init timer_of_is_type(struct device_node *np, char *type)
+{
+	struct device_node *node, *timer;
+
+	if (!of_chosen)
+		return 0;
+
+	node = of_get_child_by_name(of_chosen, type);
+	if (!node)
+		return 0;
+
+	timer = of_parse_phandle(node, "timer", 0);
+	of_node_put(node);
+	if (!timer)
+		return 0;
+
+	if (timer == np)
+		return 1;
+
+	return 0;
+}
diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
index 3f708f1be43d..24923cfe748d 100644
--- a/drivers/clocksource/timer-of.h
+++ b/drivers/clocksource/timer-of.h
@@ -70,4 +70,7 @@ extern int __init timer_of_init(struct device_node *np,
 
 extern void __init timer_of_cleanup(struct timer_of *to);
 
+extern int __init timer_of_is_type(struct device_node *np, char *type);
+#define timer_of_is_clocksource(np) timer_of_is_type(np, "linux,clocksource")
+#define timer_of_is_clockevent(np) timer_of_is_type(np, "linux,clockevent")
 #endif
-- 
2.15.1

[PATCH 3/3] clocksource/drivers: integrator-ap: parse the chosen node

[Alexandre Belloni]

The driver currently uses aliases to know whether the timer is the
clocksource or the clockevent. Add the /chosen/linux,clocksource and
/chosen/linux,clockevent parsing while keeping backward compatibility.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/clocksource/Kconfig               |  1 +
 drivers/clocksource/timer-integrator-ap.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 9a22d1048fcf..053aca99caf7 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -212,6 +212,7 @@ config KEYSTONE_TIMER
 config INTEGRATOR_AP_TIMER
 	bool "Integrator-ap timer driver" if COMPILE_TEST
 	select CLKSRC_MMIO
+	select TIMER_OF
 	help
 	  Enables support for the Integrator-ap timer.
 
diff --git a/drivers/clocksource/timer-integrator-ap.c b/drivers/clocksource/timer-integrator-ap.c
index 62d24690ba02..8f38be724aff 100644
--- a/drivers/clocksource/timer-integrator-ap.c
+++ b/drivers/clocksource/timer-integrator-ap.c
@@ -197,6 +197,17 @@ static int __init integrator_ap_timer_init_of(struct device_node *node)
 	rate = clk_get_rate(clk);
 	writel(0, base + TIMER_CTRL);
 
+	if (timer_of_is_clocksource(node))
+		/* The primary timer lacks IRQ, use as clocksource */
+		return integrator_clocksource_init(rate, base);
+
+	if (timer_of_is_clockevent(node)) {
+		/* The secondary timer will drive the clock event */
+		irq = irq_of_parse_and_map(node, 0);
+		return integrator_clockevent_init(rate, base, irq);
+	}
+
+	/* DT ABI compatibility below */
 	err = of_property_read_string(of_aliases,
 				"arm,timer-primary", &path);
 	if (err) {
-- 
2.15.1

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Rob Herring]

On Wed, Dec 13, 2017 at 12:53 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> The clocksource and clockevent timer are probed early in the boot process.
> At that time it is difficult for linux to know whether a particular timer
> can be used as the clocksource or the clockevent or by another driver,
> especially when they are all identical or have similar features.

If all identical, then it shouldn't matter. "similar" means some
difference. Describe those differences.

> Until now, multiple strategies have been used to solve that:
>  - use Kconfig option as MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK

Compile time probably means only one option is really used.

>  - use a kernel parameter as the "clocksource" early_param in mach-omap2

Yeah, OMAP was one of the previous times this came up and also
attempted something like this. This parameter predates selecting
timers based on features described in DT. Look at commit
2eb03937df3ebc (ARM: OMAP3: Update clocksource timer selection).

>  - registering the first seen timer as a clockevent and the second one as
>  a clocksource as in rk_timer_init or dw_apb_timer_init
>
> Add a linux,clocksource and a linux,clockevent node in chosen with a timer
> property pointing to the timer to use. Other properties, like the targeted
> precision may be added later.

Open ended expansion of this does not help convince me it is needed.

Rob

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Boris Brezillon]

Hi Rob,

On Wed, 13 Dec 2017 16:57:50 -0600
Rob Herring <robh+dt@kernel.org> wrote:

> On Wed, Dec 13, 2017 at 12:53 PM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
> > The clocksource and clockevent timer are probed early in the boot process.
> > At that time it is difficult for linux to know whether a particular timer
> > can be used as the clocksource or the clockevent or by another driver,
> > especially when they are all identical or have similar features.  
> 
> If all identical, then it shouldn't matter. "similar" means some
> difference. Describe those differences.

We had this discussion already. Those timers might be connected to
external pins and may serve the role of PWM generators or capture
devices. We can also chain timers and provide a clocksource with a
better resolution or one that wraps less often. In the end it's all
about how the user plans to use its system, and this has to be
described somehow. Assuming that the software can decide by itself
which timer should be used or how many of them should be chained is
just an utopia.

> 
> > Until now, multiple strategies have been used to solve that:
> >  - use Kconfig option as MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK  
> 
> Compile time probably means only one option is really used.

Compile time selection prevents using the same kernel for different
boards (in other words, no multi-v7), so not really an option here.

> 
> >  - use a kernel parameter as the "clocksource" early_param in mach-omap2  
> 
> Yeah, OMAP was one of the previous times this came up and also
> attempted something like this. This parameter predates selecting
> timers based on features described in DT. Look at commit
> 2eb03937df3ebc (ARM: OMAP3: Update clocksource timer selection).

Then, would you accept to have a property saying that a specific timer
is a free-running timer (suitable for clocksource) or a programmable
timer (suitable for clkevent)? Or are you saying that you don't like the
way timers are differentiated on omap?

> 
> >  - registering the first seen timer as a clockevent and the second one as
> >  a clocksource as in rk_timer_init or dw_apb_timer_init
> >
> > Add a linux,clocksource and a linux,clockevent node in chosen with a timer
> > property pointing to the timer to use. Other properties, like the targeted
> > precision may be added later.  
> 
> Open ended expansion of this does not help convince me it is needed.

It's not an open ended expansion, we're just trying to find a way to
describe which timer blocks should be used as free running timers
(clksource) and which one should be used as programmable timers
(clkevent). Automatically selecting timer blocks to assign to the
clkevent or clocksource is not so easy (as has been explained earlier)
because at the time the system registers its clksource/clkevent devices
we might not have all the necessary information to know which timer
blocks will be reserved for other usage later on. The use case I have
in mind is DT overlays, where one of the overlay is using a timer as a
PWM generator. If the clkevent or clksource has already claimed the
timer connected to the pins the overlay is using, then we're screwed,
and there's no way the system can know that ahead of time except by
pre-assigning a timer to the clksource or clkevent feature.

So really, we need a way to assign a specific timer to a specific
feature. You've refused the different proposals we made so far, so
what's your alternative? I mean a real alternative that solve the "an
auto-selected timer might be claimed by another driver at some point"
problem.

Thanks,

Boris

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Boris Brezillon]

On Thu, 14 Dec 2017 21:01:20 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> Hi Rob,
> 
> On Wed, 13 Dec 2017 16:57:50 -0600
> Rob Herring <robh+dt@kernel.org> wrote:
> 
> > On Wed, Dec 13, 2017 at 12:53 PM, Alexandre Belloni
> > <alexandre.belloni@free-electrons.com> wrote:  
> > > The clocksource and clockevent timer are probed early in the boot process.
> > > At that time it is difficult for linux to know whether a particular timer
> > > can be used as the clocksource or the clockevent or by another driver,
> > > especially when they are all identical or have similar features.    
> > 
> > If all identical, then it shouldn't matter. "similar" means some
> > difference. Describe those differences.  
> 
> We had this discussion already. Those timers might be connected to
> external pins and may serve the role of PWM generators or capture
> devices. We can also chain timers and provide a clocksource with a
> better resolution or one that wraps less often. In the end it's all
> about how the user plans to use its system, and this has to be
> described somehow. Assuming that the software can decide by itself
> which timer should be used or how many of them should be chained is
> just an utopia.
> 
> >   
> > > Until now, multiple strategies have been used to solve that:
> > >  - use Kconfig option as MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK    
> > 
> > Compile time probably means only one option is really used.  
> 
> Compile time selection prevents using the same kernel for different
> boards (in other words, no multi-v7), so not really an option here.
> 
> >   
> > >  - use a kernel parameter as the "clocksource" early_param in mach-omap2    
> > 
> > Yeah, OMAP was one of the previous times this came up and also
> > attempted something like this. This parameter predates selecting
> > timers based on features described in DT. Look at commit
> > 2eb03937df3ebc (ARM: OMAP3: Update clocksource timer selection).  
> 
> Then, would you accept to have a property saying that a specific timer
> is a free-running timer (suitable for clocksource) or a programmable
> timer (suitable for clkevent)? Or are you saying that you don't like the
> way timers are differentiated on omap?
> 
> >   
> > >  - registering the first seen timer as a clockevent and the second one as
> > >  a clocksource as in rk_timer_init or dw_apb_timer_init
> > >
> > > Add a linux,clocksource and a linux,clockevent node in chosen with a timer
> > > property pointing to the timer to use. Other properties, like the targeted
> > > precision may be added later.    
> > 
> > Open ended expansion of this does not help convince me it is needed.  
> 
> It's not an open ended expansion, we're just trying to find a way to
> describe which timer blocks should be used as free running timers
> (clksource) and which one should be used as programmable timers
> (clkevent). Automatically selecting timer blocks to assign to the
> clkevent or clocksource is not so easy (as has been explained earlier)
> because at the time the system registers its clksource/clkevent devices
> we might not have all the necessary information to know which timer
> blocks will be reserved for other usage later on. The use case I have
> in mind is DT overlays, where one of the overlay is using a timer as a
> PWM generator. If the clkevent or clksource has already claimed the
> timer connected to the pins the overlay is using, then we're screwed,
> and there's no way the system can know that ahead of time except by
> pre-assigning a timer to the clksource or clkevent feature.
> 
> So really, we need a way to assign a specific timer to a specific
> feature. You've refused the different proposals we made so far, so
> what's your alternative? I mean a real alternative that solve the "an
> auto-selected timer might be claimed by another driver at some point"
> problem.

Okay, it seems I was wrong, you already agreed on a generic
atmel,tcb-timer compatible [1], so it seems the only thing we're missing
is a way to assign a timer to a clocksource or a clkevent.

[1]https://patchwork.kernel.org/patch/9755341/

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Mark Rutland]

Hi,

On Wed, Dec 13, 2017 at 07:53:11PM +0100, Alexandre Belloni wrote:
> The clocksource and clockevent timer are probed early in the boot process.
> At that time it is difficult for linux to know whether a particular timer
> can be used as the clocksource or the clockevent or by another driver,
> especially when they are all identical or have similar features.

I think that to solve this problem, we need to stop treating
clocksources and clockevent devices as completely separate device types,
and instead treat them as particular cases of a more general clock
device.

That way, a driver can register a single device, with flags saying
whether it is:

* a clocksource only
* a clockevent device only
* both a clocksource and clockevent device
* both, but mutually exclusive at runtime

... and thus drivers don't have to make an impossible decision up-front
as to how the device should be used.

As more devices get registered, the core timekeeping code can improve on
its choices and re-assign devices.

That doesn't solve the case where a clock device may use resources we
want for something else, but I think we can solve that separately.

Thanks,
Mark.

> Until now, multiple strategies have been used to solve that:
>  - use Kconfig option as MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK
>  - use a kernel parameter as the "clocksource" early_param in mach-omap2
>  - registering the first seen timer as a clockevent and the second one as
>  a clocksource as in rk_timer_init or dw_apb_timer_init
> 
> Add a linux,clocksource and a linux,clockevent node in chosen with a timer
> property pointing to the timer to use. Other properties, like the targeted
> precision may be added later.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/chosen.txt | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
> index e3b13ea7d2ae..c7ee3ecb5276 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -120,3 +120,23 @@ e.g.
>  While this property does not represent a real hardware, the address
>  and the size are expressed in #address-cells and #size-cells,
>  respectively, of the root node.
> +
> +linux,clocksource and linux,clockevent
> +--------------------------------------
> +
> +Those nodes have a timer property. This property is a phandle to the timer to be
> +chosen as the clocksource or clockevent. This is only useful when the platform
> +has multiple identical timers and it is not possible to let linux make the
> +correct choice.
> +
> +/ {
> +	chosen {
> +		linux,clocksource {
> +			timer = <&timer0>;
> +		};
> +
> +		linux,clockevent {
> +			timer = <&timer1>;
> +		};
> +	};
> +};
> -- 
> 2.15.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Mark Rutland]

Hi,

On Thu, Dec 14, 2017 at 09:01:20PM +0100, Boris Brezillon wrote:
> On Wed, 13 Dec 2017 16:57:50 -0600
> Rob Herring <robh+dt@kernel.org> wrote:
> > On Wed, Dec 13, 2017 at 12:53 PM, Alexandre Belloni
> > <alexandre.belloni@free-electrons.com> wrote:

> > > The clocksource and clockevent timer are probed early in the boot process.
> > > At that time it is difficult for linux to know whether a particular timer
> > > can be used as the clocksource or the clockevent or by another driver,
> > > especially when they are all identical or have similar features.  
> > 
> > If all identical, then it shouldn't matter. "similar" means some
> > difference. Describe those differences.
> 
> We had this discussion already. Those timers might be connected to
> external pins and may serve the role of PWM generators or capture
> devices. We can also chain timers and provide a clocksource with a
> better resolution or one that wraps less often. 

Could you elaborate on the chaining case? I haven't encountered that,
and at the moment I'm not sure I follow how that works.

> > >  - registering the first seen timer as a clockevent and the second one as
> > >  a clocksource as in rk_timer_init or dw_apb_timer_init
> > >
> > > Add a linux,clocksource and a linux,clockevent node in chosen with a timer
> > > property pointing to the timer to use. Other properties, like the targeted
> > > precision may be added later.  
> > 
> > Open ended expansion of this does not help convince me it is needed.
> 
> It's not an open ended expansion, we're just trying to find a way to
> describe which timer blocks should be used as free running timers
> (clksource) and which one should be used as programmable timers
> (clkevent). Automatically selecting timer blocks to assign to the
> clkevent or clocksource is not so easy (as has been explained earlier)
> because at the time the system registers its clksource/clkevent devices
> we might not have all the necessary information to know which timer
> blocks will be reserved for other usage later on. The use case I have
> in mind is DT overlays, where one of the overlay is using a timer as a
> PWM generator. If the clkevent or clksource has already claimed the
> timer connected to the pins the overlay is using, then we're screwed,
> and there's no way the system can know that ahead of time except by
> pre-assigning a timer to the clksource or clkevent feature.

I guess that might work for the boot-time overlay case, where the user
knows ahead-of-time that there will be a conflict for resources, but
that doesn't help with the dynamic overlay case, since the user can't
know what conflicts there will be.

Can we attempt to unregister the clock device in that case, when the PWM
is requested? If the timekeeping core can select another device, then
we're free to use this one as a PWM. If not, then we're stuck anyway.

Thanks,
Mark.

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Boris Brezillon]

On Fri, 15 Dec 2017 11:40:04 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

> Hi,
> 
> On Thu, Dec 14, 2017 at 09:01:20PM +0100, Boris Brezillon wrote:
> > On Wed, 13 Dec 2017 16:57:50 -0600
> > Rob Herring <robh+dt@kernel.org> wrote:  
> > > On Wed, Dec 13, 2017 at 12:53 PM, Alexandre Belloni
> > > <alexandre.belloni@free-electrons.com> wrote:  
> 
> > > > The clocksource and clockevent timer are probed early in the boot process.
> > > > At that time it is difficult for linux to know whether a particular timer
> > > > can be used as the clocksource or the clockevent or by another driver,
> > > > especially when they are all identical or have similar features.    
> > > 
> > > If all identical, then it shouldn't matter. "similar" means some
> > > difference. Describe those differences.  
> > 
> > We had this discussion already. Those timers might be connected to
> > external pins and may serve the role of PWM generators or capture
> > devices. We can also chain timers and provide a clocksource with a
> > better resolution or one that wraps less often.   
> 
> Could you elaborate on the chaining case? I haven't encountered that,
> and at the moment I'm not sure I follow how that works.

In a TCB (Timer Counter Block) you have 3 TC (Timer Counters). Each
timer can take a regular clock (+a divider) as a source, but it can
also take the output of the previous channel (so channel 1 can take
the output of channel 0, channel 2 the output of channel 1, and channel
0 the output of channel 2). A TC output can be configured to toggle
every time the counter overflows. So when you chain 2 channels, you
double the number of bits of your counter. This is particularly
interesting if you want to create a precise timer that has an
acceptable wraparound period, otherwise, you'll have to choose between
a timer with a poor precision and an acceptable wraparound period, and
a timer with a good precision and a small wraparound.

> 
> > > >  - registering the first seen timer as a clockevent and the second one as
> > > >  a clocksource as in rk_timer_init or dw_apb_timer_init
> > > >
> > > > Add a linux,clocksource and a linux,clockevent node in chosen with a timer
> > > > property pointing to the timer to use. Other properties, like the targeted
> > > > precision may be added later.    
> > > 
> > > Open ended expansion of this does not help convince me it is needed.  
> > 
> > It's not an open ended expansion, we're just trying to find a way to
> > describe which timer blocks should be used as free running timers
> > (clksource) and which one should be used as programmable timers
> > (clkevent). Automatically selecting timer blocks to assign to the
> > clkevent or clocksource is not so easy (as has been explained earlier)
> > because at the time the system registers its clksource/clkevent devices
> > we might not have all the necessary information to know which timer
> > blocks will be reserved for other usage later on. The use case I have
> > in mind is DT overlays, where one of the overlay is using a timer as a
> > PWM generator. If the clkevent or clksource has already claimed the
> > timer connected to the pins the overlay is using, then we're screwed,
> > and there's no way the system can know that ahead of time except by
> > pre-assigning a timer to the clksource or clkevent feature.  
> 
> I guess that might work for the boot-time overlay case, where the user
> knows ahead-of-time that there will be a conflict for resources, but
> that doesn't help with the dynamic overlay case, since the user can't
> know what conflicts there will be.
> 
> Can we attempt to unregister the clock device in that case, when the PWM
> is requested? If the timekeeping core can select another device, then
> we're free to use this one as a PWM. If not, then we're stuck anyway.

Actually, the problem had already been solved with the "atmel,tcb-timer"
compatible. When this compatible is used we know the TC(s) can be used
as generic timers. If you want to reserve TC(s) for other usage (like a
PWM), you just leave the TC undefined in the device tree and an overlay
can add a new node reserving this TC afterwards. This approach has
already been accepted by Rob [1].

So right now, the problem we have is how to assign a specific timer to
a clockevent or clocksource 'device'.

[1]https://patchwork.kernel.org/patch/9755341/

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Grygorii Strashko]

On 12/13/2017 12:53 PM, Alexandre Belloni wrote:
> The clocksource and clockevent timer are probed early in the boot process.
> At that time it is difficult for linux to know whether a particular timer
> can be used as the clocksource or the clockevent or by another driver,
> especially when they are all identical or have similar features.
> 
> Until now, multiple strategies have been used to solve that:
>   - use Kconfig option as MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK
>   - use a kernel parameter as the "clocksource" early_param in mach-omap2
>   - registering the first seen timer as a clockevent and the second one as
>   a clocksource as in rk_timer_init or dw_apb_timer_init
> 
> Add a linux,clocksource and a linux,clockevent node in chosen with a timer
> property pointing to the timer to use. Other properties, like the targeted
> precision may be added later.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>   Documentation/devicetree/bindings/chosen.txt | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
> index e3b13ea7d2ae..c7ee3ecb5276 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -120,3 +120,23 @@ e.g.
>   While this property does not represent a real hardware, the address
>   and the size are expressed in #address-cells and #size-cells,
>   respectively, of the root node.
> +
> +linux,clocksource and linux,clockevent
> +--------------------------------------
> +
> +Those nodes have a timer property. This property is a phandle to the timer to be
> +chosen as the clocksource or clockevent. This is only useful when the platform
> +has multiple identical timers and it is not possible to let linux make the
> +correct choice.
> +
> +/ {
> +	chosen {
> +		linux,clocksource {
> +			timer = <&timer0>;
> +		};
> +
> +		linux,clockevent {
> +			timer = <&timer1>;
> +		};
> +	};
> +};
> 

It'd be nice if smth. like this will actually happen, as on some OMAP
platforms can be up to 3-4 alternatives for each clocksource/clockevent and
different combination need to be selected depending on SoC and platform
(and sometime - use case) which is pain in multi-platform environment now.

But more important note from my side is clocksource and clockevent selections seems 
not enough :( There are also sched clock (sched_clock_register()) and delay_timer
 (register_current_timer_delay() at least on ARM).
Both above can't be unregistered (at least last time I've checked).

-- 
regards,
-grygorii

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Tony Lindgren]

* Grygorii Strashko <grygorii.strashko@ti.com> [171216 01:59]:
> On 12/13/2017 12:53 PM, Alexandre Belloni wrote:
> > +/ {
> > +	chosen {
> > +		linux,clocksource {
> > +			timer = <&timer0>;
> > +		};
> > +
> > +		linux,clockevent {
> > +			timer = <&timer1>;
> > +		};
> > +	};
> > +};
> > 
> 
> It'd be nice if smth. like this will actually happen, as on some OMAP
> platforms can be up to 3-4 alternatives for each clocksource/clockevent and
> different combination need to be selected depending on SoC and platform
> (and sometime - use case) which is pain in multi-platform environment now.

Yeah agreed.

> But more important note from my side is clocksource and clockevent selections seems 
> not enough :( There are also sched clock (sched_clock_register()) and delay_timer
>  (register_current_timer_delay() at least on ARM).
> Both above can't be unregistered (at least last time I've checked).

So maybe they should be also defined the sam way as above or is
there something more to it?

Regards,

Tony

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Boris Brezillon]

On Fri, 15 Dec 2017 11:32:42 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

> Hi,
> 
> On Wed, Dec 13, 2017 at 07:53:11PM +0100, Alexandre Belloni wrote:
> > The clocksource and clockevent timer are probed early in the boot process.
> > At that time it is difficult for linux to know whether a particular timer
> > can be used as the clocksource or the clockevent or by another driver,
> > especially when they are all identical or have similar features.  
> 
> I think that to solve this problem, we need to stop treating
> clocksources and clockevent devices as completely separate device types,
> and instead treat them as particular cases of a more general clock
> device.
> 
> That way, a driver can register a single device, with flags saying
> whether it is:
> 
> * a clocksource only
> * a clockevent device only
> * both a clocksource and clockevent device
> * both, but mutually exclusive at runtime
> 
> ... and thus drivers don't have to make an impossible decision up-front
> as to how the device should be used.
> 
> As more devices get registered, the core timekeeping code can improve on
> its choices and re-assign devices.

I'm coming back to the Atmel TCB use case. Say you have 2 'timers' one
has been assigned 2 channels (a channel is what I call TC in my
previous explanation) and the other one only one channel. Both timers
are capable of acting as a clockevent or a clocksource device (not
both at the same time). Now, the question is, how do you decide which
one is assigned the clocksource role and which one is assigned the
clkevent role. Do you plan to pass the expected clocksource/clkevent
properties (precision, max value, wraparound period, ...) through the
device tree so that the framework can decide which timer is best suited
for a specific feature?

> 
> That doesn't solve the case where a clock device may use resources we
> want for something else, but I think we can solve that separately.
> 
> Thanks,
> Mark.
> 
> > Until now, multiple strategies have been used to solve that:
> >  - use Kconfig option as MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK
> >  - use a kernel parameter as the "clocksource" early_param in mach-omap2
> >  - registering the first seen timer as a clockevent and the second one as
> >  a clocksource as in rk_timer_init or dw_apb_timer_init
> > 
> > Add a linux,clocksource and a linux,clockevent node in chosen with a timer
> > property pointing to the timer to use. Other properties, like the targeted
> > precision may be added later.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > ---
> >  Documentation/devicetree/bindings/chosen.txt | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
> > index e3b13ea7d2ae..c7ee3ecb5276 100644
> > --- a/Documentation/devicetree/bindings/chosen.txt
> > +++ b/Documentation/devicetree/bindings/chosen.txt
> > @@ -120,3 +120,23 @@ e.g.
> >  While this property does not represent a real hardware, the address
> >  and the size are expressed in #address-cells and #size-cells,
> >  respectively, of the root node.
> > +
> > +linux,clocksource and linux,clockevent
> > +--------------------------------------
> > +
> > +Those nodes have a timer property. This property is a phandle to the timer to be
> > +chosen as the clocksource or clockevent. This is only useful when the platform
> > +has multiple identical timers and it is not possible to let linux make the
> > +correct choice.
> > +
> > +/ {
> > +	chosen {
> > +		linux,clocksource {
> > +			timer = <&timer0>;
> > +		};
> > +
> > +		linux,clockevent {
> > +			timer = <&timer1>;
> > +		};
> > +	};
> > +};
> > -- 
> > 2.15.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html  
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection

[Rob Herring]

On Fri, Dec 15, 2017 at 7:57 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
>
>
> On 12/13/2017 12:53 PM, Alexandre Belloni wrote:
>> The clocksource and clockevent timer are probed early in the boot process.
>> At that time it is difficult for linux to know whether a particular timer
>> can be used as the clocksource or the clockevent or by another driver,
>> especially when they are all identical or have similar features.
>>
>> Until now, multiple strategies have been used to solve that:
>>   - use Kconfig option as MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK
>>   - use a kernel parameter as the "clocksource" early_param in mach-omap2
>>   - registering the first seen timer as a clockevent and the second one as
>>   a clocksource as in rk_timer_init or dw_apb_timer_init
>>
>> Add a linux,clocksource and a linux,clockevent node in chosen with a timer
>> property pointing to the timer to use. Other properties, like the targeted
>> precision may be added later.
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> ---
>>   Documentation/devicetree/bindings/chosen.txt | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
>> index e3b13ea7d2ae..c7ee3ecb5276 100644
>> --- a/Documentation/devicetree/bindings/chosen.txt
>> +++ b/Documentation/devicetree/bindings/chosen.txt
>> @@ -120,3 +120,23 @@ e.g.
>>   While this property does not represent a real hardware, the address
>>   and the size are expressed in #address-cells and #size-cells,
>>   respectively, of the root node.
>> +
>> +linux,clocksource and linux,clockevent
>> +--------------------------------------
>> +
>> +Those nodes have a timer property. This property is a phandle to the timer to be
>> +chosen as the clocksource or clockevent. This is only useful when the platform
>> +has multiple identical timers and it is not possible to let linux make the
>> +correct choice.
>> +
>> +/ {
>> +     chosen {
>> +             linux,clocksource {
>> +                     timer = <&timer0>;
>> +             };
>> +
>> +             linux,clockevent {
>> +                     timer = <&timer1>;
>> +             };
>> +     };
>> +};
>>
>
> It'd be nice if smth. like this will actually happen, as on some OMAP
> platforms can be up to 3-4 alternatives for each clocksource/clockevent and
> different combination need to be selected depending on SoC and platform
> (and sometime - use case) which is pain in multi-platform environment now.

Can we have some concrete examples with why the current selection
logic is broken.

For use-cases, if a board maker can't make that decision, then I think
those should be kernel command-line options (if boot time) or sysfs
controlled.

> But more important note from my side is clocksource and clockevent selections seems
> not enough :( There are also sched clock (sched_clock_register()) and delay_timer
>  (register_current_timer_delay() at least on ARM).
> Both above can't be unregistered (at least last time I've checked).

Sounds like an OS problem...

Rob