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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dts: qcom: cpu cooling maps bound to hot trip point bypass throttling
- [Medium] dts: qcom: l2-thermal trip points are logically inverted
--

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

arm64: dts: qcom: Add initial support for MSM8952
    
Add initial support for MSM8952 SoC, including internal storage, MDSS,
basic Adsp remote proc, i2c/spi, Tsens, USB, and RPM support.

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
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8952.dtsi
[ ... ]
> +		l2-thermal {
> +			polling-delay-passive = <250>;
> +
> +			thermal-sensors = <&tsens 4>;
> +
> +			trips {
> +				l2_alert0: trip-point0 {
> +					temperature = <50000>;
> +					hysteresis = <2000>;
> +					type = "hot";
> +				};
> +
> +				l2_alert1: trip-point1 {
> +					temperature = <55000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				l2_crit: l2-crit {

[Severity: Medium]
Are these l2-thermal trip points logically inverted?

The hot trip point is placed at 50 degrees Celsius, which is lower than the
passive trip point at 55 degrees Celsius. Because a hot trip point represents
a more severe thermal condition designed to notify userspace of overheating,
while passive initiates kernel-level throttling, firing a hot trip before the
passive mitigation tier can lead to premature userspace warnings.

[ ... ]
> +		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]
Does binding the cooling device to cpu4_alert1 bypass kernel thermal
throttling?

The cooling-maps tie the cpu cooling device to cpu4_alert1, which is
configured as a hot trip point. In the Linux thermal subsystem, the default
governors do not engage cooling devices for hot trip points, as they are
intended for userspace notification.

If the CPU hits 85 degrees Celsius, it appears throttling will not engage,
causing it to continue heating until it reaches the 110 degrees Celsius
emergency shutdown trip point and abruptly powers off the system. This pattern
seems to be repeated for all other CPU thermal zones in this file.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006-msm8952-initial-support-v4-0-ecdcc1ac620c@mainlining.org?part=5