[PATCH 2/2] mach-sa1100: modernize and cleanup timer code

[PATCH 2/2] mach-sa1100: modernize and cleanup timer code

[Linus Walleij]

This gets rid of the hairy mult/shift calculation for SA1100
timers and relies on clockevent core to figure this out, and removes
the hard-coded rate constant in favor of the CLOCK_TICK_RATE
already used by the clocksource.

Cc: Kristoffer Ericson <kristoffer.ericson@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-sa1100/time.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-sa1100/time.c b/arch/arm/mach-sa1100/time.c
index 69e3353..5cb8c7b 100644
--- a/arch/arm/mach-sa1100/time.c
+++ b/arch/arm/mach-sa1100/time.c
@@ -88,20 +88,14 @@ static void __init sa1100_timer_init(void)
 	OIER = 0;
 	OSSR = OSSR_M0 | OSSR_M1 | OSSR_M2 | OSSR_M3;
 
-	setup_sched_clock(sa1100_read_sched_clock, 32, 3686400);
-
-	clockevents_calc_mult_shift(&ckevt_sa1100_osmr0, 3686400, 4);
-	ckevt_sa1100_osmr0.max_delta_ns =
-		clockevent_delta2ns(0x7fffffff, &ckevt_sa1100_osmr0);
-	ckevt_sa1100_osmr0.min_delta_ns =
-		clockevent_delta2ns(MIN_OSCR_DELTA * 2, &ckevt_sa1100_osmr0) + 1;
+	setup_sched_clock(sa1100_read_sched_clock, 32, CLOCK_TICK_RATE);
 	ckevt_sa1100_osmr0.cpumask = cpumask_of(0);
-
 	setup_irq(IRQ_OST0, &sa1100_timer_irq);
 
 	clocksource_mmio_init(&OSCR, "oscr", CLOCK_TICK_RATE, 200, 32,
 		clocksource_mmio_readl_up);
-	clockevents_register_device(&ckevt_sa1100_osmr0);
+	clockevents_config_and_register(&ckevt_sa1100_osmr0, CLOCK_TICK_RATE,
+					MIN_OSCR_DELTA * 2, 0x7fffffff);
 }
 
 #ifdef CONFIG_PM
-- 
1.7.6.4

[PATCH 2/2] mach-sa1100: modernize and cleanup timer code

[Kristoffer Ericson]

Urs?kta f?rseningen, fick n?got minne kring att cpu klockningen inte fungerade korrekt
om man inte satte direkt systemklock v?rde. Var n?got som pratade med russell om 
f?r n?got ?r sedan.
Ska bara kika lite snabbt.

On Wed, Jan 04, 2012 at 11:21:53AM +0100, Linus Walleij wrote:
> This gets rid of the hairy mult/shift calculation for SA1100
> timers and relies on clockevent core to figure this out, and removes
> the hard-coded rate constant in favor of the CLOCK_TICK_RATE
> already used by the clocksource.
> 
> Cc: Kristoffer Ericson <kristoffer.ericson@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  arch/arm/mach-sa1100/time.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-sa1100/time.c b/arch/arm/mach-sa1100/time.c
> index 69e3353..5cb8c7b 100644
> --- a/arch/arm/mach-sa1100/time.c
> +++ b/arch/arm/mach-sa1100/time.c
> @@ -88,20 +88,14 @@ static void __init sa1100_timer_init(void)
>  	OIER = 0;
>  	OSSR = OSSR_M0 | OSSR_M1 | OSSR_M2 | OSSR_M3;
>  
> -	setup_sched_clock(sa1100_read_sched_clock, 32, 3686400);
> -
> -	clockevents_calc_mult_shift(&ckevt_sa1100_osmr0, 3686400, 4);
> -	ckevt_sa1100_osmr0.max_delta_ns =
> -		clockevent_delta2ns(0x7fffffff, &ckevt_sa1100_osmr0);
> -	ckevt_sa1100_osmr0.min_delta_ns =
> -		clockevent_delta2ns(MIN_OSCR_DELTA * 2, &ckevt_sa1100_osmr0) + 1;
> +	setup_sched_clock(sa1100_read_sched_clock, 32, CLOCK_TICK_RATE);
>  	ckevt_sa1100_osmr0.cpumask = cpumask_of(0);
> -
>  	setup_irq(IRQ_OST0, &sa1100_timer_irq);
>  
>  	clocksource_mmio_init(&OSCR, "oscr", CLOCK_TICK_RATE, 200, 32,
>  		clocksource_mmio_readl_up);
> -	clockevents_register_device(&ckevt_sa1100_osmr0);
> +	clockevents_config_and_register(&ckevt_sa1100_osmr0, CLOCK_TICK_RATE,
> +					MIN_OSCR_DELTA * 2, 0x7fffffff);
>  }
>  
>  #ifdef CONFIG_PM
> -- 
> 1.7.6.4

