Re: [PATCH] PCI: of: Skip dynamic nodes for bridges without a subordinate bus

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The fix for the NULL pointer dereference is an incorrect workaround that completely skips creating the dynamic OF node for the bridge device.
--

commit 876df8df861d1c7910e8724db8a082e07c66b879
Author: Angel J <iamanaws@httpd.dev>

PCI: of: Skip dynamic nodes for bridges without a subordinate bus

This commit adds an early return in of_pci_make_dev_node() for bridge
devices lacking a subordinate bus to prevent a boot hang on certain systems.

> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index a51dff91b196d..971c79c2a77da 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -765,6 +765,10 @@ void of_pci_make_dev_node(struct pci_dev *pdev)
>  	if (!ppnode)
>  		return;
>  
> +	/* Bridge properties require a subordinate bus. */
> +	if (pci_is_bridge(pdev) && !pdev->subordinate)
> +		return;
> +

[Severity: Medium]
Does completely skipping the OF node creation for these bridges cause
unintended side effects?

Since endpoint devices also lack subordinate buses but correctly get OF nodes,
wouldn't it be better to let the node creation proceed for the bridge, and
instead skip only the bus-range and interrupt-map properties?

The actual null pointer dereference appears to happen in
of_pci_add_properties() when handling those specific properties:

drivers/pci/of_property.c:of_pci_add_properties() {
    ...
    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;
    ...
}

Could the subordinate check be moved there instead, so the bridge still
receives its basic OF node configurations like the reg property?

>  	if (pci_is_bridge(pdev))
>  		pci_type = "pci";
>  	else

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911230420.26244-1-iamanaws@httpd.dev?part=1