Re: [PATCH v6 01/11] arm64: dts: qcom: ipq5210: Add nodes for multiple peripherals

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] arm64: dts: qcom: Inverted PCIe interconnect mapping
- [Medium] dts: pcie memory ranges mapped to child address 0x0 breaks identity mapping
--

commit 5d6163228edf9eef2412de146956df64b5bb5d49
Author: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>

arm64: dts: qcom: ipq5210: Add nodes for multiple peripherals

Describe common IPQ5210 SoC peripherals in the base DTSI so board DTS files
can enable the hardware they use.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5210.dtsi b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> index 20e8b240642c6..161b5bb5a7f74 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
[ ... ]
> @@ -307,6 +619,287 @@ frame@b128000 {
>  				status = "disabled";
>  			};
>  		};
> +
> +		pcie1: pcie@50000000 {
> +			compatible = "qcom,pcie-ipq5210", "qcom,pcie-ipq9574";
[ ... ]
> +			ranges = <0x01000000 0x0 0x0 0x0 0x50200000 0x0 0x00100000>,
> +				 <0x02000000 0x0 0x0 0x0 0x50300000 0x0 0x0fd00000>;

[Severity: Medium]
Does mapping the PCI outbound memory space (0x02000000) to a child address of
0x0 break identity mapping here?

This allows the Linux PCI core to allocate memory BARs starting at PCI
address 0x0 (since PCIBIOS_MIN_MEM is 0 on arm64). Many drivers evaluate a
BAR address of 0x0 as unassigned (!pci_resource_start(...)), which can lead
to failed driver probes. Furthermore, breaking identity mapping often
conflicts with bootloader expectations and endpoints that assume the PCI bus
address matches the CPU physical address.

Could the child address be set to match the parent CPU physical address
(e.g., 0x0 0x50300000) to enforce a 1:1 mapping, as is standard practice for
other Qualcomm IPQ SoCs?

[ ... ]
> +			interconnects = <&gcc MASTER_CNOC_PCIE1 &gcc SLAVE_CNOC_PCIE1>,
> +					<&gcc MASTER_SNOC_PCIE1 &gcc SLAVE_SNOC_PCIE1>;
> +			interconnect-names = "pcie-mem", "cpu-pcie";

[Severity: Medium]
Are the interconnect-names inverted relative to the interconnects array?

The pcie-mem name is listed first, which binds it to the MASTER_CNOC_PCIE1
interconnect, while cpu-pcie binds to the MASTER_SNOC_PCIE1 interconnect.
CNOC typically governs CPU configuration accesses, while SNOC governs high
bandwidth DMA. This mismatch might cause interconnect bandwidth scaling and
power domain voting to apply to the wrong NoC paths during DMA or register
accesses.

[ ... ]
> +		pcie0: pcie@70000000 {
> +			compatible = "qcom,pcie-ipq5210", "qcom,pcie-ipq9574";
[ ... ]
> +			ranges = <0x01000000 0x0 0x0 0x0 0x70200000 0x0 0x00100000>,
> +				 <0x02000000 0x0 0x0 0x0 0x70300000 0x0 0x0fd00000>;

[Severity: Medium]
Does mapping the PCI outbound memory space (0x02000000) to a child address of
0x0 break identity mapping here as well?

Similar to pcie1, mapping this to 0x0 allows the Linux PCI core to allocate
memory BARs starting at PCI address 0x0, which causes issues for drivers
checking for an unassigned BAR.

Could the child address be identical to the parent CPU physical address
(e.g., 0x0 0x70300000) to enforce a 1:1 mapping?

[ ... ]
> +			interconnects = <&gcc MASTER_CNOC_PCIE0 &gcc SLAVE_CNOC_PCIE0>,
> +					<&gcc MASTER_SNOC_PCIE0 &gcc SLAVE_SNOC_PCIE0>;
> +			interconnect-names = "pcie-mem", "cpu-pcie";

[Severity: Medium]
Are the interconnect-names inverted relative to the interconnects array here
too?

The pcie-mem name is bound to CNOC and cpu-pcie is bound to SNOC, which
appears backwards and could lead to performance degradation or QoS failures
when NoC bandwidth scaling applies to the wrong paths.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-08-dts-v6-0-f9c4ec8664d8@oss.qualcomm.com?part=1