Messages in this thread | | | Date | Tue, 27 Jan 2009 13:53:49 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/3] fix debug message of CPU clock speed |
| |
* Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> wrote:
> LOCAL APIC is corrected by PM-Timer, when SMI occurred while LOCAL APIC > is calibrated. In this case, LOCAL APIC debug message(Boot with > apic=debug) is displayed correctly, however, CPU clock speed debug > message is displayed wrongly .
your fix looks good, but there are a few minor style details that need to be addressed:
> When SMI occured on my machine, which has 1.6GHz CPU, CPU clock speed is > displayed 3622.0205 MHz as follow. > > CPU0: Intel(R) Xeon(R) CPU 5110 @ 1.60GHz stepping 06 > Using local APIC timer interrupts. > calibrating APIC timer ... > ... lapic delta = 3773130 > ... PM timer delta = 812434 > APIC calibration not consistent with PM Timer: 226ms instead of 100ms > APIC delta adjusted to PM-Timer: 1662420 (3773130) > ..... delta 1662420 > ..... mult: 71411249 > ..... calibration result: 265987 > ..... CPU clock speed is 3622.0205 MHz. =====> here > ..... host bus clock speed is 265.0987 MHz. > > This patch fixes to displaying CPU clock speed correctly as follow. > > CPU0: Intel(R) Xeon(R) CPU 5110 @ 1.60GHz stepping 06 > Using local APIC timer interrupts. > calibrating APIC timer ... > ... lapic delta = 3773131 > ... PM timer delta = 812434 > APIC calibration not consistent with PM Timer: 226ms instead of 100ms > APIC delta adjusted to PM-Timer: 1662420 (3773131) > TSC delta adjusted to PM-Timer: 159592409 (362220564) > ..... delta 1662420 > ..... mult: 71411249 > ..... calibration result: 265987 > ..... CPU clock speed is 1595.0924 MHz. > ..... host bus clock speed is 265.0987 MHz. > > Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> > > --- > arch/x86/kernel/apic.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c > =================================================================== > --- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c 2009-01-27 11:04:40.000000000 +0900 > +++ linux-2.6.29-rc2/arch/x86/kernel/apic.c 2009-01-27 12:29:03.000000000 +0900 > @@ -535,7 +535,8 @@ static void __init lapic_cal_handler(str > } > } > > -static int __init calibrate_by_pmtimer(long deltapm, long *delta) > +static int __init calibrate_by_pmtimer(long deltapm, long *delta, > + long *deltatsc)
The cleaner prototype line-width break is:
+ static int __init + calibrate_by_pmtimer(long deltapm, long *delta, long *deltatsc)
this function:
> @@ -569,6 +570,15 @@ static int __init calibrate_by_pmtimer(l > pr_info("APIC delta adjusted to PM-Timer: " > "%lu (%ld)\n", (unsigned long)res, *delta); > *delta = (long)res; > + > + if (cpu_has_tsc) { > + res = (((u64)(*deltatsc)) * pm_100ms); > + do_div(res, deltapm); > + apic_printk(APIC_VERBOSE, "TSC delta adjusted to " > + "PM-Timer: %lu (%ld)\n", > + (unsigned long)res, *deltatsc); > + *deltatsc = (long)res; > + }
has too deep nesting in the form of:
if (A) { ... } else { ... if (B) { ... } }
return 0;
Could you please restructure it to a cleaner control flow, in the form of:
if (A) { ... return 0; } ... if (B) { ... }
return 0;
?
Thanks,
Ingo
| |