ARM: pci: kill pcibios_msi_controller

PATCH New Lorenzo Pieralisi 4 participants 9 messages Jul 13, 2015 – Jul 23, 2015

Subsystems (3)

Arm (32-bit) PCI Arm/Zynq architecture

lore

Share & tools

lore

Lorenzo Pieralisi

author

2015-07-13 10:42 UTC #0 lore raw

PATCH 1/1 ARM: pci: kill pcibios_msi_controller

On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the pcibios_msi_controller weak callback is used to look-up the msi_controller pointer, through pci_sys_data msi_ctrl pointer.

pci_sys_data is an arch specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.

Since the struct pci_bus already contains an msi_controller pointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and relate pcibios/host bridges initialization routines so that the msi_controller pointer look-up can be carried out by PCI core code through the struct pci_bus msi pointer, removing the need for arch specific pcibios_msi_controller callback and the related pci_sys_data msi_ctrl pointer.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Jingoo Han <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Simon Horman <[email protected]>

Cc Russell King <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Thierry Reding <[email protected]>

Cc Michal Simek <[email protected]>

Cc Marc Zyngier <[email protected]>

---
Bjorn, all,

I am posting this patch to sound out if that's a reasonable approach. xgene implements MSI look-up this way at present and this would represent another step forward towards having common drivers for ARM/ARM64, comments and testing welcome.

Thanks,
Lorenzo

 arch/arm/include/asm/mach/pci.h    |  3 ---
 arch/arm/kernel/bios32.c           | 35 +++++++++++++++++------------------
 drivers/pci/host/pcie-designware.c |  9 +++++++--
 drivers/pci/host/pcie-xilinx.c     | 12 ++++++++++--
 4 files changed, 34 insertions(+), 25 deletions(-)

arch/arm/include/asm/mach/pci.h +0 -3

