On Mon, 2008-12-22 at 02:56 +0400, Manu Abraham wrote:
> Michael Ellerman wrote:
> > On Sun, 2008-12-21 at 01:13 +0400, Manu Abraham wrote:
> >> Grant Grundler wrote:
> >>> On Sun, Dec 14, 2008 at 03:15:15PM +0400, Manu Abraham wrote:
> >>> ...
> >>>>> A "GSI" (Generic Sys Interrupt?) is associated with each entry in
> >>>>> the MSI-X table. Driver then calls request_irq() to bind an interrupt
> >>>>> handler to each GSI. So the driver never directly sees the "message".
> >>>> Oh, you mean the array of irq_handlers in the MSI-X table should
> >>>> correspond to a particular message ?
> >>> No. I mean each MSI-X entry has an address+message pair and each MSI-X entry
> >>> is associated with the parameters passed to each call of request_irq().
> >>> Pass in unique private data to each call of request_irq() and the driver
> >>> can determine which message was delivered when the ISR gets called.
> >>> (ISR == Interrupt Service Routine, aka driver IRQ handler)
> >>
> >> Is it possible that even when the config space says that multiple messages
> >> are supported, you cannot enable MSI-X ?
> >
> > Yes, absolutely. Have a look at pci_msi_check_device() for starters. MSI
> > can be disabled globally, or per-device by a quirk, the bridges above
> > your device might not support MSI and the arch code may prevent
> > MSI/MSI-X for some reason (ie. platform doesn't support it).
> >
> > There's also a check in pci_enable_msix() to make sure the entries you
> > pass in are valid.
> >
> >> ------------- with MSI-X -------------
> >>
> >> saa716x_pci_init (0): found a NEMO reference board PCIe card
> >> ACPI: PCI Interrupt 0000:05:00.0[A] -> GSI 19 (level, low) -> IRQ 19
> >> PCI: Setting latency timer of device 0000:05:00.0 to 64
> >> saa716x_request_irq (0): Using MSI-X mode
> >> saa716x_enable_msix (0): MSI-X request failed
> >
> > For starters you should print the error code returned by
> > pci_enable_msix() - unfortunately it returns EINVAL for many different
> > reasons, but it will narrow it down a bit.
>
> It does return -22
If you're curious you could try this patch.
>From bd27f10c9acd24bb849d14b6fc373444322c7405 Mon Sep 17 00:00:00 2001
From: Michael Ellerman <michael@xxxxxxxxxxxxxx>
Date: Mon, 22 Dec 2008 10:21:27 +1100
Subject: [PATCH] MSI debug
---
drivers/pci/msi.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 74801f7..575cca9 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -517,17 +517,26 @@ static int pci_msi_check_device(struct pci_dev* dev, int nvec, int type)
struct pci_bus *bus;
int ret;
+ if (!dev) {
+ printk(KERN_DEBUG, "MSI: null device\n");
+ return -ENODEV;
+ }
+
/* MSI must be globally enabled and supported by the device */
- if (!pci_msi_enable || !dev || dev->no_msi)
+ if (!pci_msi_enable || dev->no_msi)
+ dev_printk(KERN_DEBUG, &dev->dev, "MSI: MSI disabled\n");
return -EINVAL;
+ }
/*
* You can't ask to have 0 or less MSIs configured.
* a) it's stupid ..
* b) the list manipulation code assumes nvec >= 1.
*/
- if (nvec < 1)
+ if (nvec < 1) {
+ dev_printk(KERN_DEBUG, &dev->dev, "MSI: < 1 MSI requested\n");
return -ERANGE;
+ }
/* Any bridge which does NOT route MSI transactions from it's
* secondary bus to it's primary bus must set NO_MSI flag on
@@ -535,16 +544,24 @@ static int pci_msi_check_device(struct pci_dev* dev, int nvec, int type)
* We expect only arch-specific PCI host bus controller driver
* or quirks for specific PCI bridges to be setting NO_MSI.
*/
- for (bus = dev->bus; bus; bus = bus->parent)
- if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+ for (bus = dev->bus; bus; bus = bus->parent) {
+ if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI) {
+ dev_printk(KERN_DEBUG, &dev->dev,
+ "MSI: bus check failed\n");
return -EINVAL;
+ }
+ }
ret = arch_msi_check_device(dev, nvec, type);
- if (ret)
+ if (ret) {
+ dev_printk(KERN_DEBUG, &dev->dev, "MSI: arch check failed\n");
return ret;
+ }
- if (!pci_find_capability(dev, type))
+ if (!pci_find_capability(dev, type)) {
+ dev_printk(KERN_DEBUG, &dev->dev, "MSI: no capability found\n");
return -EINVAL;
+ }
return 0;
}
@@ -669,26 +686,37 @@ int pci_enable_msix(struct pci_dev* dev, struct msix_entry *entries, int nvec)
int i, j;
u16 control;
- if (!entries)
- return -EINVAL;
-
status = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSIX);
if (status)
return status;
+ if (!entries) {
+ dev_printk(KERN_DEBUG, &dev->dev, "MSI: null entries\n");
+ return -EINVAL;
+ }
+
pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
pci_read_config_word(dev, msi_control_reg(pos), &control);
nr_entries = multi_msix_capable(control);
- if (nvec > nr_entries)
+ if (nvec > nr_entries) {
+ dev_printk(KERN_DEBUG, &dev->dev, "MSI: too many entries\n");
return -EINVAL;
+ }
/* Check for any invalid entries */
for (i = 0; i < nvec; i++) {
- if (entries[i].entry >= nr_entries)
+ if (entries[i].entry >= nr_entries) {
+ dev_printk(KERN_DEBUG, &dev->dev,
+ "MSI: invalid entry %d\n", i);
return -EINVAL; /* invalid entry */
+ }
for (j = i + 1; j < nvec; j++) {
- if (entries[i].entry == entries[j].entry)
+ if (entries[i].entry == entries[j].entry) {
+ dev_printk(KERN_DEBUG, &dev->dev,
+ "MSI: duplicate entries %d, %d\n",
+ i, j);
return -EINVAL; /* duplicate entry */
+ }
}
}
WARN_ON(!!dev->msix_enabled);
--
1.5.3.7.1.g4e596e
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
