Discussion:
[RFT PATCH v2 01/42] PCI: xilinx-nwl: Remove nwl_pcie_enable_msi() unused bus parameter
Lorenzo Pieralisi
2017-06-08 14:13:01 UTC
Permalink
nwl_pcie_enable_msi() second parameter (ie bus) is unused and creates
a fake dependency on the struct pci_bus that just does not exist.

Remove it.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Bharat Kumar Gogada <***@xilinx.com>
---
drivers/pci/host/pcie-xilinx-nwl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-nwl.c
index 4b16b26..26e66e8 100644
--- a/drivers/pci/host/pcie-xilinx-nwl.c
+++ b/drivers/pci/host/pcie-xilinx-nwl.c
@@ -530,7 +530,7 @@ static int nwl_pcie_init_irq_domain(struct nwl_pcie *pcie)
return 0;
}

-static int nwl_pcie_enable_msi(struct nwl_pcie *pcie, struct pci_bus *bus)
+static int nwl_pcie_enable_msi(struct nwl_pcie *pcie)
{
struct device *dev = pcie->dev;
struct platform_device *pdev = to_platform_device(dev);
@@ -838,7 +838,7 @@ static int nwl_pcie_probe(struct platform_device *pdev)
}

if (IS_ENABLED(CONFIG_PCI_MSI)) {
- err = nwl_pcie_enable_msi(pcie, bus);
+ err = nwl_pcie_enable_msi(pcie);
if (err < 0) {
dev_err(dev, "failed to enable MSI support: %d\n", err);
goto error;
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:03 UTC
Permalink
Current ftpci100 driver host bridge controller driver requires struct
pci_bus to be created in order to mask and clear IRQs using standard
PCI bus config accessors.

This struct pci_bus dependency is fictitious and burdens the driver
with unneeded constraints (eg to use separate APIs to create and scan
the root bus).

Add PCI raw config space accessors to PCIe ftpci100 driver and remove the
fictitious struct pci_bus dependency.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Linus Walleij <***@linaro.org>
---
drivers/pci/host/pci-ftpci100.c | 44 ++++++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index d26501c..1c1fd60 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -204,17 +204,13 @@ static int faraday_pci_read_config(struct pci_bus *bus, unsigned int fn,
return PCIBIOS_SUCCESSFUL;
}

-static int faraday_pci_write_config(struct pci_bus *bus, unsigned int fn,
- int config, int size, u32 value)
+static int faraday_raw_pci_write_config(struct faraday_pci *p, int bus_number,
+ unsigned int fn, int config, int size,
+ u32 value)
{
- struct faraday_pci *p = bus->sysdata;
int ret = PCIBIOS_SUCCESSFUL;

- dev_dbg(&bus->dev,
- "[write] slt: %.2d, fnc: %d, cnf: 0x%.2X, val (%d bytes): 0x%.8X\n",
- PCI_SLOT(fn), PCI_FUNC(fn), config, size, value);
-
- writel(PCI_CONF_BUS(bus->number) |
+ writel(PCI_CONF_BUS(bus_number) |
PCI_CONF_DEVICE(PCI_SLOT(fn)) |
PCI_CONF_FUNCTION(PCI_FUNC(fn)) |
PCI_CONF_WHERE(config) |
@@ -238,6 +234,19 @@ static int faraday_pci_write_config(struct pci_bus *bus, unsigned int fn,
return ret;
}

+static int faraday_pci_write_config(struct pci_bus *bus, unsigned int fn,
+ int config, int size, u32 value)
+{
+ struct faraday_pci *p = bus->sysdata;
+
+ dev_dbg(&bus->dev,
+ "[write] slt: %.2d, fnc: %d, cnf: 0x%.2X, val (%d bytes): 0x%.8X\n",
+ PCI_SLOT(fn), PCI_FUNC(fn), config, size, value);
+
+ return faraday_raw_pci_write_config(p, bus->number, fn, config, size,
+ value);
+}
+
static struct pci_ops faraday_pci_ops = {
.read = faraday_pci_read_config,
.write = faraday_pci_write_config,
@@ -496,17 +505,8 @@ static int faraday_pci_probe(struct platform_device *pdev)
val |= PCI_COMMAND_MEMORY;
val |= PCI_COMMAND_MASTER;
writel(val, p->base + PCI_CTRL);
-
- list_splice_init(&res, &host->windows);
- ret = pci_register_host_bridge(host);
- if (ret) {
- dev_err(dev, "failed to register host: %d\n", ret);
- return ret;
- }
- p->bus = host->bus;
-
/* Mask and clear all interrupts */
- faraday_pci_write_config(p->bus, 0, FARADAY_PCI_CTRL2 + 2, 2, 0xF000);
+ faraday_raw_pci_write_config(p, 0, 0, FARADAY_PCI_CTRL2 + 2, 2, 0xF000);
if (variant->cascaded_irq) {
ret = faraday_pci_setup_cascaded_irq(p);
if (ret) {
@@ -519,6 +519,14 @@ static int faraday_pci_probe(struct platform_device *pdev)
if (ret)
return ret;

+ list_splice_init(&res, &host->windows);
+ ret = pci_register_host_bridge(host);
+ if (ret) {
+ dev_err(dev, "failed to register host: %d\n", ret);
+ return ret;
+ }
+ p->bus = host->bus;
+
pci_scan_child_bus(p->bus);
pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
pci_bus_assign_resources(p->bus);
--
2.10.0
Linus Walleij
2017-06-16 09:05:02 UTC
Permalink
On Thu, Jun 8, 2017 at 4:13 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Current ftpci100 driver host bridge controller driver requires struct
pci_bus to be created in order to mask and clear IRQs using standard
PCI bus config accessors.
This struct pci_bus dependency is fictitious and burdens the driver
with unneeded constraints (eg to use separate APIs to create and scan
the root bus).
Add PCI raw config space accessors to PCIe ftpci100 driver and remove the
fictitious struct pci_bus dependency.
This makes all kind of sense so:
Acked-by: Linus Walleij <***@linaro.org>

I'm hoping to be able to pull in your branch on my board
tree and actually test it too. The semantic change is a bit
scary.

Yours,
Linus Walleij
Lorenzo Pieralisi
2017-06-08 14:13:04 UTC
Permalink
The introduction of pci_register_host_bridge() kernel interface
allows PCI host controller drivers to create the struct pci_host_bridge
object, initialize it and register it with the kernel so that its
corresponding PCI bus can be scanned and its devices probed.

The host bridge device release function pci_release_host_bridge_dev is a
static function common for all struct pci_host_bridge allocated objects,
so in its current form cannot be used by PCI host bridge controllers
drivers to initialize the allocated struct pci_host_bridge, which
leaves struct pci_host_bridge devices release function uninitialized.

Since pci_release_host_bridge_dev is a function common to all PCI
host bridge objects, initialize it in pci_alloc_host_bridge() (ie
common host bridge allocation interface) so that all struct
pci_host_bridge objects have their release function initialized by
default at allocation time, removing the need for exporting the
common pci_release_host_bridge_dev function to other compilation
units.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Arnd Bergmann <***@arndb.de>
Cc: Bjorn Helgaas <***@google.com>
---
drivers/pci/probe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 19c8950..586d83d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -531,6 +531,7 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
return NULL;

INIT_LIST_HEAD(&bridge->windows);
+ bridge->dev.release = pci_release_host_bridge_dev;

return bridge;
}
@@ -2310,7 +2311,6 @@ static struct pci_bus *pci_create_root_bus_msi(struct device *parent,
return NULL;

bridge->dev.parent = parent;
- bridge->dev.release = pci_release_host_bridge_dev;

list_splice_init(resources, &bridge->windows);
bridge->sysdata = sysdata;
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:07 UTC
Permalink
When probing the PCI host controller driver, if an error condition
is trigger the probe function code does not free memory allocated
for the struct pci_host_bridge resulting in memory leakage.

Move the struct pci_host_bridge allocation over to the respective
devm interface to fix the issue.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Arnd Bergmann <***@arndb.de>
Cc: Bjorn Helgaas <***@google.com>
---
drivers/pci/host/pci-ftpci100.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index 1c1fd60..89cbb1f 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -441,7 +441,7 @@ static int faraday_pci_probe(struct platform_device *pdev)
u32 val;
LIST_HEAD(res);

- host = pci_alloc_host_bridge(sizeof(*p));
+ host = devm_pci_alloc_host_bridge(dev, sizeof(*p));
if (!host)
return -ENOMEM;
--
2.10.0
Linus Walleij
2017-06-16 09:02:13 UTC
Permalink
On Thu, Jun 8, 2017 at 4:13 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
When probing the PCI host controller driver, if an error condition
is trigger the probe function code does not free memory allocated
for the struct pci_host_bridge resulting in memory leakage.
Move the struct pci_host_bridge allocation over to the respective
devm interface to fix the issue.
Acked-by: Linus Walleij <***@linaro.org>

Yours,
Linus Walleij
Lorenzo Pieralisi
2017-06-08 14:13:30 UTC
Permalink
Since, through struct pci_host_bridge.map/swizzle_irq hooks, IRQs
are now allocated in the pci_assign_irq() callback automatically,
PCI host bridge drivers can stop relying on pci_fixup_irqs()
for IRQ allocation

Drop pci_fixup_irqs() usage from PCI tegra host bridge driver.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Thierry Reding <***@gmail.com>
---
drivers/pci/host/pci-tegra.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 0383418..0dadb81 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -2284,6 +2284,8 @@ static int tegra_pcie_probe(struct platform_device *pdev)
host->busnr = pcie->busn.start;
host->dev.parent = &pdev->dev;
host->ops = &tegra_pcie_ops;
+ host->map_irq = tegra_pcie_map_irq;
+ host->swizzle_irq = pci_common_swizzle;

err = pci_scan_root_bus_bridge(host);
if (err < 0) {
@@ -2291,7 +2293,6 @@ static int tegra_pcie_probe(struct platform_device *pdev)
goto disable_msi;
}

- pci_fixup_irqs(pci_common_swizzle, tegra_pcie_map_irq);
pci_bus_size_bridges(host->bus);
pci_bus_assign_resources(host->bus);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:34 UTC
Permalink
Since, through struct pci_host_bridge.map/swizzle_irq hooks, IRQs
are now allocated in the pci_assign_irq() callback automatically,
PCI host bridge drivers can stop relying on pci_fixup_irqs()
for IRQ allocation.

Drop pci_fixup_irqs() usage from PCI designware host bridge driver.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Jingoo Han <***@gmail.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Joao Pinto <***@synopsys.com>
---
drivers/pci/dwc/pcie-designware-host.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 351a277..d29c020 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -410,6 +410,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
bridge->sysdata = pp;
bridge->busnr = pp->root_bus_nr;
bridge->ops = &dw_pcie_ops;
+ bridge->map_irq = of_irq_parse_and_map_pci;
+ bridge->swizzle_irq = pci_common_swizzle;
if (IS_ENABLED(CONFIG_PCI_MSI)) {
bridge->msi = &dw_pcie_msi_chip;
dw_pcie_msi_chip.dev = dev;
@@ -424,11 +426,6 @@ int dw_pcie_host_init(struct pcie_port *pp)
if (pp->ops->scan_bus)
pp->ops->scan_bus(pp);

-#ifdef CONFIG_ARM
- /* support old dtbs that incorrectly describe IRQs */
- pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-#endif
-
pci_bus_size_bridges(bus);
pci_bus_assign_resources(bus);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:36 UTC
Permalink
Since, through struct pci_host_bridge.map/swizzle_irq hooks, IRQs
are now allocated in the pci_assign_irq() callback automatically,
PCI host bridge drivers can stop relying on pci_fixup_irqs()
for IRQ allocation.

Drop pci_fixup_irqs() usage from PCI host-common bridge driver.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Will Deacon <***@arm.com>
---
drivers/pci/host/pci-host-common.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
index 8e2614ba..44a47d4 100644
--- a/drivers/pci/host/pci-host-common.c
+++ b/drivers/pci/host/pci-host-common.c
@@ -149,6 +149,8 @@ int pci_host_common_probe(struct platform_device *pdev,
bridge->sysdata = cfg;
bridge->busnr = cfg->busr.start;
bridge->ops = &ops->pci_ops;
+ bridge->map_irq = of_irq_parse_and_map_pci;
+ bridge->swizzle_irq = pci_common_swizzle;

ret = pci_scan_root_bus_bridge(bridge);
if (ret < 0) {
@@ -158,10 +160,6 @@ int pci_host_common_probe(struct platform_device *pdev,

bus = bridge->bus;

-#ifdef CONFIG_ARM
- pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-#endif
-
/*
* We insert PCI resources into the iomem_resource and
* ioport_resource trees in either pci_bus_claim_resources()
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:37 UTC
Permalink
Since, through struct pci_host_bridge.map/swizzle_irq hooks, IRQs
are now allocated in the pci_assign_irq() callback automatically,
PCI host bridge drivers can stop relying on pci_fixup_irqs()
for IRQ allocation.

Drop pci_fixup_irqs() usage from PCI versatile host bridge driver.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Rob Herring <***@kernel.org>
---
drivers/pci/host/pci-versatile.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-versatile.c b/drivers/pci/host/pci-versatile.c
index a67cf0b..3be25a9 100644
--- a/drivers/pci/host/pci-versatile.c
+++ b/drivers/pci/host/pci-versatile.c
@@ -209,6 +209,8 @@ static int versatile_pci_probe(struct platform_device *pdev)
bridge->sysdata = NULL;
bridge->busnr = 0;
bridge->ops = &pci_versatile_ops;
+ host->map_irq = of_irq_parse_and_map_pci;
+ host->swizzle_irq = pci_common_swizzle;

ret = pci_scan_root_bus_bridge(bridge);
if (ret < 0)
@@ -216,8 +218,6 @@ static int versatile_pci_probe(struct platform_device *pdev)

bus = bridge->bus;

- pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-
pci_assign_unassigned_bus_resources(bus);
list_for_each_entry(child, &bus->children, node)
pcie_bus_configure_settings(child);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:38 UTC
Permalink
Since, through struct pci_host_bridge.map/swizzle_irq hooks, IRQs
are now allocated in the pci_assign_irq() callback automatically,
PCI host bridge drivers can stop relying on pci_fixup_irqs()
for IRQ allocation.

Drop pci_fixup_irqs() usage from PCI altera host bridge driver.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Ley Foon Tan <***@altera.com>
---
drivers/pci/host/pcie-altera.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pcie-altera.c b/drivers/pci/host/pcie-altera.c
index 7f023b2..4ea4f8f 100644
--- a/drivers/pci/host/pcie-altera.c
+++ b/drivers/pci/host/pcie-altera.c
@@ -620,6 +620,8 @@ static int altera_pcie_probe(struct platform_device *pdev)
bridge->sysdata = pcie;
bridge->busnr = pcie->root_bus_nr;
bridge->ops = &altera_pcie_ops;
+ bridge->map_irq = of_irq_parse_and_map_pci;
+ bridge->swizzle_irq = pci_common_swizzle;

ret = pci_scan_root_bus_bridge(bridge);
if (ret < 0)
@@ -627,8 +629,6 @@ static int altera_pcie_probe(struct platform_device *pdev)

bus = bridge->bus;

- pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-
pci_assign_unassigned_bus_resources(bus);

/* Configure PCI Express setting. */
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:39 UTC
Permalink
struct pci_host_bridge gained hooks to map/swizzle IRQs, so that
the IRQ mapping can be done automatically by PCI core code through
the pci_assign_irq() function instead of resorting to per-arch
specific implementation callbacks to carry out the same task which
force PCI host bridge drivers implementation to implement per-arch
kludges to carry out a task that is inherently architecture agnostic.

Add map/swizzle IRQs hooks to the xgene PCI host driver to move
the IRQ allocation into core code and stop relying on arch specific
callbacks.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Tanmay Inamdar <***@apm.com>
---
drivers/pci/host/pci-xgene.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 262bedf..bd89747 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -678,6 +678,8 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
bridge->sysdata = port;
bridge->busnr = 0;
bridge->ops = &xgene_pcie_ops;
+ bridge->map_irq = of_irq_parse_and_map_pci;
+ bridge->swizzle_irq = pci_common_swizzle;

ret = pci_scan_root_bus_bridge(bridge);
if (ret < 0)
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:41 UTC
Permalink
struct pci_host_bridge gained hooks to map/swizzle IRQs, so that
the IRQ mapping can be done automatically by PCI core code through
the pci_assign_irq() function instead of resorting to per-arch
specific implementation callbacks to carry out the same task which
force PCI host bridge drivers implementation to implement per-arch
kludges to carry out a task that is inherently architecture agnostic.

Add map/swizzle IRQs hooks to the xilinx-nwl PCI host driver to move
the IRQ allocation into core code and stop relying on arch specific
callbacks.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Bharat Kumar Gogada <***@xilinx.com>
---
drivers/pci/host/pcie-xilinx-nwl.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-nwl.c
index e8a5894..d1f7e4c 100644
--- a/drivers/pci/host/pcie-xilinx-nwl.c
+++ b/drivers/pci/host/pcie-xilinx-nwl.c
@@ -838,6 +838,8 @@ static int nwl_pcie_probe(struct platform_device *pdev)
bridge->sysdata = pcie;
bridge->busnr = pcie->root_busno;
bridge->ops = &nwl_pcie_ops;
+ bridge->map_irq = of_irq_parse_and_map_pci;
+ bridge->swizzle_irq = pci_common_swizzle;

if (IS_ENABLED(CONFIG_PCI_MSI)) {
err = nwl_pcie_enable_msi(pcie);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:42 UTC
Permalink
With the introduction of struct pci_host_bridge.map_irq pointer it is
possible to assign IRQs for all devices originating from a PCI host
bridge at probe time; this is implemented through pci_assign_irq() that
relies on the struct pci_host_bridge.map_irq pointer to map IRQ for a
given device.

The benefits this brings are twofold:

- the IRQ for a device is assigned once at probe time
- the IRQ assignment works also for hotplugged devices

With all DT based PCI host bridges converted to the struct
pci_host_bridge.{map/swizzle}_irq hooks mechanism the DT IRQ
allocation in ARM64 pcibios_alloc_irq() is now redundant and can
be removed.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Will Deacon <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
---
arch/arm64/kernel/pci.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 4f0e3eb..efcc351 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -39,20 +39,18 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
return res->start;
}

+#ifdef CONFIG_ACPI
/*
* Try to assign the IRQ number when probing a new device
*/
int pcibios_alloc_irq(struct pci_dev *dev)
{
- if (acpi_disabled)
- dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
-#ifdef CONFIG_ACPI
- else
- return acpi_pci_irq_enable(dev);
-#endif
+ if (!acpi_disabled)
+ acpi_pci_irq_enable(dev);

return 0;
}
+#endif

/*
* raw_pci_read/write - Platform-specific PCI config space access.
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:02 UTC
Permalink
Current iproc driver host bridge controller driver requires struct
pci_bus to be created in order to carry out PCI link checks with standard
PCI config space accessors.

This struct pci_bus dependency is fictitious and burdens the driver
with unneeded constraints (eg to use separate APIs to create and scan
the root bus).

Add PCI raw config space accessors to PCIe iproc driver and remove the
fictitious struct pci_bus dependency.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Scott Branden <***@broadcom.com>
Cc: Ray Jui <***@broadcom.com>
Cc: Jon Mason <***@broadcom.com>
---
drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
1 file changed, 74 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 0f39bd2..e48b8e2 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
* Note access to the configuration registers are protected at the higher layer
* by 'pci_lock' in drivers/pci/access.c
*/
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+ int busno,
unsigned int devfn,
int where)
{
- struct iproc_pcie *pcie = iproc_data(bus);
unsigned slot = PCI_SLOT(devfn);
unsigned fn = PCI_FUNC(devfn);
- unsigned busno = bus->number;
u32 val;
u16 offset;

@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
return (pcie->base + offset);
}

+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+ unsigned int devfn,
+ int where)
+{
+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+ where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 *val)
+{
+ void __iomem *addr;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr) {
+ *val = ~0;
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ }
+
+ *val = readl(addr);
+
+ if (size <= 2)
+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 val)
+{
+ void __iomem *addr;
+ u32 mask, tmp;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ if (size == 4) {
+ writel(val, addr);
+ return PCIBIOS_SUCCESSFUL;
+ }
+
+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+ tmp = readl(addr) & mask;
+ tmp |= val << ((where & 0x3) * 8);
+ writel(tmp, addr);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
int where, int size, u32 *val)
{
@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
}

static struct pci_ops iproc_pcie_ops = {
- .map_bus = iproc_pcie_map_cfg_bus,
+ .map_bus = iproc_pcie_bus_map_cfg_bus,
.read = iproc_pcie_config_read32,
.write = iproc_pcie_config_write32,
};
@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
msleep(100);
}

-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
{
struct device *dev = pcie->dev;
- u8 hdr_type;
- u32 link_ctrl, class, val;
- u16 pos = PCI_EXP_CAP, link_status;
+ u32 hdr_type, link_ctrl, link_status, class, val;
+ u16 pos = PCI_EXP_CAP;
bool link_is_active = false;

/*
@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
}

/* make sure we are not in EP mode */
- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type);
if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
return -EFAULT;
@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
#define PCI_CLASS_BRIDGE_MASK 0xffff00
#define PCI_CLASS_BRIDGE_SHIFT 8
- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, &class);
class &= ~PCI_CLASS_BRIDGE_MASK;
class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, class);

/* check link status to see if link is active */
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;

@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_TARGET_LINK_SPEED_MASK 0xf
#define PCI_TARGET_LINK_SPEED_GEN2 0x2
#define PCI_TARGET_LINK_SPEED_GEN1 0x1
- pci_bus_read_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2, 4,
&link_ctrl);
if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
PCI_TARGET_LINK_SPEED_GEN2) {
link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
- pci_bus_write_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
- link_ctrl);
+ iproc_pci_raw_config_write32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2,
+ 4, link_ctrl);
msleep(100);

- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
- &link_status);
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
}
@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
}
pcie->root_bus = bus;

