* [PATCH] ARM: davinci: convert to clockevents_config_and_register
@ 2013-09-24 22:02 Uwe Kleine-König
2013-10-08 14:57 ` Uwe Kleine-König
2013-10-12 16:22 ` Sekhar Nori
0 siblings, 2 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2013-09-24 22:02 UTC (permalink / raw)
To: linux-arm-kernel
clockevents_config_and_register is superior compared to setting
shift/mult and {min,max}_delta_ns by hand.
Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
Hello,
I wonder where the 50 us limit comes from. This is not a hardware limit,
is it?
Best regards
Uwe
arch/arm/mach-davinci/time.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
index 7a55b5c..627bf8b 100644
--- a/arch/arm/mach-davinci/time.c
+++ b/arch/arm/mach-davinci/time.c
@@ -331,7 +331,6 @@ static void davinci_set_mode(enum clock_event_mode mode,
static struct clock_event_device clockevent_davinci = {
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
- .shift = 32,
.set_next_event = davinci_set_next_event,
.set_mode = davinci_set_mode,
};
@@ -397,14 +396,11 @@ void __init davinci_timer_init(void)
/* setup clockevent */
clockevent_davinci.name = id_to_name[timers[TID_CLOCKEVENT].id];
- clockevent_davinci.mult = div_sc(davinci_clock_tick_rate, NSEC_PER_SEC,
- clockevent_davinci.shift);
- clockevent_davinci.max_delta_ns =
- clockevent_delta2ns(0xfffffffe, &clockevent_davinci);
- clockevent_davinci.min_delta_ns = 50000; /* 50 usec */
clockevent_davinci.cpumask = cpumask_of(0);
- clockevents_register_device(&clockevent_davinci);
+ /* min tick corresponds to 50 usec assuming a 24 MHz clock */
+ clockevents_config_and_register(&clockevent_davinci,
+ davinci_clock_tick_rate, 1200, 0xfffffffe);
for (i=0; i< ARRAY_SIZE(timers); i++)
timer32_config(&timers[i]);
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] ARM: davinci: convert to clockevents_config_and_register
2013-09-24 22:02 [PATCH] ARM: davinci: convert to clockevents_config_and_register Uwe Kleine-König
@ 2013-10-08 14:57 ` Uwe Kleine-König
2013-10-08 15:07 ` Sekhar Nori
2013-10-12 16:22 ` Sekhar Nori
1 sibling, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2013-10-08 14:57 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Wed, Sep 25, 2013 at 12:02:39AM +0200, Uwe Kleine-K?nig wrote:
> clockevents_config_and_register is superior compared to setting
> shift/mult and {min,max}_delta_ns by hand.
>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> I wonder where the 50 us limit comes from. This is not a hardware limit,
> is it?
ping; I didn't get any feedback on this yet, also it's not in next.
Best regards
Uwe
> arch/arm/mach-davinci/time.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
> index 7a55b5c..627bf8b 100644
> --- a/arch/arm/mach-davinci/time.c
> +++ b/arch/arm/mach-davinci/time.c
> @@ -331,7 +331,6 @@ static void davinci_set_mode(enum clock_event_mode mode,
>
> static struct clock_event_device clockevent_davinci = {
> .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> - .shift = 32,
> .set_next_event = davinci_set_next_event,
> .set_mode = davinci_set_mode,
> };
> @@ -397,14 +396,11 @@ void __init davinci_timer_init(void)
>
> /* setup clockevent */
> clockevent_davinci.name = id_to_name[timers[TID_CLOCKEVENT].id];
> - clockevent_davinci.mult = div_sc(davinci_clock_tick_rate, NSEC_PER_SEC,
> - clockevent_davinci.shift);
> - clockevent_davinci.max_delta_ns =
> - clockevent_delta2ns(0xfffffffe, &clockevent_davinci);
> - clockevent_davinci.min_delta_ns = 50000; /* 50 usec */
>
> clockevent_davinci.cpumask = cpumask_of(0);
> - clockevents_register_device(&clockevent_davinci);
> + /* min tick corresponds to 50 usec assuming a 24 MHz clock */
> + clockevents_config_and_register(&clockevent_davinci,
> + davinci_clock_tick_rate, 1200, 0xfffffffe);
>
> for (i=0; i< ARRAY_SIZE(timers); i++)
> timer32_config(&timers[i]);
> --
> 1.8.4.rc3
>
>
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: davinci: convert to clockevents_config_and_register
2013-10-08 14:57 ` Uwe Kleine-König
@ 2013-10-08 15:07 ` Sekhar Nori
0 siblings, 0 replies; 9+ messages in thread
From: Sekhar Nori @ 2013-10-08 15:07 UTC (permalink / raw)
To: linux-arm-kernel
Uwe,
On Tuesday 08 October 2013 08:27 PM, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Wed, Sep 25, 2013 at 12:02:39AM +0200, Uwe Kleine-K?nig wrote:
>> clockevents_config_and_register is superior compared to setting
>> shift/mult and {min,max}_delta_ns by hand.
>>
>> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>
>> ---
>> Hello,
>>
>> I wonder where the 50 us limit comes from. This is not a hardware limit,
>> is it?
> ping; I didn't get any feedback on this yet, also it's not in next.
Apologies for the delay. I am yet to look at it because I am traveling
ATM. I will look at it during this week and provide feedback.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: davinci: convert to clockevents_config_and_register
2013-09-24 22:02 [PATCH] ARM: davinci: convert to clockevents_config_and_register Uwe Kleine-König
2013-10-08 14:57 ` Uwe Kleine-König
@ 2013-10-12 16:22 ` Sekhar Nori
2013-10-13 6:55 ` Prabhakar Lad
1 sibling, 1 reply; 9+ messages in thread
From: Sekhar Nori @ 2013-10-12 16:22 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 25 September 2013 03:32 AM, Uwe Kleine-K?nig wrote:
> clockevents_config_and_register is superior compared to setting
> shift/mult and {min,max}_delta_ns by hand.
>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> I wonder where the 50 us limit comes from. This is not a hardware limit,
> is it?
>
> Best regards
> Uwe
>
> arch/arm/mach-davinci/time.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
> index 7a55b5c..627bf8b 100644
> --- a/arch/arm/mach-davinci/time.c
> +++ b/arch/arm/mach-davinci/time.c
> @@ -331,7 +331,6 @@ static void davinci_set_mode(enum clock_event_mode mode,
>
> static struct clock_event_device clockevent_davinci = {
> .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> - .shift = 32,
> .set_next_event = davinci_set_next_event,
> .set_mode = davinci_set_mode,
> };
> @@ -397,14 +396,11 @@ void __init davinci_timer_init(void)
>
> /* setup clockevent */
> clockevent_davinci.name = id_to_name[timers[TID_CLOCKEVENT].id];
> - clockevent_davinci.mult = div_sc(davinci_clock_tick_rate, NSEC_PER_SEC,
> - clockevent_davinci.shift);
> - clockevent_davinci.max_delta_ns =
> - clockevent_delta2ns(0xfffffffe, &clockevent_davinci);
> - clockevent_davinci.min_delta_ns = 50000; /* 50 usec */
>
> clockevent_davinci.cpumask = cpumask_of(0);
> - clockevents_register_device(&clockevent_davinci);
> + /* min tick corresponds to 50 usec assuming a 24 MHz clock */
> + clockevents_config_and_register(&clockevent_davinci,
> + davinci_clock_tick_rate, 1200, 0xfffffffe);
Min ticks can be set to 1, IMO. I think the 50usec limit used earlier
should have actually been 50ns assuming a 24MHz clock.
Prabhakar,
Can you please test this patch on a DaVinci platform you have with 1200
replaced with 1. I currently do not have access to a board.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: davinci: convert to clockevents_config_and_register
2013-10-12 16:22 ` Sekhar Nori
@ 2013-10-13 6:55 ` Prabhakar Lad
2013-10-13 8:36 ` [PATCH v2] " Uwe Kleine-König
0 siblings, 1 reply; 9+ messages in thread
From: Prabhakar Lad @ 2013-10-13 6:55 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sekhar,
On 10/12/13, Sekhar Nori <nsekhar@ti.com> wrote:
> On Wednesday 25 September 2013 03:32 AM, Uwe Kleine-K?nig wrote:
>> clockevents_config_and_register is superior compared to setting
>> shift/mult and {min,max}_delta_ns by hand.
[snip]
>>
>> + davinci_clock_tick_rate, 1200, 0xfffffffe);
>
> Min ticks can be set to 1, IMO. I think the 50usec limit used earlier
> should have actually been 50ns assuming a 24MHz clock.
>
> Prabhakar,
>
> Can you please test this patch on a DaVinci platform you have with 1200
> replaced with 1. I currently do not have access to a board.
>
I have boot tested this patch on OMAP-L138 and tested the uptime
(with min ticks set to 1), should that be enough ?
Thanks,
--Prabhakar Lad
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] ARM: davinci: convert to clockevents_config_and_register
2013-10-13 6:55 ` Prabhakar Lad
@ 2013-10-13 8:36 ` Uwe Kleine-König
2013-10-16 3:19 ` Sekhar Nori
0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2013-10-13 8:36 UTC (permalink / raw)
To: linux-arm-kernel
clockevents_config_and_register is superior compared to setting
shift/mult and {min,max}_delta_ns by hand.
Tested-by: Prabhakar Lad <prabhakar.csengg@gmail.com>
Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
Prabhakar Lad wrote:
> I have boot tested this patch on OMAP-L138 and tested the uptime
> (with min ticks set to 1), should that be enough ?
I don't know for sure, but I guess so. If you agree, here is an updated
patch.
Best regards
Uwe
arch/arm/mach-davinci/time.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
index 7a55b5c..29a1a5d 100644
--- a/arch/arm/mach-davinci/time.c
+++ b/arch/arm/mach-davinci/time.c
@@ -331,7 +331,6 @@ static void davinci_set_mode(enum clock_event_mode mode,
static struct clock_event_device clockevent_davinci = {
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
- .shift = 32,
.set_next_event = davinci_set_next_event,
.set_mode = davinci_set_mode,
};
@@ -397,14 +396,10 @@ void __init davinci_timer_init(void)
/* setup clockevent */
clockevent_davinci.name = id_to_name[timers[TID_CLOCKEVENT].id];
- clockevent_davinci.mult = div_sc(davinci_clock_tick_rate, NSEC_PER_SEC,
- clockevent_davinci.shift);
- clockevent_davinci.max_delta_ns =
- clockevent_delta2ns(0xfffffffe, &clockevent_davinci);
- clockevent_davinci.min_delta_ns = 50000; /* 50 usec */
clockevent_davinci.cpumask = cpumask_of(0);
- clockevents_register_device(&clockevent_davinci);
+ clockevents_config_and_register(&clockevent_davinci,
+ davinci_clock_tick_rate, 1, 0xfffffffe);
for (i=0; i< ARRAY_SIZE(timers); i++)
timer32_config(&timers[i]);
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2] ARM: davinci: convert to clockevents_config_and_register
2013-10-13 8:36 ` [PATCH v2] " Uwe Kleine-König
@ 2013-10-16 3:19 ` Sekhar Nori
2013-10-16 7:50 ` Uwe Kleine-König
0 siblings, 1 reply; 9+ messages in thread
From: Sekhar Nori @ 2013-10-16 3:19 UTC (permalink / raw)
To: linux-arm-kernel
On Sunday 13 October 2013 02:06 PM, Uwe Kleine-K?nig wrote:
> clockevents_config_and_register is superior compared to setting
> shift/mult and {min,max}_delta_ns by hand.
>
> Tested-by: Prabhakar Lad <prabhakar.csengg@gmail.com>
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
> Prabhakar Lad wrote:
>> I have boot tested this patch on OMAP-L138 and tested the uptime
>> (with min ticks set to 1), should that be enough ?
> I don't know for sure, but I guess so. If you agree, here is an updated
> patch.
>
> Best regards
> Uwe
>
>
> arch/arm/mach-davinci/time.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
> index 7a55b5c..29a1a5d 100644
> --- a/arch/arm/mach-davinci/time.c
> +++ b/arch/arm/mach-davinci/time.c
> @@ -331,7 +331,6 @@ static void davinci_set_mode(enum clock_event_mode mode,
>
> static struct clock_event_device clockevent_davinci = {
> .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> - .shift = 32,
> .set_next_event = davinci_set_next_event,
> .set_mode = davinci_set_mode,
> };
> @@ -397,14 +396,10 @@ void __init davinci_timer_init(void)
>
> /* setup clockevent */
> clockevent_davinci.name = id_to_name[timers[TID_CLOCKEVENT].id];
> - clockevent_davinci.mult = div_sc(davinci_clock_tick_rate, NSEC_PER_SEC,
> - clockevent_davinci.shift);
> - clockevent_davinci.max_delta_ns =
> - clockevent_delta2ns(0xfffffffe, &clockevent_davinci);
> - clockevent_davinci.min_delta_ns = 50000; /* 50 usec */
>
> clockevent_davinci.cpumask = cpumask_of(0);
> - clockevents_register_device(&clockevent_davinci);
> + clockevents_config_and_register(&clockevent_davinci,
> + davinci_clock_tick_rate, 1, 0xfffffffe);
checkpatch --strict complains about alignment of line break. Fixed locally.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] ARM: davinci: convert to clockevents_config_and_register
2013-10-16 3:19 ` Sekhar Nori
@ 2013-10-16 7:50 ` Uwe Kleine-König
2013-10-16 16:38 ` Sekhar Nori
0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2013-10-16 7:50 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Wed, Oct 16, 2013 at 08:49:17AM +0530, Sekhar Nori wrote:
> On Sunday 13 October 2013 02:06 PM, Uwe Kleine-K?nig wrote:
> > clockevents_config_and_register is superior compared to setting
> > shift/mult and {min,max}_delta_ns by hand.
> >
> > Tested-by: Prabhakar Lad <prabhakar.csengg@gmail.com>
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > ---
> > Prabhakar Lad wrote:
> >> I have boot tested this patch on OMAP-L138 and tested the uptime
> >> (with min ticks set to 1), should that be enough ?
> > I don't know for sure, but I guess so. If you agree, here is an updated
> > patch.
> >
> > Best regards
> > Uwe
> >
> >
> > arch/arm/mach-davinci/time.c | 9 ++-------
> > 1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
> > index 7a55b5c..29a1a5d 100644
> > --- a/arch/arm/mach-davinci/time.c
> > +++ b/arch/arm/mach-davinci/time.c
> > @@ -331,7 +331,6 @@ static void davinci_set_mode(enum clock_event_mode mode,
> >
> > static struct clock_event_device clockevent_davinci = {
> > .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> > - .shift = 32,
> > .set_next_event = davinci_set_next_event,
> > .set_mode = davinci_set_mode,
> > };
> > @@ -397,14 +396,10 @@ void __init davinci_timer_init(void)
> >
> > /* setup clockevent */
> > clockevent_davinci.name = id_to_name[timers[TID_CLOCKEVENT].id];
> > - clockevent_davinci.mult = div_sc(davinci_clock_tick_rate, NSEC_PER_SEC,
> > - clockevent_davinci.shift);
> > - clockevent_davinci.max_delta_ns =
> > - clockevent_delta2ns(0xfffffffe, &clockevent_davinci);
> > - clockevent_davinci.min_delta_ns = 50000; /* 50 usec */
> >
> > clockevent_davinci.cpumask = cpumask_of(0);
> > - clockevents_register_device(&clockevent_davinci);
> > + clockevents_config_and_register(&clockevent_davinci,
> > + davinci_clock_tick_rate, 1, 0xfffffffe);
>
> checkpatch --strict complains about alignment of line break. Fixed locally.
I used the same style as in other places in that file.
Another thought that came to me is that it's probably worth mentioning
that the min_delta_ns is changed from 50 usec to 1 hw tick. Something
like:
Note that the minimum delay is changed from 50 us to 1 hw tick
(which corresponds to 42 ns assuming a 24 MHz clock). It's
expected that the old value was miscalculated.
That would be interesting in case that the patch results in regressions.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] ARM: davinci: convert to clockevents_config_and_register
2013-10-16 7:50 ` Uwe Kleine-König
@ 2013-10-16 16:38 ` Sekhar Nori
0 siblings, 0 replies; 9+ messages in thread
From: Sekhar Nori @ 2013-10-16 16:38 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 16 October 2013 01:20 PM, Uwe Kleine-K?nig wrote:
> Hello,
>
> On Wed, Oct 16, 2013 at 08:49:17AM +0530, Sekhar Nori wrote:
>> On Sunday 13 October 2013 02:06 PM, Uwe Kleine-K?nig wrote:
>>> clockevents_config_and_register is superior compared to setting
>>> shift/mult and {min,max}_delta_ns by hand.
>>>
>>> Tested-by: Prabhakar Lad <prabhakar.csengg@gmail.com>
>>> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>>> ---
>>> Prabhakar Lad wrote:
>>>> I have boot tested this patch on OMAP-L138 and tested the uptime
>>>> (with min ticks set to 1), should that be enough ?
>>> I don't know for sure, but I guess so. If you agree, here is an updated
>>> patch.
>>>
>>> Best regards
>>> Uwe
>>>
>>>
>>> arch/arm/mach-davinci/time.c | 9 ++-------
>>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-davinci/time.c b/arch/arm/mach-davinci/time.c
>>> index 7a55b5c..29a1a5d 100644
>>> --- a/arch/arm/mach-davinci/time.c
>>> +++ b/arch/arm/mach-davinci/time.c
>>> @@ -331,7 +331,6 @@ static void davinci_set_mode(enum clock_event_mode mode,
>>>
>>> static struct clock_event_device clockevent_davinci = {
>>> .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
>>> - .shift = 32,
>>> .set_next_event = davinci_set_next_event,
>>> .set_mode = davinci_set_mode,
>>> };
>>> @@ -397,14 +396,10 @@ void __init davinci_timer_init(void)
>>>
>>> /* setup clockevent */
>>> clockevent_davinci.name = id_to_name[timers[TID_CLOCKEVENT].id];
>>> - clockevent_davinci.mult = div_sc(davinci_clock_tick_rate, NSEC_PER_SEC,
>>> - clockevent_davinci.shift);
>>> - clockevent_davinci.max_delta_ns =
>>> - clockevent_delta2ns(0xfffffffe, &clockevent_davinci);
>>> - clockevent_davinci.min_delta_ns = 50000; /* 50 usec */
>>>
>>> clockevent_davinci.cpumask = cpumask_of(0);
>>> - clockevents_register_device(&clockevent_davinci);
>>> + clockevents_config_and_register(&clockevent_davinci,
>>> + davinci_clock_tick_rate, 1, 0xfffffffe);
>>
>> checkpatch --strict complains about alignment of line break. Fixed locally.
> I used the same style as in other places in that file.
Okay, but I prefer to fix for new code.
>
> Another thought that came to me is that it's probably worth mentioning
> that the min_delta_ns is changed from 50 usec to 1 hw tick. Something
> like:
>
> Note that the minimum delay is changed from 50 us to 1 hw tick
> (which corresponds to 42 ns assuming a 24 MHz clock). It's
> expected that the old value was miscalculated.
>
> That would be interesting in case that the patch results in regressions.
Too late as I already sent my pull request. Adding this to patch
description would have been nice, I agree.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-10-16 16:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-24 22:02 [PATCH] ARM: davinci: convert to clockevents_config_and_register Uwe Kleine-König
2013-10-08 14:57 ` Uwe Kleine-König
2013-10-08 15:07 ` Sekhar Nori
2013-10-12 16:22 ` Sekhar Nori
2013-10-13 6:55 ` Prabhakar Lad
2013-10-13 8:36 ` [PATCH v2] " Uwe Kleine-König
2013-10-16 3:19 ` Sekhar Nori
2013-10-16 7:50 ` Uwe Kleine-König
2013-10-16 16:38 ` Sekhar Nori
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.