RE: 2.6.9-rc2-mm1

From: Protasevich, Natalie
Date: Fri Sep 17 2004 - 00:21:44 EST



> On Thursday 16 September 2004 11:14 am, Jesse Barnes wrote:
> > On Thursday, September 16, 2004 2:40 am, Andrew Morton wrote:
> > bk-acpi.patch
> >
> > Looks like some changes in this patch break sn2. In particular,
this
> > hunk in
> > acpi_pci_irq_enable():
> >
> > - if (dev->irq && (dev->irq <= 0xF)) {
> > + if (dev->irq >= 0 && (dev->irq <= 0xF)) {
> > printk(" - using IRQ %d\n", dev->irq);
> > return_VALUE(dev->irq);
> > }
> > else {
> > printk("\n");
> > - return_VALUE(0);
> > + return_VALUE(-EINVAL);
> > }
> >
> > Now instead of returning 0, we'll get -EINVAL when a driver calls
> > pci_enable_device. This is arguably correct since there's no _PRT
> > entry (and in fact no ACPI namespace on sn2), but shouldn't the code

> > above be looking at the 'pin' value instead of dev->irq? The sn2
> > specific PCI code sets up each
> > dev->irq long before this with the correct values...
>
> I think the change above is actually from
> incorrect-pci-interrupt-assignment-on-es7000-for-pin-zero.patch

> of which I am officially ignorant :-)

I realize now that this is very involved piece of code and a lot was
built around the assumption that IRQ0 is a timer interrupt (pin 0 is for
PCI on ES7000), and assumption that everyone honors this assumption :)
However, it seems wrong that we are not able to read literally what ACPI
says, such as irq 0 for INTA. Maybe, it would be better if the code
above was returing an error code, not an irq, which is returned in dev
anyway. It should be some creative way to resolve this issue... I think
the idea in the comment above by Jesse Barnes has good potential.

Thanks,
--Natalie

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