Re: [PATCH v3 3/3] arm64: dts: qcom: qcs6490-rb3gen2: add endpoint buses

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] PCI endpoint dynamic ranges update fails due to static node definition
- [Low] dtc warning: pci-ep-bus@4 unit-address lacks reg property
--

commit 8f41aa3c7aef831e8b83dd47d6e93a9ac9228f4f
Author: Alex Elder <elder@riscstar.com>

arm64: dts: qcom: qcs6490-rb3gen2: add endpoint buses

The TC9564 SoC incorporates a PCIe switch, which is connected via the second
PCI segment (0001) on the RB3gen2 platform. The downstream port has an
embedded PCIe endpoint that implements two PCIe functions, and this patch
adds the devicetree nodes representing these buses.

> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> index 3bb5fca8e2b13..5b7f5292a8864 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> @@ -955,11 +955,41 @@ pcie@3,0 {
>  			bus-range = <0x5 0xff>;
>  
>  			dev@0,0 {
> +				compatible = "pci1179,0220";
>  				reg = <0x50000 0x0 0x0 0x0 0x0>;
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				/* Ranges will be updated dynamically */
> +				ranges = <0x0 0x0 0x0  0x83050000 0x0 0x0  0x0 0x4000>,
> +					 <0x2 0x0 0x0  0x83050000 0x0 0x4000  0x0 0x80000>,
> +					 <0x4 0x0 0x0  0x83050000 0x0 0x84000  0x0 0x200000>;

[Severity: High]
Will these ranges actually be updated dynamically during boot?

The commit message states the ranges property will be updated dynamically.
However, the PCI quirk calls of_pci_make_dev_node() which has an early return
if the device node is already statically defined in the device tree:

    if (pci_device_to_OF_node(pdev))
        return;

Since dev@0,0 is defined here in the devicetree, it seems they are linked
during initial PCI enumeration, causing the update to be skipped entirely.
Furthermore, even if the early return was bypassed, the update uses
__of_add_property(), which rejects existing properties with -EEXIST.

Could this leave devices on the pci-ep-bus with invalid MMIO addresses mapped
to unassigned PCI regions?

> +
> +				pci-ep-bus@4 {
> +					compatible = "simple-bus";
> +					#address-cells = <1>;
> +					#size-cells = <1>;
> +					/* Map 0x0-0x1fffff to BAR 4 */
> +					ranges = <0x0  0x4 0x0 0x0  0x200000>;
> +				};

[Severity: Low]
Does this unit address trigger a devicetree compiler warning?

The pci-ep-bus@4 node is assigned a unit address of @4, but it lacks a
matching reg property. According to the Devicetree specification, if a node
uses a unit address, it must have a matching reg property.

For simple buses without a reg property, the unit address must match the
first child address in the ranges property, which is 0x0 here, rather than 4.

>  			};
>  
>  			dev@0,1 {
> +				compatible = "pci1179,0220";
>  				reg = <0x50100 0x0 0x0 0x0 0x0>;
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				/* Ranges will be updated dynamically */
> +				ranges = <0x0 0x0 0x0  0x83050100 0x0 0x0  0x0 0x4000>,
> +					 <0x2 0x0 0x0  0x83050100 0x0 0x4000  0x0 0x80000>,
> +					 <0x4 0x0 0x0  0x83050100 0x0 0x84000  0x0 0x200000>;

[Severity: High]
As with dev@0,0 above, it appears this node might also be skipped by
of_pci_make_dev_node() because it is statically defined. Will this prevent
the ranges property from being dynamically updated for this second endpoint?

> +
> +				pci-ep-bus@4 {
> +					compatible = "simple-bus";
> +					#address-cells = <1>;
> +					#size-cells = <1>;
> +					/* Map 0x0-0x1fffff to BAR 4 */
> +					ranges = <0x0  0x4 0x0 0x0  0x200000>;
> +				};

[Severity: Low]
Does this second pci-ep-bus@4 node also trigger a compiler warning for
missing a reg property while having a unit address?

>  			};
>  		};
>  	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915181003.1633543-1-elder@riscstar.com?part=3