- ret = iproc_pcie_check_link(pcie, bus);
+ ret = iproc_pcie_check_link(pcie);
if (ret) {
dev_err(dev, "no PCIe EP device detected\n");
goto err_rm_root_bus;
--
2.10.0
Ray Jui
2017-06-08 15:56:05 UTC
Permalink
Hi Lorenzo,

Thanks, I'll try my best to find time to test this along 15/42 and 33/42
patches. Hopefully I can get to that some time next week.

I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you changed?

If so, is there a repo I can pull them?

Thanks,

Ray
Post by Lorenzo Pieralisi
Current iproc driver host bridge controller driver requires struct
pci_bus to be created in order to carry out PCI link checks with standard
PCI config space accessors.
This struct pci_bus dependency is fictitious and burdens the driver
with unneeded constraints (eg to use separate APIs to create and scan
the root bus).
Add PCI raw config space accessors to PCIe iproc driver and remove the
fictitious struct pci_bus dependency.
---
drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
1 file changed, 74 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 0f39bd2..e48b8e2 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
* Note access to the configuration registers are protected at the higher layer
* by 'pci_lock' in drivers/pci/access.c
*/
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+ int busno,
unsigned int devfn,
int where)
{
- struct iproc_pcie *pcie = iproc_data(bus);
unsigned slot = PCI_SLOT(devfn);
unsigned fn = PCI_FUNC(devfn);
- unsigned busno = bus->number;
u32 val;
u16 offset;
@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
return (pcie->base + offset);
}
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+ unsigned int devfn,
+ int where)
+{
+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+ where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 *val)
+{
+ void __iomem *addr;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr) {
+ *val = ~0;
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ }
+
+ *val = readl(addr);
+
+ if (size <= 2)
+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 val)
+{
+ void __iomem *addr;
+ u32 mask, tmp;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ if (size == 4) {
+ writel(val, addr);
+ return PCIBIOS_SUCCESSFUL;
+ }
+
+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+ tmp = readl(addr) & mask;
+ tmp |= val << ((where & 0x3) * 8);
+ writel(tmp, addr);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
int where, int size, u32 *val)
{
@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
}
static struct pci_ops iproc_pcie_ops = {
- .map_bus = iproc_pcie_map_cfg_bus,
+ .map_bus = iproc_pcie_bus_map_cfg_bus,
.read = iproc_pcie_config_read32,
.write = iproc_pcie_config_write32,
};
@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
msleep(100);
}
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
{
struct device *dev = pcie->dev;
- u8 hdr_type;
- u32 link_ctrl, class, val;
- u16 pos = PCI_EXP_CAP, link_status;
+ u32 hdr_type, link_ctrl, link_status, class, val;
+ u16 pos = PCI_EXP_CAP;
bool link_is_active = false;
/*
@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
}
/* make sure we are not in EP mode */
- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type);
if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
return -EFAULT;
@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
#define PCI_CLASS_BRIDGE_MASK 0xffff00
#define PCI_CLASS_BRIDGE_SHIFT 8
- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, &class);
class &= ~PCI_CLASS_BRIDGE_MASK;
class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, class);
/* check link status to see if link is active */
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_TARGET_LINK_SPEED_MASK 0xf
#define PCI_TARGET_LINK_SPEED_GEN2 0x2
#define PCI_TARGET_LINK_SPEED_GEN1 0x1
- pci_bus_read_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2, 4,
&link_ctrl);
if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
PCI_TARGET_LINK_SPEED_GEN2) {
link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
- pci_bus_write_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
- link_ctrl);
+ iproc_pci_raw_config_write32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2,
+ 4, link_ctrl);
msleep(100);
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
- &link_status);
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
}
@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
}
pcie->root_bus = bus;
- ret = iproc_pcie_check_link(pcie, bus);
+ ret = iproc_pcie_check_link(pcie);
if (ret) {
dev_err(dev, "no PCIe EP device detected\n");
goto err_rm_root_bus;
Lorenzo Pieralisi
2017-06-08 16:36:14 UTC
Permalink
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you
changed?
If so, is there a repo I can pull them?
I added it in the cover letter but anyway here it is:

git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2

Thanks for testing it, please let me know.

Thank you !
Lorenzo
Post by Ray Jui
Thanks,
Ray
Post by Lorenzo Pieralisi
Current iproc driver host bridge controller driver requires struct
pci_bus to be created in order to carry out PCI link checks with standard
PCI config space accessors.
This struct pci_bus dependency is fictitious and burdens the driver
with unneeded constraints (eg to use separate APIs to create and scan
the root bus).
Add PCI raw config space accessors to PCIe iproc driver and remove the
fictitious struct pci_bus dependency.
---
drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
1 file changed, 74 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 0f39bd2..e48b8e2 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
* Note access to the configuration registers are protected at the higher layer
* by 'pci_lock' in drivers/pci/access.c
*/
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+ int busno,
unsigned int devfn,
int where)
{
- struct iproc_pcie *pcie = iproc_data(bus);
unsigned slot = PCI_SLOT(devfn);
unsigned fn = PCI_FUNC(devfn);
- unsigned busno = bus->number;
u32 val;
u16 offset;
@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
return (pcie->base + offset);
}
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+ unsigned int devfn,
+ int where)
+{
+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+ where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 *val)
+{
+ void __iomem *addr;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr) {
+ *val = ~0;
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ }
+
+ *val = readl(addr);
+
+ if (size <= 2)
+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 val)
+{
+ void __iomem *addr;
+ u32 mask, tmp;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ if (size == 4) {
+ writel(val, addr);
+ return PCIBIOS_SUCCESSFUL;
+ }
+
+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+ tmp = readl(addr) & mask;
+ tmp |= val << ((where & 0x3) * 8);
+ writel(tmp, addr);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
int where, int size, u32 *val)
{
@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
}
static struct pci_ops iproc_pcie_ops = {
- .map_bus = iproc_pcie_map_cfg_bus,
+ .map_bus = iproc_pcie_bus_map_cfg_bus,
.read = iproc_pcie_config_read32,
.write = iproc_pcie_config_write32,
};
@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
msleep(100);
}
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
{
struct device *dev = pcie->dev;
- u8 hdr_type;
- u32 link_ctrl, class, val;
- u16 pos = PCI_EXP_CAP, link_status;
+ u32 hdr_type, link_ctrl, link_status, class, val;
+ u16 pos = PCI_EXP_CAP;
bool link_is_active = false;
/*
@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
}
/* make sure we are not in EP mode */
- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type);
if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
return -EFAULT;
@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
#define PCI_CLASS_BRIDGE_MASK 0xffff00
#define PCI_CLASS_BRIDGE_SHIFT 8
- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, &class);
class &= ~PCI_CLASS_BRIDGE_MASK;
class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, class);
/* check link status to see if link is active */
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_TARGET_LINK_SPEED_MASK 0xf
#define PCI_TARGET_LINK_SPEED_GEN2 0x2
#define PCI_TARGET_LINK_SPEED_GEN1 0x1
- pci_bus_read_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2, 4,
&link_ctrl);
if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
PCI_TARGET_LINK_SPEED_GEN2) {
link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
- pci_bus_write_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
- link_ctrl);
+ iproc_pci_raw_config_write32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2,
+ 4, link_ctrl);
msleep(100);
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
- &link_status);
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
}
@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
}
pcie->root_bus = bus;
- ret = iproc_pcie_check_link(pcie, bus);
+ ret = iproc_pcie_check_link(pcie);
if (ret) {
dev_err(dev, "no PCIe EP device detected\n");
goto err_rm_root_bus;
Oza Oza
2017-06-11 04:12:34 UTC
Permalink
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you
changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,

I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.


Hi Bjorn,

please help to review following changes, to avoid merge conflicts
since Lorenzo's changes in iproc driver is touching basic config APIs.

[PATCH v3 1/2] PCI: iproc: Retry request when CRS returned from EP
[PATCH v3 2/2] PCI: iproc: add device shutdown for PCI RC

Regards,
Oza.
Post by Lorenzo Pieralisi
Thanks for testing it, please let me know.
Thank you !
Lorenzo
Post by Ray Jui
Thanks,
Ray
Post by Lorenzo Pieralisi
Current iproc driver host bridge controller driver requires struct
pci_bus to be created in order to carry out PCI link checks with standard
PCI config space accessors.
This struct pci_bus dependency is fictitious and burdens the driver
with unneeded constraints (eg to use separate APIs to create and scan
the root bus).
Add PCI raw config space accessors to PCIe iproc driver and remove the
fictitious struct pci_bus dependency.
---
drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
1 file changed, 74 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 0f39bd2..e48b8e2 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
* Note access to the configuration registers are protected at the higher layer
* by 'pci_lock' in drivers/pci/access.c
*/
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+ int busno,
unsigned int devfn,
int where)
{
- struct iproc_pcie *pcie = iproc_data(bus);
unsigned slot = PCI_SLOT(devfn);
unsigned fn = PCI_FUNC(devfn);
- unsigned busno = bus->number;
u32 val;
u16 offset;
@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
return (pcie->base + offset);
}
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+ unsigned int devfn,
+ int where)
+{
+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+ where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 *val)
+{
+ void __iomem *addr;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr) {
+ *val = ~0;
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ }
+
+ *val = readl(addr);
+
+ if (size <= 2)
+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 val)
+{
+ void __iomem *addr;
+ u32 mask, tmp;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ if (size == 4) {
+ writel(val, addr);
+ return PCIBIOS_SUCCESSFUL;
+ }
+
+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+ tmp = readl(addr) & mask;
+ tmp |= val << ((where & 0x3) * 8);
+ writel(tmp, addr);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
int where, int size, u32 *val)
{
@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
}
static struct pci_ops iproc_pcie_ops = {
- .map_bus = iproc_pcie_map_cfg_bus,
+ .map_bus = iproc_pcie_bus_map_cfg_bus,
.read = iproc_pcie_config_read32,
.write = iproc_pcie_config_write32,
};
@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
msleep(100);
}
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
{
struct device *dev = pcie->dev;
- u8 hdr_type;
- u32 link_ctrl, class, val;
- u16 pos = PCI_EXP_CAP, link_status;
+ u32 hdr_type, link_ctrl, link_status, class, val;
+ u16 pos = PCI_EXP_CAP;
bool link_is_active = false;
/*
@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
}
/* make sure we are not in EP mode */
- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type);
if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
return -EFAULT;
@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
#define PCI_CLASS_BRIDGE_MASK 0xffff00
#define PCI_CLASS_BRIDGE_SHIFT 8
- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, &class);
class &= ~PCI_CLASS_BRIDGE_MASK;
class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, class);
/* check link status to see if link is active */
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_TARGET_LINK_SPEED_MASK 0xf
#define PCI_TARGET_LINK_SPEED_GEN2 0x2
#define PCI_TARGET_LINK_SPEED_GEN1 0x1
- pci_bus_read_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2, 4,
&link_ctrl);
if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
PCI_TARGET_LINK_SPEED_GEN2) {
link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
- pci_bus_write_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
- link_ctrl);
+ iproc_pci_raw_config_write32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2,
+ 4, link_ctrl);
msleep(100);
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
- &link_status);
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
}
@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
}
pcie->root_bus = bus;
- ret = iproc_pcie_check_link(pcie, bus);
+ ret = iproc_pcie_check_link(pcie);
if (ret) {
dev_err(dev, "no PCIe EP device detected\n");
goto err_rm_root_bus;
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Lorenzo Pieralisi
2017-06-12 16:13:58 UTC
Permalink
Post by Oza Oza
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,
I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.
Thank you. If you could also check it is all OK from a legacy
IRQ allocation (ie same as before applying series) I'd be grateful.

Thanks a lot,
Lorenzo
Post by Oza Oza
Hi Bjorn,
please help to review following changes, to avoid merge conflicts
since Lorenzo's changes in iproc driver is touching basic config APIs.
[PATCH v3 1/2] PCI: iproc: Retry request when CRS returned from EP
[PATCH v3 2/2] PCI: iproc: add device shutdown for PCI RC
Regards,
Oza.
Post by Lorenzo Pieralisi
Thanks for testing it, please let me know.
Thank you !
Lorenzo
Post by Ray Jui
Thanks,
Ray
Post by Lorenzo Pieralisi
Current iproc driver host bridge controller driver requires struct
pci_bus to be created in order to carry out PCI link checks with standard
PCI config space accessors.
This struct pci_bus dependency is fictitious and burdens the driver
with unneeded constraints (eg to use separate APIs to create and scan
the root bus).
Add PCI raw config space accessors to PCIe iproc driver and remove the
fictitious struct pci_bus dependency.
---
drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
1 file changed, 74 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 0f39bd2..e48b8e2 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
* Note access to the configuration registers are protected at the higher layer
* by 'pci_lock' in drivers/pci/access.c
*/
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+ int busno,
unsigned int devfn,
int where)
{
- struct iproc_pcie *pcie = iproc_data(bus);
unsigned slot = PCI_SLOT(devfn);
unsigned fn = PCI_FUNC(devfn);
- unsigned busno = bus->number;
u32 val;
u16 offset;
@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
return (pcie->base + offset);
}
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+ unsigned int devfn,
+ int where)
+{
+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+ where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 *val)
+{
+ void __iomem *addr;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr) {
+ *val = ~0;
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ }
+
+ *val = readl(addr);
+
+ if (size <= 2)
+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 val)
+{
+ void __iomem *addr;
+ u32 mask, tmp;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ if (size == 4) {
+ writel(val, addr);
+ return PCIBIOS_SUCCESSFUL;
+ }
+
+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+ tmp = readl(addr) & mask;
+ tmp |= val << ((where & 0x3) * 8);
+ writel(tmp, addr);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
int where, int size, u32 *val)
{
@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
}
static struct pci_ops iproc_pcie_ops = {
- .map_bus = iproc_pcie_map_cfg_bus,
+ .map_bus = iproc_pcie_bus_map_cfg_bus,
.read = iproc_pcie_config_read32,
.write = iproc_pcie_config_write32,
};
@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
msleep(100);
}
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
{
struct device *dev = pcie->dev;
- u8 hdr_type;
- u32 link_ctrl, class, val;
- u16 pos = PCI_EXP_CAP, link_status;
+ u32 hdr_type, link_ctrl, link_status, class, val;
+ u16 pos = PCI_EXP_CAP;
bool link_is_active = false;
/*
@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
}
/* make sure we are not in EP mode */
- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type);
if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
return -EFAULT;
@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
#define PCI_CLASS_BRIDGE_MASK 0xffff00
#define PCI_CLASS_BRIDGE_SHIFT 8
- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, &class);
class &= ~PCI_CLASS_BRIDGE_MASK;
class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, class);
/* check link status to see if link is active */
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_TARGET_LINK_SPEED_MASK 0xf
#define PCI_TARGET_LINK_SPEED_GEN2 0x2
#define PCI_TARGET_LINK_SPEED_GEN1 0x1
- pci_bus_read_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2, 4,
&link_ctrl);
if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
PCI_TARGET_LINK_SPEED_GEN2) {
link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
- pci_bus_write_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
- link_ctrl);
+ iproc_pci_raw_config_write32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2,
+ 4, link_ctrl);
msleep(100);
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
- &link_status);
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
}
@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
}
pcie->root_bus = bus;
- ret = iproc_pcie_check_link(pcie, bus);
+ ret = iproc_pcie_check_link(pcie);
if (ret) {
dev_err(dev, "no PCIe EP device detected\n");
goto err_rm_root_bus;
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Oza Oza
2017-06-12 17:40:03 UTC
Permalink
On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Oza Oza
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,
I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.
Thank you. If you could also check it is all OK from a legacy
IRQ allocation (ie same as before applying series) I'd be grateful.
I disabled msi and it should default to legacy IRQ, looks like there
is a problem, if I missed any change !

before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

after applying series:
***@bcm958742k:~# dmesg | grep nvme
[ 3.855466] nvme 0000:01:00.0: assign irq: got 0
[ 3.855469] nvme 0000:01:00.0: assigning IRQ 00
[ 3.855515] nvme nvme0: pci function 0000:01:00.0
[ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[ 4.276787] nvme 0000:01:00.0: enabling bus mastering
[ 4.276817] nvme nvme0: Removing after probe failure status: -22

here is my git status of your series, let me know if I am missing any
change with respect to legacy IRQ ?

modified: arch/arm/include/asm/mach/pci.h
modified: arch/arm/kernel/bios32.c
modified: arch/arm/mach-dove/pcie.c
modified: arch/arm/mach-iop13xx/pci.c
modified: arch/arm/mach-iop13xx/pci.h
modified: arch/arm/mach-mv78xx0/pcie.c
modified: arch/arm/mach-orion5x/common.h
modified: arch/arm/mach-orion5x/pci.c
modified: arch/arm64/kernel/pci.c
modified: drivers/of/of_pci_irq.c
modified: drivers/pci/Makefile
modified: drivers/pci/host/pci-aardvark.c
modified: drivers/pci/host/pci-ftpci100.c
modified: drivers/pci/host/pci-host-common.c
modified: drivers/pci/host/pci-tegra.c
modified: drivers/pci/host/pci-versatile.c
modified: drivers/pci/host/pci-xgene.c
modified: drivers/pci/host/pcie-altera.c
modified: drivers/pci/host/pcie-iproc-bcma.c
modified: drivers/pci/host/pcie-iproc-platform.c
modified: drivers/pci/host/pcie-iproc.c
modified: drivers/pci/host/pcie-iproc.h
modified: drivers/pci/host/pcie-rcar.c
modified: drivers/pci/host/pcie-rockchip.c
modified: drivers/pci/host/pcie-xilinx-nwl.c
modified: drivers/pci/host/pcie-xilinx.c
modified: drivers/pci/pci-driver.c
modified: drivers/pci/probe.c
modified: drivers/pci/setup-irq.c
modified: include/linux/pci.h
Post by Lorenzo Pieralisi
Thanks a lot,
Lorenzo
Post by Oza Oza
Hi Bjorn,
please help to review following changes, to avoid merge conflicts
since Lorenzo's changes in iproc driver is touching basic config APIs.
[PATCH v3 1/2] PCI: iproc: Retry request when CRS returned from EP
[PATCH v3 2/2] PCI: iproc: add device shutdown for PCI RC
Regards,
Oza.
Post by Lorenzo Pieralisi
Thanks for testing it, please let me know.
Thank you !
Lorenzo
Post by Ray Jui
Thanks,
Ray
Post by Lorenzo Pieralisi
Current iproc driver host bridge controller driver requires struct
pci_bus to be created in order to carry out PCI link checks with standard
PCI config space accessors.
This struct pci_bus dependency is fictitious and burdens the driver
with unneeded constraints (eg to use separate APIs to create and scan
the root bus).
Add PCI raw config space accessors to PCIe iproc driver and remove the
fictitious struct pci_bus dependency.
---
drivers/pci/host/pcie-iproc.c | 94 ++++++++++++++++++++++++++++++++++---------
1 file changed, 74 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 0f39bd2..e48b8e2 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -452,14 +452,13 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
* Note access to the configuration registers are protected at the higher layer
* by 'pci_lock' in drivers/pci/access.c
*/
-static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
+static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
+ int busno,
unsigned int devfn,
int where)
{
- struct iproc_pcie *pcie = iproc_data(bus);
unsigned slot = PCI_SLOT(devfn);
unsigned fn = PCI_FUNC(devfn);
- unsigned busno = bus->number;
u32 val;
u16 offset;
@@ -499,6 +498,58 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct pci_bus *bus,
return (pcie->base + offset);
}
+static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
+ unsigned int devfn,
+ int where)
+{
+ return iproc_pcie_map_cfg_bus(iproc_data(bus), bus->number, devfn,
+ where);
+}
+
+static int iproc_pci_raw_config_read32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 *val)
+{
+ void __iomem *addr;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr) {
+ *val = ~0;
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ }
+
+ *val = readl(addr);
+
+ if (size <= 2)
+ *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int iproc_pci_raw_config_write32(struct iproc_pcie *pcie,
+ unsigned int devfn, int where,
+ int size, u32 val)
+{
+ void __iomem *addr;
+ u32 mask, tmp;
+
+ addr = iproc_pcie_map_cfg_bus(pcie, 0, devfn, where & ~0x3);
+ if (!addr)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ if (size == 4) {
+ writel(val, addr);
+ return PCIBIOS_SUCCESSFUL;
+ }
+
+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+ tmp = readl(addr) & mask;
+ tmp |= val << ((where & 0x3) * 8);
+ writel(tmp, addr);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
static int iproc_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
int where, int size, u32 *val)
{
@@ -524,7 +575,7 @@ static int iproc_pcie_config_write32(struct pci_bus *bus, unsigned int devfn,
}
static struct pci_ops iproc_pcie_ops = {
- .map_bus = iproc_pcie_map_cfg_bus,
+ .map_bus = iproc_pcie_bus_map_cfg_bus,
.read = iproc_pcie_config_read32,
.write = iproc_pcie_config_write32,
};
@@ -556,12 +607,11 @@ static void iproc_pcie_reset(struct iproc_pcie *pcie)
msleep(100);
}
-static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
+static int iproc_pcie_check_link(struct iproc_pcie *pcie)
{
struct device *dev = pcie->dev;
- u8 hdr_type;
- u32 link_ctrl, class, val;
- u16 pos = PCI_EXP_CAP, link_status;
+ u32 hdr_type, link_ctrl, link_status, class, val;
+ u16 pos = PCI_EXP_CAP;
bool link_is_active = false;
/*
@@ -578,7 +628,7 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
}
/* make sure we are not in EP mode */
- pci_bus_read_config_byte(bus, 0, PCI_HEADER_TYPE, &hdr_type);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_HEADER_TYPE, 1, &hdr_type);
if ((hdr_type & 0x7f) != PCI_HEADER_TYPE_BRIDGE) {
dev_err(dev, "in EP mode, hdr=%#02x\n", hdr_type);
return -EFAULT;
@@ -588,13 +638,16 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_BRIDGE_CTRL_REG_OFFSET 0x43c
#define PCI_CLASS_BRIDGE_MASK 0xffff00
#define PCI_CLASS_BRIDGE_SHIFT 8
- pci_bus_read_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, &class);
+ iproc_pci_raw_config_read32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, &class);
class &= ~PCI_CLASS_BRIDGE_MASK;
class |= (PCI_CLASS_BRIDGE_PCI << PCI_CLASS_BRIDGE_SHIFT);
- pci_bus_write_config_dword(bus, 0, PCI_BRIDGE_CTRL_REG_OFFSET, class);
+ iproc_pci_raw_config_write32(pcie, 0, PCI_BRIDGE_CTRL_REG_OFFSET,
+ 4, class);
/* check link status to see if link is active */
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA, &link_status);
+ iproc_pci_raw_config_read32(pcie, 0, pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
@@ -603,20 +656,21 @@ static int iproc_pcie_check_link(struct iproc_pcie *pcie, struct pci_bus *bus)
#define PCI_TARGET_LINK_SPEED_MASK 0xf
#define PCI_TARGET_LINK_SPEED_GEN2 0x2
#define PCI_TARGET_LINK_SPEED_GEN1 0x1
- pci_bus_read_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2, 4,
&link_ctrl);
if ((link_ctrl & PCI_TARGET_LINK_SPEED_MASK) ==
PCI_TARGET_LINK_SPEED_GEN2) {
link_ctrl &= ~PCI_TARGET_LINK_SPEED_MASK;
link_ctrl |= PCI_TARGET_LINK_SPEED_GEN1;
- pci_bus_write_config_dword(bus, 0,
- pos + PCI_EXP_LNKCTL2,
- link_ctrl);
+ iproc_pci_raw_config_write32(pcie, 0,
+ pos + PCI_EXP_LNKCTL2,
+ 4, link_ctrl);
msleep(100);
- pci_bus_read_config_word(bus, 0, pos + PCI_EXP_LNKSTA,
- &link_status);
+ iproc_pci_raw_config_read32(pcie, 0,
+ pos + PCI_EXP_LNKSTA,
+ 2, &link_status);
if (link_status & PCI_EXP_LNKSTA_NLW)
link_is_active = true;
}
@@ -1260,7 +1314,7 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
}
pcie->root_bus = bus;
- ret = iproc_pcie_check_link(pcie, bus);
+ ret = iproc_pcie_check_link(pcie);
if (ret) {
dev_err(dev, "no PCIe EP device detected\n");
goto err_rm_root_bus;
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ray Jui
2017-06-12 18:52:36 UTC
Permalink
Post by Oza Oza
On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Oza Oza
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,
I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.
Thank you. If you could also check it is all OK from a legacy
IRQ allocation (ie same as before applying series) I'd be grateful.
I disabled msi and it should default to legacy IRQ, looks like there
is a problem, if I missed any change !
before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
[ 3.855466] nvme 0000:01:00.0: assign irq: got 0
[ 3.855469] nvme 0000:01:00.0: assigning IRQ 00
[ 3.855515] nvme nvme0: pci function 0000:01:00.0
[ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[ 4.276787] nvme 0000:01:00.0: enabling bus mastering
[ 4.276817] nvme nvme0: Removing after probe failure status: -22
here is my git status of your series, let me know if I am missing any
change with respect to legacy IRQ ?
modified: arch/arm/include/asm/mach/pci.h
modified: arch/arm/kernel/bios32.c
modified: arch/arm/mach-dove/pcie.c
modified: arch/arm/mach-iop13xx/pci.c
modified: arch/arm/mach-iop13xx/pci.h
modified: arch/arm/mach-mv78xx0/pcie.c
modified: arch/arm/mach-orion5x/common.h
modified: arch/arm/mach-orion5x/pci.c
modified: arch/arm64/kernel/pci.c
modified: drivers/of/of_pci_irq.c
modified: drivers/pci/Makefile
modified: drivers/pci/host/pci-aardvark.c
modified: drivers/pci/host/pci-ftpci100.c
modified: drivers/pci/host/pci-host-common.c
modified: drivers/pci/host/pci-tegra.c
modified: drivers/pci/host/pci-versatile.c
modified: drivers/pci/host/pci-xgene.c
modified: drivers/pci/host/pcie-altera.c
modified: drivers/pci/host/pcie-iproc-bcma.c
modified: drivers/pci/host/pcie-iproc-platform.c
modified: drivers/pci/host/pcie-iproc.c
modified: drivers/pci/host/pcie-iproc.h
modified: drivers/pci/host/pcie-rcar.c
modified: drivers/pci/host/pcie-rockchip.c
modified: drivers/pci/host/pcie-xilinx-nwl.c
modified: drivers/pci/host/pcie-xilinx.c
modified: drivers/pci/pci-driver.c
modified: drivers/pci/probe.c
modified: drivers/pci/setup-irq.c
modified: include/linux/pci.h
Did you test with or without my change in bcm/master to emulate legacy
interrupt through dummy IRQ domain?

Thanks,

Ray
Oza Oza
2017-06-13 08:22:41 UTC
Permalink
Post by Ray Jui
Post by Oza Oza
On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Oza Oza
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,
I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.
Thank you. If you could also check it is all OK from a legacy
IRQ allocation (ie same as before applying series) I'd be grateful.
I disabled msi and it should default to legacy IRQ, looks like there
is a problem, if I missed any change !
before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
[ 3.855466] nvme 0000:01:00.0: assign irq: got 0
[ 3.855469] nvme 0000:01:00.0: assigning IRQ 00
[ 3.855515] nvme nvme0: pci function 0000:01:00.0
[ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[ 4.276787] nvme 0000:01:00.0: enabling bus mastering
[ 4.276817] nvme nvme0: Removing after probe failure status: -22
here is my git status of your series, let me know if I am missing any
change with respect to legacy IRQ ?
modified: arch/arm/include/asm/mach/pci.h
modified: arch/arm/kernel/bios32.c
modified: arch/arm/mach-dove/pcie.c
modified: arch/arm/mach-iop13xx/pci.c
modified: arch/arm/mach-iop13xx/pci.h
modified: arch/arm/mach-mv78xx0/pcie.c
modified: arch/arm/mach-orion5x/common.h
modified: arch/arm/mach-orion5x/pci.c
modified: arch/arm64/kernel/pci.c
modified: drivers/of/of_pci_irq.c
modified: drivers/pci/Makefile
modified: drivers/pci/host/pci-aardvark.c
modified: drivers/pci/host/pci-ftpci100.c
modified: drivers/pci/host/pci-host-common.c
modified: drivers/pci/host/pci-tegra.c
modified: drivers/pci/host/pci-versatile.c
modified: drivers/pci/host/pci-xgene.c
modified: drivers/pci/host/pcie-altera.c
modified: drivers/pci/host/pcie-iproc-bcma.c
modified: drivers/pci/host/pcie-iproc-platform.c
modified: drivers/pci/host/pcie-iproc.c
modified: drivers/pci/host/pcie-iproc.h
modified: drivers/pci/host/pcie-rcar.c
modified: drivers/pci/host/pcie-rockchip.c
modified: drivers/pci/host/pcie-xilinx-nwl.c
modified: drivers/pci/host/pcie-xilinx.c
modified: drivers/pci/pci-driver.c
modified: drivers/pci/probe.c
modified: drivers/pci/setup-irq.c
modified: include/linux/pci.h
Did you test with or without my change in bcm/master to emulate legacy
interrupt through dummy IRQ domain?
Thanks,
Ray
Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now.
please mainline those changes.

Hi Lorenzo,

Legacy IRQ is working fine as well wiht your patches.

cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1

NVMe data transfer is also fine.

Regards,
Oza.
Ray Jui
2017-06-13 17:18:14 UTC
Permalink
Hi Oza/Lorenzo,
Post by Oza Oza
Post by Ray Jui
Post by Oza Oza
On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Oza Oza
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,
I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.
Thank you. If you could also check it is all OK from a legacy
IRQ allocation (ie same as before applying series) I'd be grateful.
I disabled msi and it should default to legacy IRQ, looks like there
is a problem, if I missed any change !
before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
[ 3.855466] nvme 0000:01:00.0: assign irq: got 0
[ 3.855469] nvme 0000:01:00.0: assigning IRQ 00
[ 3.855515] nvme nvme0: pci function 0000:01:00.0
[ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[ 4.276787] nvme 0000:01:00.0: enabling bus mastering
[ 4.276817] nvme nvme0: Removing after probe failure status: -22
here is my git status of your series, let me know if I am missing any
change with respect to legacy IRQ ?
modified: arch/arm/include/asm/mach/pci.h
modified: arch/arm/kernel/bios32.c
modified: arch/arm/mach-dove/pcie.c
modified: arch/arm/mach-iop13xx/pci.c
modified: arch/arm/mach-iop13xx/pci.h
modified: arch/arm/mach-mv78xx0/pcie.c
modified: arch/arm/mach-orion5x/common.h
modified: arch/arm/mach-orion5x/pci.c
modified: arch/arm64/kernel/pci.c
modified: drivers/of/of_pci_irq.c
modified: drivers/pci/Makefile
modified: drivers/pci/host/pci-aardvark.c
modified: drivers/pci/host/pci-ftpci100.c
modified: drivers/pci/host/pci-host-common.c
modified: drivers/pci/host/pci-tegra.c
modified: drivers/pci/host/pci-versatile.c
modified: drivers/pci/host/pci-xgene.c
modified: drivers/pci/host/pcie-altera.c
modified: drivers/pci/host/pcie-iproc-bcma.c
modified: drivers/pci/host/pcie-iproc-platform.c
modified: drivers/pci/host/pcie-iproc.c
modified: drivers/pci/host/pcie-iproc.h
modified: drivers/pci/host/pcie-rcar.c
modified: drivers/pci/host/pcie-rockchip.c
modified: drivers/pci/host/pcie-xilinx-nwl.c
modified: drivers/pci/host/pcie-xilinx.c
modified: drivers/pci/pci-driver.c
modified: drivers/pci/probe.c
modified: drivers/pci/setup-irq.c
modified: include/linux/pci.h
Did you test with or without my change in bcm/master to emulate legacy
interrupt through dummy IRQ domain?
Thanks,
Ray
Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now.
please mainline those changes.
Hi Lorenzo,
Legacy IRQ is working fine as well wiht your patches.
cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
NVMe data transfer is also fine.
Regards,
Oza.
Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been
sanity tested for link detection and legacy interrupt support. No
regression is seen. Thanks, Oza.

I'll send out the INTx irqdomain support patch when I have time.

Ray
Lorenzo Pieralisi
2017-06-14 13:39:38 UTC
Permalink
Post by Ray Jui
Hi Oza/Lorenzo,
Post by Oza Oza
Post by Ray Jui
Post by Oza Oza
On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Oza Oza
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,
I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.
Thank you. If you could also check it is all OK from a legacy
IRQ allocation (ie same as before applying series) I'd be grateful.
I disabled msi and it should default to legacy IRQ, looks like there
is a problem, if I missed any change !
before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
[ 3.855466] nvme 0000:01:00.0: assign irq: got 0
[ 3.855469] nvme 0000:01:00.0: assigning IRQ 00
[ 3.855515] nvme nvme0: pci function 0000:01:00.0
[ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[ 4.276787] nvme 0000:01:00.0: enabling bus mastering
[ 4.276817] nvme nvme0: Removing after probe failure status: -22
here is my git status of your series, let me know if I am missing any
change with respect to legacy IRQ ?
modified: arch/arm/include/asm/mach/pci.h
modified: arch/arm/kernel/bios32.c
modified: arch/arm/mach-dove/pcie.c
modified: arch/arm/mach-iop13xx/pci.c
modified: arch/arm/mach-iop13xx/pci.h
modified: arch/arm/mach-mv78xx0/pcie.c
modified: arch/arm/mach-orion5x/common.h
modified: arch/arm/mach-orion5x/pci.c
modified: arch/arm64/kernel/pci.c
modified: drivers/of/of_pci_irq.c
modified: drivers/pci/Makefile
modified: drivers/pci/host/pci-aardvark.c
modified: drivers/pci/host/pci-ftpci100.c
modified: drivers/pci/host/pci-host-common.c
modified: drivers/pci/host/pci-tegra.c
modified: drivers/pci/host/pci-versatile.c
modified: drivers/pci/host/pci-xgene.c
modified: drivers/pci/host/pcie-altera.c
modified: drivers/pci/host/pcie-iproc-bcma.c
modified: drivers/pci/host/pcie-iproc-platform.c
modified: drivers/pci/host/pcie-iproc.c
modified: drivers/pci/host/pcie-iproc.h
modified: drivers/pci/host/pcie-rcar.c
modified: drivers/pci/host/pcie-rockchip.c
modified: drivers/pci/host/pcie-xilinx-nwl.c
modified: drivers/pci/host/pcie-xilinx.c
modified: drivers/pci/pci-driver.c
modified: drivers/pci/probe.c
modified: drivers/pci/setup-irq.c
modified: include/linux/pci.h
Did you test with or without my change in bcm/master to emulate legacy
interrupt through dummy IRQ domain?
Thanks,
Ray
Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now.
please mainline those changes.
Hi Lorenzo,
Legacy IRQ is working fine as well wiht your patches.
cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
NVMe data transfer is also fine.
Regards,
Oza.
Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been
sanity tested for link detection and legacy interrupt support. No
regression is seen. Thanks, Oza.
Yes, that's how I take it too, I did not understand what was triggering
the first regression reported - my series should work on top of mainline
if the current code works and should not depend on any other patch
being merged - please make sure that's the case to prevent unwanted
regressions in case other patches do not make it into the mainline.

Thank you very much for your help in testing it.

Lorenzo
Post by Ray Jui
I'll send out the INTx irqdomain support patch when I have time.
Ray
Oza Oza
2017-06-21 14:39:14 UTC
Permalink
On Wed, Jun 14, 2017 at 7:09 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Ray Jui
Hi Oza/Lorenzo,
Post by Oza Oza
Post by Ray Jui
Post by Oza Oza
On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Oza Oza
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,
I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.
Thank you. If you could also check it is all OK from a legacy
IRQ allocation (ie same as before applying series) I'd be grateful.
I disabled msi and it should default to legacy IRQ, looks like there
is a problem, if I missed any change !
before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
[ 3.855466] nvme 0000:01:00.0: assign irq: got 0
[ 3.855469] nvme 0000:01:00.0: assigning IRQ 00
[ 3.855515] nvme nvme0: pci function 0000:01:00.0
[ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[ 4.276787] nvme 0000:01:00.0: enabling bus mastering
[ 4.276817] nvme nvme0: Removing after probe failure status: -22
here is my git status of your series, let me know if I am missing any
change with respect to legacy IRQ ?
modified: arch/arm/include/asm/mach/pci.h
modified: arch/arm/kernel/bios32.c
modified: arch/arm/mach-dove/pcie.c
modified: arch/arm/mach-iop13xx/pci.c
modified: arch/arm/mach-iop13xx/pci.h
modified: arch/arm/mach-mv78xx0/pcie.c
modified: arch/arm/mach-orion5x/common.h
modified: arch/arm/mach-orion5x/pci.c
modified: arch/arm64/kernel/pci.c
modified: drivers/of/of_pci_irq.c
modified: drivers/pci/Makefile
modified: drivers/pci/host/pci-aardvark.c
modified: drivers/pci/host/pci-ftpci100.c
modified: drivers/pci/host/pci-host-common.c
modified: drivers/pci/host/pci-tegra.c
modified: drivers/pci/host/pci-versatile.c
modified: drivers/pci/host/pci-xgene.c
modified: drivers/pci/host/pcie-altera.c
modified: drivers/pci/host/pcie-iproc-bcma.c
modified: drivers/pci/host/pcie-iproc-platform.c
modified: drivers/pci/host/pcie-iproc.c
modified: drivers/pci/host/pcie-iproc.h
modified: drivers/pci/host/pcie-rcar.c
modified: drivers/pci/host/pcie-rockchip.c
modified: drivers/pci/host/pcie-xilinx-nwl.c
modified: drivers/pci/host/pcie-xilinx.c
modified: drivers/pci/pci-driver.c
modified: drivers/pci/probe.c
modified: drivers/pci/setup-irq.c
modified: include/linux/pci.h
Did you test with or without my change in bcm/master to emulate legacy
interrupt through dummy IRQ domain?
Thanks,
Ray
Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now.
please mainline those changes.
Hi Lorenzo,
Legacy IRQ is working fine as well wiht your patches.
cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
NVMe data transfer is also fine.
Regards,
Oza.
Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been
sanity tested for link detection and legacy interrupt support. No
regression is seen. Thanks, Oza.
Yes, that's how I take it too, I did not understand what was triggering
the first regression reported - my series should work on top of mainline
if the current code works and should not depend on any other patch
being merged - please make sure that's the case to prevent unwanted
regressions in case other patches do not make it into the mainline.
Thank you very much for your help in testing it.
Hi Lorenzo,

I have ported and tested my inbound memory patches on top of your
patches and that work fine as well.
This is mainly to do with IOVA reservations.

Regards,
Oza.
Post by Lorenzo Pieralisi
Lorenzo
Post by Ray Jui
I'll send out the INTx irqdomain support patch when I have time.
Ray
Oza Oza
2017-07-19 12:13:32 UTC
Permalink
Post by Oza Oza
On Wed, Jun 14, 2017 at 7:09 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Ray Jui
Hi Oza/Lorenzo,
Post by Oza Oza
Post by Ray Jui
Post by Oza Oza
On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Oza Oza
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,
I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.
Thank you. If you could also check it is all OK from a legacy
IRQ allocation (ie same as before applying series) I'd be grateful.
I disabled msi and it should default to legacy IRQ, looks like there
is a problem, if I missed any change !
before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
[ 3.855466] nvme 0000:01:00.0: assign irq: got 0
[ 3.855469] nvme 0000:01:00.0: assigning IRQ 00
[ 3.855515] nvme nvme0: pci function 0000:01:00.0
[ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[ 4.276787] nvme 0000:01:00.0: enabling bus mastering
[ 4.276817] nvme nvme0: Removing after probe failure status: -22
here is my git status of your series, let me know if I am missing any
change with respect to legacy IRQ ?
modified: arch/arm/include/asm/mach/pci.h
modified: arch/arm/kernel/bios32.c
modified: arch/arm/mach-dove/pcie.c
modified: arch/arm/mach-iop13xx/pci.c
modified: arch/arm/mach-iop13xx/pci.h
modified: arch/arm/mach-mv78xx0/pcie.c
modified: arch/arm/mach-orion5x/common.h
modified: arch/arm/mach-orion5x/pci.c
modified: arch/arm64/kernel/pci.c
modified: drivers/of/of_pci_irq.c
modified: drivers/pci/Makefile
modified: drivers/pci/host/pci-aardvark.c
modified: drivers/pci/host/pci-ftpci100.c
modified: drivers/pci/host/pci-host-common.c
modified: drivers/pci/host/pci-tegra.c
modified: drivers/pci/host/pci-versatile.c
modified: drivers/pci/host/pci-xgene.c
modified: drivers/pci/host/pcie-altera.c
modified: drivers/pci/host/pcie-iproc-bcma.c
modified: drivers/pci/host/pcie-iproc-platform.c
modified: drivers/pci/host/pcie-iproc.c
modified: drivers/pci/host/pcie-iproc.h
modified: drivers/pci/host/pcie-rcar.c
modified: drivers/pci/host/pcie-rockchip.c
modified: drivers/pci/host/pcie-xilinx-nwl.c
modified: drivers/pci/host/pcie-xilinx.c
modified: drivers/pci/pci-driver.c
modified: drivers/pci/probe.c
modified: drivers/pci/setup-irq.c
modified: include/linux/pci.h
Did you test with or without my change in bcm/master to emulate legacy
interrupt through dummy IRQ domain?
Thanks,
Ray
Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now.
please mainline those changes.
Hi Lorenzo,
Legacy IRQ is working fine as well wiht your patches.
cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
NVMe data transfer is also fine.
Regards,
Oza.
Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been
sanity tested for link detection and legacy interrupt support. No
regression is seen. Thanks, Oza.
Yes, that's how I take it too, I did not understand what was triggering
the first regression reported - my series should work on top of mainline
if the current code works and should not depend on any other patch
being merged - please make sure that's the case to prevent unwanted
regressions in case other patches do not make it into the mainline.
Thank you very much for your help in testing it.
Hi Lorenzo,
I have ported and tested my inbound memory patches on top of your
patches and that work fine as well.
This is mainly to do with IOVA reservations.
Regards,
Oza.
Post by Lorenzo Pieralisi
Lorenzo
Post by Ray Jui
I'll send out the INTx irqdomain support patch when I have time.
Ray
Hi Bjorn,

I have made v8 for inbound memory patches as you suggested, on top of
Lorenzo's patches.
but I can not post them since Lorenzo's patches have not made in.

Also gentle reminder to review following series
[PATCH v5 1/2] PCI: iproc: Retry request when CRS returned from EP

because that also conflict with Lorenzo's changes.

Regards,
Oza.
Lorenzo Pieralisi
2017-07-19 17:48:11 UTC
Permalink
Post by Oza Oza
Post by Oza Oza
On Wed, Jun 14, 2017 at 7:09 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Ray Jui
Hi Oza/Lorenzo,
Post by Oza Oza
Post by Ray Jui
Post by Oza Oza
On Mon, Jun 12, 2017 at 9:43 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Oza Oza
On Thu, Jun 8, 2017 at 10:06 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
[dropped rock-chips maintainers, email bounces]
Post by Ray Jui
Hi Lorenzo,
Thanks, I'll try my best to find time to test this along 15/42 and
33/42 patches. Hopefully I can get to that some time next week.
I have not yet reviewed these patches in details. Do they have
dependency on other patches to the generic framework code you
changed?
If so, is there a repo I can pull them?
git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Hi Lorenzo,
I picked up your changes, and tested on iproc based SOC,
and ran basic fio data transfer.. and it looks okay.
Thank you. If you could also check it is all OK from a legacy
IRQ allocation (ie same as before applying series) I'd be grateful.
I disabled msi and it should default to legacy IRQ, looks like there
is a problem, if I missed any change !
before applying series: cat /proc/interrupts
377: 168 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
[ 3.855466] nvme 0000:01:00.0: assign irq: got 0
[ 3.855469] nvme 0000:01:00.0: assigning IRQ 00
[ 3.855515] nvme nvme0: pci function 0000:01:00.0
[ 4.270850] nvme 0000:01:00.0: enabling device (0000 -> 0002)
[ 4.276787] nvme 0000:01:00.0: enabling bus mastering
[ 4.276817] nvme nvme0: Removing after probe failure status: -22
here is my git status of your series, let me know if I am missing any
change with respect to legacy IRQ ?
modified: arch/arm/include/asm/mach/pci.h
modified: arch/arm/kernel/bios32.c
modified: arch/arm/mach-dove/pcie.c
modified: arch/arm/mach-iop13xx/pci.c
modified: arch/arm/mach-iop13xx/pci.h
modified: arch/arm/mach-mv78xx0/pcie.c
modified: arch/arm/mach-orion5x/common.h
modified: arch/arm/mach-orion5x/pci.c
modified: arch/arm64/kernel/pci.c
modified: drivers/of/of_pci_irq.c
modified: drivers/pci/Makefile
modified: drivers/pci/host/pci-aardvark.c
modified: drivers/pci/host/pci-ftpci100.c
modified: drivers/pci/host/pci-host-common.c
modified: drivers/pci/host/pci-tegra.c
modified: drivers/pci/host/pci-versatile.c
modified: drivers/pci/host/pci-xgene.c
modified: drivers/pci/host/pcie-altera.c
modified: drivers/pci/host/pcie-iproc-bcma.c
modified: drivers/pci/host/pcie-iproc-platform.c
modified: drivers/pci/host/pcie-iproc.c
modified: drivers/pci/host/pcie-iproc.h
modified: drivers/pci/host/pcie-rcar.c
modified: drivers/pci/host/pcie-rockchip.c
modified: drivers/pci/host/pcie-xilinx-nwl.c
modified: drivers/pci/host/pcie-xilinx.c
modified: drivers/pci/pci-driver.c
modified: drivers/pci/probe.c
modified: drivers/pci/setup-irq.c
modified: include/linux/pci.h
Did you test with or without my change in bcm/master to emulate legacy
interrupt through dummy IRQ domain?
Thanks,
Ray
Hi Ray,
yes irqdomain was missing, and with that legacy IRQ are fine now.
please mainline those changes.
Hi Lorenzo,
Legacy IRQ is working fine as well wiht your patches.
cat /proc/interrupts
122: 160 0 0 0 0 0
0 0 dummy 1 Edge nvme0q0, nvme0q1
NVMe data transfer is also fine.
Regards,
Oza.
Okay, so I take this as Lorenzo's changes on iProc PCIe driver have been
sanity tested for link detection and legacy interrupt support. No
regression is seen. Thanks, Oza.
Yes, that's how I take it too, I did not understand what was triggering
the first regression reported - my series should work on top of mainline
if the current code works and should not depend on any other patch
being merged - please make sure that's the case to prevent unwanted
regressions in case other patches do not make it into the mainline.
Thank you very much for your help in testing it.
Hi Lorenzo,
I have ported and tested my inbound memory patches on top of your
patches and that work fine as well.
This is mainly to do with IOVA reservations.
Regards,
Oza.
Post by Lorenzo Pieralisi
Lorenzo
Post by Ray Jui
I'll send out the INTx irqdomain support patch when I have time.
Ray
Hi Bjorn,
I have made v8 for inbound memory patches as you suggested, on top of
Lorenzo's patches.
but I can not post them since Lorenzo's patches have not made in.
It depends on what kernel you are pulling. If you pulled v4.13-rc1
this series would be there.

Lorenzo

Lorenzo Pieralisi
2017-06-08 14:13:09 UTC
Permalink
Current pci_scan_root_bus() interface is made up of two main
code paths:

- pci_create_root_bus()
- pci_scan_child_bus()

pci_create_root_bus() is a wrapper function that allows to create
a struct pci_host_bridge structure, initialize it with the passed
parameters and register it with the kernel.

As the struct pci_host_bridge require additional struct members,
pci_create_root_bus() parameters list has grown in time, making
it unwieldy to add further parameters to it in case the struct
pci_host_bridge gains more members fields to augment its functionality.

Since PCI core code provides functions to allocate struct
pci_host_bridge, instead of forcing the pci_create_root_bus() interface
to add new parameters to cater for new struct pci_host_bridge
functionality, it is more suitable to add an interface in PCI
core code to scan a PCI bus straight from a struct pci_host_bridge
created and customized by each specific PCI host controller driver.

Add a pci_scan_root_bus_bridge() function to allow PCI host controller
drivers to create and initialize struct pci_host_bridge and scan
the resulting bus.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Arnd Bergmann <***@arndb.de>
Cc: Bjorn Helgaas <***@google.com>
---
drivers/pci/probe.c | 39 +++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 40 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e3f6965..690f0b3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2426,6 +2426,45 @@ void pci_bus_release_busn_res(struct pci_bus *b)
res, ret ? "can not be" : "is");
}

+int pci_scan_root_bus_bridge(struct pci_host_bridge *bridge)
+{
+ struct resource_entry *window;
+ bool found = false;
+ struct pci_bus *b;
+ int max, bus, ret;
+
+ if (!bridge)
+ return -EINVAL;
+
+ resource_list_for_each_entry(window, &bridge->windows)
+ if (window->res->flags & IORESOURCE_BUS) {
+ found = true;
+ break;
+ }
+
+ ret = pci_register_host_bridge(bridge);
+ if (ret < 0)
+ return ret;
+
+ b = bridge->bus;
+ bus = bridge->busnr;
+
+ if (!found) {
+ dev_info(&b->dev,
+ "No busn resource found for root bus, will use [bus %02x-ff]\n",
+ bus);
+ pci_bus_insert_busn_res(b, bus, 255);
+ }
+
+ max = pci_scan_child_bus(b);
+
+ if (!found)
+ pci_bus_update_busn_res_end(b, max);
+
+ return 0;
+}
+EXPORT_SYMBOL(pci_scan_root_bus_bridge);
+
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)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b21308e..72940cc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -862,6 +862,7 @@ struct pci_bus *pci_scan_root_bus_msi(struct device *parent, int bus,
struct pci_bus *pci_scan_root_bus(struct device *parent, int bus,
struct pci_ops *ops, void *sysdata,
struct list_head *resources);
+int pci_scan_root_bus_bridge(struct pci_host_bridge *bridge);
struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
int busnr);
void pcie_update_link_speed(struct pci_bus *bus, u16 link_status);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:21 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI rockchip host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Wenrui Li <***@rock-chips.com>
Cc: Shawn Lin <***@rock-chips.com>
---
drivers/pci/host/pcie-rockchip.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 0e020b6..cae5a6d 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -1284,6 +1284,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
struct rockchip_pcie *rockchip;
struct device *dev = &pdev->dev;
struct pci_bus *bus, *child;
+ struct pci_host_bridge *bridge;
struct resource_entry *win;
resource_size_t io_base;
struct resource *mem;
@@ -1295,10 +1296,12 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
if (!dev->of_node)
return -ENODEV;

- rockchip = devm_kzalloc(dev, sizeof(*rockchip), GFP_KERNEL);
- if (!rockchip)
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rockchip));
+ if (!bridge)
return -ENOMEM;

+ rockchip = pci_host_bridge_priv(bridge);
+
platform_set_drvdata(pdev, rockchip);
rockchip->dev = dev;

@@ -1396,11 +1399,18 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
goto err_free_res;
}

- bus = pci_scan_root_bus(&pdev->dev, 0, &rockchip_pcie_ops, rockchip, &res);
- if (!bus) {
- err = -ENOMEM;
+ list_splice_init(&res, &bridge->windows);
+ bridge->dev.parent = &pdev->dev;
+ bridge->sysdata = rockchip;
+ bridge->busnr = 0;
+ bridge->ops = &rockchip_pcie_ops;
+
+ err = pci_scan_root_bus_bridge(bridge);
+ if (!err)
goto err_free_res;
- }
+
+ bus = bridge->bus;
+
rockchip->root_bus = bus;

pci_bus_size_bridges(bus);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:40 UTC
Permalink
struct pci_host_bridge gained hooks to map/swizzle IRQs, so that
the IRQ mapping can be done automatically by PCI core code through
the pci_assign_irq() function instead of resorting to per-arch
specific implementation callbacks to carry out the same task which
force PCI host bridge drivers implementation to implement per-arch
kludges to carry out a task that is inherently architecture agnostic.

Add map/swizzle IRQs hooks to the rockchip PCI host driver to move
the IRQ allocation into core code and stop relying on arch specific
callbacks.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Wenrui Li <***@rock-chips.com>
Cc: Shawn Lin <***@rock-chips.com>
---
drivers/pci/host/pcie-rockchip.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index cae5a6d..29332ba 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -1404,6 +1404,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
bridge->sysdata = rockchip;
bridge->busnr = 0;
bridge->ops = &rockchip_pcie_ops;
+ bridge->map_irq = of_irq_parse_and_map_pci;
+ bridge->swizzle_irq = pci_common_swizzle;

err = pci_scan_root_bus_bridge(bridge);
if (!err)
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:31 UTC
Permalink
Since, through struct pci_host_bridge.map/swizzle_irq hooks, IRQs
are now allocated in the pci_assign_irq() callback automatically,
PCI host bridge drivers can stop relying on pci_fixup_irqs()
for IRQ allocation.

Drop pci_fixup_irqs() usage from PCI xilinx host bridge driver.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Michal Simek <***@xilinx.com>
---
drivers/pci/host/pcie-xilinx.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 761f048..d09b005 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -680,6 +680,8 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
bridge->sysdata = port;
bridge->busnr = 0;
bridge->ops = &xilinx_pcie_ops;
+ bridge->map_irq = of_irq_parse_and_map_pci;
+ bridge->swizzle_irq = pci_common_swizzle;

#ifdef CONFIG_PCI_MSI
xilinx_pcie_msi_chip.dev = dev;
@@ -692,9 +694,6 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
bus = bridge->bus;

pci_assign_unassigned_bus_resources(bus);
-#ifndef CONFIG_MICROBLAZE
- pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-#endif
list_for_each_entry(child, &bus->children, node)
pcie_bus_configure_settings(child);
pci_bus_add_devices(bus);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:35 UTC
Permalink
Since, through struct pci_host_bridge.map/swizzle_irq hooks, IRQs
are now allocated in the pci_assign_irq() callback automatically,
PCI host bridge drivers can stop relying on pci_fixup_irqs()
for IRQ allocation.

Drop pci_fixup_irqs() usage from PCI ftpci100 host bridge driver.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Linus Walleij <***@linaro.org>
---
drivers/pci/host/pci-ftpci100.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index 435a646..9cfc90a 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -449,6 +449,8 @@ static int faraday_pci_probe(struct platform_device *pdev)
host->ops = &faraday_pci_ops;
host->busnr = 0;
host->msi = NULL;
+ host->map_irq = of_irq_parse_and_map_pci;
+ host->swizzle_irq = pci_common_swizzle;
p = pci_host_bridge_priv(host);
host->sysdata = p;
p->dev = dev;
@@ -527,7 +529,6 @@ static int faraday_pci_probe(struct platform_device *pdev)
}
p->bus = host->bus;

- pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
pci_bus_assign_resources(p->bus);
pci_bus_add_devices(p->bus);
pci_free_resource_list(&res);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:26 UTC
Permalink
From: Matthew Minter <***@masarand.com>

Here we delete the static pdev_fixup_irq() function which is currently
what pci_fixup_irqs() uses to actually assign the irqs and replace it
with the pci_assign_irq() function which changes the interface and
makes use of the new function pointers stored in the host bridge
structure.

Eventually this will allow pci_fixup_irqs() to be removed entirely
and the new deferred assignment code path will call pci_assign_irq
directly. However to ensure current users continue to work a
new implementation of pci_fixup_irqs() is introduced which simply
wraps the functionality of pci_assign_irq().

Signed-off-by: Matthew Minter <***@masarand.com>
[***@arm.com: reworked comments/log]
Signed-off-by: Lorenzo Pieralisi <***@arm.com>
---
drivers/pci/setup-irq.c | 45 +++++++++++++++++++++++++++++++++++----------
include/linux/pci.h | 1 +
2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c
index 95c225b..5eeee79 100644
--- a/drivers/pci/setup-irq.c
+++ b/drivers/pci/setup-irq.c
@@ -15,6 +15,7 @@
#include <linux/errno.h>
#include <linux/ioport.h>
#include <linux/cache.h>
+#include "pci.h"

void __weak pcibios_update_irq(struct pci_dev *dev, int irq)
{
@@ -22,12 +23,17 @@ void __weak pcibios_update_irq(struct pci_dev *dev, int irq)
pci_write_config_byte(dev, PCI_INTERRUPT_LINE, irq);
}

-static void pdev_fixup_irq(struct pci_dev *dev,
- u8 (*swizzle)(struct pci_dev *, u8 *),
- int (*map_irq)(const struct pci_dev *, u8, u8))
+void pci_assign_irq(struct pci_dev *dev)
{
- u8 pin, slot;
+ u8 pin;
+ u8 slot = -1;
int irq = 0;
+ struct pci_host_bridge *hbrg = pci_find_host_bridge(dev->bus);
+
+ if (!(hbrg->map_irq)) {
+ dev_dbg(&dev->dev, "runtime irq mapping not provided by arch\n");
+ return;
+ }

/* If this device is not on the primary bus, we need to figure out
which interrupt pin it will come in on. We know which slot it
@@ -40,17 +46,22 @@ static void pdev_fixup_irq(struct pci_dev *dev,
if (pin > 4)
pin = 1;

- if (pin != 0) {
+ if (pin) {
/* Follow the chain of bridges, swizzling as we go. */
- slot = (*swizzle)(dev, &pin);
+ if (hbrg->swizzle_irq)
+ slot = (*(hbrg->swizzle_irq))(dev, &pin);

- irq = (*map_irq)(dev, slot, pin);
+ /*
+ * If a swizzling function is not used map_irq must
+ * ignore slot
+ */
+ irq = (*(hbrg->map_irq))(dev, slot, pin);
if (irq == -1)
irq = 0;
}
dev->irq = irq;

- dev_dbg(&dev->dev, "fixup irq: got %d\n", dev->irq);
+ dev_dbg(&dev->dev, "assign irq: got %d\n", dev->irq);

