lkml.org 
[lkml]   [2009]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] fix debug message of CPU clock speed
Hi, Ingo

Thank you for your comments.
I apply your comments to the patch ASAP.

Regard,
Yasuaki Ishimatsu

Ingo Molnar wrote:
> * 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
>



\
 
 \ /
  Last update: 2009-01-28 03:43    [W:0.076 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site