Re: [PATCH v2 17/19] 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] dt-bindings: msm8952: truncated GIC memory map breaks virtualization and GICC_DIR
- [High] dt-bindings: msm8952: cpu cooling maps bound to hot trips bypass throttling
--

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

arm64: dts: qcom: Add initial support for MSM8952

This commit adds initial support for the MSM8952 SoC. It introduces the
device tree source file covering core components like CPUs, RPM, MDSS,
USB, pinctrl, and basic thermal zones.

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..a73032f75a8b7
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8952.dtsi
[ ... ]
> +		intc: interrupt-controller@b000000 {
> +			compatible = "qcom,msm-qgic2";
> +			interrupt-controller;
> +			#interrupt-cells = <3>;
> +			reg = <0x0b000000 0x1000>,
> +			      <0x0b002000 0x1000>;
> +		};

[Severity: High]
Does this node omit the GICH and GICV mappings along with the maintenance
interrupt? 

Additionally, the CPU interface size at 0x0b002000 appears to be truncated to
0x1000 instead of 0x2000. Mapping only 0x1000 bytes for the CPU interface
prevents access to the GICC_DIR register, which breaks interrupt deactivation
if the kernel uses EOI mode 1. 

Without the 0x2000-sized CPU interface mapping and the missing GICH/GICV
regions and maintenance interrupt, KVM initialization will fail and guest VMs
cannot run.

[ ... ]
> +		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]
Are these cooling maps binding to the correct trip points? 

The cooling device is bound to cpu4_alert1, which is a hot trip point. The
kernel thermal governors (like step_wise or power_allocator) explicitly ignore
THERMAL_TRIP_HOT trips for cooling device actuation. Cooling maps must be
bound to passive or active trips to invoke throttling. 

By binding to a hot trip instead of the alert0 passive trip, the CPUs will
never be thermally throttled, allowing the system to overheat until it hits
the critical trip and forcefully shuts down.

[ ... ]
> +		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]
Does this face the same throttling issue as cpu4-thermal? 

Binding the cpu0-3 cluster to the cpu0_alert1 hot trip point will also bypass
thermal throttling for these CPUs, resulting in unmanaged heating until a
critical thermal shutdown is triggered.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-msm8952-initial-support-v2-0-798c2602dde1@mainlining.org?part=17