[PATCH 2/2] mach-sa1100: modernize and cleanup timer code

[Linus Walleij]

On Thu, Jan 12, 2012 at 5:10 PM, Kristoffer Ericson
<kristoffer.ericson@gmail.com> wrote:

> Urs?kta f?rseningen, fick n?got minne kring att cpu klockningen inte fungerade korrekt
> om man inte satte direkt systemklock v?rde. Var n?got som pratade med russell om
> f?r n?got ?r sedan.

Menar du detta:
MIN_OSCR_DELTA * 2

S? ?r det ju med i den nya koden oxo, det ?r ingen semantisk skillnad
s? vitt jag kan se...

Det enda som egentligen ?ndras ?r att minimum lapse mellan tv? events
(mult+shift) ber?knas av clockevent core.

Linus

[PATCH 2/2] mach-sa1100: modernize and cleanup timer code

[Russell King - ARM Linux]

On Sat, Jan 14, 2012 at 08:33:54AM +0100, Linus Walleij wrote:
> On Thu, Jan 12, 2012 at 5:10 PM, Kristoffer Ericson
> <kristoffer.ericson@gmail.com> wrote:
> 
> > Urs?kta f?rseningen, fick n?got minne kring att cpu klockningen inte fungerade korrekt
> > om man inte satte direkt systemklock v?rde. Var n?got som pratade med russell om
> > f?r n?got ?r sedan.
> 
> Menar du detta:
> MIN_OSCR_DELTA * 2
> 
> S? ?r det ju med i den nya koden oxo, det ?r ingen semantisk skillnad
> s? vitt jag kan se...
> 
> Det enda som egentligen ?ndras ?r att minimum lapse mellan tv? events
> (mult+shift) ber?knas av clockevent core.

If you speak english on these lists, other people might be able to
understand what you're saying.

[PATCH 2/2] mach-sa1100: modernize and cleanup timer code

[Linus Walleij]

2012/1/14 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Sat, Jan 14, 2012 at 08:33:54AM +0100, Linus Walleij wrote:
>> On Thu, Jan 12, 2012 at 5:10 PM, Kristoffer Ericson
>> <kristoffer.ericson@gmail.com> wrote:
>>
>> > Urs?kta f?rseningen, fick n?got minne kring att cpu klockningen inte fungerade korrekt
>> > om man inte satte direkt systemklock v?rde. Var n?got som pratade med russell om
>> > f?r n?got ?r sedan.
>>
>> Menar du detta:
>> MIN_OSCR_DELTA * 2
>>
>> S? ?r det ju med i den nya koden oxo, det ?r ingen semantisk skillnad
>> s? vitt jag kan se...
>>
>> Det enda som egentligen ?ndras ?r att minimum lapse mellan tv? events
>> (mult+shift) ber?knas av clockevent core.
>
> If you speak english on these lists, other people might be able to
> understand what you're saying.

Haha sorry Russell I didn't notice that the response was in LAKML :-)

So Kristoffer remarked that the CPU clocking was sensitive on the
SA1100 so he wanted to test it first.

And I said that apart from the minimum scheduling interval now
handled by the clockevent core it should be semantically equivalent.

But it never hurts to test it so I'll wait for Kristoffers ACK.

Yours,
Linus Walleij

[PATCH 2/2] mach-sa1100: modernize and cleanup timer code

[Russell King - ARM Linux]

