Re: [PATCH v8 3/3] thermal/drivers/imx: Add calibration offset support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing pm_runtime_put() on the error path in imx_get_temp() causes a runtime PM leak.
- [High] Missing pm_runtime_put() on the error path in imx_set_trip_temp() causes a runtime PM leak.
--

--- Patch [3]: [PATCH v8 3/3] thermal/drivers/imx: Add calibration offset support ---
commit a5acc3eb3799b4fe9bc9d1fba54049882d4b5c06
Author: Haoning CHENG <Haoning.CHENG@cn.bosch.com>

thermal/drivers/imx: Add calibration offset support

This commit adds support for an optional calibration offset to align
reported CPU temperatures with board-level measurements. It reads the
fsl,temp-calibration-offset-millicelsius property from the device tree
and applies it symmetrically to the threshold and temperature readings
for i.MX6/6SX/7D devices.

> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 168069a16e930..87ae2e40cc6fe 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
[ ... ]
> @@ -283,6 +290,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>  		*temp = (n_meas - data->c1 + 25) * 1000;
>  	else
>  		*temp = data->c2 - n_meas * data->c1;
> +	*temp += data->calibration_offset;
>  
>  	/* Update alarm value to next higher trip point for TEMPMON_IMX6Q */
>  	if (data->socdata->version == TEMPMON_IMX6Q) {

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this file I
noticed a potential issue earlier in imx_get_temp(). Does the early return 
for invalid sensor data leak a PM runtime reference?

Earlier in the function, it calls pm_runtime_resume_and_get():

    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 the sensor is polled before data is valid and it takes this -EAGAIN
path, shouldn't there be a matching pm_runtime_put(data->dev) to prevent
the device and its power domain from permanently remaining awake?

[Severity: High]
This is also a pre-existing issue, but the same pattern appears to exist
in imx_set_trip_temp() during bounds checking. Does this code leak the
PM runtime usage counter?

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

    if (temp < 0 || temp > trips[IMX_TRIP_CRITICAL].temperature)
        return -EINVAL;
    ...
}

If a user writes an out-of-bounds temperature, this early return misses
the pm_runtime_put(data->dev) call. Could this be updated to properly
release the reference on this error path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-b4-symana21-11221-imx-thermal-support-upstream-6-18-v8-0-d54d8690e16e@cn.bosch.com?part=3