Messages in this thread Patches in this message | | | From | Jim Cromie <> | Date | Tue, 24 Jan 2012 11:53:32 -0700 | Subject | Re: [RFC][PATCH 2/3] clocksource: scx200_hrt: Convert scx200 to use clocksource_register_hz |
| |
On Mon, Jan 23, 2012 at 5:05 PM, John Stultz <john.stultz@linaro.org> wrote: > Converts the scx200 clocksource to using clocksource_register_hz. > > This wasn't a straight conversion, so would like some extra review > by the maintainers. >
hi John,
very nice. Tested with mhz27 too.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
It eliminates this noise that Ive started seeing recently (for some value of recent)
enabling scx200 high-res timer (1 MHz +0 ppm) ------------[ cut here ]------------ WARNING: at /home/jimc/projects/lx/linux-2.6/kernel/time/clocksource.c:740 clocksource_register+0x78/0xbd() Clocksource scx200_hrt might overflow on 11% adjustment Modules linked in: scx200_hrt(+) scx200 pc8736x_gpio nsc_gpio pc87360 hwmon_vid scx200_acb ohci_hcd pata_sc1200 Pid: 291, comm: modprobe Not tainted 3.2.0-skd-00063-g296c136 #256 Call Trace: [<c01201a1>] warn_slowpath_common+0x7c/0x91 [<c0140266>] ? clocksource_register+0x78/0xbd [<c0140266>] ? clocksource_register+0x78/0xbd [<c0120241>] warn_slowpath_fmt+0x33/0x35 [<c0140266>] clocksource_register+0x78/0xbd [<c886412d>] init_hrt_clocksource+0x12d/0x139 [scx200_hrt] [<c010108b>] do_one_initcall+0x8b/0x138 [<c8864000>] ? 0xc8863fff [<c014a6db>] sys_init_module+0x13ef/0x160e [<c042015c>] syscall_call+0x7/0xb ---[ end trace 69c8ca76cf89ab2d ]--- Switching to clocksource scx200_hrt Loading kernel modules...chrdev_alloc_region: chrdev_alloc_region reserves a dynamic major, but was called with major:3077. Caller should pass major:0 for this. done. ------------[ cut here ]------------ WARNING: at /home/jimc/projects/lx/linux-2.6/kernel/time/timekeeping.c:863 do_timer+0x56f/0x6ae() Adjusting scx200_hrt more then 11% (-100663808 vs 360710144)
NB - s/then/than/ Im attaching a patch for that. Also, "when then" is a little awkward. /* * Have to be careful here. If we're in oneshot mode, * before we call tick_periodic() in a loop, we need * to be sure we're using a real hardware clocksource. * Otherwise we could get trapped in an infinite * loop, as the tick_periodic() increments jiffies, * when then will increment time, posibly causing * the loop to trigger again and again. */
Modules linked in: scx200_gpio scx200_hrt scx200 pc8736x_gpio nsc_gpio pc87360 hwmon_vid scx200_acb ohci_hcd pata_sc1200 Pid: 58, comm: collector Tainted: G W 3.2.0-skd-00063-g296c136 #256 Call Trace: [<c01201a1>] warn_slowpath_common+0x7c/0x91 [<c013ebbe>] ? do_timer+0x56f/0x6ae [<c013ebbe>] ? do_timer+0x56f/0x6ae [<c0120241>] warn_slowpath_fmt+0x33/0x35 [<c013ebbe>] do_timer+0x56f/0x6ae [<c014319b>] tick_do_update_jiffies64+0xc0/0xee [<c01432e1>] tick_sched_timer+0x45/0x9e [<c0139c01>] ? __remove_hrtimer+0x2a/0x87 [<c0139d81>] __run_hrtimer+0x93/0x131 [<c014329c>] ? tick_nohz_handler+0xd3/0xd3 [<c013a549>] hrtimer_interrupt+0xe1/0x1f4 [<c010346d>] timer_interrupt+0x18/0x1f [<c015fbcf>] handle_irq_event_percpu+0x4a/0x197 [<c014bcfe>] ? pm_qos_request+0xe/0x16 [<c0104155>] ? mask_and_ack_8259A+0x77/0xde [<c0161a3c>] ? unmask_irq+0x23/0x23 [<c015fd58>] handle_irq_event+0x3c/0x54 [<c0161a8e>] handle_level_irq+0x52/0x80 <IRQ> [<c0102f2c>] ? do_IRQ+0x39/0x87 [<c02b561a>] ? disk_seqf_start+0x2e/0x84 [<c0424c70>] ? common_interrupt+0x30/0x40 [<c01c007b>] ? do_vfs_ioctl+0x30c/0x481 [<c01f21e9>] ? proc_readdir_de+0x166/0x186 [<c01c0324>] ? filldir64+0xcc/0xcc [<c01f222a>] ? proc_readdir+0x21/0x25 [<c01c0324>] ? filldir64+0xcc/0xcc [<c01edbe9>] ? proc_root_readdir+0x1f/0x41 [<c01c0509>] ? vfs_readdir+0x6a/0x8f [<c01c0324>] ? filldir64+0xcc/0xcc [<c01c05e7>] ? sys_getdents+0x63/0x9f [<c042015c>] ? syscall_call+0x7/0xb [<c0420000>] ? rwsem_down_failed_common+0x5f/0xbc ---[ end trace 69c8ca76cf89ab2e ]---
> CC: Jim Cromie <jim.cromie@gmail.com> > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > drivers/clocksource/scx200_hrt.c | 19 ++++++------------- > 1 files changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/clocksource/scx200_hrt.c b/drivers/clocksource/scx200_hrt.c > index 27f4d96..24f884c 100644 > --- a/drivers/clocksource/scx200_hrt.c > +++ b/drivers/clocksource/scx200_hrt.c > @@ -49,9 +49,6 @@ static cycle_t read_hrt(struct clocksource *cs) > return (cycle_t) inl(scx200_cb_base + SCx200_TIMER_OFFSET); > } > > -#define HRT_SHIFT_1 22 > -#define HRT_SHIFT_27 26 > - > static struct clocksource cs_hrt = { > .name = "scx200_hrt", > .rating = 250, > @@ -63,6 +60,7 @@ static struct clocksource cs_hrt = { > > static int __init init_hrt_clocksource(void) > { > + u32 freq; > /* Make sure scx200 has initialized the configuration block */ > if (!scx200_cb_present()) > return -ENODEV; > @@ -79,19 +77,14 @@ static int __init init_hrt_clocksource(void) > outb(HR_TMEN | (mhz27 ? HR_TMCLKSEL : 0), > scx200_cb_base + SCx200_TMCNFG_OFFSET); > > - if (mhz27) { > - cs_hrt.shift = HRT_SHIFT_27; > - cs_hrt.mult = clocksource_hz2mult((HRT_FREQ + ppm) * 27, > - cs_hrt.shift); > - } else { > - cs_hrt.shift = HRT_SHIFT_1; > - cs_hrt.mult = clocksource_hz2mult(HRT_FREQ + ppm, > - cs_hrt.shift); > - } > + freq = (HRT_FREQ + ppm); > + if (mhz27) > + freq *= 27; > + > printk(KERN_INFO "enabling scx200 high-res timer (%s MHz +%d ppm)\n", > mhz27 ? "27":"1", ppm); > > - return clocksource_register(&cs_hrt); > + return clocksource_register_hz(&cs_hrt, freq); > } > > module_init(init_hrt_clocksource); > -- > 1.7.3.2.146.gca209 > From f75b23207d5729a9ec72ba0d05d9a07e86291b1b Mon Sep 17 00:00:00 2001 From: Jim Cromie <jim.cromie@gmail.com> Date: Tue, 24 Jan 2012 11:47:55 -0700 Subject: [PATCH] time: s/then/than/ where appropriate
Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- kernel/time/clocksource.c | 2 +- kernel/time/timekeeping.c | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c index d3ad022..ebf98fb 100644 --- a/kernel/time/clocksource.c +++ b/kernel/time/clocksource.c @@ -500,7 +500,7 @@ static u32 clocksource_max_adjustment(struct clocksource *cs) { u64 ret; /* - * We won't try to correct for more then 11% adjustments (110,000 ppm), + * We won't try to correct for more than 11% adjustments (110,000 ppm), */ ret = (u64)cs->mult * 11; do_div(ret,100); diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 2378413..e77c168 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -807,7 +807,7 @@ static void timekeeping_adjust(s64 offset) int adj; /* - * The point of this is to check if the error is greater then half + * The point of this is to check if the error is greater than half * an interval. * * First we shift it down from NTP_SHIFT to clocksource->shifted nsecs. @@ -815,7 +815,7 @@ static void timekeeping_adjust(s64 offset) * Note we subtract one in the shift, so that error is really error*2. * This "saves" dividing(shifting) intererval twice, but keeps the * (error > interval) comparision as still measuring if error is - * larger then half an interval. + * larger than half an interval. * * Note: It does not "save" on aggrivation when reading the code. */ @@ -823,7 +823,7 @@ static void timekeeping_adjust(s64 offset) if (error > interval) { /* * We now divide error by 4(via shift), which checks if - * the error is greater then twice the interval. + * the error is greater than twice the interval. * If it is greater, we need a bigadjust, if its smaller, * we can adjust by 1. */ @@ -857,7 +857,7 @@ static void timekeeping_adjust(s64 offset) WARN_ONCE(timekeeper.clock->maxadj && (timekeeper.mult + adj > timekeeper.clock->mult + timekeeper.clock->maxadj), - "Adjusting %s more then 11%% (%ld vs %ld)\n", + "Adjusting %s more than 11%% (%ld vs %ld)\n", timekeeper.clock->name, (long)timekeeper.mult + adj, (long)timekeeper.clock->mult + timekeeper.clock->maxadj); @@ -932,7 +932,7 @@ static cycle_t logarithmic_accumulation(cycle_t offset, int shift) u64 nsecps = (u64)NSEC_PER_SEC << timekeeper.shift; u64 raw_nsecs; - /* If the offset is smaller then a shifted interval, do nothing */ + /* If the offset is smaller than a shifted interval, do nothing */ if (offset < timekeeper.cycle_interval<<shift) return offset; @@ -995,13 +995,13 @@ static void update_wall_time(void) * With NO_HZ we may have to accumulate many cycle_intervals * (think "ticks") worth of time at once. To do this efficiently, * we calculate the largest doubling multiple of cycle_intervals - * that is smaller then the offset. We then accumulate that + * that is smaller than the offset. We then accumulate that * chunk in one go, and then try to consume the next smaller * doubled multiple. */ shift = ilog2(offset) - ilog2(timekeeper.cycle_interval); shift = max(0, shift); - /* Bound shift to one less then what overflows tick_length */ + /* Bound shift to one less than what overflows tick_length */ maxshift = (8*sizeof(tick_length) - (ilog2(tick_length)+1)) - 1; shift = min(shift, maxshift); while (offset >= timekeeper.cycle_interval) { @@ -1047,7 +1047,7 @@ static void update_wall_time(void) /* * Finally, make sure that after the rounding - * xtime.tv_nsec isn't larger then NSEC_PER_SEC + * xtime.tv_nsec isn't larger than NSEC_PER_SEC */ if (unlikely(xtime.tv_nsec >= NSEC_PER_SEC)) { xtime.tv_nsec -= NSEC_PER_SEC; -- 1.7.8.1
| |