/* Always tell the device, so the driver knows what is
the real IRQ to use; the device does not use it. */
@@ -60,9 +71,23 @@ static void pdev_fixup_irq(struct pci_dev *dev,
void pci_fixup_irqs(u8 (*swizzle)(struct pci_dev *, u8 *),
int (*map_irq)(const struct pci_dev *, u8, u8))
{
+ /*
+ * Implement pci_fixup_irqs() through pci_assign_irq().
+ * This code should be remove eventually, it is a wrapper
+ * around pci_assign_irq() interface to keep current
+ * pci_fixup_irqs() behaviour unchanged on architecture
+ * code still relying on its interface.
+ */
struct pci_dev *dev = NULL;
+ struct pci_host_bridge *hbrg = NULL;

- for_each_pci_dev(dev)
- pdev_fixup_irq(dev, swizzle, map_irq);
+ for_each_pci_dev(dev) {
+ hbrg = pci_find_host_bridge(dev->bus);
+ hbrg->swizzle_irq = swizzle;
+ hbrg->map_irq = map_irq;
+ pci_assign_irq(dev);
+ hbrg->swizzle_irq = NULL;
+ hbrg->map_irq = NULL;
+ }
}
EXPORT_SYMBOL_GPL(pci_fixup_irqs);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 72b4796..23baddc 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1152,6 +1152,7 @@ void pdev_enable_device(struct pci_dev *);
int pci_enable_resources(struct pci_dev *, int mask);
void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
int (*)(const struct pci_dev *, u8, u8));
+void pci_assign_irq(struct pci_dev *dev);
struct resource *pci_find_resource(struct pci_dev *dev, struct resource *res);
#define HAVE_PCI_REQ_REGIONS 2
int __must_check pci_request_regions(struct pci_dev *, const char *);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:25 UTC
Permalink
From: Matthew Minter <***@masarand.com>

In order to defer IRQ assignment arches must be able to register functions
to map and swizzle interrupts. These registered functions are stored in
the pci_host_bridge struct.

Signed-off-by: Matthew Minter <***@masarand.com>
---
include/linux/pci.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 31ee3b3..72b4796 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -437,6 +437,8 @@ struct pci_host_bridge {
void *sysdata;
int busnr;
struct list_head windows; /* resource_entry */
+ u8 (*swizzle_irq)(struct pci_dev *, u8 *); /* platform irq swizzler */
+ int (*map_irq)(const struct pci_dev *, u8, u8);
void (*release_fn)(struct pci_host_bridge *);
void *release_data;
struct msi_controller *msi;
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:29 UTC
Permalink
Legacy PCI host controllers (ie host controllers that set-up the PCI bus
through the ARM pci_common_init() API) are currently relying on
pci_fixup_irqs() to assign legacy PCI irqs to devices. This is not ideal
in that pci_fixup_irqs() assign IRQs for all PCI devices present in a
given system some of which may well be enabled by the time
pci_fixup_irqs() is called (ie a system with multiple host controllers).
With the introduction of struct pci_host_bridge.(*map_irq) pointer it is
possible to assign IRQs for all devices originating from a PCI host
bridge at probe time; this is implemented through pci_assign_irq() that
relies on the struct pci_host_bridge.map_irq pointer to map IRQ for a
given device.

The benefits this brings are twofold:

- the IRQ for a device is assigned once at probe time
- the IRQ assignment works also for hotplugged devices

Remove pci_fixup_irqs() call from bios32 code and rely on
pci_assign_irq() to carry out the IRQ mapping at device probe
time.

The map_irq() and swizzle_irq() struct pci_host_bridge callbacks are
set-up in the struct pci_host_bridge created in the bios32
pcibios_init_hw() function and mach-* code paths (for PCI mach
implementations that require a specific struct hw_pci.(*scan) function
callback).

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Jason Cooper <***@lakedaemon.net>
Cc: Bjorn Helgaas <***@google.com>
Cc: Russell King <***@armlinux.org.uk>
Cc: Andrew Lunn <***@lunn.ch>
---
arch/arm/kernel/bios32.c | 4 ++--
arch/arm/mach-dove/pcie.c | 18 ++++++++++--------
arch/arm/mach-iop13xx/pci.c | 2 ++
arch/arm/mach-orion5x/pci.c | 28 +++++++++++++++-------------
4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 4c7621a..da201a2 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -494,6 +494,8 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
bridge->busnr = sys->busnr;
bridge->ops = hw->ops;
bridge->msi = hw->msi_ctrl;
+ bridge->map_irq = pcibios_map_irq;
+ bridge->swizzle_irq = pcibios_swizzle;
bridge->align_resource =
hw->align_resource;

@@ -530,8 +532,6 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw)
if (hw->postinit)
hw->postinit();

