[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5125C45A.5020208@suse.de>
Date: Thu, 21 Feb 2013 07:53:14 +0100
From: Hannes Reinecke <hare@...e.de>
To: Yinghai Lu <yinghai@...nel.org>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, linux-kernel@...r.kernel.org,
Frederik Himpe <fhimpe@....ac.be>,
Oliver Neukum <oneukum@...e.de>,
David Haerdeman <david@...deman.nu>, linux-usb@...r.kernel.org,
linux-pci@...r.kernel.org
Subject: Re: [PATCH] pci: do not try to assign irq 255
On 02/20/2013 05:57 PM, Yinghai Lu wrote:
> On Tue, Feb 19, 2013 at 11:58 PM, Hannes Reinecke <hare@...e.de> wrote:
>>>
>> Apparently this device is meant to use MSI _only_ so the BIOS developer
>> didn't feel the need to assign an INTx here.
>>
>> According to PCI-3.0, section 6.8 (Message Signalled Interrupts):
>>> It is recommended that devices implement interrupt pins to
>>> provide compatibility in systems that do not support MSI
>>> (devices default to interrupt pins). However, it is expected
>>> that the need for interrupt pins will diminish over time.
>>> Devices that do not support interrupt pins due to pin
>>> constraints (rely on polling for device service) may implement
>>> messages to increase performance without adding additional pins. >
>>> Therefore, system configuration software must not assume that a
>>> message capable device has an interrupt pin.
>>
>> Which sounds to me as if the implementation is valid...
>
> 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.
00:14.0 USB controller [0c03]: Intel Corporation 7 Series/C210
Series Chipset Family USB xHCI Host Controller [8086:1e31] (rev 04)
(prog-if 30 [XHCI])
Subsystem: Hewlett-Packard Company Device [103c:179b]
Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
Interrupt: pin A routed to IRQ 255
Region 0: Memory at d4720000 (64-bit, non-prefetchable) [size=64K]
Capabilities: [70] Power Management version 2
Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA
PME(D0-,D1-,D2-,D3hot+,D3cold+)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [80] MSI: Enable- Count=1/8 Maskable- 64bit+
Address: 0000000000000000 Data: 0000
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.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@...e.de +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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists