| Message ID | 20111103010111.951358660@samba.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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
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.
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
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;
Use clocksource_register_hz which calculates the shift/mult factors for us. Signed-off-by: Anton Blanchard <anton@samba.org> ---