- pci_fixup_irqs(pcibios_swizzle, pcibios_map_irq);
-
list_for_each_entry(sys, &head, node) {
struct pci_bus *bus = sys->bus;

diff --git a/arch/arm/mach-dove/pcie.c b/arch/arm/mach-dove/pcie.c
index dfb62f3..8d54a67 100644
--- a/arch/arm/mach-dove/pcie.c
+++ b/arch/arm/mach-dove/pcie.c
@@ -152,6 +152,14 @@ static void rc_pci_fixup(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL, PCI_ANY_ID, rc_pci_fixup);

+static int __init dove_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+{
+ struct pci_sys_data *sys = dev->sysdata;
+ struct pcie_port *pp = sys->private_data;
+
+ return pp->index ? IRQ_DOVE_PCIE1 : IRQ_DOVE_PCIE0;
+}
+
static int __init
dove_pcie_scan_bus(int nr, struct pci_host_bridge *bridge)
{
@@ -167,18 +175,12 @@ dove_pcie_scan_bus(int nr, struct pci_host_bridge *bridge)
bridge->sysdata = sys;
bridge->busnr = sys->busnr;
bridge->ops = &pcie_ops;
+ bridge->map_irq = dove_pcie_map_irq;
+ bridge->swizzle_irq = pci_common_swizzle;

return pci_scan_root_bus_bridge(bridge);
}

-static int __init dove_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-{
- struct pci_sys_data *sys = dev->sysdata;
- struct pcie_port *pp = sys->private_data;
-
- return pp->index ? IRQ_DOVE_PCIE1 : IRQ_DOVE_PCIE0;
-}
-
static struct hw_pci dove_pci __initdata = {
.nr_controllers = 2,
.setup = dove_pcie_setup,
diff --git a/arch/arm/mach-iop13xx/pci.c b/arch/arm/mach-iop13xx/pci.c
index 4babf15..b97879b 100644
--- a/arch/arm/mach-iop13xx/pci.c
+++ b/arch/arm/mach-iop13xx/pci.c
@@ -532,6 +532,7 @@ int iop13xx_scan_bus(int nr, struct pci_host_bridge *bridge)
bridge->dev.parent = NULL;
bridge->sysdata = sys;
bridge->busnr = sys->busnr;
+ bridge->swizzle_irq = pci_common_swizzle;

switch (which_atu) {
case IOP13XX_INIT_ATU_ATUX:
@@ -547,6 +548,7 @@ int iop13xx_scan_bus(int nr, struct pci_host_bridge *bridge)
break;
case IOP13XX_INIT_ATU_ATUE:
bridge->ops = &iop13xx_atue_ops;
+ bridge->map_irq = iop13xx_pcie_map_irq;
ret = pci_scan_root_bus_bridge(bridge);
if (!ret)
pci_bus_atue = bridge->bus;
diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
index 76951bf..0f1c343 100644
--- a/arch/arm/mach-orion5x/pci.c
+++ b/arch/arm/mach-orion5x/pci.c
@@ -555,6 +555,19 @@ int __init orion5x_pci_sys_setup(int nr, struct pci_sys_data *sys)
return 0;
}

+int __init orion5x_pci_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+{
+ int bus = dev->bus->number;
+
+ /*
+ * PCIe endpoint?
+ */
+ if (orion5x_pci_disabled || bus < orion5x_pci_local_bus_nr())
+ return IRQ_ORION5X_PCIE0_INT;
+
+ return -1;
+}
+
int __init orion5x_pci_sys_scan_bus(int nr, struct pci_host_bridge *bridge)
{
struct pci_sys_data *sys = pci_host_bridge_priv(bridge);
@@ -563,6 +576,8 @@ int __init orion5x_pci_sys_scan_bus(int nr, struct pci_host_bridge *bridge)
bridge->dev.parent = NULL;
bridge->sysdata = sys;
bridge->busnr = sys->busnr;
+ bridge->map_irq = orion5x_pci_map_irq;
+ bridge->swizzle_irq = pci_common_swizzle;

if (nr == 0) {
bridge->ops = &pcie_ops;
@@ -577,16 +592,3 @@ int __init orion5x_pci_sys_scan_bus(int nr, struct pci_host_bridge *bridge)
BUG();
return -ENODEV;
}
-
-int __init orion5x_pci_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-{
- int bus = dev->bus->number;
-
- /*
- * PCIe endpoint?
- */
- if (orion5x_pci_disabled || bus < orion5x_pci_local_bus_nr())
- return IRQ_ORION5X_PCIE0_INT;
-
- return -1;
-}
--
2.10.0
Lorenzo Pieralisi
2017-07-01 14:06:30 UTC
Permalink
Hi Bjorn,
Post by Lorenzo Pieralisi
Legacy PCI host controllers (ie host controllers that set-up the PCI bus
through the ARM pci_common_init() API) are currently relying on
pci_fixup_irqs() to assign legacy PCI irqs to devices. This is not ideal
in that pci_fixup_irqs() assign IRQs for all PCI devices present in a
given system some of which may well be enabled by the time
pci_fixup_irqs() is called (ie a system with multiple host controllers).
With the introduction of struct pci_host_bridge.(*map_irq) pointer it is
possible to assign IRQs for all devices originating from a PCI host
bridge at probe time; this is implemented through pci_assign_irq() that
relies on the struct pci_host_bridge.map_irq pointer to map IRQ for a
given device.
- the IRQ for a device is assigned once at probe time
- the IRQ assignment works also for hotplugged devices
Remove pci_fixup_irqs() call from bios32 code and rely on
pci_assign_irq() to carry out the IRQ mapping at device probe
time.
The map_irq() and swizzle_irq() struct pci_host_bridge callbacks are
set-up in the struct pci_host_bridge created in the bios32
pcibios_init_hw() function and mach-* code paths (for PCI mach
implementations that require a specific struct hw_pci.(*scan) function
callback).
---
arch/arm/kernel/bios32.c | 4 ++--
arch/arm/mach-dove/pcie.c | 18 ++++++++++--------
arch/arm/mach-iop13xx/pci.c | 2 ++
arch/arm/mach-orion5x/pci.c | 28 +++++++++++++++-------------
4 files changed, 29 insertions(+), 23 deletions(-)
As I mentioned to you, this patch is incomplete and needs an upgrade
(ie I misread how Orion and the gazillions board file that go with
it to map irqs - apologies but it is very hard to untangle).

I am inlining below an incremental patch to be folded in commit:

feaec00e757a ("ARM/PCI: Remove pci_fixup_irqs() call for bios32 host
controllers")

in your pci/irq-fixups branch, or if you prefer I can send you the
resulting squashed-in patch to replace this one, as you prefer.

I understand timing is tight I am doing all I can to fix this series
up and make 4.13.

Please let me know how I can help, thank you !
Lorenzo

-- >8 --
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index da201a2..56dc1a3 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -484,6 +484,9 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
break;
}

+ bridge->map_irq = pcibios_map_irq;
+ bridge->swizzle_irq = pcibios_swizzle;
+
if (hw->scan)
ret = hw->scan(nr, bridge);
else {
@@ -494,8 +497,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
bridge->busnr = sys->busnr;
bridge->ops = hw->ops;
bridge->msi = hw->msi_ctrl;
- bridge->map_irq = pcibios_map_irq;
- bridge->swizzle_irq = pcibios_swizzle;
bridge->align_resource =
hw->align_resource;

diff --git a/arch/arm/mach-dove/pcie.c b/arch/arm/mach-dove/pcie.c
index 8d54a67..dfb62f3 100644
--- a/arch/arm/mach-dove/pcie.c
+++ b/arch/arm/mach-dove/pcie.c
@@ -152,14 +152,6 @@ static void rc_pci_fixup(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL, PCI_ANY_ID, rc_pci_fixup);

-static int __init dove_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-{
- struct pci_sys_data *sys = dev->sysdata;
- struct pcie_port *pp = sys->private_data;
-
- return pp->index ? IRQ_DOVE_PCIE1 : IRQ_DOVE_PCIE0;
-}
-
static int __init
dove_pcie_scan_bus(int nr, struct pci_host_bridge *bridge)
{
@@ -175,12 +167,18 @@ dove_pcie_scan_bus(int nr, struct pci_host_bridge *bridge)
bridge->sysdata = sys;
bridge->busnr = sys->busnr;
bridge->ops = &pcie_ops;
- bridge->map_irq = dove_pcie_map_irq;
- bridge->swizzle_irq = pci_common_swizzle;

return pci_scan_root_bus_bridge(bridge);
}

+static int __init dove_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+{
+ struct pci_sys_data *sys = dev->sysdata;
+ struct pcie_port *pp = sys->private_data;
+
+ return pp->index ? IRQ_DOVE_PCIE1 : IRQ_DOVE_PCIE0;
+}
+
static struct hw_pci dove_pci __initdata = {
.nr_controllers = 2,
.setup = dove_pcie_setup,
diff --git a/arch/arm/mach-iop13xx/pci.c b/arch/arm/mach-iop13xx/pci.c
index 3c51a9b..070d92a 100644
--- a/arch/arm/mach-iop13xx/pci.c
+++ b/arch/arm/mach-iop13xx/pci.c
@@ -532,7 +532,6 @@ int iop13xx_scan_bus(int nr, struct pci_host_bridge *bridge)
bridge->dev.parent = NULL;
bridge->sysdata = sys;
bridge->busnr = sys->busnr;
- bridge->swizzle_irq = pci_common_swizzle;

switch (which_atu) {
case IOP13XX_INIT_ATU_ATUX:
@@ -548,7 +547,6 @@ int iop13xx_scan_bus(int nr, struct pci_host_bridge *bridge)
break;
case IOP13XX_INIT_ATU_ATUE:
bridge->ops = &iop13xx_atue_ops;
- bridge->map_irq = iop13xx_pcie_map_irq;
ret = pci_scan_root_bus_bridge(bridge);
if (!ret)
pci_bus_atue = bridge->bus;
diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
index 0f1c343..76951bf 100644
--- a/arch/arm/mach-orion5x/pci.c
+++ b/arch/arm/mach-orion5x/pci.c
@@ -555,19 +555,6 @@ int __init orion5x_pci_sys_setup(int nr, struct pci_sys_data *sys)
return 0;
}

-int __init orion5x_pci_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-{
- int bus = dev->bus->number;
-
- /*
- * PCIe endpoint?
- */
- if (orion5x_pci_disabled || bus < orion5x_pci_local_bus_nr())
- return IRQ_ORION5X_PCIE0_INT;
-
- return -1;
-}
-
int __init orion5x_pci_sys_scan_bus(int nr, struct pci_host_bridge *bridge)
{
struct pci_sys_data *sys = pci_host_bridge_priv(bridge);
@@ -576,8 +563,6 @@ int __init orion5x_pci_sys_scan_bus(int nr, struct pci_host_bridge *bridge)
bridge->dev.parent = NULL;
bridge->sysdata = sys;
bridge->busnr = sys->busnr;
- bridge->map_irq = orion5x_pci_map_irq;
- bridge->swizzle_irq = pci_common_swizzle;

if (nr == 0) {
bridge->ops = &pcie_ops;
@@ -592,3 +577,16 @@ int __init orion5x_pci_sys_scan_bus(int nr, struct pci_host_bridge *bridge)
BUG();
return -ENODEV;
}
+
+int __init orion5x_pci_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
+{
+ int bus = dev->bus->number;
+
+ /*
+ * PCIe endpoint?
+ */
+ if (orion5x_pci_disabled || bus < orion5x_pci_local_bus_nr())
+ return IRQ_ORION5X_PCIE0_INT;
+
+ return -1;
+}
--
2.10.0
Bjorn Helgaas
2017-07-02 21:19:50 UTC
Permalink
Post by Oza Oza
Hi Bjorn,
Post by Lorenzo Pieralisi
Legacy PCI host controllers (ie host controllers that set-up the PCI bus
through the ARM pci_common_init() API) are currently relying on
pci_fixup_irqs() to assign legacy PCI irqs to devices. This is not ideal
in that pci_fixup_irqs() assign IRQs for all PCI devices present in a
given system some of which may well be enabled by the time
pci_fixup_irqs() is called (ie a system with multiple host controllers).
With the introduction of struct pci_host_bridge.(*map_irq) pointer it is
possible to assign IRQs for all devices originating from a PCI host
bridge at probe time; this is implemented through pci_assign_irq() that
relies on the struct pci_host_bridge.map_irq pointer to map IRQ for a
given device.
- the IRQ for a device is assigned once at probe time
- the IRQ assignment works also for hotplugged devices
Remove pci_fixup_irqs() call from bios32 code and rely on
pci_assign_irq() to carry out the IRQ mapping at device probe
time.
The map_irq() and swizzle_irq() struct pci_host_bridge callbacks are
set-up in the struct pci_host_bridge created in the bios32
pcibios_init_hw() function and mach-* code paths (for PCI mach
implementations that require a specific struct hw_pci.(*scan) function
callback).
---
arch/arm/kernel/bios32.c | 4 ++--
arch/arm/mach-dove/pcie.c | 18 ++++++++++--------
arch/arm/mach-iop13xx/pci.c | 2 ++
arch/arm/mach-orion5x/pci.c | 28 +++++++++++++++-------------
4 files changed, 29 insertions(+), 23 deletions(-)
As I mentioned to you, this patch is incomplete and needs an upgrade
(ie I misread how Orion and the gazillions board file that go with
it to map irqs - apologies but it is very hard to untangle).
feaec00e757a ("ARM/PCI: Remove pci_fixup_irqs() call for bios32 host
controllers")
in your pci/irq-fixups branch, or if you prefer I can send you the
resulting squashed-in patch to replace this one, as you prefer.
Thanks, I folded this into pci/irq-fixups. The resulting patch is
merely this:

commit 16508469c0f3198a63923aaec08efa1c63f5fbcc
Author: Lorenzo Pieralisi <***@arm.com>
Date: Wed Jun 28 15:14:04 2017 -0500

ARM/PCI: Remove pci_fixup_irqs() call for bios32 host controllers

Legacy PCI host controllers (ie host controllers that set-up the PCI bus
through the ARM pci_common_init() API) are currently relying on
pci_fixup_irqs() to assign legacy PCI irqs to devices. This is not ideal
in that pci_fixup_irqs() assigns IRQs for all PCI devices present in a given
system some of which may well be enabled by the time pci_fixup_irqs() is
called (ie a system with multiple host controllers). With the introduction
of struct pci_host_bridge.(*map_irq) pointer it is possible to assign IRQs
for all devices originating from a PCI host bridge at probe time; this is
implemented through pci_assign_irq() that relies on the struct
pci_host_bridge.map_irq pointer to map IRQ for a given device.

The benefits this brings are twofold:

- the IRQ for a device is assigned once at probe time
- the IRQ assignment works also for hotplugged devices

Remove pci_fixup_irqs() call from bios32 code and rely on pci_assign_irq()
to carry out the IRQ mapping at device probe time.

The map_irq() and swizzle_irq() struct pci_host_bridge callbacks are set-up
in the struct pci_host_bridge created in the bios32 pcibios_init_hw()
function and mach-* code paths (for PCI mach implementations that require a
specific struct hw_pci.(*scan) function callback).

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
[bhelgaas: folded in fixes from Lorenzo:
http://lkml.kernel.org/r/***@red-moon]
Signed-off-by: Bjorn Helgaas <***@google.com>
Cc: Jason Cooper <***@lakedaemon.net>
Cc: Russell King <***@armlinux.org.uk>
Cc: Andrew Lunn <***@lunn.ch>

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 4c7621ac389c..56dc1a3a33b4 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -484,6 +484,9 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
break;
}

+ bridge->map_irq = pcibios_map_irq;
+ bridge->swizzle_irq = pcibios_swizzle;
+
if (hw->scan)
ret = hw->scan(nr, bridge);
else {
@@ -530,8 +533,6 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw)
if (hw->postinit)
hw->postinit();

- pci_fixup_irqs(pcibios_swizzle, pcibios_map_irq);
-
list_for_each_entry(sys, &head, node) {
struct pci_bus *bus = sys->bus;
Lorenzo Pieralisi
2017-07-03 10:26:19 UTC
Permalink
Post by Bjorn Helgaas
Post by Oza Oza
Hi Bjorn,
Post by Lorenzo Pieralisi
Legacy PCI host controllers (ie host controllers that set-up the PCI bus
through the ARM pci_common_init() API) are currently relying on
pci_fixup_irqs() to assign legacy PCI irqs to devices. This is not ideal
in that pci_fixup_irqs() assign IRQs for all PCI devices present in a
given system some of which may well be enabled by the time
pci_fixup_irqs() is called (ie a system with multiple host controllers).
With the introduction of struct pci_host_bridge.(*map_irq) pointer it is
possible to assign IRQs for all devices originating from a PCI host
bridge at probe time; this is implemented through pci_assign_irq() that
relies on the struct pci_host_bridge.map_irq pointer to map IRQ for a
given device.
- the IRQ for a device is assigned once at probe time
- the IRQ assignment works also for hotplugged devices
Remove pci_fixup_irqs() call from bios32 code and rely on
pci_assign_irq() to carry out the IRQ mapping at device probe
time.
The map_irq() and swizzle_irq() struct pci_host_bridge callbacks are
set-up in the struct pci_host_bridge created in the bios32
pcibios_init_hw() function and mach-* code paths (for PCI mach
implementations that require a specific struct hw_pci.(*scan) function
callback).
---
arch/arm/kernel/bios32.c | 4 ++--
arch/arm/mach-dove/pcie.c | 18 ++++++++++--------
arch/arm/mach-iop13xx/pci.c | 2 ++
arch/arm/mach-orion5x/pci.c | 28 +++++++++++++++-------------
4 files changed, 29 insertions(+), 23 deletions(-)
As I mentioned to you, this patch is incomplete and needs an upgrade
(ie I misread how Orion and the gazillions board file that go with
it to map irqs - apologies but it is very hard to untangle).
feaec00e757a ("ARM/PCI: Remove pci_fixup_irqs() call for bios32 host
controllers")
in your pci/irq-fixups branch, or if you prefer I can send you the
resulting squashed-in patch to replace this one, as you prefer.
Thanks, I folded this into pci/irq-fixups. The resulting patch is
Yes that's what I expected, we remove pci_fixup_irqs() and add the
hooks into the host bridge structure that should cover all host
bridges initialized via pci_common_init_dev().

Thanks !
Lorenzo
Post by Bjorn Helgaas
commit 16508469c0f3198a63923aaec08efa1c63f5fbcc
Date: Wed Jun 28 15:14:04 2017 -0500
ARM/PCI: Remove pci_fixup_irqs() call for bios32 host controllers
Legacy PCI host controllers (ie host controllers that set-up the PCI bus
through the ARM pci_common_init() API) are currently relying on
pci_fixup_irqs() to assign legacy PCI irqs to devices. This is not ideal
in that pci_fixup_irqs() assigns IRQs for all PCI devices present in a given
system some of which may well be enabled by the time pci_fixup_irqs() is
called (ie a system with multiple host controllers). With the introduction
of struct pci_host_bridge.(*map_irq) pointer it is possible to assign IRQs
for all devices originating from a PCI host bridge at probe time; this is
implemented through pci_assign_irq() that relies on the struct
pci_host_bridge.map_irq pointer to map IRQ for a given device.
- the IRQ for a device is assigned once at probe time
- the IRQ assignment works also for hotplugged devices
Remove pci_fixup_irqs() call from bios32 code and rely on pci_assign_irq()
to carry out the IRQ mapping at device probe time.
The map_irq() and swizzle_irq() struct pci_host_bridge callbacks are set-up
in the struct pci_host_bridge created in the bios32 pcibios_init_hw()
function and mach-* code paths (for PCI mach implementations that require a
specific struct hw_pci.(*scan) function callback).
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 4c7621ac389c..56dc1a3a33b4 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -484,6 +484,9 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
break;
}
+ bridge->map_irq = pcibios_map_irq;
+ bridge->swizzle_irq = pcibios_swizzle;
+
if (hw->scan)
ret = hw->scan(nr, bridge);
else {
@@ -530,8 +533,6 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw)
if (hw->postinit)
hw->postinit();
- pci_fixup_irqs(pcibios_swizzle, pcibios_map_irq);
-
list_for_each_entry(sys, &head, node) {
struct pci_bus *bus = sys->bus;
Lorenzo Pieralisi
2017-06-08 14:13:24 UTC
Permalink
From: Matthew Minter <***@masarand.com>

The functions included in setup-irq.o currently apply only to a selection
of architectures which share common IRQ assignment code. However this
code needs to be generalised for all arches to allow deferred IRQ
assignment. So the first step is to build it on all architectures.

Signed-off-by: Matthew Minter <***@masarand.com>
---
drivers/pci/Makefile | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index 462c1f5..66a21ac 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -4,7 +4,8 @@

obj-y += access.o bus.o probe.o host-bridge.o remove.o pci.o \
pci-driver.o search.o pci-sysfs.o rom.o setup-res.o \
- irq.o vpd.o setup-bus.o vc.o mmap.o
+ irq.o vpd.o setup-bus.o vc.o mmap.o setup-irq.o
+
obj-$(CONFIG_PROC_FS) += proc.o
obj-$(CONFIG_SYSFS) += slot.o

@@ -29,20 +30,6 @@ obj-$(CONFIG_PCI_ATS) += ats.o
obj-$(CONFIG_PCI_IOV) += iov.o

#
-# Some architectures use the generic PCI setup functions
-#
-obj-$(CONFIG_ALPHA) += setup-irq.o
-obj-$(CONFIG_ARC) += setup-irq.o
-obj-$(CONFIG_ARM) += setup-irq.o
-obj-$(CONFIG_ARM64) += setup-irq.o
-obj-$(CONFIG_UNICORE32) += setup-irq.o
-obj-$(CONFIG_SUPERH) += setup-irq.o
-obj-$(CONFIG_MIPS) += setup-irq.o
-obj-$(CONFIG_TILE) += setup-irq.o
-obj-$(CONFIG_SPARC_LEON) += setup-irq.o
-obj-$(CONFIG_M68K) += setup-irq.o
-
-#
# ACPI Related PCI FW Functions
# ACPI _DSM provided firmware instance and string name
#
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:20 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI host-common code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Will Deacon <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
---
drivers/pci/host/pci-host-common.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
index e9a53ba..8e2614ba 100644
--- a/drivers/pci/host/pci-host-common.c
+++ b/drivers/pci/host/pci-host-common.c
@@ -117,8 +117,14 @@ int pci_host_common_probe(struct platform_device *pdev,
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
struct pci_bus *bus, *child;
+ struct pci_host_bridge *bridge;
struct pci_config_window *cfg;
struct list_head resources;
+ int ret;
+
+ bridge = devm_pci_alloc_host_bridge(dev, 0);
+ if (!bridge)
+ return -ENOMEM;

type = of_get_property(np, "device_type", NULL);
if (!type || strcmp(type, "pci")) {
@@ -138,13 +144,20 @@ int pci_host_common_probe(struct platform_device *pdev,
if (!pci_has_flag(PCI_PROBE_ONLY))
pci_add_flags(PCI_REASSIGN_ALL_RSRC | PCI_REASSIGN_ALL_BUS);

- bus = pci_scan_root_bus(dev, cfg->busr.start, &ops->pci_ops, cfg,
- &resources);
- if (!bus) {
- dev_err(dev, "Scanning rootbus failed");
- return -ENODEV;
+ list_splice_init(&resources, &bridge->windows);
+ bridge->dev.parent = dev;
+ bridge->sysdata = cfg;
+ bridge->busnr = cfg->busr.start;
+ bridge->ops = &ops->pci_ops;
+
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (ret < 0) {
+ dev_err(dev, "Scanning root bridge failed");
+ return ret;
}

+ bus = bridge->bus;
+
#ifdef CONFIG_ARM
pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
#endif
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:18 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI xilinx host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
CC: Michal Simek <***@xilinx.com>
---
drivers/pci/host/pcie-xilinx.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/host/pcie-xilinx.c b/drivers/pci/host/pcie-xilinx.c
index 2fe2df5..761f048 100644
--- a/drivers/pci/host/pcie-xilinx.c
+++ b/drivers/pci/host/pcie-xilinx.c
@@ -633,6 +633,7 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct xilinx_pcie_port *port;
struct pci_bus *bus, *child;
+ struct pci_host_bridge *bridge;
int err;
resource_size_t iobase = 0;
LIST_HEAD(res);
@@ -640,9 +641,11 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
if (!dev->of_node)
return -ENODEV;

- port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
- if (!port)
- return -ENOMEM;
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
+ if (!bridge)
+ return -ENODEV;
+
+ port = pci_host_bridge_priv(bridge);

port->dev = dev;

@@ -671,17 +674,23 @@ static int xilinx_pcie_probe(struct platform_device *pdev)
if (err)
goto error;

- bus = pci_create_root_bus(dev, 0, &xilinx_pcie_ops, port, &res);
- if (!bus) {
- err = -ENOMEM;
- goto error;
- }
+
+ list_splice_init(&res, &bridge->windows);
+ bridge->dev.parent = dev;
+ bridge->sysdata = port;
+ bridge->busnr = 0;
+ bridge->ops = &xilinx_pcie_ops;

#ifdef CONFIG_PCI_MSI
xilinx_pcie_msi_chip.dev = dev;
- bus->msi = &xilinx_pcie_msi_chip;
+ bridge->msi = &xilinx_pcie_msi_chip;
#endif
- pci_scan_child_bus(bus);
+ err = pci_scan_root_bus_bridge(bridge);
+ if (err < 0)
+ goto error;
+
+ bus = bridge->bus;
+
pci_assign_unassigned_bus_resources(bus);
#ifndef CONFIG_MICROBLAZE
pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:19 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI xgene host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Tanmay Inamdar <***@apm.com>
---
drivers/pci/host/pci-xgene.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 8cae013..262bedf 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -636,13 +636,16 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
struct xgene_pcie_port *port;
resource_size_t iobase = 0;
struct pci_bus *bus, *child;
+ struct pci_host_bridge *bridge;
int ret;
LIST_HEAD(res);

- port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
- if (!port)
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*port));
+ if (!bridge)
return -ENOMEM;

+ port = pci_host_bridge_priv(bridge);
+
port->node = of_node_get(dn);
port->dev = dev;

@@ -670,11 +673,17 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
if (ret)
goto error;

- bus = pci_create_root_bus(dev, 0, &xgene_pcie_ops, port, &res);
- if (!bus) {
- ret = -ENOMEM;
+ list_splice_init(&res, &bridge->windows);
+ bridge->dev.parent = dev;
+ bridge->sysdata = port;
+ bridge->busnr = 0;
+ bridge->ops = &xgene_pcie_ops;
+
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (ret < 0)
goto error;
- }
+
+ bus = bridge->bus;

pci_scan_child_bus(bus);
pci_assign_unassigned_bus_resources(bus);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:17 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI altera host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Ley Foon Tan <***@altera.com>
---
drivers/pci/host/pcie-altera.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pcie-altera.c b/drivers/pci/host/pcie-altera.c
index 75ec5ce..7f023b2 100644
--- a/drivers/pci/host/pcie-altera.c
+++ b/drivers/pci/host/pcie-altera.c
@@ -579,12 +579,14 @@ static int altera_pcie_probe(struct platform_device *pdev)
struct altera_pcie *pcie;
struct pci_bus *bus;
struct pci_bus *child;
+ struct pci_host_bridge *bridge;
int ret;

- pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
- if (!pcie)
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
+ if (!bridge)
return -ENOMEM;

+ pcie = pci_host_bridge_priv(bridge);
pcie->pdev = pdev;

ret = altera_pcie_parse_dt(pcie);
@@ -613,12 +615,20 @@ static int altera_pcie_probe(struct platform_device *pdev)
cra_writel(pcie, P2A_INT_ENA_ALL, P2A_INT_ENABLE);
altera_pcie_host_init(pcie);

- bus = pci_scan_root_bus(dev, pcie->root_bus_nr, &altera_pcie_ops,
- pcie, &pcie->resources);
- if (!bus)
- return -ENOMEM;
+ list_splice_init(&pcie->resources, &bridge->windows);
+ bridge->dev.parent = dev;
+ bridge->sysdata = pcie;
+ bridge->busnr = pcie->root_bus_nr;
+ bridge->ops = &altera_pcie_ops;
+
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (ret < 0)
+ return ret;
+
+ bus = bridge->bus;

pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+
pci_assign_unassigned_bus_resources(bus);

/* Configure PCI Express setting. */
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:12 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI designware host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Jingoo Han <***@gmail.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Joao Pinto <***@synopsys.com>
---
drivers/pci/dwc/pcie-designware-host.c | 36 +++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c
index 28ed32b..351a277 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -280,9 +280,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
struct device_node *np = dev->of_node;
struct platform_device *pdev = to_platform_device(dev);
struct pci_bus *bus, *child;
+ struct pci_host_bridge *bridge;
struct resource *cfg_res;
int i, ret;
- LIST_HEAD(res);
struct resource_entry *win, *tmp;

cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
@@ -295,16 +295,21 @@ int dw_pcie_host_init(struct pcie_port *pp)
dev_err(dev, "missing *config* reg space\n");
}

- ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base);
+ bridge = pci_alloc_host_bridge(0);
+ if (!bridge)
+ return -ENOMEM;
+
+ ret = of_pci_get_host_bridge_resources(np, 0, 0xff,
+ &bridge->windows, &pp->io_base);
if (ret)
return ret;

- ret = devm_request_pci_bus_resources(dev, &res);
+ ret = devm_request_pci_bus_resources(dev, &bridge->windows);
if (ret)
goto error;

/* Get the I/O and memory ranges from DT */
- resource_list_for_each_entry_safe(win, tmp, &res) {
+ resource_list_for_each_entry_safe(win, tmp, &bridge->windows) {
switch (resource_type(win->res)) {
case IORESOURCE_IO:
ret = pci_remap_iospace(win->res, pp->io_base);
@@ -400,19 +405,22 @@ int dw_pcie_host_init(struct pcie_port *pp)
pp->ops->host_init(pp);

pp->root_bus_nr = pp->busn->start;
+
+ bridge->dev.parent = dev;
+ bridge->sysdata = pp;
+ bridge->busnr = pp->root_bus_nr;
+ bridge->ops = &dw_pcie_ops;
if (IS_ENABLED(CONFIG_PCI_MSI)) {
- bus = pci_scan_root_bus_msi(dev, pp->root_bus_nr,
- &dw_pcie_ops, pp, &res,
- &dw_pcie_msi_chip);
+ bridge->msi = &dw_pcie_msi_chip;
dw_pcie_msi_chip.dev = dev;
- } else
- bus = pci_scan_root_bus(dev, pp->root_bus_nr, &dw_pcie_ops,
- pp, &res);
- if (!bus) {
- ret = -ENOMEM;
- goto error;
}

+ ret = pci_scan_root_bus_bridge(bridge);
+ if (ret)
+ goto error;
+
+ bus = bridge->bus;
+
if (pp->ops->scan_bus)
pp->ops->scan_bus(pp);

@@ -431,7 +439,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
return 0;

error:
- pci_free_resource_list(&res);
+ pci_free_host_bridge(bridge);
return ret;
}
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:06 UTC
Permalink
struct pci_host_bridge can be allocated by PCI host bridge drivers
which usually allocate and map memory through devm managed interfaces.

Add a devm version for the pci_alloc_host_bridge() interface to
simplify PCI host controller drivers porting and simplify the
drivers failures paths.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Arnd Bergmann <***@arndb.de>
Cc: Bjorn Helgaas <***@google.com>
---
Documentation/driver-model/devres.txt | 1 +
drivers/pci/probe.c | 24 ++++++++++++++++++++++--
include/linux/pci.h | 2 ++
3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index e72587f..ffbc891 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -342,6 +342,7 @@ PER-CPU MEM
devm_free_percpu()

PCI
+ devm_pci_alloc_host_bridge() : managed PCI host bridge allocation
devm_pci_remap_cfgspace() : ioremap PCI configuration space
devm_pci_remap_cfg_resource() : ioremap PCI configuration space resource
pcim_enable_device() : after success, all PCI ops become managed
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index cbf0d0c..e3f6965 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -510,14 +510,18 @@ static struct pci_bus *pci_alloc_bus(struct pci_bus *parent)
return b;
}

-static void pci_release_host_bridge_dev(struct device *dev)
+static void devm_pci_release_host_bridge_dev(struct device *dev)
{
struct pci_host_bridge *bridge = to_pci_host_bridge(dev);

if (bridge->release_fn)
bridge->release_fn(bridge);
+}

- pci_free_host_bridge(bridge);
+static void pci_release_host_bridge_dev(struct device *dev)
+{
+ devm_pci_release_host_bridge_dev(dev);
+ pci_free_host_bridge(to_pci_host_bridge(dev));
}

struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
@@ -535,6 +539,22 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
}
EXPORT_SYMBOL(pci_alloc_host_bridge);

+struct pci_host_bridge *devm_pci_alloc_host_bridge(struct device *dev,
+ size_t priv)
+{
+ struct pci_host_bridge *bridge;
+
+ bridge = devm_kzalloc(dev, sizeof(*bridge) + priv, GFP_KERNEL);
+ if (!bridge)
+ return NULL;
+
+ INIT_LIST_HEAD(&bridge->windows);
+ bridge->dev.release = devm_pci_release_host_bridge_dev;
+
+ return bridge;
+}
+EXPORT_SYMBOL(devm_pci_alloc_host_bridge);
+
void pci_free_host_bridge(struct pci_host_bridge *bridge)
{
pci_free_resource_list(&bridge->windows);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5720635..b21308e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -463,6 +463,8 @@ static inline struct pci_host_bridge *pci_host_bridge_from_priv(void *priv)
}

struct pci_host_bridge *pci_alloc_host_bridge(size_t priv);
+struct pci_host_bridge *devm_pci_alloc_host_bridge(struct device *dev,
+ size_t priv);
void pci_free_host_bridge(struct pci_host_bridge *bridge);
int pci_register_host_bridge(struct pci_host_bridge *bridge);
struct pci_host_bridge *pci_find_host_bridge(struct pci_bus *bus);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:23 UTC
Permalink
The pci_scan_root_bus_bridge() function allows passing a parameterized
struct pci_host_bridge and scanning the resulting PCI bus; since the
struct msi_controller is part of the struct pci_host_bridge and the
struct pci_host_bridge can now be passed to pci_scan_root_bus_bridge()
explicitly, there is no need for a scan interface with a MSI controller
parameter.

With all PCI host controller drivers and platform code relying
on pci_scan_root_bus_msi() converted over to pci_scan_root_bus_bridge()
the pci_scan_root_bus_msi() becomes obsolete and can be removed.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
---
drivers/pci/probe.c | 27 +++++----------------------
include/linux/pci.h | 4 ----
2 files changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 5c457c1..bd42ed4 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2324,9 +2324,8 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
{
}

-static struct pci_bus *pci_create_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_create_root_bus(struct device *parent, int bus,
+ struct pci_ops *ops, void *sysdata, struct list_head *resources)
{
int error;
struct pci_host_bridge *bridge;
@@ -2341,7 +2340,6 @@ static struct pci_bus *pci_create_root_bus_msi(struct device *parent,
bridge->sysdata = sysdata;
bridge->busnr = bus;
bridge->ops = ops;
- bridge->msi = msi;

error = pci_register_host_bridge(bridge);
if (error < 0)
@@ -2353,13 +2351,6 @@ static struct pci_bus *pci_create_root_bus_msi(struct device *parent,
kfree(bridge);
return NULL;
}
-
-struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
- struct pci_ops *ops, void *sysdata, struct list_head *resources)
-{
- return pci_create_root_bus_msi(parent, bus, ops, sysdata, resources,
- NULL);
-}
EXPORT_SYMBOL_GPL(pci_create_root_bus);

int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
@@ -2464,9 +2455,8 @@ int pci_scan_root_bus_bridge(struct pci_host_bridge *bridge)
}
EXPORT_SYMBOL(pci_scan_root_bus_bridge);

-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)
{
struct resource_entry *window;
bool found = false;
@@ -2479,7 +2469,7 @@ struct pci_bus *pci_scan_root_bus_msi(struct device *parent, int bus,
break;
}

- b = pci_create_root_bus_msi(parent, bus, ops, sysdata, resources, msi);
+ b = pci_create_root_bus(parent, bus, ops, sysdata, resources);
if (!b)
return NULL;

@@ -2497,13 +2487,6 @@ struct pci_bus *pci_scan_root_bus_msi(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,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 22f549e..31ee3b3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -854,10 +854,6 @@ 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);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:15 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI iproc host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Scott Branden <***@broadcom.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Ray Jui <***@broadcom.com>
Cc: Jon Mason <***@broadcom.com>
---
drivers/pci/host/pcie-iproc-bcma.c | 7 +++++--
drivers/pci/host/pcie-iproc-platform.c | 7 +++++--
drivers/pci/host/pcie-iproc.c | 38 ++++++++++++++++++----------------
3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/host/pcie-iproc-bcma.c b/drivers/pci/host/pcie-iproc-bcma.c
index 384c27e..f03d5e3 100644
--- a/drivers/pci/host/pcie-iproc-bcma.c
+++ b/drivers/pci/host/pcie-iproc-bcma.c
@@ -45,12 +45,15 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev)
struct device *dev = &bdev->dev;
struct iproc_pcie *pcie;
LIST_HEAD(resources);
+ struct pci_host_bridge *bridge;
int ret;

- pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
- if (!pcie)
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
+ if (!bridge)
return -ENOMEM;

+ pcie = pci_host_bridge_priv(bridge);
+
pcie->dev = dev;

pcie->type = IPROC_PCIE_PAXB_BCMA;
diff --git a/drivers/pci/host/pcie-iproc-platform.c b/drivers/pci/host/pcie-iproc-platform.c
index 90d2bdd..2253119 100644
--- a/drivers/pci/host/pcie-iproc-platform.c
+++ b/drivers/pci/host/pcie-iproc-platform.c
@@ -52,12 +52,15 @@ static int iproc_pcie_pltfm_probe(struct platform_device *pdev)
struct resource reg;
resource_size_t iobase = 0;
LIST_HEAD(resources);
+ struct pci_host_bridge *bridge;
int ret;

- pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
- if (!pcie)
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
+ if (!bridge)
return -ENOMEM;

+ pcie = pci_host_bridge_priv(bridge);
+
pcie->dev = dev;
pcie->type = (enum iproc_pcie_type) of_device_get_match_data(dev);

diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index e48b8e2..55a3665 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -1259,7 +1259,8 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
struct device *dev;
int ret;
void *sysdata;
- struct pci_bus *bus, *child;
+ struct pci_bus *child;
+ struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);

dev = pcie->dev;

@@ -1306,18 +1307,10 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
sysdata = pcie;
#endif

- bus = pci_create_root_bus(dev, 0, &iproc_pcie_ops, sysdata, res);
- if (!bus) {
- dev_err(dev, "unable to create PCI root bus\n");
- ret = -ENOMEM;
- goto err_power_off_phy;
- }
- pcie->root_bus = bus;
-
ret = iproc_pcie_check_link(pcie);
if (ret) {
dev_err(dev, "no PCIe EP device detected\n");
- goto err_rm_root_bus;
+ goto err_power_off_phy;
}

iproc_pcie_enable(pcie);
@@ -1326,23 +1319,32 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
if (iproc_pcie_msi_enable(pcie))
dev_info(dev, "not using iProc MSI\n");

- pci_scan_child_bus(bus);
- pci_assign_unassigned_bus_resources(bus);
+ list_splice_init(res, &host->windows);
+ host->busnr = 0;
+ host->dev.parent = dev;
+ host->ops = &iproc_pcie_ops;
+ host->sysdata = sysdata;
+
+ ret = pci_scan_root_bus_bridge(host);
+ if (ret < 0) {
+ dev_err(dev, "failed to scan host: %d\n", ret);
+ goto err_power_off_phy;
+ }

if (pcie->map_irq)
pci_fixup_irqs(pci_common_swizzle, pcie->map_irq);

- list_for_each_entry(child, &bus->children, node)
+ pci_assign_unassigned_bus_resources(host->bus);
+
+ pcie->root_bus = host->bus;
+
+ list_for_each_entry(child, &host->bus->children, node)
pcie_bus_configure_settings(child);

- pci_bus_add_devices(bus);
+ pci_bus_add_devices(host->bus);

return 0;

-err_rm_root_bus:
- pci_stop_root_bus(bus);
- pci_remove_root_bus(bus);
-
err_power_off_phy:
phy_power_off(pcie->phy);
err_exit_phy:
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:27 UTC
Permalink
From: Matthew Minter <***@masarand.com>

With the introduction of struct pci_host_bridge.(*map_irq) function
pointer add another direct usage of of_irq_parse_and_map_pci().

Update the function comment to reflect this behaviour.

Signed-off-by: Matthew Minter <***@masarand.com>
Signed-off-by: Lorenzo Pieralisi <***@arm.com>
---
drivers/of/of_pci_irq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
index c175d9c..3a05568 100644
--- a/drivers/of/of_pci_irq.c
+++ b/drivers/of/of_pci_irq.c
@@ -113,7 +113,8 @@ EXPORT_SYMBOL_GPL(of_irq_parse_pci);
* @pin: PCI irq pin number; passed when used as map_irq callback. Unused
*
* @slot and @pin are unused, but included in the function so that this
- * function can be used directly as the map_irq callback to pci_fixup_irqs().
+ * function can be used directly as the map_irq callback to
+ * pci_assign_irq() and struct pci_host_bridge.map_irq pointer
*/
int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8 slot, u8 pin)
{
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:22 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI xilinx-nwl host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Bharat Kumar Gogada <***@xilinx.com>
---
drivers/pci/host/pcie-xilinx-nwl.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-nwl.c
index 26e66e8..e8a5894 100644
--- a/drivers/pci/host/pcie-xilinx-nwl.c
+++ b/drivers/pci/host/pcie-xilinx-nwl.c
@@ -791,13 +791,16 @@ static int nwl_pcie_probe(struct platform_device *pdev)
struct nwl_pcie *pcie;
struct pci_bus *bus;
struct pci_bus *child;
+ struct pci_host_bridge *bridge;
int err;
resource_size_t iobase = 0;
LIST_HEAD(res);

- pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
- if (!pcie)
- return -ENOMEM;
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
+ if (!bridge)
+ return -ENODEV;
+
+ pcie = pci_host_bridge_priv(bridge);

pcie->dev = dev;
pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
@@ -830,12 +833,11 @@ static int nwl_pcie_probe(struct platform_device *pdev)
goto error;
}

- bus = pci_create_root_bus(dev, pcie->root_busno,
- &nwl_pcie_ops, pcie, &res);
- if (!bus) {
- err = -ENOMEM;
- goto error;
- }
+ list_splice_init(&res, &bridge->windows);
+ bridge->dev.parent = dev;
+ bridge->sysdata = pcie;
+ bridge->busnr = pcie->root_busno;
+ bridge->ops = &nwl_pcie_ops;

if (IS_ENABLED(CONFIG_PCI_MSI)) {
err = nwl_pcie_enable_msi(pcie);
@@ -844,7 +846,13 @@ static int nwl_pcie_probe(struct platform_device *pdev)
goto error;
}
}
- pci_scan_child_bus(bus);
+
+ err = pci_scan_root_bus_bridge(bridge);
+ if (err)
+ goto error;
+
+ bus = bridge->bus;
+
pci_assign_unassigned_bus_resources(bus);
list_for_each_entry(child, &bus->children, node)
pcie_bus_configure_settings(child);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:08 UTC
Permalink
When probing the PCI host controller driver, if an error condition
is triggered the probe function code does not free memory allocated
for the struct pci_host_bridge resulting in memory leakage.

Move the struct pci_host_bridge allocation over to the respective
devm interface to fix the issue.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Arnd Bergmann <***@arndb.de>
Cc: Bjorn Helgaas <***@google.com>
---
drivers/pci/host/pci-tegra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 2618f87..84c98a2 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -2238,7 +2238,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
struct pci_bus *child;
int err;

- host = pci_alloc_host_bridge(sizeof(*pcie));
+ host = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
if (!host)
return -ENOMEM;
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:13 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI aardvark host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Thomas Petazzoni <***@free-electrons.com>
---
drivers/pci/host/pci-aardvark.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
index 37d0bcd..5fb9b62 100644
--- a/drivers/pci/host/pci-aardvark.c
+++ b/drivers/pci/host/pci-aardvark.c
@@ -886,12 +886,14 @@ static int advk_pcie_probe(struct platform_device *pdev)
struct advk_pcie *pcie;
struct resource *res;
struct pci_bus *bus, *child;
+ struct pci_host_bridge *bridge;
int ret, irq;

- pcie = devm_kzalloc(dev, sizeof(struct advk_pcie), GFP_KERNEL);
- if (!pcie)
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct advk_pcie));
+ if (!bridge)
return -ENOMEM;

+ pcie = pci_host_bridge_priv(bridge);
pcie->pdev = pdev;

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -929,14 +931,21 @@ static int advk_pcie_probe(struct platform_device *pdev)
return ret;
}

- bus = pci_scan_root_bus(dev, 0, &advk_pcie_ops,
- pcie, &pcie->resources);
- if (!bus) {
+ list_splice_init(&pcie->resources, &bridge->windows);
+ bridge->dev.parent = dev;
+ bridge->sysdata = pcie;
+ bridge->busnr = 0;
+ bridge->ops = &advk_pcie_ops;
+
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (ret < 0) {
advk_pcie_remove_msi_irq_domain(pcie);
advk_pcie_remove_irq_domain(pcie);
- return -ENOMEM;
+ return ret;
}

+ bus = bridge->bus;
+
pci_bus_assign_resources(bus);

list_for_each_entry(child, &bus->children, node)
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:14 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI rcar host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Simon Horman <***@verge.net.au>
---
drivers/pci/host/pcie-rcar.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index cb07c45..8380274 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -450,28 +450,32 @@ static void rcar_pcie_force_speedup(struct rcar_pcie *pcie)
static int rcar_pcie_enable(struct rcar_pcie *pcie)
{
struct device *dev = pcie->dev;
+ struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
struct pci_bus *bus, *child;
- LIST_HEAD(res);
+ int ret;

/* Try setting 5 GT/s link speed */
rcar_pcie_force_speedup(pcie);

- rcar_pcie_setup(&res, pcie);
+ rcar_pcie_setup(&bridge->windows, pcie);

pci_add_flags(PCI_REASSIGN_ALL_RSRC | PCI_REASSIGN_ALL_BUS);

+ bridge->dev.parent = dev;
+ bridge->sysdata = pcie;
+ bridge->busnr = pcie->root_bus_nr;
+ bridge->ops = &rcar_pcie_ops;
if (IS_ENABLED(CONFIG_PCI_MSI))
- bus = pci_scan_root_bus_msi(dev, pcie->root_bus_nr,
- &rcar_pcie_ops, pcie, &res, &pcie->msi.chip);
- else
- bus = pci_scan_root_bus(dev, pcie->root_bus_nr,
- &rcar_pcie_ops, pcie, &res);
+ bridge->msi = &pcie->msi.chip;

- if (!bus) {
- dev_err(dev, "Scanning rootbus failed");
- return -ENODEV;
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (ret < 0) {
+ kfree(bridge);
+ return ret;
}

+ bus = bridge->bus;
+
pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);

pci_bus_size_bridges(bus);
@@ -1127,11 +1131,14 @@ static int rcar_pcie_probe(struct platform_device *pdev)
unsigned int data;
int err;
int (*hw_init_fn)(struct rcar_pcie *);
+ struct pci_host_bridge *bridge;

- pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
- if (!pcie)
+ bridge = pci_alloc_host_bridge(sizeof(*pcie));
+ if (!bridge)
return -ENOMEM;

+ pcie = pci_host_bridge_priv(bridge);
+
pcie->dev = dev;

INIT_LIST_HEAD(&pcie->resources);
@@ -1141,12 +1148,12 @@ static int rcar_pcie_probe(struct platform_device *pdev)
err = rcar_pcie_get_resources(pcie);
if (err < 0) {
dev_err(dev, "failed to request resources: %d\n", err);
- return err;
+ goto err_free_bridge;
}

err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
if (err)
- return err;
+ goto err_free_bridge;

pm_runtime_enable(dev);
err = pm_runtime_get_sync(dev);
@@ -1183,6 +1190,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)

return 0;

+err_free_bridge:
+ pci_free_host_bridge(bridge);
+
err_pm_put:
pm_runtime_put(dev);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:33 UTC
Permalink
Since, through struct pci_host_bridge.map/swizzle_irq hooks, IRQs
are now allocated in the pci_assign_irq() callback automatically,
PCI host bridge drivers can stop relying on pci_fixup_irqs()
for IRQ allocation.

Drop pci_fixup_irqs() usage from PCI iproc host bridge driver.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Scott Branden <***@broadcom.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Ray Jui <***@broadcom.com>
Cc: Jon Mason <***@broadcom.com>
---
drivers/pci/host/pcie-iproc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c
index 55a3665..c574863 100644
--- a/drivers/pci/host/pcie-iproc.c
+++ b/drivers/pci/host/pcie-iproc.c
@@ -1324,6 +1324,8 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
host->dev.parent = dev;
host->ops = &iproc_pcie_ops;
host->sysdata = sysdata;
+ host->map_irq = pcie->map_irq;
+ host->swizzle_irq = pci_common_swizzle;

ret = pci_scan_root_bus_bridge(host);
if (ret < 0) {
@@ -1331,9 +1333,6 @@ int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res)
goto err_power_off_phy;
}

- if (pcie->map_irq)
- pci_fixup_irqs(pci_common_swizzle, pcie->map_irq);
-
pci_assign_unassigned_bus_resources(host->bus);

pcie->root_bus = host->bus;
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:10 UTC
Permalink
With the introduction of pci_scan_root_bus_bridge() there is no
need to export pci_register_host_bridge() to other kernel subsystems
other than the PCI compilation unit that needs it.

Make pci_register_host_bridge() static to its compilation unit and
convert the existing drivers usage over to pci_scan_root_bus_bridge().

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Arnd Bergmann <***@arndb.de>
Cc: Bjorn Helgaas <***@google.com>
---
drivers/pci/host/pci-ftpci100.c | 5 ++---
drivers/pci/host/pci-tegra.c | 4 +---
drivers/pci/probe.c | 3 +--
include/linux/pci.h | 1 -
4 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index 89cbb1f..435a646 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -520,14 +520,13 @@ static int faraday_pci_probe(struct platform_device *pdev)
return ret;

list_splice_init(&res, &host->windows);
- ret = pci_register_host_bridge(host);
+ ret = pci_scan_root_bus_bridge(host);
if (ret) {
- dev_err(dev, "failed to register host: %d\n", ret);
+ dev_err(dev, "failed to scan host: %d\n", ret);
return ret;
}
p->bus = host->bus;

- pci_scan_child_bus(p->bus);
pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
pci_bus_assign_resources(p->bus);
pci_bus_add_devices(p->bus);
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 84c98a2..0383418 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -2285,14 +2285,12 @@ static int tegra_pcie_probe(struct platform_device *pdev)
host->dev.parent = &pdev->dev;
host->ops = &tegra_pcie_ops;

- err = pci_register_host_bridge(host);
+ err = pci_scan_root_bus_bridge(host);
if (err < 0) {
dev_err(dev, "failed to register host: %d\n", err);
goto disable_msi;
}

- pci_scan_child_bus(host->bus);
-
pci_fixup_irqs(pci_common_swizzle, tegra_pcie_map_irq);
pci_bus_size_bridges(host->bus);
pci_bus_assign_resources(host->bus);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 690f0b3..5c457c1 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -746,7 +746,7 @@ static void pci_set_bus_msi_domain(struct pci_bus *bus)
dev_set_msi_domain(&bus->dev, d);
}

-int pci_register_host_bridge(struct pci_host_bridge *bridge)
+static int pci_register_host_bridge(struct pci_host_bridge *bridge)
{
struct device *parent = bridge->dev.parent;
struct resource_entry *window, *n;
@@ -861,7 +861,6 @@ int pci_register_host_bridge(struct pci_host_bridge *bridge)
kfree(bus);
return err;
}
-EXPORT_SYMBOL(pci_register_host_bridge);

static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
struct pci_dev *bridge, int busnr)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 72940cc..22f549e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -466,7 +466,6 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv);
struct pci_host_bridge *devm_pci_alloc_host_bridge(struct device *dev,
size_t priv);
void pci_free_host_bridge(struct pci_host_bridge *bridge);
-int pci_register_host_bridge(struct pci_host_bridge *bridge);
struct pci_host_bridge *pci_find_host_bridge(struct pci_bus *bus);

