Re: [PATCH RFC 18/20] 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] MPM pin map contains GIC INTIDs instead of GIC SPI numbers, breaking wakeup interrupt routing.
- [High] Passive CPU cooling devices are bound to 'hot' trip points, completely disabling thermal throttling.
--

commit 316713382c786eea1b0593b2d6ec702dcfda18de
Author: Muzaffer Kadir <muzafferkadir@mainlining.org>
arm64: dts: qcom: Add initial support for MSM8952

This commit adds device tree support for the Qualcomm MSM8952 SoC, enabling
peripherals like the MDSS, USB, RPM, and other basic features.

> diff --git a/arch/arm64/boot/dts/qcom/msm8952.dtsi b/arch/arm64/boot/dts/qcom/msm8952.dtsi
> new file mode 100644
> index 0000000000000..8b7fa84e7986a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8952.dtsi
[ ... ]
> +		mpm: interrupt-controller {
> +			compatible = "qcom,mpm";
> +			qcom,rpm-msg-ram = <&apss_mpm>;
> +			interrupts = <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>;
> +			mboxes = <&apcs1 1>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			#power-domain-cells = <0>;
> +			interrupt-parent = <&intc>;
> +			qcom,mpm-pin-count = <64>;
> +			qcom,mpm-pin-map = <2 216>, /* TSENS UPLOW */
> +				<49 172>, /* USB ASYNC */
> +				<53 104>, /* MDSS */
> +				<58 166>, /* USB */
> +				<62 222>; /* SPMI */
> +		};

[Severity: High]
Does this mpm-pin-map define GIC INTIDs (SPI + 32) instead of the expected
raw GIC SPI numbers?

When providing INTIDs instead of raw SPI numbers, system wakeup interrupts for
peripherals like TSENS, USB, and MDSS will be misrouted. When the SoC wakes up,
the MPM driver will re-trigger the wrong parent SPI (for example, requesting
SPI 216 instead of 184 for TSENS).

This can cause the peripheral driver to miss the interrupt and potentially leave
the system in a stuck state.

[ ... ]
> +		cpu4-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +			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 cpu4 cooling device ever be engaged?

The cooling map in cpu4-thermal binds the passive CPU cooling device to the
cpu4_alert1 trip point, but this trip point is defined as type "hot".

The thermal core and default passive cooling governors (such as step_wise or
power_allocator) explicitly ignore "hot" trip points. Because of this, the
bound cooling device will not be triggered to reduce the CPU frequency when the
85C threshold is reached, leading to thermal runaway and an uncontrolled
shutdown when the critical trip is hit.

This pattern appears to also be present in the cpu5-thermal, cpu6-thermal,
cpu7-thermal, and cpu0-3-thermal blocks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-msm8952-initial-support-v1-0-0e742578b524@mainlining.org?part=18