[PATCH 0/4] PowerPC time fixes

[PATCH 0/4] PowerPC time fixes

[Anton Blanchard]

Switch to using the common clockevents_calc_mult_shift and
clocksource_register_hz functions, and do some cleanup along
the way.

[PATCH 1/4] powerpc/time: Use clockevents_calc_mult_shift

[Anton Blanchard]

We can use clockevents_calc_mult_shift instead of doing all
the work ourselves.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux-build/arch/powerpc/kernel/time.c
===================================================================
--- linux-build.orig/arch/powerpc/kernel/time.c	2011-11-03 10:19:55.225606490 +1100
+++ linux-build/arch/powerpc/kernel/time.c	2011-11-03 10:19:59.493679032 +1100
@@ -112,8 +112,6 @@ static void decrementer_set_mode(enum cl
 static struct clock_event_device decrementer_clockevent = {
        .name           = "decrementer",
        .rating         = 200,
-       .shift          = 0,	/* To be filled in */
-       .mult           = 0,	/* To be filled in */
        .irq            = 0,
        .set_next_event = decrementer_set_next_event,
        .set_mode       = decrementer_set_mode,
@@ -904,31 +902,6 @@ static void decrementer_set_mode(enum cl
 		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_sc64(hz, NSEC_PER_SEC, shift);
-		if (mult && (mult >> 32UL) == 0UL)
-			break;
-
-		shift--;
-	}
-
-	decrementer_clockevent.shift = shift;
-	decrementer_clockevent.mult = mult;
-}
-
 static void register_decrementer_clockevent(int cpu)
 {
 	struct clock_event_device *dec = &per_cpu(decrementers, cpu).event;
@@ -946,7 +919,8 @@ static void __init init_decrementer_cloc
 {
 	int cpu = smp_processor_id();
 
-	setup_clockevent_multiplier(ppc_tb_freq);
+	clockevents_calc_mult_shift(&decrementer_clockevent, ppc_tb_freq, 4);
+
 	decrementer_clockevent.max_delta_ns =
 		clockevent_delta2ns(DECREMENTER_MAX, &decrementer_clockevent);
 	decrementer_clockevent.min_delta_ns =

[PATCH 2/4] powerpc/time: Use clocksource_register_hz

[Anton Blanchard]

Use clocksource_register_hz which calculates the shift/mult
factors for us.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux-build/arch/powerpc/kernel/time.c
===================================================================
--- linux-build.orig/arch/powerpc/kernel/time.c	2011-11-03 10:19:59.493679032 +1100
+++ linux-build/arch/powerpc/kernel/time.c	2011-11-03 10:20:00.965704053 +1100
@@ -86,8 +86,6 @@ static struct clocksource clocksource_rt
 	.rating       = 400,
 	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
 	.mask         = CLOCKSOURCE_MASK(64),
-	.shift        = 22,
-	.mult         = 0,	/* To be filled in */
 	.read         = rtc_read,
 };
 
@@ -97,8 +95,6 @@ static struct clocksource clocksource_ti
 	.rating       = 400,
 	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
 	.mask         = CLOCKSOURCE_MASK(64),
-	.shift        = 22,
-	.mult         = 0,	/* To be filled in */
 	.read         = timebase_read,
 };
 
@@ -875,9 +871,7 @@ static void __init clocksource_init(void
 	else
 		clock = &clocksource_timebase;
 
-	clock->mult = clocksource_hz2mult(tb_ticks_per_sec, clock->shift);
-
-	if (clocksource_register(clock)) {
+	if (clocksource_register_hz(clock, tb_ticks_per_sec)) {
 		printk(KERN_ERR "clocksource: %s is already registered\n",
 		       clock->name);
 		return;

[PATCH 3/4] powerpc/time: Remove unnecessary sanity check of decrementer expiration

[Anton Blanchard]

The clockevents code uses max_delta_ns to avoid calling a clockevent
with too large a value.

Remove the redundant version of this in the timer_interrupt code.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux-build/arch/powerpc/kernel/time.c
===================================================================
--- linux-build.orig/arch/powerpc/kernel/time.c	2011-11-03 10:20:00.965704053 +1100
+++ linux-build/arch/powerpc/kernel/time.c	2011-11-03 10:20:02.117723634 +1100
@@ -114,12 +114,7 @@ static struct clock_event_device decreme
        .features       = CLOCK_EVT_FEAT_ONESHOT,
 };
 
-struct decrementer_clock {
-	struct clock_event_device event;
-	u64 next_tb;
-};
-
-static DEFINE_PER_CPU(struct decrementer_clock, decrementers);
+static DEFINE_PER_CPU(struct clock_event_device, decrementers);
 
 #ifdef CONFIG_PPC_ISERIES
 static unsigned long __initdata iSeries_recal_titan;
@@ -570,9 +565,7 @@ void arch_irq_work_raise(void)
 void timer_interrupt(struct pt_regs * regs)
 {
 	struct pt_regs *old_regs;
-	struct decrementer_clock *decrementer =  &__get_cpu_var(decrementers);
-	struct clock_event_device *evt = &decrementer->event;
-	u64 now;
+	struct clock_event_device *evt = &__get_cpu_var(decrementers);
 
 	/* Ensure a positive value is written to the decrementer, or else
 	 * some CPUs will continue to take decrementer exceptions.
@@ -607,16 +600,7 @@ void timer_interrupt(struct pt_regs * re
 		get_lppaca()->int_dword.fields.decr_int = 0;
 #endif
 
-	now = get_tb_or_rtc();
-	if (now >= decrementer->next_tb) {
-		decrementer->next_tb = ~(u64)0;
-		if (evt->event_handler)
-			evt->event_handler(evt);
-	} else {
-		now = decrementer->next_tb - now;
-		if (now <= DECREMENTER_MAX)
-			set_dec((int)now);
-	}
+	evt->event_handler(evt);
 
 #ifdef CONFIG_PPC_ISERIES
 	if (firmware_has_feature(FW_FEATURE_ISERIES) && hvlpevent_is_pending())
@@ -884,7 +868,6 @@ static void __init clocksource_init(void
 static int decrementer_set_next_event(unsigned long evt,
 				      struct clock_event_device *dev)
 {
-	__get_cpu_var(decrementers).next_tb = get_tb_or_rtc() + evt;
 	set_dec(evt);
 	return 0;
 }
@@ -898,7 +881,7 @@ static void decrementer_set_mode(enum cl
 
 static void register_decrementer_clockevent(int cpu)
 {
-	struct clock_event_device *dec = &per_cpu(decrementers, cpu).event;
+	struct clock_event_device *dec = &per_cpu(decrementers, cpu);
 
 	*dec = decrementer_clockevent;
 	dec->cpumask = cpumask_of(cpu);

[PATCH 4/4] powerpc/time: Fix some style issues

[Anton Blanchard]

Fix some formatting issues and use the DECREMENTER_MAX
define instead of 0x7fffffff.

Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: linux-build/arch/powerpc/kernel/time.c
===================================================================
--- linux-build.orig/arch/powerpc/kernel/time.c	2011-11-03 10:20:02.117723634 +1100
+++ linux-build/arch/powerpc/kernel/time.c	2011-11-03 10:20:03.101740361 +1100
@@ -82,20 +82,20 @@
 
 static cycle_t rtc_read(struct clocksource *);
 static struct clocksource clocksource_rtc = {
-	.name         = "rtc",
-	.rating       = 400,
-	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
-	.mask         = CLOCKSOURCE_MASK(64),
-	.read         = rtc_read,
+	.name		= "rtc",
+	.rating		= 400,
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.mask		= CLOCKSOURCE_MASK(64),
+	.read		= rtc_read,
 };
 
 static cycle_t timebase_read(struct clocksource *);
 static struct clocksource clocksource_timebase = {
-	.name         = "timebase",
-	.rating       = 400,
-	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
-	.mask         = CLOCKSOURCE_MASK(64),
-	.read         = timebase_read,
+	.name		= "timebase",
+	.rating		= 400,
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.mask		= CLOCKSOURCE_MASK(64),
+	.read		= timebase_read,
 };
 
 #define DECREMENTER_MAX	0x7fffffff
@@ -106,12 +106,12 @@ static void decrementer_set_mode(enum cl
 				 struct clock_event_device *dev);
 
 static struct clock_event_device decrementer_clockevent = {
-       .name           = "decrementer",
-       .rating         = 200,
-       .irq            = 0,
-       .set_next_event = decrementer_set_next_event,
-       .set_mode       = decrementer_set_mode,
-       .features       = CLOCK_EVT_FEAT_ONESHOT,
+	.name		= "decrementer",
+	.rating		= 200,
+	.irq		= 0,
+	.set_next_event	= decrementer_set_next_event,
+	.set_mode	= decrementer_set_mode,
+	.features	= CLOCK_EVT_FEAT_ONESHOT,
 };
 
 static DEFINE_PER_CPU(struct clock_event_device, decrementers);
@@ -430,7 +430,7 @@ EXPORT_SYMBOL(profile_pc);
 /* 
  * This function recalibrates the timebase based on the 49-bit time-of-day
  * value in the Titan chip.  The Titan is much more accurate than the value
- * returned by the service processor for the timebase frequency.  
+ * returned by the service processor for the timebase frequency.
  */
 
 static int __init iSeries_tb_recal(void)
@@ -628,9 +628,9 @@ static void generic_suspend_disable_irqs
 	 * with suspending.
 	 */
 
-	set_dec(0x7fffffff);
+	set_dec(DECREMENTER_MAX);
 	local_irq_disable();
-	set_dec(0x7fffffff);
+	set_dec(DECREMENTER_MAX);
 }
 
 static void generic_suspend_enable_irqs(void)
@@ -965,10 +965,10 @@ void __init time_init(void)
 	boot_tb = get_tb_or_rtc();
 
 	/* If platform provided a timezone (pmac), we correct the time */
-        if (timezone_offset) {
+	if (timezone_offset) {
 		sys_tz.tz_minuteswest = -timezone_offset / 60;
 		sys_tz.tz_dsttime = 0;
-        }
+	}
 
 	vdso_data->tb_update_count = 0;
 	vdso_data->tb_ticks_per_sec = tb_ticks_per_sec;

Re: [PATCH 2/4] powerpc/time: Use clocksource_register_hz

[John Stultz]

On Thu, 2011-11-03 at 11:59 +1100, Anton Blanchard wrote:
> plain text document attachment (clock3)
> Use clocksource_register_hz which calculates the shift/mult
> factors for us.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> 
> Index: linux-build/arch/powerpc/kernel/time.c
> ===================================================================
> --- linux-build.orig/arch/powerpc/kernel/time.c	2011-11-03 10:19:59.493679032 +1100
> +++ linux-build/arch/powerpc/kernel/time.c	2011-11-03 10:20:00.965704053 +1100
> @@ -86,8 +86,6 @@ static struct clocksource clocksource_rt
>  	.rating       = 400,
>  	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
>  	.mask         = CLOCKSOURCE_MASK(64),
> -	.shift        = 22,
> -	.mult         = 0,	/* To be filled in */
>  	.read         = rtc_read,
>  };
> 
> @@ -97,8 +95,6 @@ static struct clocksource clocksource_ti
>  	.rating       = 400,
>  	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
>  	.mask         = CLOCKSOURCE_MASK(64),
> -	.shift        = 22,
> -	.mult         = 0,	/* To be filled in */
>  	.read         = timebase_read,
>  };

So I've held off on ppc conversion to clocksource_register_hz due to the
fact that the ppc vdso gettimeofday at least used to make assumptions
that shift was 22.

Is that no longer the case?

thanks
-john

Re: [PATCH 2/4] powerpc/time: Use clocksource_register_hz

[Paul Mackerras]

On Thu, Nov 03, 2011 at 09:14:44AM -0400, John Stultz wrote:
> On Thu, 2011-11-03 at 11:59 +1100, Anton Blanchard wrote:
> > plain text document attachment (clock3)
> > Use clocksource_register_hz which calculates the shift/mult
> > factors for us.
> > 
> > Signed-off-by: Anton Blanchard <anton@samba.org>
> > ---
> > 
> > Index: linux-build/arch/powerpc/kernel/time.c
> > ===================================================================
> > --- linux-build.orig/arch/powerpc/kernel/time.c	2011-11-03 10:19:59.493679032 +1100
> > +++ linux-build/arch/powerpc/kernel/time.c	2011-11-03 10:20:00.965704053 +1100
> > @@ -86,8 +86,6 @@ static struct clocksource clocksource_rt
> >  	.rating       = 400,
> >  	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
> >  	.mask         = CLOCKSOURCE_MASK(64),
> > -	.shift        = 22,
> > -	.mult         = 0,	/* To be filled in */
> >  	.read         = rtc_read,
> >  };
> > 
> > @@ -97,8 +95,6 @@ static struct clocksource clocksource_ti
> >  	.rating       = 400,
> >  	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
> >  	.mask         = CLOCKSOURCE_MASK(64),
> > -	.shift        = 22,
> > -	.mult         = 0,	/* To be filled in */
> >  	.read         = timebase_read,
> >  };
> 
> So I've held off on ppc conversion to clocksource_register_hz due to the
> fact that the ppc vdso gettimeofday at least used to make assumptions
> that shift was 22.
> 
> Is that no longer the case?

