[PATCH/RFC] powerpc: Fix decrementer setup on 1GHz boards

[PATCH/RFC] powerpc: Fix decrementer setup on 1GHz boards

[Stefan Roese]

We noticed that recent kernels didn't boot on our 1GHz Canyonlands 460EX
boards anymore. As it seems, patch 8d165db1 [powerpc: Improve
decrementer accuracy] introduced this problem. The routine div_sc()
overflows with shift = 32 resulting in this incorrect setup:

time_init: decrementer frequency = 1000.000012 MHz
time_init: processor frequency   = 1000.000012 MHz
clocksource: timebase mult[400000] shift[22] registered
clockevent: decrementer mult[33] shift[32] cpu[0]

This patch now introduces a local div_dc64() version of this function
so that this overflow doesn't happen anymore.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Anton Blanchard <anton@samba.org>
Cc: Detlev Zundel <dzu@denx.de>
---
Ben, Anton, how should we handle this? Is this patch acceptable? Or how
should this be solved?

 arch/powerpc/kernel/time.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 92dc844..3ad729f 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -882,12 +882,21 @@ static void decrementer_set_mode(enum clock_event_mode mode,
 		decrementer_set_next_event(DECREMENTER_MAX, dev);
 }
 
+static inline uint64_t div_sc64(unsigned long ticks, unsigned long nsec,
+				int shift)
+{
+	uint64_t tmp = ((uint64_t)ticks) << shift;
+
+	do_div(tmp, nsec);
+	return tmp;
+}
+
 static void __init setup_clockevent_multiplier(unsigned long hz)
 {
 	u64 mult, shift = 32;
 
 	while (1) {
-		mult = div_sc(hz, NSEC_PER_SEC, shift);
+		mult = div_sc64(hz, NSEC_PER_SEC, shift);
 		if (mult && (mult >> 32UL) == 0UL)
 			break;
 
-- 
1.6.4.4

Re: [PATCH/RFC] powerpc: Fix decrementer setup on 1GHz boards

[Benjamin Herrenschmidt]

On Thu, 2009-10-08 at 16:49 +0200, Stefan Roese wrote:
> We noticed that recent kernels didn't boot on our 1GHz Canyonlands 460EX
> boards anymore. As it seems, patch 8d165db1 [powerpc: Improve
> decrementer accuracy] introduced this problem. The routine div_sc()
> overflows with shift = 32 resulting in this incorrect setup:
> 
> time_init: decrementer frequency = 1000.000012 MHz
> time_init: processor frequency   = 1000.000012 MHz
> clocksource: timebase mult[400000] shift[22] registered
> clockevent: decrementer mult[33] shift[32] cpu[0]
> 
> This patch now introduces a local div_dc64() version of this function
> so that this overflow doesn't happen anymore.

Ugh ? We never expected that the decrementer would be ticking that fast.
This is too fast actually. Somebody at AMCC must be told to stick a
divider in front of the timebase when reaching such frequencies.

Ben.

> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Anton Blanchard <anton@samba.org>
> Cc: Detlev Zundel <dzu@denx.de>
> ---
> Ben, Anton, how should we handle this? Is this patch acceptable? Or how
> should this be solved?
> 
>  arch/powerpc/kernel/time.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index 92dc844..3ad729f 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -882,12 +882,21 @@ static void decrementer_set_mode(enum clock_event_mode mode,
>  		decrementer_set_next_event(DECREMENTER_MAX, dev);
>  }
>  
> +static inline uint64_t div_sc64(unsigned long ticks, unsigned long nsec,
> +				int shift)
> +{
> +	uint64_t tmp = ((uint64_t)ticks) << shift;
> +
> +	do_div(tmp, nsec);
> +	return tmp;
> +}
> +
>  static void __init setup_clockevent_multiplier(unsigned long hz)
>  {
>  	u64 mult, shift = 32;
>  
>  	while (1) {
> -		mult = div_sc(hz, NSEC_PER_SEC, shift);
> +		mult = div_sc64(hz, NSEC_PER_SEC, shift);
>  		if (mult && (mult >> 32UL) == 0UL)
>  			break;
>  

Re: [PATCH/RFC] powerpc: Fix decrementer setup on 1GHz boards

[Stefan Roese]

On Thursday 08 October 2009 22:12:56 Benjamin Herrenschmidt wrote:
> On Thu, 2009-10-08 at 16:49 +0200, Stefan Roese wrote:
> > We noticed that recent kernels didn't boot on our 1GHz Canyonlands 
460EX
> > boards anymore. As it seems, patch 8d165db1 [powerpc: Improve
> > decrementer accuracy] introduced this problem. The routine div_sc()
> > overflows with shift = 32 resulting in this incorrect setup:
> >
> > time_init: decrementer frequency = 1000.000012 MHz
> > time_init: processor frequency   = 1000.000012 MHz
> > clocksource: timebase mult[400000] shift[22] registered
> > clockevent: decrementer mult[33] shift[32] cpu[0]
> >
> > This patch now introduces a local div_dc64() version of this function
> > so that this overflow doesn't happen anymore.
> 
> Ugh ? We never expected that the decrementer would be ticking that fast.
> This is too fast actually. Somebody at AMCC must be told to stick a
> divider in front of the timebase when reaching such frequencies.

As Josh already pointed out on IRC, the source of timebase can be either 
the CPU clock or an external provided clock. We always used the CPU clock 
as timebase source till now. 

I looked at switching to external clock on Canyonlands, but this doesn't 
look promising. The external timer clock here is the output of the RTC 
M41T62, which is a max of 32768 Hz. Enabling this clock also has the 
disadvantage to drain the RTC battery quickly. That's why this clock output 
is currently disabled per default.

So how could we solve this issue now? Apply my current patch? Any other 
suggestions?

Thanks.

Cheers,
Stefan

Re: [PATCH/RFC] powerpc: Fix decrementer setup on 1GHz boards

[Benjamin Herrenschmidt]

On Fri, 2009-10-09 at 07:28 +0200, Stefan Roese wrote:
> 
> So how could we solve this issue now? Apply my current patch? Any
> other suggestions? 

Well, we probably need your current patch, but I'm not 100% it's enough.
We need to check for example that the vDSO code for 32-bit isn't going
to go out of bounds neither.

Cheers,
Ben.