* [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA
@ 2026-08-07 16:05 Semih Baskan
2026-09-10 11:14 ` Manivannan Sadhasivam
2026-09-11 19:14 ` Bjorn Helgaas
0 siblings, 2 replies; 7+ messages in thread
From: Semih Baskan @ 2026-08-07 16:05 UTC (permalink / raw)
To: lpieralisi, kwilczynski, mani, robh, bhelgaas, rjui, sbranden,
bcm-kernel-feedback-list, rafal, florian.fainelli, arnd
Cc: linux-pci, linux-arm-kernel, linux-kernel, rosenp, rani.hod
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA
2026-08-07 16:05 [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA Semih Baskan
@ 2026-09-10 11:14 ` Manivannan Sadhasivam
2026-09-11 19:14 ` Bjorn Helgaas
1 sibling, 0 replies; 7+ messages in thread
From: Manivannan Sadhasivam @ 2026-09-10 11:14 UTC (permalink / raw)
To: lpieralisi, kwilczynski, mani, robh, bhelgaas, rjui, sbranden,
bcm-kernel-feedback-list, rafal, florian.fainelli, arnd,
Semih Baskan
Cc: linux-pci, linux-arm-kernel, linux-kernel, rosenp, rani.hod
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,
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA
2026-08-07 16:05 [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA Semih Baskan
2026-09-10 11:14 ` Manivannan Sadhasivam
@ 2026-09-11 19:14 ` Bjorn Helgaas
2026-09-12 4:32 ` Semih Baskan
1 sibling, 1 reply; 7+ messages in thread
From: Bjorn Helgaas @ 2026-09-11 19:14 UTC (permalink / raw)
To: Semih Baskan, Rafał Miłecki
Cc: lpieralisi, kwilczynski, mani, robh, bhelgaas, rjui, sbranden,
bcm-kernel-feedback-list, rafal, florian.fainelli, arnd,
linux-pci, linux-arm-kernel, linux-kernel, rosenp, rani.hod
[+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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA
2026-09-11 19:14 ` Bjorn Helgaas
@ 2026-09-12 4:32 ` Semih Baskan
2026-09-12 4:36 ` [PATCH] PCI: iproc: Use pci_alloc_host_bridge() " Semih Baskan
0 siblings, 1 reply; 7+ messages in thread
From: Semih Baskan @ 2026-09-12 4:32 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rafał Miłecki, lpieralisi, kwilczynski, mani, robh,
bhelgaas, rjui, sbranden, bcm-kernel-feedback-list, rafal,
florian.fainelli, arnd, linux-pci, linux-arm-kernel,
linux-kernel, rosenp, rani.hod
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
2026-09-12 4:32 ` Semih Baskan
@ 2026-09-12 4:36 ` Semih Baskan
2026-09-14 17:46 ` Bjorn Helgaas
0 siblings, 1 reply; 7+ messages in thread
From: Semih Baskan @ 2026-09-12 4:36 UTC (permalink / raw)
To: lpieralisi, kwilczynski, mani, robh, bhelgaas, rjui, sbranden,
bcm-kernel-feedback-list, rafal, zajec5, florian.fainelli, arnd
Cc: linux-pci, linux-arm-kernel, linux-kernel, rosenp, rani.hod,
Bjorn Helgaas
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
2026-09-12 4:36 ` [PATCH] PCI: iproc: Use pci_alloc_host_bridge() " Semih Baskan
@ 2026-09-14 17:46 ` Bjorn Helgaas
2026-09-16 8:12 ` Semih Baskan
0 siblings, 1 reply; 7+ messages in thread
From: Bjorn Helgaas @ 2026-09-14 17:46 UTC (permalink / raw)
To: Semih Baskan
Cc: lpieralisi, kwilczynski, mani, robh, bhelgaas, rjui, sbranden,
bcm-kernel-feedback-list, rafal, zajec5, florian.fainelli, arnd,
linux-pci, linux-arm-kernel, linux-kernel, rosenp, rani.hod
On Sat, Sep 12, 2026 at 07:36:26AM +0300, Semih Baskan wrote:
> 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.
I guess this goes back to 767012397976 ("ARM: dts: BCM5301X: Describe
PCIe controllers fully"), but I'm confused about this. Why are two
incompatible devices (rev 0x01 and 0x07) described with the same DT
with address ranges that are wrong for rev 0x01? I thought DT was
supposed to be matched with the hardware in the box?
And I guess I missed this part about the platform and the bcma drivers
both trying to claim the same device. That seems like something that
should be solved somewhere in the bus drivers (platform, bcma), not in
pcie-iproc-bcma.c and pcie-iproc-platform.c.
Why is this not a problem for other BCMA devices (bgmac_bcma_driver,
b43_bcma_driver, brcms_bcma_driver, bcma_hcd_driver)?
Is this another consequence of using a DT that describes
"brcm,iproc-pcie" controllers that don't match the hardware? Why
don't we have different DTs for these two kinds of hardware?
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
2026-09-14 17:46 ` Bjorn Helgaas
@ 2026-09-16 8:12 ` Semih Baskan
0 siblings, 0 replies; 7+ messages in thread
From: Semih Baskan @ 2026-09-16 8:12 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: lpieralisi, kwilczynski, mani, robh, bhelgaas, rjui, sbranden,
bcm-kernel-feedback-list, rafal, zajec5, florian.fainelli, arnd,
linux-pci, linux-arm-kernel, linux-kernel, rosenp, rani.hod
On Mon, Sep 14, 2026 at 12:46:24PM -0500, Bjorn Helgaas wrote:
> I guess this goes back to 767012397976 ("ARM: dts: BCM5301X: Describe
> PCIe controllers fully"), but I'm confused about this. Why are two
> incompatible devices (rev 0x01 and 0x07) described with the same DT
> with address ranges that are wrong for rev 0x01? I thought DT was
> supposed to be matched with the hardware in the box?
Yes, it goes back to that commit. The PCIe nodes are in bcm-ns.dtsi,
the include behind bcm4708.dtsi, bcm47081.dtsi, bcm4709.dtsi and
bcm47094.dtsi. Before 767012397976 they had reg and the cell sizes
only. That commit added the compatible, the interrupt maps, bus-range
and one set of ranges, and widened the axi node's ranges to the same
three windows. Its log says it was tested on BCM47094 with the
platform driver.
Both revisions are the same PCIe Gen 2 core, BCMA_CORE_NS_PCIEG2, and
pcie-iproc-bcma matches it at BCMA_ANY_REV. The difference this driver
meets is the fixed outbound window base. The first controller decodes
0x08000000 on both. The second and third decode 0x20000000/0x28000000
on revision 0x07 (BCM47094) and 0x40000000/0x48000000 on revision 0x01
(BCM47081 and BCM4709, measured). The values in the DT are the
revision 0x07 ones. Broadcom's own driver in the 2.6.36 vendor
kernels, arch/arm/plat-brcm/bcm5301x_pcie.c, has 0x40000000 and
0x48000000 as its default table and switches to 0x20000000 and
0x28000000 only when the core revision reads 0x7. The enumeration ROM
reports the same base per core, and that is what the applied patch
reads through bcma.
> And I guess I missed this part about the platform and the bcma drivers
> both trying to claim the same device. That seems like something that
> should be solved somewhere in the bus drivers (platform, bcma), not in
> pcie-iproc-bcma.c and pcie-iproc-platform.c.
Yes. Both drivers have bound these nodes since 767012397976 in any
build with both enabled, and neither the applied patch nor the
follow-up changes which one binds first. The follow-up only stops this
driver from requesting windows it never programs. It is in the commit
log because that changes what happens after the collision on revision
0x01, and multi_v7_defconfig builds both drivers (both symbols default
to y under ARCH_BCM_5301X). OpenWrt builds only the BCMA one.
> Why is this not a problem for other BCMA devices (bgmac_bcma_driver,
> b43_bcma_driver, brcms_bcma_driver, bcma_hcd_driver)?
Because the pcie nodes are the only children of the axi node in
bcm-ns.dtsi with a compatible at all. bcma_bus_register() runs
of_platform_default_populate() on the axi node before it registers its
cores, so a core whose node has a compatible gets a device from both
sides.
The gmac nodes have no compatible, so only bgmac-bcma binds them;
bgmac-platform matches brcm,amac, brcm,nsp-amac and brcm,ns2-amac,
none of which appear there. The usb2 and usb3 nodes have no compatible
either; bcma-hcd claims the core and populates the generic-ehci, ohci
and xhci children itself. b43 and brcmsmac have no platform
counterpart.
> Is this another consequence of using a DT that describes
> "brcm,iproc-pcie" controllers that don't match the hardware? Why
> don't we have different DTs for these two kinds of hardware?
The double claim comes from the compatible. Revision 0x07, where the
DT window matches the EROM, gets the two probes as well, so the ranges
play no part in it. bcma attaches the node to the core by its reg
either way, and the wifi child nodes in bcm4709-netgear-r8000.dts hang
off it, so both drivers see the same node however the ranges are
split.
The DT is already split per SoC family, and BCM47094 has its own
dtsi. The shared part is the include. The per-SoC files can carry the
window difference; today they do not.
Best regards,
Semih
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-16 8:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-07 16:05 [PATCH v3] PCI: iproc: Use the EROM outbound window on BCMA Semih Baskan
2026-09-10 11:14 ` Manivannan Sadhasivam
2026-09-11 19:14 ` Bjorn Helgaas
2026-09-12 4:32 ` Semih Baskan
2026-09-12 4:36 ` [PATCH] PCI: iproc: Use pci_alloc_host_bridge() " Semih Baskan
2026-09-14 17:46 ` Bjorn Helgaas
2026-09-16 8:12 ` Semih Baskan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®