Messages in this thread | | | Date | Wed, 25 Oct 2006 14:20:22 -0700 | From | "Om Narasimhan" <> | Subject | Re: 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/
| |