ARM: PCI: bios32: replace panic with WARN messages on failures
Subsystems (3)
PATCH v2 1/2 ARM: PCI: bios32: replace panic with WARN messages on failures
In the ARM PCI bios32 layer, failures to dynamically allocate pci_sys_data for a PCI bus, or a PCI bus scan failure have to be considered serious warnings but they should not trigger a system panic so that at least the system is given a chance to be debugged.
This patch replaces the panic statements with WARN() messages to improve error reporting in the ARM PCI bios32 layer.
--- arch/arm/kernel/bios32.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
arch/arm/kernel/bios32.c +6 -4
@@ -459,8 +459,8 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,for (nr = busnr =0 ; nr < hw->nr_controllers; nr++) { sys =kzalloc (sizeof (struct pci_sys_data ),GFP_KERNEL ); -if (!sys) -panic ("PCI: unable to allocate sys data!" ); +if (WARN (!sys,"PCI: unable to allocate sys data!" )) +break ;#ifdef CONFIG_PCI_MSI sys->msi_ctrl = hw->msi_ctrl;
@@ -489,8 +489,10 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, sys->bus =pci_scan_root_bus (parent, sys->busnr, hw->ops, sys, &sys->resources); -if (!sys->bus) -panic ("PCI: unable to scan bus!" ); +if (WARN (!sys->bus,"PCI: unable to scan bus!" )) { +kfree (sys); +break ; + } busnr = sys->bus->busn_res.end +1 ;
Signature · 1 line
2.2.1
PATCH v2 2/2 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 ARM 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 related 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 the arch specific pcibios_msi_controller callback and the related pci_sys_data msi_ctrl pointer.
ARM is the only arch relying on the pcibios_msi_controller() weak function, hence this patch removes the default weak implementation from PCI core code since it becomes of no use.
---v1->v2
- Added patch to replace panic statements with WARN
- Removed unused pcibios_msi_controller() and pci_msi_controller() from core code
- Dropped RFT status
v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356028.html
arch/arm/include/asm/mach/pci.h | 3 --- arch/arm/kernel/bios32.c | 29 +++++++++++++---------------- drivers/pci/host/pcie-designware.c | 9 +++++++-- drivers/pci/host/pcie-xilinx.c | 12 ++++++++++-- drivers/pci/msi.c | 17 +---------------- 5 files changed, 31 insertions(+), 39 deletions(-)
arch/arm/include/asm/mach/pci.h +0 -3
@@ -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 +13 -16
@@ -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 (WARN (!sys,"PCI: unable to allocate sys data!" ))break ; -#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,11 +471,20 @@ 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); + }else { + sys->bus =pci_create_root_bus (parent, + sys->busnr, + hw->ops, sys, + &sys->resources); +if (sys->bus) { +#ifdef CONFIG_PCI_MSI + sys->bus->msi = hw->msi_ctrl; +#endif +pci_scan_child_bus (sys->bus); + } + }if (WARN (!sys->bus,"PCI: unable to scan bus!" )) {kfree (sys);
drivers/pci/host/pcie-designware.c +7 -2
@@ -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
@@ -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);
drivers/pci/msi.c +1 -16
@@ -77,24 +77,9 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)/* Arch hooks */ -struct msi_controller *__weak pcibios_msi_controller (struct pci_dev *dev) -{ -return NULL ; -} - -static struct msi_controller *pci_msi_controller (struct pci_dev *dev) -{ -struct msi_controller *msi_ctrl = dev->bus->msi; - -if (msi_ctrl) -return msi_ctrl; - -return pcibios_msi_controller (dev); -} -int __weak arch_setup_msi_irq (struct pci_dev *dev,struct msi_desc *desc) { -struct msi_controller *chip =pci_msi_controller (dev); +struct msi_controller *chip = dev->bus->msi;int err;if (!chip || !chip->setup_irq)
Signature · 1 line
2.2.1
PATCH v2 2/2 ARM: pci: kill pcibios_msi_controller
Quoted 48 lines · Lorenzo Pieralisi
On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the
pcibios_msi_controllerweak callback is used to look-up themsi_controllerpointer, throughpci_sys_datamsi_ctrlpointer.
pci_sys_datais an ARM specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.Since the struct
pci_busalready contains anmsi_controllerpointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and related pcibios/host bridges initialization routines so that themsi_controllerpointer look-up can be carried out by PCI core code through the structpci_busmsi pointer, removing the need for the arch specificpcibios_msi_controllercallback and the relatedpci_sys_datamsi_ctrlpointer.ARM is the only arch relying on the
pcibios_msi_controller()weak function, hence this patch removes the default weak implementation from PCI core code since it becomes of no use.Signed-off-by Lorenzo Pieralisi <[email protected]>Acked-by Marc Zyngier <[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]>---
v1->v2- Added patch to replace panic statements with WARN
- Removed unusedpcibios_msi_controller()andpci_msi_controller()from core code
- Dropped RFT statusv1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356028.html
arch/arm/include/asm/mach/pci.h | 3 --- arch/arm/kernel/bios32.c | 29 +++++++++++++---------------- drivers/pci/host/pcie-designware.c | 9 +++++++-- drivers/pci/host/pcie-xilinx.c | 12 ++++++++++-- drivers/pci/msi.c | 17 +---------------- 5 files changed, 31 insertions(+), 39 deletions(-)
[...]
Quoted 20 lines
drivers/pci/msi.c +0
-12@@ -77,24 +77,9 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)/* Arch hooks */ -struct msi_controller *__weak pcibios_msi_controller (struct pci_dev *dev) -{ -return NULL ; -} - -static struct msi_controller *pci_msi_controller (struct pci_dev *dev) -{ -struct msi_controller *msi_ctrl = dev->bus->msi; - -if (msi_ctrl) -return msi_ctrl; -
drivers/pci/msi.c static void pci_msi_teardown_msi_irqs(struct pci_dev *dev) · line 85
Quoted 10 lines
drivers/pci/msi.c +1
-4-return pcibios_msi_controller (dev); -} -int __weak arch_setup_msi_irq (struct pci_dev *dev,struct msi_desc *desc) { -struct msi_controller *chip =pci_msi_controller (dev); +struct msi_controller *chip = dev->bus->msi;int err;if (!chip || !chip->setup_irq)
Don't you have to go to the top level bus and get the ->msi pointer? Something like:
for (bus = dev->bus; bus != NULL; bus = bus->parent) if (bus->msi) return bus->msi;
I have not been following this closely, so I may have missed some patches.
JC.
PATCH v2 2/2 ARM: pci: kill pcibios_msi_controller
Quoted 84 lines · Jayachandran C.
Quoted 48 lines · Lorenzo Pieralisi
On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the
pcibios_msi_controllerweak callback is used to look-up themsi_controllerpointer, throughpci_sys_datamsi_ctrlpointer.
pci_sys_datais an ARM specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.Since the struct
pci_busalready contains anmsi_controllerpointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and related pcibios/host bridges initialization routines so that themsi_controllerpointer look-up can be carried out by PCI core code through the structpci_busmsi pointer, removing the need for the arch specificpcibios_msi_controllercallback and the relatedpci_sys_datamsi_ctrlpointer.ARM is the only arch relying on the
pcibios_msi_controller()weak function, hence this patch removes the default weak implementation from PCI core code since it becomes of no use.Signed-off-by Lorenzo Pieralisi <[email protected]>Acked-by Marc Zyngier <[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]>---
v1->v2- Added patch to replace panic statements with WARN
- Removed unusedpcibios_msi_controller()andpci_msi_controller()from core code
- Dropped RFT statusv1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356028.html
arch/arm/include/asm/mach/pci.h | 3 --- arch/arm/kernel/bios32.c | 29 +++++++++++++---------------- drivers/pci/host/pcie-designware.c | 9 +++++++-- drivers/pci/host/pcie-xilinx.c | 12 ++++++++++-- drivers/pci/msi.c | 17 +---------------- 5 files changed, 31 insertions(+), 39 deletions(-)[...]
Quoted 20 lines
drivers/pci/msi.c +0
-12@@ -77,24 +77,9 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)/* Arch hooks */ -struct msi_controller *__weak pcibios_msi_controller (struct pci_dev *dev) -{ -return NULL ; -} - -static struct msi_controller *pci_msi_controller (struct pci_dev *dev) -{ -struct msi_controller *msi_ctrl = dev->bus->msi; - -if (msi_ctrl) -return msi_ctrl; -drivers/pci/msi.c static void pci_msi_teardown_msi_irqs(struct pci_dev *dev) · line 85
Quoted 10 lines
drivers/pci/msi.c +1
-4-return pcibios_msi_controller (dev); -} -int __weak arch_setup_msi_irq (struct pci_dev *dev,struct msi_desc *desc) { -struct msi_controller *chip =pci_msi_controller (dev); +struct msi_controller *chip = dev->bus->msi;int err;if (!chip || !chip->setup_irq)Don't you have to go to the top level bus and get the ->msi pointer? Something like:
Quoted 5 lines
for (bus = dev->bus; bus != NULL; bus = bus->parent) if (bus->msi) return bus->msi;I have not been following this closely, so I may have missed some patches.
The msi pointer is initialized from parent to child in pci_alloc_child_bus(), so PCI core does that for us, that's my understanding.
It works the same as sysdata pointer, which is why pcibios_msi_controller works in the current kernel.
On a side note, are you able to prepare a new version of your set to enable the PCI generic host on ARM64 (and remove pci_sys_data and related ifdef CONFIG_ARM from it) ?
We are not far from removing the pci_sys_data dependency, if you can't prepare a new version of your series let me know I can do it on your behalf.
Thanks !
Lorenzo
PATCH v2 2/2 ARM: pci: kill pcibios_msi_controller
Quoted 89 lines · Lorenzo Pieralisi
Quoted 88 lines · Jayachandran C.
Quoted 48 lines · Lorenzo Pieralisi
On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the
pcibios_msi_controllerweak callback is used to look-up themsi_controllerpointer, throughpci_sys_datamsi_ctrlpointer.
pci_sys_datais an ARM specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.Since the struct
pci_busalready contains anmsi_controllerpointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and related pcibios/host bridges initialization routines so that themsi_controllerpointer look-up can be carried out by PCI core code through the structpci_busmsi pointer, removing the need for the arch specificpcibios_msi_controllercallback and the relatedpci_sys_datamsi_ctrlpointer.ARM is the only arch relying on the
pcibios_msi_controller()weak function, hence this patch removes the default weak implementation from PCI core code since it becomes of no use.Signed-off-by Lorenzo Pieralisi <[email protected]>Acked-by Marc Zyngier <[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]>---
v1->v2- Added patch to replace panic statements with WARN
- Removed unusedpcibios_msi_controller()andpci_msi_controller()from core code
- Dropped RFT statusv1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356028.html
arch/arm/include/asm/mach/pci.h | 3 --- arch/arm/kernel/bios32.c | 29 +++++++++++++---------------- drivers/pci/host/pcie-designware.c | 9 +++++++-- drivers/pci/host/pcie-xilinx.c | 12 ++++++++++-- drivers/pci/msi.c | 17 +---------------- 5 files changed, 31 insertions(+), 39 deletions(-)[...]
Quoted 20 lines
drivers/pci/msi.c +0
-12@@ -77,24 +77,9 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)/* Arch hooks */ -struct msi_controller *__weak pcibios_msi_controller (struct pci_dev *dev) -{ -return NULL ; -} - -static struct msi_controller *pci_msi_controller (struct pci_dev *dev) -{ -struct msi_controller *msi_ctrl = dev->bus->msi; - -if (msi_ctrl) -return msi_ctrl; -drivers/pci/msi.c static void pci_msi_teardown_msi_irqs(struct pci_dev *dev) · line 85
Quoted 10 lines
drivers/pci/msi.c +1
-4-return pcibios_msi_controller (dev); -} -int __weak arch_setup_msi_irq (struct pci_dev *dev,struct msi_desc *desc) { -struct msi_controller *chip =pci_msi_controller (dev); +struct msi_controller *chip = dev->bus->msi;int err;if (!chip || !chip->setup_irq)Don't you have to go to the top level bus and get the ->msi pointer? Something like:
for (bus = dev->bus; bus != NULL; bus = bus->parent) if (bus->msi) return bus->msi;
Quoted 8 lines
Quoted 1 line
I have not been following this closely, so I may have missed some patches.
The msi pointer is initialized from parent to child in
pci_alloc_child_bus(), so PCI core does that for us, that's my understanding.It works the same as sysdata pointer, which is why
pcibios_msi_controllerworks in the current kernel.
Thanks.
Quoted 8 lines
On a side note, are you able to prepare a new version of your set to enable the PCI generic host on ARM64 (and remove
pci_sys_dataand related ifdefCONFIG_ARMfrom it) ?We are not far from removing the
pci_sys_datadependency, if you can't prepare a new version of your series let me know I can do it on your behalf.
I will post an updated patch with a minor fix. I will also add a patch for parsing msi-parent and setting ->msi on the root bus.
JC.
PATCH v2 2/2 ARM: pci: kill pcibios_msi_controller
Quoted 106 lines · Jayachandran C.
Quoted 89 lines · Lorenzo Pieralisi
Quoted 88 lines · Jayachandran C.
Quoted 48 lines · Lorenzo Pieralisi
On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the
pcibios_msi_controllerweak callback is used to look-up themsi_controllerpointer, throughpci_sys_datamsi_ctrlpointer.
pci_sys_datais an ARM specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.Since the struct
pci_busalready contains anmsi_controllerpointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and related pcibios/host bridges initialization routines so that themsi_controllerpointer look-up can be carried out by PCI core code through the structpci_busmsi pointer, removing the need for the arch specificpcibios_msi_controllercallback and the relatedpci_sys_datamsi_ctrlpointer.ARM is the only arch relying on the
pcibios_msi_controller()weak function, hence this patch removes the default weak implementation from PCI core code since it becomes of no use.Signed-off-by Lorenzo Pieralisi <[email protected]>Acked-by Marc Zyngier <[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]>---
v1->v2- Added patch to replace panic statements with WARN
- Removed unusedpcibios_msi_controller()andpci_msi_controller()from core code
- Dropped RFT statusv1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356028.html
arch/arm/include/asm/mach/pci.h | 3 --- arch/arm/kernel/bios32.c | 29 +++++++++++++---------------- drivers/pci/host/pcie-designware.c | 9 +++++++-- drivers/pci/host/pcie-xilinx.c | 12 ++++++++++-- drivers/pci/msi.c | 17 +---------------- 5 files changed, 31 insertions(+), 39 deletions(-)[...]
Quoted 20 lines
drivers/pci/msi.c +0
-12@@ -77,24 +77,9 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)/* Arch hooks */ -struct msi_controller *__weak pcibios_msi_controller (struct pci_dev *dev) -{ -return NULL ; -} - -static struct msi_controller *pci_msi_controller (struct pci_dev *dev) -{ -struct msi_controller *msi_ctrl = dev->bus->msi; - -if (msi_ctrl) -return msi_ctrl; -drivers/pci/msi.c static void pci_msi_teardown_msi_irqs(struct pci_dev *dev) · line 85
Quoted 10 lines
drivers/pci/msi.c +1
-4-return pcibios_msi_controller (dev); -} -int __weak arch_setup_msi_irq (struct pci_dev *dev,struct msi_desc *desc) { -struct msi_controller *chip =pci_msi_controller (dev); +struct msi_controller *chip = dev->bus->msi;int err;if (!chip || !chip->setup_irq)Don't you have to go to the top level bus and get the ->msi pointer? Something like:
for (bus = dev->bus; bus != NULL; bus = bus->parent) if (bus->msi) return bus->msi;Quoted 8 lines
Quoted 1 line
I have not been following this closely, so I may have missed some patches.
The msi pointer is initialized from parent to child in
pci_alloc_child_bus(), so PCI core does that for us, that's my understanding.It works the same as sysdata pointer, which is why
pcibios_msi_controllerworks in the current kernel.Thanks.
Quoted 5 lines
On a side note, are you able to prepare a new version of your set to enable the PCI generic host on ARM64 (and remove
pci_sys_dataand related ifdefCONFIG_ARMfrom it) ?
Quoted 7 lines
Quoted 3 lines
We are not far from removing the
pci_sys_datadependency, if you can't prepare a new version of your series let me know I can do it on your behalf.I will post an updated patch with a minor fix. I will also add a patch for parsing msi-parent and setting ->msi on the root bus.
There is no need for any msi parsing in the PCI generic host, Marc implemented mechanism for us in core code:
https://lkml.org/lkml/2015/7/7/724
Thanks,
Lorenzo
PATCH v2 2/2 ARM: pci: kill pcibios_msi_controller
In reply to Jayachandran C.Quoted 106 lines · Jayachandran C.
Quoted 89 lines · Lorenzo Pieralisi
Quoted 88 lines · Jayachandran C.
Quoted 48 lines · Lorenzo Pieralisi
On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the
pcibios_msi_controllerweak callback is used to look-up themsi_controllerpointer, throughpci_sys_datamsi_ctrlpointer.
pci_sys_datais an ARM specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.Since the struct
pci_busalready contains anmsi_controllerpointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and related pcibios/host bridges initialization routines so that themsi_controllerpointer look-up can be carried out by PCI core code through the structpci_busmsi pointer, removing the need for the arch specificpcibios_msi_controllercallback and the relatedpci_sys_datamsi_ctrlpointer.ARM is the only arch relying on the
pcibios_msi_controller()weak function, hence this patch removes the default weak implementation from PCI core code since it becomes of no use.Signed-off-by Lorenzo Pieralisi <[email protected]>Acked-by Marc Zyngier <[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]>---
v1->v2- Added patch to replace panic statements with WARN
- Removed unusedpcibios_msi_controller()andpci_msi_controller()from core code
- Dropped RFT statusv1: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/356028.html
arch/arm/include/asm/mach/pci.h | 3 --- arch/arm/kernel/bios32.c | 29 +++++++++++++---------------- drivers/pci/host/pcie-designware.c | 9 +++++++-- drivers/pci/host/pcie-xilinx.c | 12 ++++++++++-- drivers/pci/msi.c | 17 +---------------- 5 files changed, 31 insertions(+), 39 deletions(-)[...]
Quoted 20 lines
drivers/pci/msi.c +0
-12@@ -77,24 +77,9 @@ static void pci_msi_teardown_msi_irqs(struct pci_dev *dev)/* Arch hooks */ -struct msi_controller *__weak pcibios_msi_controller (struct pci_dev *dev) -{ -return NULL ; -} - -static struct msi_controller *pci_msi_controller (struct pci_dev *dev) -{ -struct msi_controller *msi_ctrl = dev->bus->msi; - -if (msi_ctrl) -return msi_ctrl; -drivers/pci/msi.c static void pci_msi_teardown_msi_irqs(struct pci_dev *dev) · line 85
Quoted 10 lines
drivers/pci/msi.c +1
-4-return pcibios_msi_controller (dev); -} -int __weak arch_setup_msi_irq (struct pci_dev *dev,struct msi_desc *desc) { -struct msi_controller *chip =pci_msi_controller (dev); +struct msi_controller *chip = dev->bus->msi;int err;if (!chip || !chip->setup_irq)Don't you have to go to the top level bus and get the ->msi pointer? Something like:
for (bus = dev->bus; bus != NULL; bus = bus->parent) if (bus->msi) return bus->msi;Quoted 8 lines
Quoted 1 line
I have not been following this closely, so I may have missed some patches.
The msi pointer is initialized from parent to child in
pci_alloc_child_bus(), so PCI core does that for us, that's my understanding.It works the same as sysdata pointer, which is why
pcibios_msi_controllerworks in the current kernel.Thanks.
Quoted 5 lines
On a side note, are you able to prepare a new version of your set to enable the PCI generic host on ARM64 (and remove
pci_sys_dataand related ifdefCONFIG_ARMfrom it) ?
Quoted 7 lines
Quoted 3 lines
We are not far from removing the
pci_sys_datadependency, if you can't prepare a new version of your series let me know I can do it on your behalf.I will post an updated patch with a minor fix. I will also add a patch for parsing msi-parent and setting ->msi on the root bus.
That shouldn't be necessary if you base your series on this one:
http://lwn.net/Articles/652151/
Patch 5 in this series does it for you as part of the core PCI code.
Thanks,
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH v2 2/2 ARM: pci: kill pcibios_msi_controller
In reply to Lorenzo PieralisiQuoted 15 lines · Lorenzo Pieralisi
On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the
pcibios_msi_controllerweak callback is used to look-up themsi_controllerpointer, throughpci_sys_datamsi_ctrlpointer.
pci_sys_datais an ARM specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.Since the struct
pci_busalready contains anmsi_controllerpointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and related pcibios/host bridges initialization routines so that themsi_controllerpointer look-up can be carried out by PCI core code through the structpci_busmsi pointer, removing the need for the arch specificpcibios_msi_controllercallback and the relatedpci_sys_datamsi_ctrlpointer.
Quoted 3 lines
ARM is the only arch relying on the
pcibios_msi_controller()weak function, hence this patch removes the default weak implementation from PCI core code since it becomes of no use.
You don't mention the change from using pci_scan_root_bus() to using pci_create_root_bus() here.
arch/arm/kernel/bios32.c static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, · line 480
Quoted 7 lines
arch/arm/kernel/bios32.c +5
-2- sys->bus =pci_scan_root_bus (parent, sys->busnr, - hw->ops, sys, &sys->resources); + }else { + sys->bus =pci_create_root_bus (parent, + sys->busnr, + hw->ops, sys, + &sys->resources);
By making this change, there is no nothing which will call pci_bus_insert_busn_res().
What about the 18 users of the ->scan method, at least IOP13xx appears to be MSI-enabled, though it's not clear whether it works with MSI.
This doesn't seem to be a good approach. Maybe having a version of pci_scan_root_bus() which takes the MSI data as an argument would be better than selectively copying pci_scan_root_bus() into the ARM code?
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH v2 2/2 ARM: pci: kill pcibios_msi_controller
Quoted 23 lines · Russell King - ARM Linux
Quoted 15 lines · Lorenzo Pieralisi
On ARM PCI systems relying on the pcibios API to initialize PCI host controllers, the
pcibios_msi_controllerweak callback is used to look-up themsi_controllerpointer, throughpci_sys_datamsi_ctrlpointer.
pci_sys_datais an ARM specific structure, which prevents using the same mechanism (so same PCI host controller drivers) on ARM64 systems.Since the struct
pci_busalready contains anmsi_controllerpointer and the kernel already uses it to look-up the msi controller, this patch converts ARM host controller and related pcibios/host bridges initialization routines so that themsi_controllerpointer look-up can be carried out by PCI core code through the structpci_busmsi pointer, removing the need for the arch specificpcibios_msi_controllercallback and the relatedpci_sys_datamsi_ctrlpointer.Quoted 3 lines
ARM is the only arch relying on the
pcibios_msi_controller()weak function, hence this patch removes the default weak implementation from PCI core code since it becomes of no use.You don't mention the change from using
pci_scan_root_bus()to usingpci_create_root_bus()here.
Quoted 10 lines
arch/arm/kernel/bios32.c static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, · line 480
Quoted 7 lines
arch/arm/kernel/bios32.c +5
-2- sys->bus =pci_scan_root_bus (parent, sys->busnr, - hw->ops, sys, &sys->resources); + }else { + sys->bus =pci_create_root_bus (parent, + sys->busnr, + hw->ops, sys, + &sys->resources);By making this change, there is no nothing which will call
pci_bus_insert_busn_res().
Yes, you are correct I noticed that too (I guess all ARM bridges relying on pci_scan_root_bus do NOT initialize the BUS resource so they rely on pci_scan_root_bus, and related BUS resource insertion to function properly).
Quoted 2 lines
What about the 18 users of the ->scan method, at least IOP13xx appears to be MSI-enabled, though it's not clear whether it works with MSI.
I do not think IOP13xx is affected by this change so we should be fine on that side, I should have chased all msi_ctrl users in this series, but the point raised above needs tackling regardless.
Quoted 3 lines
This doesn't seem to be a good approach. Maybe having a version of
pci_scan_root_bus()which takes the MSI data as an argument would be better than selectively copyingpci_scan_root_bus()into the ARM code?
I understand your point, let's try to find a fast way forward, we are stuck otherwise:
1) I do not touch bios32 at all. I convert all host bridges that require an msi pointer to use the scan pointer in struct hw_pci and basically implement the change in bios32 in their scan method (I did that already for the in tree host bridges that rely on the scan method and require an msi pointer in pci_bus to function)
2) I add pci_bus_insert_busn_res() in the bios32 code, therefore as you said literally copying core code into bios32 (+ msi pointer initialization), horrible, but I promise it will disappear as soon as we are done converting all ARM PCI host to the new IRQ MSI domain infrastructure, which do not require an msi pointer in the struct pci_bus structure to be populated at all.
3) I patch pci_scan_root_bus() to pass the MSI pointer. I think that pcibios_msi_controller was added to avoid doing this, that function has already 5 parameters and it is a treewide change, I suspect Bjorn won't be happy about this at all.
Those are the reasonable ideas I have in mind, I think (1) is the fastest (basically I _hope_ it should mean that I only have to patch drivers/pci/host/pcie-rcar.c) and I avoid patching ARM bios32.
Comments very welcome before I proceed and I would appreciate some feedback from PCI host bridges platform maintainers too to find a way forward.
Thanks,
Lorenzo
PATCH v2 2/2 ARM: pci: kill pcibios_msi_controller
Quoted 5 lines · Lorenzo Pieralisi
Quoted 3 lines · Russell King - ARM Linux
This doesn't seem to be a good approach. Maybe having a version of
pci_scan_root_bus()which takes the MSI data as an argument would be better than selectively copyingpci_scan_root_bus()into the ARM code?
Quoted 7 lines
I understand your point, let's try to find a fast way forward, we are stuck otherwise:
3) I patch
pci_scan_root_bus()to pass the MSI pointer. I think thatpcibios_msi_controllerwas added to avoid doing this, that function has already 5 parameters and it is a treewide change, I suspect Bjorn won't be happy about this at all.
Or you could use the suggestion I made in the paragraph you quoted above, which is a variation on your (3) without the down-sides of needing to change lots of callsites. Something like this (bios32.c is incomplete):
arch/arm/kernel/bios32.c | 8 +++----- drivers/pci/probe.c | 15 +++++++++++++-- include/linux/pci.h | 4 ++++ 3 files changed, 20 insertions(+), 7 deletions(-)
arch/arm/kernel/bios32.c +3 -5
@@ -462,9 +462,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;
@@ -486,8 +483,9 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,if (hw->scan) sys->bus = hw->scan (nr, sys);else - sys->bus =pci_scan_root_bus (parent, sys->busnr, - hw->ops, sys, &sys->resources); + sys->bus =pci_scan_root_bus_msi (parent, + sys->busnr, hw->ops, sys, + &sys->resources, hw->msi_ctrl);if (!sys->bus)panic ("PCI: unable to scan bus!" );
drivers/pci/probe.c +13 -2
@@ -2096,8 +2096,9 @@ void pci_bus_release_busn_res(struct pci_bus *b) res, ret ? "can not be" : "is"); } -struct pci_bus *pci_scan_root_bus (struct device *parent,int bus, -struct pci_ops *ops,void *sysdata,struct list_head *resources) +struct pci_bus *pci_scan_root_bus_msi (struct device *parent,int bus, +struct pci_ops *ops,void *sysdata, +struct list_head *resources,struct msi_controller *msi) {struct resource_entry *window;bool found = false;
@@ -2114,6 +2115,8 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,if (!b)return NULL ; + b->msi = msi; +if (!found) { dev_info(&b->dev, "No busn resource found for root bus, will use [bus %02x-ff]\n",
@@ -2128,6 +2131,14 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,return b; } +EXPORT_SYMBOL (pci_scan_root_bus_msi); + +struct pci_bus *pci_scan_root_bus (struct device *parent,int bus, +struct pci_ops *ops,void *sysdata,struct list_head *resources) +{ +return pci_scan_root_bus_msi (parent, bus, ops, sysdata, resources, +NULL ); +}EXPORT_SYMBOL (pci_scan_root_bus); struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,
include/linux/pci.h +4 -0
@@ -787,6 +787,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,int pci_bus_insert_busn_res (struct pci_bus *b,int bus,int busmax);int pci_bus_update_busn_res_end (struct pci_bus *b,int busmax);void pci_bus_release_busn_res (struct pci_bus *b); +struct pci_bus *pci_scan_root_bus_msi (struct device *parent,int bus, +struct pci_ops *ops,void *sysdata, +struct list_head *resources, +struct msi_controller *msi);struct pci_bus *pci_scan_root_bus (struct device *parent,int bus,struct pci_ops *ops,void *sysdata,struct list_head *resources);
And it's probably a good idea to kill off the ifdef around this:
struct hw_pci {
#ifdef CONFIG_PCI_MSI
struct msi_controller *msi_ctrl;#endif
so that we can avoid ifdefs in random places in arch/arm code (as is the decision in the rest of PCI for this.)
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH v2 2/2 ARM: pci: kill pcibios_msi_controller
Quoted 9 lines · Russell King - ARM Linux
Quoted 8 lines · Lorenzo Pieralisi
Quoted 3 lines · Russell King - ARM Linux
This doesn't seem to be a good approach. Maybe having a version of
pci_scan_root_bus()which takes the MSI data as an argument would be better than selectively copyingpci_scan_root_bus()into the ARM code?I understand your point, let's try to find a fast way forward, we are stuck otherwise:
Quoted 8 lines
Quoted 4 lines
3) I patch
pci_scan_root_bus()to pass the MSI pointer. I think thatpcibios_msi_controllerwas added to avoid doing this, that function has already 5 parameters and it is a treewide change, I suspect Bjorn won't be happy about this at all.Or you could use the suggestion I made in the paragraph you quoted above, which is a variation on your (3) without the down-sides of needing to change lots of callsites. Something like this (
bios32.cis incomplete):
It is fine by me, I do not know if Bjorn is willing to accept the PCI core change required below.
Bjorn, what do you think ? I will fold the diff below in the original patch if we are all happy with the change.
Thanks !
Lorenzo
Quoted 101 lines
arch/arm/kernel/bios32.c | 8 +++----- drivers/pci/probe.c | 15 +++++++++++++-- include/linux/pci.h | 4 ++++ 3 files changed, 20 insertions(+), 7 deletions(-)arch/arm/kernel/bios32.c +3
-5@@ -462,9 +462,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;@@ -486,8 +483,9 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,if (hw->scan) sys->bus = hw->scan (nr, sys);else - sys->bus =pci_scan_root_bus (parent, sys->busnr, - hw->ops, sys, &sys->resources); + sys->bus =pci_scan_root_bus_msi (parent, + sys->busnr, hw->ops, sys, + &sys->resources, hw->msi_ctrl);if (!sys->bus)panic ("PCI: unable to scan bus!" );drivers/pci/probe.c +13
-2@@ -2096,8 +2096,9 @@ void pci_bus_release_busn_res(struct pci_bus *b) res, ret ? "can not be" : "is"); } -struct pci_bus *pci_scan_root_bus (struct device *parent,int bus, -struct pci_ops *ops,void *sysdata,struct list_head *resources) +struct pci_bus *pci_scan_root_bus_msi (struct device *parent,int bus, +struct pci_ops *ops,void *sysdata, +struct list_head *resources,struct msi_controller *msi) {struct resource_entry *window;bool found = false;@@ -2114,6 +2115,8 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,if (!b)return NULL ; + b->msi = msi; +if (!found) { dev_info(&b->dev, "No busn resource found for root bus, will use [bus %02x-ff]\n",@@ -2128,6 +2131,14 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,return b; } +EXPORT_SYMBOL (pci_scan_root_bus_msi); + +struct pci_bus *pci_scan_root_bus (struct device *parent,int bus, +struct pci_ops *ops,void *sysdata,struct list_head *resources) +{ +return pci_scan_root_bus_msi (parent, bus, ops, sysdata, resources, +NULL ); +}EXPORT_SYMBOL (pci_scan_root_bus); struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops,include/linux/pci.h +4
-0@@ -787,6 +787,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,int pci_bus_insert_busn_res (struct pci_bus *b,int bus,int busmax);int pci_bus_update_busn_res_end (struct pci_bus *b,int busmax);void pci_bus_release_busn_res (struct pci_bus *b); +struct pci_bus *pci_scan_root_bus_msi (struct device *parent,int bus, +struct pci_ops *ops,void *sysdata, +struct list_head *resources, +struct msi_controller *msi);struct pci_bus *pci_scan_root_bus (struct device *parent,int bus,struct pci_ops *ops,void *sysdata,struct list_head *resources);And it's probably a good idea to kill off the ifdef around this:
struct
hw_pci{#ifdef CONFIG_PCI_MSI struct msi_controller *msi_ctrl;#endif
so that we can avoid ifdefs in random places in
arch/armcode (as is the decision in the rest of PCI for this.)Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
PATCH v2 1/2 ARM: PCI: bios32: replace panic with WARN messages on failures
In reply to Lorenzo PieralisiQuoted 5 lines · Lorenzo Pieralisi
In the ARM PCI bios32 layer, failures to dynamically allocate
pci_sys_datafor a PCI bus, or a PCI bus scan failure have to be considered serious warnings but they should not trigger a system panic so that at least the system is given a chance to be debugged.
Quoted 7 lines
This patch replaces the panic statements with
WARN()messages to improve error reporting in the ARM PCI bios32 layer.Signed-off-by Lorenzo Pieralisi <[email protected]>Cc Bjorn Helgaas <[email protected]>Cc Russell King <[email protected]>Cc Marc Zyngier <[email protected]>
M.
Signature · 1 line
Jazz is not dead. It just smells funny...