[PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA

[PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA

[Semih Baskan]

The PCIe outbound window base on Northstar depends on the PCIe Gen2 core
revision. Revision 0x01 uses 0x08000000, 0x40000000 and 0x48000000 for
controllers 0 to 2, while revision 0x07 (NS-B0) uses 0x08000000,
0x20000000 and 0x28000000. Broadcom's own driver branches on the core
revision for exactly this reason.

bcm-ns.dtsi is shared by every Northstar SoC, so it cannot carry a value
that is correct on both. Commit 767012397976 ("ARM: dts: BCM5301X:
Describe PCIe controllers fully") gave the controllers a ranges property.
The commit shipped in v7.1.

With that property present, two things go wrong with this driver:

 - devm_pci_alloc_host_bridge() parses those ranges and requests them,
   then this driver adds its own window and requests the whole list a
   second time, so every controller fails to probe with -EBUSY.

 - The DT window itself is only correct on core revision 0x07. On
   revision 0x01 it points at an address the hardware does not decode,
   and the first MMIO access to a BAR takes an imprecise external
   abort.

The enumeration ROM reports the correct base for the revision actually
present, and bcma already provides it as addr_s[0]. Drop any memory
window that came from the device tree and use that instead, requesting
only the window this driver owns. This makes the driver correct whether
or not the DT describes a window.

When a dropped window does not match what the EROM reports, print a
warning naming both. The mismatch means the devicetree describes a
window the hardware does not decode, and that should be fixed in the
dts rather than ignored silently.

The same commit also added compatible = "brcm,iproc-pcie", so these
nodes now match pcie-iproc-platform. With CONFIG_PCIE_IPROC_PLATFORM
enabled, which is the default on ARCH_BCM_IPROC, that driver binds them
first and this driver's probe fails inside devm_pci_alloc_host_bridge().
This patch fixes the configurations where the BCMA driver is the one in
use; OpenWrt builds that way, with PCIE_IPROC_PLATFORM disabled. The
platform path takes the DT window as-is and has the same wrong address
on core revision 0x01, so that side needs a devicetree fix either way.

Tested on an ASUS RT-N18U (BCM47081) and a Linksys EA9200 (BCM4709),
both core revision 0x01.

Fixes: 767012397976 ("ARM: dts: BCM5301X: Describe PCIe controllers fully")
Tested-by: Rani Hod <rani.hod@gmail.com>
Cc: stable@vger.kernel.org # v7.1+
Signed-off-by: Semih Baskan <strst.gs@gmail.com>
---
v2 -> v3: format the two problem descriptions as bullet points.
Requested by Bjorn Helgaas.

v1 -> v2: print a warning for every devicetree memory window that does
not match the EROM window. Requested by Arnd Bergmann:
https://lore.kernel.org/all/d05ffeca-f289-42dd-b454-5a7c7741c6d5@app.fastmail.com/

v2: https://lore.kernel.org/all/20260807035726.387-1-strst.gs@gmail.com/
v1: https://lore.kernel.org/all/20260727140939.389-1-strst.gs@gmail.com/

 drivers/pci/controller/pcie-iproc-bcma.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-iproc-bcma.c b/drivers/pci/controller/pcie-iproc-bcma.c
index 593418c2b..06a471f4a 100644
--- a/drivers/pci/controller/pcie-iproc-bcma.c
+++ b/drivers/pci/controller/pcie-iproc-bcma.c
@@ -36,6 +36,7 @@ static int iproc_bcma_pcie_probe(struct bcma_device *bdev)
 	struct device *dev = &bdev->dev;
 	struct iproc_pcie *pcie;
 	struct pci_host_bridge *bridge;
+	struct resource_entry *win, *tmp;
 	int ret;
 
 	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
@@ -59,8 +60,22 @@ static int iproc_bcma_pcie_probe(struct bcma_device *bdev)
 	pcie->mem.end = bdev->addr_s[0] + SZ_128M - 1;
 	pcie->mem.name = "PCIe MEM space";
 	pcie->mem.flags = IORESOURCE_MEM;
+
+	resource_list_for_each_entry_safe(win, tmp, &bridge->windows) {
+		if (resource_type(win->res) != IORESOURCE_MEM)
+			continue;
+
+		if (win->res->start != pcie->mem.start ||
+		    win->res->end != pcie->mem.end)
+			dev_warn(dev, "DT window %pR does not match EROM window %pR, using EROM\n",
+				 win->res, &pcie->mem);
+
+		devm_release_resource(dev, win->res);
+		resource_list_destroy_entry(win);
+	}
+
 	pci_add_resource(&bridge->windows, &pcie->mem);
-	ret = devm_request_pci_bus_resources(dev, &bridge->windows);
+	ret = devm_request_resource(dev, &iomem_resource, &pcie->mem);
 	if (ret)
 		return ret;
 
-- 
2.43.0

Re: [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA

[Manivannan Sadhasivam]

On Fri, 07 Aug 2026 19:05:26 +0300, Semih Baskan wrote:
> The PCIe outbound window base on Northstar depends on the PCIe Gen2 core
> revision. Revision 0x01 uses 0x08000000, 0x40000000 and 0x48000000 for
> controllers 0 to 2, while revision 0x07 (NS-B0) uses 0x08000000,
> 0x20000000 and 0x28000000. Broadcom's own driver branches on the core
> revision for exactly this reason.
> 
> bcm-ns.dtsi is shared by every Northstar SoC, so it cannot carry a value
> that is correct on both. Commit 767012397976 ("ARM: dts: BCM5301X:
> Describe PCIe controllers fully") gave the controllers a ranges property.
> The commit shipped in v7.1.
> 
> [...]

Applied, thanks!

[1/1] PCI: iproc: Use the EROM outbound window on BCMA
      commit: 552aa843e4c5aa92012d8c55df8471e6c5ba621c

Best regards,
-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA

[Bjorn Helgaas]

[+to Rafał; probably same as rafal@milecki.pl, but MAINTAINERS lists
zajec5@gmail.com for BCMA]

On Fri, Aug 07, 2026 at 07:05:26PM +0300, Semih Baskan wrote:
> The PCIe outbound window base on Northstar depends on the PCIe Gen2 core
> revision. Revision 0x01 uses 0x08000000, 0x40000000 and 0x48000000 for
> controllers 0 to 2, while revision 0x07 (NS-B0) uses 0x08000000,
> 0x20000000 and 0x28000000. Broadcom's own driver branches on the core
> revision for exactly this reason.
> 
> bcm-ns.dtsi is shared by every Northstar SoC, so it cannot carry a value
> that is correct on both. Commit 767012397976 ("ARM: dts: BCM5301X:
> Describe PCIe controllers fully") gave the controllers a ranges property.
> The commit shipped in v7.1.
> 
> With that property present, two things go wrong with this driver:
> 
>  - devm_pci_alloc_host_bridge() parses those ranges and requests them,
>    then this driver adds its own window and requests the whole list a
>    second time, so every controller fails to probe with -EBUSY.
> 
>  - The DT window itself is only correct on core revision 0x07. On
>    revision 0x01 it points at an address the hardware does not decode,
>    and the first MMIO access to a BAR takes an imprecise external
>    abort.
> 
> The enumeration ROM reports the correct base for the revision actually
> present, and bcma already provides it as addr_s[0]. Drop any memory
> window that came from the device tree and use that instead, requesting
> only the window this driver owns. This makes the driver correct whether
> or not the DT describes a window.
> 
> When a dropped window does not match what the EROM reports, print a
> warning naming both. The mismatch means the devicetree describes a
> window the hardware does not decode, and that should be fixed in the
> dts rather than ignored silently.
> 
> The same commit also added compatible = "brcm,iproc-pcie", so these
> nodes now match pcie-iproc-platform. With CONFIG_PCIE_IPROC_PLATFORM
> enabled, which is the default on ARCH_BCM_IPROC, that driver binds them
> first and this driver's probe fails inside devm_pci_alloc_host_bridge().
> This patch fixes the configurations where the BCMA driver is the one in
> use; OpenWrt builds that way, with PCIE_IPROC_PLATFORM disabled. The
> platform path takes the DT window as-is and has the same wrong address
> on core revision 0x01, so that side needs a devicetree fix either way.
> 
> Tested on an ASUS RT-N18U (BCM47081) and a Linksys EA9200 (BCM4709),
> both core revision 0x01.
> 
> Fixes: 767012397976 ("ARM: dts: BCM5301X: Describe PCIe controllers fully")
> Tested-by: Rani Hod <rani.hod@gmail.com>
> Cc: stable@vger.kernel.org # v7.1+
> Signed-off-by: Semih Baskan <strst.gs@gmail.com>
> ---
> v2 -> v3: format the two problem descriptions as bullet points.
> Requested by Bjorn Helgaas.
> 
> v1 -> v2: print a warning for every devicetree memory window that does
> not match the EROM window. Requested by Arnd Bergmann:
> https://lore.kernel.org/all/d05ffeca-f289-42dd-b454-5a7c7741c6d5@app.fastmail.com/
> 
> v2: https://lore.kernel.org/all/20260807035726.387-1-strst.gs@gmail.com/
> v1: https://lore.kernel.org/all/20260727140939.389-1-strst.gs@gmail.com/
> 
>  drivers/pci/controller/pcie-iproc-bcma.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-iproc-bcma.c b/drivers/pci/controller/pcie-iproc-bcma.c
> index 593418c2b..06a471f4a 100644
> --- a/drivers/pci/controller/pcie-iproc-bcma.c
> +++ b/drivers/pci/controller/pcie-iproc-bcma.c
> @@ -36,6 +36,7 @@ static int iproc_bcma_pcie_probe(struct bcma_device *bdev)
>  	struct device *dev = &bdev->dev;
>  	struct iproc_pcie *pcie;
>  	struct pci_host_bridge *bridge;
> +	struct resource_entry *win, *tmp;
>  	int ret;
>  
>  	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
> @@ -59,8 +60,22 @@ static int iproc_bcma_pcie_probe(struct bcma_device *bdev)
>  	pcie->mem.end = bdev->addr_s[0] + SZ_128M - 1;
>  	pcie->mem.name = "PCIe MEM space";
>  	pcie->mem.flags = IORESOURCE_MEM;
> +
> +	resource_list_for_each_entry_safe(win, tmp, &bridge->windows) {
> +		if (resource_type(win->res) != IORESOURCE_MEM)
> +			continue;
> +
> +		if (win->res->start != pcie->mem.start ||
> +		    win->res->end != pcie->mem.end)
> +			dev_warn(dev, "DT window %pR does not match EROM window %pR, using EROM\n",
> +				 win->res, &pcie->mem);
> +
> +		devm_release_resource(dev, win->res);
> +		resource_list_destroy_entry(win);
> +	}
> +
>  	pci_add_resource(&bridge->windows, &pcie->mem);
> -	ret = devm_request_pci_bus_resources(dev, &bridge->windows);
> +	ret = devm_request_resource(dev, &iomem_resource, &pcie->mem);

If the EROM supplies the information the driver needs, why does this
BCMA driver use DT at all?

AFAICS, pcie-iproc-bcma.c doesn't use any of the information from DT.
devm_pci_alloc_host_bridge() *looks* in DT, parses it, and fills in
bridge->windows, and requests those windows, but I don't think
iproc-bcma ever uses them.

iproc_bcma_pcie_probe() passes &bridge->windows to iproc_pcie_setup(),
where I think it's ignored because pcie->need_ob_cfg is never set for
iproc_bcma.

It looks like iproc_bcma_pcie_probe() should just call
pci_alloc_host_bridge() directly and skip devm_pci_alloc_host_bridge()
and the DT things it does.

>  	if (ret)
>  		return ret;
>  
> -- 
> 2.43.0
> 

Re: [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA

[Semih Baskan]

On Fri, Sep 11, 2026 at 02:14:29PM -0500, Bjorn Helgaas wrote:
> If the EROM supplies the information the driver needs, why does this
> BCMA driver use DT at all?
>
> AFAICS, pcie-iproc-bcma.c doesn't use any of the information from DT.

It doesn't. need_ob_cfg is set only in pcie-iproc-platform.c, so the
windows devm_pci_alloc_host_bridge() parses never reach
iproc_pcie_map_ranges() here; the applied patch only had to get them
out of the way of the EROM window.

The follow-up keeps bridge->dev.parent set by hand, because the wifi
nodes under pcie_bridge0 in bcm4709-netgear-r8000.dts are resolved
through the root bus, and frees the bridge itself on the error paths
and in remove(). The one DT dependency left is the IRQ, and that goes
through bcma_core_irq(), not through this driver.

The follow-up is in reply to this mail: pci_alloc_host_bridge(), nothing
from DT requested or handed to the PCI core, request_resource() and
release_resource() instead of devm since pcie->mem sits inside the
bridge allocation. The ranges property is still read, but only to
compare it with the EROM and warn. That is the warning Arnd asked for
before v2: the same property feeds the platform driver on the same
nodes, and on core revision 0x01 it is wrong, which only a BCMA boot
can notice. If you would rather the driver did not open the DT at all,
that check is one separate function and comes out cleanly.
Both shapes are tested on the RT-N18U (core revision 0x01): the warning
lines match the applied version, /proc/iomem and the enumerated devices
are unchanged.

One thing changes with it. With PCIE_IPROC_PLATFORM and PCIE_IPROC_BCMA
both enabled, a core revision 0x01 board now gets a second probe from
this driver: the DT window sits elsewhere there, so the collision that
stopped it before is gone. Revision 0x07 still collides. That
configuration does not work on revision 0x01 either way, since the DT
window there is one the hardware does not decode, and Rafał's DT commit
was tested with the platform driver, so I take that as the intended
path on mainline. It is in the commit log.

If it is better squashed into 552aa843e4c5 while that is still on the
topic branch, I can send it that way instead.

Best regards,
Semih

[PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA

[Semih Baskan]

The BCMA driver takes its register base and, since commit 552aa843e4c5
("PCI: iproc: Use the EROM outbound window on BCMA"), its outbound
window from what bcma read out of the enumeration ROM. It still
allocates its host bridge with devm_pci_alloc_host_bridge(), which since
commit 669cbc708122 ("PCI: Move DT resource setup into
devm_pci_alloc_host_bridge()") parses ranges, dma-ranges and bus-range
from the device's OF node and requests the windows it finds. None of
that reaches the hardware here: need_ob_cfg is only ever set by the
platform driver, so iproc_pcie_setup() never maps the parsed windows,
and the EROM commit above had to throw them away again to keep them
from colliding with its own window.

Allocate the bridge with pci_alloc_host_bridge() instead, so nothing
from the devicetree is requested or handed to the PCI core, and free it
on the error paths and in remove(). The driver now sets
bridge->dev.parent itself, as devm_pci_alloc_host_bridge() did, since
the wifi nodes under pcie_bridge0 in bcm4709-netgear-r8000.dts are
resolved through the root bus. The window request
moves from devm to request_resource() and release_resource() because
the resource lives inside the bridge allocation and has to be released
before the bridge is freed.

The ranges property is still read, but only to compare. bcm-ns.dtsi
describes the same window for the platform driver, and on core revision
0x01 it points at an address the hardware does not decode. The warning
from the EROM commit stays for that reason: a wrong dts is visible on
BCMA boots, where nothing else would show it. Without bus-range the
root bus also logs "No busn resource found for root bus, will use
[bus 00-ff]" again, which changes nothing else.

With PCIE_IPROC_PLATFORM and PCIE_IPROC_BCMA both enabled, the platform
driver binds the same nodes first and claims the devicetree window. On
core revision 0x07 that is the EROM window, so this driver's request
still fails and the probe backs out as before. On revision 0x01 the
devicetree window is elsewhere, so this driver now probes as well, next
to a platform driver instance whose window the hardware does not decode.
That instance does not work either; the only difference is that the
second probe is no longer stopped by the collision.

Tested on an ASUS RT-N18U (BCM47081, core revision 0x01): the warning
lines are identical to the applied version, /proc/iomem and the
enumerated devices are unchanged.

Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Link: https://lore.kernel.org/r/20260911191429.GA549927@bhelgaas/
Signed-off-by: Semih Baskan <strst.gs@gmail.com>
---
 drivers/pci/controller/pcie-iproc-bcma.c | 70 +++++++++++++++++-------
 1 file changed, 49 insertions(+), 21 deletions(-)

diff --git a/drivers/pci/controller/pcie-iproc-bcma.c b/drivers/pci/controller/pcie-iproc-bcma.c
index 06a471f4a..fcae83ed5 100644
--- a/drivers/pci/controller/pcie-iproc-bcma.c
+++ b/drivers/pci/controller/pcie-iproc-bcma.c
@@ -11,6 +11,7 @@
 #include <linux/phy/phy.h>
 #include <linux/bcma/bcma.h>
 #include <linux/ioport.h>
+#include <linux/of_address.h>
 
 #include "pcie-iproc.h"
 
@@ -31,18 +32,42 @@ static int iproc_bcma_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
 	return bcma_core_irq(bdev, 5);
 }
 
+static void iproc_bcma_pcie_check_dt_window(struct iproc_pcie *pcie)
+{
+	struct device_node *np = pcie->dev->of_node;
+	struct of_pci_range_parser parser;
+	struct of_pci_range range;
+	struct resource res;
+
+	if (!np || of_pci_range_parser_init(&parser, np))
+		return;
+
+	for_each_of_pci_range(&parser, &range) {
+		if ((range.flags & IORESOURCE_TYPE_BITS) != IORESOURCE_MEM)
+			continue;
+
+		if (of_pci_range_to_resource(&range, np, &res))
+			continue;
+
+		if (res.start != pcie->mem.start || res.end != pcie->mem.end)
+			dev_warn(pcie->dev, "DT window %pR does not match EROM window %pR, using EROM\n",
+				 &res, &pcie->mem);
+	}
+}
+
 static int iproc_bcma_pcie_probe(struct bcma_device *bdev)
 {
 	struct device *dev = &bdev->dev;
 	struct iproc_pcie *pcie;
 	struct pci_host_bridge *bridge;
-	struct resource_entry *win, *tmp;
 	int ret;
 
-	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*pcie));
+	bridge = pci_alloc_host_bridge(sizeof(*pcie));
 	if (!bridge)
 		return -ENOMEM;
 
+	bridge->dev.parent = dev;
+
 	pcie = pci_host_bridge_priv(bridge);
 
 	pcie->dev = dev;
@@ -51,7 +76,8 @@ static int iproc_bcma_pcie_probe(struct bcma_device *bdev)
 	pcie->base = bdev->io_addr;
 	if (!pcie->base) {
 		dev_err(dev, "no controller registers\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_free_bridge;
 	}
 
 	pcie->base_addr = bdev->addr;
@@ -60,37 +86,39 @@ static int iproc_bcma_pcie_probe(struct bcma_device *bdev)
 	pcie->mem.end = bdev->addr_s[0] + SZ_128M - 1;
 	pcie->mem.name = "PCIe MEM space";
 	pcie->mem.flags = IORESOURCE_MEM;
-
-	resource_list_for_each_entry_safe(win, tmp, &bridge->windows) {
-		if (resource_type(win->res) != IORESOURCE_MEM)
-			continue;
-
-		if (win->res->start != pcie->mem.start ||
-		    win->res->end != pcie->mem.end)
-			dev_warn(dev, "DT window %pR does not match EROM window %pR, using EROM\n",
-				 win->res, &pcie->mem);
-
-		devm_release_resource(dev, win->res);
-		resource_list_destroy_entry(win);
-	}
-
+	iproc_bcma_pcie_check_dt_window(pcie);
 	pci_add_resource(&bridge->windows, &pcie->mem);
-	ret = devm_request_resource(dev, &iomem_resource, &pcie->mem);
-	if (ret)
-		return ret;
+	ret = request_resource(&iomem_resource, &pcie->mem);
+	if (ret) {
+		dev_err(dev, "can't claim %pR\n", &pcie->mem);
+		goto err_free_bridge;
+	}
 
 	bridge->map_irq = iproc_bcma_pcie_map_irq;
 
 	bcma_set_drvdata(bdev, pcie);
 
-	return iproc_pcie_setup(pcie, &bridge->windows);
+	ret = iproc_pcie_setup(pcie, &bridge->windows);
+	if (ret)
+		goto err_release_mem;
+
+	return 0;
+
+err_release_mem:
+	release_resource(&pcie->mem);
+err_free_bridge:
+	pci_free_host_bridge(bridge);
+	return ret;
 }
 
 static void iproc_bcma_pcie_remove(struct bcma_device *bdev)
 {
 	struct iproc_pcie *pcie = bcma_get_drvdata(bdev);
+	struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
 
 	iproc_pcie_remove(pcie);
+	release_resource(&pcie->mem);
+	pci_free_host_bridge(bridge);
 }
 
 static const struct bcma_device_id iproc_bcma_pcie_table[] = {
-- 
2.43.0