[PATCH] thermal/drivers/imx: Disable clock on runtime resume failure

[PATCH] thermal/drivers/imx: Disable clock on runtime resume failure

[Can Peng]

imx_thermal_runtime_resume() enables the thermal clock before
powering up the sensor and enabling measurements.

If either regmap_write() fails, the function returns with the clock
still enabled. This leaves the clock enable count unbalanced after a
failed runtime resume.

Disable the clock on those failure paths before returning the error.

Fixes: 4cf2ddf16e17 ("thermal/drivers/imx: Implement runtime PM support")
Cc: stable@vger.kernel.org
Signed-off-by: Can Peng <pengcan@kylinos.cn>
---
 drivers/thermal/imx_thermal.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 5aaacbc53478..b0e88b6a9352 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -832,12 +832,12 @@ static int imx_thermal_runtime_resume(struct device *dev)
 	ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
 			   socdata->power_down_mask);
 	if (ret)
-		return ret;
+		goto disable_clk;
 
 	ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
 			   socdata->measure_temp_mask);
 	if (ret)
-		return ret;
+		goto disable_clk;
 
 	/*
 	 * According to the temp sensor designers, it may require up to ~17us
@@ -846,6 +846,11 @@ static int imx_thermal_runtime_resume(struct device *dev)
 	usleep_range(20, 50);
 
 	return 0;
+
+disable_clk:
+	clk_disable_unprepare(data->thermal_clk);
+
+	return ret;
 }
 
 static const struct dev_pm_ops imx_thermal_pm_ops = {
-- 
2.53.0

Re: [PATCH] thermal/drivers/imx: Disable clock on runtime resume failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing pm_runtime_put() in imx_get_temp() error path.
- [High] Unbalanced pm_runtime_put_noidle() in probe error path and remove causes usage counter underflow.
- [High] Potential Use-After-Free of data->tz in the thermal alarm IRQ handler during device removal.
- [High] Missing thermal_zone_device_unregister() causes a resource leak when pm_runtime_resume_and_get() fails during probe.
--

--- Patch [1]: [PATCH] thermal/drivers/imx: Disable clock on runtime resume failure ---
commit 623893dc8c9eeba8997b3f891585bf7c9155faa2
Author: Can Peng <pengcan@kylinos.cn>
Date:   Wed Jul 22 08:49:09 2026 +0000

    thermal/drivers/imx: Disable clock on runtime resume failure
    
    imx_thermal_runtime_resume() enables the thermal clock before
    powering up the sensor and enabling measurements.
    
    If either regmap_write() fails, the function returns with the clock
    still enabled. This leaves the clock enable count unbalanced after a
    failed runtime resume.
    
    Disable the clock on those failure paths before returning the error.

> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 5aaacbc534786..b0e88b6a9352a 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -832,12 +832,12 @@ static int imx_thermal_runtime_resume(struct device *dev)

[ ... ]

>  	/*
>  	 * According to the temp sensor designers, it may require up to ~17us
> @@ -846,6 +846,11 @@ static int imx_thermal_runtime_resume(struct device *dev)
>  	usleep_range(20, 50);
>  
>  	return 0;
> +
> +disable_clk:
> +	clk_disable_unprepare(data->thermal_clk);
> +
> +	return ret;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but doesn't this error path in
imx_get_temp() leak the PM usage counter permanently?

drivers/thermal/imx_thermal.c:imx_get_temp() {
    ...
	ret = pm_runtime_resume_and_get(data->dev);
	if (ret < 0)
		return ret;

	regmap_read(map, soc_data->temp_data, &val);

	if ((val & soc_data->temp_valid_mask) == 0)
		return -EAGAIN;
    ...
}

If pm_runtime_resume_and_get() succeeds, but the hardware returns an
invalid temperature mask, the function returns directly without calling
pm_runtime_put(). Does this prevent the device from ever entering runtime
suspend?

[Severity: High]
This is a pre-existing issue, but does pm_runtime_put_noidle() cause a
usage counter underflow when pm_runtime_resume_and_get() fails during probe?

drivers/thermal/imx_thermal.c:imx_thermal_probe() {
    ...
	ret = pm_runtime_resume_and_get(data->dev);
	if (ret < 0)
		goto disable_runtime_pm;
    ...
disable_runtime_pm:
	pm_runtime_put_noidle(data->dev);
    ...
}

Since pm_runtime_resume_and_get() already decrements the usage counter
internally on failure, doesn't calling pm_runtime_put_noidle() double-drop
the count?

A similar issue appears to be present in imx_thermal_remove():

drivers/thermal/imx_thermal.c:imx_thermal_remove() {
	struct imx_thermal_data *data = platform_get_drvdata(pdev);

	pm_runtime_put_noidle(data->dev);
	pm_runtime_disable(data->dev);
    ...
}

Does unconditionally dropping the usage count here leave the hardware
active without being synchronously suspended before removal?

[Severity: High]
This is a pre-existing issue, but is there a potential use-after-free
involving data->tz during device removal?

drivers/thermal/imx_thermal.c:imx_thermal_remove() {
    ...
	thermal_zone_device_unregister(data->tz);
	imx_thermal_unregister_legacy_cooling(data);
}

The alarm interrupt was requested using devm_request_threaded_irq() in probe,
so it remains active until after imx_thermal_remove() completes. If the
interrupt fires during this window, wouldn't imx_thermal_alarm_irq_thread()
dereference the freed data->tz?

[Severity: High]
This isn't a bug introduced by this patch, but is there a resource leak
if pm_runtime_resume_and_get() fails during probe?

drivers/thermal/imx_thermal.c:imx_thermal_probe() {
    ...
	ret = pm_runtime_resume_and_get(data->dev);
	if (ret < 0)
		goto disable_runtime_pm;
    ...
thermal_zone_unregister:
	thermal_zone_device_unregister(data->tz);
disable_runtime_pm:
	pm_runtime_put_noidle(data->dev);
	pm_runtime_disable(data->dev);
    ...
}

By jumping to disable_runtime_pm, the code skips the thermal_zone_unregister
label. Does this leave the thermal zone device registered and leak the
subsystem structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084909.463437-1-pengcan@kylinos.cn?part=1

Re: [PATCH] thermal/drivers/imx: Disable clock on runtime resume failure

[Frank Li]

On Wed, Jul 22, 2026 at 04:49:09PM +0800, Can Peng wrote:
> imx_thermal_runtime_resume() enables the thermal clock before
> powering up the sensor and enabling measurements.
>
> If either regmap_write() fails, the function returns with the clock
> still enabled. This leaves the clock enable count unbalanced after a
> failed runtime resume.
>
> Disable the clock on those failure paths before returning the error.
>
> Fixes: 4cf2ddf16e17 ("thermal/drivers/imx: Implement runtime PM support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Can Peng <pengcan@kylinos.cn>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/thermal/imx_thermal.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 5aaacbc53478..b0e88b6a9352 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -832,12 +832,12 @@ static int imx_thermal_runtime_resume(struct device *dev)
>  	ret = regmap_write(map, socdata->sensor_ctrl + REG_CLR,
>  			   socdata->power_down_mask);
>  	if (ret)
> -		return ret;
> +		goto disable_clk;
>
>  	ret = regmap_write(map, socdata->sensor_ctrl + REG_SET,
>  			   socdata->measure_temp_mask);
>  	if (ret)
> -		return ret;
> +		goto disable_clk;
>
>  	/*
>  	 * According to the temp sensor designers, it may require up to ~17us
> @@ -846,6 +846,11 @@ static int imx_thermal_runtime_resume(struct device *dev)
>  	usleep_range(20, 50);
>
>  	return 0;
> +
> +disable_clk:
> +	clk_disable_unprepare(data->thermal_clk);
> +
> +	return ret;
>  }
>
>  static const struct dev_pm_ops imx_thermal_pm_ops = {
> --
> 2.53.0
>
>