diff mbox series

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

Message ID 20171213185313.20017-2-alexandre.belloni@free-electrons.com
State Changes Requested, archived
Headers show
Series clocksource/drivers: introduce DT based selection | expand

Commit Message

Alexandre Belloni Dec. 13, 2017, 6:53 p.m. UTC
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(+)

Comments

Rob Herring Dec. 13, 2017, 10:57 p.m. UTC | #1
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
--
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
Boris Brezillon Dec. 14, 2017, 8:01 p.m. UTC | #2
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
--
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
Boris Brezillon Dec. 14, 2017, 8:37 p.m. UTC | #3
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/
--
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
Mark Rutland Dec. 15, 2017, 11:32 a.m. UTC | #4
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
--
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
Mark Rutland Dec. 15, 2017, 11:40 a.m. UTC | #5
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.
--
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
Boris Brezillon Dec. 15, 2017, 12:30 p.m. UTC | #6
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/

--
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
Grygorii Strashko Dec. 16, 2017, 1:57 a.m. UTC | #7
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).
Tony Lindgren Dec. 18, 2017, 5:12 p.m. UTC | #8
* 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
--
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
Boris Brezillon Dec. 18, 2017, 7:46 p.m. UTC | #9
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

--
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
Rob Herring Dec. 19, 2017, 11:20 p.m. UTC | #10
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
--
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
diff mbox series

Patch

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>;
+		};
+	};
+};