* [PATCH 0/3] clocksource/drivers: introduce DT based selection @ 2017-12-13 18:53 Alexandre Belloni 2017-12-13 18:53 ` [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection Alexandre Belloni ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Alexandre Belloni @ 2017-12-13 18:53 UTC (permalink / raw) To: Rob Herring, Daniel Lezcano Cc: Nicolas Ferre, Mark Rutland, Thomas Gleixner, devicetree, linux-kernel, linux-arm-kernel, 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-13 18:53 [PATCH 0/3] clocksource/drivers: introduce DT based selection Alexandre Belloni @ 2017-12-13 18:53 ` Alexandre Belloni 2017-12-13 22:57 ` Rob Herring ` (2 more replies) 2017-12-13 18:53 ` [PATCH 2/3] clocksource/drivers: timer-of: parse the chosen node Alexandre Belloni 2017-12-13 18:53 ` [PATCH 3/3] clocksource/drivers: integrator-ap: " Alexandre Belloni 2 siblings, 3 replies; 14+ messages in thread From: Alexandre Belloni @ 2017-12-13 18:53 UTC (permalink / raw) To: Rob Herring, Daniel Lezcano Cc: Nicolas Ferre, Mark Rutland, Thomas Gleixner, devicetree, linux-kernel, linux-arm-kernel, 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-13 18:53 ` [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection Alexandre Belloni @ 2017-12-13 22:57 ` Rob Herring 2017-12-14 20:01 ` Boris Brezillon 2017-12-15 11:32 ` Mark Rutland 2017-12-16 1:57 ` Grygorii Strashko 2 siblings, 1 reply; 14+ messages in thread From: Rob Herring @ 2017-12-13 22:57 UTC (permalink / raw) To: Alexandre Belloni Cc: Daniel Lezcano, Nicolas Ferre, Mark Rutland, Thomas Gleixner, devicetree, linux-kernel, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-13 22:57 ` Rob Herring @ 2017-12-14 20:01 ` Boris Brezillon 2017-12-14 20:37 ` Boris Brezillon 2017-12-15 11:40 ` Mark Rutland 0 siblings, 2 replies; 14+ messages in thread From: Boris Brezillon @ 2017-12-14 20:01 UTC (permalink / raw) To: Rob Herring Cc: Alexandre Belloni, Mark Rutland, devicetree, Daniel Lezcano, linux-kernel, Thomas Gleixner, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-14 20:01 ` Boris Brezillon @ 2017-12-14 20:37 ` Boris Brezillon 2017-12-15 11:40 ` Mark Rutland 1 sibling, 0 replies; 14+ messages in thread From: Boris Brezillon @ 2017-12-14 20:37 UTC (permalink / raw) To: Rob Herring Cc: Alexandre Belloni, Mark Rutland, devicetree, Daniel Lezcano, linux-kernel, Thomas Gleixner, linux-arm-kernel 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/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-14 20:01 ` Boris Brezillon 2017-12-14 20:37 ` Boris Brezillon @ 2017-12-15 11:40 ` Mark Rutland 2017-12-15 12:30 ` Boris Brezillon 1 sibling, 1 reply; 14+ messages in thread From: Mark Rutland @ 2017-12-15 11:40 UTC (permalink / raw) To: Boris Brezillon Cc: Rob Herring, Alexandre Belloni, devicetree, Daniel Lezcano, linux-kernel, Thomas Gleixner, linux-arm-kernel 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-15 11:40 ` Mark Rutland @ 2017-12-15 12:30 ` Boris Brezillon 0 siblings, 0 replies; 14+ messages in thread From: Boris Brezillon @ 2017-12-15 12:30 UTC (permalink / raw) To: Mark Rutland Cc: Rob Herring, Alexandre Belloni, devicetree, Daniel Lezcano, linux-kernel, Thomas Gleixner, linux-arm-kernel 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/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-13 18:53 ` [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection Alexandre Belloni 2017-12-13 22:57 ` Rob Herring @ 2017-12-15 11:32 ` Mark Rutland 2017-12-18 19:46 ` Boris Brezillon 2017-12-16 1:57 ` Grygorii Strashko 2 siblings, 1 reply; 14+ messages in thread From: Mark Rutland @ 2017-12-15 11:32 UTC (permalink / raw) To: Alexandre Belloni Cc: Rob Herring, Daniel Lezcano, Nicolas Ferre, Thomas Gleixner, devicetree, linux-kernel, linux-arm-kernel, marc.zyngier 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-15 11:32 ` Mark Rutland @ 2017-12-18 19:46 ` Boris Brezillon 0 siblings, 0 replies; 14+ messages in thread From: Boris Brezillon @ 2017-12-18 19:46 UTC (permalink / raw) To: Mark Rutland Cc: Alexandre Belloni, devicetree, marc.zyngier, Daniel Lezcano, linux-kernel, Rob Herring, Thomas Gleixner, linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-13 18:53 ` [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection Alexandre Belloni 2017-12-13 22:57 ` Rob Herring 2017-12-15 11:32 ` Mark Rutland @ 2017-12-16 1:57 ` Grygorii Strashko 2017-12-18 17:12 ` Tony Lindgren 2017-12-19 23:20 ` Rob Herring 2 siblings, 2 replies; 14+ messages in thread From: Grygorii Strashko @ 2017-12-16 1:57 UTC (permalink / raw) To: Alexandre Belloni, Rob Herring, Daniel Lezcano Cc: Mark Rutland, devicetree, linux-kernel, Thomas Gleixner, linux-arm-kernel, Boris Brezillon, Mark Rutland, Tony Lindgren 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-16 1:57 ` Grygorii Strashko @ 2017-12-18 17:12 ` Tony Lindgren 2017-12-19 23:20 ` Rob Herring 1 sibling, 0 replies; 14+ messages in thread From: Tony Lindgren @ 2017-12-18 17:12 UTC (permalink / raw) To: Grygorii Strashko Cc: Alexandre Belloni, Rob Herring, Daniel Lezcano, Mark Rutland, devicetree, linux-kernel, Thomas Gleixner, linux-arm-kernel, Boris Brezillon * 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection 2017-12-16 1:57 ` Grygorii Strashko 2017-12-18 17:12 ` Tony Lindgren @ 2017-12-19 23:20 ` Rob Herring 1 sibling, 0 replies; 14+ messages in thread From: Rob Herring @ 2017-12-19 23:20 UTC (permalink / raw) To: Grygorii Strashko Cc: Alexandre Belloni, Daniel Lezcano, Mark Rutland, devicetree, linux-kernel, Thomas Gleixner, linux-arm-kernel, Boris Brezillon, Tony Lindgren 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] clocksource/drivers: timer-of: parse the chosen node 2017-12-13 18:53 [PATCH 0/3] clocksource/drivers: introduce DT based selection Alexandre Belloni 2017-12-13 18:53 ` [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection Alexandre Belloni @ 2017-12-13 18:53 ` Alexandre Belloni 2017-12-13 18:53 ` [PATCH 3/3] clocksource/drivers: integrator-ap: " Alexandre Belloni 2 siblings, 0 replies; 14+ messages in thread From: Alexandre Belloni @ 2017-12-13 18:53 UTC (permalink / raw) To: Rob Herring, Daniel Lezcano Cc: Nicolas Ferre, Mark Rutland, Thomas Gleixner, devicetree, linux-kernel, linux-arm-kernel, 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] clocksource/drivers: integrator-ap: parse the chosen node 2017-12-13 18:53 [PATCH 0/3] clocksource/drivers: introduce DT based selection Alexandre Belloni 2017-12-13 18:53 ` [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection Alexandre Belloni 2017-12-13 18:53 ` [PATCH 2/3] clocksource/drivers: timer-of: parse the chosen node Alexandre Belloni @ 2017-12-13 18:53 ` Alexandre Belloni 2 siblings, 0 replies; 14+ messages in thread From: Alexandre Belloni @ 2017-12-13 18:53 UTC (permalink / raw) To: Rob Herring, Daniel Lezcano Cc: Nicolas Ferre, Mark Rutland, Thomas Gleixner, devicetree, linux-kernel, linux-arm-kernel, 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-12-19 23:20 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-13 18:53 [PATCH 0/3] clocksource/drivers: introduce DT based selection Alexandre Belloni 2017-12-13 18:53 ` [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection Alexandre Belloni 2017-12-13 22:57 ` Rob Herring 2017-12-14 20:01 ` Boris Brezillon 2017-12-14 20:37 ` Boris Brezillon 2017-12-15 11:40 ` Mark Rutland 2017-12-15 12:30 ` Boris Brezillon 2017-12-15 11:32 ` Mark Rutland 2017-12-18 19:46 ` Boris Brezillon 2017-12-16 1:57 ` Grygorii Strashko 2017-12-18 17:12 ` Tony Lindgren 2017-12-19 23:20 ` Rob Herring 2017-12-13 18:53 ` [PATCH 2/3] clocksource/drivers: timer-of: parse the chosen node Alexandre Belloni 2017-12-13 18:53 ` [PATCH 3/3] clocksource/drivers: integrator-ap: " Alexandre Belloni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).