Re: [PATCH v2 1/2] arm64: dts: rockchip: describe PCIe Ethernet controllers on NanoPC-T6

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Hardcoded and incorrectly bounded PCI bus numbers in device tree for dynamically enumerable PCI devices.
--

commit 79c9b71b546cb8ecf96eb0ea66c76b17346368b3
Author: Ricardo Pardini <ricardo@pardini.net>

arm64: dts: rockchip: describe PCIe Ethernet controllers on NanoPC-T6

This commit adds device tree nodes describing the two on-board Realtek
RTL8125BG NICs on the NanoPC-T6. It attaches ethernet0 and ethernet1
aliases to these fixed PCI function nodes.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi
> index 84b6b53f016ab..04c4479f08170 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi
[ ... ]
> @@ -635,6 +637,20 @@ &pcie2x1l0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie2_0_rst>;
>  	status = "okay";
> +
> +	pcie@0,0 {
> +		reg = <0x200000 0 0 0 0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		ranges;
> +		device_type = "pci";
> +		bus-range = <0x20 0x2f>;

[Severity: Medium]
Is this bus-range declaration correct for a bridge?

The first value of a bridge's bus-range is its secondary bus. If the Root
Port is on primary bus 0x20 (as defined by the host bridge), setting the
secondary bus to 0x20 sets it equal to the primary bus, which violates PCI
topology rules. Furthermore, the child device is declared below on bus
0x21, which contradicts this declaration.

> +
> +		rtl_eth0: ethernet@0,0 {
> +			compatible = "pci10ec,8125";
> +			reg = <0x210000 0 0 0 0>;

[Severity: Medium]
Does this hardcode a dynamic PCI bus number?

Using a specific bus number like 0x210000 instead of a relative address like
<0 0 0 0 0> can cause problems with strict parsers or bootloaders (such as
U-Boot) that perform dynamic enumeration. They might fail to match the nodes,
which could prevent device tree aliases from resolving correctly and break
MAC address injection.

> +		};
> +	};
>  };
>  
[ ... ]
> @@ -651,6 +667,20 @@ &pcie2x1l2 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie2_2_rst>;
>  	status = "okay";
> +
> +	pcie@0,0 {
> +		reg = <0x400000 0 0 0 0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		ranges;
> +		device_type = "pci";
> +		bus-range = <0x40 0x4f>;

[Severity: Medium]
Is this bus-range declaration correct for a bridge?

The first value of a bridge's bus-range is its secondary bus. If the Root
Port is on primary bus 0x40 (as defined by the host bridge), setting the
secondary bus to 0x40 sets it equal to the primary bus, which violates PCI
topology rules. Furthermore, the child device is declared below on bus
0x41, which contradicts this declaration.

> +
> +		rtl_eth1: ethernet@0,0 {
> +			compatible = "pci10ec,8125";
> +			reg = <0x410000 0 0 0 0>;

[Severity: Medium]
Does this hardcode a dynamic PCI bus number?

Using a specific bus number like 0x410000 instead of a relative address like
<0 0 0 0 0> can cause problems with strict parsers or bootloaders (such as
U-Boot) that perform dynamic enumeration. They might fail to match the nodes,
which could prevent device tree aliases from resolving correctly and break
MAC address injection.

> +		};
> +	};
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529-rk3588-dts-rtl-eth-describe-dt-alias-v2-0-49700248143f@pardini.net?part=1