lkml.org 
[lkml]   [2008]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86: Get irq for hpet timer
On Wed, 28 May 2008, Kevin Hao wrote:

> BTW, Maciej. I am not familiar with ACPI subsystem. But I think the
> acpi_register_gsi should return -1 if mp_find_ioapic return error.
> Is that right? If you also think so, I will make a patch for that.

Well, the interface is documented in the sources -- I do not feel like
reading through all the execution paths to see whether the implementation
matches though. ;)

A few comments below -- I may not have time available to throw your piece
of code at a piece of hardware though.

> + /* we prefer level triggered mode */
> + v = readl(&timer->hpet_config);
> + if (!(v & Tn_INT_TYPE_CNF_MASK)) {
> + v |= Tn_INT_TYPE_CNF_MASK;
> + writel(v, &timer->hpet_config);
> + }

Please note that when routing through the 8259A you need to match the
trigger mode set for the respective input. It is normally set in the ELCR
register in the hardware and should be recorded in the ACPI interrupt
source tables too -- the default for IRQ0-15 is edge-triggered, active
high, unless overridden.

> + v = (readq(&timer->hpet_config) & Tn_INT_ROUTE_CAP_MASK)
> + >> Tn_INT_ROUTE_CAP_SHIFT;

The operator should be at the end of the first line IMHO.

> + /*
> + * In PIC mode, skip IRQ0-4, IRQ6-8, IRQ12-15 which is always used by
> + * legacy device. In IO APIC mode, we skip all the legacy IRQS.
> + */

Quite reasonable assumption IMO, but again -- I'd expect the information
about legacy devices present on these lines to be recorded in ACPI tables.
Note that IRQ9 is used for the SCI -- I am not sure if that's a good
choice for sharing.

> + gsi = acpi_register_gsi(irq, ACPI_LEVEL_SENSITIVE,
> + ACPI_ACTIVE_LOW);

I am assuming you have verified ACPI_ACTIVE_LOW works here, contrary to
the HPET spec.

> + if (irq < HPET_MAX_IRQ) {

Single space here.

> @@ -37,6 +37,7 @@ struct hpet {
> #define hpet_compare _u1._hpet_compare
>
> #define HPET_MAX_TIMERS (32)
> +#define HPET_MAX_IRQ (32)

Tab here for consistency with the others?

Otherwise no obvious problems.

Maciej


\
 
 \ /
  Last update: 2008-05-29 05:17    [W:0.069 / U:0.376 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site