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

login
register
mail settings
Submitter Anton Blanchard
Date Nov. 3, 2011, 12:59 a.m.
Message ID <20111103010111.951358660@samba.org>
Download mbox | patch
Permalink /patch/123379/
State Superseded
Headers show

Comments

Anton Blanchard - Nov. 3, 2011, 12:59 a.m.
Use clocksource_register_hz which calculates the shift/mult
factors for us.

Signed-off-by: Anton Blanchard <anton@samba.org>
---
john stultz - Nov. 3, 2011, 1:14 p.m.
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
Paul Mackerras - Nov. 5, 2011, 12:55 a.m.
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.
john stultz - Nov. 7, 2011, 6:26 p.m.
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

Patch

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;