Re: [PATCH v5 5/7] arm64: dts: qcom: Add initial support for MSM8952

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dts: thermal: CPU cooling-maps improperly bound to 'hot' trip points
--

commit cc303e4c44111ecaf69188771f4ff5a8e34c3552
Author: Muzaffer Kadir <muzafferkadir@mainlining.org>

arm64: dts: qcom: Add initial support for MSM8952

This commit adds initial device tree support for the Qualcomm MSM8952 SoC.
It introduces the base dtsi file containing CPU topology, memory maps,
system controllers, and thermal configurations.

Link: https://github.com/Asderdd/android_kernel_generalmobile_shamrock/blob/cm-14.1/arch/arm/boot/dts/qcom/msm8952.dtsi
Link: https://android.googlesource.com/kernel/msm.git/+/refs/tags/android-wear-7.0.0_r0.3/arch/arm/boot/dts/qcom/msm8952.dtsi
Link: https://android.googlesource.com/kernel/msm.git/+/refs/tags/android-wear-7.0.0_r0.3/drivers/thermal/msm-tsens.c#1621

> diff --git a/arch/arm64/boot/dts/qcom/msm8952.dtsi b/arch/arm64/boot/dts/qcom/msm8952.dtsi
> new file mode 100644
> index 0000000000000..18010f56ac5d0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8952.dtsi

[ ... ]

> +		cpu4-thermal {
> +			polling-delay-passive = <250>;
> +
> +			thermal-sensors = <&tsens 5>;
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&cpu4_alert1>;
> +					cooling-device = <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +			};
> +
> +			trips {
> +				cpu4_alert0: trip-point0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu4_alert1: trip-point1 {
> +					temperature = <85000>;
> +					hysteresis = <2000>;
> +					type = "hot";
					^^^^^^^^^^^

[Severity: High]
Will the thermal governor ignore this cooling action? Under the step_wise
governor, hot trip points are typically ignored for CPU throttling, which
might leave the CPU unprotected from overheating.

Would it be better to alter the trip point type to passive to ensure
throttling engages properly before shutdown limits are reached?

> +				};
> +
> +				cpu4_crit: cpu-crit {
> +					temperature = <110000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +		};

[ ... ]

> +		cpu0-3-thermal {
> +			polling-delay-passive = <250>;
> +
> +			thermal-sensors = <&tsens 9>;
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&cpu0_alert1>;
> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +			};
> +
> +			trips {
> +				cpu0_alert0: trip-point0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu0_alert1: trip-point1 {
> +					temperature = <85000>;
> +					hysteresis = <2000>;
> +					type = "hot";
					^^^^^^^^^^^

[Severity: High]
This appears to apply the same logical pattern to the little cores thermal
zone. Since cpu0_alert1 is also defined as a hot trip point, could this
likewise fail to throttle the cluster and leave the SoC unprotected?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-msm8952-initial-support-v5-0-1857074992ec@mainlining.org?part=5