Re: [PATCH 1/2] arm64: dts: qcom: ipq5210: Add nodes for multiple peripherals

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `gcc` node lacks the mandatory `#interconnect-cells` property.
- [Low] Missing interconnect binding header leading to build failure.
- [Low] Non-generic device node name `usb3@8a00000`.
--

--- Patch [1]: [PATCH 1/2] arm64: dts: qcom: ipq5210: Add nodes for multiple peripherals ---
commit 56c06e7d589960ad61fe5ba2fab0b467dc066d7b
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..6c97417e82db6 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq5210.dtsi
> @@ -4,7 +4,9 @@
>   */
>  
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/qcom,apss-ipq.h>
>  #include <dt-bindings/clock/qcom,ipq5210-gcc.h>
> +#include <dt-bindings/interconnect/qcom,ipq5210.h>

[Severity: Low]
Does this header exist in the tree? The inclusion of
<dt-bindings/interconnect/qcom,ipq5210.h> seems to cause a build failure
because the file is missing.

[ ... ]

> @@ -146,8 +358,8 @@ gcc: clock-controller@1800000 {
>  			reg = <0x0 0x01800000 0x0 0x40000>;
>  			clocks = <&xo_board>,
>  				 <&sleep_clk>,
> -				 <0>,
> -				 <0>,
> +				 <&pcie0_phy>,
> +				 <&pcie1_phy>,
>  				 <0>,
>  				 <0>;
>  			#clock-cells = <1>;

[Severity: Medium]
Since the pcie0 and pcie1 nodes specify interconnects pointing to the
gcc node, shouldn't the gcc node have an #interconnect-cells property?

For example, the pcie1 node added later in the patch uses:

			interconnects = <&gcc MASTER_CNOC_PCIE1 &gcc SLAVE_CNOC_PCIE1>,
					<&gcc MASTER_SNOC_PCIE1 &gcc SLAVE_SNOC_PCIE1>;

Without #interconnect-cells in the gcc node, Device Tree parsing might fail
at runtime.

[ ... ]

> +		usb3: usb3@8a00000 {
> +			compatible = "qcom,ipq5210-dwc3", "qcom,snps-dwc3";
> +			reg = <0x0 0x08a00000 0x0 0x200000>;

[Severity: Low]
Is it possible to use a more generic node name here, like usb@8a00000 instead
of usb3@8a00000, to align with Devicetree Specification naming conventions?

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