Re: [PATCH v2] PCI: of_property: Omit bus properties without a subordinate bus

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Sep 11, 2026 at 09:31:06PM -0700, Angel J wrote:
> A device can satisfy pci_is_bridge() without having a subordinate bus.
> The dynamic OF helpers for bus-range and interrupt-map dereference
> pdev->subordinate without checking it.
> 
> On a Dell XPS 8940, device 0000:00:00.0 [8086:4c43] has no subordinate
> bus, and enabling CONFIG_PCI_DYNAMIC_OF_NODES causes an early boot hang.
> 
> Generate bus-range and interrupt-map only when a subordinate bus exists.
> Keep the node and its remaining properties for bridges without one.
> 
> Boot-tested on Linux 6.18.44 with CONFIG_PCI_DYNAMIC_OF_NODES=y. The
> system boots and the node for 00:00.0 retains device_type, reg and
> compatible. The other bridges retain their nodes and bus ranges.

I think this is the right approach, with something like this included
in the commit log:

  A bridge (a device with a Type 1 header) may not have a secondary
  bus allocated (pdev->subordinate), e.g., if there are no available
  bus numbers or the bridge secondary/subordinate bus numbers are not
  writable.

I don't think it's necessary to connect this with [8086:4c43] because
I think this situation is very common, and I don't want to leave the
impression that it's related to a Dell system or to that specific
device.  It's just a bug to assume that pci_is_bridge() implies
pdev->subordinate is valid.

> Fixes: 407d1a51921e ("PCI: Create device tree node for bridge")
> Cc: stable@vger.kernel.org
> Signed-off-by: Angel J <iamanaws@httpd.dev>
> ---
> Changes in v2:
> - Keep the dynamic OF node and skip only bus-range and interrupt-map
>   when no subordinate bus exists, following review of v1.
> - Tested on Linux 6.18.44 with the v1 guard removed.
> 
>  drivers/pci/of_property.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/of_property.c b/drivers/pci/of_property.c
> index 75a358f73..acd2e0f70 100644
> --- a/drivers/pci/of_property.c
> +++ b/drivers/pci/of_property.c
> @@ -361,13 +361,15 @@ int of_pci_add_properties(struct pci_dev *pdev, struct of_changeset *ocs,
>  		if (ret)
>  			return ret;
>  
> -		ret = of_pci_prop_bus_range(pdev, ocs, np);
> -		if (ret)
> -			return ret;
> +		if (pdev->subordinate) {
> +			ret = of_pci_prop_bus_range(pdev, ocs, np);
> +			if (ret)
> +				return ret;
>  
> -		ret = of_pci_prop_intr_map(pdev, ocs, np);
> -		if (ret)
> -			return ret;
> +			ret = of_pci_prop_intr_map(pdev, ocs, np);
> +			if (ret)
> +				return ret;

I think this is the best solution, but I do think it would be a little
cleaner to check for pdev->subordinate inside of_pci_prop_bus_range()
and of_pci_prop_intr_map() instead of assuming things here about the
internals of those functions.  We could just make them return
immediately without doing anything.

> +		}
>  	} else {
>  		ret = of_pci_prop_intr_ctrl(pdev, ocs, np);
>  		if (ret)
> -- 
> 2.54.0
>