Re: [PATCH v7 4/5] arm64: dts: exynos: gs101: Add thermal management unit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The TMU APB clock (priv->clk) is left enabled during system suspend, causing a power leak.
--

commit 45da7d1dd2f371f5e33444f5d1be0481464a464a
Author: Tudor Ambarus <tudor.ambarus@linaro.org>

arm64: dts: exynos: gs101: Add thermal management unit

This commit adds Thermal Management Unit (TMU) support for the Google
GS101 SoC. It describes the TMU using a consolidated SoC node that includes
memory resources and a phandle to the ACPM IPC interface. It also defines
thermal zones for the CPU clusters to enable thermal mitigation.

> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> index 86933f22647b7..b6866ef99fb34 100644
> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi

[ ... ]

> @@ -639,6 +647,15 @@ watchdog_cl1: watchdog@10070000 {
>  			status = "disabled";
>  		};
>  
> +		tmu_top: thermal-sensor@100a0000 {
> +			compatible = "google,gs101-tmu-top";
> +			reg = <0x100a0000 0x800>;
> +			clocks = <&cmu_misc CLK_GOUT_MISC_TMU_TOP_PCLK>;
> +			interrupts = <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH 0>;
> +			samsung,acpm-ipc = <&acpm_ipc>;
> +			#thermal-sensor-cells = <1>;
> +		};

[Severity: High]
Does enabling this device tree node lead to a clock leak during system
suspend in the corresponding driver?

Looking at acpm_tmu_suspend() in drivers/thermal/samsung/acpm-tmu.c:

drivers/thermal/samsung/acpm-tmu.c:acpm_tmu_suspend() {
    ...
    ret = acpm_tmu_control(priv, false);
    ...
}

During system suspend, the core PM subsystem calls pm_runtime_get_noresume()
in the device_prepare() phase, which artificially increments the runtime PM
usage count to prevent runtime suspend transitions.

When acpm_tmu_suspend() is invoked, it calls acpm_tmu_control(), which
temporarily increments and decrements the usage count via
pm_runtime_resume_and_get() and pm_runtime_put_sync().

Because the core PM still holds its reference, the usage count never drops
to 0. Consequently, acpm_tmu_runtime_suspend() is never invoked, meaning
clk_disable_unprepare() is skipped.

The peripheral clock remains unconditionally enabled for the entire duration
of the system sleep, leading to battery drain and preventing deep sleep
states for the parent clock and power domain.

The driver author intended the clock to be off here (as noted by the comment
'APB clock not required for this specific msg'), but missed the core PM
refcount behavior.

Should the driver be modified to ensure the clock is disabled or synced via
pm_runtime_force_suspend() instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-acpm-tmu-v7-0-8047dd54b0e9@linaro.org?part=4