void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:05 UTC
Permalink
Commit a52d1443bba1 ("PCI: Export host bridge registration interface")
exported the pci_alloc_host_bridge() interface so that PCI host
controllers drivers can make use of it.

Introduce pci_alloc_host_bridge() kernel counterpart to free the
host bridge data structures, pci_free_host_bridge(), export it
and update kernel functions releasing host bridge objects allocated
memory to make use of it.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Arnd Bergmann <***@arndb.de>
Cc: Bjorn Helgaas <***@google.com>
---
drivers/pci/probe.c | 12 +++++++++---
include/linux/pci.h | 1 +
2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 586d83d..cbf0d0c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -517,9 +517,7 @@ static void pci_release_host_bridge_dev(struct device *dev)
if (bridge->release_fn)
bridge->release_fn(bridge);

- pci_free_resource_list(&bridge->windows);
-
- kfree(bridge);
+ pci_free_host_bridge(bridge);
}

struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
@@ -537,6 +535,14 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv)
}
EXPORT_SYMBOL(pci_alloc_host_bridge);

+void pci_free_host_bridge(struct pci_host_bridge *bridge)
+{
+ pci_free_resource_list(&bridge->windows);
+
+ kfree(bridge);
+}
+EXPORT_SYMBOL(pci_free_host_bridge);
+
static const unsigned char pcix_bus_speed[] = {
PCI_SPEED_UNKNOWN, /* 0 */
PCI_SPEED_66MHz_PCIX, /* 1 */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8039f9f..5720635 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -463,6 +463,7 @@ static inline struct pci_host_bridge *pci_host_bridge_from_priv(void *priv)
}

struct pci_host_bridge *pci_alloc_host_bridge(size_t priv);
+void pci_free_host_bridge(struct pci_host_bridge *bridge);
int pci_register_host_bridge(struct pci_host_bridge *bridge);
struct pci_host_bridge *pci_find_host_bridge(struct pci_bus *bus);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:32 UTC
Permalink
Since, through struct pci_host_bridge.map/swizzle_irq hooks, IRQs
are now allocated in the pci_assign_irq() callback automatically,
PCI host bridge drivers can stop relying on pci_fixup_irqs()
for IRQ allocation.

Drop pci_fixup_irqs() usage from PCI rcar host bridge driver.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Simon Horman <***@verge.net.au>
---
drivers/pci/host/pcie-rcar.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 8380274..246d485 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -465,6 +465,8 @@ static int rcar_pcie_enable(struct rcar_pcie *pcie)
bridge->sysdata = pcie;
bridge->busnr = pcie->root_bus_nr;
bridge->ops = &rcar_pcie_ops;
+ bridge->map_irq = of_irq_parse_and_map_pci;
+ bridge->swizzle_irq = pci_common_swizzle;
if (IS_ENABLED(CONFIG_PCI_MSI))
bridge->msi = &pcie->msi.chip;

@@ -476,8 +478,6 @@ static int rcar_pcie_enable(struct rcar_pcie *pcie)

bus = bridge->bus;

- pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
-
pci_bus_size_bridges(bus);
pci_bus_assign_resources(bus);
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:28 UTC
Permalink
From: Matthew Minter <***@masarand.com>

The pci_assign_irq() function allows assignment of an irq to devices
during device enable time rather than only at boot. Therefore call it
in the pci_device_probe() function during the enable device code path
so this assignment can be performed.

This patch will do nothing on arches which do not set the irq mapping
function pointers and is therefore currently a nop, however as support
for these function pointers is added to arch specific code this will
cause irq assignment to migrate to device enable time allowing the new
code paths to be used.

Signed-off-by: Matthew Minter <***@masarand.com>
[***@arm.com: moved pci_assign_irq() call site]
Signed-off-by: Lorenzo Pieralisi <***@arm.com>
---
drivers/pci/pci-driver.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 192e7b6..11167c6 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -412,6 +412,8 @@ static int pci_device_probe(struct device *dev)
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);

+ pci_assign_irq(pci_dev);
+
error = pcibios_alloc_irq(pci_dev);
if (error < 0)
return error;
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:11 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert ARM bios32 code to pci_scan_root_bus_bridge() to improve
the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Jason Cooper <***@lakedaemon.net>
Cc: Bjorn Helgaas <***@google.com>
Cc: Russell King <***@armlinux.org.uk>
Cc: Andrew Lunn <***@lunn.ch>
---
arch/arm/include/asm/mach/pci.h | 3 ++-
arch/arm/kernel/bios32.c | 39 +++++++++++++++++++++++++--------------
arch/arm/mach-dove/pcie.c | 17 ++++++++++++-----
arch/arm/mach-iop13xx/pci.c | 29 ++++++++++++++++++-----------
arch/arm/mach-iop13xx/pci.h | 3 ++-
arch/arm/mach-mv78xx0/pcie.c | 8 +++-----
arch/arm/mach-orion5x/common.h | 3 ++-
arch/arm/mach-orion5x/pci.c | 25 +++++++++++++++++--------
8 files changed, 81 insertions(+), 46 deletions(-)

diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
index 2d88af5..233b4b5 100644
--- a/arch/arm/include/asm/mach/pci.h
+++ b/arch/arm/include/asm/mach/pci.h
@@ -16,6 +16,7 @@
struct pci_sys_data;
struct pci_ops;
struct pci_bus;
+struct pci_host_bridge;
struct device;