It is still the case; specifically, update_vsyscall() in
arch/powerpc/kernel/time.c converts a multiplier value to a 'tb_to_xs'
multiplier (timebase to xsec conversion factor, where 1 xsec = 2^-20
seconds) using a factor which assumes a shift of 22.  The factor needs
to be 2^(20 + 64 - shift) / 1e9, so we could accommodate other shift
values by changing the line that computes new_tb_to_xs to do

       new_tb_to_xs = (u64) mult * (19342813113834067ULL >> shift);

assuming the shift value is easily available to update_vsyscall
(I assume it would be clock->shift).

Paul.

Re: [PATCH 2/4] powerpc/time: Use clocksource_register_hz

[john stultz]

On Sat, 2011-11-05 at 11:55 +1100, Paul Mackerras wrote:
> On Thu, Nov 03, 2011 at 09:14:44AM -0400, John Stultz wrote:
> > On Thu, 2011-11-03 at 11:59 +1100, Anton Blanchard wrote:
> > > plain text document attachment (clock3)
> > > Use clocksource_register_hz which calculates the shift/mult
> > > factors for us.
> > > 
> > > Signed-off-by: Anton Blanchard <anton@samba.org>
> > > ---
> > > 
> > > Index: linux-build/arch/powerpc/kernel/time.c
> > > ===================================================================
> > > --- linux-build.orig/arch/powerpc/kernel/time.c	2011-11-03 10:19:59.493679032 +1100
> > > +++ linux-build/arch/powerpc/kernel/time.c	2011-11-03 10:20:00.965704053 +1100
> > > @@ -86,8 +86,6 @@ static struct clocksource clocksource_rt
> > >  	.rating       = 400,
> > >  	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
> > >  	.mask         = CLOCKSOURCE_MASK(64),
> > > -	.shift        = 22,
> > > -	.mult         = 0,	/* To be filled in */
> > >  	.read         = rtc_read,
> > >  };
> > > 
> > > @@ -97,8 +95,6 @@ static struct clocksource clocksource_ti
> > >  	.rating       = 400,
> > >  	.flags        = CLOCK_SOURCE_IS_CONTINUOUS,
> > >  	.mask         = CLOCKSOURCE_MASK(64),
> > > -	.shift        = 22,
> > > -	.mult         = 0,	/* To be filled in */
> > >  	.read         = timebase_read,
> > >  };
> > 
> > So I've held off on ppc conversion to clocksource_register_hz due to the
> > fact that the ppc vdso gettimeofday at least used to make assumptions
> > that shift was 22.
> > 
> > Is that no longer the case?
> 
> It is still the case; specifically, update_vsyscall() in
> arch/powerpc/kernel/time.c converts a multiplier value to a 'tb_to_xs'
> multiplier (timebase to xsec conversion factor, where 1 xsec = 2^-20
> seconds) using a factor which assumes a shift of 22.  The factor needs
> to be 2^(20 + 64 - shift) / 1e9, so we could accommodate other shift
> values by changing the line that computes new_tb_to_xs to do
> 
>        new_tb_to_xs = (u64) mult * (19342813113834067ULL >> shift);
> 
> assuming the shift value is easily available to update_vsyscall
> (I assume it would be clock->shift).

Ok. That sounds reasonable. clock->shift should be correct there.

thanks
-john