Re: [RFC][patch 10/12] move NTP adjusted clock multiplier to struct timekeeper

john stultz
2009-07-30T22:04:52Z

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].

thanks
-john