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

Semih Baskan posted 1 patch 1 week, 6 days ago
drivers/pci/controller/pcie-iproc-bcma.c | 70 +++++++++++++++++-------
1 file changed, 49 insertions(+), 21 deletions(-)
[PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Semih Baskan 1 week, 6 days ago
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
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Bjorn Helgaas 1 week, 3 days ago
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
>
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Semih Baskan 1 week, 2 days ago
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
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Bjorn Helgaas 1 week, 1 day ago
On Wed, Sep 16, 2026 at 11:12:28AM +0300, Semih Baskan wrote:
> 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.

If bcma finds both of these devices via EROM, why do they need a
"brcm,iproc-pcie" compatible?  If we omitted that compatible, what
would break?

Sorry to be dense, I'm probably asking dumb questions because I'm not
a DT expert.  I just imagine DT as being a substitute for native
enumeration protocols (e.g., PCI, ACPI, EROM), and as specific to a
piece of hardware, so this single DT that describes enumerable devices
with incompatible addresses doesn't fit my simple mental model.

Bjorn
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Semih Baskan 1 week, 1 day ago
On Wed, Sep 16, 2026 at 03:01:10PM -0500, Bjorn Helgaas wrote:
> If bcma finds both of these devices via EROM, why do they need a
> "brcm,iproc-pcie" compatible?  If we omitted that compatible, what
> would break?

bcma does not need it. pcie-iproc-platform does.

bcma attaches a child node to a core by comparing the node's reg with
the core's base address, in bcma_of_find_child_device(), and the only
compatible it looks at is the bus's own "brcm,bus-axi". The PCI
devices below then find their nodes by devfn under the core's node,
through pci_set_bus_of_node() and of_pci_find_child_device(). The wifi
nodes in bcm4709-netgear-r8000.dts go back to 5d1f2d2c2530 ("ARM: dts:
BCM5301X: Set 5 GHz wireless frequency limits on Netgear R8000") in
2017, which added pcie0 and pcie1 to the dtsi with reg and nothing
else. The core IRQs have come from the axi node's interrupt-map since
1f80de6863ca ("ARM: BCM5301X: add IRQ numbers for PCIe controller") in
2015.

The only code that matches the compatible is the pcie-iproc-platform
match table. The compatible is also what makes
of_platform_default_populate() create a platform device for the node.
Without one the node would be skipped and the platform driver would
never bind it. That driver reads the ranges, bus-range and the pcie
node's own interrupt-map, through devm_pci_alloc_host_bridge() and
of_irq_parse_and_map_pci(). The BCMA driver does not depend on any of
them. It has taken its window from the EROM since it was added,
552aa843e4c5 made it drop the DT one, and its interrupt comes from
bcma. With the follow-up it reads ranges only to compare them with the
EROM window.

What would break is a build with PCIE_IPROC_PLATFORM and without
PCIE_IPROC_BCMA. No in-tree defconfig sets either symbol. Both default
to y under ARCH_BCM_5301X, so multi_v7_defconfig builds both and would
still get PCIe through bcma. OpenWrt builds only the BCMA one.

The dtbs_check warnings in 767012397976's log would not come back. The
schema behind them, pci-bus.yaml in that log and pci-bus-common.yaml
in current dtschema, is selected by the node name, "^pcie?@", and
requires device_type, ranges and the cell sizes, not a compatible. I
checked with dt-validate from dtschema 2026.6: a pcie@12000 node with
reg and the cell sizes only gives the same two warnings, and the same
node with device_type and ranges added and still no compatible gives
none.

> Sorry to be dense, I'm probably asking dumb questions because I'm not
> a DT expert.  I just imagine DT as being a substitute for native
> enumeration protocols (e.g., PCI, ACPI, EROM), and as specific to a
> piece of hardware, so this single DT that describes enumerable devices
> with incompatible addresses doesn't fit my simple mental model.

The bcma binding reads the same way. bindings/bus/brcm,bus-axi.txt
says the cores are detected by bcma with the memory ranges they use.
The child nodes are there for what cannot be detected, IRQ numbers in
particular, and for cores that do extra things, such as ChipCommon and
its GPIO chip. Under that binding a pcie node is attached to a core
the EROM found, by reg.

Best regards,
Semih
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Bjorn Helgaas 1 week ago
On Thu, Sep 17, 2026 at 08:40:00AM +0300, Semih Baskan wrote:
> On Wed, Sep 16, 2026 at 03:01:10PM -0500, Bjorn Helgaas wrote:
> > If bcma finds both of these devices via EROM, why do they need a
> > "brcm,iproc-pcie" compatible?  If we omitted that compatible, what
> > would break?
> 
> bcma does not need it. pcie-iproc-platform does.

And why do we need pcie-iproc-platform, since pcie-iproc-bcma.c will
claim them?

Is there a requirement that pcie-iproc-platform must be able to claim
the PCI host bridge if CONFIG_PCIE_IPROC_BCMA is not enabled?
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Semih Baskan 1 week ago
On Thu, Sep 17, 2026 at 10:59:29AM -0500, Bjorn Helgaas wrote:
> And why do we need pcie-iproc-platform, since pcie-iproc-bcma.c will
> claim them?

I do not see anything on Northstar that needs it. I think
pcie-iproc-platform is the front end for the iProc SoCs that have no
bcma bus.

> Is there a requirement that pcie-iproc-platform must be able to claim
> the PCI host bridge if CONFIG_PCIE_IPROC_BCMA is not enabled?

None that I can find. Neither symbol depends on the other, no in-tree
defconfig sets either one, and multi_v7_defconfig gets both from their
defaults. OpenWrt builds only the BCMA one. The one use of the
platform driver on Northstar I know of is Rafał's: 767012397976 says
it was tested with it on BCM47094, and his test in the thread on the
axi ranges ran both drivers on his board. The platform driver
enumerated the two BCM4366 devices on its own:
https://lore.kernel.org/all/93ee7456-4179-4d78-b980-1df0b7e0278a@gmail.com/
I have not looked outside the tree.

Best regards,
Semih
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Bjorn Helgaas 1 week ago
On Thu, Sep 17, 2026 at 09:02:23PM +0300, Semih Baskan wrote:
> On Thu, Sep 17, 2026 at 10:59:29AM -0500, Bjorn Helgaas wrote:
> > And why do we need pcie-iproc-platform, since pcie-iproc-bcma.c will
> > claim them?
> 
> I do not see anything on Northstar that needs it. I think
> pcie-iproc-platform is the front end for the iProc SoCs that have no
> bcma bus.
> 
> > Is there a requirement that pcie-iproc-platform must be able to claim
> > the PCI host bridge if CONFIG_PCIE_IPROC_BCMA is not enabled?
> 
> None that I can find. Neither symbol depends on the other, no in-tree
> defconfig sets either one, and multi_v7_defconfig gets both from their
> defaults. OpenWrt builds only the BCMA one. The one use of the
> platform driver on Northstar I know of is Rafał's: 767012397976 says
> it was tested with it on BCM47094, and his test in the thread on the
> axi ranges ran both drivers on his board. The platform driver
> enumerated the two BCM4366 devices on its own:
> https://lore.kernel.org/all/93ee7456-4179-4d78-b980-1df0b7e0278a@gmail.com/
> I have not looked outside the tree.

Then it seems like we should remove the "brcm,iproc-pcie" compatible
from DTs for platforms where bcma can enumerate it via EROM.  Those
platforms can enable CONFIG_PCIE_IPROC_BCMA.  They still need the PCIe
controller node in DT so they can use the wifi nodes below it, but if
there's no "brcm,iproc-pcie" compatible, pcie-iproc-platform shouldn't
do anything.

Arnd, Rafał, jump in if I'm in the weeds here.
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Arnd Bergmann 1 week ago
On Thu, Sep 17, 2026, at 20:36, Bjorn Helgaas wrote:
> On Thu, Sep 17, 2026 at 09:02:23PM +0300, Semih Baskan wrote:
>> On Thu, Sep 17, 2026 at 10:59:29AM -0500, Bjorn Helgaas wrote:
>
> Then it seems like we should remove the "brcm,iproc-pcie" compatible
> from DTs for platforms where bcma can enumerate it via EROM.  Those
> platforms can enable CONFIG_PCIE_IPROC_BCMA.  They still need the PCIe
> controller node in DT so they can use the wifi nodes below it, but if
> there's no "brcm,iproc-pcie" compatible, pcie-iproc-platform shouldn't
> do anything.
>
> Arnd, Rafał, jump in if I'm in the weeds here.

It feels wrong to me to change the devicetee file when that arguably
describes the device correctly, and we still need the node for the
bcma bus probe in the end.

Normally, there should be a list of compatible strings so the driver
can make a more informed decision, e.g. the platform driver could
skip a device if it identifies the compatible string as one that
can be probed using the bcma bus when that driver is also enabled.

Unfortunately, it appears that all of these only have the generic
"brcm,iproc-pcie" string (or only the paxc variant), so that doesn't
work here.

Would it be help to change the probe order so the bcma driver
always comes before the platform driver?

      Arnd
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Semih Baskan 1 week ago
On Thu, Sep 17, 2026 at 10:27:54PM +0200, Arnd Bergmann wrote:
> Would it be help to change the probe order so the bcma driver
> always comes before the platform driver?

Only where the DT window matches the EROM one, because that is where
the platform driver backs out.

In the tree as it is, the platform driver binds first.
bcma_bus_register() populates the axi node's children before it
registers the cores (drivers/bcma/main.c), and with both drivers built
in the two PCIe drivers are already registered by then (both are
device_initcall and drivers/pci links before drivers/bcma), so the pcie
platform devices are bound as soon as they are created. The order of the
two objects in drivers/pci/controller/Makefile does not change that.

I tried both orders on an RT-N18U (BCM47081, revision 0x01) with both
drivers built in, on an OpenWrt 6.18 kernel with pcie-iproc-bcma.c as
in this patch and no wifi driver built.

With the platform driver first, the board did not come up, on two
flashes. The same tree built with only the BCMA driver boots it.

With of_platform_default_populate() moved after
bcma_register_devices() as a test, it boots. pcie-iproc-bcma takes the
first controller and enumerates the BCM4360 as before. The platform
driver then probes the same node and fails in
devm_pci_alloc_host_bridge(), which it reports as -ENOMEM:

  iproc-pcie 18012000.pcie: resource collision: [mem
0x08000000-0x0fffffff] conflicts with PCIe MEM space [mem
0x08000000-0x0fffffff]

On the second and third controllers the DT window is
0x20000000/0x28000000 and the EROM one 0x40000000/0x48000000, so the
platform driver's request goes through and its probe carries on with
the DT window until the link check, which fails because nothing is
connected to those two on this board. That is as far as this board can
show.

On the first controller the DT window and the EROM one are the same,
so the window plays no part in the no-boot. With PCIE_IPROC_BCMA not
set at all the board did not come up either. Clearing
has_apb_err_disable for IPROC_PCIE_PAXB, one line, makes that
platform-only kernel boot and enumerate the BCM4360; nothing else
differs between the two builds. The flag makes the driver read and
write APB_ERR_EN (0xf40) around every config access to bus 1 and
above (538928fd6ce8). In the bcma-first build the platform driver
never reached bus 1. The PAXB_BCMA register table has no such
register, and 404349c5c806 ("PCI: iproc: Add BCMA type") describes
the BCMA-based NS as a legacy PAXB with some registers missing. On
Rafał's BCM47094 (revision 0x07) the platform driver as shipped
enumerates both BCM4366s. I could not see what the access to 0xf40
does on revision 0x01, only that the board comes up without it.

Best regards,
Semih
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Bjorn Helgaas 1 week ago
On Thu, Sep 17, 2026 at 10:27:54PM +0200, Arnd Bergmann wrote:
> On Thu, Sep 17, 2026, at 20:36, Bjorn Helgaas wrote:
> > On Thu, Sep 17, 2026 at 09:02:23PM +0300, Semih Baskan wrote:
> >> On Thu, Sep 17, 2026 at 10:59:29AM -0500, Bjorn Helgaas wrote:
> >
> > Then it seems like we should remove the "brcm,iproc-pcie" compatible
> > from DTs for platforms where bcma can enumerate it via EROM.  Those
> > platforms can enable CONFIG_PCIE_IPROC_BCMA.  They still need the PCIe
> > controller node in DT so they can use the wifi nodes below it, but if
> > there's no "brcm,iproc-pcie" compatible, pcie-iproc-platform shouldn't
> > do anything.
> >
> > Arnd, Rafał, jump in if I'm in the weeds here.
> 
> It feels wrong to me to change the devicetee file when that arguably
> describes the device correctly, and we still need the node for the
> bcma bus probe in the end.

I don't think pcie-iproc-bcma.c needs anything from DT at all.
Everything it needs is in EROM.

Some PCI drivers, e.g., wifi, need DT nodes, and I think those need to
be enclosed in a node for the PCI host controller, but
pcie-iproc-bcma.c itself doesn't care.

Bjorn
Re: [PATCH] PCI: iproc: Use pci_alloc_host_bridge() on BCMA
Posted by Arnd Bergmann 1 week ago
On Thu, Sep 17, 2026, at 23:48, Bjorn Helgaas wrote:
> On Thu, Sep 17, 2026 at 10:27:54PM +0200, Arnd Bergmann wrote:
>> On Thu, Sep 17, 2026, at 20:36, Bjorn Helgaas wrote:
>> 
>> It feels wrong to me to change the devicetee file when that arguably
>> describes the device correctly, and we still need the node for the
>> bcma bus probe in the end.
>
> I don't think pcie-iproc-bcma.c needs anything from DT at all.
> Everything it needs is in EROM.
>
> Some PCI drivers, e.g., wifi, need DT nodes, and I think those need to
> be enclosed in a node for the PCI host controller, but
> pcie-iproc-bcma.c itself doesn't care.

I understand that it just works without that, my only concerns is
that having a DT node (which is required for adding child nodes)
without a compatible string is going to cause problems later,

From the DT perspective, it would be better to add a second
(or third) compatible string to identify the device better and
keep the existing generic string as a fallback. The
pcie-iproc-platform driver can then skip the device based
on the more specific string.

Simply replacing the existing compatible string with a new one
would solve the probe order problem and at least leave a
well-formed device node, but this would still count as an
incompatible binding update, which we try hard to avoid.

        Arnd