lkml.org 
[lkml]   [2006]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: HPET : Legacy Routing Replacement Enable - 3rd try.
On 10/25/06, Pallipadi, Venkatesh <venkatesh.pallipadi@intel.com> wrote:
>
> General comment. I guess this patch will conflict with timer cleanups
> and hrt timer patches. This patch being smaller, it may be easier to
> rebase this against hrt timer patches.
Thanks for comments.
Rebasing against hrt and other cleanups is a good idea. I would wait
till they make it into mainline.
>
> >+ */
> >+ printk(KERN_INFO PREFIX "HPET id: %#x. ACPI LRR bit %s SET\n",
> >+ hpet_tbl->id, acpi_hpet_lrr ? "": "NOT");
>
> I don't see acpi_hpet_lrr getting used anywhere in the patch? Are you
> planning to change it in any subsequent patches?

No. Let me explain what I observed.

I tested against five different bioses (some with 8132, some with
CK-804 ..etc) and I observed three different patterns.

1. HW is LRR capable, HPET ACPI it is 1, timer interrupt is on INT2.
Before the fix: Linux cannot get timer interrupts on INT0, goes for ACPI timer.
After the fix : Works fine. This is according to hpet spec.

2. HW is LRR capable, HPET ACPI bit is set to 0. timer interrupt is on INT2.
Before the fix : Linux cannot get timer interrupts on INT0, goes for
ACPI timer.
After the fix: Faulty BIOS behavior. Introduced parameter
hpet_lrr_force to handle this situation.

3. HW is LRR capable, HPET ACPI bit is set to 1. timer interrupt is on INT0.
Before the fix: Linux works fine.
After the fix : Faulty BIOS. No way to know which INT timer is connected to.

To handle case 3, I removed all references to acpi_hpet_lrr, explained
this case in the code and decided to solely rely on the command line
parameter for LRR capability. Rational for this approach is ,
1. At present, there are not many BIOSes which implement LRR (correctly)
2. People would see the bootup message (MP-BIOS bug...) if LRR is
enabled and no timer interrupt on INT0. They can pass the hpet_lrr=1
to make everything work fine.
Is it the right approach?

>
> >
> > #ifdef CONFIG_X86_64
> > vxtime.hpet_address = hpet_tbl->addr.addrl |
> >diff --git a/arch/i386/kernel/time_hpet.c
> >b/arch/i386/kernel/time_hpet.c
> >index 1a2a979..01b2f67 100644
> >--- a/arch/i386/kernel/time_hpet.c
> >+++ b/arch/i386/kernel/time_hpet.c
> >@@ -94,7 +94,8 @@ static int hpet_timer_stop_set_go(unsign
> > * Go!
> > */
> > cfg = hpet_readl(HPET_CFG);
> >- if (hpet_use_timer)
> >+ /* Ideally the following should be &&(acpi_hpet_lrr ||
> >hpet_lrr_force) */
> >+ if (hpet_use_timer && hpet_lrr_force)
>
> What will be the value of hpet_lrr_force if no boot parameter was used.
zero.
> It will end up coming from uninitialized data section. Right?
Since it is a global variable, I assumed it would be automatically
initialized to zero, and zero is the expected default. Did I miss
something obvious?

>
> So, CFG_LEGACY will not be set on any platforms unless lrr_force
> parameter is used? Is that the intention or am I missing something?
Yes. That is the way I could think of to handle faulty bios implementations.
>
> >- setup_irq(0, &irq0);
> >+ printk(KERN_WARNING PREFIX "Registering Timer IRQ =
> >%d\n", timer_irq);
>
> Why is this an unconditional warning?
hmm... That is not required. I should have removed it.

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

\
 
 \ /
  Last update: 2006-10-25 23:23    [W:0.052 / U:0.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site