RE: [PATCH] Typo in x86 apic.c with DIVISOR setup

[Pallipadi, Venkatesh]

>-----Original Message-----
>From: Cyrill Gorcunov [mailto:gorcunov@xxxxxxxxx]
>Sent: Thursday, October 09, 2008 9:38 PM
>To: Pallipadi, Venkatesh
>Cc: Ingo Molnar; Thomas Gleixner; H. Peter Anvin; linux-kernel
>Subject: Re: [PATCH] Typo in x86 apic.c with DIVISOR setup
>
>On Fri, Oct 10, 2008 at 4:22 AM, Venki Pallipadi
><venkatesh.pallipadi@xxxxxxxxx> wrote:
>>
>> Fix a typo in x86 apic.c CONFG_X86_64 -> CONFIG_X86_64.
>> The intention looks like was to have divisor of 1 on x86_64 kernel.
>>
>> Also, for DIVISOR value of 1 to work, APIC_TDR_DIV should also change
>> depending on divide by 1 or divide by 16 is used.
>>
>> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@xxxxxxxxx>
>>
>> ---
>>  arch/x86/kernel/apic.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> Index: tip/arch/x86/kernel/apic.c
>> ===================================================================
>> --- tip.orig/arch/x86/kernel/apic.c     2008-10-09
>17:12:39.000000000 -0700
>> +++ tip/arch/x86/kernel/apic.c  2008-10-09 17:13:29.000000000 -0700
>> @@ -332,10 +332,12 @@ int lapic_get_maxlvt(void)
>>  */
>>
>>  /* Clock divisor */
>> -#ifdef CONFG_X86_64
>> +#ifdef CONFIG_X86_64
>>  #define APIC_DIVISOR 1
>> +#define APIC_TDR_DIV APIC_TDR_DIV_1
>>  #else
>>  #define APIC_DIVISOR 16
>> +#define APIC_TDR_DIV APIC_TDR_DIV_16
>>  #endif
>>
>>  /*
>> @@ -369,7 +371,7 @@ static void __setup_APIC_LVTT(unsigned i
>>        tmp_value = apic_read(APIC_TDCR);
>>        apic_write(APIC_TDCR,
>>                (tmp_value & ~(APIC_TDR_DIV_1 |
>APIC_TDR_DIV_TMBASE)) |
>> -               APIC_TDR_DIV_16);
>> +               APIC_TDR_DIV);
>>
>>        if (!oneshot)
>>                apic_write(APIC_TMICT, clocks / APIC_DIVISOR);
>>
>
>Hi Venki,
>
>you know I don't have code under my hands now to take a look on
>(i'm in office now) -- but if I recall correctly it's not a typo :) By
>writting (1) instead of (16) you make counter decrease faster by
>16 times (if to compare with what we have now). So we could get
>counter underflow before we've finish calibration. That is was the
>original intension of this 16 divisor being used. Did you test this
>patch?
>

Typo part I mentioned was this

>> -#ifdef CONFG_X86_64
>> +#ifdef CONFIG_X86_64

Looks like intention was to use DIVISOR of 1 for x86_64 and divisor of
16 for 32 bit kernel. Once I changed that, there was still one place in
64 bit code that was assuming divisor of 16. The rest of the patch
changes that part.

Yes. Tested this patch on couple of systems here and worked fine.

Thanks,
Venki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/