index 28b9bb3..32abc0c 100644
@@ -42,9 +42,6 @@ struct hw_pci {
  * Per-controller structure
  */
 struct pci_sys_data {
-#ifdef CONFIG_PCI_MSI
-	struct msi_controller *msi_ctrl;
-#endif
 	struct list_head node;
 	int		busnr;		/* primary bus number			*/
 	u64		mem_offset;	/* bus->cpu memory mapping offset	*/

arch/arm/kernel/bios32.c +17 ~~-18~~

index fcbbbb1..c841b33 100644
@@ -18,15 +18,6 @@
 
 static int debug_pci;
 
-#ifdef CONFIG_PCI_MSI
-struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
-{
-	struct pci_sys_data *sysdata = dev->bus->sysdata;
-
-	return sysdata->msi_ctrl;
-}
-#endif
-
 /*
  * We can't use pci_get_device() here since we are
  * called from interrupt context.

@@ -462,9 +453,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		if (!sys)
 			panic("PCI: unable to allocate sys data!");
 
-#ifdef CONFIG_PCI_MSI
-		sys->msi_ctrl = hw->msi_ctrl;
-#endif
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;

@@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 				break;
 			}
 
-			if (hw->scan)
+			if (hw->scan) {
 				sys->bus = hw->scan(nr, sys);
-			else
-				sys->bus = pci_scan_root_bus(parent, sys->busnr,
-						hw->ops, sys, &sys->resources);
+				if (!sys->bus)
+					panic("PCI: unable to scan bus!");
+			} else {
+				sys->bus = pci_create_root_bus(parent,
+							       sys->busnr,
+							       hw->ops, sys,
+							       &sys->resources);
+				if (WARN_ON(!sys->bus)) {
+					kfree(sys);
+					break;
+				}
+#ifdef CONFIG_PCI_MSI
+				sys->bus->msi = hw->msi_ctrl;
+#endif
 
-			if (!sys->bus)
-				panic("PCI: unable to scan bus!");
+				pci_scan_child_bus(sys->bus);
+			}
 
 			busnr = sys->bus->busn_res.end + 1;

drivers/pci/host/pcie-designware.c +7 -2

index 69486be..e584dfa 100644
@@ -526,7 +526,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 #ifdef CONFIG_PCI_MSI
 	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
 #endif
 
 	dw_pci.nr_controllers = 1;

@@ -708,11 +707,17 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pcie_port *pp = sys_to_pcie(sys);
 
 	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
+	bus = pci_create_root_bus(pp->dev, sys->busnr,
 				  &dw_pcie_ops, sys, &sys->resources);
 	if (!bus)
 		return NULL;
 
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &dw_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	if (bus && pp->ops->scan_bus)
 		pp->ops->scan_bus(pp);

drivers/pci/host/pcie-xilinx.c +10 -2

index f1a06a0..b21eb7d 100644
@@ -647,9 +647,18 @@ static struct pci_bus *xilinx_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pci_bus *bus;
 
 	port->root_busno = sys->busnr;
-	bus = pci_scan_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
+	bus = pci_create_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
 				sys, &sys->resources);
 
+	if (!bus)
+		return NULL;
+
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &xilinx_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	return bus;
 }

@@ -847,7 +856,6 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_PCI_MSI
 	xilinx_pcie_msi_chip.dev = port->dev;
-	hw.msi_ctrl = &xilinx_pcie_msi_chip;
 #endif
 	pci_common_init_dev(dev, &hw);

Signature · 1 line

2.2.1

Lorenzo Pieralisi

author

2015-07-22 09:02 UTC #1 lore raw

PATCH 1/1 ARM: pci: kill pcibios_msi_controller

Quoted 26 lines · Lorenzo Pieralisi

On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the pcibios_msi_controller weak callback is used to look-up the msi_controller pointer, through pci_sys_data msi_ctrl pointer.
pci_sys_data is an arch specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.
Since the struct pci_bus already contains an msi_controller pointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and relate pcibios/host bridges initialization routines so that the msi_controller pointer look-up can be carried out by PCI core code through the struct pci_bus msi pointer, removing the need for arch specific pcibios_msi_controller callback and the related pci_sys_data msi_ctrl pointer.
Signed-off-by Lorenzo Pieralisi <[email protected]>
Cc Pratyush Anand <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Jingoo Han <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Simon Horman <[email protected]>
Cc Russell King <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Michal Simek <[email protected]>
Cc Marc Zyngier <[email protected]>

Quoted 7 lines

---
Bjorn, all,
I am posting this patch to sound out if that's a reasonable approach. xgene implements MSI look-up this way at present and this would represent another step forward towards having common drivers for ARM/ARM64, comments and testing welcome.

Any comments/testing on this patch ? I do not have platforms with these host bridges handy (apart from an iMX6 Sabrelite) so I can't test on them (change is quite mechanical though), help on testing and feedback on the patch much appreciated.

Thanks,
Lorenzo

Quoted 148 lines

 arch/arm/include/asm/mach/pci.h    |  3 ---
 arch/arm/kernel/bios32.c           | 35 +++++++++++++++++------------------
 drivers/pci/host/pcie-designware.c |  9 +++++++--
 drivers/pci/host/pcie-xilinx.c     | 12 ++++++++++--
 4 files changed, 34 insertions(+), 25 deletions(-)

arch/arm/include/asm/mach/pci.h +0 -3

index 28b9bb3..32abc0c 100644
@@ -42,9 +42,6 @@ struct hw_pci {
  * Per-controller structure
  */
 struct pci_sys_data {
-#ifdef CONFIG_PCI_MSI
-	struct msi_controller *msi_ctrl;
-#endif
 	struct list_head node;
 	int		busnr;		/* primary bus number			*/
 	u64		mem_offset;	/* bus->cpu memory mapping offset	*/

arch/arm/kernel/bios32.c +17 ~~-18~~

index fcbbbb1..c841b33 100644
@@ -18,15 +18,6 @@
 
 static int debug_pci;
 
-#ifdef CONFIG_PCI_MSI
-struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
-{
-	struct pci_sys_data *sysdata = dev->bus->sysdata;
-
-	return sysdata->msi_ctrl;
-}
-#endif
-
 /*
  * We can't use pci_get_device() here since we are
  * called from interrupt context.

@@ -462,9 +453,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		if (!sys)
 			panic("PCI: unable to allocate sys data!");
 
-#ifdef CONFIG_PCI_MSI
-		sys->msi_ctrl = hw->msi_ctrl;
-#endif
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;

@@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 				break;
 			}
 
-			if (hw->scan)
+			if (hw->scan) {
 				sys->bus = hw->scan(nr, sys);
-			else
-				sys->bus = pci_scan_root_bus(parent, sys->busnr,
-						hw->ops, sys, &sys->resources);
+				if (!sys->bus)
+					panic("PCI: unable to scan bus!");
+			} else {
+				sys->bus = pci_create_root_bus(parent,
+							       sys->busnr,
+							       hw->ops, sys,
+							       &sys->resources);
+				if (WARN_ON(!sys->bus)) {
+					kfree(sys);
+					break;
+				}
+#ifdef CONFIG_PCI_MSI
+				sys->bus->msi = hw->msi_ctrl;
+#endif
 
-			if (!sys->bus)
-				panic("PCI: unable to scan bus!");
+				pci_scan_child_bus(sys->bus);
+			}
 
 			busnr = sys->bus->busn_res.end + 1;

drivers/pci/host/pcie-designware.c +7 -2

index 69486be..e584dfa 100644
@@ -526,7 +526,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 #ifdef CONFIG_PCI_MSI
 	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
 #endif
 
 	dw_pci.nr_controllers = 1;

@@ -708,11 +707,17 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pcie_port *pp = sys_to_pcie(sys);
 
 	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
+	bus = pci_create_root_bus(pp->dev, sys->busnr,
 				  &dw_pcie_ops, sys, &sys->resources);
 	if (!bus)
 		return NULL;
 
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &dw_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	if (bus && pp->ops->scan_bus)
 		pp->ops->scan_bus(pp);

drivers/pci/host/pcie-xilinx.c +10 -2

index f1a06a0..b21eb7d 100644
@@ -647,9 +647,18 @@ static struct pci_bus *xilinx_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pci_bus *bus;
 
 	port->root_busno = sys->busnr;
-	bus = pci_scan_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
+	bus = pci_create_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
 				sys, &sys->resources);
 
+	if (!bus)
+		return NULL;
+
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &xilinx_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	return bus;
 }

@@ -847,7 +856,6 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_PCI_MSI
 	xilinx_pcie_msi_chip.dev = port->dev;
-	hw.msi_ctrl = &xilinx_pcie_msi_chip;
 #endif
 	pci_common_init_dev(dev, &hw);

Signature · 1 line

2.2.1

Marc Zyngier

2015-07-22 09:24 UTC #2 lore raw

PATCH 1/1 ARM: pci: kill pcibios_msi_controller

Quoted 35 lines · Lorenzo Pieralisi

Quoted 26 lines · Lorenzo Pieralisi
On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the pcibios_msi_controller weak callback is used to look-up the msi_controller pointer, through pci_sys_data msi_ctrl pointer.
pci_sys_data is an arch specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.
Since the struct pci_bus already contains an msi_controller pointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and relate pcibios/host bridges initialization routines so that the msi_controller pointer look-up can be carried out by PCI core code through the struct pci_bus msi pointer, removing the need for arch specific pcibios_msi_controller callback and the related pci_sys_data msi_ctrl pointer.
Signed-off-by Lorenzo Pieralisi <[email protected]>
Cc Pratyush Anand <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Jingoo Han <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Simon Horman <[email protected]>
Cc Russell King <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Michal Simek <[email protected]>
Cc Marc Zyngier <[email protected]>
Quoted 7 lines
---
Bjorn, all,
I am posting this patch to sound out if that's a reasonable approach. xgene implements MSI look-up this way at present and this would represent another step forward towards having common drivers for ARM/ARM64, comments and testing welcome.

Quoted 4 lines

Any comments/testing on this patch ? I do not have platforms with these host bridges handy (apart from an iMX6 Sabrelite) so I can't test on them (change is quite mechanical though), help on testing and feedback on the patch much appreciated.

Hi Lorenzo,

I can't test it (no relevant HW), but that seems like a very sensible approach to me (the less dependency on sysdata, the better).

One remark below:

Quoted 54 lines

 arch/arm/include/asm/mach/pci.h    |  3 ---
 arch/arm/kernel/bios32.c           | 35 +++++++++++++++++------------------
 drivers/pci/host/pcie-designware.c |  9 +++++++--
 drivers/pci/host/pcie-xilinx.c     | 12 ++++++++++--
 4 files changed, 34 insertions(+), 25 deletions(-)

arch/arm/include/asm/mach/pci.h +0 -3

index 28b9bb3..32abc0c 100644
@@ -42,9 +42,6 @@ struct hw_pci {
  * Per-controller structure
  */
 struct pci_sys_data {
-#ifdef CONFIG_PCI_MSI
-	struct msi_controller *msi_ctrl;
-#endif
 	struct list_head node;
 	int		busnr;		/* primary bus number			*/
 	u64		mem_offset;	/* bus->cpu memory mapping offset	*/

arch/arm/kernel/bios32.c +0 ~~-12~~

index fcbbbb1..c841b33 100644
@@ -18,15 +18,6 @@
 
 static int debug_pci;
 
-#ifdef CONFIG_PCI_MSI
-struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
-{
-	struct pci_sys_data *sysdata = dev->bus->sysdata;
-
-	return sysdata->msi_ctrl;
-}
-#endif
-
 /*
  * We can't use pci_get_device() here since we are
  * called from interrupt context.

@@ -462,9 +453,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		if (!sys)
 			panic("PCI: unable to allocate sys data!");
 
-#ifdef CONFIG_PCI_MSI
-		sys->msi_ctrl = hw->msi_ctrl;
-#endif
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;

@@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 				break;
 			}

Quoted 8 lines

arch/arm/kernel/bios32.c static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, · line 477
Quoted 8 lines
arch/arm/kernel/bios32.c +3 -4
-			if (hw->scan)
+			if (hw->scan) {
 				sys->bus = hw->scan(nr, sys);
-			else
-				sys->bus = pci_scan_root_bus(parent, sys->busnr,
-						hw->ops, sys, &sys->resources);
+				if (!sys->bus)
+					panic("PCI: unable to scan bus!");

This was in the original code, but I have to ask: Do we really want to panic the kernel if we couldn't scan the bus? Worse case, the system won't be able to boot at all and will panic somewhere else anyway, but we should give the user a chance to understand what's happening...

Quoted 83 lines

arch/arm/kernel/bios32.c +14 -2

+			} else {
+				sys->bus = pci_create_root_bus(parent,
+							       sys->busnr,
+							       hw->ops, sys,
+							       &sys->resources);
+				if (WARN_ON(!sys->bus)) {
+					kfree(sys);
+					break;
+				}
+#ifdef CONFIG_PCI_MSI
+				sys->bus->msi = hw->msi_ctrl;
+#endif
 
-			if (!sys->bus)
-				panic("PCI: unable to scan bus!");
+				pci_scan_child_bus(sys->bus);
+			}
 
 			busnr = sys->bus->busn_res.end + 1;

drivers/pci/host/pcie-designware.c +7 -2

index 69486be..e584dfa 100644
@@ -526,7 +526,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 #ifdef CONFIG_PCI_MSI
 	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
 #endif
 
 	dw_pci.nr_controllers = 1;

@@ -708,11 +707,17 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pcie_port *pp = sys_to_pcie(sys);
 
 	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
+	bus = pci_create_root_bus(pp->dev, sys->busnr,
 				  &dw_pcie_ops, sys, &sys->resources);
 	if (!bus)
 		return NULL;
 
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &dw_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	if (bus && pp->ops->scan_bus)
 		pp->ops->scan_bus(pp);

drivers/pci/host/pcie-xilinx.c +10 -2

index f1a06a0..b21eb7d 100644
@@ -647,9 +647,18 @@ static struct pci_bus *xilinx_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pci_bus *bus;
 
 	port->root_busno = sys->busnr;
-	bus = pci_scan_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
+	bus = pci_create_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
 				sys, &sys->resources);
 
+	if (!bus)
+		return NULL;
+
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &xilinx_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	return bus;
 }

@@ -847,7 +856,6 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_PCI_MSI
 	xilinx_pcie_msi_chip.dev = port->dev;
-	hw.msi_ctrl = &xilinx_pcie_msi_chip;
 #endif
 	pci_common_init_dev(dev, &hw);

Quoted 4 lines

Quoted 3 lines
Signature · 1 line
2.2.1

Bar the nit above:

Acked-by Marc Zyngier <[email protected]>

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Lorenzo Pieralisi

author

2015-07-22 12:48 UTC #3 lore raw

PATCH 1/1 ARM: pci: kill pcibios_msi_controller

Hi Marc,

Quoted 41 lines · Marc Zyngier

Quoted 35 lines · Lorenzo Pieralisi
Quoted 26 lines · Lorenzo Pieralisi
On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the pcibios_msi_controller weak callback is used to look-up the msi_controller pointer, through pci_sys_data msi_ctrl pointer.
pci_sys_data is an arch specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.
Since the struct pci_bus already contains an msi_controller pointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and relate pcibios/host bridges initialization routines so that the msi_controller pointer look-up can be carried out by PCI core code through the struct pci_bus msi pointer, removing the need for arch specific pcibios_msi_controller callback and the related pci_sys_data msi_ctrl pointer.
Signed-off-by Lorenzo Pieralisi <[email protected]>
Cc Pratyush Anand <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Jingoo Han <[email protected]>
Cc Bjorn Helgaas <[email protected]>
Cc Simon Horman <[email protected]>
Cc Russell King <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Michal Simek <[email protected]>
Cc Marc Zyngier <[email protected]>
Quoted 7 lines
---
Bjorn, all,
I am posting this patch to sound out if that's a reasonable approach. xgene implements MSI look-up this way at present and this would represent another step forward towards having common drivers for ARM/ARM64, comments and testing welcome.
Quoted 4 lines
Any comments/testing on this patch ? I do not have platforms with these host bridges handy (apart from an iMX6 Sabrelite) so I can't test on them (change is quite mechanical though), help on testing and feedback on the patch much appreciated.

Quoted 4 lines

Hi Lorenzo,
I can't test it (no relevant HW), but that seems like a very sensible approach to me (the less dependency on sysdata, the better).

Thanks !

Quoted 56 lines

One remark below:

Quoted 54 lines

 arch/arm/include/asm/mach/pci.h    |  3 ---
 arch/arm/kernel/bios32.c           | 35 +++++++++++++++++------------------
 drivers/pci/host/pcie-designware.c |  9 +++++++--
 drivers/pci/host/pcie-xilinx.c     | 12 ++++++++++--
 4 files changed, 34 insertions(+), 25 deletions(-)

arch/arm/include/asm/mach/pci.h +0 -3

index 28b9bb3..32abc0c 100644
@@ -42,9 +42,6 @@ struct hw_pci {
  * Per-controller structure
  */
 struct pci_sys_data {
-#ifdef CONFIG_PCI_MSI
-	struct msi_controller *msi_ctrl;
-#endif
 	struct list_head node;
 	int		busnr;		/* primary bus number			*/
 	u64		mem_offset;	/* bus->cpu memory mapping offset	*/

arch/arm/kernel/bios32.c +0 ~~-12~~

index fcbbbb1..c841b33 100644
@@ -18,15 +18,6 @@
 
 static int debug_pci;
 
-#ifdef CONFIG_PCI_MSI
-struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
-{
-	struct pci_sys_data *sysdata = dev->bus->sysdata;
-
-	return sysdata->msi_ctrl;
-}
-#endif
-
 /*
  * We can't use pci_get_device() here since we are
  * called from interrupt context.

@@ -462,9 +453,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		if (!sys)
 			panic("PCI: unable to allocate sys data!");
 
-#ifdef CONFIG_PCI_MSI
-		sys->msi_ctrl = hw->msi_ctrl;
-#endif
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;

@@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 				break;
 			}

Quoted 13 lines

Quoted 8 lines
arch/arm/kernel/bios32.c static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, · line 477
Quoted 8 lines
arch/arm/kernel/bios32.c +3 -4
-			if (hw->scan)
+			if (hw->scan) {
 				sys->bus = hw->scan(nr, sys);
-			else
-				sys->bus = pci_scan_root_bus(parent, sys->busnr,
-						hw->ops, sys, &sys->resources);
+				if (!sys->bus)
+					panic("PCI: unable to scan bus!");
This was in the original code, but I have to ask: Do we really want to panic the kernel if we couldn't scan the bus? Worse case, the system won't be able to boot at all and will panic somewhere else anyway, but we should give the user a chance to understand what's happening...

No, it was in the original code but I was very tempted to remove it or merge the error paths and make it a warning, and that's what I am going to do, unless someone complains (that panic statement has been there forever).

Quoted 83 lines

arch/arm/kernel/bios32.c +14 -2

+			} else {
+				sys->bus = pci_create_root_bus(parent,
+							       sys->busnr,
+							       hw->ops, sys,
+							       &sys->resources);
+				if (WARN_ON(!sys->bus)) {
+					kfree(sys);
+					break;
+				}
+#ifdef CONFIG_PCI_MSI
+				sys->bus->msi = hw->msi_ctrl;
+#endif
 
-			if (!sys->bus)
-				panic("PCI: unable to scan bus!");
+				pci_scan_child_bus(sys->bus);
+			}
 
 			busnr = sys->bus->busn_res.end + 1;

drivers/pci/host/pcie-designware.c +7 -2

index 69486be..e584dfa 100644
@@ -526,7 +526,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 #ifdef CONFIG_PCI_MSI
 	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
 #endif
 
 	dw_pci.nr_controllers = 1;

@@ -708,11 +707,17 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pcie_port *pp = sys_to_pcie(sys);
 
 	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
+	bus = pci_create_root_bus(pp->dev, sys->busnr,
 				  &dw_pcie_ops, sys, &sys->resources);
 	if (!bus)
 		return NULL;
 
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &dw_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	if (bus && pp->ops->scan_bus)
 		pp->ops->scan_bus(pp);

drivers/pci/host/pcie-xilinx.c +10 -2

index f1a06a0..b21eb7d 100644
@@ -647,9 +647,18 @@ static struct pci_bus *xilinx_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pci_bus *bus;
 
 	port->root_busno = sys->busnr;
-	bus = pci_scan_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
+	bus = pci_create_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
 				sys, &sys->resources);
 
+	if (!bus)
+		return NULL;
+
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &xilinx_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	return bus;
 }

@@ -847,7 +856,6 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_PCI_MSI
 	xilinx_pcie_msi_chip.dev = port->dev;
-	hw.msi_ctrl = &xilinx_pcie_msi_chip;
 #endif
 	pci_common_init_dev(dev, &hw);

Quoted 8 lines

Quoted 4 lines
Quoted 3 lines
Signature · 1 line
2.2.1
Bar the nit above:
Acked-by Marc Zyngier <[email protected]>

Thanks ! I will prepare a v2 with required changes.

Lorenzo

Bjorn Helgaas

maintainer

2015-07-23 15:10 UTC #4 lore raw

PATCH 1/1 ARM: pci: kill pcibios_msi_controller

Quoted 18 lines · Lorenzo Pieralisi

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
(quoted diff) +0 -0
@@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 				break;
 			}
 
Quoted 13 lines
Quoted 8 lines
arch/arm/kernel/bios32.c static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, · line 477
Quoted 8 lines
arch/arm/kernel/bios32.c +3 -4
-			if (hw->scan)
+			if (hw->scan) {
 				sys->bus = hw->scan(nr, sys);
-			else
-				sys->bus = pci_scan_root_bus(parent, sys->busnr,
-						hw->ops, sys, &sys->resources);
+				if (!sys->bus)
+					panic("PCI: unable to scan bus!");
This was in the original code, but I have to ask: Do we really want to panic the kernel if we couldn't scan the bus? Worse case, the system won't be able to boot at all and will panic somewhere else anyway, but we should give the user a chance to understand what's happening...

Quoted 4 lines

No, it was in the original code but I was very tempted to remove it or merge the error paths and make it a warning, and that's what I am going to do, unless someone complains (that panic statement has been there forever).

I agree the panic should be removed. I would do it in a separate patch since it's not related to your main objective here.

Bjorn

Lorenzo Pieralisi

author

2015-07-23 17:14 UTC #5 lore raw

PATCH 1/1 ARM: pci: kill pcibios_msi_controller

Quoted 20 lines · Bjorn Helgaas

Quoted 18 lines · Lorenzo Pieralisi
Quoted 17 lines
Quoted 4 lines
Quoted 4 lines
(quoted diff) +0 -0
@@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 				break;
 			}
 
Quoted 8 lines
arch/arm/kernel/bios32.c static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, · line 477
Quoted 8 lines
arch/arm/kernel/bios32.c +3 -4
-			if (hw->scan)
+			if (hw->scan) {
 				sys->bus = hw->scan(nr, sys);
-			else
-				sys->bus = pci_scan_root_bus(parent, sys->busnr,
-						hw->ops, sys, &sys->resources);
+				if (!sys->bus)
+					panic("PCI: unable to scan bus!");
This was in the original code, but I have to ask: Do we really want to panic the kernel if we couldn't scan the bus? Worse case, the system won't be able to boot at all and will panic somewhere else anyway, but we should give the user a chance to understand what's happening...

Quoted 7 lines

Quoted 4 lines
No, it was in the original code but I was very tempted to remove it or merge the error paths and make it a warning, and that's what I am going to do, unless someone complains (that panic statement has been there forever).
I agree the panic should be removed. I would do it in a separate patch since it's not related to your main objective here.

Ok, I will post v2 shortly.

Thanks !
Lorenzo

Zhou Wang

2015-07-22 09:30 UTC #6 lore raw

PATCH 1/1 ARM: pci: kill pcibios_msi_controller

In reply to Lorenzo Pieralisi

Quoted 176 lines · Lorenzo Pieralisi

pci_sys_data is an arch specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Jingoo Han <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Simon Horman <[email protected]>

Cc Russell King <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Thierry Reding <[email protected]>

Cc Michal Simek <[email protected]>

Cc Marc Zyngier <[email protected]>

---
Bjorn, all,

Thanks,
Lorenzo

 arch/arm/include/asm/mach/pci.h    |  3 ---
 arch/arm/kernel/bios32.c           | 35 +++++++++++++++++------------------
 drivers/pci/host/pcie-designware.c |  9 +++++++--
 drivers/pci/host/pcie-xilinx.c     | 12 ++++++++++--
 4 files changed, 34 insertions(+), 25 deletions(-)

arch/arm/include/asm/mach/pci.h +0 -3

index 28b9bb3..32abc0c 100644
@@ -42,9 +42,6 @@ struct hw_pci {
  * Per-controller structure
  */
 struct pci_sys_data {
-#ifdef CONFIG_PCI_MSI
-	struct msi_controller *msi_ctrl;
-#endif
 	struct list_head node;
 	int		busnr;		/* primary bus number			*/
 	u64		mem_offset;	/* bus->cpu memory mapping offset	*/

arch/arm/kernel/bios32.c +17 ~~-18~~

index fcbbbb1..c841b33 100644
@@ -18,15 +18,6 @@
 
 static int debug_pci;
 
-#ifdef CONFIG_PCI_MSI
-struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
-{
-	struct pci_sys_data *sysdata = dev->bus->sysdata;
-
-	return sysdata->msi_ctrl;
-}
-#endif
-
 /*
  * We can't use pci_get_device() here since we are
  * called from interrupt context.

@@ -462,9 +453,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		if (!sys)
 			panic("PCI: unable to allocate sys data!");
 
-#ifdef CONFIG_PCI_MSI
-		sys->msi_ctrl = hw->msi_ctrl;
-#endif
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;

@@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 				break;
 			}
 
-			if (hw->scan)
+			if (hw->scan) {
 				sys->bus = hw->scan(nr, sys);
-			else
-				sys->bus = pci_scan_root_bus(parent, sys->busnr,
-						hw->ops, sys, &sys->resources);
+				if (!sys->bus)
+					panic("PCI: unable to scan bus!");
+			} else {
+				sys->bus = pci_create_root_bus(parent,
+							       sys->busnr,
+							       hw->ops, sys,
+							       &sys->resources);
+				if (WARN_ON(!sys->bus)) {
+					kfree(sys);
+					break;
+				}
+#ifdef CONFIG_PCI_MSI
+				sys->bus->msi = hw->msi_ctrl;
+#endif
 
-			if (!sys->bus)
-				panic("PCI: unable to scan bus!");
+				pci_scan_child_bus(sys->bus);
+			}
 
 			busnr = sys->bus->busn_res.end + 1;

drivers/pci/host/pcie-designware.c +7 -2

index 69486be..e584dfa 100644
@@ -526,7 +526,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 #ifdef CONFIG_PCI_MSI
 	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
 #endif
 
 	dw_pci.nr_controllers = 1;

@@ -708,11 +707,17 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pcie_port *pp = sys_to_pcie(sys);
 
 	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
+	bus = pci_create_root_bus(pp->dev, sys->busnr,
 				  &dw_pcie_ops, sys, &sys->resources);
 	if (!bus)
 		return NULL;
 
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &dw_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	if (bus && pp->ops->scan_bus)
 		pp->ops->scan_bus(pp);

drivers/pci/host/pcie-xilinx.c +10 -1

index f1a06a0..b21eb7d 100644
@@ -647,9 +647,18 @@ static struct pci_bus *xilinx_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pci_bus *bus;
 
 	port->root_busno = sys->busnr;
-	bus = pci_scan_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
+	bus = pci_create_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
 				sys, &sys->resources);
 
+	if (!bus)
+		return NULL;
+
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &xilinx_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	return bus;
 }

drivers/pci/host/pcie-xilinx.c static int xilinx_pcie_probe(struct platform_device *pdev) · line 861

Quoted 7 lines

drivers/pci/host/pcie-xilinx.c +0 -1

@@ -847,7 +856,6 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_PCI_MSI
 	xilinx_pcie_msi_chip.dev = port->dev;
-	hw.msi_ctrl = &xilinx_pcie_msi_chip;
 #endif
 	pci_common_init_dev(dev, &hw);

Hi Lorenzo,

I wonder if we can delete weak pcibios_msi_controller and the call in pci_msi_controller in drivers/pci/msi.c

Regards,
Zhou

Marc Zyngier

2015-07-22 10:15 UTC #7 lore raw

PATCH 1/1 ARM: pci: kill pcibios_msi_controller

Quoted 187 lines · Zhou Wang

Quoted 176 lines · Lorenzo Pieralisi

pci_sys_data is an arch specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Jingoo Han <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Simon Horman <[email protected]>

Cc Russell King <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Thierry Reding <[email protected]>

Cc Michal Simek <[email protected]>

Cc Marc Zyngier <[email protected]>

---
Bjorn, all,

Thanks,
Lorenzo

 arch/arm/include/asm/mach/pci.h    |  3 ---
 arch/arm/kernel/bios32.c           | 35 +++++++++++++++++------------------
 drivers/pci/host/pcie-designware.c |  9 +++++++--
 drivers/pci/host/pcie-xilinx.c     | 12 ++++++++++--
 4 files changed, 34 insertions(+), 25 deletions(-)

arch/arm/include/asm/mach/pci.h +0 -3

index 28b9bb3..32abc0c 100644
@@ -42,9 +42,6 @@ struct hw_pci {
  * Per-controller structure
  */
 struct pci_sys_data {
-#ifdef CONFIG_PCI_MSI
-	struct msi_controller *msi_ctrl;
-#endif
 	struct list_head node;
 	int		busnr;		/* primary bus number			*/
 	u64		mem_offset;	/* bus->cpu memory mapping offset	*/

arch/arm/kernel/bios32.c +17 ~~-18~~

index fcbbbb1..c841b33 100644
@@ -18,15 +18,6 @@
 
 static int debug_pci;
 
-#ifdef CONFIG_PCI_MSI
-struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
-{
-	struct pci_sys_data *sysdata = dev->bus->sysdata;
-
-	return sysdata->msi_ctrl;
-}
-#endif
-
 /*
  * We can't use pci_get_device() here since we are
  * called from interrupt context.

@@ -462,9 +453,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		if (!sys)
 			panic("PCI: unable to allocate sys data!");
 
-#ifdef CONFIG_PCI_MSI
-		sys->msi_ctrl = hw->msi_ctrl;
-#endif
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;

@@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 				break;
 			}
 
-			if (hw->scan)
+			if (hw->scan) {
 				sys->bus = hw->scan(nr, sys);
-			else
-				sys->bus = pci_scan_root_bus(parent, sys->busnr,
-						hw->ops, sys, &sys->resources);
+				if (!sys->bus)
+					panic("PCI: unable to scan bus!");
+			} else {
+				sys->bus = pci_create_root_bus(parent,
+							       sys->busnr,
+							       hw->ops, sys,
+							       &sys->resources);
+				if (WARN_ON(!sys->bus)) {
+					kfree(sys);
+					break;
+				}
+#ifdef CONFIG_PCI_MSI
+				sys->bus->msi = hw->msi_ctrl;
+#endif
 
-			if (!sys->bus)
-				panic("PCI: unable to scan bus!");
+				pci_scan_child_bus(sys->bus);
+			}
 
 			busnr = sys->bus->busn_res.end + 1;

drivers/pci/host/pcie-designware.c +7 -2

index 69486be..e584dfa 100644
@@ -526,7 +526,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 #ifdef CONFIG_PCI_MSI
 	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
 #endif
 
 	dw_pci.nr_controllers = 1;

@@ -708,11 +707,17 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pcie_port *pp = sys_to_pcie(sys);
 
 	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
+	bus = pci_create_root_bus(pp->dev, sys->busnr,
 				  &dw_pcie_ops, sys, &sys->resources);
 	if (!bus)
 		return NULL;
 
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &dw_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	if (bus && pp->ops->scan_bus)
 		pp->ops->scan_bus(pp);

drivers/pci/host/pcie-xilinx.c +10 -1

index f1a06a0..b21eb7d 100644
@@ -647,9 +647,18 @@ static struct pci_bus *xilinx_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pci_bus *bus;
 
 	port->root_busno = sys->busnr;
-	bus = pci_scan_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
+	bus = pci_create_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
 				sys, &sys->resources);
 
+	if (!bus)
+		return NULL;
+
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &xilinx_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	return bus;
 }

drivers/pci/host/pcie-xilinx.c static int xilinx_pcie_probe(struct platform_device *pdev) · line 861

Quoted 7 lines

drivers/pci/host/pcie-xilinx.c +0 -1

@@ -847,7 +856,6 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_PCI_MSI
 	xilinx_pcie_msi_chip.dev = port->dev;
-	hw.msi_ctrl = &xilinx_pcie_msi_chip;
 #endif
 	pci_common_init_dev(dev, &hw);

Hi Lorenzo,

Quoted 2 lines

I wonder if we can delete weak pcibios_msi_controller and the call in pci_msi_controller in drivers/pci/msi.c

Yeah, I think we can replace it with a straight "return NULL;" in pci_msi_controller, as ARM is the only arch to implement this function (which Lorenzo now removes).

Thanks,

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Lorenzo Pieralisi

author

2015-07-22 12:51 UTC #8 lore raw

PATCH 1/1 ARM: pci: kill pcibios_msi_controller

Quoted 188 lines · Marc Zyngier

Quoted 187 lines · Zhou Wang

Quoted 176 lines · Lorenzo Pieralisi

pci_sys_data is an arch specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.

Signed-off-by Lorenzo Pieralisi <[email protected]>

Cc Pratyush Anand <[email protected]>

Cc Arnd Bergmann <[email protected]>

Cc Jingoo Han <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Simon Horman <[email protected]>

Cc Russell King <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Thierry Reding <[email protected]>

Cc Michal Simek <[email protected]>

Cc Marc Zyngier <[email protected]>

---
Bjorn, all,

Thanks,
Lorenzo

 arch/arm/include/asm/mach/pci.h    |  3 ---
 arch/arm/kernel/bios32.c           | 35 +++++++++++++++++------------------
 drivers/pci/host/pcie-designware.c |  9 +++++++--
 drivers/pci/host/pcie-xilinx.c     | 12 ++++++++++--
 4 files changed, 34 insertions(+), 25 deletions(-)

arch/arm/include/asm/mach/pci.h +0 -3

index 28b9bb3..32abc0c 100644
@@ -42,9 +42,6 @@ struct hw_pci {
  * Per-controller structure
  */
 struct pci_sys_data {
-#ifdef CONFIG_PCI_MSI
-	struct msi_controller *msi_ctrl;
-#endif
 	struct list_head node;
 	int		busnr;		/* primary bus number			*/
 	u64		mem_offset;	/* bus->cpu memory mapping offset	*/

arch/arm/kernel/bios32.c +17 ~~-18~~

index fcbbbb1..c841b33 100644
@@ -18,15 +18,6 @@
 
 static int debug_pci;
 
-#ifdef CONFIG_PCI_MSI
-struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
-{
-	struct pci_sys_data *sysdata = dev->bus->sysdata;
-
-	return sysdata->msi_ctrl;
-}
-#endif
-
 /*
  * We can't use pci_get_device() here since we are
  * called from interrupt context.

@@ -462,9 +453,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		if (!sys)
 			panic("PCI: unable to allocate sys data!");
 
-#ifdef CONFIG_PCI_MSI
-		sys->msi_ctrl = hw->msi_ctrl;
-#endif
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;

@@ -483,14 +471,25 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 				break;
 			}
 
-			if (hw->scan)
+			if (hw->scan) {
 				sys->bus = hw->scan(nr, sys);
-			else
-				sys->bus = pci_scan_root_bus(parent, sys->busnr,
-						hw->ops, sys, &sys->resources);
+				if (!sys->bus)
+					panic("PCI: unable to scan bus!");
+			} else {
+				sys->bus = pci_create_root_bus(parent,
+							       sys->busnr,
+							       hw->ops, sys,
+							       &sys->resources);
+				if (WARN_ON(!sys->bus)) {
+					kfree(sys);
+					break;
+				}
+#ifdef CONFIG_PCI_MSI
+				sys->bus->msi = hw->msi_ctrl;
+#endif
 
-			if (!sys->bus)
-				panic("PCI: unable to scan bus!");
+				pci_scan_child_bus(sys->bus);
+			}
 
 			busnr = sys->bus->busn_res.end + 1;

drivers/pci/host/pcie-designware.c +7 -2

index 69486be..e584dfa 100644
@@ -526,7 +526,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
 
 #ifdef CONFIG_PCI_MSI
 	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
 #endif
 
 	dw_pci.nr_controllers = 1;

@@ -708,11 +707,17 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pcie_port *pp = sys_to_pcie(sys);
 
 	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
+	bus = pci_create_root_bus(pp->dev, sys->busnr,
 				  &dw_pcie_ops, sys, &sys->resources);
 	if (!bus)
 		return NULL;
 
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &dw_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	if (bus && pp->ops->scan_bus)
 		pp->ops->scan_bus(pp);

drivers/pci/host/pcie-xilinx.c +10 -1

index f1a06a0..b21eb7d 100644
@@ -647,9 +647,18 @@ static struct pci_bus *xilinx_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pci_bus *bus;
 
 	port->root_busno = sys->busnr;
-	bus = pci_scan_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
+	bus = pci_create_root_bus(port->dev, sys->busnr, &xilinx_pcie_ops,
 				sys, &sys->resources);
 
+	if (!bus)
+		return NULL;
+
+#ifdef CONFIG_PCI_MSI
+	bus->msi = &xilinx_pcie_msi_chip;
+#endif
+
+	pci_scan_child_bus(bus);
+
 	return bus;
 }

drivers/pci/host/pcie-xilinx.c static int xilinx_pcie_probe(struct platform_device *pdev) · line 861

Quoted 7 lines

drivers/pci/host/pcie-xilinx.c +0 -1

@@ -847,7 +856,6 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_PCI_MSI
 	xilinx_pcie_msi_chip.dev = port->dev;
-	hw.msi_ctrl = &xilinx_pcie_msi_chip;
 #endif
 	pci_common_init_dev(dev, &hw);

Hi Lorenzo,

Quoted 6 lines

Quoted 2 lines
I wonder if we can delete weak pcibios_msi_controller and the call in pci_msi_controller in drivers/pci/msi.c
Yeah, I think we can replace it with a straight "return NULL;" in pci_msi_controller, as ARM is the only arch to implement this function (which Lorenzo now removes).

Agreed, I did not do it since I wanted to get feedback on the approach first, if we agree that's the right thing to do we can kill pcibios_msi_controller() in the core too (since it becomes unused).

Lorenzo