lkml.org 
[lkml]   [2009]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][patch 10/12] move NTP adjusted clock multiplier to struct timekeeper
    On Thu, 30 Jul 2009 15:04:48 -0700
    john stultz <johnstul@us.ibm.com> wrote:

    > On Wed, 2009-07-29 at 15:41 +0200, Martin Schwidefsky wrote:
    > > plain text document attachment (timekeeper-mult.diff)
    > > From: Martin Schwidefsky <schwidefsky@de.ibm.com>
    > >
    > > The clocksource structure has two multipliers, the unmodified multiplier
    > > clock->mult_orig and the NTP corrected multiplier clock->mult. The NTP
    > > multiplier is misplaced in the struct clocksource, this is private
    > > information of the timekeeping code. Add the mult field to the struct
    > > timekeeper to contain the NTP corrected value, keep the unmodifed
    > > multiplier in clock->mult and remove clock->mult_orig. For consistency
    > > add the shift value associated with the NTP corrected mult value to
    > > struct timekeeper as well.
    > >
    > > Cc: Ingo Molnar <mingo@elte.hu>
    > > Cc: Thomas Gleixner <tglx@linutronix.de>
    > > Cc: john stultz <johnstul@us.ibm.com>
    > > Cc: Daniel Walker <dwalker@fifo99.com>
    > > Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
    > > ---
    > > arch/arm/plat-omap/common.c | 2 +-
    > > include/linux/clocksource.h | 4 +---
    > > kernel/time/timekeeping.c | 43 +++++++++++++++++++++++++------------------
    > > 3 files changed, 27 insertions(+), 22 deletions(-)
    > >
    > > Index: linux-2.6/include/linux/clocksource.h
    > > ===================================================================
    > > --- linux-2.6.orig/include/linux/clocksource.h
    > > +++ linux-2.6/include/linux/clocksource.h
    > > @@ -149,8 +149,7 @@ extern u64 timecounter_cyc2time(struct t
    > > * @disable: optional function to disable the clocksource
    > > * @mask: bitmask for two's complement
    > > * subtraction of non 64 bit counters
    > > - * @mult: cycle to nanosecond multiplier (adjusted by NTP)
    > > - * @mult_orig: cycle to nanosecond multiplier (unadjusted by NTP)
    > > + * @mult: cycle to nanosecond multiplier
    > > * @shift: cycle to nanosecond divisor (power of two)
    > > * @flags: flags describing special properties
    > > * @vread: vsyscall based read
    > > @@ -168,7 +167,6 @@ struct clocksource {
    > > void (*disable)(struct clocksource *cs);
    > > cycle_t mask;
    > > u32 mult;
    > > - u32 mult_orig;
    > > u32 shift;
    > > unsigned long flags;
    > > cycle_t (*vread)(void);
    > > Index: linux-2.6/kernel/time/timekeeping.c
    > > ===================================================================
    > > --- linux-2.6.orig/kernel/time/timekeeping.c
    > > +++ linux-2.6/kernel/time/timekeeping.c
    > > @@ -29,6 +29,8 @@ struct timekeeper {
    > > s64 ntp_error;
    > > int xtime_shift;
    > > int ntp_error_shift;
    > > + u32 mult; /* NTP adjusted clock multiplier */
    > > + u32 shift; /* NTP adjusted clock shift */
    >
    > I'm a little confused here. NTP doesn't adjust the shift value. This
    > seems redundant with the xtime_shift value introduced in [patch 9/12].

    True the NTP code does not adjust the shift value. But mult and shift
    build a pair, no? I think it is more readable if we use timekeeper.mult
    and timekeeper.shift instead of timekeeper.mult and clock->shift.
    And the xtime_shift does not need to be the same value as the clock
    shift - at least in theory. It is just a shift to increase precision.
    Therefore I would like to use the xtime_shift and ntp_error_shift values
    exclusively as the shifts for the respective xtime_nsec and ntp_error
    values. Makes sense ?

    --
    blue skies,
    Martin.

    "Reality continues to ruin my life." - Calvin.



    \
     
     \ /
      Last update: 2009-07-31 09:55    [W:2.383 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site