Re: [PATCH 2/2] 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:

New issues:
- [High] The i.MX7D alarm temperature calculation truncates towards zero, leading to an early hardware trigger and an IRQ storm when the calibration offset is not a multiple of 1000.

Pre-existing issues:
- [High] The computed `alarm_value` is shifted and written to the high alarm control hardware register without bitwise masking, allowing out-of-bounds or negative values to corrupt adjacent register fields.
--

--- Patch [2]: [PATCH 2/2] thermal/drivers/imx: Add calibration offset support ---
commit b15c0dce27dccd66bd58b8f22bb746933db67a41
Author: HaoNing Cheng <Haoning.CHENG@cn.bosch.com>

thermal/drivers/imx: Add calibration offset support

Some boards need a small per-design correction to align the reported CPU
temperature with board-level measurements.

> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 5aaacbc534786..3e1eec2fc3613 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -241,7 +242,7 @@ static void imx_set_alarm_temp(struct imx_thermal_data *data,
>  	data->alarm_temp = alarm_temp;
>  
>  	if (data->socdata->version == TEMPMON_IMX7D)
> -		alarm_value = alarm_temp / 1000 + data->c1 - 25;
> +		alarm_value = (alarm_temp - data->calibration_offset) / 1000 + data->c1 - 25;

[Severity: High]
If the device tree property fsl,temp-calibration-offset-millicelsius is set
to a value that is not a multiple of 1000 (e.g., 500), does this integer
division truncate towards zero and cause an early hardware alarm trigger?

When the hardware asserts the thermal interrupt due to the truncated threshold,
the IRQ handler would read the temperature, calculate that the temperature is
still below the alarm_temp, and immediately re-enable the IRQ. Could this
regression lead to an infinite IRQ storm?

[Severity: High]
This is a pre-existing issue, but the computed alarm_value is shifted and
written to the high alarm control register shortly after this without masking:

    regmap_write(map, soc_data->high_alarm_ctrl + REG_SET,
                 alarm_value << soc_data->high_alarm_shift);

Since the newly introduced calibration offset could make alarm_value negative
or exceed its 9-bit mask, shifting a negative signed value will set the upper
bits to 1. 

Because this is a bit-set operation, could writing this value overwrite and
corrupt the adjacent IMX7_TEMPSENSE0_PANIC_ALARM field (bits 18-26)? The same
pattern appears to exist for the panic alarm threshold in imx_set_panic_temp().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-b4-symana21-11221-imx-thermal-support-upstream-6-18-v1-0-00f88f42930b@cn.bosch.com?part=2