struct hw_pci {
@@ -25,7 +26,7 @@ struct hw_pci {
unsigned int io_optional:1;
void **private_data;
int (*setup)(int nr, struct pci_sys_data *);
- struct pci_bus *(*scan)(int nr, struct pci_sys_data *);
+ int (*scan)(int nr, struct pci_host_bridge *);
void (*preinit)(void);
void (*postinit)(void);
u8 (*swizzle)(struct pci_dev *dev, u8 *pin);
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index b259956..4c7621a 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -458,10 +458,14 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
int nr, busnr;

for (nr = busnr = 0; nr < hw->nr_controllers; nr++) {
- sys = kzalloc(sizeof(struct pci_sys_data), GFP_KERNEL);
- if (WARN(!sys, "PCI: unable to allocate sys data!"))
+ struct pci_host_bridge *bridge;
+
+ bridge = pci_alloc_host_bridge(sizeof(struct pci_sys_data));
+ if (WARN(!bridge, "PCI: unable to allocate bridge!"))
break;

+ sys = pci_host_bridge_priv(bridge);
+
sys->busnr = busnr;
sys->swizzle = hw->swizzle;
sys->map_irq = hw->map_irq;
@@ -473,7 +477,6 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
ret = hw->setup(nr, sys);

if (ret > 0) {
- struct pci_host_bridge *host_bridge;

ret = pcibios_init_resource(nr, sys, hw->io_optional);
if (ret) {
@@ -482,25 +485,33 @@ 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_msi(parent,
- sys->busnr, hw->ops, sys,
- &sys->resources, hw->msi_ctrl);
+ ret = hw->scan(nr, bridge);
+ else {
+ list_splice_init(&sys->resources,
+ &bridge->windows);
+ bridge->dev.parent = parent;
+ bridge->sysdata = sys;
+ bridge->busnr = sys->busnr;
+ bridge->ops = hw->ops;
+ bridge->msi = hw->msi_ctrl;
+ bridge->align_resource =
+ hw->align_resource;
+
+ ret = pci_scan_root_bus_bridge(bridge);
+ }

- if (WARN(!sys->bus, "PCI: unable to scan bus!")) {
- kfree(sys);
+ if (WARN(ret < 0, "PCI: unable to scan bus!")) {
+ pci_free_host_bridge(bridge);
break;
}

+ sys->bus = bridge->bus;
+
busnr = sys->bus->busn_res.end + 1;

list_add(&sys->node, head);
-
- host_bridge = pci_find_host_bridge(sys->bus);
- host_bridge->align_resource = hw->align_resource;
} else {
- kfree(sys);
+ pci_free_host_bridge(bridge);
if (ret < 0)
break;
}
diff --git a/arch/arm/mach-dove/pcie.c b/arch/arm/mach-dove/pcie.c
index 91fe971..dfb62f3 100644
--- a/arch/arm/mach-dove/pcie.c
+++ b/arch/arm/mach-dove/pcie.c
@@ -152,16 +152,23 @@ static void rc_pci_fixup(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL, PCI_ANY_ID, rc_pci_fixup);

-static struct pci_bus __init *
-dove_pcie_scan_bus(int nr, struct pci_sys_data *sys)
+static int __init
+dove_pcie_scan_bus(int nr, struct pci_host_bridge *bridge)
{
+ struct pci_sys_data *sys = pci_host_bridge_priv(bridge);
+
if (nr >= num_pcie_ports) {
BUG();
- return NULL;
+ return -EINVAL;
}

- return pci_scan_root_bus(NULL, sys->busnr, &pcie_ops, sys,
- &sys->resources);
+ list_splice_init(&sys->resources, &bridge->windows);
+ bridge->dev.parent = NULL;
+ bridge->sysdata = sys;
+ bridge->busnr = sys->busnr;
+ bridge->ops = &pcie_ops;
+
+ return pci_scan_root_bus_bridge(bridge);
}

static int __init dove_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
diff --git a/arch/arm/mach-iop13xx/pci.c b/arch/arm/mach-iop13xx/pci.c
index 204eb44..4babf15 100644
--- a/arch/arm/mach-iop13xx/pci.c
+++ b/arch/arm/mach-iop13xx/pci.c
@@ -504,10 +504,10 @@ iop13xx_pci_abort(unsigned long addr, unsigned int fsr, struct pt_regs *regs)

/* Scan an IOP13XX PCI bus. nr selects which ATU we use.
*/
-struct pci_bus *iop13xx_scan_bus(int nr, struct pci_sys_data *sys)
+int iop13xx_scan_bus(int nr, struct pci_host_bridge *bridge)
{
- int which_atu;
- struct pci_bus *bus = NULL;
+ int which_atu, ret;
+ struct pci_sys_data *sys = pci_host_bridge_priv(bridge);

switch (init_atu) {
case IOP13XX_INIT_ATU_ATUX:
@@ -525,9 +525,14 @@ struct pci_bus *iop13xx_scan_bus(int nr, struct pci_sys_data *sys)

if (!which_atu) {
BUG();
- return NULL;
+ return -ENODEV;
}

+ list_splice_init(&sys->resources, &bridge->windows);
+ bridge->dev.parent = NULL;
+ bridge->sysdata = sys;
+ bridge->busnr = sys->busnr;
+
switch (which_atu) {
case IOP13XX_INIT_ATU_ATUX:
if (time_after_eq(jiffies + msecs_to_jiffies(1000),
@@ -535,18 +540,20 @@ struct pci_bus *iop13xx_scan_bus(int nr, struct pci_sys_data *sys)
while(time_before(jiffies, atux_trhfa_timeout))
udelay(100);

- bus = pci_bus_atux = pci_scan_root_bus(NULL, sys->busnr,
- &iop13xx_atux_ops,
- sys, &sys->resources);
+ bridge->ops = &iop13xx_atux_ops;
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (!ret)
+ pci_bus_atux = bridge->bus;
break;
case IOP13XX_INIT_ATU_ATUE:
- bus = pci_bus_atue = pci_scan_root_bus(NULL, sys->busnr,
- &iop13xx_atue_ops,
- sys, &sys->resources);
+ bridge->ops = &iop13xx_atue_ops;
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (!ret)
+ pci_bus_atue = bridge->bus;
break;
}

- return bus;
+ return ret;
}

/* This function is called from iop13xx_pci_init() after assigning valid
diff --git a/arch/arm/mach-iop13xx/pci.h b/arch/arm/mach-iop13xx/pci.h
index 71b9c57..8dc343c 100644
--- a/arch/arm/mach-iop13xx/pci.h
+++ b/arch/arm/mach-iop13xx/pci.h
@@ -11,9 +11,10 @@ extern size_t iop13xx_atue_mem_size;
extern size_t iop13xx_atux_mem_size;

struct pci_sys_data;
+struct pci_host_bridge;
struct hw_pci;
int iop13xx_pci_setup(int nr, struct pci_sys_data *sys);
-struct pci_bus *iop13xx_scan_bus(int nr, struct pci_sys_data *);
+int iop13xx_scan_bus(int nr, struct pci_host_bridge *bridge);
void iop13xx_atu_select(struct hw_pci *plat_pci);
void iop13xx_pci_init(void);
void iop13xx_map_pci_memory(void);
diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
index 81ff432..2b406e9 100644
--- a/arch/arm/mach-mv78xx0/pcie.c
+++ b/arch/arm/mach-mv78xx0/pcie.c
@@ -194,16 +194,14 @@ static void rc_pci_fixup(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL, PCI_ANY_ID, rc_pci_fixup);

-static struct pci_bus __init *
-mv78xx0_pcie_scan_bus(int nr, struct pci_sys_data *sys)
+static int __init mv78xx0_pcie_scan_bus(int nr, struct pci_host_bridge *bridge)
{
if (nr >= num_pcie_ports) {
BUG();
- return NULL;
+ return -EINVAL;
}

- return pci_scan_root_bus(NULL, sys->busnr, &pcie_ops, sys,
- &sys->resources);
+ return pci_scan_root_bus_bridge(bridge);
}

static int __init mv78xx0_pcie_map_irq(const struct pci_dev *dev, u8 slot,
diff --git a/arch/arm/mach-orion5x/common.h b/arch/arm/mach-orion5x/common.h
index efeffc6..4c0c7de 100644
--- a/arch/arm/mach-orion5x/common.h
+++ b/arch/arm/mach-orion5x/common.h
@@ -54,6 +54,7 @@ void orion5x_restart(enum reboot_mode, const char *);
* PCIe/PCI functions.
*/
struct pci_bus;
+struct pci_host_bridge;
struct pci_sys_data;
struct pci_dev;

@@ -61,7 +62,7 @@ void orion5x_pcie_id(u32 *dev, u32 *rev);
void orion5x_pci_disable(void);
void orion5x_pci_set_cardbus_mode(void);
int orion5x_pci_sys_setup(int nr, struct pci_sys_data *sys);
-struct pci_bus *orion5x_pci_sys_scan_bus(int nr, struct pci_sys_data *sys);
+int orion5x_pci_sys_scan_bus(int nr, struct pci_host_bridge *bridge);
int orion5x_pci_map_irq(const struct pci_dev *dev, u8 slot, u8 pin);

struct tag;
diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
index ecb998e..76951bf 100644
--- a/arch/arm/mach-orion5x/pci.c
+++ b/arch/arm/mach-orion5x/pci.c
@@ -555,18 +555,27 @@ int __init orion5x_pci_sys_setup(int nr, struct pci_sys_data *sys)
return 0;
}

-struct pci_bus __init *orion5x_pci_sys_scan_bus(int nr, struct pci_sys_data *sys)
+int __init orion5x_pci_sys_scan_bus(int nr, struct pci_host_bridge *bridge)
{
- if (nr == 0)
- return pci_scan_root_bus(NULL, sys->busnr, &pcie_ops, sys,
- &sys->resources);
+ struct pci_sys_data *sys = pci_host_bridge_priv(bridge);

- if (nr == 1 && !orion5x_pci_disabled)
- return pci_scan_root_bus(NULL, sys->busnr, &pci_ops, sys,
- &sys->resources);
+ list_splice_init(&sys->resources, &bridge->windows);
+ bridge->dev.parent = NULL;
+ bridge->sysdata = sys;
+ bridge->busnr = sys->busnr;
+
+ if (nr == 0) {
+ bridge->ops = &pcie_ops;
+ return pci_scan_root_bus_bridge(bridge);
+ }
+
+ if (nr == 1 && !orion5x_pci_disabled) {
+ bridge->ops = &pci_ops;
+ return pci_scan_root_bus_bridge(bridge);
+ }

BUG();
- return NULL;
+ return -ENODEV;
}

int __init orion5x_pci_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
--
2.10.0
Lorenzo Pieralisi
2017-06-08 14:13:16 UTC
Permalink
The introduction of pci_scan_root_bus_bridge() provides a PCI core
API to scan a PCI root bus backed by an already initialized
struct pci_host_bridge object, which simplifies the bus scan
interface and makes the PCI scan root bus interface easier to
generalize as members are added to the struct pci_host_bridge.

Convert PCI versatile host code to pci_scan_root_bus_bridge() to
improve the PCI root bus scanning interface.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
Cc: Bjorn Helgaas <***@google.com>
Cc: Rob Herring <***@kernel.org>
---
drivers/pci/host/pci-versatile.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pci-versatile.c b/drivers/pci/host/pci-versatile.c
index 9281eee..a67cf0b 100644
--- a/drivers/pci/host/pci-versatile.c
+++ b/drivers/pci/host/pci-versatile.c
@@ -125,8 +125,13 @@ static int versatile_pci_probe(struct platform_device *pdev)
u32 val;
void __iomem *local_pci_cfg_base;
struct pci_bus *bus, *child;
+ struct pci_host_bridge *bridge;
LIST_HEAD(pci_res);

+ bridge = devm_pci_alloc_host_bridge(dev, 0);
+ if (!bridge)
+ return -ENOMEM;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
versatile_pci_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(versatile_pci_base))
@@ -199,11 +204,20 @@ static int versatile_pci_probe(struct platform_device *pdev)
pci_add_flags(PCI_ENABLE_PROC_DOMAINS);
pci_add_flags(PCI_REASSIGN_ALL_BUS | PCI_REASSIGN_ALL_RSRC);

- bus = pci_scan_root_bus(&pdev->dev, 0, &pci_versatile_ops, NULL, &pci_res);
- if (!bus)
- return -ENOMEM;
+ list_splice_init(&pci_res, &bridge->windows);
+ bridge->dev.parent = &pdev->dev;
+ bridge->sysdata = NULL;
+ bridge->busnr = 0;
+ bridge->ops = &pci_versatile_ops;
+
+ ret = pci_scan_root_bus_bridge(bridge);
+ if (ret < 0)
+ return ret;
+
+ bus = bridge->bus;

pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+
pci_assign_unassigned_bus_resources(bus);
list_for_each_entry(child, &bus->children, node)
pcie_bus_configure_settings(child);
--
2.10.0
Will Deacon
2017-06-12 15:45:38 UTC
Permalink
v1 -> v2
- Split pci_fixup_irqs() removal in several patches
- Embedded pci_register_host_bridge() in pci_scan_root_bus_bridge()
- Refactored iproc, faraday and xilinx-nwl host bridges in preparation
for irq fixup removal
- Converted all DT PCI host bridge drivers to pci_scan_root_bus_bridge()
- Dropped RFC tag
- Added devm_pci_alloc_host_bridge() interface
- Rebased against v4.12-rc3
For the arm64 and the host-common bits:

Acked-by: Will Deacon <***@arm.com>

Will
Lorenzo Pieralisi
2017-06-12 16:20:36 UTC
Permalink
v1 -> v2
- Split pci_fixup_irqs() removal in several patches
- Embedded pci_register_host_bridge() in pci_scan_root_bus_bridge()
- Refactored iproc, faraday and xilinx-nwl host bridges in preparation
for irq fixup removal
- Converted all DT PCI host bridge drivers to pci_scan_root_bus_bridge()
- Dropped RFC tag
- Added devm_pci_alloc_host_bridge() interface
- Rebased against v4.12-rc3
Thank you !
Lorenzo
Khuong Dinh
2017-06-12 23:58:20 UTC
Permalink
Hi Lorenzo,

Tested-by: Khuong Dinh <***@apm.com>

I tested the patchset on X-Gene boards. It works fine with e1000e.

Best regards,
Khuong Dinh

On Thu, Jun 8, 2017 at 7:13 AM, Lorenzo Pieralisi
v1 -> v2
- Split pci_fixup_irqs() removal in several patches
- Embedded pci_register_host_bridge() in pci_scan_root_bus_bridge()
- Refactored iproc, faraday and xilinx-nwl host bridges in preparation
for irq fixup removal
- Converted all DT PCI host bridge drivers to pci_scan_root_bus_bridge()
- Dropped RFC tag
- Added devm_pci_alloc_host_bridge() interface
- Rebased against v4.12-rc3
v1: http://marc.info/?l=linux-pci&m=149320545522205&w=2
Current pci_fixup_irqs() usage on ARM/ARM64 host controller drivers is
flawed in that pci_fixup_irqs() allocates IRQs for all PCI devices present
in a system; those PCI devices possibly belong to different PCI bus trees
(and possibly rooted at different host bridges) and may well be enabled
(ie probed and bound to a driver) by the time pci_fixup_irqs() is called
when probing a given host bridge driver.
Furthermore, current kernel code relying on pci_fixup_irqs() to
assign legacy PCI IRQs to devices does not work at all for
hotplugged devices in that the code carrying out the IRQ fixup
is called at host bridge driver probe time, which just cannot take
into account devices hotplugged after system has booted.
this series[1] adds IRQs mapping and swizzling primitives to
the struct pci_host_bridge which allows IRQs to be allocated for
for a device at probe time with host bridge specific functions,
fixing the aforementioned limitations.
Current series remove pci_fixup_irqs() usage on ARM/ARM64; removal
can be extended to other architectures provided the IRQs map/swizzle
functions are set-up properly in the respective host bridges
set-up/probe paths.
Tested on kvmtool with PCI host generic.
[1] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
PCI: xilinx-nwl: Remove nwl_pcie_enable_msi() unused bus parameter
drivers: pci: host: iproc: Convert link check to raw PCI config
accessors
drivers: pci: host: ftpci100: convert IRQ masking to raw PCI config
accessors
PCI: Initialize bridge release function at bridge allocation
PCI: Add pci_free_host_bridge interface
PCI: Add devm_pci_alloc_host_bridge() interface
drivers: pci: host: ftpci100: Fix host bridge memory leakage
drivers: pci: host: tegra: Fix host bridge memory leakage
PCI: Introduce pci_scan_root_bus_bridge()
PCI: Make pci_register_host_bridge() PCI core internal
ARM: PCI: bios32: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: designware: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: aardvark: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: rcar: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: iproc: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: versatile: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: altera: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: xilinx: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: xgene: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: host-common: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: rockchip: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: xilinx-nwl: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: Remove pci_scan_root_bus_msi()
ARM: PCI: Remove pci_fixup_irqs() call for bios32 host controllers
PCI: tegra: Drop pci_fixup_irqs()
PCI: xilinx: Drop pci_fixup_irqs()
PCI: rcar: Drop pci_fixup_irqs()
PCI: iproc: Drop pci_fixup_irqs()
PCI: designware-host: Drop pci_fixup_irqs()
PCI: ftpci100: Drop pci_fixup_irqs()
PCI: host-common: Drop pci_fixup_irqs()
PCI: versatile: Drop pci_fixup_irqs()
PCI: altera: Drop pci_fixup_irqs()
PCI: xgene: Move to struct pci_host_bridge IRQ mapping functions
PCI: rockchip: Move to struct pci_host_bridge IRQ mapping functions
PCI: xilinx-nwl: Move to struct pci_host_bridge IRQ mapping functions
ARM64: PCI: Drop DT IRQ allocation from pcibios_alloc_irq()
PCI: Build setup-irq.o on all arches
PCI: Add IRQ mapping function pointers to pci_host_bridge struct
PCI: Add pci_assign_irq() function and have pci_fixup_irqs() use it
OF/PCI: Update of_irq_parse_and_map_pci() comment
PCI: Add a call to pci_assign_irq() in pci_device_probe()
Documentation/driver-model/devres.txt | 1 +
arch/arm/include/asm/mach/pci.h | 3 +-
arch/arm/kernel/bios32.c | 43 +++++++----
arch/arm/mach-dove/pcie.c | 33 +++++---
arch/arm/mach-iop13xx/pci.c | 31 +++++---
arch/arm/mach-iop13xx/pci.h | 3 +-
arch/arm/mach-mv78xx0/pcie.c | 8 +-
arch/arm/mach-orion5x/common.h | 3 +-
arch/arm/mach-orion5x/pci.c | 39 ++++++----
arch/arm64/kernel/pci.c | 10 +--
drivers/of/of_pci_irq.c | 3 +-
drivers/pci/Makefile | 17 +----
drivers/pci/dwc/pcie-designware-host.c | 43 ++++++-----
drivers/pci/host/pci-aardvark.c | 21 +++--
drivers/pci/host/pci-ftpci100.c | 50 +++++++-----
drivers/pci/host/pci-host-common.c | 27 +++++--
drivers/pci/host/pci-tegra.c | 9 +--
drivers/pci/host/pci-versatile.c | 22 +++++-
drivers/pci/host/pci-xgene.c | 23 ++++--
drivers/pci/host/pcie-altera.c | 24 ++++--
drivers/pci/host/pcie-iproc-bcma.c | 7 +-
drivers/pci/host/pcie-iproc-platform.c | 7 +-
drivers/pci/host/pcie-iproc.c | 135 +++++++++++++++++++++++----------
drivers/pci/host/pcie-rcar.c | 40 ++++++----
drivers/pci/host/pcie-rockchip.c | 24 ++++--
drivers/pci/host/pcie-xilinx-nwl.c | 34 ++++++---
drivers/pci/host/pcie-xilinx.c | 34 +++++----
drivers/pci/pci-driver.c | 2 +
drivers/pci/probe.c | 105 ++++++++++++++++++-------
drivers/pci/setup-irq.c | 45 ++++++++---
include/linux/pci.h | 12 +--
31 files changed, 565 insertions(+), 293 deletions(-)
--
2.10.0
Bjorn Helgaas
2017-06-19 23:12:17 UTC
Permalink
v1 -> v2
- Split pci_fixup_irqs() removal in several patches
- Embedded pci_register_host_bridge() in pci_scan_root_bus_bridge()
- Refactored iproc, faraday and xilinx-nwl host bridges in preparation
for irq fixup removal
- Converted all DT PCI host bridge drivers to pci_scan_root_bus_bridge()
- Dropped RFC tag
- Added devm_pci_alloc_host_bridge() interface
- Rebased against v4.12-rc3
v1: http://marc.info/?l=linux-pci&m=149320545522205&w=2
Current pci_fixup_irqs() usage on ARM/ARM64 host controller drivers is
flawed in that pci_fixup_irqs() allocates IRQs for all PCI devices present
in a system; those PCI devices possibly belong to different PCI bus trees
(and possibly rooted at different host bridges) and may well be enabled
(ie probed and bound to a driver) by the time pci_fixup_irqs() is called
when probing a given host bridge driver.
Furthermore, current kernel code relying on pci_fixup_irqs() to
assign legacy PCI IRQs to devices does not work at all for
hotplugged devices in that the code carrying out the IRQ fixup
is called at host bridge driver probe time, which just cannot take
into account devices hotplugged after system has booted.
this series[1] adds IRQs mapping and swizzling primitives to
the struct pci_host_bridge which allows IRQs to be allocated for
for a device at probe time with host bridge specific functions,
fixing the aforementioned limitations.
Current series remove pci_fixup_irqs() usage on ARM/ARM64; removal
can be extended to other architectures provided the IRQs map/swizzle
functions are set-up properly in the respective host bridges
set-up/probe paths.
Tested on kvmtool with PCI host generic.
[1] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
PCI: xilinx-nwl: Remove nwl_pcie_enable_msi() unused bus parameter
drivers: pci: host: iproc: Convert link check to raw PCI config
accessors
drivers: pci: host: ftpci100: convert IRQ masking to raw PCI config
accessors
PCI: Initialize bridge release function at bridge allocation
PCI: Add pci_free_host_bridge interface
PCI: Add devm_pci_alloc_host_bridge() interface
drivers: pci: host: ftpci100: Fix host bridge memory leakage
drivers: pci: host: tegra: Fix host bridge memory leakage
PCI: Introduce pci_scan_root_bus_bridge()
PCI: Make pci_register_host_bridge() PCI core internal
ARM: PCI: bios32: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: designware: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: aardvark: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: rcar: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: iproc: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: versatile: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: altera: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: xilinx: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: xgene: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: host-common: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: rockchip: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: xilinx-nwl: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: Remove pci_scan_root_bus_msi()
ARM: PCI: Remove pci_fixup_irqs() call for bios32 host controllers
PCI: tegra: Drop pci_fixup_irqs()
PCI: xilinx: Drop pci_fixup_irqs()
PCI: rcar: Drop pci_fixup_irqs()
PCI: iproc: Drop pci_fixup_irqs()
PCI: designware-host: Drop pci_fixup_irqs()
PCI: ftpci100: Drop pci_fixup_irqs()
PCI: host-common: Drop pci_fixup_irqs()
PCI: versatile: Drop pci_fixup_irqs()
PCI: altera: Drop pci_fixup_irqs()
PCI: xgene: Move to struct pci_host_bridge IRQ mapping functions
PCI: rockchip: Move to struct pci_host_bridge IRQ mapping functions
PCI: xilinx-nwl: Move to struct pci_host_bridge IRQ mapping functions
ARM64: PCI: Drop DT IRQ allocation from pcibios_alloc_irq()
PCI: Build setup-irq.o on all arches
PCI: Add IRQ mapping function pointers to pci_host_bridge struct
PCI: Add pci_assign_irq() function and have pci_fixup_irqs() use it
OF/PCI: Update of_irq_parse_and_map_pci() comment
PCI: Add a call to pci_assign_irq() in pci_device_probe()
Documentation/driver-model/devres.txt | 1 +
arch/arm/include/asm/mach/pci.h | 3 +-
arch/arm/kernel/bios32.c | 43 +++++++----
arch/arm/mach-dove/pcie.c | 33 +++++---
arch/arm/mach-iop13xx/pci.c | 31 +++++---
arch/arm/mach-iop13xx/pci.h | 3 +-
arch/arm/mach-mv78xx0/pcie.c | 8 +-
arch/arm/mach-orion5x/common.h | 3 +-
arch/arm/mach-orion5x/pci.c | 39 ++++++----
arch/arm64/kernel/pci.c | 10 +--
drivers/of/of_pci_irq.c | 3 +-
drivers/pci/Makefile | 17 +----
drivers/pci/dwc/pcie-designware-host.c | 43 ++++++-----
drivers/pci/host/pci-aardvark.c | 21 +++--
drivers/pci/host/pci-ftpci100.c | 50 +++++++-----
drivers/pci/host/pci-host-common.c | 27 +++++--
drivers/pci/host/pci-tegra.c | 9 +--
drivers/pci/host/pci-versatile.c | 22 +++++-
drivers/pci/host/pci-xgene.c | 23 ++++--
drivers/pci/host/pcie-altera.c | 24 ++++--
drivers/pci/host/pcie-iproc-bcma.c | 7 +-
drivers/pci/host/pcie-iproc-platform.c | 7 +-
drivers/pci/host/pcie-iproc.c | 135 +++++++++++++++++++++++----------
drivers/pci/host/pcie-rcar.c | 40 ++++++----
drivers/pci/host/pcie-rockchip.c | 24 ++++--
drivers/pci/host/pcie-xilinx-nwl.c | 34 ++++++---
drivers/pci/host/pcie-xilinx.c | 34 +++++----
drivers/pci/pci-driver.c | 2 +
drivers/pci/probe.c | 105 ++++++++++++++++++-------
drivers/pci/setup-irq.c | 45 ++++++++---
include/linux/pci.h | 12 +--
31 files changed, 565 insertions(+), 293 deletions(-)
This is awesome! I put these on pci/enumeration with the acks received so
far. I hope we can merge these for v4.13, so please take a look at your
drivers and test if possible.

Bjorn
Lorenzo Pieralisi
2017-06-20 14:44:40 UTC
Permalink
Post by Bjorn Helgaas
v1 -> v2
- Split pci_fixup_irqs() removal in several patches
- Embedded pci_register_host_bridge() in pci_scan_root_bus_bridge()
- Refactored iproc, faraday and xilinx-nwl host bridges in preparation
for irq fixup removal
- Converted all DT PCI host bridge drivers to pci_scan_root_bus_bridge()
- Dropped RFC tag
- Added devm_pci_alloc_host_bridge() interface
- Rebased against v4.12-rc3
v1: http://marc.info/?l=linux-pci&m=149320545522205&w=2
Current pci_fixup_irqs() usage on ARM/ARM64 host controller drivers is
flawed in that pci_fixup_irqs() allocates IRQs for all PCI devices present
in a system; those PCI devices possibly belong to different PCI bus trees
(and possibly rooted at different host bridges) and may well be enabled
(ie probed and bound to a driver) by the time pci_fixup_irqs() is called
when probing a given host bridge driver.
Furthermore, current kernel code relying on pci_fixup_irqs() to
assign legacy PCI IRQs to devices does not work at all for
hotplugged devices in that the code carrying out the IRQ fixup
is called at host bridge driver probe time, which just cannot take
into account devices hotplugged after system has booted.
this series[1] adds IRQs mapping and swizzling primitives to
the struct pci_host_bridge which allows IRQs to be allocated for
for a device at probe time with host bridge specific functions,
fixing the aforementioned limitations.
Current series remove pci_fixup_irqs() usage on ARM/ARM64; removal
can be extended to other architectures provided the IRQs map/swizzle
functions are set-up properly in the respective host bridges
set-up/probe paths.
Tested on kvmtool with PCI host generic.
[1] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
PCI: xilinx-nwl: Remove nwl_pcie_enable_msi() unused bus parameter
drivers: pci: host: iproc: Convert link check to raw PCI config
accessors
drivers: pci: host: ftpci100: convert IRQ masking to raw PCI config
accessors
PCI: Initialize bridge release function at bridge allocation
PCI: Add pci_free_host_bridge interface
PCI: Add devm_pci_alloc_host_bridge() interface
drivers: pci: host: ftpci100: Fix host bridge memory leakage
drivers: pci: host: tegra: Fix host bridge memory leakage
PCI: Introduce pci_scan_root_bus_bridge()
PCI: Make pci_register_host_bridge() PCI core internal
ARM: PCI: bios32: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: designware: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: aardvark: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: rcar: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: iproc: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: versatile: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: altera: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: xilinx: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: xgene: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: host-common: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: rockchip: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: xilinx-nwl: Convert PCI scan API to pci_scan_root_bus_bridge()
PCI: Remove pci_scan_root_bus_msi()
ARM: PCI: Remove pci_fixup_irqs() call for bios32 host controllers
PCI: tegra: Drop pci_fixup_irqs()
PCI: xilinx: Drop pci_fixup_irqs()
PCI: rcar: Drop pci_fixup_irqs()
PCI: iproc: Drop pci_fixup_irqs()
PCI: designware-host: Drop pci_fixup_irqs()
PCI: ftpci100: Drop pci_fixup_irqs()
PCI: host-common: Drop pci_fixup_irqs()
PCI: versatile: Drop pci_fixup_irqs()
PCI: altera: Drop pci_fixup_irqs()
PCI: xgene: Move to struct pci_host_bridge IRQ mapping functions
PCI: rockchip: Move to struct pci_host_bridge IRQ mapping functions
PCI: xilinx-nwl: Move to struct pci_host_bridge IRQ mapping functions
ARM64: PCI: Drop DT IRQ allocation from pcibios_alloc_irq()
PCI: Build setup-irq.o on all arches
PCI: Add IRQ mapping function pointers to pci_host_bridge struct
PCI: Add pci_assign_irq() function and have pci_fixup_irqs() use it
OF/PCI: Update of_irq_parse_and_map_pci() comment
PCI: Add a call to pci_assign_irq() in pci_device_probe()
Documentation/driver-model/devres.txt | 1 +
arch/arm/include/asm/mach/pci.h | 3 +-
arch/arm/kernel/bios32.c | 43 +++++++----
arch/arm/mach-dove/pcie.c | 33 +++++---
arch/arm/mach-iop13xx/pci.c | 31 +++++---
arch/arm/mach-iop13xx/pci.h | 3 +-
arch/arm/mach-mv78xx0/pcie.c | 8 +-
arch/arm/mach-orion5x/common.h | 3 +-
arch/arm/mach-orion5x/pci.c | 39 ++++++----
arch/arm64/kernel/pci.c | 10 +--
drivers/of/of_pci_irq.c | 3 +-
drivers/pci/Makefile | 17 +----
drivers/pci/dwc/pcie-designware-host.c | 43 ++++++-----
drivers/pci/host/pci-aardvark.c | 21 +++--
drivers/pci/host/pci-ftpci100.c | 50 +++++++-----
drivers/pci/host/pci-host-common.c | 27 +++++--
drivers/pci/host/pci-tegra.c | 9 +--
drivers/pci/host/pci-versatile.c | 22 +++++-
drivers/pci/host/pci-xgene.c | 23 ++++--
drivers/pci/host/pcie-altera.c | 24 ++++--
drivers/pci/host/pcie-iproc-bcma.c | 7 +-
drivers/pci/host/pcie-iproc-platform.c | 7 +-
drivers/pci/host/pcie-iproc.c | 135 +++++++++++++++++++++++----------
drivers/pci/host/pcie-rcar.c | 40 ++++++----
drivers/pci/host/pcie-rockchip.c | 24 ++++--
drivers/pci/host/pcie-xilinx-nwl.c | 34 ++++++---
drivers/pci/host/pcie-xilinx.c | 34 +++++----
drivers/pci/pci-driver.c | 2 +
drivers/pci/probe.c | 105 ++++++++++++++++++-------
drivers/pci/setup-irq.c | 45 ++++++++---
include/linux/pci.h | 12 +--
31 files changed, 565 insertions(+), 293 deletions(-)
This is awesome! I put these on pci/enumeration with the acks received so
far. I hope we can merge these for v4.13, so please take a look at your
drivers and test if possible.
Thank you very much Bjorn. Yes it would help me a lot if CC'ed people
can help me test it (and also Matt's patch [PATCH 26] reworked
pci_fixup_irqs() on top of struct pci_host_bridge hooks so even if they
should not be affected all arches other than ARM/ARM64 should be tested
too - I have not extended the CC list since it is huge already).

Thanks !
Lorenzo
Linus Walleij
2017-06-21 08:39:31 UTC
Permalink
On Thu, Jun 8, 2017 at 4:13 PM, Lorenzo Pieralisi
[1] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
And then this happens:

OF: PCI: host bridge /soc/***@50000000 ranges:
OF: PCI: IO 0x50000000..0x500fffff -> 0x00000000
OF: PCI: MEM 0x58000000..0x5fffffff -> 0x58000000
Unable to handle kernel NULL pointer dereference at virtual address 00000070
pgd = c0004000
[00000070] *pgd=00000000
Internal error: Oops: 17 [#1] PREEMPT ARM
CPU: 0 PID: 254 Comm: kworker/0:1 Not tainted 4.12.0-rc3+ #888
Hardware name: Gemini (Device Tree)
Workqueue: events deferred_probe_work_func
task: c38c3a40 task.stack: c390a000
PC is at faraday_pci_probe+0x274/0x65c
LR is at __irq_put_desc_unlock+0x18/0x70
pc : [<c0232648>] lr : [<c004be48>] psr: 80000013
sp : c390bdf0 ip : c3424274 fp : c3ac2ee0
r10: c3f77718 r9 : c3ad49b0 r8 : c3907800
r7 : 00000004 r6 : c390be18 r5 : c3907810 r4 : c3ad4810
r3 : 00000000 r2 : 00000000 r1 : 00000001 r0 : c3ac2de0
Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 0000397f Table: 03a9c000 DAC: 00000053
Process kworker/0:1 (pid: 254, stack limit = 0xc390a190)
Stack: (0xc390bdf0 to 0xc390c000)
bde0: c0534a60 c3ad49b0 00000003 c024cb68
be00: 00000001 20000013 00000000 50000000 00000007 000020dd c3ae2680 c3ae2740
be20: c07c861f c0134328 00000000 c3ad5fa0 c3910a50 c3910a50 00000001 00000000
be40: 00000000 c01344ec c3910a50 c3910a50 00000001 00000000 c3910a50 c39d76e0
be60: c3ad5fa0 c05f6f58 c3910a50 c3907810 c3907810 c07cba88 fffffdfb 00000003
be80: 00000000 c07c5040 c386e420 c02bbbe0 c3907810 c081c6c4 00000000 c07cba88
bea0: 00000003 c02ba9dc 00000000 c390bee0 c02bab30 00000001 c07bf660 00000000
bec0: c07c5040 c02b93dc c382829c c39886f4 c3907810 c3907844 c3907810 c02ba674
bee0: c3907810 00000001 c390bf0c c3907810 c07d3a80 c3907810 c07d38e4 c02b961c
bf00: c3907810 c07d38d0 c07d38d0 c02ba210 c07d38ec c386e420 00000000 c3f6b100
bf20: c07bf660 c002dac0 00000008 c07bf660 c07bf660 c386e438 c390a000 c07bf674
bf40: 00000008 c07bf660 c07c5040 c002e044 c390a000 00000001 00000000 00000000
bf60: c3901518 c3901500 ffffe000 c38e2700 00000000 c3901518 c386e420 c002ddb4
bf80: c3839ee4 c00335ec c390a000 c38e2700 c003350c 00000000 00000000 00000000
bfa0: 00000000 00000000 00000000 c000a270 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
[<c0232648>] (faraday_pci_probe) from [<c02bbbe0>]
(platform_drv_probe+0x50/0xb4)
[<c02bbbe0>] (platform_drv_probe) from [<c02ba9dc>]
(driver_probe_device+0x248/0x2dc)
[<c02ba9dc>] (driver_probe_device) from [<c02b93dc>]
(bus_for_each_drv+0x68/0x98)
[<c02b93dc>] (bus_for_each_drv) from [<c02ba674>] (__device_attach+0xa8/0x110)
[<c02ba674>] (__device_attach) from [<c02b961c>] (bus_probe_device+0x88/0x90)
[<c02b961c>] (bus_probe_device) from [<c02ba210>]
(deferred_probe_work_func+0x54/0x80)
[<c02ba210>] (deferred_probe_work_func) from [<c002dac0>]
(process_one_work+0x13c/0x430)
[<c002dac0>] (process_one_work) from [<c002e044>] (worker_thread+0x290/0x5f4)
[<c002e044>] (worker_thread) from [<c00335ec>] (kthread+0xe0/0x120)
[<c00335ec>] (kthread) from [<c000a270>] (ret_from_fork+0x14/0x24)
Code: e59401b0 e3700a01 8a00001d e59421ac (e5d23070)
---[ end trace 1e313b1ea11e101c ]---

I'm trying to figure out what is causing it.

Yours,
Linus Walleij
Lorenzo Pieralisi
2017-06-21 09:50:53 UTC
Permalink
Post by Linus Walleij
On Thu, Jun 8, 2017 at 4:13 PM, Lorenzo Pieralisi
[1] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
Thank you Linus for helping me out.
Post by Linus Walleij
OF: PCI: IO 0x50000000..0x500fffff -> 0x00000000
OF: PCI: MEM 0x58000000..0x5fffffff -> 0x58000000
Unable to handle kernel NULL pointer dereference at virtual address 00000070
pgd = c0004000
[00000070] *pgd=00000000
Internal error: Oops: 17 [#1] PREEMPT ARM
CPU: 0 PID: 254 Comm: kworker/0:1 Not tainted 4.12.0-rc3+ #888
Hardware name: Gemini (Device Tree)
Workqueue: events deferred_probe_work_func
task: c38c3a40 task.stack: c390a000
PC is at faraday_pci_probe+0x274/0x65c
LR is at __irq_put_desc_unlock+0x18/0x70
pc : [<c0232648>] lr : [<c004be48>] psr: 80000013
I think I see what it is. faraday_pci_irq_handler() relies on
struct faraday_pci->bus to access config space and with this
series it can't be set till pci_scan_root_bus_bridge() returns.

If that's the case we will have to update the config accessors in
faraday_pci_irq_handler() to raw ones (ie ones that do not depend on the
struct pci_bus), code is already there, patch coming, sorry about this
but it is extremely complicated for me to untangle these drivers inner
workings.

Thanks,
Lorenzo
Post by Linus Walleij
sp : c390bdf0 ip : c3424274 fp : c3ac2ee0
r10: c3f77718 r9 : c3ad49b0 r8 : c3907800
r7 : 00000004 r6 : c390be18 r5 : c3907810 r4 : c3ad4810
r3 : 00000000 r2 : 00000000 r1 : 00000001 r0 : c3ac2de0
Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 0000397f Table: 03a9c000 DAC: 00000053
Process kworker/0:1 (pid: 254, stack limit = 0xc390a190)
Stack: (0xc390bdf0 to 0xc390c000)
bde0: c0534a60 c3ad49b0 00000003 c024cb68
be00: 00000001 20000013 00000000 50000000 00000007 000020dd c3ae2680 c3ae2740
be20: c07c861f c0134328 00000000 c3ad5fa0 c3910a50 c3910a50 00000001 00000000
be40: 00000000 c01344ec c3910a50 c3910a50 00000001 00000000 c3910a50 c39d76e0
be60: c3ad5fa0 c05f6f58 c3910a50 c3907810 c3907810 c07cba88 fffffdfb 00000003
be80: 00000000 c07c5040 c386e420 c02bbbe0 c3907810 c081c6c4 00000000 c07cba88
bea0: 00000003 c02ba9dc 00000000 c390bee0 c02bab30 00000001 c07bf660 00000000
bec0: c07c5040 c02b93dc c382829c c39886f4 c3907810 c3907844 c3907810 c02ba674
bee0: c3907810 00000001 c390bf0c c3907810 c07d3a80 c3907810 c07d38e4 c02b961c
bf00: c3907810 c07d38d0 c07d38d0 c02ba210 c07d38ec c386e420 00000000 c3f6b100
bf20: c07bf660 c002dac0 00000008 c07bf660 c07bf660 c386e438 c390a000 c07bf674
bf40: 00000008 c07bf660 c07c5040 c002e044 c390a000 00000001 00000000 00000000
bf60: c3901518 c3901500 ffffe000 c38e2700 00000000 c3901518 c386e420 c002ddb4
bf80: c3839ee4 c00335ec c390a000 c38e2700 c003350c 00000000 00000000 00000000
bfa0: 00000000 00000000 00000000 c000a270 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
[<c0232648>] (faraday_pci_probe) from [<c02bbbe0>]
(platform_drv_probe+0x50/0xb4)
[<c02bbbe0>] (platform_drv_probe) from [<c02ba9dc>]
(driver_probe_device+0x248/0x2dc)
[<c02ba9dc>] (driver_probe_device) from [<c02b93dc>]
(bus_for_each_drv+0x68/0x98)
[<c02b93dc>] (bus_for_each_drv) from [<c02ba674>] (__device_attach+0xa8/0x110)
[<c02ba674>] (__device_attach) from [<c02b961c>] (bus_probe_device+0x88/0x90)
[<c02b961c>] (bus_probe_device) from [<c02ba210>]
(deferred_probe_work_func+0x54/0x80)
[<c02ba210>] (deferred_probe_work_func) from [<c002dac0>]
(process_one_work+0x13c/0x430)
[<c002dac0>] (process_one_work) from [<c002e044>] (worker_thread+0x290/0x5f4)
[<c002e044>] (worker_thread) from [<c00335ec>] (kthread+0xe0/0x120)
[<c00335ec>] (kthread) from [<c000a270>] (ret_from_fork+0x14/0x24)
Code: e59401b0 e3700a01 8a00001d e59421ac (e5d23070)
---[ end trace 1e313b1ea11e101c ]---
I'm trying to figure out what is causing it.
Yours,
Linus Walleij
Lorenzo Pieralisi
2017-06-21 10:30:42 UTC
Permalink
Post by Linus Walleij
On Thu, Jun 8, 2017 at 4:13 PM, Lorenzo Pieralisi
[1] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
OF: PCI: IO 0x50000000..0x500fffff -> 0x00000000
OF: PCI: MEM 0x58000000..0x5fffffff -> 0x58000000
Unable to handle kernel NULL pointer dereference at virtual address 00000070
pgd = c0004000
[00000070] *pgd=00000000
Internal error: Oops: 17 [#1] PREEMPT ARM
CPU: 0 PID: 254 Comm: kworker/0:1 Not tainted 4.12.0-rc3+ #888
Hardware name: Gemini (Device Tree)
Workqueue: events deferred_probe_work_func
task: c38c3a40 task.stack: c390a000
PC is at faraday_pci_probe+0x274/0x65c
LR is at __irq_put_desc_unlock+0x18/0x70
pc : [<c0232648>] lr : [<c004be48>] psr: 80000013
sp : c390bdf0 ip : c3424274 fp : c3ac2ee0
r10: c3f77718 r9 : c3ad49b0 r8 : c3907800
r7 : 00000004 r6 : c390be18 r5 : c3907810 r4 : c3ad4810
r3 : 00000000 r2 : 00000000 r1 : 00000001 r0 : c3ac2de0
Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 0000397f Table: 03a9c000 DAC: 00000053
Process kworker/0:1 (pid: 254, stack limit = 0xc390a190)
Stack: (0xc390bdf0 to 0xc390c000)
bde0: c0534a60 c3ad49b0 00000003 c024cb68
be00: 00000001 20000013 00000000 50000000 00000007 000020dd c3ae2680 c3ae2740
be20: c07c861f c0134328 00000000 c3ad5fa0 c3910a50 c3910a50 00000001 00000000
be40: 00000000 c01344ec c3910a50 c3910a50 00000001 00000000 c3910a50 c39d76e0
be60: c3ad5fa0 c05f6f58 c3910a50 c3907810 c3907810 c07cba88 fffffdfb 00000003
be80: 00000000 c07c5040 c386e420 c02bbbe0 c3907810 c081c6c4 00000000 c07cba88
bea0: 00000003 c02ba9dc 00000000 c390bee0 c02bab30 00000001 c07bf660 00000000
bec0: c07c5040 c02b93dc c382829c c39886f4 c3907810 c3907844 c3907810 c02ba674
bee0: c3907810 00000001 c390bf0c c3907810 c07d3a80 c3907810 c07d38e4 c02b961c
bf00: c3907810 c07d38d0 c07d38d0 c02ba210 c07d38ec c386e420 00000000 c3f6b100
bf20: c07bf660 c002dac0 00000008 c07bf660 c07bf660 c386e438 c390a000 c07bf674
bf40: 00000008 c07bf660 c07c5040 c002e044 c390a000 00000001 00000000 00000000
bf60: c3901518 c3901500 ffffe000 c38e2700 00000000 c3901518 c386e420 c002ddb4
bf80: c3839ee4 c00335ec c390a000 c38e2700 c003350c 00000000 00000000 00000000
bfa0: 00000000 00000000 00000000 c000a270 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
[<c0232648>] (faraday_pci_probe) from [<c02bbbe0>]
(platform_drv_probe+0x50/0xb4)
[<c02bbbe0>] (platform_drv_probe) from [<c02ba9dc>]
(driver_probe_device+0x248/0x2dc)
[<c02ba9dc>] (driver_probe_device) from [<c02b93dc>]
(bus_for_each_drv+0x68/0x98)
[<c02b93dc>] (bus_for_each_drv) from [<c02ba674>] (__device_attach+0xa8/0x110)
[<c02ba674>] (__device_attach) from [<c02b961c>] (bus_probe_device+0x88/0x90)
[<c02b961c>] (bus_probe_device) from [<c02ba210>]
(deferred_probe_work_func+0x54/0x80)
[<c02ba210>] (deferred_probe_work_func) from [<c002dac0>]
(process_one_work+0x13c/0x430)
[<c002dac0>] (process_one_work) from [<c002e044>] (worker_thread+0x290/0x5f4)
[<c002e044>] (worker_thread) from [<c00335ec>] (kthread+0xe0/0x120)
[<c00335ec>] (kthread) from [<c000a270>] (ret_from_fork+0x14/0x24)
Code: e59401b0 e3700a01 8a00001d e59421ac (e5d23070)
---[ end trace 1e313b1ea11e101c ]---
I'm trying to figure out what is causing it.
Mind trying this patch please on top of my branch (compile tested, that's
all I can do)

-- >8 --
diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index 9cfc90a..316ee50 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -178,12 +178,11 @@ static int faraday_res_to_memcfg(resource_size_t mem_base,
return 0;
}

-static int faraday_pci_read_config(struct pci_bus *bus, unsigned int fn,
- int config, int size, u32 *value)
+static int faraday_raw_pci_read_config(struct faraday_pci *p, int bus_number,
+ unsigned int fn, int config, int size,
+ u32 *value)
{
- struct faraday_pci *p = bus->sysdata;
-
- writel(PCI_CONF_BUS(bus->number) |
+ writel(PCI_CONF_BUS(bus_number) |
PCI_CONF_DEVICE(PCI_SLOT(fn)) |
PCI_CONF_FUNCTION(PCI_FUNC(fn)) |
PCI_CONF_WHERE(config) |
@@ -197,11 +196,19 @@ static int faraday_pci_read_config(struct pci_bus *bus, unsigned int fn,
else if (size == 2)
*value = (*value >> (8 * (config & 3))) & 0xFFFF;

+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int faraday_pci_read_config(struct pci_bus *bus, unsigned int fn,
+ int config, int size, u32 *value)
+{
+ struct faraday_pci *p = bus->sysdata;
+
dev_dbg(&bus->dev,
"[read] slt: %.2d, fnc: %d, cnf: 0x%.2X, val (%d bytes): 0x%.8X\n",
PCI_SLOT(fn), PCI_FUNC(fn), config, size, *value);

- return PCIBIOS_SUCCESSFUL;
+ return faraday_raw_pci_read_config(p, bus->number, fn, config, size, value);
}

static int faraday_raw_pci_write_config(struct faraday_pci *p, int bus_number,
@@ -257,10 +264,10 @@ static void faraday_pci_ack_irq(struct irq_data *d)
struct faraday_pci *p = irq_data_get_irq_chip_data(d);
unsigned int reg;

- faraday_pci_read_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, &reg);
+ faraday_raw_pci_read_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, &reg);
reg &= ~(0xF << PCI_CTRL2_INTSTS_SHIFT);
reg |= BIT(irqd_to_hwirq(d) + PCI_CTRL2_INTSTS_SHIFT);
- faraday_pci_write_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, reg);
+ faraday_raw_pci_write_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, reg);
}

static void faraday_pci_mask_irq(struct irq_data *d)
@@ -268,10 +275,10 @@ static void faraday_pci_mask_irq(struct irq_data *d)
struct faraday_pci *p = irq_data_get_irq_chip_data(d);
unsigned int reg;

- faraday_pci_read_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, &reg);
+ faraday_raw_pci_read_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, &reg);
reg &= ~((0xF << PCI_CTRL2_INTSTS_SHIFT)
| BIT(irqd_to_hwirq(d) + PCI_CTRL2_INTMASK_SHIFT));
- faraday_pci_write_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, reg);
+ faraday_raw_pci_write_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, reg);
}

static void faraday_pci_unmask_irq(struct irq_data *d)
@@ -279,10 +286,10 @@ static void faraday_pci_unmask_irq(struct irq_data *d)
struct faraday_pci *p = irq_data_get_irq_chip_data(d);
unsigned int reg;

- faraday_pci_read_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, &reg);
+ faraday_raw_pci_read_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, &reg);
reg &= ~(0xF << PCI_CTRL2_INTSTS_SHIFT);
reg |= BIT(irqd_to_hwirq(d) + PCI_CTRL2_INTMASK_SHIFT);
- faraday_pci_write_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, reg);
+ faraday_raw_pci_write_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, reg);
}

static void faraday_pci_irq_handler(struct irq_desc *desc)
@@ -291,7 +298,7 @@ static void faraday_pci_irq_handler(struct irq_desc *desc)
struct irq_chip *irqchip = irq_desc_get_chip(desc);
unsigned int irq_stat, reg, i;

- faraday_pci_read_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, &reg);
+ faraday_raw_pci_read_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, &reg);
irq_stat = reg >> PCI_CTRL2_INTSTS_SHIFT;

chained_irq_enter(irqchip, desc);
Lorenzo Pieralisi
2017-06-21 10:45:38 UTC
Permalink
Post by Lorenzo Pieralisi
Post by Linus Walleij
On Thu, Jun 8, 2017 at 4:13 PM, Lorenzo Pieralisi
[1] git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git pci/pci-fixup-irqs-removal-v2
OF: PCI: IO 0x50000000..0x500fffff -> 0x00000000
OF: PCI: MEM 0x58000000..0x5fffffff -> 0x58000000
Unable to handle kernel NULL pointer dereference at virtual address 00000070
pgd = c0004000
[00000070] *pgd=00000000
Internal error: Oops: 17 [#1] PREEMPT ARM
CPU: 0 PID: 254 Comm: kworker/0:1 Not tainted 4.12.0-rc3+ #888
Hardware name: Gemini (Device Tree)
Workqueue: events deferred_probe_work_func
task: c38c3a40 task.stack: c390a000
PC is at faraday_pci_probe+0x274/0x65c
LR is at __irq_put_desc_unlock+0x18/0x70
pc : [<c0232648>] lr : [<c004be48>] psr: 80000013
sp : c390bdf0 ip : c3424274 fp : c3ac2ee0
r10: c3f77718 r9 : c3ad49b0 r8 : c3907800
r7 : 00000004 r6 : c390be18 r5 : c3907810 r4 : c3ad4810
r3 : 00000000 r2 : 00000000 r1 : 00000001 r0 : c3ac2de0
Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 0000397f Table: 03a9c000 DAC: 00000053
Process kworker/0:1 (pid: 254, stack limit = 0xc390a190)
Stack: (0xc390bdf0 to 0xc390c000)
bde0: c0534a60 c3ad49b0 00000003 c024cb68
be00: 00000001 20000013 00000000 50000000 00000007 000020dd c3ae2680 c3ae2740
be20: c07c861f c0134328 00000000 c3ad5fa0 c3910a50 c3910a50 00000001 00000000
be40: 00000000 c01344ec c3910a50 c3910a50 00000001 00000000 c3910a50 c39d76e0
be60: c3ad5fa0 c05f6f58 c3910a50 c3907810 c3907810 c07cba88 fffffdfb 00000003
be80: 00000000 c07c5040 c386e420 c02bbbe0 c3907810 c081c6c4 00000000 c07cba88
bea0: 00000003 c02ba9dc 00000000 c390bee0 c02bab30 00000001 c07bf660 00000000
bec0: c07c5040 c02b93dc c382829c c39886f4 c3907810 c3907844 c3907810 c02ba674
bee0: c3907810 00000001 c390bf0c c3907810 c07d3a80 c3907810 c07d38e4 c02b961c
bf00: c3907810 c07d38d0 c07d38d0 c02ba210 c07d38ec c386e420 00000000 c3f6b100
bf20: c07bf660 c002dac0 00000008 c07bf660 c07bf660 c386e438 c390a000 c07bf674
bf40: 00000008 c07bf660 c07c5040 c002e044 c390a000 00000001 00000000 00000000
bf60: c3901518 c3901500 ffffe000 c38e2700 00000000 c3901518 c386e420 c002ddb4
bf80: c3839ee4 c00335ec c390a000 c38e2700 c003350c 00000000 00000000 00000000
bfa0: 00000000 00000000 00000000 c000a270 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
[<c0232648>] (faraday_pci_probe) from [<c02bbbe0>]
(platform_drv_probe+0x50/0xb4)
[<c02bbbe0>] (platform_drv_probe) from [<c02ba9dc>]
(driver_probe_device+0x248/0x2dc)
[<c02ba9dc>] (driver_probe_device) from [<c02b93dc>]
(bus_for_each_drv+0x68/0x98)
[<c02b93dc>] (bus_for_each_drv) from [<c02ba674>] (__device_attach+0xa8/0x110)
[<c02ba674>] (__device_attach) from [<c02b961c>] (bus_probe_device+0x88/0x90)
[<c02b961c>] (bus_probe_device) from [<c02ba210>]
(deferred_probe_work_func+0x54/0x80)
[<c02ba210>] (deferred_probe_work_func) from [<c002dac0>]
(process_one_work+0x13c/0x430)
[<c002dac0>] (process_one_work) from [<c002e044>] (worker_thread+0x290/0x5f4)
[<c002e044>] (worker_thread) from [<c00335ec>] (kthread+0xe0/0x120)
[<c00335ec>] (kthread) from [<c000a270>] (ret_from_fork+0x14/0x24)
Code: e59401b0 e3700a01 8a00001d e59421ac (e5d23070)
---[ end trace 1e313b1ea11e101c ]---
I'm trying to figure out what is causing it.
Mind trying this patch please on top of my branch (compile tested, that's
all I can do)
Sorry I miss yet another usage of struct pci_bus before bus is
scanned in faraday_pci_parse_map_dma_ranges(), all these config
accessors want is to access bus number 0 and writei/read some config
values in there, please confirm.

Updated patch here:

-- >8 --
Subject: [PATCH] drivers: pci: host: ftpci100: Remove pre-scan bus dependency

Remove leftover pci_bus dependencies from the probe path.

Signed-off-by: Lorenzo Pieralisi <***@arm.com>
---
drivers/pci/host/pci-ftpci100.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index 9cfc90a..e938eeb 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -178,12 +178,11 @@ static int faraday_res_to_memcfg(resource_size_t mem_base,
return 0;
}

-static int faraday_pci_read_config(struct pci_bus *bus, unsigned int fn,
- int config, int size, u32 *value)
+static int faraday_raw_pci_read_config(struct faraday_pci *p, int bus_number,
+ unsigned int fn, int config, int size,
+ u32 *value)
{
- struct faraday_pci *p = bus->sysdata;
-
- writel(PCI_CONF_BUS(bus->number) |
+ writel(PCI_CONF_BUS(bus_number) |
PCI_CONF_DEVICE(PCI_SLOT(fn)) |
PCI_CONF_FUNCTION(PCI_FUNC(fn)) |
PCI_CONF_WHERE(config) |
@@ -197,11 +196,19 @@ static int faraday_pci_read_config(struct pci_bus *bus, unsigned int fn,
else if (size == 2)
*value = (*value >> (8 * (config & 3))) & 0xFFFF;

+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int faraday_pci_read_config(struct pci_bus *bus, unsigned int fn,
+ int config, int size, u32 *value)
+{
+ struct faraday_pci *p = bus->sysdata;
+
dev_dbg(&bus->dev,
"[read] slt: %.2d, fnc: %d, cnf: 0x%.2X, val (%d bytes): 0x%.8X\n",
PCI_SLOT(fn), PCI_FUNC(fn), config, size, *value);

- return PCIBIOS_SUCCESSFUL;
+ return faraday_raw_pci_read_config(p, bus->number, fn, config, size, value);
}

static int faraday_raw_pci_write_config(struct faraday_pci *p, int bus_number,
@@ -257,10 +264,10 @@ static void faraday_pci_ack_irq(struct irq_data *d)
struct faraday_pci *p = irq_data_get_irq_chip_data(d);
unsigned int reg;

- faraday_pci_read_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, &reg);
+ faraday_raw_pci_read_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, &reg);
reg &= ~(0xF << PCI_CTRL2_INTSTS_SHIFT);
reg |= BIT(irqd_to_hwirq(d) + PCI_CTRL2_INTSTS_SHIFT);
- faraday_pci_write_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, reg);
+ faraday_raw_pci_write_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, reg);
}

static void faraday_pci_mask_irq(struct irq_data *d)
@@ -268,10 +275,10 @@ static void faraday_pci_mask_irq(struct irq_data *d)
struct faraday_pci *p = irq_data_get_irq_chip_data(d);
unsigned int reg;

- faraday_pci_read_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, &reg);
+ faraday_raw_pci_read_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, &reg);
reg &= ~((0xF << PCI_CTRL2_INTSTS_SHIFT)
| BIT(irqd_to_hwirq(d) + PCI_CTRL2_INTMASK_SHIFT));
- faraday_pci_write_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, reg);
+ faraday_raw_pci_write_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, reg);
}

static void faraday_pci_unmask_irq(struct irq_data *d)
@@ -279,10 +286,10 @@ static void faraday_pci_unmask_irq(struct irq_data *d)
struct faraday_pci *p = irq_data_get_irq_chip_data(d);
unsigned int reg;

- faraday_pci_read_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, &reg);
+ faraday_raw_pci_read_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, &reg);
reg &= ~(0xF << PCI_CTRL2_INTSTS_SHIFT);
reg |= BIT(irqd_to_hwirq(d) + PCI_CTRL2_INTMASK_SHIFT);
- faraday_pci_write_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, reg);
+ faraday_raw_pci_write_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, reg);
}

static void faraday_pci_irq_handler(struct irq_desc *desc)
@@ -291,7 +298,7 @@ static void faraday_pci_irq_handler(struct irq_desc *desc)
struct irq_chip *irqchip = irq_desc_get_chip(desc);
unsigned int irq_stat, reg, i;

- faraday_pci_read_config(p->bus, 0, FARADAY_PCI_CTRL2, 4, &reg);
+ faraday_raw_pci_read_config(p, 0, 0, FARADAY_PCI_CTRL2, 4, &reg);
irq_stat = reg >> PCI_CTRL2_INTSTS_SHIFT;

chained_irq_enter(irqchip, desc);
@@ -412,8 +419,8 @@ static int faraday_pci_parse_map_dma_ranges(struct faraday_pci *p,
dev_info(dev, "DMA MEM%d BASE: 0x%016llx -> 0x%016llx config %08x\n",
i + 1, range.pci_addr, end, val);
if (i <= 2) {
- faraday_pci_write_config(p->bus, 0, confreg[i],
- 4, val);
+ faraday_raw_pci_write_config(p, 0, 0, confreg[i],
+ 4, val);
} else {
dev_err(dev, "ignore extraneous dma-range %d\n", i);
break;
--
2.10.0
Linus Walleij
2017-06-21 14:51:40 UTC
Permalink
On Wed, Jun 21, 2017 at 12:45 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Post by Linus Walleij
I'm trying to figure out what is causing it.
Mind trying this patch please on top of my branch (compile tested, that's
all I can do)
Sorry I miss yet another usage of struct pci_bus before bus is
scanned in faraday_pci_parse_map_dma_ranges(), all these config
accessors want is to access bus number 0 and writei/read some config
values in there, please confirm.
-- >8 --
Subject: [PATCH] drivers: pci: host: ftpci100: Remove pre-scan bus dependency
Remove leftover pci_bus dependencies from the probe path.
Hm I tried this but sort of the same problem:

OF: PCI: host bridge /soc/***@50000000 ranges:
OF: PCI: IO 0x50000000..0x500fffff -> 0x00000000
OF: PCI: MEM 0x58000000..0x5fffffff -> 0x58000000
Unable to handle kernel NULL pointer dereference at virtual address 00000070
pgd = c0004000
[00000070] *pgd=00000000
Internal error: Oops: 17 [#1] PREEMPT ARM
CPU: 0 PID: 254 Comm: kworker/0:1 Not tainted 4.12.0-rc3+ #894
Hardware name: Gemini (Device Tree)
Workqueue: events deferred_probe_work_func
task: c387f2c0 task.stack: c387c000
PC is at faraday_pci_probe+0x274/0x650
LR is at __irq_put_desc_unlock+0x18/0x70
pc : [<c02324e8>] lr : [<c004be48>] psr: 80000013
sp : c387ddf0 ip : c3424274 fp : c3ac0ee0
r10: c3f77718 r9 : c3ae09b0 r8 : c3906800
r7 : 00000004 r6 : c387de18 r5 : c3906810 r4 : c3ae0810
r3 : 00000000 r2 : 00000000 r1 : 00000001 r0 : c3ac0de0
Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 0000397f Table: 03ab8000 DAC: 00000053
Process kworker/0:1 (pid: 254, stack limit = 0xc387c190)
Stack: (0xc387ddf0 to 0xc387e000)
dde0: c0534a60 c3ae09b0 00000003 c024ca90
de00: 00000001 20000013 00000000 50000000 00000007 000020dd c3ae25c0 c3ae2700
de20: c07c861f c0134328 00000000 c3ae1fa0 c3913a50 c3913a50 00000001 00000000
de40: 00000000 c01344ec c3913a50 c3913a50 00000001 00000000 c3913a50 c39c16e0
de60: c3ae1fa0 c05f6f48 c3913a50 c3906810 c3906810 c07cba88 fffffdfb 00000003
de80: 00000000 c07c5040 c3916420 c02bbb08 c3906810 c081c6c4 00000000 c07cba88
dea0: 00000003 c02ba904 00000000 c387dee0 c02baa58 00000001 c07bf660 00000000
dec0: c07c5040 c02b9304 c382829c c39886f4 c3906810 c3906844 c3906810 c02ba59c
dee0: c3906810 00000001 c387df0c c3906810 c07d3a80 c3906810 c07d38e4 c02b9544
df00: c3906810 c07d38d0 c07d38d0 c02ba138 c07d38ec c3916420 00000000 c3f6b100
df20: c07bf660 c002dac0 00000008 c07bf660 c07bf660 c3916438 c387c000 c07bf674
df40: 00000008 c07bf660 c07c5040 c002e044 c387c000 00000001 00000000 00000000
df60: c39397d8 c39397c0 ffffe000 c38c4740 00000000 c39397d8 c3916420 c002ddb4
df80: c3839ee4 c00335ec c387c000 c38c4740 c003350c 00000000 00000000 00000000
dfa0: 00000000 00000000 00000000 c000a270 00000000 00000000 00000000 00000000
dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00030400 00080000
[<c02324e8>] (faraday_pci_probe) from [<c02bbb08>]
(platform_drv_probe+0x50/0xb4)
[<c02bbb08>] (platform_drv_probe) from [<c02ba904>]
(driver_probe_device+0x248/0x2dc)
[<c02ba904>] (driver_probe_device) from [<c02b9304>]
(bus_for_each_drv+0x68/0x98)
[<c02b9304>] (bus_for_each_drv) from [<c02ba59c>] (__device_attach+0xa8/0x110)
[<c02ba59c>] (__device_attach) from [<c02b9544>] (bus_probe_device+0x88/0x90)
[<c02b9544>] (bus_probe_device) from [<c02ba138>]
(deferred_probe_work_func+0x54/0x80)
[<c02ba138>] (deferred_probe_work_func) from [<c002dac0>]
(process_one_work+0x13c/0x430)
[<c002dac0>] (process_one_work) from [<c002e044>] (worker_thread+0x290/0x5f4)
[<c002e044>] (worker_thread) from [<c00335ec>] (kthread+0xe0/0x120)
[<c00335ec>] (kthread) from [<c000a270>] (ret_from_fork+0x14/0x24)
Code: e59401b0 e3700a01 8a00001d e59421ac (e5d23070)
---[ end trace 538dce2df8d99ba7 ]---

I will try to figure it out, just a bit short on time right now...

Yours,
Linus Walleij
Linus Walleij
2017-06-21 15:14:13 UTC
Permalink
On Wed, Jun 21, 2017 at 12:45 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Sorry I miss yet another usage of struct pci_bus before bus is
scanned in faraday_pci_parse_map_dma_ranges(), all these config
accessors want is to access bus number 0 and writei/read some config
values in there, please confirm.
I fixed it. This was needed:
diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index 86f8a3d1c1da..5162dffc102b 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -450,6 +450,8 @@ static int faraday_pci_probe(struct platform_device *pdev)
struct resource *io;
struct pci_host_bridge *host;
struct clk *clk;
+ unsigned char max_bus_speed = PCI_SPEED_33MHz;
+ unsigned char cur_bus_speed = PCI_SPEED_33MHz;
int ret;
u32 val;
LIST_HEAD(res);
@@ -553,27 +555,27 @@ static int faraday_pci_probe(struct platform_device *pdev)
unsigned long rate;
u32 val;

- faraday_pci_read_config(p->bus, 0,
- FARADAY_PCI_STATUS_CMD, 4, &val);
+ faraday_raw_pci_read_config(p, 0, 0,
+ FARADAY_PCI_STATUS_CMD, 4, &val);
rate = clk_get_rate(p->bus_clk);

if ((rate == 33000000) && (val & PCI_STATUS_66MHZ_CAPABLE)) {
dev_info(dev, "33MHz bus is 66MHz capable\n");
- p->bus->max_bus_speed = PCI_SPEED_66MHz;
+ max_bus_speed = PCI_SPEED_66MHz;
ret = clk_set_rate(p->bus_clk, 66000000);
if (ret)
dev_err(dev, "failed to set bus clock\n");
} else {
dev_info(dev, "33MHz only bus\n");
- p->bus->max_bus_speed = PCI_SPEED_33MHz;
+ max_bus_speed = PCI_SPEED_33MHz;
}

/* Bumping the clock may fail so read back the rate */
rate = clk_get_rate(p->bus_clk);
if (rate == 33000000)
- p->bus->cur_bus_speed = PCI_SPEED_33MHz;
+ cur_bus_speed = PCI_SPEED_33MHz;
if (rate == 66000000)
- p->bus->cur_bus_speed = PCI_SPEED_66MHz;
+ cur_bus_speed = PCI_SPEED_66MHz;
}

ret = faraday_pci_parse_map_dma_ranges(p, dev->of_node);
@@ -587,6 +589,8 @@ static int faraday_pci_probe(struct platform_device *pdev)
return ret;
}
p->bus = host->bus;
+ p->bus->max_bus_speed = max_bus_speed;
+ p->bus->cur_bus_speed = cur_bus_speed;

pci_bus_assign_resources(p->bus);
pci_bus_add_devices(p->bus);


I'll send a modified and tested version of your patch.

Yours,
Linus Walleij
Lorenzo Pieralisi
2017-06-21 15:41:36 UTC
Permalink
Hi Linus,
Post by Linus Walleij
On Wed, Jun 21, 2017 at 12:45 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Sorry I miss yet another usage of struct pci_bus before bus is
scanned in faraday_pci_parse_map_dma_ranges(), all these config
accessors want is to access bus number 0 and writei/read some config
values in there, please confirm.
diff --git a/drivers/pci/host/pci-ftpci100.c b/drivers/pci/host/pci-ftpci100.c
index 86f8a3d1c1da..5162dffc102b 100644
--- a/drivers/pci/host/pci-ftpci100.c
+++ b/drivers/pci/host/pci-ftpci100.c
@@ -450,6 +450,8 @@ static int faraday_pci_probe(struct platform_device *pdev)
struct resource *io;
struct pci_host_bridge *host;
struct clk *clk;
+ unsigned char max_bus_speed = PCI_SPEED_33MHz;
+ unsigned char cur_bus_speed = PCI_SPEED_33MHz;
I could not fix it myself since we do not have the same code base,
I can't find any clock handling in the mainline as per v4.12-rc6.

Is the ftpci100.c clock handling being queued for this merge window ?

This patch:

https://patchwork.ozlabs.org/patch/778716/

fixes a commit that is not in v4.12-rc6, I need some input to understand
how to proceed.

Thanks,
Lorenzo
Post by Linus Walleij
int ret;
u32 val;
LIST_HEAD(res);
@@ -553,27 +555,27 @@ static int faraday_pci_probe(struct platform_device *pdev)
unsigned long rate;
u32 val;
- faraday_pci_read_config(p->bus, 0,
- FARADAY_PCI_STATUS_CMD, 4, &val);
+ faraday_raw_pci_read_config(p, 0, 0,
+ FARADAY_PCI_STATUS_CMD, 4, &val);
rate = clk_get_rate(p->bus_clk);
if ((rate == 33000000) && (val & PCI_STATUS_66MHZ_CAPABLE)) {
dev_info(dev, "33MHz bus is 66MHz capable\n");
- p->bus->max_bus_speed = PCI_SPEED_66MHz;
+ max_bus_speed = PCI_SPEED_66MHz;
ret = clk_set_rate(p->bus_clk, 66000000);
if (ret)
dev_err(dev, "failed to set bus clock\n");
} else {
dev_info(dev, "33MHz only bus\n");
- p->bus->max_bus_speed = PCI_SPEED_33MHz;
+ max_bus_speed = PCI_SPEED_33MHz;
}
/* Bumping the clock may fail so read back the rate */
rate = clk_get_rate(p->bus_clk);
if (rate == 33000000)
- p->bus->cur_bus_speed = PCI_SPEED_33MHz;
+ cur_bus_speed = PCI_SPEED_33MHz;
if (rate == 66000000)
- p->bus->cur_bus_speed = PCI_SPEED_66MHz;
+ cur_bus_speed = PCI_SPEED_66MHz;
}
ret = faraday_pci_parse_map_dma_ranges(p, dev->of_node);
@@ -587,6 +589,8 @@ static int faraday_pci_probe(struct platform_device *pdev)
return ret;
}
p->bus = host->bus;
+ p->bus->max_bus_speed = max_bus_speed;
+ p->bus->cur_bus_speed = cur_bus_speed;
pci_bus_assign_resources(p->bus);
pci_bus_add_devices(p->bus);
I'll send a modified and tested version of your patch.
Yours,
Linus Walleij
Linus Walleij
2017-06-21 16:28:39 UTC
Permalink
On Wed, Jun 21, 2017 at 5:41 PM, Lorenzo Pieralisi
Post by Lorenzo Pieralisi
Is the ftpci100.c clock handling being queued for this merge window ?
Yups, it's in Bjorn's tree and visible in linux-next.
Post by Lorenzo Pieralisi
https://patchwork.ozlabs.org/patch/778716/
fixes a commit that is not in v4.12-rc6, I need some input to understand
how to proceed.
Yeah that is just a fix I had to do when fixing this other issue.

It's cool, I just sent a patch. If Bjorn applied your series he can just
apply this on top and all will be fine (for me).

Yours,
Linus Walleij
Loading...