Re: [PATCH v4 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] The `cpu1-thermal` zone defines a `cpu-passive` trip point but lacks a `cooling-maps` block to enforce CPU throttling.
- [Low] The `cpu0-thermal` cooling maps use `THERMAL_NO_LIMIT` bounds, which contradicts the stated intention to cap the frequency at 816 MHz.
--

--- Patch [1]: [PATCH v4 1/3] arm64: dts: qcom: ipq5210: Add nodes for multiple peripherals ---
commit 1a65f86590b0031ce931c8fec8d83a0167f7d018
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. Add nodes for USB, PCIe, QPIC NAND/BAM,
QFPROM, TSENS and thermal zones. This also describes the CPU OPP table and
CPU cooling cells needed for frequency scaling and thermal throttling.

> 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

[ ... ]

> @@ -306,6 +618,287 @@ frame@b128000 {

[ ... ]

> +		cpu0-thermal {
> +			thermal-sensors = <&tsens 14>;
> +
> +			trips {
> +				cpu-critical {
> +					temperature = <120000>;
> +					hysteresis = <9000>;
> +					type = "critical";
> +				};
> +
> +				cpu0_thermal_alert: cpu-passive {
> +					temperature = <110000>;
> +					hysteresis = <9000>;
> +					type = "passive";
> +				};
> +			};
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&cpu0_thermal_alert>;
> +					/* Set maximum frequency as 816 MHz */
> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

[Severity: Low]
This isn't a bug, but does THERMAL_NO_LIMIT match the comment about setting
the maximum frequency to 816 MHz?

Passing THERMAL_NO_LIMIT simply delegates the entire range of cooling states
to the thermal governor rather than applying a hard upper frequency limit.

If the intention is to clamp the frequency to 816 MHz, should explicit cooling
states be used?

> +							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +			};
> +		};
> +
> +		cpu1-thermal {
> +			thermal-sensors = <&tsens 15>;
> +
> +			trips {
> +				cpu-critical {
> +					temperature = <120000>;
> +					hysteresis = <9000>;
> +					type = "critical";
> +				};
> +
> +				cpu-passive {
> +					temperature = <110000>;
> +					hysteresis = <9000>;
> +					type = "passive";
> +				};
> +			};
> +		};

[Severity: High]
Is a cooling-maps block missing for this thermal zone?

The cpu1-thermal zone defines a passive trip point but lacks a cooling-maps
section to enforce CPU throttling.

If CPU1's thermal sensor detects a temperature exceeding 110C independent of
CPU0, won't it exceed the passive threshold unmitigated until it reaches the
120C critical trip, causing an abrupt hardware thermal shutdown?

>  	};
>  
>  	timer {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-08-dts-v4-0-65e61a1061f0@oss.qualcomm.com?part=1