* [PATCH] HPET: handle multiple ACPI EXTENDED_IRQ resources
@ 2006-02-07 23:50 Bjorn Helgaas
2006-02-12 22:55 ` Bob Picco
0 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2006-02-07 23:50 UTC (permalink / raw)
To: Bob Picco
Cc: Venkatesh Pallipadi, linux-acpi, Len Brown, Adam Belay,
Andrew Morton
When the _CRS for a single HPET contains multiple EXTENDED_IRQ
resources, we overwrote hdp->hd_nirqs every time we found one.
So the driver worked when all the IRQs were described in a
single EXTENDED_IRQ resource, but failed when multiple resources
were used. (Strictly speaking, I think the latter is actually
more correct, but both styles have been used.)
Someday we should remove all the ACPI stuff from hpet.c and use
PNP driver registration instead. But currently PNP_MAX_IRQ is 2,
and HPETs often have more IRQs. Hint, hint, Adam :-)
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Index: work-mm4/drivers/char/hpet.c
===================================================================
--- work-mm4.orig/drivers/char/hpet.c 2006-02-07 16:18:46.000000000 -0700
+++ work-mm4/drivers/char/hpet.c 2006-02-07 16:29:00.000000000 -0700
@@ -953,22 +953,18 @@
}
} else if (res->type == ACPI_RESOURCE_TYPE_EXTENDED_IRQ) {
struct acpi_resource_extended_irq *irqp;
- int i;
+ int i, irq;
irqp = &res->data.extended_irq;
- if (irqp->interrupt_count > 0) {
- hdp->hd_nirqs = irqp->interrupt_count;
+ for (i = 0; i < irqp->interrupt_count; i++) {
+ irq = acpi_register_gsi(irqp->interrupts[i],
+ irqp->triggering, irqp->polarity);
+ if (irq < 0)
+ return AE_ERROR;
- for (i = 0; i < hdp->hd_nirqs; i++) {
- int rc =
- acpi_register_gsi(irqp->interrupts[i],
- irqp->triggering,
- irqp->polarity);
- if (rc < 0)
- return AE_ERROR;
- hdp->hd_irq[i] = rc;
- }
+ hdp->hd_irq[hdp->hd_nirqs] = irq;
+ hdp->hd_nirqs++;
}
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] HPET: handle multiple ACPI EXTENDED_IRQ resources
2006-02-07 23:50 [PATCH] HPET: handle multiple ACPI EXTENDED_IRQ resources Bjorn Helgaas
@ 2006-02-12 22:55 ` Bob Picco
2006-02-13 21:24 ` Bjorn Helgaas
0 siblings, 1 reply; 5+ messages in thread
From: Bob Picco @ 2006-02-12 22:55 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bob Picco, Venkatesh Pallipadi, linux-acpi, Len Brown, Adam Belay,
Andrew Morton
Bjorn Helgaas wrote: [Tue Feb 07 2006, 06:50:27PM EST]
> When the _CRS for a single HPET contains multiple EXTENDED_IRQ
> resources, we overwrote hdp->hd_nirqs every time we found one.
>
> So the driver worked when all the IRQs were described in a
> single EXTENDED_IRQ resource, but failed when multiple resources
> were used. (Strictly speaking, I think the latter is actually
> more correct, but both styles have been used.)
>
> Someday we should remove all the ACPI stuff from hpet.c and use
> PNP driver registration instead. But currently PNP_MAX_IRQ is 2,
> and HPETs often have more IRQs. Hint, hint, Adam :-)
>
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
>
> Index: work-mm4/drivers/char/hpet.c
> ===================================================================
> --- work-mm4.orig/drivers/char/hpet.c 2006-02-07 16:18:46.000000000 -0700
> +++ work-mm4/drivers/char/hpet.c 2006-02-07 16:29:00.000000000 -0700
> @@ -953,22 +953,18 @@
> }
> } else if (res->type == ACPI_RESOURCE_TYPE_EXTENDED_IRQ) {
> struct acpi_resource_extended_irq *irqp;
> - int i;
> + int i, irq;
>
> irqp = &res->data.extended_irq;
>
> - if (irqp->interrupt_count > 0) {
> - hdp->hd_nirqs = irqp->interrupt_count;
> + for (i = 0; i < irqp->interrupt_count; i++) {
> + irq = acpi_register_gsi(irqp->interrupts[i],
> + irqp->triggering, irqp->polarity);
> + if (irq < 0)
> + return AE_ERROR;
>
> - for (i = 0; i < hdp->hd_nirqs; i++) {
> - int rc =
> - acpi_register_gsi(irqp->interrupts[i],
> - irqp->triggering,
> - irqp->polarity);
> - if (rc < 0)
> - return AE_ERROR;
> - hdp->hd_irq[i] = rc;
> - }
> + hdp->hd_irq[hdp->hd_nirqs] = irq;
> + hdp->hd_nirqs++;
> }
> }
>
Acked-by: Bob Picco <bob.picco@hp.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] HPET: handle multiple ACPI EXTENDED_IRQ resources
2006-02-12 22:55 ` Bob Picco
@ 2006-02-13 21:24 ` Bjorn Helgaas
2006-02-13 21:49 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2006-02-13 21:24 UTC (permalink / raw)
To: Bob Picco
Cc: Bob Picco, Venkatesh Pallipadi, linux-acpi, Len Brown, Adam Belay,
Andrew Morton, Randy.Dunlap
Andrew, I know it's late in the 2.6.16 cycle, but it would be really
nice if this patch made it. Without it, HPETs on new HP boxes don't
work, so we're going to have to get it in the distros some way or
another.
On Sunday 12 February 2006 15:55, Bob Picco wrote:
> Bjorn Helgaas wrote: [Tue Feb 07 2006, 06:50:27PM EST]
> > When the _CRS for a single HPET contains multiple EXTENDED_IRQ
> > resources, we overwrote hdp->hd_nirqs every time we found one.
> >
> > So the driver worked when all the IRQs were described in a
> > single EXTENDED_IRQ resource, but failed when multiple resources
> > were used. (Strictly speaking, I think the latter is actually
> > more correct, but both styles have been used.)
> >
> > Someday we should remove all the ACPI stuff from hpet.c and use
> > PNP driver registration instead. But currently PNP_MAX_IRQ is 2,
> > and HPETs often have more IRQs. Hint, hint, Adam :-)
> >
> > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> >
> > Index: work-mm4/drivers/char/hpet.c
> > ===================================================================
> > --- work-mm4.orig/drivers/char/hpet.c 2006-02-07 16:18:46.000000000 -0700
> > +++ work-mm4/drivers/char/hpet.c 2006-02-07 16:29:00.000000000 -0700
> > @@ -953,22 +953,18 @@
> > }
> > } else if (res->type == ACPI_RESOURCE_TYPE_EXTENDED_IRQ) {
> > struct acpi_resource_extended_irq *irqp;
> > - int i;
> > + int i, irq;
> >
> > irqp = &res->data.extended_irq;
> >
> > - if (irqp->interrupt_count > 0) {
> > - hdp->hd_nirqs = irqp->interrupt_count;
> > + for (i = 0; i < irqp->interrupt_count; i++) {
> > + irq = acpi_register_gsi(irqp->interrupts[i],
> > + irqp->triggering, irqp->polarity);
> > + if (irq < 0)
> > + return AE_ERROR;
> >
> > - for (i = 0; i < hdp->hd_nirqs; i++) {
> > - int rc =
> > - acpi_register_gsi(irqp->interrupts[i],
> > - irqp->triggering,
> > - irqp->polarity);
> > - if (rc < 0)
> > - return AE_ERROR;
> > - hdp->hd_irq[i] = rc;
> > - }
> > + hdp->hd_irq[hdp->hd_nirqs] = irq;
> > + hdp->hd_nirqs++;
> > }
> > }
> >
>
> Acked-by: Bob Picco <bob.picco@hp.com>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] HPET: handle multiple ACPI EXTENDED_IRQ resources
2006-02-13 21:24 ` Bjorn Helgaas
@ 2006-02-13 21:49 ` Andrew Morton
2006-02-13 21:55 ` Bjorn Helgaas
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2006-02-13 21:49 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: bob.picco, robert.picco, venkatesh.pallipadi, linux-acpi,
len.brown, ambx1, rdunlap
Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
>
> Andrew, I know it's late in the 2.6.16 cycle, but it would be really
> nice if this patch made it. Without it, HPETs on new HP boxes don't
> work, so we're going to have to get it in the distros some way or
> another.
OK..
What about hpet-fix-acpi-memory-range-length-handling.patch?
From: Bjorn Helgaas <bjorn.helgaas@hp.com>
ACPI address space descriptors contain _MIN, _MAX, and _LEN. _MIN and _MAX
are the bounds within which the region can be moved (this is clarified in
Table 6-38 of the ACPI 3.0 spec). We should use _LEN to determine the size
of the region, not _MAX - _MIN + 1.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
drivers/char/hpet.c | 5 +----
1 files changed, 1 insertion(+), 4 deletions(-)
diff -puN drivers/char/hpet.c~hpet-fix-acpi-memory-range-length-handling drivers/char/hpet.c
--- 25/drivers/char/hpet.c~hpet-fix-acpi-memory-range-length-handling Tue Feb 7 15:47:01 2006
+++ 25-akpm/drivers/char/hpet.c Tue Feb 7 15:47:01 2006
@@ -925,11 +925,8 @@ static acpi_status hpet_resources(struct
status = acpi_resource_to_address64(res, &addr);
if (ACPI_SUCCESS(status)) {
- unsigned long size;
-
- size = addr.maximum - addr.minimum + 1;
hdp->hd_phys_address = addr.minimum;
- hdp->hd_address = ioremap(addr.minimum, size);
+ hdp->hd_address = ioremap(addr.minimum, addr.address_length);
if (hpet_is_known(hdp)) {
printk(KERN_DEBUG "%s: 0x%lx is busy\n",
_
(It's hard for me to judge when people don't describe what the impact of
the problem is, so thanks for the poke).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] HPET: handle multiple ACPI EXTENDED_IRQ resources
2006-02-13 21:49 ` Andrew Morton
@ 2006-02-13 21:55 ` Bjorn Helgaas
0 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2006-02-13 21:55 UTC (permalink / raw)
To: Andrew Morton
Cc: bob.picco, robert.picco, venkatesh.pallipadi, linux-acpi,
len.brown, ambx1, rdunlap
On Monday 13 February 2006 14:49, Andrew Morton wrote:
> Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> >
> > Andrew, I know it's late in the 2.6.16 cycle, but it would be really
> > nice if this patch made it. Without it, HPETs on new HP boxes don't
> > work, so we're going to have to get it in the distros some way or
> > another.
>
> OK..
>
> What about hpet-fix-acpi-memory-range-length-handling.patch?
The range length patch has no functional impact on any machine I
know about; it's just a cleanup/align-with-spec thing. Definitely
post-2.6.16 material.
> From: Bjorn Helgaas <bjorn.helgaas@hp.com>
>
> ACPI address space descriptors contain _MIN, _MAX, and _LEN. _MIN and _MAX
> are the bounds within which the region can be moved (this is clarified in
> Table 6-38 of the ACPI 3.0 spec). We should use _LEN to determine the size
> of the region, not _MAX - _MIN + 1.
>
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> Signed-off-by: Andrew Morton <akpm@osdl.org>
> ---
>
> drivers/char/hpet.c | 5 +----
> 1 files changed, 1 insertion(+), 4 deletions(-)
>
> diff -puN drivers/char/hpet.c~hpet-fix-acpi-memory-range-length-handling drivers/char/hpet.c
> --- 25/drivers/char/hpet.c~hpet-fix-acpi-memory-range-length-handling Tue Feb 7 15:47:01 2006
> +++ 25-akpm/drivers/char/hpet.c Tue Feb 7 15:47:01 2006
> @@ -925,11 +925,8 @@ static acpi_status hpet_resources(struct
> status = acpi_resource_to_address64(res, &addr);
>
> if (ACPI_SUCCESS(status)) {
> - unsigned long size;
> -
> - size = addr.maximum - addr.minimum + 1;
> hdp->hd_phys_address = addr.minimum;
> - hdp->hd_address = ioremap(addr.minimum, size);
> + hdp->hd_address = ioremap(addr.minimum, addr.address_length);
>
> if (hpet_is_known(hdp)) {
> printk(KERN_DEBUG "%s: 0x%lx is busy\n",
> _
>
>
> (It's hard for me to judge when people don't describe what the impact of
> the problem is, so thanks for the poke).
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2006-02-13 21:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-07 23:50 [PATCH] HPET: handle multiple ACPI EXTENDED_IRQ resources Bjorn Helgaas
2006-02-12 22:55 ` Bob Picco
2006-02-13 21:24 ` Bjorn Helgaas
2006-02-13 21:49 ` Andrew Morton
2006-02-13 21:55 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox