[PATCH RFC V2] Factor out pci_fixup_irqs and give IRQs to hot-added devices

[PATCH RFC V2] Factor out pci_fixup_irqs and give IRQs to hot-added devices

[matthew_minter]

pci_fixup_irqs is the current method used in most arches to assign IRQs to
PCI devices. This has a number of flaws including it requiring an extra walk
of the PCI bus and most importantly not running for devices which are added
after boot time, this includes hot-added devices on boards that support this.

This patch set (which is a respin of my earlier patch set) modifies the
IRQ mapping and swizzling infrastructure such that functions are registered
by the boot code (instead of being run directly by the boot code)
and then run later in the enable_device path such that it will apply to all
devices and not only those inserted at boot time.

This is cleaner as it unifies how each architecture allocates PCI IRQs as much
as possible (some arches were particularly resistant to these changes so some
work-arounds have been used).

The caveat here is that I have been forced to modify some architecture specific
code for various architectures which I cannot test due to not having such boards
available. The code seems correct and the changes in most cases are small and
trivial however I need some help testing these patches.

I am also a little unsure if these should all be here or if some of them should
be sent to architecture specific lists, the reason I picked here as these
all need to be applied at once in order to safely work.

Any comments on this patch revision would be very appreciated, particularly if
anyone can catch bugs in any of the arches which are listed as (untested)
in the patch names.

Many thanks,
Matthew

arch/alpha/kernel/pci.c                 | 16 ++++++++++------
arch/alpha/kernel/sys_nautilus.c        |  1 -
arch/arm/kernel/bios32.c                | 13 +++++++++----
arch/cris/arch-v32/drivers/pci/bios.c   | 14 +++++---------
arch/frv/mb93090-mb00/pci-frv.h         |  1 -
arch/frv/mb93090-mb00/pci-irq.c         | 28 +++++++++++++++++-----------
arch/frv/mb93090-mb00/pci-vdk.c         |  1 -
arch/ia64/pci/pci.c                     |  3 +++
arch/m68k/platform/coldfire/pci.c       |  8 +++++++-
arch/microblaze/pci/pci-common.c        | 10 +++++++---
arch/mips/pci/pci.c                     |  9 +++++++--
arch/mn10300/unit-asb2305/pci-asb2305.h |  5 +----
arch/mn10300/unit-asb2305/pci-irq.c     | 25 +++++--------------------
arch/mn10300/unit-asb2305/pci.c         | 21 ++++++++-------------
arch/parisc/kernel/pci.c                |  8 +++++++-
arch/powerpc/kernel/pci-common.c        | 26 +++++++++++++-------------
arch/s390/pci/pci.c                     |  7 +++++++
arch/sh/drivers/pci/fixups-cayman.c     |  2 +-
arch/sh/drivers/pci/fixups-dreamcast.c  |  2 +-
arch/sh/drivers/pci/fixups-r7780rp.c    |  2 +-
arch/sh/drivers/pci/fixups-rts7751r2d.c |  6 +++---
arch/sh/drivers/pci/fixups-sdk7780.c    |  4 ++--
arch/sh/drivers/pci/fixups-se7751.c     |  2 +-
arch/sh/drivers/pci/fixups-sh03.c       |  2 +-
arch/sh/drivers/pci/fixups-snapgear.c   |  2 +-
arch/sh/drivers/pci/fixups-titan.c      |  4 ++--
arch/sh/drivers/pci/pci.c               | 10 +++++++---
arch/sh/drivers/pci/pcie-sh7786.c       |  2 +-
arch/sparc/kernel/leon_pci.c            | 12 +++++++++++-
arch/sparc/kernel/pci.c                 | 21 +++++++++++++++++----
arch/tile/kernel/pci.c                  | 10 +++++++---
arch/tile/kernel/pci_gx.c               | 10 +++++++---
arch/unicore32/kernel/pci.c             | 11 ++++++++---
arch/x86/include/asm/pci_x86.h          |  7 ++++---
arch/x86/kernel/x86_init.c              |  1 -
arch/x86/pci/acpi.c                     |  5 ++++-
arch/x86/pci/common.c                   |  2 --
arch/x86/pci/irq.c                      | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------
drivers/of/of_pci_irq.c                 |  2 +-
drivers/pci/Makefile                    | 15 ++-------------
drivers/pci/host-bridge.c               |  2 +-
drivers/pci/pci.c                       |  6 +++++-
drivers/pci/pci.h                       |  1 +
drivers/pci/probe.c                     | 12 ------------
drivers/pci/setup-irq.c                 | 34 ++++++++++++++++------------------
include/linux/pci.h                     |  8 ++++----
46 files changed, 266 insertions(+), 222 deletions(-)

[PATCH 01/22] Added some infrastructure to allow assigning PCI device IRQs at device enable time and removed pci_fixup_irqs as the other change obsolites it.

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 drivers/pci/Makefile      | 15 ++-------------
 drivers/pci/host-bridge.c |  2 +-
 drivers/pci/pci.c         |  6 +++++-
 drivers/pci/pci.h         |  1 +
 drivers/pci/probe.c       | 12 ------------
 drivers/pci/setup-irq.c   | 25 +++++++++----------------
 include/linux/pci.h       |  8 ++++----
 7 files changed, 22 insertions(+), 47 deletions(-)

diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index e04fe2d..38c4cb0 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -4,7 +4,8 @@
 
 obj-y		+= access.o bus.o probe.o host-bridge.o remove.o pci.o \
 			pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
-			irq.o vpd.o setup-bus.o vc.o
+			irq.o vpd.o setup-bus.o vc.o setup-irq.o
+
 obj-$(CONFIG_PROC_FS) += proc.o
 obj-$(CONFIG_SYSFS) += slot.o
 
@@ -31,18 +32,6 @@ obj-$(CONFIG_PCI_ATS) += ats.o
 obj-$(CONFIG_PCI_IOV) += iov.o
 
 #
-# Some architectures use the generic PCI setup functions
-#
-obj-$(CONFIG_ALPHA) += setup-irq.o
-obj-$(CONFIG_ARM) += setup-irq.o
-obj-$(CONFIG_UNICORE32) += setup-irq.o
-obj-$(CONFIG_SUPERH) += setup-irq.o
-obj-$(CONFIG_MIPS) += setup-irq.o
-obj-$(CONFIG_TILE) += setup-irq.o
-obj-$(CONFIG_SPARC_LEON) += setup-irq.o
-obj-$(CONFIG_M68K) += setup-irq.o
-
-#
 # ACPI Related PCI FW Functions
 # ACPI _DSM provided firmware instance and string name
 #
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index 0e5f3c9..8ed186f 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -16,7 +16,7 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
 	return bus;
 }
 
-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
 {
 	struct pci_bus *root_bus = find_pci_root_bus(bus);
 
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 63a54a3..d51d076 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1197,10 +1197,15 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
 	int err;
 	u16 cmd;
 	u8 pin;
+	struct pci_host_bridge *hbrg = find_pci_host_bridge(dev->bus);
 
 	err = pci_set_power_state(dev, PCI_D0);
 	if (err < 0 && err != -EIO)
 		return err;
+
+	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
+	pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
+
 	err = pcibios_enable_device(dev, bars);
 	if (err < 0)
 		return err;
@@ -1209,7 +1214,6 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
 	if (dev->msi_enabled || dev->msix_enabled)
 		return 0;
 
-	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
 	if (pin) {
 		pci_read_config_word(dev, PCI_COMMAND, &cmd);
 		if (cmd & PCI_COMMAND_INTX_DISABLE)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 0601890..a6cf445 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -70,6 +70,7 @@ void pci_config_pm_runtime_put(struct pci_dev *dev);
 void pci_pm_init(struct pci_dev *dev);
 void pci_allocate_cap_save_buffers(struct pci_dev *dev);
 void pci_free_cap_save_buffers(struct pci_dev *dev);
+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus);
 
 static inline void pci_wakeup_event(struct pci_dev *dev)
 {
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e3cf8a2..bb9f35c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1729,18 +1729,6 @@ unsigned int pci_scan_child_bus(struct pci_bus *bus)
 }
 EXPORT_SYMBOL_GPL(pci_scan_child_bus);
 
-/**
- * pcibios_root_bridge_prepare - Platform-specific host bridge setup.
- * @bridge: Host bridge to set up.
- *
- * Default empty implementation.  Replace with an architecture-specific setup
- * routine, if necessary.
- */
-int __weak pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
-{
-	return 0;
-}
-
 void __weak pcibios_add_bus(struct pci_bus *bus)
 {
 }
diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c
index 38c96c8..a994e07 100644
--- a/drivers/pci/setup-irq.c
+++ b/drivers/pci/setup-irq.c
@@ -22,11 +22,12 @@ void __weak pcibios_update_irq(struct pci_dev *dev, int irq)
 	pci_write_config_byte(dev, PCI_INTERRUPT_LINE, irq);
 }
 
-void pdev_fixup_irq(struct pci_dev *dev,
+void pdev_assign_irq(struct pci_dev *dev,
 			   u8 (*swizzle)(struct pci_dev *, u8 *),
-			   int (*map_irq)(const struct pci_dev *, u8, u8))
+			   int (*map_irq)(struct pci_dev *, u8, u8))
 {
-	u8 pin, slot;
+	u8 pin;
+	u8 slot = -1;
 	int irq = 0;
 
 	/* If this device is not on the primary bus, we need to figure out
@@ -41,27 +42,19 @@ void pdev_fixup_irq(struct pci_dev *dev,
 		pin = 1;
 
 	if (pin != 0) {
-		/* Follow the chain of bridges, swizzling as we go.  */
-		slot = (*swizzle)(dev, &pin);
+		/* Follow the chain of bridges, swizzling as we go. */
+		if(swizzle)
+			slot = (*swizzle)(dev, &pin);
 
+		/* If a swizzling function is not used map_irq must ignore slot */
 		irq = (*map_irq)(dev, slot, pin);
 		if (irq == -1)
 			irq = 0;
 	}
-	dev->irq = irq;
 
 	dev_dbg(&dev->dev, "fixup irq: got %d\n", dev->irq);
-
+	dev->irq = irq;
 	/* Always tell the device, so the driver knows what is
 	   the real IRQ to use; the device does not use it. */
 	pcibios_update_irq(dev, irq);
 }
-
-void pci_fixup_irqs(u8 (*swizzle)(struct pci_dev *, u8 *),
-		    int (*map_irq)(const struct pci_dev *, u8, u8))
-{
-	struct pci_dev *dev = NULL;
-
-	for_each_pci_dev(dev)
-		pdev_fixup_irq(dev, swizzle, map_irq);
-}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4c1b1b3..f8b9b95 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -402,6 +402,8 @@ struct pci_host_bridge {
 	struct device dev;
 	struct pci_bus *bus;		/* root bus */
 	struct list_head windows;	/* pci_host_bridge_windows */
+	u8 (*swizzle_irq)(struct pci_dev *, u8 *); /* platform irq swizzler */
+	int (*map_irq)(struct pci_dev *, u8, u8);
 	void (*release_fn)(struct pci_host_bridge *);
 	void *release_data;
 };
@@ -1056,10 +1058,8 @@ void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
 void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus);
 void pdev_enable_device(struct pci_dev *);
 int pci_enable_resources(struct pci_dev *, int mask);
-void pdev_fixup_irq(struct pci_dev *, u8 (*)(struct pci_dev *, u8 *),
-		    int (*)(const struct pci_dev *, u8, u8));
-void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
-		    int (*)(const struct pci_dev *, u8, u8));
+void pdev_assign_irq(struct pci_dev *dev, u8 (*swizzle)(struct pci_dev *, u8 *),
+		     int (*map_irq)(struct pci_dev *, u8, u8));
 #define HAVE_PCI_REQ_REGIONS	2
 int __must_check pci_request_regions(struct pci_dev *, const char *);
 int __must_check pci_request_regions_exclusive(struct pci_dev *, const char *);
-- 
2.0.4

[PATCH 02/22] Modified x86 initialisation to accomodate PCI irq changes

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/x86/include/asm/pci_x86.h |  5 ++-
 arch/x86/kernel/x86_init.c     |  1 -
 arch/x86/pci/acpi.c            |  5 ++-
 arch/x86/pci/common.c          |  2 -
 arch/x86/pci/irq.c             | 95 +++++++++++++++++++++++-------------------
 5 files changed, 58 insertions(+), 50 deletions(-)

diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index fa1195d..436b9e5 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -90,6 +90,7 @@ extern unsigned int pcibios_irq_mask;
 
 extern raw_spinlock_t pci_config_lock;
 
+extern int pci_map_irq(struct pci_dev *dev, u8 slot, u8 pin);
 extern int (*pcibios_enable_irq)(struct pci_dev *dev);
 extern void (*pcibios_disable_irq)(struct pci_dev *dev);
 
@@ -119,7 +120,7 @@ extern int __init pci_acpi_init(void);
 extern void __init pcibios_irq_init(void);
 extern int __init pcibios_init(void);
 extern int pci_legacy_init(void);
-extern void pcibios_fixup_irqs(void);
+extern int pcibios_fixup_irq(struct pci_dev *dev, u8 pin);
 
 /* pci-mmconfig.c */
 
@@ -200,7 +201,7 @@ static inline void mmio_config_writel(void __iomem *pos, u32 val)
 #  define x86_default_pci_init		pci_legacy_init
 # endif
 # define x86_default_pci_init_irq	pcibios_irq_init
-# define x86_default_pci_fixup_irqs	pcibios_fixup_irqs
+# define x86_default_pci_fixup_irq	pcibios_fixup_irq
 #else
 # define x86_default_pci_init		NULL
 # define x86_default_pci_init_irq	NULL
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index e48b674..064457f 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -81,7 +81,6 @@ struct x86_init_ops x86_init __initdata = {
 	.pci = {
 		.init			= x86_default_pci_init,
 		.init_irq		= x86_default_pci_init_irq,
-		.fixup_irqs		= x86_default_pci_fixup_irqs,
 	},
 };
 
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 5075371..4ffad2f 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -311,7 +311,7 @@ static acpi_status setup_resource(struct acpi_resource *acpi_res, void *data)
 	} else if (orig_end != end) {
 		dev_info(&info->bridge->dev,
 			"host bridge window [%#llx-%#llx] "
-			"([%#llx-%#llx] ignored, not CPU addressable)\n", 
+			"([%#llx-%#llx] ignored, not CPU addressable)\n",
 			start, orig_end, end + 1, orig_end);
 	}
 
@@ -571,6 +571,9 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 	struct pci_sysdata *sd = bridge->bus->sysdata;
 
 	ACPI_COMPANION_SET(&bridge->dev, sd->companion);
+
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = pci_map_irq;
 	return 0;
 }
 
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 059a76c..d4ed0b0 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -662,8 +662,6 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
 	if ((err = pci_enable_resources(dev, mask)) < 0)
 		return err;
 
-	if (!pci_dev_msi_enabled(dev))
-		return pcibios_enable_irq(dev);
 	return 0;
 }
 
diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index 84112f5..0e2cfe8 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -593,9 +593,9 @@ static __init int intel_router_probe(struct irq_router *r, struct pci_dev *route
 		return 1;
 	}
 
-	if ((device >= PCI_DEVICE_ID_INTEL_5_3400_SERIES_LPC_MIN && 
-	     device <= PCI_DEVICE_ID_INTEL_5_3400_SERIES_LPC_MAX) 
-	||  (device >= PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MIN && 
+	if ((device >= PCI_DEVICE_ID_INTEL_5_3400_SERIES_LPC_MIN &&
+	     device <= PCI_DEVICE_ID_INTEL_5_3400_SERIES_LPC_MAX)
+	||  (device >= PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MIN &&
 	     device <= PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MAX)
 	||  (device >= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MIN &&
 	     device <= PCI_DEVICE_ID_INTEL_DH89XXCC_LPC_MAX)
@@ -874,9 +874,8 @@ static struct irq_info *pirq_get_info(struct pci_dev *dev)
 	return NULL;
 }
 
-static int pcibios_lookup_irq(struct pci_dev *dev, int assign)
+static int pcibios_lookup_irq(struct pci_dev *dev, u8 pin, int assign)
 {
-	u8 pin;
 	struct irq_info *info;
 	int i, pirq, newirq;
 	int irq = 0;
@@ -886,7 +885,6 @@ static int pcibios_lookup_irq(struct pci_dev *dev, int assign)
 	char *msg = NULL;
 
 	/* Find IRQ pin */
-	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
 	if (!pin) {
 		dev_dbg(&dev->dev, "no interrupt pin\n");
 		return 0;
@@ -1016,50 +1014,45 @@ static int pcibios_lookup_irq(struct pci_dev *dev, int assign)
 					 irq, pci_name(dev2));
 		}
 	}
-	return 1;
+	/*
+	 * Due to the complicated platform specific behvaiour we cannot defer
+	 * assigning dev->irq to the caller but will return it anyway
+	 */
+	return dev->irq;
 }
 
-void __init pcibios_fixup_irqs(void)
+int pcibios_fixup_irq(struct pci_dev *dev, u8 pin)
 {
-	struct pci_dev *dev = NULL;
-	u8 pin;
-
+	int irq = dev->irq;
 	DBG(KERN_DEBUG "PCI: IRQ fixup\n");
-	for_each_pci_dev(dev) {
-		/*
-		 * If the BIOS has set an out of range IRQ number, just
-		 * ignore it.  Also keep track of which IRQ's are
-		 * already in use.
-		 */
-		if (dev->irq >= 16) {
-			dev_dbg(&dev->dev, "ignoring bogus IRQ %d\n", dev->irq);
-			dev->irq = 0;
-		}
-		/*
-		 * If the IRQ is already assigned to a PCI device,
-		 * ignore its ISA use penalty
-		 */
-		if (pirq_penalty[dev->irq] >= 100 &&
-				pirq_penalty[dev->irq] < 100000)
-			pirq_penalty[dev->irq] = 0;
-		pirq_penalty[dev->irq]++;
+	/*
+	 * If the BIOS has set an out of range IRQ number, just
+	 * ignore it.  Also keep track of which IRQ's are
+	 * already in use.
+	 */
+	if (irq >= 16) {
+		dev_dbg(&dev->dev, "ignoring bogus IRQ %d\n", irq);
+		irq = 0;
 	}
+	/*
+	 * If the IRQ is already assigned to a PCI device,
+	 * ignore its ISA use penalty
+	 */
+	if (pirq_penalty[irq] >= 100 &&
+			pirq_penalty[irq] < 100000)
+		pirq_penalty[irq] = 0;
+	pirq_penalty[irq]++;
 
-	if (io_apic_assign_pci_irqs)
-		return;
+	if (io_apic_assign_pci_irqs || !pin)
+		return irq;
 
-	dev = NULL;
-	for_each_pci_dev(dev) {
-		pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
-		if (!pin)
-			continue;
+	/*
+	 * Still no IRQ? Try to lookup one...
+	 */
+	if (!irq)
+		irq = pcibios_lookup_irq(dev, pin, 0);
 
-		/*
-		 * Still no IRQ? Try to lookup one...
-		 */
-		if (!dev->irq)
-			pcibios_lookup_irq(dev, 0);
-	}
+	return irq;
 }
 
 /*
@@ -1160,6 +1153,13 @@ void __init pcibios_irq_init(void)
 	}
 }
 
+int __weak pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = pci_map_irq;
+	return 0;
+}
+
 static void pirq_penalize_isa_irq(int irq, int active)
 {
 	/*
@@ -1184,12 +1184,19 @@ void pcibios_penalize_isa_irq(int irq, int active)
 		pirq_penalize_isa_irq(irq, active);
 }
 
+int pci_map_irq(struct pci_dev *dev, u8 slot, u8 pin)
+{
+	dev->irq = pcibios_fixup_irq(dev, pin);
+	if (pcibios_enable_irq(dev))
+		return -1;
+	return dev->irq;
+}
+
 static int pirq_enable_irq(struct pci_dev *dev)
 {
 	u8 pin;
-
 	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
-	if (pin && !pcibios_lookup_irq(dev, 1)) {
+	if (pin && !pcibios_lookup_irq(dev, pin, 1)) {
 		char *msg = "";
 
 		if (!io_apic_assign_pci_irqs && dev->irq)
-- 
2.0.4

[PATCH 03/22] Added (untested) support to powerpc initialisation to fix pci irq changes

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/powerpc/kernel/pci-common.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index b49c72f..ed4bd53 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -226,7 +226,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node)
  * If the interrupt is used, then gets the interrupt line from the
  * openfirmware and sets it in the pci_dev and pci_config line.
  */
-static int pci_read_irq_line(struct pci_dev *pci_dev)
+static int pci_read_irq_line(struct pci_dev *pci_dev, u8 pin)
 {
 	struct of_phandle_args oirq;
 	unsigned int virq;
@@ -238,7 +238,7 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
 #endif
 	/* Try to get a mapping from the device-tree */
 	if (of_irq_parse_pci(pci_dev, &oirq)) {
-		u8 line, pin;
+		u8 line;
 
 		/* If that fails, lets fallback to what is in the config
 		 * space and map that through the default controller. We
@@ -247,10 +247,6 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
 		 * either provide a proper interrupt tree or don't use this
 		 * function.
 		 */
-		if (pci_read_config_byte(pci_dev, PCI_INTERRUPT_PIN, &pin))
-			return -1;
-		if (pin == 0)
-			return -1;
 		if (pci_read_config_byte(pci_dev, PCI_INTERRUPT_LINE, &line) ||
 		    line == 0xff || line == 0) {
 			return -1;
@@ -275,9 +271,16 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
 
 	pr_debug(" Mapped to linux irq %d\n", virq);
 
-	pci_dev->irq = virq;
+	return virq;
+}
 
-	return 0;
+int pci_map_irq(struct pci_dev *dev, u8 slot, u8 pin)
+{
+	/* Read default IRQs and fixup if necessary */
+	int irq = pci_read_irq_line(dev, pin);
+	if (ppc_md.pci_irq_fixup)
+		ppc_md.pci_irq_fixup(dev);
+	return irq;
 }
 
 /*
@@ -775,6 +778,8 @@ int pci_proc_domain(struct pci_bus *bus)
 
 int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 {
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = pci_map_irq;
 	if (ppc_md.pcibios_root_bridge_prepare)
 		return ppc_md.pcibios_root_bridge_prepare(bridge);
 
@@ -977,11 +982,6 @@ static void pcibios_setup_device(struct pci_dev *dev)
 	/* Additional platform DMA/iommu setup */
 	if (ppc_md.pci_dma_dev_setup)
 		ppc_md.pci_dma_dev_setup(dev);
-
-	/* Read default IRQs and fixup if necessary */
-	pci_read_irq_line(dev);
-	if (ppc_md.pci_irq_fixup)
-		ppc_md.pci_irq_fixup(dev);
 }
 
 int pcibios_add_device(struct pci_dev *dev)
-- 
2.0.4

[PATCH 04/22] Added support to ARM architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/arm/kernel/bios32.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 17a26c1..4dcf3f7 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -389,7 +389,7 @@ void pcibios_remove_bus(struct pci_bus *bus)
  * PCI standard swizzle is implemented on plug-in cards and Cardbus based
  * PCI extenders, so it can not be ignored.
  */
-static u8 pcibios_swizzle(struct pci_dev *dev, u8 *pin)
+u8 pcibios_swizzle(struct pci_dev *dev, u8 *pin)
 {
 	struct pci_sys_data *sys = dev->sysdata;
 	int slot, oldpin = *pin;
@@ -409,7 +409,7 @@ static u8 pcibios_swizzle(struct pci_dev *dev, u8 *pin)
 /*
  * Map a slot/pin to an IRQ.
  */
-static int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+int pcibios_map_irq(struct pci_dev *dev, u8 slot, u8 pin)
 {
 	struct pci_sys_data *sys = dev->sysdata;
 	int irq = -1;
@@ -424,6 +424,13 @@ static int pcibios_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 	return irq;
 }
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = pcibios_swizzle;
+	bridge->map_irq = pcibios_map_irq;
+	return 0;
+}
+
 static int pcibios_init_resources(int busnr, struct pci_sys_data *sys)
 {
 	int ret;
@@ -523,8 +530,6 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw)
 	if (hw->postinit)
 		hw->postinit();
 
-	pci_fixup_irqs(pcibios_swizzle, pcibios_map_irq);
-
 	list_for_each_entry(sys, &head, node) {
 		struct pci_bus *bus = sys->bus;
 
-- 
2.0.4

[PATCH 05/22] Added (untested) support to frv architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/frv/mb93090-mb00/pci-frv.h |  1 -
 arch/frv/mb93090-mb00/pci-irq.c | 28 +++++++++++++++++-----------
 arch/frv/mb93090-mb00/pci-vdk.c |  1 -
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/frv/mb93090-mb00/pci-frv.h b/arch/frv/mb93090-mb00/pci-frv.h
index a7e487fe..492b56d 100644
--- a/arch/frv/mb93090-mb00/pci-frv.h
+++ b/arch/frv/mb93090-mb00/pci-frv.h
@@ -36,5 +36,4 @@ extern struct pci_ops *__nongpreldata pci_root_ops;
 extern unsigned int pcibios_irq_mask;
 
 void pcibios_irq_init(void);
-void pcibios_fixup_irqs(void);
 void pcibios_enable_irq(struct pci_dev *dev);
diff --git a/arch/frv/mb93090-mb00/pci-irq.c b/arch/frv/mb93090-mb00/pci-irq.c
index 1c35c93..3463e30 100644
--- a/arch/frv/mb93090-mb00/pci-irq.c
+++ b/arch/frv/mb93090-mb00/pci-irq.c
@@ -40,19 +40,25 @@ void __init pcibios_irq_init(void)
 {
 }
 
-void __init pcibios_fixup_irqs(void)
+int pcibios_map_irq(struct pci_dev *dev, uint8_t slot, uint8_t pin)
 {
-	struct pci_dev *dev = NULL;
-	uint8_t line, pin;
-
-	for_each_pci_dev(dev) {
-		pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
-		if (pin) {
-			dev->irq = pci_bus0_irq_routing[PCI_SLOT(dev->devfn)][pin - 1];
-			pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq);
-		}
-		pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &line);
+	uint8_t line;
+	int irq;
+
+	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
+	if (pin) {
+		irq = pci_bus0_irq_routing[PCI_SLOT(dev->devfn)][pin - 1];
+		pci_write_config_byte(dev, PCI_INTERRUPT_LINE, irq);
 	}
+	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &line);
+	return irq;
+}
+
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = pcibios_map_irq;
+	return 0;
 }
 
 void pcibios_enable_irq(struct pci_dev *dev)
diff --git a/arch/frv/mb93090-mb00/pci-vdk.c b/arch/frv/mb93090-mb00/pci-vdk.c
index efa5d65..86657a7 100644
--- a/arch/frv/mb93090-mb00/pci-vdk.c
+++ b/arch/frv/mb93090-mb00/pci-vdk.c
@@ -386,7 +386,6 @@ int __init pcibios_init(void)
 	pci_scan_root_bus(NULL, 0, pci_root_ops, NULL, &resources);
 
 	pcibios_irq_init();
-	pcibios_fixup_irqs();
 	pcibios_resource_survey();
 
 	return 0;
-- 
2.0.4

[PATCH 06/22] Added (untested) support to mips architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/mips/pci/pci.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 1bf60b1..dc660a0 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -242,13 +242,18 @@ static int __init pcibios_init(void)
 	for (hose = hose_head; hose; hose = hose->next)
 		pcibios_scanbus(hose);
 
-	pci_fixup_irqs(pci_common_swizzle, pcibios_map_irq);
-
 	pci_initialized = 1;
 
 	return 0;
 }
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+       bridge->swizzle_irq = pci_common_swizzle;
+       bridge->map_irq = pcibios_map_irq;
+       return 0;
+}
+
 subsys_initcall(pcibios_init);
 
 static int pcibios_enable_resources(struct pci_dev *dev, int mask)
-- 
2.0.4

[PATCH 07/22] Fixed PCI initilisation code for arches which dont need to map irqs

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 drivers/pci/setup-irq.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c
index a994e07..7bb4356 100644
--- a/drivers/pci/setup-irq.c
+++ b/drivers/pci/setup-irq.c
@@ -30,6 +30,11 @@ void pdev_assign_irq(struct pci_dev *dev,
 	u8 slot = -1;
 	int irq = 0;
 
+	if (!map_irq) {
+		dev_dbg(&dev->dev, "map_irq not provided by arch\n", dev->irq);
+		return;
+	}
+
 	/* If this device is not on the primary bus, we need to figure out
 	   which interrupt pin it will come in on.   We know which slot it
 	   will come in on 'cos that slot is where the bridge is.   Each
-- 
2.0.4

[PATCH 08/22] Added (untested) workaround to ia64 architecture to accomodate new irq init code. Unfortunately adding runtime irq assignment will not work but avoided breaking the existing code.

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/ia64/pci/pci.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 291a582..398d5d4 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -483,6 +483,9 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 {
 	struct pci_controller *controller = bridge->bus->sysdata;
 
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = NULL;
+
 	ACPI_COMPANION_SET(&bridge->dev, controller->companion);
 	return 0;
 }
-- 
2.0.4

[PATCH 09/22] Added (untested) support to microblaze architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/microblaze/pci/pci-common.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 9037914..6843b34 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -850,12 +850,16 @@ void pcibios_setup_bus_devices(struct pci_bus *bus)
 		 * code and is needed by the DMA init
 		 */
 		set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
-
-		/* Read default IRQs and fixup if necessary */
-		dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
 	}
 }
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = of_irq_parse_and_map_pci;
+	return 0;
+}
+
 void pcibios_fixup_bus(struct pci_bus *bus)
 {
 	/* When called from the generic PCI probe, read PCI<->PCI bridge
-- 
2.0.4

[PATCH 10/22] Added (untested) support to cris architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/cris/arch-v32/drivers/pci/bios.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/arch/cris/arch-v32/drivers/pci/bios.c b/arch/cris/arch-v32/drivers/pci/bios.c
index 64a5fb9..d04ecc8 100644
--- a/arch/cris/arch-v32/drivers/pci/bios.c
+++ b/arch/cris/arch-v32/drivers/pci/bios.c
@@ -80,20 +80,16 @@ int pcibios_enable_resources(struct pci_dev *dev, int mask)
 	return 0;
 }
 
-int pcibios_enable_irq(struct pci_dev *dev)
+int pcibios_enable_irq(struct pci_dev *dev, u8 slot, u8 pin)
 {
 	dev->irq = EXT_INTR_VECT;
 	return 0;
 }
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 {
-	int err;
-
-	if ((err = pcibios_enable_resources(dev, mask)) < 0)
-		return err;
-
-	if (!dev->msi_enabled)
-		pcibios_enable_irq(dev);
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = pcibios_enable_irq;
 	return 0;
 }
+
-- 
2.0.4

[PATCH 11/22] Added (untested) support to mn10300 architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/mn10300/unit-asb2305/pci-asb2305.h |  5 +----
 arch/mn10300/unit-asb2305/pci-irq.c     | 25 +++++--------------------
 arch/mn10300/unit-asb2305/pci.c         | 21 ++++++++-------------
 3 files changed, 14 insertions(+), 37 deletions(-)

diff --git a/arch/mn10300/unit-asb2305/pci-asb2305.h b/arch/mn10300/unit-asb2305/pci-asb2305.h
index 9e17aca..fb520cf 100644
--- a/arch/mn10300/unit-asb2305/pci-asb2305.h
+++ b/arch/mn10300/unit-asb2305/pci-asb2305.h
@@ -67,9 +67,6 @@ struct irq_routing_table {
 } __attribute__((packed));
 
 extern unsigned int pcibios_irq_mask;
-
-extern void pcibios_irq_init(void);
-extern void pcibios_fixup_irqs(void);
-extern void pcibios_enable_irq(struct pci_dev *dev);
+extern int pci_map_irq(struct pci_dev *, u8 slot, u8 pin);
 
 #endif /* PCI_ASB2305_H */
diff --git a/arch/mn10300/unit-asb2305/pci-irq.c b/arch/mn10300/unit-asb2305/pci-irq.c
index fcb28ce..5be6fea 100644
--- a/arch/mn10300/unit-asb2305/pci-irq.c
+++ b/arch/mn10300/unit-asb2305/pci-irq.c
@@ -20,27 +20,12 @@
 #include <asm/smp.h>
 #include "pci-asb2305.h"
 
-void __init pcibios_irq_init(void)
+int pci_map_irq(struct pci_dev *, u8 slot, u8 pin)
 {
-}
-
-void __init pcibios_fixup_irqs(void)
-{
-	struct pci_dev *dev = NULL;
-	u8 line, pin;
+	u8 line;
 
-	for_each_pci_dev(dev) {
-		pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
-		if (pin) {
-			dev->irq = XIRQ1;
-			pci_write_config_byte(dev, PCI_INTERRUPT_LINE,
-					      dev->irq);
-		}
-		pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &line);
-	}
-}
-
-void pcibios_enable_irq(struct pci_dev *dev)
-{
+	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
+	dev->irq = XIRQ1;
 	pci_write_config_byte(dev, PCI_INTERRUPT_LINE, dev->irq);
+	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &line);
 }
diff --git a/arch/mn10300/unit-asb2305/pci.c b/arch/mn10300/unit-asb2305/pci.c
index 6b4339f..22f8786 100644
--- a/arch/mn10300/unit-asb2305/pci.c
+++ b/arch/mn10300/unit-asb2305/pci.c
@@ -377,13 +377,18 @@ static int __init pcibios_init(void)
 	pci_add_resource_offset(&resources, &pci_ioport_resource, io_offset);
 	pci_add_resource_offset(&resources, &pci_iomem_resource, mem_offset);
 	pci_scan_root_bus(NULL, 0, &pci_direct_ampci, NULL, &resources);
-
-	pcibios_irq_init();
-	pcibios_fixup_irqs();
 	pcibios_resource_survey();
 	return 0;
 }
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = pci_map_irq;
+	return 0;
+}
+
+
 arch_initcall(pcibios_init);
 
 char *__init pcibios_setup(char *str)
@@ -396,16 +401,6 @@ char *__init pcibios_setup(char *str)
 	return str;
 }
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	int err;
-
-	err = pci_enable_resources(dev, mask);
-	if (err == 0)
-		pcibios_enable_irq(dev);
-	return err;
-}
-
 /*
  * disable the ethernet chipset
  */
-- 
2.0.4

[PATCH 12/22] Added (untested) workaround to parisc architecture to accomodate new irq init code. Unfortunately adding runtime irq assignment will not work but avoided breaking the existing code.

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/parisc/kernel/pci.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/parisc/kernel/pci.c b/arch/parisc/kernel/pci.c
index 64f2764..9654c69 100644
--- a/arch/parisc/kernel/pci.c
+++ b/arch/parisc/kernel/pci.c
@@ -105,7 +105,13 @@ PCI_PORT_OUT(b,  8)
 PCI_PORT_OUT(w, 16)
 PCI_PORT_OUT(l, 32)
 
-
+/* We do not support hot-add irq assignment */
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = NULL;
+	return 0;
+}
 
 /*
  * BIOS32 replacement.
-- 
2.0.4

[PATCH 13/22] Added (untested) support to sparc architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/sparc/kernel/leon_pci.c | 12 +++++++++++-
 arch/sparc/kernel/pci.c      | 21 +++++++++++++++++----
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 899b720..1079eb7 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -13,6 +13,8 @@
 #include <asm/leon.h>
 #include <asm/leon_pci.h>
 
+int (*leon_pci_map_irq)(const struct pci_dev *dev, u8 slot, u8 pin);
+
 /* The LEON architecture does not rely on a BIOS or bootloader to setup
  * PCI for us. The Linux generic routines are used to setup resources,
  * reset values of configuration-space register settings are preserved.
@@ -36,7 +38,7 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
 				     &resources);
 	if (root_bus) {
 		/* Setup IRQs of all devices using custom routines */
-		pci_fixup_irqs(pci_common_swizzle, info->map_irq);
+		leon_pci_map_irq = info->map_irq;
 
 		/* Assign devices with resources */
 		pci_assign_unassigned_resources();
@@ -45,6 +47,14 @@ void leon_pci_init(struct platform_device *ofdev, struct leon_pci_info *info)
 	}
 }
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = pci_common_swizzle;
+	bridge->map_irq = leon_pci_map_irq;
+	return 0;
+}
+
+
 void pcibios_fixup_bus(struct pci_bus *pbus)
 {
 	struct pci_dev *dev;
diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 539babf..af2826c 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -340,10 +340,6 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 	} else {
 		dev->hdr_type = PCI_HEADER_TYPE_NORMAL;
 		dev->rom_base_reg = PCI_ROM_ADDRESS;
-
-		dev->irq = sd->op->archdata.irqs[0];
-		if (dev->irq == 0xffffffff)
-			dev->irq = PCI_IRQ_NONE;
 	}
 
 	pci_parse_of_addrs(sd->op, node, dev);
@@ -356,6 +352,23 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm,
 	return dev;
 }
 
+int pci_map_irq(struct pci_dev *dev, u8 slot, u8 pin)
+{
+	int irq;
+	struct platform_device *op = of_find_device_by_node(dev->sysdata);
+	irq = op->archdata.irqs[0];
+	if (irq == 0xffffffff)
+		irq = PCI_IRQ_NONE;
+	return irq;
+}
+
+int __weak pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = pci_map_irq;
+	return 0;
+}
+
 static void apb_calc_first_last(u8 map, u32 *first_p, u32 *last_p)
 {
 	u32 idx, first, last;
-- 
2.0.4

[PATCH 14/22] Added (untested) support to alpha architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/alpha/kernel/pci.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 076c35c..338537c 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -28,7 +28,7 @@
 
 
 /*
- * Some string constants used by the various core logics. 
+ * Some string constants used by the various core logics.
  */
 
 const char *const pci_io_names[] = {
@@ -247,7 +247,7 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 	if (pci_has_flag(PCI_PROBE_ONLY) && dev &&
  		   (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
  		pci_read_bridge_bases(bus);
-	} 
+	}
 
 	list_for_each_entry(dev, &bus->devices, bus_list) {
 		pdev_save_srm_config(dev);
@@ -338,7 +338,7 @@ common_init_pci(void)
 		hose->need_domain_info = need_domain_info;
 		next_busno = bus->busn_res.end + 1;
 		/* Don't allow 8-bit bus number overflow inside the hose -
-		   reserve some space for bridges. */ 
+		   reserve some space for bridges. */
 		if (next_busno > 224) {
 			next_busno = 0;
 			need_domain_info = 1;
@@ -346,11 +346,15 @@ common_init_pci(void)
 	}
 
 	pcibios_claim_console_setup();
-
 	pci_assign_unassigned_resources();
-	pci_fixup_irqs(alpha_mv.pci_swizzle, alpha_mv.pci_map_irq);
 }
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = alpha_mv.pci_swizzle;
+	bridge->map_irq = alpha_mv.pci_map_irq;
+	return 0;
+}
 
 struct pci_controller * __init
 alloc_pci_controller(void)
@@ -387,7 +391,7 @@ sys_pciconfig_iobase(long which, unsigned long bus, unsigned long dfn)
 
 	/* from hose or from bus.devfn */
 	if (which & IOBASE_FROM_HOSE) {
-		for(hose = hose_head; hose; hose = hose->next) 
+		for(hose = hose_head; hose; hose = hose->next)
 			if (hose->index == bus) break;
 		if (!hose) return -ENODEV;
 	} else {
-- 
2.0.4

[PATCH 15/22] Added (untested) support to m68k architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/m68k/platform/coldfire/pci.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/m68k/platform/coldfire/pci.c b/arch/m68k/platform/coldfire/pci.c
index df96792..058ca86 100644
--- a/arch/m68k/platform/coldfire/pci.c
+++ b/arch/m68k/platform/coldfire/pci.c
@@ -316,10 +316,16 @@ static int __init mcf_pci_init(void)
 	rootbus->resource[0] = &mcf_pci_io;
 	rootbus->resource[1] = &mcf_pci_mem;
 
-	pci_fixup_irqs(pci_common_swizzle, mcf_pci_map_irq);
 	pci_bus_size_bridges(rootbus);
 	pci_bus_assign_resources(rootbus);
 	return 0;
 }
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = pci_common_swizzle;
+	bridge->map_irq = mcf_pci_map_irq;
+	return 0;
+}
+
 subsys_initcall(mcf_pci_init);
-- 
2.0.4

[PATCH 16/22] Added (untested) support to tile architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/tile/kernel/pci.c    | 10 +++++++---
 arch/tile/kernel/pci_gx.c | 10 +++++++---
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/tile/kernel/pci.c b/arch/tile/kernel/pci.c
index 1f80a88..6dc0bd8 100644
--- a/arch/tile/kernel/pci.c
+++ b/arch/tile/kernel/pci.c
@@ -313,9 +313,6 @@ int __init pcibios_init(void)
 		}
 	}
 
-	/* Do machine dependent PCI interrupt routing */
-	pci_fixup_irqs(pci_common_swizzle, tile_map_irq);
-
 	/*
 	 * This comes from the generic Linux PCI driver.
 	 *
@@ -367,6 +364,13 @@ int __init pcibios_init(void)
 }
 subsys_initcall(pcibios_init);
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = pci_common_swizzle;
+	bridge->map_irq = tile_map_irq;
+	return 0;
+}
+
 /*
  * No bus fixups needed.
  */
diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
index e39f9c5..7d1b5bf 100644
--- a/arch/tile/kernel/pci_gx.c
+++ b/arch/tile/kernel/pci_gx.c
@@ -893,9 +893,6 @@ int __init pcibios_init(void)
 		next_busno = bus->busn_res.end + 1;
 	}
 
-	/* Do machine dependent PCI interrupt routing */
-	pci_fixup_irqs(pci_common_swizzle, tile_map_irq);
-
 	/*
 	 * This comes from the generic Linux PCI driver.
 	 *
@@ -1048,6 +1045,13 @@ alloc_mem_map_failed:
 }
 subsys_initcall(pcibios_init);
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = pci_common_swizzle;
+	bridge->map_irq = tile_map_irq;
+	return 0;
+}
+
 /* No bus fixups needed. */
 void pcibios_fixup_bus(struct pci_bus *bus)
 {
-- 
2.0.4

[PATCH 17/22] Added (untested) support to sh architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/sh/drivers/pci/fixups-cayman.c     |  2 +-
 arch/sh/drivers/pci/fixups-dreamcast.c  |  2 +-
 arch/sh/drivers/pci/fixups-r7780rp.c    |  2 +-
 arch/sh/drivers/pci/fixups-rts7751r2d.c |  6 +++---
 arch/sh/drivers/pci/fixups-sdk7780.c    |  4 ++--
 arch/sh/drivers/pci/fixups-se7751.c     |  2 +-
 arch/sh/drivers/pci/fixups-sh03.c       |  2 +-
 arch/sh/drivers/pci/fixups-snapgear.c   |  2 +-
 arch/sh/drivers/pci/fixups-titan.c      |  4 ++--
 arch/sh/drivers/pci/pci.c               | 10 +++++++---
 arch/sh/drivers/pci/pcie-sh7786.c       |  2 +-
 11 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/arch/sh/drivers/pci/fixups-cayman.c b/arch/sh/drivers/pci/fixups-cayman.c
index edc2fb7..3246788 100644
--- a/arch/sh/drivers/pci/fixups-cayman.c
+++ b/arch/sh/drivers/pci/fixups-cayman.c
@@ -5,7 +5,7 @@
 #include <cpu/irq.h>
 #include "pci-sh5.h"
 
-int __init pcibios_map_platform_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 {
 	int result = -1;
 
diff --git a/arch/sh/drivers/pci/fixups-dreamcast.c b/arch/sh/drivers/pci/fixups-dreamcast.c
index 1d1c5a2..9d597f7 100644
--- a/arch/sh/drivers/pci/fixups-dreamcast.c
+++ b/arch/sh/drivers/pci/fixups-dreamcast.c
@@ -76,7 +76,7 @@ static void gapspci_fixup_resources(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, gapspci_fixup_resources);
 
-int __init pcibios_map_platform_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 {
 	/*
 	 * The interrupt routing semantics here are quite trivial.
diff --git a/arch/sh/drivers/pci/fixups-r7780rp.c b/arch/sh/drivers/pci/fixups-r7780rp.c
index 57ed3f0..2c9b58f 100644
--- a/arch/sh/drivers/pci/fixups-r7780rp.c
+++ b/arch/sh/drivers/pci/fixups-r7780rp.c
@@ -15,7 +15,7 @@
 #include <linux/sh_intc.h>
 #include "pci-sh4.h"
 
-int __init pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
 	return evt2irq(0xa20) + slot;
 }
diff --git a/arch/sh/drivers/pci/fixups-rts7751r2d.c b/arch/sh/drivers/pci/fixups-rts7751r2d.c
index eaddb56..358ac10 100644
--- a/arch/sh/drivers/pci/fixups-rts7751r2d.c
+++ b/arch/sh/drivers/pci/fixups-rts7751r2d.c
@@ -20,18 +20,18 @@
 #define PCIMCR_MRSET_OFF	0xBFFFFFFF
 #define PCIMCR_RFSH_OFF		0xFFFFFFFB
 
-static u8 rts7751r2d_irq_tab[] __initdata = {
+static u8 rts7751r2d_irq_tab[] = {
 	IRQ_PCI_INTA,
 	IRQ_PCI_INTB,
 	IRQ_PCI_INTC,
 	IRQ_PCI_INTD,
 };
 
-static char lboxre2_irq_tab[] __initdata = {
+static char lboxre2_irq_tab[] = {
 	IRQ_ETH0, IRQ_ETH1, IRQ_INTA, IRQ_INTD,
 };
 
-int __init pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
 	if (mach_is_lboxre2())
 		return lboxre2_irq_tab[slot];
diff --git a/arch/sh/drivers/pci/fixups-sdk7780.c b/arch/sh/drivers/pci/fixups-sdk7780.c
index c0a015a..24e96df 100644
--- a/arch/sh/drivers/pci/fixups-sdk7780.c
+++ b/arch/sh/drivers/pci/fixups-sdk7780.c
@@ -22,7 +22,7 @@
 #define IRQ_INTD	evt2irq(0xa80)
 
 /* IDSEL [16][17][18][19][20][21][22][23][24][25][26][27][28][29][30][31] */
-static char sdk7780_irq_tab[4][16] __initdata = {
+static char sdk7780_irq_tab[4][16] = {
 	/* INTA */
 	{ IRQ_INTA, IRQ_INTD, IRQ_INTC, IRQ_INTD, -1, -1, -1, -1, -1, -1,
 	  -1, -1, -1, -1, -1, -1 },
@@ -37,7 +37,7 @@ static char sdk7780_irq_tab[4][16] __initdata = {
 	  -1, -1, -1 },
 };
 
-int __init pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
        return sdk7780_irq_tab[pin-1][slot];
 }
diff --git a/arch/sh/drivers/pci/fixups-se7751.c b/arch/sh/drivers/pci/fixups-se7751.c
index 84a88ca..1cb8d0a 100644
--- a/arch/sh/drivers/pci/fixups-se7751.c
+++ b/arch/sh/drivers/pci/fixups-se7751.c
@@ -7,7 +7,7 @@
 #include <linux/sh_intc.h>
 #include "pci-sh4.h"
 
-int __init pcibios_map_platform_irq(const struct pci_dev *, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *, u8 slot, u8 pin)
 {
         switch (slot) {
         case 0: return evt2irq(0x3a0);
diff --git a/arch/sh/drivers/pci/fixups-sh03.c b/arch/sh/drivers/pci/fixups-sh03.c
index 16207be..55ac1ba 100644
--- a/arch/sh/drivers/pci/fixups-sh03.c
+++ b/arch/sh/drivers/pci/fixups-sh03.c
@@ -4,7 +4,7 @@
 #include <linux/pci.h>
 #include <linux/sh_intc.h>
 
-int __init pcibios_map_platform_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 {
 	int irq;
 
diff --git a/arch/sh/drivers/pci/fixups-snapgear.c b/arch/sh/drivers/pci/fixups-snapgear.c
index 6e33ba4..a931e59 100644
--- a/arch/sh/drivers/pci/fixups-snapgear.c
+++ b/arch/sh/drivers/pci/fixups-snapgear.c
@@ -19,7 +19,7 @@
 #include <linux/sh_intc.h>
 #include "pci-sh4.h"
 
-int __init pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
 	int irq = -1;
 
diff --git a/arch/sh/drivers/pci/fixups-titan.c b/arch/sh/drivers/pci/fixups-titan.c
index bd1addb..a9d563e 100644
--- a/arch/sh/drivers/pci/fixups-titan.c
+++ b/arch/sh/drivers/pci/fixups-titan.c
@@ -19,7 +19,7 @@
 #include <mach/titan.h>
 #include "pci-sh4.h"
 
-static char titan_irq_tab[] __initdata = {
+static char titan_irq_tab[] = {
 	TITAN_IRQ_WAN,
 	TITAN_IRQ_LAN,
 	TITAN_IRQ_MPCIA,
@@ -27,7 +27,7 @@ static char titan_irq_tab[] __initdata = {
 	TITAN_IRQ_USB,
 };
 
-int __init pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
 	int irq = titan_irq_tab[slot];
 
diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index 1bc09ee..718fae3 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -141,16 +141,20 @@ static int __init pcibios_init(void)
 	for (hose = hose_head; hose; hose = hose->next)
 		pcibios_scanbus(hose);
 
-	pci_fixup_irqs(pci_common_swizzle, pcibios_map_platform_irq);
-
 	dma_debug_add_bus(&pci_bus_type);
-
 	pci_initialized = 1;
 
 	return 0;
 }
 subsys_initcall(pcibios_init);
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = pci_common_swizzle;
+	bridge->map_irq = pcibios_map_platform_irq;
+	return 0;
+}
+
 /*
  *  Called after each bus is probed, but before its children
  *  are examined.
diff --git a/arch/sh/drivers/pci/pcie-sh7786.c b/arch/sh/drivers/pci/pcie-sh7786.c
index a162a7f..0167a73 100644
--- a/arch/sh/drivers/pci/pcie-sh7786.c
+++ b/arch/sh/drivers/pci/pcie-sh7786.c
@@ -467,7 +467,7 @@ static int __init pcie_init(struct sh7786_pcie_port *port)
 	return 0;
 }
 
-int __init pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
+int pcibios_map_platform_irq(const struct pci_dev *pdev, u8 slot, u8 pin)
 {
         return evt2irq(0xae0);
 }
-- 
2.0.4

[PATCH 18/22] Added (untested) support to unicore32 architecture to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/unicore32/kernel/pci.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/unicore32/kernel/pci.c b/arch/unicore32/kernel/pci.c
index 374a055..6a9da80 100644
--- a/arch/unicore32/kernel/pci.c
+++ b/arch/unicore32/kernel/pci.c
@@ -101,7 +101,7 @@ void pci_puv3_preinit(void)
 	writel(readl(PCIBRI_CMD) | PCIBRI_CMD_IO | PCIBRI_CMD_MEM, PCIBRI_CMD);
 }
 
-static int __init pci_puv3_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+static int pci_puv3_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 {
 	if (dev->bus->number == 0) {
 #ifdef CONFIG_ARCH_FPGA /* 4 pci slots */
@@ -263,8 +263,6 @@ static int __init pci_common_init(void)
 	if (!puv3_bus)
 		panic("PCI: unable to scan bus!");
 
-	pci_fixup_irqs(pci_common_swizzle, pci_puv3_map_irq);
-
 	if (!pci_has_flag(PCI_PROBE_ONLY)) {
 		/*
 		 * Size the bridge windows.
@@ -281,6 +279,13 @@ static int __init pci_common_init(void)
 }
 subsys_initcall(pci_common_init);
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = pci_common_swizzle;
+	bridge->map_irq = pci_puv3_map_irq;
+	return 0;
+}
+
 char * __init pcibios_setup(char *str)
 {
 	if (!strcmp(str, "debug")) {
-- 
2.0.4

[PATCH 19/22] Added (untested) workaround to s390 architecture to accomodate new irq init code. Unfortunately adding runtime irq assignment will not work but avoided breaking the existing code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/s390/pci/pci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 9ddc51e..be7d1fe 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -474,6 +474,13 @@ out:
 	return rc;
 }
 
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+	bridge->swizzle_irq = NULL;
+	bridge->map_irq = NULL;
+	return 0;
+}
+
 void arch_teardown_msi_irqs(struct pci_dev *pdev)
 {
 	struct zpci_dev *zdev = get_zdev(pdev);
-- 
2.0.4

[PATCH 20/22] Added (untested) support to alpha nautilus boards to accomodate new irq init code

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/alpha/kernel/sys_nautilus.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/alpha/kernel/sys_nautilus.c b/arch/alpha/kernel/sys_nautilus.c
index 837c0fa..7fb4d51 100644
--- a/arch/alpha/kernel/sys_nautilus.c
+++ b/arch/alpha/kernel/sys_nautilus.c
@@ -252,7 +252,6 @@ nautilus_init_pci(void)
 	/* pci_common_swizzle() relies on bus->self being NULL
 	   for the root bus, so just clear it. */
 	bus->self = NULL;
-	pci_fixup_irqs(alpha_mv.pci_swizzle, alpha_mv.pci_map_irq);
 }
 
 /*
-- 
2.0.4

[PATCH 21/22] Fixes so non PCI x86 systems should work

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 arch/x86/include/asm/pci_x86.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index 436b9e5..16fd8e9 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -205,5 +205,5 @@ static inline void mmio_config_writel(void __iomem *pos, u32 val)
 #else
 # define x86_default_pci_init		NULL
 # define x86_default_pci_init_irq	NULL
-# define x86_default_pci_fixup_irqs	NULL
+# define x86_default_pci_fixup_irq	NULL
 #endif
-- 
2.0.4

[PATCH 22/22] Tidy up including documentation and messages

[matthew_minter]

From: matthew_minter <matthew_minter@xyratex.com>

---
 drivers/of/of_pci_irq.c | 2 +-
 drivers/pci/setup-irq.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
index 1710d9d..205eb7a 100644
--- a/drivers/of/of_pci_irq.c
+++ b/drivers/of/of_pci_irq.c
@@ -97,7 +97,7 @@ EXPORT_SYMBOL_GPL(of_irq_parse_pci);
  * @pin: PCI irq pin number; passed when used as map_irq callback. Unused
  *
  * @slot and @pin are unused, but included in the function so that this
- * function can be used directly as the map_irq callback to pci_fixup_irqs().
+ * function can be used directly as the map_irq callback to pdev_assign_irq().
  */
 int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin)
 {
diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c
index 7bb4356..203bf7e 100644
--- a/drivers/pci/setup-irq.c
+++ b/drivers/pci/setup-irq.c
@@ -31,7 +31,7 @@ void pdev_assign_irq(struct pci_dev *dev,
 	int irq = 0;
 
 	if (!map_irq) {
-		dev_dbg(&dev->dev, "map_irq not provided by arch\n", dev->irq);
+		dev_dbg(&dev->dev, "runtime irq mapping not provided by arch\n");
 		return;
 	}
 
@@ -46,7 +46,7 @@ void pdev_assign_irq(struct pci_dev *dev,
 	if (pin > 4)
 		pin = 1;
 
-	if (pin != 0) {
+	if (pin) {
 		/* Follow the chain of bridges, swizzling as we go. */
 		if(swizzle)
 			slot = (*swizzle)(dev, &pin);
@@ -57,7 +57,7 @@ void pdev_assign_irq(struct pci_dev *dev,
 			irq = 0;
 	}
 
-	dev_dbg(&dev->dev, "fixup irq: got %d\n", dev->irq);
+	dev_dbg(&dev->dev, "assign irq: got %d\n", dev->irq);
 	dev->irq = irq;
 	/* Always tell the device, so the driver knows what is
 	   the real IRQ to use; the device does not use it. */
-- 
2.0.4

Re: [PATCH 01/22] Added some infrastructure to allow assigning PCI device IRQs at device enable time and removed pci_fixup_irqs as the other change obsolites it.

[Wei Yang]

On Tue, Aug 05, 2014 at 05:11:02PM +0100, matthew_minter@xyratex.com wrote:
>From: matthew_minter <matthew_minter@xyratex.com>
>
>---
> drivers/pci/Makefile      | 15 ++-------------
> drivers/pci/host-bridge.c |  2 +-
> drivers/pci/pci.c         |  6 +++++-
> drivers/pci/pci.h         |  1 +
> drivers/pci/probe.c       | 12 ------------
> drivers/pci/setup-irq.c   | 25 +++++++++----------------
> include/linux/pci.h       |  8 ++++----
> 7 files changed, 22 insertions(+), 47 deletions(-)
>
>diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>index e04fe2d..38c4cb0 100644
>--- a/drivers/pci/Makefile
>+++ b/drivers/pci/Makefile
>@@ -4,7 +4,8 @@
>
> obj-y		+= access.o bus.o probe.o host-bridge.o remove.o pci.o \
> 			pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
>-			irq.o vpd.o setup-bus.o vc.o
>+			irq.o vpd.o setup-bus.o vc.o setup-irq.o
>+
> obj-$(CONFIG_PROC_FS) += proc.o
> obj-$(CONFIG_SYSFS) += slot.o
>
>@@ -31,18 +32,6 @@ obj-$(CONFIG_PCI_ATS) += ats.o
> obj-$(CONFIG_PCI_IOV) += iov.o
>
> #
>-# Some architectures use the generic PCI setup functions
>-#
>-obj-$(CONFIG_ALPHA) += setup-irq.o
>-obj-$(CONFIG_ARM) += setup-irq.o
>-obj-$(CONFIG_UNICORE32) += setup-irq.o
>-obj-$(CONFIG_SUPERH) += setup-irq.o
>-obj-$(CONFIG_MIPS) += setup-irq.o
>-obj-$(CONFIG_TILE) += setup-irq.o
>-obj-$(CONFIG_SPARC_LEON) += setup-irq.o
>-obj-$(CONFIG_M68K) += setup-irq.o
>-
>-#
> # ACPI Related PCI FW Functions
> # ACPI _DSM provided firmware instance and string name
> #
>diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>index 0e5f3c9..8ed186f 100644
>--- a/drivers/pci/host-bridge.c
>+++ b/drivers/pci/host-bridge.c
>@@ -16,7 +16,7 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
> 	return bus;
> }
>
>-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> {
> 	struct pci_bus *root_bus = find_pci_root_bus(bus);
>
>diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>index 63a54a3..d51d076 100644
>--- a/drivers/pci/pci.c
>+++ b/drivers/pci/pci.c
>@@ -1197,10 +1197,15 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
> 	int err;
> 	u16 cmd;
> 	u8 pin;
>+	struct pci_host_bridge *hbrg = find_pci_host_bridge(dev->bus);
>
> 	err = pci_set_power_state(dev, PCI_D0);
> 	if (err < 0 && err != -EIO)
> 		return err;
>+
>+	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>+	pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>+
> 	err = pcibios_enable_device(dev, bars);
> 	if (err < 0)
> 		return err;
>@@ -1209,7 +1214,6 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
> 	if (dev->msi_enabled || dev->msix_enabled)
> 		return 0;
>
>-	pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);

Not sure why you move this up.

Before your change, the call flow would be like this:

| scan_bus
|   + pci_fixup_irqs
|      +    pci_dev->irq and PCI_INTERRUPT_PIN assigned with correct number
|
| pci_enable_device, called from driver
|   + do_pci_enable_device, read the pin from pci_dev

As my understanding, this ensures the pin read from hardware is fixuped. While
after your change, the pci_fixup_irqs is removed and the pin will be assigned
in pdev_assign_irq() after you fix it properly.

Do I missed something?

> 	if (pin) {
> 		pci_read_config_word(dev, PCI_COMMAND, &cmd);
> 		if (cmd & PCI_COMMAND_INTX_DISABLE)

-- 
Richard Yang
Help you, Help me

Re: [PATCH 01/22] Added some infrastructure to allow assigning PCI device IRQs at device enable time and removed pci_fixup_irqs as the other change obsolites it.

[Matthew Minter]

Just to confirm, are you saying that:

>+
>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>+

Should become:

>+
>+     pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>+     pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>+

So that we read the config byte after running the assignment and fixup?

I had not thought about the case where the fixup changes the
PCI_INTERRUPT_PIN value.
It looks to me like you are correct there and this should indeed be fixed.
I can certainly swap those around for the next patch revision if that
is what you are suggesting?

I will give that a quick test but it sounds like you are correct.

Many thanks,
Matthew

On 7 August 2014 04:43, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
> On Tue, Aug 05, 2014 at 05:11:02PM +0100, matthew_minter@xyratex.com wrote:
>>From: matthew_minter <matthew_minter@xyratex.com>
>>
>>---
>> drivers/pci/Makefile      | 15 ++-------------
>> drivers/pci/host-bridge.c |  2 +-
>> drivers/pci/pci.c         |  6 +++++-
>> drivers/pci/pci.h         |  1 +
>> drivers/pci/probe.c       | 12 ------------
>> drivers/pci/setup-irq.c   | 25 +++++++++----------------
>> include/linux/pci.h       |  8 ++++----
>> 7 files changed, 22 insertions(+), 47 deletions(-)
>>
>>diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>>index e04fe2d..38c4cb0 100644
>>--- a/drivers/pci/Makefile
>>+++ b/drivers/pci/Makefile
>>@@ -4,7 +4,8 @@
>>
>> obj-y         += access.o bus.o probe.o host-bridge.o remove.o pci.o \
>>                       pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
>>-                      irq.o vpd.o setup-bus.o vc.o
>>+                      irq.o vpd.o setup-bus.o vc.o setup-irq.o
>>+
>> obj-$(CONFIG_PROC_FS) += proc.o
>> obj-$(CONFIG_SYSFS) += slot.o
>>
>>@@ -31,18 +32,6 @@ obj-$(CONFIG_PCI_ATS) += ats.o
>> obj-$(CONFIG_PCI_IOV) += iov.o
>>
>> #
>>-# Some architectures use the generic PCI setup functions
>>-#
>>-obj-$(CONFIG_ALPHA) += setup-irq.o
>>-obj-$(CONFIG_ARM) += setup-irq.o
>>-obj-$(CONFIG_UNICORE32) += setup-irq.o
>>-obj-$(CONFIG_SUPERH) += setup-irq.o
>>-obj-$(CONFIG_MIPS) += setup-irq.o
>>-obj-$(CONFIG_TILE) += setup-irq.o
>>-obj-$(CONFIG_SPARC_LEON) += setup-irq.o
>>-obj-$(CONFIG_M68K) += setup-irq.o
>>-
>>-#
>> # ACPI Related PCI FW Functions
>> # ACPI _DSM provided firmware instance and string name
>> #
>>diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>>index 0e5f3c9..8ed186f 100644
>>--- a/drivers/pci/host-bridge.c
>>+++ b/drivers/pci/host-bridge.c
>>@@ -16,7 +16,7 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>>       return bus;
>> }
>>
>>-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>>+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>> {
>>       struct pci_bus *root_bus = find_pci_root_bus(bus);
>>
>>diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>index 63a54a3..d51d076 100644
>>--- a/drivers/pci/pci.c
>>+++ b/drivers/pci/pci.c
>>@@ -1197,10 +1197,15 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
>>       int err;
>>       u16 cmd;
>>       u8 pin;
>>+      struct pci_host_bridge *hbrg = find_pci_host_bridge(dev->bus);
>>
>>       err = pci_set_power_state(dev, PCI_D0);
>>       if (err < 0 && err != -EIO)
>>               return err;
>>+
>>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>+
>>       err = pcibios_enable_device(dev, bars);
>>       if (err < 0)
>>               return err;
>>@@ -1209,7 +1214,6 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
>>       if (dev->msi_enabled || dev->msix_enabled)
>>               return 0;
>>
>>-      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>
> Not sure why you move this up.
>
> Before your change, the call flow would be like this:
>
> | scan_bus
> |   + pci_fixup_irqs
> |      +    pci_dev->irq and PCI_INTERRUPT_PIN assigned with correct number
> |
> | pci_enable_device, called from driver
> |   + do_pci_enable_device, read the pin from pci_dev
>
> As my understanding, this ensures the pin read from hardware is fixuped. While
> after your change, the pci_fixup_irqs is removed and the pin will be assigned
> in pdev_assign_irq() after you fix it properly.
>
> Do I missed something?
>
>>       if (pin) {
>>               pci_read_config_word(dev, PCI_COMMAND, &cmd);
>>               if (cmd & PCI_COMMAND_INTX_DISABLE)
>
> --
> Richard Yang
> Help you, Help me
>

-- 


------------------------------
For additional information including the registered office and the treatment of Xyratex confidential information please visit www.xyratex.com

------------------------------

Re: [PATCH 01/22] Added some infrastructure to allow assigning PCI device IRQs at device enable time and removed pci_fixup_irqs as the other change obsolites it.

[Wei Yang]

On Thu, Aug 07, 2014 at 12:35:41PM +0100, Matthew Minter wrote:
>Just to confirm, are you saying that:
>
>>+
>>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>+
>
>Should become:
>
>>+
>>+     pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>+     pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>+
>
>So that we read the config byte after running the assignment and fixup?
>
>I had not thought about the case where the fixup changes the
>PCI_INTERRUPT_PIN value.
>It looks to me like you are correct there and this should indeed be fixed.
>I can certainly swap those around for the next patch revision if that
>is what you are suggesting?
>

Yes, pci_read_config_byte() should be called after pdev_assign_irq().

>I will give that a quick test but it sounds like you are correct.

I am willing to know the result, especially in the original one, what pin
value you retrieved from pci_dev.

>
>Many thanks,
>Matthew
>
>On 7 August 2014 04:43, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>> On Tue, Aug 05, 2014 at 05:11:02PM +0100, matthew_minter@xyratex.com wrote:
>>>From: matthew_minter <matthew_minter@xyratex.com>
>>>
>>>---
>>> drivers/pci/Makefile      | 15 ++-------------
>>> drivers/pci/host-bridge.c |  2 +-
>>> drivers/pci/pci.c         |  6 +++++-
>>> drivers/pci/pci.h         |  1 +
>>> drivers/pci/probe.c       | 12 ------------
>>> drivers/pci/setup-irq.c   | 25 +++++++++----------------
>>> include/linux/pci.h       |  8 ++++----
>>> 7 files changed, 22 insertions(+), 47 deletions(-)
>>>
>>>diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>>>index e04fe2d..38c4cb0 100644
>>>--- a/drivers/pci/Makefile
>>>+++ b/drivers/pci/Makefile
>>>@@ -4,7 +4,8 @@
>>>
>>> obj-y         += access.o bus.o probe.o host-bridge.o remove.o pci.o \
>>>                       pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
>>>-                      irq.o vpd.o setup-bus.o vc.o
>>>+                      irq.o vpd.o setup-bus.o vc.o setup-irq.o
>>>+
>>> obj-$(CONFIG_PROC_FS) += proc.o
>>> obj-$(CONFIG_SYSFS) += slot.o
>>>
>>>@@ -31,18 +32,6 @@ obj-$(CONFIG_PCI_ATS) += ats.o
>>> obj-$(CONFIG_PCI_IOV) += iov.o
>>>
>>> #
>>>-# Some architectures use the generic PCI setup functions
>>>-#
>>>-obj-$(CONFIG_ALPHA) += setup-irq.o
>>>-obj-$(CONFIG_ARM) += setup-irq.o
>>>-obj-$(CONFIG_UNICORE32) += setup-irq.o
>>>-obj-$(CONFIG_SUPERH) += setup-irq.o
>>>-obj-$(CONFIG_MIPS) += setup-irq.o
>>>-obj-$(CONFIG_TILE) += setup-irq.o
>>>-obj-$(CONFIG_SPARC_LEON) += setup-irq.o
>>>-obj-$(CONFIG_M68K) += setup-irq.o
>>>-
>>>-#
>>> # ACPI Related PCI FW Functions
>>> # ACPI _DSM provided firmware instance and string name
>>> #
>>>diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>>>index 0e5f3c9..8ed186f 100644
>>>--- a/drivers/pci/host-bridge.c
>>>+++ b/drivers/pci/host-bridge.c
>>>@@ -16,7 +16,7 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>>>       return bus;
>>> }
>>>
>>>-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>>>+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>>> {
>>>       struct pci_bus *root_bus = find_pci_root_bus(bus);
>>>
>>>diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>>index 63a54a3..d51d076 100644
>>>--- a/drivers/pci/pci.c
>>>+++ b/drivers/pci/pci.c
>>>@@ -1197,10 +1197,15 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
>>>       int err;
>>>       u16 cmd;
>>>       u8 pin;
>>>+      struct pci_host_bridge *hbrg = find_pci_host_bridge(dev->bus);
>>>
>>>       err = pci_set_power_state(dev, PCI_D0);
>>>       if (err < 0 && err != -EIO)
>>>               return err;
>>>+
>>>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>>+
>>>       err = pcibios_enable_device(dev, bars);
>>>       if (err < 0)
>>>               return err;
>>>@@ -1209,7 +1214,6 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
>>>       if (dev->msi_enabled || dev->msix_enabled)
>>>               return 0;
>>>
>>>-      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>
>> Not sure why you move this up.
>>
>> Before your change, the call flow would be like this:
>>
>> | scan_bus
>> |   + pci_fixup_irqs
>> |      +    pci_dev->irq and PCI_INTERRUPT_PIN assigned with correct number
>> |
>> | pci_enable_device, called from driver
>> |   + do_pci_enable_device, read the pin from pci_dev
>>
>> As my understanding, this ensures the pin read from hardware is fixuped. While
>> after your change, the pci_fixup_irqs is removed and the pin will be assigned
>> in pdev_assign_irq() after you fix it properly.
>>
>> Do I missed something?
>>
>>>       if (pin) {
>>>               pci_read_config_word(dev, PCI_COMMAND, &cmd);
>>>               if (cmd & PCI_COMMAND_INTX_DISABLE)
>>
>> --
>> Richard Yang
>> Help you, Help me
>>
>
>-- 
>
>
>------------------------------
>For additional information including the registered office and the treatment of Xyratex confidential information please visit www.xyratex.com
>
>------------------------------

-- 
Richard Yang
Help you, Help me

Re: [PATCH 01/22] Added some infrastructure to allow assigning PCI device IRQs at device enable time and removed pci_fixup_irqs as the other change obsolites it.

[Matthew Minter]

It seems that by coincidence, my testing boxes did not have any
devices which needed the PCI_INTERRUPT_PIN fixed up. Therefore I had
not noticed the fact the fixed value was not used in the enable path.

I would assume that someone with a different setup would have had a
problem there however so thank you for pointing out this bug. I have
integrated your fix into version 2 of the patch set.

Many thanks.

On 8 August 2014 02:20, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
> On Thu, Aug 07, 2014 at 12:35:41PM +0100, Matthew Minter wrote:
>>Just to confirm, are you saying that:
>>
>>>+
>>>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>>+
>>
>>Should become:
>>
>>>+
>>>+     pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>>+     pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>>+
>>
>>So that we read the config byte after running the assignment and fixup?
>>
>>I had not thought about the case where the fixup changes the
>>PCI_INTERRUPT_PIN value.
>>It looks to me like you are correct there and this should indeed be fixed.
>>I can certainly swap those around for the next patch revision if that
>>is what you are suggesting?
>>
>
> Yes, pci_read_config_byte() should be called after pdev_assign_irq().
>
>>I will give that a quick test but it sounds like you are correct.
>
> I am willing to know the result, especially in the original one, what pin
> value you retrieved from pci_dev.
>
>>
>>Many thanks,
>>Matthew
>>
>>On 7 August 2014 04:43, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>>> On Tue, Aug 05, 2014 at 05:11:02PM +0100, matthew_minter@xyratex.com wrote:
>>>>From: matthew_minter <matthew_minter@xyratex.com>
>>>>
>>>>---
>>>> drivers/pci/Makefile      | 15 ++-------------
>>>> drivers/pci/host-bridge.c |  2 +-
>>>> drivers/pci/pci.c         |  6 +++++-
>>>> drivers/pci/pci.h         |  1 +
>>>> drivers/pci/probe.c       | 12 ------------
>>>> drivers/pci/setup-irq.c   | 25 +++++++++----------------
>>>> include/linux/pci.h       |  8 ++++----
>>>> 7 files changed, 22 insertions(+), 47 deletions(-)
>>>>
>>>>diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>>>>index e04fe2d..38c4cb0 100644
>>>>--- a/drivers/pci/Makefile
>>>>+++ b/drivers/pci/Makefile
>>>>@@ -4,7 +4,8 @@
>>>>
>>>> obj-y         += access.o bus.o probe.o host-bridge.o remove.o pci.o \
>>>>                       pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
>>>>-                      irq.o vpd.o setup-bus.o vc.o
>>>>+                      irq.o vpd.o setup-bus.o vc.o setup-irq.o
>>>>+
>>>> obj-$(CONFIG_PROC_FS) += proc.o
>>>> obj-$(CONFIG_SYSFS) += slot.o
>>>>
>>>>@@ -31,18 +32,6 @@ obj-$(CONFIG_PCI_ATS) += ats.o
>>>> obj-$(CONFIG_PCI_IOV) += iov.o
>>>>
>>>> #
>>>>-# Some architectures use the generic PCI setup functions
>>>>-#
>>>>-obj-$(CONFIG_ALPHA) += setup-irq.o
>>>>-obj-$(CONFIG_ARM) += setup-irq.o
>>>>-obj-$(CONFIG_UNICORE32) += setup-irq.o
>>>>-obj-$(CONFIG_SUPERH) += setup-irq.o
>>>>-obj-$(CONFIG_MIPS) += setup-irq.o
>>>>-obj-$(CONFIG_TILE) += setup-irq.o
>>>>-obj-$(CONFIG_SPARC_LEON) += setup-irq.o
>>>>-obj-$(CONFIG_M68K) += setup-irq.o
>>>>-
>>>>-#
>>>> # ACPI Related PCI FW Functions
>>>> # ACPI _DSM provided firmware instance and string name
>>>> #
>>>>diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>>>>index 0e5f3c9..8ed186f 100644
>>>>--- a/drivers/pci/host-bridge.c
>>>>+++ b/drivers/pci/host-bridge.c
>>>>@@ -16,7 +16,7 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>>>>       return bus;
>>>> }
>>>>
>>>>-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>>>>+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>>>> {
>>>>       struct pci_bus *root_bus = find_pci_root_bus(bus);
>>>>
>>>>diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>>>index 63a54a3..d51d076 100644
>>>>--- a/drivers/pci/pci.c
>>>>+++ b/drivers/pci/pci.c
>>>>@@ -1197,10 +1197,15 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
>>>>       int err;
>>>>       u16 cmd;
>>>>       u8 pin;
>>>>+      struct pci_host_bridge *hbrg = find_pci_host_bridge(dev->bus);
>>>>
>>>>       err = pci_set_power_state(dev, PCI_D0);
>>>>       if (err < 0 && err != -EIO)
>>>>               return err;
>>>>+
>>>>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>>>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>>>+
>>>>       err = pcibios_enable_device(dev, bars);
>>>>       if (err < 0)
>>>>               return err;
>>>>@@ -1209,7 +1214,6 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
>>>>       if (dev->msi_enabled || dev->msix_enabled)
>>>>               return 0;
>>>>
>>>>-      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>>
>>> Not sure why you move this up.
>>>
>>> Before your change, the call flow would be like this:
>>>
>>> | scan_bus
>>> |   + pci_fixup_irqs
>>> |      +    pci_dev->irq and PCI_INTERRUPT_PIN assigned with correct number
>>> |
>>> | pci_enable_device, called from driver
>>> |   + do_pci_enable_device, read the pin from pci_dev
>>>
>>> As my understanding, this ensures the pin read from hardware is fixuped. While
>>> after your change, the pci_fixup_irqs is removed and the pin will be assigned
>>> in pdev_assign_irq() after you fix it properly.
>>>
>>> Do I missed something?
>>>
>>>>       if (pin) {
>>>>               pci_read_config_word(dev, PCI_COMMAND, &cmd);
>>>>               if (cmd & PCI_COMMAND_INTX_DISABLE)
>>>
>>> --
>>> Richard Yang
>>> Help you, Help me
>>>
>>
>>--
>>
>>
>>------------------------------
>>For additional information including the registered office and the treatment of Xyratex confidential information please visit www.xyratex.com
>>
>>------------------------------
>
> --
> Richard Yang
> Help you, Help me
>

-- 


------------------------------
For additional information including the registered office and the treatment of Xyratex confidential information please visit www.xyratex.com

------------------------------

Re: [PATCH 01/22] Added some infrastructure to allow assigning PCI device IRQs at device enable time and removed pci_fixup_irqs as the other change obsolites it.

[Wei Yang]

On Mon, Aug 11, 2014 at 11:16:50AM +0100, Matthew Minter wrote:
>It seems that by coincidence, my testing boxes did not have any
>devices which needed the PCI_INTERRUPT_PIN fixed up. Therefore I had
>not noticed the fact the fixed value was not used in the enable path.

ok

>
>I would assume that someone with a different setup would have had a
>problem there however so thank you for pointing out this bug. I have
>integrated your fix into version 2 of the patch set.

thanks

>
>Many thanks.
>
>On 8 August 2014 02:20, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>> On Thu, Aug 07, 2014 at 12:35:41PM +0100, Matthew Minter wrote:
>>>Just to confirm, are you saying that:
>>>
>>>>+
>>>>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>>>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>>>+
>>>
>>>Should become:
>>>
>>>>+
>>>>+     pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>>>+     pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>>>+
>>>
>>>So that we read the config byte after running the assignment and fixup?
>>>
>>>I had not thought about the case where the fixup changes the
>>>PCI_INTERRUPT_PIN value.
>>>It looks to me like you are correct there and this should indeed be fixed.
>>>I can certainly swap those around for the next patch revision if that
>>>is what you are suggesting?
>>>
>>
>> Yes, pci_read_config_byte() should be called after pdev_assign_irq().
>>
>>>I will give that a quick test but it sounds like you are correct.
>>
>> I am willing to know the result, especially in the original one, what pin
>> value you retrieved from pci_dev.
>>
>>>
>>>Many thanks,
>>>Matthew
>>>
>>>On 7 August 2014 04:43, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
>>>> On Tue, Aug 05, 2014 at 05:11:02PM +0100, matthew_minter@xyratex.com wrote:
>>>>>From: matthew_minter <matthew_minter@xyratex.com>
>>>>>
>>>>>---
>>>>> drivers/pci/Makefile      | 15 ++-------------
>>>>> drivers/pci/host-bridge.c |  2 +-
>>>>> drivers/pci/pci.c         |  6 +++++-
>>>>> drivers/pci/pci.h         |  1 +
>>>>> drivers/pci/probe.c       | 12 ------------
>>>>> drivers/pci/setup-irq.c   | 25 +++++++++----------------
>>>>> include/linux/pci.h       |  8 ++++----
>>>>> 7 files changed, 22 insertions(+), 47 deletions(-)
>>>>>
>>>>>diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>>>>>index e04fe2d..38c4cb0 100644
>>>>>--- a/drivers/pci/Makefile
>>>>>+++ b/drivers/pci/Makefile
>>>>>@@ -4,7 +4,8 @@
>>>>>
>>>>> obj-y         += access.o bus.o probe.o host-bridge.o remove.o pci.o \
>>>>>                       pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
>>>>>-                      irq.o vpd.o setup-bus.o vc.o
>>>>>+                      irq.o vpd.o setup-bus.o vc.o setup-irq.o
>>>>>+
>>>>> obj-$(CONFIG_PROC_FS) += proc.o
>>>>> obj-$(CONFIG_SYSFS) += slot.o
>>>>>
>>>>>@@ -31,18 +32,6 @@ obj-$(CONFIG_PCI_ATS) += ats.o
>>>>> obj-$(CONFIG_PCI_IOV) += iov.o
>>>>>
>>>>> #
>>>>>-# Some architectures use the generic PCI setup functions
>>>>>-#
>>>>>-obj-$(CONFIG_ALPHA) += setup-irq.o
>>>>>-obj-$(CONFIG_ARM) += setup-irq.o
>>>>>-obj-$(CONFIG_UNICORE32) += setup-irq.o
>>>>>-obj-$(CONFIG_SUPERH) += setup-irq.o
>>>>>-obj-$(CONFIG_MIPS) += setup-irq.o
>>>>>-obj-$(CONFIG_TILE) += setup-irq.o
>>>>>-obj-$(CONFIG_SPARC_LEON) += setup-irq.o
>>>>>-obj-$(CONFIG_M68K) += setup-irq.o
>>>>>-
>>>>>-#
>>>>> # ACPI Related PCI FW Functions
>>>>> # ACPI _DSM provided firmware instance and string name
>>>>> #
>>>>>diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
>>>>>index 0e5f3c9..8ed186f 100644
>>>>>--- a/drivers/pci/host-bridge.c
>>>>>+++ b/drivers/pci/host-bridge.c
>>>>>@@ -16,7 +16,7 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
>>>>>       return bus;
>>>>> }
>>>>>
>>>>>-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>>>>>+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
>>>>> {
>>>>>       struct pci_bus *root_bus = find_pci_root_bus(bus);
>>>>>
>>>>>diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>>>>index 63a54a3..d51d076 100644
>>>>>--- a/drivers/pci/pci.c
>>>>>+++ b/drivers/pci/pci.c
>>>>>@@ -1197,10 +1197,15 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
>>>>>       int err;
>>>>>       u16 cmd;
>>>>>       u8 pin;
>>>>>+      struct pci_host_bridge *hbrg = find_pci_host_bridge(dev->bus);
>>>>>
>>>>>       err = pci_set_power_state(dev, PCI_D0);
>>>>>       if (err < 0 && err != -EIO)
>>>>>               return err;
>>>>>+
>>>>>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>>>>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
>>>>>+
>>>>>       err = pcibios_enable_device(dev, bars);
>>>>>       if (err < 0)
>>>>>               return err;
>>>>>@@ -1209,7 +1214,6 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
>>>>>       if (dev->msi_enabled || dev->msix_enabled)
>>>>>               return 0;
>>>>>
>>>>>-      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
>>>>
>>>> Not sure why you move this up.
>>>>
>>>> Before your change, the call flow would be like this:
>>>>
>>>> | scan_bus
>>>> |   + pci_fixup_irqs
>>>> |      +    pci_dev->irq and PCI_INTERRUPT_PIN assigned with correct number
>>>> |
>>>> | pci_enable_device, called from driver
>>>> |   + do_pci_enable_device, read the pin from pci_dev
>>>>
>>>> As my understanding, this ensures the pin read from hardware is fixuped. While
>>>> after your change, the pci_fixup_irqs is removed and the pin will be assigned
>>>> in pdev_assign_irq() after you fix it properly.
>>>>
>>>> Do I missed something?
>>>>
>>>>>       if (pin) {
>>>>>               pci_read_config_word(dev, PCI_COMMAND, &cmd);
>>>>>               if (cmd & PCI_COMMAND_INTX_DISABLE)
>>>>
>>>> --
>>>> Richard Yang
>>>> Help you, Help me
>>>>
>>>
>>>--
>>>
>>>
>>>------------------------------
>>>For additional information including the registered office and the treatment of Xyratex confidential information please visit www.xyratex.com
>>>
>>>------------------------------
>>
>> --
>> Richard Yang
>> Help you, Help me
>>
>
>-- 
>
>
>------------------------------
>For additional information including the registered office and the treatment of Xyratex confidential information please visit www.xyratex.com
>
>------------------------------

-- 
Richard Yang
Help you, Help me

Re: [PATCH 01/22] Added some infrastructure to allow assigning PCI device IRQs at device enable time and removed pci_fixup_irqs as the other change obsolites it.

[Bjorn Helgaas]

On Mon, Aug 11, 2014 at 11:16:50AM +0100, Matthew Minter wrote:
> It seems that by coincidence, my testing boxes did not have any
> devices which needed the PCI_INTERRUPT_PIN fixed up. Therefore I had
> not noticed the fact the fixed value was not used in the enable path.
> 
> I would assume that someone with a different setup would have had a
> problem there however so thank you for pointing out this bug. I have
> integrated your fix into version 2 of the patch set.

I'm dropping this series and I'll watch for v2.

> Many thanks.
> 
> On 8 August 2014 02:20, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
> > On Thu, Aug 07, 2014 at 12:35:41PM +0100, Matthew Minter wrote:
> >>Just to confirm, are you saying that:
> >>
> >>>+
> >>>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
> >>>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
> >>>+
> >>
> >>Should become:
> >>
> >>>+
> >>>+     pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
> >>>+     pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
> >>>+
> >>
> >>So that we read the config byte after running the assignment and fixup?
> >>
> >>I had not thought about the case where the fixup changes the
> >>PCI_INTERRUPT_PIN value.
> >>It looks to me like you are correct there and this should indeed be fixed.
> >>I can certainly swap those around for the next patch revision if that
> >>is what you are suggesting?
> >>
> >
> > Yes, pci_read_config_byte() should be called after pdev_assign_irq().
> >
> >>I will give that a quick test but it sounds like you are correct.
> >
> > I am willing to know the result, especially in the original one, what pin
> > value you retrieved from pci_dev.
> >
> >>
> >>Many thanks,
> >>Matthew
> >>
> >>On 7 August 2014 04:43, Wei Yang <weiyang@linux.vnet.ibm.com> wrote:
> >>> On Tue, Aug 05, 2014 at 05:11:02PM +0100, matthew_minter@xyratex.com wrote:
> >>>>From: matthew_minter <matthew_minter@xyratex.com>
> >>>>
> >>>>---
> >>>> drivers/pci/Makefile      | 15 ++-------------
> >>>> drivers/pci/host-bridge.c |  2 +-
> >>>> drivers/pci/pci.c         |  6 +++++-
> >>>> drivers/pci/pci.h         |  1 +
> >>>> drivers/pci/probe.c       | 12 ------------
> >>>> drivers/pci/setup-irq.c   | 25 +++++++++----------------
> >>>> include/linux/pci.h       |  8 ++++----
> >>>> 7 files changed, 22 insertions(+), 47 deletions(-)
> >>>>
> >>>>diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> >>>>index e04fe2d..38c4cb0 100644
> >>>>--- a/drivers/pci/Makefile
> >>>>+++ b/drivers/pci/Makefile
> >>>>@@ -4,7 +4,8 @@
> >>>>
> >>>> obj-y         += access.o bus.o probe.o host-bridge.o remove.o pci.o \
> >>>>                       pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
> >>>>-                      irq.o vpd.o setup-bus.o vc.o
> >>>>+                      irq.o vpd.o setup-bus.o vc.o setup-irq.o
> >>>>+
> >>>> obj-$(CONFIG_PROC_FS) += proc.o
> >>>> obj-$(CONFIG_SYSFS) += slot.o
> >>>>
> >>>>@@ -31,18 +32,6 @@ obj-$(CONFIG_PCI_ATS) += ats.o
> >>>> obj-$(CONFIG_PCI_IOV) += iov.o
> >>>>
> >>>> #
> >>>>-# Some architectures use the generic PCI setup functions
> >>>>-#
> >>>>-obj-$(CONFIG_ALPHA) += setup-irq.o
> >>>>-obj-$(CONFIG_ARM) += setup-irq.o
> >>>>-obj-$(CONFIG_UNICORE32) += setup-irq.o
> >>>>-obj-$(CONFIG_SUPERH) += setup-irq.o
> >>>>-obj-$(CONFIG_MIPS) += setup-irq.o
> >>>>-obj-$(CONFIG_TILE) += setup-irq.o
> >>>>-obj-$(CONFIG_SPARC_LEON) += setup-irq.o
> >>>>-obj-$(CONFIG_M68K) += setup-irq.o
> >>>>-
> >>>>-#
> >>>> # ACPI Related PCI FW Functions
> >>>> # ACPI _DSM provided firmware instance and string name
> >>>> #
> >>>>diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> >>>>index 0e5f3c9..8ed186f 100644
> >>>>--- a/drivers/pci/host-bridge.c
> >>>>+++ b/drivers/pci/host-bridge.c
> >>>>@@ -16,7 +16,7 @@ static struct pci_bus *find_pci_root_bus(struct pci_bus *bus)
> >>>>       return bus;
> >>>> }
> >>>>
> >>>>-static struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> >>>>+struct pci_host_bridge *find_pci_host_bridge(struct pci_bus *bus)
> >>>> {
> >>>>       struct pci_bus *root_bus = find_pci_root_bus(bus);
> >>>>
> >>>>diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> >>>>index 63a54a3..d51d076 100644
> >>>>--- a/drivers/pci/pci.c
> >>>>+++ b/drivers/pci/pci.c
> >>>>@@ -1197,10 +1197,15 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
> >>>>       int err;
> >>>>       u16 cmd;
> >>>>       u8 pin;
> >>>>+      struct pci_host_bridge *hbrg = find_pci_host_bridge(dev->bus);
> >>>>
> >>>>       err = pci_set_power_state(dev, PCI_D0);
> >>>>       if (err < 0 && err != -EIO)
> >>>>               return err;
> >>>>+
> >>>>+      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
> >>>>+      pdev_assign_irq(dev, hbrg->swizzle_irq, hbrg->map_irq);
> >>>>+
> >>>>       err = pcibios_enable_device(dev, bars);
> >>>>       if (err < 0)
> >>>>               return err;
> >>>>@@ -1209,7 +1214,6 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars)
> >>>>       if (dev->msi_enabled || dev->msix_enabled)
> >>>>               return 0;
> >>>>
> >>>>-      pci_read_config_byte(dev, PCI_INTERRUPT_PIN, &pin);
> >>>
> >>> Not sure why you move this up.
> >>>
> >>> Before your change, the call flow would be like this:
> >>>
> >>> | scan_bus
> >>> |   + pci_fixup_irqs
> >>> |      +    pci_dev->irq and PCI_INTERRUPT_PIN assigned with correct number
> >>> |
> >>> | pci_enable_device, called from driver
> >>> |   + do_pci_enable_device, read the pin from pci_dev
> >>>
> >>> As my understanding, this ensures the pin read from hardware is fixuped. While
> >>> after your change, the pci_fixup_irqs is removed and the pin will be assigned
> >>> in pdev_assign_irq() after you fix it properly.
> >>>
> >>> Do I missed something?
> >>>
> >>>>       if (pin) {
> >>>>               pci_read_config_word(dev, PCI_COMMAND, &cmd);
> >>>>               if (cmd & PCI_COMMAND_INTX_DISABLE)
> >>>
> >>> --
> >>> Richard Yang
> >>> Help you, Help me
> >>>
> >>
> >>--
> >>
> >>
> >>------------------------------
> >>For additional information including the registered office and the treatment of Xyratex confidential information please visit www.xyratex.com
> >>
> >>------------------------------
> >
> > --
> > Richard Yang
> > Help you, Help me
> >
> 
> -- 
> 
> 
> ------------------------------
> For additional information including the registered office and the treatment of Xyratex confidential information please visit www.xyratex.com
> 
> ------------------------------

Re: [PATCH 01/22] Added some infrastructure to allow assigning PCI device IRQs at device enable time and removed pci_fixup_irqs as the other change obsolites it.

[Bjorn Helgaas]

On Thu, Sep 04, 2014 at 04:00:44PM -0600, Bjorn Helgaas wrote:
> On Mon, Aug 11, 2014 at 11:16:50AM +0100, Matthew Minter wrote:
> > It seems that by coincidence, my testing boxes did not have any
> > devices which needed the PCI_INTERRUPT_PIN fixed up. Therefore I had
> > not noticed the fact the fixed value was not used in the enable path.
> > 
> > I would assume that someone with a different setup would have had a
> > problem there however so thank you for pointing out this bug. I have
> > integrated your fix into version 2 of the patch set.
> 
> I'm dropping this series and I'll watch for v2.

BTW, when you do repost this, the individual patches need changelogs, and
the summary (subject lines) can be a little shorter and more specific,
e.g., no need to mention "untested", but "accomodate new irq init code"
doesn't really convey much information to someone who is looking only at
the arch/m68k commits.

Bjorn

Re: [PATCH 01/22] Added some infrastructure to allow assigning PCI device IRQs at device enable time and removed pci_fixup_irqs as the other change obsolites it.

[Bjorn Helgaas]

On Thu, Sep 04, 2014 at 04:18:43PM -0600, Bjorn Helgaas wrote:
> On Thu, Sep 04, 2014 at 04:00:44PM -0600, Bjorn Helgaas wrote:
> > On Mon, Aug 11, 2014 at 11:16:50AM +0100, Matthew Minter wrote:
> > > It seems that by coincidence, my testing boxes did not have any
> > > devices which needed the PCI_INTERRUPT_PIN fixed up. Therefore I had
> > > not noticed the fact the fixed value was not used in the enable path.
> > > 
> > > I would assume that someone with a different setup would have had a
> > > problem there however so thank you for pointing out this bug. I have
> > > integrated your fix into version 2 of the patch set.
> > 
> > I'm dropping this series and I'll watch for v2.
> 
> BTW, when you do repost this, the individual patches need changelogs, and
> the summary (subject lines) can be a little shorter and more specific,
> e.g., no need to mention "untested", but "accomodate new irq init code"
> doesn't really convey much information to someone who is looking only at
> the arch/m68k commits.

And they need Signed-off-by tags.  See Documentation/SubmittingPatches for
more details.

Bjorn