On 10/25/06, Pallipadi, Venkatesh <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
