Re: [PATCH] pci: do not try to assign irq 255

From: Hannes Reinecke
Date: Tue Feb 26 2013 - 08:50:11 EST


On 02/26/2013 02:29 PM, David Härdeman wrote:
On Thu, Feb 21, 2013 at 07:53:14AM +0100, Hannes Reinecke wrote:
On 02/20/2013 05:57 PM, Yinghai Lu wrote:
it seems you mess pin with interrupt line.

current code:
unsigned char irq;

pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &irq);
dev->pin = irq;
if (irq)
pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
dev->irq = irq;

so if the device does not have interrupt pin implemented, pin should be zero.
and pin and irq in dev should
be all 0.

But the device _has_ an interrupt pin implemented.
The whole point here is that the interrupt line is _NOT_ zero.

...

So at one point we have to decide that ->irq is not valid, despite it
being not set to zero.
An alternative fix would be this:

diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index 68a921d..4a480cb 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -469,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
} else {
dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
pin_name(pin));
+ dev->irq = 0;
}
return 0;
}

Which probably is a better solution, as here ->irq is _definitely_
not valid, so we should reset it to '0' to avoid confusion on upper
layers.


Is there any agreement on how to proceed?

I would actually prefer the second solution, as the ACPI code gives
some better guarantees here. With the original solution it _might_ be that on non-ACPI systems an interrupt 255 is valid, so it might
incur unwanted regressions.

However, for an ACPI system we only have the two choices, assigning
an interrupt via ACPI tables or use a default GSI value.
If both failed the interrupt definitely is not valid and can safely
be reset to 0.

But this would need a formal ACK from the ACPI gods ...
Len? Rafael?

Cheers,

Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxx +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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/