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

[john stultz <johnstul@us.ibm.com>]

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