On Sat, Jan 14, 2012 at 12:41:21PM +0100, Linus Walleij wrote:
> 2012/1/14 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > On Sat, Jan 14, 2012 at 08:33:54AM +0100, Linus Walleij wrote:
> >> On Thu, Jan 12, 2012 at 5:10 PM, Kristoffer Ericson
> >> <kristoffer.ericson@gmail.com> wrote:
> >>
> >> > Urs?kta f?rseningen, fick n?got minne kring att cpu klockningen inte fungerade korrekt
> >> > om man inte satte direkt systemklock v?rde. Var n?got som pratade med russell om
> >> > f?r n?got ?r sedan.
> >>
> >> Menar du detta:
> >> MIN_OSCR_DELTA * 2
> >>
> >> S? ?r det ju med i den nya koden oxo, det ?r ingen semantisk skillnad
> >> s? vitt jag kan se...
> >>
> >> Det enda som egentligen ?ndras ?r att minimum lapse mellan tv? events
> >> (mult+shift) ber?knas av clockevent core.
> >
> > If you speak english on these lists, other people might be able to
> > understand what you're saying.
> 
> Haha sorry Russell I didn't notice that the response was in LAKML :-)
> 
> So Kristoffer remarked that the CPU clocking was sensitive on the
> SA1100 so he wanted to test it first.
> 
> And I said that apart from the minimum scheduling interval now
> handled by the clockevent core it should be semantically equivalent.
> 
> But it never hurts to test it so I'll wait for Kristoffers ACK.

The only point I have on the patch is that it shouldn't use CLOCK_TICK_RATE
lets try to get away from this horrible definition in mach/timex.h.

[PATCH 2/2] mach-sa1100: modernize and cleanup timer code

[Kristoffer Ericson]

On Sat, Jan 14, 2012 at 12:09:24PM +0000, Russell King - ARM Linux wrote:
> On Sat, Jan 14, 2012 at 12:41:21PM +0100, Linus Walleij wrote:
> > 2012/1/14 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> > > On Sat, Jan 14, 2012 at 08:33:54AM +0100, Linus Walleij wrote:
> > >> On Thu, Jan 12, 2012 at 5:10 PM, Kristoffer Ericson
> > >> <kristoffer.ericson@gmail.com> wrote:
> > >>
> > >> > Urs?kta f?rseningen, fick n?got minne kring att cpu klockningen inte fungerade korrekt
> > >> > om man inte satte direkt systemklock v?rde. Var n?got som pratade med russell om
> > >> > f?r n?got ?r sedan.
> > >>
> > >> Menar du detta:
> > >> MIN_OSCR_DELTA * 2
> > >>
> > >> S? ?r det ju med i den nya koden oxo, det ?r ingen semantisk skillnad
> > >> s? vitt jag kan se...
> > >>
> > >> Det enda som egentligen ?ndras ?r att minimum lapse mellan tv? events
> > >> (mult+shift) ber?knas av clockevent core.
> > >
> > > If you speak english on these lists, other people might be able to
> > > understand what you're saying.
> > 

Sorry, same mistake here :)


> > Haha sorry Russell I didn't notice that the response was in LAKML :-)
> > 
> > So Kristoffer remarked that the CPU clocking was sensitive on the
> > SA1100 so he wanted to test it first.
> > 
> > And I said that apart from the minimum scheduling interval now
> > handled by the clockevent core it should be semantically equivalent.
> > 
> > But it never hurts to test it so I'll wait for Kristoffers ACK.
> 

Ive looked at it and it checks out as you said. So acked-by: Kristoffer Ericson <kristoffer.ericson@gmail.com>
After Russels suggestion of course.


> The only point I have on the patch is that it shouldn't use CLOCK_TICK_RATE
> lets try to get away from this horrible definition in mach/timex.h.

[PATCH 2/2] mach-sa1100: modernize and cleanup timer code

[Linus Walleij]

On Sat, Jan 14, 2012 at 1:09 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

> The only point I have on the patch is that it shouldn't use CLOCK_TICK_RATE
> lets try to get away from this horrible definition in mach/timex.h.

OK I'll replace that with (clockrate / HZ) as is custom.

Thanks!
Linus Walleij