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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Missing `#interconnect-cells` property in the `gcc` clock controller node.
- [High] TSENS thermal sensor interrupt is incorrectly configured as edge-triggered.
- [High] Missing fallback compatible string for `pcie0_phy`.
- [Low] Missing `<dt-bindings/thermal/thermal.h>` include for `THERMAL_NO_LIMIT` macro.
- [Low] Included interconnect bindings header file does not exist.
--

--- Patch [1]: [PATCH v3 1/3] arm64: dts: qcom: ipq5210: Add nodes for multiple peripherals ---
commit b50b502c781c4dc3d469ed6c9eaf41d79df96251
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 so board
DTS files can enable the hardware they use. It adds nodes for USB, PCIe, QPIC
NAND/BAM, QFPROM, TSENS, and thermal zones, as well as 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..18e4a1dfa2682 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 dt-bindings interconnect header exist yet?

Including a non-existent header will cause a preprocessor fatal error during
the device tree build.

[ ... ]

> +		pcie0_phy: phy@84000 {
> +			compatible = "qcom,ipq5210-qmp-gen3x1-pcie-phy";

[Severity: High]
Is this missing a fallback compatible string?

Unlike pcie1_phy and the USB PHYs in this patch, pcie0_phy only lists the new
compatible string. Without a known fallback string, such as
"qcom,ipq9574-qmp-gen3x1-pcie-phy", will the existing QMP PCIe PHY driver fail
to bind and permanently break the PCIe0 subsystem?

[ ... ]

> +		tsens: thermal-sensor@4a9000 {
> +			compatible = "qcom,ipq5210-tsens", "qcom,ipq5332-tsens";
> +			reg = <0x0 0x004a9000 0x0 0x1000>,
> +			      <0x0 0x004a8000 0x0 0x1000>;
[ ... ]
> +			interrupts = <GIC_SPI 52 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-names = "combined";

[Severity: High]
Should this TSENS interrupt be configured as level-sensitive instead of
edge-triggered?

TSENS hardware typically emits active-high level-sensitive interrupts for
thermal thresholds. If configured as IRQ_TYPE_EDGE_RISING, a second thermal
threshold crossed while the interrupt line is already asserted will not
generate a new rising edge. 

This could cause the interrupt controller to miss the event, silently breaking
thermal throttling and potentially causing hardware damage.

[ ... ]

> @@ -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>;

[Severity: High]
Since the gcc node acts as an interconnect provider for the new pcie0 and pcie1
nodes, does it need an #interconnect-cells = <1>; property?

Without the #interconnect-cells property in the provider node, calls to
of_interconnect_get() from the PCIe drivers will fail with -EINVAL, causing
driver probe failures for all consumers.

[ ... ]

> +		cpu0-thermal {
> +			thermal-sensors = <&tsens 14>;
[ ... ]
> +			cooling-maps {
> +				map0 {
> +					trip = <&cpu0_thermal_alert>;
> +					/* Set maximum frequency as 816 MHz */
> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

[Severity: Low]
Can this cause a syntax error during the device tree compilation?

This line relies on THERMAL_NO_LIMIT being defined, but the required
<dt-bindings/thermal/thermal.h> header was not included in this file. The C
preprocessor will not expand the macro, leaving it as a string token, which
causes a dtc syntax error when evaluating the integer expressions.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-08-dts-v3-0-074eb131f382@oss.qualcomm.com?part=1