Re: [PATCH v5 1/3] 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:
- [High] PCIe interconnects are mapped backwards, swapping CNOC and SNOC.
- [High] The GCC (Global Clock Controller) node misses the linkage to the USB3 PHY pipe clock, hardcoding it to <0> instead of using the newly added ssphy_0 node.
--

commit 0bc8b6a2ddf853ed819f7ab663b2d660520bbccc
Author: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>

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

This commit describes common IPQ5210 SoC peripherals in the base DTSI. It
adds nodes for USB, PCIe, QPIC NAND/BAM, QFPROM, TSENS, and thermal zones,
along with the CPU OPP table and CPU cooling cells.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5210.dtsi b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> index 2cdc10529c489..2e6f99d894aba 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
[ ... ]
> @@ -146,8 +359,8 @@ gcc: clock-controller@1800000 {
>  			reg = <0x0 0x01800000 0x0 0x40000>;
>  			clocks = <&xo_board>,
>  				 <&sleep_clk>,
> -				 <0>,
> -				 <0>,
> +				 <&pcie0_phy>,
> +				 <&pcie1_phy>,
>  				 <0>,
>  				 <0>;

[Severity: High]
Is the 5th element (index 4) supposed to remain <0> here instead of linking to
<&ssphy_0>?

The qcom,ipq5210-gcc clock controller expects the USB3 PHY pipe clock at index
4. Since this patch adds the ssphy_0 node which provides the "usb0_pipe_clk",
leaving this unlinked could leave the USB3 pipe clock as an orphan, breaking
USB3 SuperSpeed functionality.

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

[Severity: High]
Are these interconnect mappings inverted?

The "pcie-mem" name (which corresponds to DMA traffic) maps to the first entry
(MASTER_CNOC_PCIE1), while "cpu-pcie" (CPU configuration traffic) maps to the
second entry (MASTER_SNOC_PCIE1).

This seems to incorrectly map high-bandwidth DMA traffic to the low-speed
Configuration NOC (CNOC), and low-bandwidth CPU config traffic to the
high-speed System NOC (SNOC). Will this lead to incorrect bus speeds being
set and DMA bandwidth starvation?

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

[Severity: High]
This appears to duplicate the reversed interconnect mapping seen in pcie1,
mapping "pcie-mem" to CNOC and "cpu-pcie" to SNOC.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-08-dts-v5-0-584e5ab50b40@oss.qualcomm.com?part=1