* [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller()
@ 2015-08-04 21:53 Bjorn Helgaas
2015-08-04 21:53 ` [PATCH v5 1/9] ARM/PCI: Replace panic with WARN messages on failures Bjorn Helgaas
` (9 more replies)
0 siblings, 10 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:53 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
The first 5 patches here are essentially the same as Lorenzo's v4 posting
at
http://lkml.kernel.org/1438169598-24490-1-git-send-email-lorenzo.pieralisi@arm.com
I added some at the end to fix what look like some X-Gene bugs to me, and
to make X-Gene use pci_scan_root_bus_msi() like the other drivers that use
msi_controller.
v4->v5 changes:
- Declare pci_scan_root_bus_msi() in drivers/pci/pci.h instead of
include/linux/pci.h. I like the idea, but I hope we can replace this and
other "scan_root_bus" interfaces with a single, more-generic interface,
so I'd rather not expose this one widely.
- Split "kill pcibios_msi_controller" into several patches. My intent was
to make it easier to review; I don't think I made any actual code
changes. I did drop Marc's ack because I fiddled with stuff enough
that I wasn't comfortable keeping it.
- I added of_node_put() after of_parse_phandle() in MVEBU and X-Gene.
I'm not an OF guy, so tell me if this is the wrong thing.
- I changed a couple X-Gene OF "msi-parent" things to match (I think) what
MVEBU does. Again, tell me if I got this wrong.
- I used pci_scan_root_bus_msi() in X-Gene to follow what Lorenzo already
did in DesignWare and Xilinx.
---
Lorenzo Pieralisi (5):
ARM/PCI: Replace panic with WARN messages on failures
PCI: Add pci_scan_root_bus_msi()
ARM/PCI, designware, xilinx: Use pci_scan_root_bus_msi()
ARM/PCI: Remove msi_controller from struct pci_sys_data
PCI/MSI: Remove unused pcibios_msi_controller() hook
Bjorn Helgaas (4):
PCI: Drop references acquired by of_parse_phandle()
PCI: xgene: Set msi_controller->dev to platform_device, not pci_bus
PCI: xgene: Look for OF "msi-parent" in host controller, not root bus
PCI: xgene: Use pci_scan_root_bus_msi()
arch/arm/include/asm/mach/pci.h | 5 -----
arch/arm/kernel/bios32.c | 27 +++++++++------------------
drivers/pci/host/pci-mvebu.c | 1 +
drivers/pci/host/pci-xgene.c | 26 +++++++++++++-------------
drivers/pci/host/pcie-designware.c | 6 +++---
drivers/pci/host/pcie-xilinx.c | 5 ++---
drivers/pci/msi.c | 17 +----------------
drivers/pci/pci.h | 5 +++++
drivers/pci/probe.c | 14 ++++++++++++--
9 files changed, 46 insertions(+), 60 deletions(-)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 1/9] ARM/PCI: Replace panic with WARN messages on failures
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
@ 2015-08-04 21:53 ` Bjorn Helgaas
2015-08-06 14:46 ` Jingoo Han
2015-08-04 21:54 ` [PATCH v5 2/9] PCI: Add pci_scan_root_bus_msi() Bjorn Helgaas
` (8 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:53 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
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.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm/kernel/bios32.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index bf370bc..4e95260 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -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;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v5 2/9] PCI: Add pci_scan_root_bus_msi()
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
2015-08-04 21:53 ` [PATCH v5 1/9] ARM/PCI: Replace panic with WARN messages on failures Bjorn Helgaas
@ 2015-08-04 21:54 ` Bjorn Helgaas
2015-08-06 14:47 ` Jingoo Han
2015-08-04 21:54 ` [PATCH v5 3/9] ARM/PCI, designware, xilinx: Use pci_scan_root_bus_msi() Bjorn Helgaas
` (7 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Add a pci_scan_root_bus_msi() interface so an arch can specify the MSI
controller up front. This removes the need for a pcibios callback to set
the MSI controller later.
This is not exported because I'd like to replace the variety of "scan root
bus" interfaces with a single, more extensible interface that can handle
the MSI controller, domain, pci_ops, resources, etc. I hope this interface
is temporary.
[bhelgaas: changelog, split into separate patch, move to drivers/pci/pci.h]
Suggested-by: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pci.h | 5 +++++
drivers/pci/probe.c | 14 ++++++++++++--
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4ff0ff1..1d4c95c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -223,6 +223,11 @@ enum pci_bar_type {
pci_bar_mem64, /* A 64-bit memory BAR */
};
+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);
+
bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
int crs_timeout);
int pci_setup_device(struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index cefd636..9ff4df0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -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,13 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
return b;
}
+
+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,
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v5 3/9] ARM/PCI, designware, xilinx: Use pci_scan_root_bus_msi()
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
2015-08-04 21:53 ` [PATCH v5 1/9] ARM/PCI: Replace panic with WARN messages on failures Bjorn Helgaas
2015-08-04 21:54 ` [PATCH v5 2/9] PCI: Add pci_scan_root_bus_msi() Bjorn Helgaas
@ 2015-08-04 21:54 ` Bjorn Helgaas
2015-08-06 14:49 ` Jingoo Han
2015-08-04 21:54 ` [PATCH v5 4/9] ARM/PCI: Remove msi_controller from struct pci_sys_data Bjorn Helgaas
` (6 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
ARM previously stored the msi_controller pointer in its sysdata, struct
pci_sys_data, and implemented pcibios_msi_controller() to retrieve it.
That made PCI host controller drivers specific to ARM because they had to
put the msi_controller pointer in the ARM-specific pci_sys_data.
There is now a generic mechanism, pci_scan_root_bus_msi(), for giving the
msi_controller pointer to the PCI core. Use this for all ARM systems and
for the DesignWare and Xilinx PCI host controller drivers.
This removes an ARM dependency from the DesignWare, DRA7xx, EXYNOS, i.MX6,
Keystone, Layerscape, SPEAr13xx, and Xilinx drivers.
[bhelgaas: changelog, split into separate patch]
Suggested-by: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Pratyush Anand <pratyush.anand@gmail.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Jingoo Han <jingoohan1@gmail.com>
CC: Simon Horman <horms@verge.net.au>
CC: Russell King <linux@arm.linux.org.uk>
CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
CC: Thierry Reding <thierry.reding@gmail.com>
CC: Michal Simek <michal.simek@xilinx.com>
CC: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm/include/asm/mach/pci.h | 2 --
arch/arm/kernel/bios32.c | 5 +++--
drivers/pci/host/pcie-designware.c | 6 +++---
drivers/pci/host/pcie-xilinx.c | 5 ++---
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
index 28b9bb3..c074e7a 100644
--- a/arch/arm/include/asm/mach/pci.h
+++ b/arch/arm/include/asm/mach/pci.h
@@ -19,9 +19,7 @@ struct pci_bus;
struct device;
struct hw_pci {
-#ifdef CONFIG_PCI_MSI
struct msi_controller *msi_ctrl;
-#endif
struct pci_ops *ops;
int nr_controllers;
void **private_data;
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 4e95260..283bc1c 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -486,8 +486,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 (WARN(!sys->bus, "PCI: unable to scan bus!")) {
kfree(sys);
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 69486be..bd0aeec 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -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,8 +707,9 @@ 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,
- &dw_pcie_ops, sys, &sys->resources);
+ bus = pci_scan_root_bus_msi(pp->dev, sys->busnr, &dw_pcie_ops, sys,
+ &sys->resources, &dw_pcie_msi_chip);
+
if (!bus)
return NULL;
diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index f1a06a0..526807d 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -647,8 +647,8 @@ 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,
- sys, &sys->resources);
+ bus = pci_scan_root_bus_msi(port->dev, sys->busnr, &xilinx_pcie_ops,
+ sys, &sys->resources, &xilinx_pcie_msi_chip);
return bus;
}
@@ -847,7 +847,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);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v5 4/9] ARM/PCI: Remove msi_controller from struct pci_sys_data
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
` (2 preceding siblings ...)
2015-08-04 21:54 ` [PATCH v5 3/9] ARM/PCI, designware, xilinx: Use pci_scan_root_bus_msi() Bjorn Helgaas
@ 2015-08-04 21:54 ` Bjorn Helgaas
2015-08-06 14:51 ` Jingoo Han
2015-08-04 21:54 ` [PATCH v5 5/9] PCI/MSI: Remove unused pcibios_msi_controller() hook Bjorn Helgaas
` (5 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
ARM now uses pci_bus->msi to store the msi_controller pointer, so we don't
need to save it in struct pci_sys_data, and we don't need to implement
pcibios_msi_controller() to get it out of pci_sys_data.
Remove msi_controller from struct pci_sys_data and
pcibios_msi_controller().
[bhelgaas: changelog, split into separate patch]
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
arch/arm/include/asm/mach/pci.h | 3 ---
arch/arm/kernel/bios32.c | 12 ------------
2 files changed, 15 deletions(-)
diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
index c074e7a..8857d28 100644
--- a/arch/arm/include/asm/mach/pci.h
+++ b/arch/arm/include/asm/mach/pci.h
@@ -40,9 +40,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 */
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 283bc1c..874e182 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -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;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v5 5/9] PCI/MSI: Remove unused pcibios_msi_controller() hook
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
` (3 preceding siblings ...)
2015-08-04 21:54 ` [PATCH v5 4/9] ARM/PCI: Remove msi_controller from struct pci_sys_data Bjorn Helgaas
@ 2015-08-04 21:54 ` Bjorn Helgaas
2015-08-04 21:54 ` [PATCH v5 6/9] PCI: Drop references acquired by of_parse_phandle() Bjorn Helgaas
` (4 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
The pcibios_msi_controller() hook was only implemented by ARM, and it sets
pci_bus->msi now, so it doesn't need this hook anymore.
Remove the unused pcibios_msi_controller() hook.
[bhelgaas: changelog, split into separate patch]
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/msi.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index f66be868..0d20142 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -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)
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v5 6/9] PCI: Drop references acquired by of_parse_phandle()
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
` (4 preceding siblings ...)
2015-08-04 21:54 ` [PATCH v5 5/9] PCI/MSI: Remove unused pcibios_msi_controller() hook Bjorn Helgaas
@ 2015-08-04 21:54 ` Bjorn Helgaas
2015-08-10 21:39 ` Bjorn Helgaas
2015-08-12 11:24 ` Lorenzo Pieralisi
2015-08-04 21:54 ` [PATCH v5 7/9] PCI: xgene: Set msi_controller->dev to platform_device, not pci_bus Bjorn Helgaas
` (3 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
of_parse_phandle() returns a device_node pointer with the refcount
incremented. We should dispose of this reference when we're finished.
Drop the reference acquired by of_parse_phandle().
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pci-mvebu.c | 1 +
drivers/pci/host/pci-xgene.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 70aa095..67ec5e1 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -879,6 +879,7 @@ static void mvebu_pcie_msi_enable(struct mvebu_pcie *pcie)
return;
pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
+ of_node_put(msi_node);
if (pcie->msi)
pcie->msi->dev = &pcie->pdev->dev;
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index a9dfb70..4c2fb1f 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -514,6 +514,7 @@ static int xgene_pcie_msi_enable(struct pci_bus *bus)
if (!bus->msi)
return -ENODEV;
+ of_node_put(msi_node);
bus->msi->dev = &bus->dev;
return 0;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v5 7/9] PCI: xgene: Set msi_controller->dev to platform_device, not pci_bus
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
` (5 preceding siblings ...)
2015-08-04 21:54 ` [PATCH v5 6/9] PCI: Drop references acquired by of_parse_phandle() Bjorn Helgaas
@ 2015-08-04 21:54 ` Bjorn Helgaas
2015-08-04 22:58 ` Bjorn Helgaas
2015-08-04 21:54 ` [PATCH v5 8/9] PCI: xgene: Look for OF "msi-parent" in host controller, not root bus Bjorn Helgaas
` (2 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
Other users of struct msi_controller set msi->dev to the platform_device of
the PCI host controller, not the device of the pci_bus for the root bus.
Set X-Gene's msi_controller->dev to the PCI host controller platform_device
as other platforms do.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pci-xgene.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 4c2fb1f..5e0d6de 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -501,7 +501,8 @@ static int xgene_pcie_setup(struct xgene_pcie_port *port,
return 0;
}
-static int xgene_pcie_msi_enable(struct pci_bus *bus)
+static int xgene_pcie_msi_enable(struct xgene_pcie_port *port,
+ struct pci_bus *bus)
{
struct device_node *msi_node;
@@ -515,7 +516,7 @@ static int xgene_pcie_msi_enable(struct pci_bus *bus)
return -ENODEV;
of_node_put(msi_node);
- bus->msi->dev = &bus->dev;
+ bus->msi->dev = &port->dev;
return 0;
}
@@ -560,7 +561,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
return -ENOMEM;
if (IS_ENABLED(CONFIG_PCI_MSI))
- if (xgene_pcie_msi_enable(bus))
+ if (xgene_pcie_msi_enable(port, bus))
dev_info(port->dev, "failed to enable MSI\n");
pci_scan_child_bus(bus);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v5 8/9] PCI: xgene: Look for OF "msi-parent" in host controller, not root bus
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
` (6 preceding siblings ...)
2015-08-04 21:54 ` [PATCH v5 7/9] PCI: xgene: Set msi_controller->dev to platform_device, not pci_bus Bjorn Helgaas
@ 2015-08-04 21:54 ` Bjorn Helgaas
2015-08-04 21:54 ` [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi() Bjorn Helgaas
2015-08-04 23:00 ` [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
9 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
Look for the "msi-parent" in the PCI host controller OF node, not the root
bus OF node. This makes it the same as MVEBU.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pci-xgene.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 5e0d6de..3cb58a3 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -506,8 +506,7 @@ static int xgene_pcie_msi_enable(struct xgene_pcie_port *port,
{
struct device_node *msi_node;
- msi_node = of_parse_phandle(bus->dev.of_node,
- "msi-parent", 0);
+ msi_node = of_parse_phandle(port->dev.of_node, "msi-parent", 0);
if (!msi_node)
return -ENODEV;
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi()
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
` (7 preceding siblings ...)
2015-08-04 21:54 ` [PATCH v5 8/9] PCI: xgene: Look for OF "msi-parent" in host controller, not root bus Bjorn Helgaas
@ 2015-08-04 21:54 ` Bjorn Helgaas
2015-08-06 15:26 ` Marc Zyngier
2015-08-04 23:00 ` [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
9 siblings, 1 reply; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 21:54 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
Previously there was no way to specify the MSI controller when creating a
new PCI root bus, so we had to create the bus, set its MSI controller, then
scan the bus. With the new pci_scan_root_bus_msi() interface, we can
specify the MSI controller up front and get rid of that intermediate step.
Look up the MSI controller first, then use pci_scan_root_bus_msi() to
create and scan the root PCI bus.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pci-xgene.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 3cb58a3..514f41b 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -68,6 +68,7 @@
struct xgene_pcie_port {
struct device_node *node;
struct device *dev;
+ struct msi_controller *msi;
struct clk *clk;
void __iomem *csr_base;
void __iomem *cfg_base;
@@ -501,8 +502,7 @@ static int xgene_pcie_setup(struct xgene_pcie_port *port,
return 0;
}
-static int xgene_pcie_msi_enable(struct xgene_pcie_port *port,
- struct pci_bus *bus)
+static int xgene_pcie_msi_enable(struct xgene_pcie_port *port)
{
struct device_node *msi_node;
@@ -510,12 +510,12 @@ static int xgene_pcie_msi_enable(struct xgene_pcie_port *port,
if (!msi_node)
return -ENODEV;
- bus->msi = of_pci_find_msi_chip_by_node(msi_node);
- if (!bus->msi)
+ port->msi = of_pci_find_msi_chip_by_node(msi_node);
+ if (!port->msi)
return -ENODEV;
of_node_put(msi_node);
- bus->msi->dev = &port->dev;
+ port->msi->dev = &port->dev;
return 0;
}
@@ -554,16 +554,15 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
if (ret)
return ret;
- bus = pci_create_root_bus(&pdev->dev, 0,
- &xgene_pcie_ops, port, &res);
- if (!bus)
- return -ENOMEM;
-
if (IS_ENABLED(CONFIG_PCI_MSI))
- if (xgene_pcie_msi_enable(port, bus))
+ if (xgene_pcie_msi_enable(port))
dev_info(port->dev, "failed to enable MSI\n");
- pci_scan_child_bus(bus);
+ bus = pci_scan_root_bus_msi(&pdev->dev, 0, &xgene_pcie_ops, port,
+ &res, port->msi);
+ if (!bus)
+ return -ENOMEM;
+
pci_assign_unassigned_bus_resources(bus);
pci_bus_add_devices(bus);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v5 7/9] PCI: xgene: Set msi_controller->dev to platform_device, not pci_bus
2015-08-04 21:54 ` [PATCH v5 7/9] PCI: xgene: Set msi_controller->dev to platform_device, not pci_bus Bjorn Helgaas
@ 2015-08-04 22:58 ` Bjorn Helgaas
0 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 22:58 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
On Tue, Aug 04, 2015 at 04:54:42PM -0500, Bjorn Helgaas wrote:
> Other users of struct msi_controller set msi->dev to the platform_device of
> the PCI host controller, not the device of the pci_bus for the root bus.
>
> Set X-Gene's msi_controller->dev to the PCI host controller platform_device
> as other platforms do.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/host/pci-xgene.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> index 4c2fb1f..5e0d6de 100644
> --- a/drivers/pci/host/pci-xgene.c
> +++ b/drivers/pci/host/pci-xgene.c
> @@ -501,7 +501,8 @@ static int xgene_pcie_setup(struct xgene_pcie_port *port,
> return 0;
> }
>
> -static int xgene_pcie_msi_enable(struct pci_bus *bus)
> +static int xgene_pcie_msi_enable(struct xgene_pcie_port *port,
> + struct pci_bus *bus)
> {
> struct device_node *msi_node;
>
> @@ -515,7 +516,7 @@ static int xgene_pcie_msi_enable(struct pci_bus *bus)
> return -ENODEV;
>
> of_node_put(msi_node);
> - bus->msi->dev = &bus->dev;
> + bus->msi->dev = &port->dev;
Thomas, the surrounding code here and in mvebu_pcie_msi_enable() looks like
this:
pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
pcie->msi->dev = &pcie->pdev->dev;
It seems sort of strange to search for a struct msi_controller, then set
the "dev" field inside it. This code isn't really the owner of the
msi_controller, and it seems like in principle at least, the
of_pci_find_msi_chip_by_node() interface could be used by several clients.
Then it's not clear which one of them should update msi->dev.
It would make more sense to me if the caller of of_pci_msi_chip_add() set
the msi->dev field. But I don't know whether that's feasible. I don't
even know what msi->dev is used for.
Bjorn
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller()
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
` (8 preceding siblings ...)
2015-08-04 21:54 ` [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi() Bjorn Helgaas
@ 2015-08-04 23:00 ` Bjorn Helgaas
9 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-04 23:00 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper
On Tue, Aug 04, 2015 at 04:53:48PM -0500, Bjorn Helgaas wrote:
> The first 5 patches here are essentially the same as Lorenzo's v4 posting
> at
> http://lkml.kernel.org/1438169598-24490-1-git-send-email-lorenzo.pieralisi@arm.com
>
> I added some at the end to fix what look like some X-Gene bugs to me, and
> to make X-Gene use pci_scan_root_bus_msi() like the other drivers that use
> msi_controller.
>
> v4->v5 changes:
> - Declare pci_scan_root_bus_msi() in drivers/pci/pci.h instead of
> include/linux/pci.h. I like the idea, but I hope we can replace this and
> other "scan_root_bus" interfaces with a single, more-generic interface,
> so I'd rather not expose this one widely.
This is one thing I screwed up already. pci_scan_root_bus_msi() can't be
declared in drivers/pci/pci.h because that's not visible to
arch/arm/kernel/bios32.c.
In case anybody wants to try it, I updated the branch here:
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/enumeration
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 1/9] ARM/PCI: Replace panic with WARN messages on failures
2015-08-04 21:53 ` [PATCH v5 1/9] ARM/PCI: Replace panic with WARN messages on failures Bjorn Helgaas
@ 2015-08-06 14:46 ` Jingoo Han
0 siblings, 0 replies; 25+ messages in thread
From: Jingoo Han @ 2015-08-06 14:46 UTC (permalink / raw)
To: 'Bjorn Helgaas', 'Lorenzo Pieralisi'
Cc: 'Thomas Petazzoni', 'Jayachandran C',
'Pratyush Anand', 'Russell King',
'Arnd Bergmann', 'Gabriele Paoloni',
'Marc Zyngier', linux-pci, 'Duc Dang',
'Michal Simek', 'Simon Horman',
'James Morse', 'Tanmay Inamdar',
'Thierry Reding', linux-arm-kernel,
'Jason Cooper', 'Jingoo Han'
On Wednesday, August 05, 2015 6:54 AM, Bjorn Helgaas wrote:
>
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
> 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.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> CC: Russell King <linux@arm.linux.org.uk>
> CC: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Jingoo Han <jingoohan1@gmail.com>
In ARM, WARN message looks good.
Best regards,
Jingoo Han
> ---
> arch/arm/kernel/bios32.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index bf370bc..4e95260 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -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;
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 2/9] PCI: Add pci_scan_root_bus_msi()
2015-08-04 21:54 ` [PATCH v5 2/9] PCI: Add pci_scan_root_bus_msi() Bjorn Helgaas
@ 2015-08-06 14:47 ` Jingoo Han
0 siblings, 0 replies; 25+ messages in thread
From: Jingoo Han @ 2015-08-06 14:47 UTC (permalink / raw)
To: 'Bjorn Helgaas', 'Lorenzo Pieralisi'
Cc: 'Thomas Petazzoni', 'Jayachandran C',
'Pratyush Anand', 'Russell King',
'Arnd Bergmann', 'Gabriele Paoloni',
'Marc Zyngier', linux-pci, 'Duc Dang',
'Michal Simek', 'Simon Horman',
'James Morse', 'Tanmay Inamdar',
'Thierry Reding', linux-arm-kernel,
'Jason Cooper', 'Jingoo Han'
On Wednesday, August 05, 2015 6:54 AM, Bjorn Helgaas wrote:
>
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
> Add a pci_scan_root_bus_msi() interface so an arch can specify the MSI
> controller up front. This removes the need for a pcibios callback to set
> the MSI controller later.
>
> This is not exported because I'd like to replace the variety of "scan root
> bus" interfaces with a single, more extensible interface that can handle
> the MSI controller, domain, pci_ops, resources, etc. I hope this interface
> is temporary.
>
> [bhelgaas: changelog, split into separate patch, move to drivers/pci/pci.h]
> Suggested-by: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Jingoo Han <jingoohan1@gmail.com>
Best regards,
Jingoo Han
> ---
> drivers/pci/pci.h | 5 +++++
> drivers/pci/probe.c | 14 ++++++++++++--
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 4ff0ff1..1d4c95c 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -223,6 +223,11 @@ enum pci_bar_type {
> pci_bar_mem64, /* A 64-bit memory BAR */
> };
>
> +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);
> +
> bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *pl,
> int crs_timeout);
> int pci_setup_device(struct pci_dev *dev);
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index cefd636..9ff4df0 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -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,13 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
>
> return b;
> }
> +
> +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,
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 3/9] ARM/PCI, designware, xilinx: Use pci_scan_root_bus_msi()
2015-08-04 21:54 ` [PATCH v5 3/9] ARM/PCI, designware, xilinx: Use pci_scan_root_bus_msi() Bjorn Helgaas
@ 2015-08-06 14:49 ` Jingoo Han
0 siblings, 0 replies; 25+ messages in thread
From: Jingoo Han @ 2015-08-06 14:49 UTC (permalink / raw)
To: 'Bjorn Helgaas', 'Lorenzo Pieralisi'
Cc: 'Thomas Petazzoni', 'Jayachandran C',
'Pratyush Anand', 'Russell King',
'Arnd Bergmann', 'Gabriele Paoloni',
'Marc Zyngier', linux-pci, 'Duc Dang',
'Michal Simek', 'Simon Horman',
'James Morse', 'Tanmay Inamdar',
'Thierry Reding', linux-arm-kernel,
'Jason Cooper', 'Jingoo Han'
On Wednesday, August 05, 2015 6:54 AM, Bjorn Helgaas wrote:
>
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
> ARM previously stored the msi_controller pointer in its sysdata, struct
> pci_sys_data, and implemented pcibios_msi_controller() to retrieve it.
> That made PCI host controller drivers specific to ARM because they had to
> put the msi_controller pointer in the ARM-specific pci_sys_data.
>
> There is now a generic mechanism, pci_scan_root_bus_msi(), for giving the
> msi_controller pointer to the PCI core. Use this for all ARM systems and
> for the DesignWare and Xilinx PCI host controller drivers.
>
> This removes an ARM dependency from the DesignWare, DRA7xx, EXYNOS, i.MX6,
> Keystone, Layerscape, SPEAr13xx, and Xilinx drivers.
>
> [bhelgaas: changelog, split into separate patch]
> Suggested-by: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Pratyush Anand <pratyush.anand@gmail.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Jingoo Han <jingoohan1@gmail.com>
Acked-by: Jingoo Han <jingoohan1@gmail.com>
Best regards,
Jingoo Han
> CC: Simon Horman <horms@verge.net.au>
> CC: Russell King <linux@arm.linux.org.uk>
> CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> CC: Thierry Reding <thierry.reding@gmail.com>
> CC: Michal Simek <michal.simek@xilinx.com>
> CC: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/include/asm/mach/pci.h | 2 --
> arch/arm/kernel/bios32.c | 5 +++--
> drivers/pci/host/pcie-designware.c | 6 +++---
> drivers/pci/host/pcie-xilinx.c | 5 ++---
> 4 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
> index 28b9bb3..c074e7a 100644
> --- a/arch/arm/include/asm/mach/pci.h
> +++ b/arch/arm/include/asm/mach/pci.h
> @@ -19,9 +19,7 @@ struct pci_bus;
> struct device;
>
> struct hw_pci {
> -#ifdef CONFIG_PCI_MSI
> struct msi_controller *msi_ctrl;
> -#endif
> struct pci_ops *ops;
> int nr_controllers;
> void **private_data;
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index 4e95260..283bc1c 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -486,8 +486,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 (WARN(!sys->bus, "PCI: unable to scan bus!")) {
> kfree(sys);
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 69486be..bd0aeec 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -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,8 +707,9 @@ 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,
> - &dw_pcie_ops, sys, &sys->resources);
> + bus = pci_scan_root_bus_msi(pp->dev, sys->busnr, &dw_pcie_ops, sys,
> + &sys->resources, &dw_pcie_msi_chip);
> +
> if (!bus)
> return NULL;
>
> diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
> index f1a06a0..526807d 100644
> --- a/drivers/pci/host/pcie-xilinx.c
> +++ b/drivers/pci/host/pcie-xilinx.c
> @@ -647,8 +647,8 @@ 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,
> - sys, &sys->resources);
> + bus = pci_scan_root_bus_msi(port->dev, sys->busnr, &xilinx_pcie_ops,
> + sys, &sys->resources, &xilinx_pcie_msi_chip);
>
> return bus;
> }
> @@ -847,7 +847,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);
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 4/9] ARM/PCI: Remove msi_controller from struct pci_sys_data
2015-08-04 21:54 ` [PATCH v5 4/9] ARM/PCI: Remove msi_controller from struct pci_sys_data Bjorn Helgaas
@ 2015-08-06 14:51 ` Jingoo Han
0 siblings, 0 replies; 25+ messages in thread
From: Jingoo Han @ 2015-08-06 14:51 UTC (permalink / raw)
To: 'Bjorn Helgaas', 'Lorenzo Pieralisi'
Cc: 'Thomas Petazzoni', 'Jayachandran C',
'Pratyush Anand', 'Russell King',
'Arnd Bergmann', 'Gabriele Paoloni',
'Marc Zyngier', linux-pci, 'Duc Dang',
'Michal Simek', 'Simon Horman',
'James Morse', 'Tanmay Inamdar',
'Thierry Reding', linux-arm-kernel,
'Jason Cooper', 'Jingoo Han'
On Wednesday, August 05, 2015 6:54 AM, Bjorn Helgaas wrote:
>
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
> ARM now uses pci_bus->msi to store the msi_controller pointer, so we don't
> need to save it in struct pci_sys_data, and we don't need to implement
> pcibios_msi_controller() to get it out of pci_sys_data.
>
> Remove msi_controller from struct pci_sys_data and
> pcibios_msi_controller().
>
> [bhelgaas: changelog, split into separate patch]
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Jingoo Han <jingoohan1@gmail.com>
Best regards,
Jingoo Han
> ---
> arch/arm/include/asm/mach/pci.h | 3 ---
> arch/arm/kernel/bios32.c | 12 ------------
> 2 files changed, 15 deletions(-)
>
> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
> index c074e7a..8857d28 100644
> --- a/arch/arm/include/asm/mach/pci.h
> +++ b/arch/arm/include/asm/mach/pci.h
> @@ -40,9 +40,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 */
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index 283bc1c..874e182 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -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;
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi()
2015-08-04 21:54 ` [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi() Bjorn Helgaas
@ 2015-08-06 15:26 ` Marc Zyngier
2015-08-06 16:41 ` Ley Foon Tan
2015-08-10 22:04 ` Bjorn Helgaas
0 siblings, 2 replies; 25+ messages in thread
From: Marc Zyngier @ 2015-08-06 15:26 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, linux-pci@vger.kernel.org,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel@lists.infradead.org,
Jason Cooper
Hi Bjorn,
On 04/08/15 22:54, Bjorn Helgaas wrote:
> Previously there was no way to specify the MSI controller when creating a
> new PCI root bus, so we had to create the bus, set its MSI controller, then
> scan the bus. With the new pci_scan_root_bus_msi() interface, we can
> specify the MSI controller up front and get rid of that intermediate step.
>
> Look up the MSI controller first, then use pci_scan_root_bus_msi() to
> create and scan the root PCI bus.
I'm wondering about these XGene patches.
With the code that is queued for v4.3 in tip/irq/core, the X-Gene MSI
driver doesn't export a struct msi_controller anymore, and entirely
relies on IRQ domains to identify to be matched with the actual PCI driver.
Do you intend this as a cleanup until everything lands in mainline? At
that point, we'd be able to remove all traces of struct msi_controller
from this driver.
Alternatively, we could ask tglx to add an extra patch to the existing
queue in order to clean up pci-xgene.c (nuking the whole
xgene_pcie_msi_enable function).
Thoughts?
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi()
2015-08-06 15:26 ` Marc Zyngier
@ 2015-08-06 16:41 ` Ley Foon Tan
2015-08-06 16:53 ` Marc Zyngier
2015-08-10 22:04 ` Bjorn Helgaas
1 sibling, 1 reply; 25+ messages in thread
From: Ley Foon Tan @ 2015-08-06 16:41 UTC (permalink / raw)
To: Marc Zyngier
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Thomas Petazzoni,
Jayachandran C, Pratyush Anand, Russell King, Arnd Bergmann,
Gabriele Paoloni, linux-pci@vger.kernel.org, Duc Dang,
Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel@lists.infradead.org,
Jason Cooper
On Thu, Aug 6, 2015 at 11:26 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> Hi Bjorn,
>
> On 04/08/15 22:54, Bjorn Helgaas wrote:
>> Previously there was no way to specify the MSI controller when creating a
>> new PCI root bus, so we had to create the bus, set its MSI controller, then
>> scan the bus. With the new pci_scan_root_bus_msi() interface, we can
>> specify the MSI controller up front and get rid of that intermediate step.
>>
>> Look up the MSI controller first, then use pci_scan_root_bus_msi() to
>> create and scan the root PCI bus.
>
> I'm wondering about these XGene patches.
>
> With the code that is queued for v4.3 in tip/irq/core, the X-Gene MSI
> driver doesn't export a struct msi_controller anymore, and entirely
> relies on IRQ domains to identify to be matched with the actual PCI driver.
>
> Do you intend this as a cleanup until everything lands in mainline? At
> that point, we'd be able to remove all traces of struct msi_controller
> from this driver.
>
> Alternatively, we could ask tglx to add an extra patch to the existing
> queue in order to clean up pci-xgene.c (nuking the whole
> xgene_pcie_msi_enable function).
>
> Thoughts?
In pcie-altera driver, it uses pci_scan_root_bus() and it doesn't call
to of_pci_find_msi_chip_by_node() to retrieve msi_controller struct as
well.
But, the msi domain stuff is still working. Should I change it to
pci_scan_root_bus_msi()?
Regards
Ley Foon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi()
2015-08-06 16:41 ` Ley Foon Tan
@ 2015-08-06 16:53 ` Marc Zyngier
2015-08-07 2:18 ` Ley Foon Tan
0 siblings, 1 reply; 25+ messages in thread
From: Marc Zyngier @ 2015-08-06 16:53 UTC (permalink / raw)
To: Ley Foon Tan
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Thomas Petazzoni,
Jayachandran C, Pratyush Anand, Russell King, Arnd Bergmann,
Gabriele Paoloni, linux-pci@vger.kernel.org, Duc Dang,
Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel@lists.infradead.org,
Jason Cooper
On 06/08/15 17:41, Ley Foon Tan wrote:
> On Thu, Aug 6, 2015 at 11:26 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Hi Bjorn,
>>
>> On 04/08/15 22:54, Bjorn Helgaas wrote:
>>> Previously there was no way to specify the MSI controller when creating a
>>> new PCI root bus, so we had to create the bus, set its MSI controller, then
>>> scan the bus. With the new pci_scan_root_bus_msi() interface, we can
>>> specify the MSI controller up front and get rid of that intermediate step.
>>>
>>> Look up the MSI controller first, then use pci_scan_root_bus_msi() to
>>> create and scan the root PCI bus.
>>
>> I'm wondering about these XGene patches.
>>
>> With the code that is queued for v4.3 in tip/irq/core, the X-Gene MSI
>> driver doesn't export a struct msi_controller anymore, and entirely
>> relies on IRQ domains to identify to be matched with the actual PCI driver.
>>
>> Do you intend this as a cleanup until everything lands in mainline? At
>> that point, we'd be able to remove all traces of struct msi_controller
>> from this driver.
>>
>> Alternatively, we could ask tglx to add an extra patch to the existing
>> queue in order to clean up pci-xgene.c (nuking the whole
>> xgene_pcie_msi_enable function).
>>
>> Thoughts?
>
> In pcie-altera driver, it uses pci_scan_root_bus() and it doesn't call
> to of_pci_find_msi_chip_by_node() to retrieve msi_controller struct as
> well.
> But, the msi domain stuff is still working. Should I change it to
> pci_scan_root_bus_msi()?
If you're not using struct msi_controller, I don't see a reason to use
pci_scan_root_bus_msi(). The MSI irq domain patches are queued for 4.3,
and I don't believe your driver will be merged before that.
Eventually, I'd like to kill msi_controller entirely. It is completely
redundant with the use of irq domains.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi()
2015-08-06 16:53 ` Marc Zyngier
@ 2015-08-07 2:18 ` Ley Foon Tan
0 siblings, 0 replies; 25+ messages in thread
From: Ley Foon Tan @ 2015-08-07 2:18 UTC (permalink / raw)
To: Marc Zyngier
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Thomas Petazzoni,
Jayachandran C, Pratyush Anand, Russell King, Arnd Bergmann,
Gabriele Paoloni, linux-pci@vger.kernel.org, Duc Dang,
Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel@lists.infradead.org,
Jason Cooper
On Fri, Aug 7, 2015 at 12:53 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 06/08/15 17:41, Ley Foon Tan wrote:
>> On Thu, Aug 6, 2015 at 11:26 PM, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> Hi Bjorn,
>>>
>>> On 04/08/15 22:54, Bjorn Helgaas wrote:
>>>> Previously there was no way to specify the MSI controller when creating a
>>>> new PCI root bus, so we had to create the bus, set its MSI controller, then
>>>> scan the bus. With the new pci_scan_root_bus_msi() interface, we can
>>>> specify the MSI controller up front and get rid of that intermediate step.
>>>>
>>>> Look up the MSI controller first, then use pci_scan_root_bus_msi() to
>>>> create and scan the root PCI bus.
>>>
>>> I'm wondering about these XGene patches.
>>>
>>> With the code that is queued for v4.3 in tip/irq/core, the X-Gene MSI
>>> driver doesn't export a struct msi_controller anymore, and entirely
>>> relies on IRQ domains to identify to be matched with the actual PCI driver.
>>>
>>> Do you intend this as a cleanup until everything lands in mainline? At
>>> that point, we'd be able to remove all traces of struct msi_controller
>>> from this driver.
>>>
>>> Alternatively, we could ask tglx to add an extra patch to the existing
>>> queue in order to clean up pci-xgene.c (nuking the whole
>>> xgene_pcie_msi_enable function).
>>>
>>> Thoughts?
>>
>> In pcie-altera driver, it uses pci_scan_root_bus() and it doesn't call
>> to of_pci_find_msi_chip_by_node() to retrieve msi_controller struct as
>> well.
>> But, the msi domain stuff is still working. Should I change it to
>> pci_scan_root_bus_msi()?
>
> If you're not using struct msi_controller, I don't see a reason to use
> pci_scan_root_bus_msi(). The MSI irq domain patches are queued for 4.3,
> and I don't believe your driver will be merged before that.
>
> Eventually, I'd like to kill msi_controller entirely. It is completely
> redundant with the use of irq domains.
>
Got it. It doesn't require struct msi_controller.
Thanks.
Regards
Ley Foon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 6/9] PCI: Drop references acquired by of_parse_phandle()
2015-08-04 21:54 ` [PATCH v5 6/9] PCI: Drop references acquired by of_parse_phandle() Bjorn Helgaas
@ 2015-08-10 21:39 ` Bjorn Helgaas
2015-08-10 22:19 ` Rob Herring
2015-08-12 11:24 ` Lorenzo Pieralisi
1 sibling, 1 reply; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-10 21:39 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier, linux-pci,
Duc Dang, Michal Simek, Simon Horman, James Morse, Tanmay Inamdar,
Jingoo Han, Thierry Reding, linux-arm-kernel, Jason Cooper,
Benjamin Herrenschmidt, Rob Herring
[+cc Ben, Rob]
On Tue, Aug 04, 2015 at 04:54:35PM -0500, Bjorn Helgaas wrote:
> of_parse_phandle() returns a device_node pointer with the refcount
> incremented. We should dispose of this reference when we're finished.
>
> Drop the reference acquired by of_parse_phandle().
I cc'd everybody who wrote or signed off on 0d5a6db3aa46 ("of: pci: add
registry of MSI chips"), which added of_pci_find_msi_chip_by_node().
The existing code (before this patch) looks wrong to me, but I'm not an OF
person.
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/host/pci-mvebu.c | 1 +
> drivers/pci/host/pci-xgene.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index 70aa095..67ec5e1 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -879,6 +879,7 @@ static void mvebu_pcie_msi_enable(struct mvebu_pcie *pcie)
> return;
>
> pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
> + of_node_put(msi_node);
>
> if (pcie->msi)
> pcie->msi->dev = &pcie->pdev->dev;
> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> index a9dfb70..4c2fb1f 100644
> --- a/drivers/pci/host/pci-xgene.c
> +++ b/drivers/pci/host/pci-xgene.c
> @@ -514,6 +514,7 @@ static int xgene_pcie_msi_enable(struct pci_bus *bus)
> if (!bus->msi)
> return -ENODEV;
>
> + of_node_put(msi_node);
> bus->msi->dev = &bus->dev;
> return 0;
> }
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi()
2015-08-06 15:26 ` Marc Zyngier
2015-08-06 16:41 ` Ley Foon Tan
@ 2015-08-10 22:04 ` Bjorn Helgaas
2015-08-10 22:28 ` Duc Dang
1 sibling, 1 reply; 25+ messages in thread
From: Bjorn Helgaas @ 2015-08-10 22:04 UTC (permalink / raw)
To: Marc Zyngier
Cc: Lorenzo Pieralisi, Thomas Petazzoni, Jayachandran C,
Pratyush Anand, Russell King, Arnd Bergmann, Gabriele Paoloni,
linux-pci@vger.kernel.org, Duc Dang, Michal Simek, Simon Horman,
James Morse, Tanmay Inamdar, Jingoo Han, Thierry Reding,
linux-arm-kernel@lists.infradead.org, Jason Cooper
Hi Marc,
On Thu, Aug 06, 2015 at 04:26:10PM +0100, Marc Zyngier wrote:
> On 04/08/15 22:54, Bjorn Helgaas wrote:
> > Previously there was no way to specify the MSI controller when creating a
> > new PCI root bus, so we had to create the bus, set its MSI controller, then
> > scan the bus. With the new pci_scan_root_bus_msi() interface, we can
> > specify the MSI controller up front and get rid of that intermediate step.
> >
> > Look up the MSI controller first, then use pci_scan_root_bus_msi() to
> > create and scan the root PCI bus.
>
> I'm wondering about these XGene patches.
>
> With the code that is queued for v4.3 in tip/irq/core, the X-Gene MSI
> driver doesn't export a struct msi_controller anymore, and entirely
> relies on IRQ domains to identify to be matched with the actual PCI driver.
>
> Do you intend this as a cleanup until everything lands in mainline? At
> that point, we'd be able to remove all traces of struct msi_controller
> from this driver.
I haven't been following the IRQ domain stuff or tip/irq/core, so I
really don't know how this relates to that. I took a look, and I see
8d63bc7beaee ("PCI/MSI: pci-xgene-msi: Get rid of struct
msi_controller"), which removes an msi_controller pointer from
pci-xgene-msi.c, but I don't see any pci-xgene.c changes in
tip/irq/core.
Are you saying that xgene_pcie_msi_enable() will go away eventually?
And the OF "msi-parent" lookup will go away, or at least be moved
elsewhere? We currently have:
xgene_pcie_probe_bridge(...)
{
...
bus = pci_create_root_bus(...);
xgene_pcie_msi_enable(...);
pci_scan_child_bus(bus);
...
}
I'd like to get rid of that arch-specific MSI enable stuff because
then we can use a higher level interface, e.g., pci_scan_root_bus(),
and make the X-Gene code slightly simpler.
> Alternatively, we could ask tglx to add an extra patch to the existing
> queue in order to clean up pci-xgene.c (nuking the whole
> xgene_pcie_msi_enable function).
Ah, I guess you *are* saying that xgene_pcie_msi_enable() will go away
eventually. I don't know how to do that, but apparently you do, so
I'll just drop these X-Gene-related patches for now.
Bjorn
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 6/9] PCI: Drop references acquired by of_parse_phandle()
2015-08-10 21:39 ` Bjorn Helgaas
@ 2015-08-10 22:19 ` Rob Herring
0 siblings, 0 replies; 25+ messages in thread
From: Rob Herring @ 2015-08-10 22:19 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Lorenzo Pieralisi, Thomas Petazzoni, Jayachandran C,
Pratyush Anand, Russell King, Arnd Bergmann, Gabriele Paoloni,
Marc Zyngier, linux-pci@vger.kernel.org, Duc Dang, Michal Simek,
Simon Horman, James Morse, Tanmay Inamdar, Jingoo Han,
Thierry Reding, linux-arm-kernel@lists.infradead.org,
Jason Cooper, Benjamin Herrenschmidt
On Mon, Aug 10, 2015 at 4:39 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [+cc Ben, Rob]
>
> On Tue, Aug 04, 2015 at 04:54:35PM -0500, Bjorn Helgaas wrote:
>> of_parse_phandle() returns a device_node pointer with the refcount
>> incremented. We should dispose of this reference when we're finished.
>>
>> Drop the reference acquired by of_parse_phandle().
>
> I cc'd everybody who wrote or signed off on 0d5a6db3aa46 ("of: pci: add
> registry of MSI chips"), which added of_pci_find_msi_chip_by_node().
>
> The existing code (before this patch) looks wrong to me, but I'm not an OF
> person.
In general, how DT reference counting works is broken and difficult to
get right. It doesn't really matter for most cases anyway (as nodes
don't get removed). I cannot tell you whether this patch is right or
wrong just looking at the patch. It seems correct based on your
description.
Rob
>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>> drivers/pci/host/pci-mvebu.c | 1 +
>> drivers/pci/host/pci-xgene.c | 1 +
>> 2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
>> index 70aa095..67ec5e1 100644
>> --- a/drivers/pci/host/pci-mvebu.c
>> +++ b/drivers/pci/host/pci-mvebu.c
>> @@ -879,6 +879,7 @@ static void mvebu_pcie_msi_enable(struct mvebu_pcie *pcie)
>> return;
>>
>> pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
>> + of_node_put(msi_node);
>>
>> if (pcie->msi)
>> pcie->msi->dev = &pcie->pdev->dev;
>> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
>> index a9dfb70..4c2fb1f 100644
>> --- a/drivers/pci/host/pci-xgene.c
>> +++ b/drivers/pci/host/pci-xgene.c
>> @@ -514,6 +514,7 @@ static int xgene_pcie_msi_enable(struct pci_bus *bus)
>> if (!bus->msi)
>> return -ENODEV;
>>
>> + of_node_put(msi_node);
>> bus->msi->dev = &bus->dev;
>> return 0;
>> }
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi()
2015-08-10 22:04 ` Bjorn Helgaas
@ 2015-08-10 22:28 ` Duc Dang
0 siblings, 0 replies; 25+ messages in thread
From: Duc Dang @ 2015-08-10 22:28 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Marc Zyngier, Lorenzo Pieralisi, Thomas Petazzoni, Jayachandran C,
Pratyush Anand, Russell King, Arnd Bergmann, Gabriele Paoloni,
linux-pci@vger.kernel.org, Michal Simek, Simon Horman,
James Morse, Tanmay Inamdar, Jingoo Han, Thierry Reding,
linux-arm-kernel@lists.infradead.org, Jason Cooper
On Mon, Aug 10, 2015 at 3:04 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Hi Marc,
>
> On Thu, Aug 06, 2015 at 04:26:10PM +0100, Marc Zyngier wrote:
>> On 04/08/15 22:54, Bjorn Helgaas wrote:
>> > Previously there was no way to specify the MSI controller when creating a
>> > new PCI root bus, so we had to create the bus, set its MSI controller, then
>> > scan the bus. With the new pci_scan_root_bus_msi() interface, we can
>> > specify the MSI controller up front and get rid of that intermediate step.
>> >
>> > Look up the MSI controller first, then use pci_scan_root_bus_msi() to
>> > create and scan the root PCI bus.
>>
>> I'm wondering about these XGene patches.
>>
>> With the code that is queued for v4.3 in tip/irq/core, the X-Gene MSI
>> driver doesn't export a struct msi_controller anymore, and entirely
>> relies on IRQ domains to identify to be matched with the actual PCI driver.
>>
>> Do you intend this as a cleanup until everything lands in mainline? At
>> that point, we'd be able to remove all traces of struct msi_controller
>> from this driver.
>
> I haven't been following the IRQ domain stuff or tip/irq/core, so I
> really don't know how this relates to that. I took a look, and I see
> 8d63bc7beaee ("PCI/MSI: pci-xgene-msi: Get rid of struct
> msi_controller"), which removes an msi_controller pointer from
> pci-xgene-msi.c, but I don't see any pci-xgene.c changes in
> tip/irq/core.
>
> Are you saying that xgene_pcie_msi_enable() will go away eventually?
> And the OF "msi-parent" lookup will go away, or at least be moved
> elsewhere? We currently have:
>
> xgene_pcie_probe_bridge(...)
> {
> ...
> bus = pci_create_root_bus(...);
> xgene_pcie_msi_enable(...);
> pci_scan_child_bus(bus);
> ...
> }
>
> I'd like to get rid of that arch-specific MSI enable stuff because
> then we can use a higher level interface, e.g., pci_scan_root_bus(),
> and make the X-Gene code slightly simpler.
>
>> Alternatively, we could ask tglx to add an extra patch to the existing
>> queue in order to clean up pci-xgene.c (nuking the whole
>> xgene_pcie_msi_enable function).
>
> Ah, I guess you *are* saying that xgene_pcie_msi_enable() will go away
> eventually. I don't know how to do that, but apparently you do, so
> I'll just drop these X-Gene-related patches for now.
Hi Bjorn,
I tested PCIe MSI with Marc's patch on X-Gene platform and it was working fine.
xgene_pcie_msi_enable will go away. I will post clean up patch to
remove these msi_controller traces
for X-Gene.
>
> Bjorn
--
Regards,
Duc Dang.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 6/9] PCI: Drop references acquired by of_parse_phandle()
2015-08-04 21:54 ` [PATCH v5 6/9] PCI: Drop references acquired by of_parse_phandle() Bjorn Helgaas
2015-08-10 21:39 ` Bjorn Helgaas
@ 2015-08-12 11:24 ` Lorenzo Pieralisi
1 sibling, 0 replies; 25+ messages in thread
From: Lorenzo Pieralisi @ 2015-08-12 11:24 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Thomas Petazzoni, Jayachandran C, Pratyush Anand, Russell King,
Arnd Bergmann, Gabriele Paoloni, Marc Zyngier,
linux-pci@vger.kernel.org, Duc Dang, Michal Simek, Simon Horman,
James Morse, Tanmay Inamdar, Jingoo Han, Thierry Reding,
linux-arm-kernel@lists.infradead.org, Jason Cooper
On Tue, Aug 04, 2015 at 10:54:35PM +0100, Bjorn Helgaas wrote:
> of_parse_phandle() returns a device_node pointer with the refcount
> incremented. We should dispose of this reference when we're finished.
>
> Drop the reference acquired by of_parse_phandle().
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/host/pci-mvebu.c | 1 +
> drivers/pci/host/pci-xgene.c | 1 +
> 2 files changed, 2 insertions(+)
It looks fine to me:
Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index 70aa095..67ec5e1 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -879,6 +879,7 @@ static void mvebu_pcie_msi_enable(struct mvebu_pcie *pcie)
> return;
>
> pcie->msi = of_pci_find_msi_chip_by_node(msi_node);
> + of_node_put(msi_node);
>
> if (pcie->msi)
> pcie->msi->dev = &pcie->pdev->dev;
> diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
> index a9dfb70..4c2fb1f 100644
> --- a/drivers/pci/host/pci-xgene.c
> +++ b/drivers/pci/host/pci-xgene.c
> @@ -514,6 +514,7 @@ static int xgene_pcie_msi_enable(struct pci_bus *bus)
> if (!bus->msi)
> return -ENODEV;
>
> + of_node_put(msi_node);
> bus->msi->dev = &bus->dev;
> return 0;
> }
>
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2015-08-12 11:24 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-04 21:53 [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
2015-08-04 21:53 ` [PATCH v5 1/9] ARM/PCI: Replace panic with WARN messages on failures Bjorn Helgaas
2015-08-06 14:46 ` Jingoo Han
2015-08-04 21:54 ` [PATCH v5 2/9] PCI: Add pci_scan_root_bus_msi() Bjorn Helgaas
2015-08-06 14:47 ` Jingoo Han
2015-08-04 21:54 ` [PATCH v5 3/9] ARM/PCI, designware, xilinx: Use pci_scan_root_bus_msi() Bjorn Helgaas
2015-08-06 14:49 ` Jingoo Han
2015-08-04 21:54 ` [PATCH v5 4/9] ARM/PCI: Remove msi_controller from struct pci_sys_data Bjorn Helgaas
2015-08-06 14:51 ` Jingoo Han
2015-08-04 21:54 ` [PATCH v5 5/9] PCI/MSI: Remove unused pcibios_msi_controller() hook Bjorn Helgaas
2015-08-04 21:54 ` [PATCH v5 6/9] PCI: Drop references acquired by of_parse_phandle() Bjorn Helgaas
2015-08-10 21:39 ` Bjorn Helgaas
2015-08-10 22:19 ` Rob Herring
2015-08-12 11:24 ` Lorenzo Pieralisi
2015-08-04 21:54 ` [PATCH v5 7/9] PCI: xgene: Set msi_controller->dev to platform_device, not pci_bus Bjorn Helgaas
2015-08-04 22:58 ` Bjorn Helgaas
2015-08-04 21:54 ` [PATCH v5 8/9] PCI: xgene: Look for OF "msi-parent" in host controller, not root bus Bjorn Helgaas
2015-08-04 21:54 ` [PATCH v5 9/9] PCI: xgene: Use pci_scan_root_bus_msi() Bjorn Helgaas
2015-08-06 15:26 ` Marc Zyngier
2015-08-06 16:41 ` Ley Foon Tan
2015-08-06 16:53 ` Marc Zyngier
2015-08-07 2:18 ` Ley Foon Tan
2015-08-10 22:04 ` Bjorn Helgaas
2015-08-10 22:28 ` Duc Dang
2015-08-04 23:00 ` [PATCH v5 0/9] ARM: PCI: kill pcibios_msi_controller() Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).