From 0d07348931daef854aca8c834a89f1a99ba4ff2b Mon Sep 17 00:00:00 2001 From: Hidetoshi Seto Date: Wed, 24 Jun 2009 12:08:27 +0900 Subject: PCI MSI: Return if alloc_msi_entry for MSI-X failed In current code it continues setup even if alloc_msi_entry() for MSI-X is failed due to lack of memory. It means arch_setup_msi_irqs() might be called with msi_desc entries less than its argument nvec. At least x86's arch_setup_msi_irqs() uses list_for_each_entry() for dev->msi_list that suspected to have entries same numbers as nvec, and it doesn't check the number of allocated vectors and passed arg nvec. Therefore it will result in success of pci_enable_msix(), with less vectors allocated than requested. This patch fixes the error route to return -ENOMEM, instead of continuing the setup (proposed by Matthew Wilcox). Note that there is no iounmap in msi_free_irqs() if no msi_disc is allocated. Reviewed-by: Matthew Wilcox Signed-off-by: Hidetoshi Seto Signed-off-by: Jesse Barnes --- drivers/pci/msi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index d9f06fb..628c141 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -439,8 +439,14 @@ static int msix_capability_init(struct pci_dev *dev, for (i = 0; i < nvec; i++) { entry = alloc_msi_entry(dev); - if (!entry) - break; + if (!entry) { + if (!i) + iounmap(base); + else + msi_free_irqs(dev); + /* No enough memory. Don't try again */ + return -ENOMEM; + } j = entries[i].entry; entry->msi_attrib.is_msix = 1; -- cgit v1.1 From 50e5628a4ac465a52f0d4ca6567343be029731a0 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Sun, 28 Jun 2009 09:26:40 -0700 Subject: PCI ECRC: Remove unnecessary semicolons Acked-by: Andrew Patterson Signed-off-by: Joe Perches Signed-off-by: Jesse Barnes --- drivers/pci/pcie/aer/ecrc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pcie/aer/ecrc.c b/drivers/pci/pcie/aer/ecrc.c index ece97df..a928d8a 100644 --- a/drivers/pci/pcie/aer/ecrc.c +++ b/drivers/pci/pcie/aer/ecrc.c @@ -106,7 +106,7 @@ void pcie_set_ecrc_checking(struct pci_dev *dev) disable_ecrc_checking(dev); break; case ECRC_POLICY_ON: - enable_ecrc_checking(dev);; + enable_ecrc_checking(dev); break; default: return; -- cgit v1.1 From 654b75e044119bf8e7d773bce41ea039281bbfbe Mon Sep 17 00:00:00 2001 From: Yu Zhao Date: Fri, 26 Jun 2009 14:04:46 +0800 Subject: PCI: check if bus has a proper bridge device before triggering SBR For devices attached to the root bus, we can't trigger Secondary Bus Reset because there is no bridge device associated with the bus. So need to check bus->self again NULL first before using it. Reviewed-by: Kenji Kaneshige Signed-off-by: Yu Zhao Signed-off-by: Jesse Barnes --- drivers/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 6c93af5..d5d6f56 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2171,7 +2171,7 @@ static int pci_parent_bus_reset(struct pci_dev *dev, int probe) u16 ctrl; struct pci_dev *pdev; - if (dev->subordinate) + if (pci_is_root_bus(dev->bus) || dev->subordinate || !dev->bus->self) return -ENOTTY; list_for_each_entry(pdev, &dev->bus->devices, bus_list) -- cgit v1.1 From 503998ca4a295f7da233689850ba4b9d13cf41e7 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Wed, 24 Jun 2009 09:18:14 -0700 Subject: PCI: fix kernel-doc warnings Add documentation for missing parameters in PCI hotplug code. Signed-off-by: Randy Dunlap Signed-off-by: Jesse Barnes --- drivers/pci/hotplug/pci_hotplug_core.c | 2 ++ drivers/pci/slot.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c index 8445804..5c5043f 100644 --- a/drivers/pci/hotplug/pci_hotplug_core.c +++ b/drivers/pci/hotplug/pci_hotplug_core.c @@ -555,6 +555,8 @@ static struct hotplug_slot *get_slot_from_name (const char *name) * @slot: pointer to the &struct hotplug_slot to register * @devnr: device number * @name: name registered with kobject core + * @owner: caller module owner + * @mod_name: caller module name * * Registers a hotplug slot with the pci hotplug subsystem, which will allow * userspace interaction to the slot. diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c index eddb074..8c02b6c 100644 --- a/drivers/pci/slot.c +++ b/drivers/pci/slot.c @@ -311,7 +311,7 @@ EXPORT_SYMBOL_GPL(pci_destroy_slot); #include /** * pci_hp_create_link - create symbolic link to the hotplug driver module. - * @slot: struct pci_slot + * @pci_slot: struct pci_slot * * Helper function for pci_hotplug_core.c to create symbolic link to * the hotplug driver module. @@ -334,7 +334,7 @@ EXPORT_SYMBOL_GPL(pci_hp_create_module_link); /** * pci_hp_remove_link - remove symbolic link to the hotplug driver module. - * @slot: struct pci_slot + * @pci_slot: struct pci_slot * * Helper function for pci_hotplug_core.c to remove symbolic link to * the hotplug driver module. -- cgit v1.1 From 7a661c6f1082693a7e9627e9ad2d1546a9337fdc Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Wed, 24 Jun 2009 16:02:27 +0100 Subject: PCI: More PATA quirks for not entering D3 The ALi loses some state if it goes into D3. Unfortunately even with the chipset documents I can't figure out how to restore some bits of it. The VIA one saves/restores apparently fine but the ACPI _GTM methods break on some platforms if we do this and this causes cable misdetections. These are both effectively regressions as historically nothing matched the devices and then decided not to bind to them. Nowdays something is binding to all sorts of devices and a result they get dumped into D3. Signed-off-by: Alan Cox Acked-by: Jeff Garzik Signed-off-by: Jesse Barnes --- drivers/pci/quirks.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/pci') diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 56552d7..06b9656 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -1058,6 +1058,11 @@ static void __devinit quirk_no_ata_d3(struct pci_dev *pdev) } DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, PCI_ANY_ID, quirk_no_ata_d3); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, PCI_ANY_ID, quirk_no_ata_d3); +/* ALi loses some register settings that we cannot then restore */ +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AL, PCI_ANY_ID, quirk_no_ata_d3); +/* VIA comes back fine but we need to keep it alive or ACPI GTM failures + occur when mode detecting */ +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_VIA, PCI_ANY_ID, quirk_no_ata_d3); /* This was originally an Alpha specific thing, but it really fits here. * The i82375 PCI/EISA bridge appears as non-classified. Fix that. -- cgit v1.1 From 2c21fd4b333e4c780a46edcd6d1e85bfc6cdf371 Mon Sep 17 00:00:00 2001 From: Hidetoshi Seto Date: Tue, 23 Jun 2009 17:40:04 +0900 Subject: PCI MSI: shorten PCI_MSIX_ENTRY_* symbol names These names are too long! Drop _OFFSET to save some bytes/lines. Reviewed-by: Matthew Wilcox Signed-off-by: Hidetoshi Seto Signed-off-by: Jesse Barnes --- drivers/pci/msi.c | 18 ++++++++---------- drivers/pci/msi.h | 10 +++++----- 2 files changed, 13 insertions(+), 15 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 628c141..a088fc6 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -151,7 +151,7 @@ static void msix_mask_irq(struct msi_desc *desc, u32 flag) { u32 mask_bits = desc->masked; unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE + - PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET; + PCI_MSIX_ENTRY_VECTOR_CTRL; mask_bits &= ~1; mask_bits |= flag; writel(mask_bits, desc->mask_base + offset); @@ -188,9 +188,9 @@ void read_msi_msg_desc(struct irq_desc *desc, struct msi_msg *msg) void __iomem *base = entry->mask_base + entry->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE; - msg->address_lo = readl(base + PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET); - msg->address_hi = readl(base + PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET); - msg->data = readl(base + PCI_MSIX_ENTRY_DATA_OFFSET); + msg->address_lo = readl(base + PCI_MSIX_ENTRY_LOWER_ADDR); + msg->address_hi = readl(base + PCI_MSIX_ENTRY_UPPER_ADDR); + msg->data = readl(base + PCI_MSIX_ENTRY_DATA); } else { struct pci_dev *dev = entry->dev; int pos = entry->msi_attrib.pos; @@ -225,11 +225,9 @@ void write_msi_msg_desc(struct irq_desc *desc, struct msi_msg *msg) base = entry->mask_base + entry->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE; - writel(msg->address_lo, - base + PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET); - writel(msg->address_hi, - base + PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET); - writel(msg->data, base + PCI_MSIX_ENTRY_DATA_OFFSET); + writel(msg->address_lo, base + PCI_MSIX_ENTRY_LOWER_ADDR); + writel(msg->address_hi, base + PCI_MSIX_ENTRY_UPPER_ADDR); + writel(msg->data, base + PCI_MSIX_ENTRY_DATA); } else { struct pci_dev *dev = entry->dev; int pos = entry->msi_attrib.pos; @@ -493,7 +491,7 @@ static int msix_capability_init(struct pci_dev *dev, set_irq_msi(entry->irq, entry); j = entries[i].entry; entry->masked = readl(base + j * PCI_MSIX_ENTRY_SIZE + - PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET); + PCI_MSIX_ENTRY_VECTOR_CTRL); msix_mask_irq(entry, 1); i++; } diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h index a066284..de27c1c 100644 --- a/drivers/pci/msi.h +++ b/drivers/pci/msi.h @@ -6,11 +6,11 @@ #ifndef MSI_H #define MSI_H -#define PCI_MSIX_ENTRY_SIZE 16 -#define PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET 0 -#define PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET 4 -#define PCI_MSIX_ENTRY_DATA_OFFSET 8 -#define PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET 12 +#define PCI_MSIX_ENTRY_SIZE 16 +#define PCI_MSIX_ENTRY_LOWER_ADDR 0 +#define PCI_MSIX_ENTRY_UPPER_ADDR 4 +#define PCI_MSIX_ENTRY_DATA 8 +#define PCI_MSIX_ENTRY_VECTOR_CTRL 12 #define msi_control_reg(base) (base + PCI_MSI_FLAGS) #define msi_lower_address_reg(base) (base + PCI_MSI_ADDRESS_LO) -- cgit v1.1 From 7ba1930db02fc3118165338ef4e562869f575583 Mon Sep 17 00:00:00 2001 From: Hidetoshi Seto Date: Tue, 23 Jun 2009 17:39:27 +0900 Subject: PCI MSI: Unmask MSI if setup failed The initial state of mask register of MSI is unmasked. We set it masked before calling arch_setup_msi_irqs(). If arch_setup_msi_irq() fails, it is better to restore the state of the mask register. Reviewed-by: Matthew Wilcox Signed-off-by: Hidetoshi Seto Signed-off-by: Jesse Barnes --- drivers/pci/msi.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/pci') diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index a088fc6..9ab4fe8 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -383,6 +383,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec) /* Configure MSI capability structure */ ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSI); if (ret) { + msi_mask_irq(entry, mask, ~mask); msi_free_irqs(dev); return ret; } -- cgit v1.1 From 12abb8ba8444f7c9b355bbdd44a6d0839f7a41b6 Mon Sep 17 00:00:00 2001 From: Hidetoshi Seto Date: Wed, 24 Jun 2009 12:08:09 +0900 Subject: PCI MSI: Fix restoration of MSI/MSI-X mask states in suspend/resume There are 2 problems on mask states in suspend/resume. [1]: It is better to restore the mask states of MSI/MSI-X to initial states (MSI is unmasked, MSI-X is masked) when we release the device. The pci_msi_shutdown() does the restoration of mask states for MSI, while the msi_free_irqs() does it for MSI-X. In other words, in the "disable" path both of MSI and MSI-X are handled, but in the "shutdown" path only MSI is handled. MSI: pci_disable_msi() => pci_msi_shutdown() [ mask states for MSI restored ] => msi_set_enable(dev, pos, 0); => msi_free_irqs() MSI-X: pci_disable_msix() => pci_msix_shutdown() => msix_set_enable(dev, 0); => msix_free_all_irqs => msi_free_irqs() [ mask states for MSI-X restored ] This patch moves the masking for MSI-X from msi_free_irqs() to pci_msix_shutdown(). This change has some positive side effects: - It prevents OS from touching mask states before reading preserved bits in the register, which can be happen if msi_free_irqs() is called from error path in msix_capability_init(). - It also prevents touching the register after turning off MSI-X in "disable" path, which can be a problem on some devices. [2]: We have cache of the mask state in msi_desc, which is automatically updated when msi/msix_mask_irq() is called. This cached states are used for the resume. But since what need to be restored in the resume is the states before the shutdown on the suspend, calling msi/msix_mask_irq() from pci_msi/msix_shutdown() is not appropriate. This patch introduces __msi/msix_mask_irq() that do mask as same as msi/msix_mask_irq() but does not update cached state, for use in pci_msi/msix_shutdown(). [updated: get rid of msi/msix_mask_irq_nocache() (proposed by Matthew Wilcox)] Reviewed-by: Matthew Wilcox Signed-off-by: Hidetoshi Seto Signed-off-by: Jesse Barnes --- drivers/pci/msi.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 9ab4fe8..d986afb 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -127,17 +127,23 @@ static inline __attribute_const__ u32 msi_enabled_mask(u16 control) * reliably as devices without an INTx disable bit will then generate a * level IRQ which will never be cleared. */ -static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag) +static u32 __msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag) { u32 mask_bits = desc->masked; if (!desc->msi_attrib.maskbit) - return; + return 0; mask_bits &= ~mask; mask_bits |= flag; pci_write_config_dword(desc->dev, desc->mask_pos, mask_bits); - desc->masked = mask_bits; + + return mask_bits; +} + +static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag) +{ + desc->masked = __msi_mask_irq(desc, mask, flag); } /* @@ -147,7 +153,7 @@ static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag) * file. This saves a few milliseconds when initialising devices with lots * of MSI-X interrupts. */ -static void msix_mask_irq(struct msi_desc *desc, u32 flag) +static u32 __msix_mask_irq(struct msi_desc *desc, u32 flag) { u32 mask_bits = desc->masked; unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE + @@ -155,7 +161,13 @@ static void msix_mask_irq(struct msi_desc *desc, u32 flag) mask_bits &= ~1; mask_bits |= flag; writel(mask_bits, desc->mask_base + offset); - desc->masked = mask_bits; + + return mask_bits; +} + +static void msix_mask_irq(struct msi_desc *desc, u32 flag) +{ + desc->masked = __msix_mask_irq(desc, flag); } static void msi_set_mask_bit(unsigned irq, u32 flag) @@ -616,9 +628,11 @@ void pci_msi_shutdown(struct pci_dev *dev) pci_intx_for_msi(dev, 1); dev->msi_enabled = 0; + /* Return the device with MSI unmasked as initial states */ pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &ctrl); mask = msi_capable_mask(ctrl); - msi_mask_irq(desc, mask, ~mask); + /* Keep cached state to be restored */ + __msi_mask_irq(desc, mask, ~mask); /* Restore dev->irq to its default pin-assertion irq */ dev->irq = desc->msi_attrib.default_irq; @@ -658,7 +672,6 @@ static int msi_free_irqs(struct pci_dev* dev) list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) { if (entry->msi_attrib.is_msix) { - msix_mask_irq(entry, 1); if (list_is_last(&entry->list, &dev->msi_list)) iounmap(entry->mask_base); } @@ -746,9 +759,17 @@ static void msix_free_all_irqs(struct pci_dev *dev) void pci_msix_shutdown(struct pci_dev* dev) { + struct msi_desc *entry; + if (!pci_msi_enable || !dev || !dev->msix_enabled) return; + /* Return the device with MSI-X masked as initial states */ + list_for_each_entry(entry, &dev->msi_list, list) { + /* Keep cached states to be restored */ + __msix_mask_irq(entry, 1); + } + msix_set_enable(dev, 0); pci_intx_for_msi(dev, 1); dev->msix_enabled = 0; -- cgit v1.1 From 46b952a3c3a94afa339bd4961a4f3d1482436599 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 1 Jul 2009 14:24:30 -0700 Subject: PCI: Fix IRQ swizzling for ARI-enabled devices For many purposes, including interrupt-swizzling, devices with ARI enabled behave as if they have one device (number 0) and 256 functions. This probably hasn't bitten us in practice because all ARI devices I've seen are also IOV devices, and IOV devices are required to use MSI. This isn't guaranteed, and there are legitimate reasons to use ARI without IOV, and hence potentially use pin-based interrupts. Signed-off-by: Matthew Wilcox Signed-off-by: Jesse Barnes --- drivers/pci/pci.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d5d6f56..dbd0f94 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1517,11 +1517,20 @@ void pci_enable_ari(struct pci_dev *dev) * * Perform INTx swizzling for a device behind one level of bridge. This is * required by section 9.1 of the PCI-to-PCI bridge specification for devices - * behind bridges on add-in cards. + * behind bridges on add-in cards. For devices with ARI enabled, the slot + * number is always 0 (see the Implementation Note in section 2.2.8.1 of + * the PCI Express Base Specification, Revision 2.1) */ u8 pci_swizzle_interrupt_pin(struct pci_dev *dev, u8 pin) { - return (((pin - 1) + PCI_SLOT(dev->devfn)) % 4) + 1; + int slot; + + if (pci_ari_enabled(dev->bus)) + slot = 0; + else + slot = PCI_SLOT(dev->devfn); + + return (((pin - 1) + slot) % 4) + 1; } int -- cgit v1.1