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

[Frieder Schrempf]

On 14.07.26 12:28, Haoning CHENG via B4 Relay wrote:
> From: Haoning CHENG <Haoning.CHENG@xxxxxxxxxxxx>
> 
> Some boards need a small per-design correction to align the reported CPU
> temperature with board-level measurements. Read the optional
> fsl,temp-calibration-offset-millicelsius property from DT and apply it
> uniformly to the i.MX6/6SX/7D calibration formulas.
> 

Sorry to chime in so late. I just want to understand what this
calibration offset is about. Why would there be a need of a
board-specific offset? The sensor is in the SoC and if you add a
board-specific offset, you no longer measure the SoC core temperature,
right?

How would you determine the offset in the first place? How would I know
what fsl,temp-calibration-offset-millicelsius should be set to? I could
put a sensor on the SoC case and use the delta as offset, but then I
would just account for the thermal resistance of the casing and not
measure the SoC core temperature anymore, right?

Maybe I'm just missing something obvious here, so if anyone could
enlighten me that would be appreciated.

Thanks!

> The offset is applied symmetrically at two points to ensure the thermal
> framework sees calibrated temperatures while hardware thresholds remain
> correctly positioned:
> 
> 1. In imx_set_alarm_temp() and imx_set_panic_temp(): the temperature
>    threshold is *subtracted* by the offset before being converted to a
>    hardware register value. This shifts the hardware IRQ trigger to the
>    physical temperature that corresponds to the intended threshold.
> 
> 2. In imx_get_temp(): after computing physical temperature from the
>    hardware register, the offset is *added* back. The thermal framework
>    always sees the calibrated temperature.
> 
> For example, if DT sets offset = +3000 m°C (board reads 3°C too low)
> and the passive trip is 95°C:
> 
>   imx_set_alarm_temp(95000):
>     alarm_temp = 95000 - 3000 = 92000
>     → hardware register programmed for 92°C physical
> 
>   Hardware IRQ fires at 92°C physical
> 
>   imx_get_temp():
>     reads hardware, computes 92°C physical
>     *temp = 92000 + 3000 = 95000
>     → thermal framework sees 95°C → correct trip
> 
> When the property is not present, the offset defaults to 0, preserving
> the current behavior.
> 
> Signed-off-by: Haoning CHENG <Haoning.CHENG@xxxxxxxxxxxx>
> ---
>  drivers/thermal/imx_thermal.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 7f7d1116b9d6..d471acc16bce 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -85,6 +85,10 @@ enum imx_thermal_trip {
>  #define TEMPMON_IMX6SX			2
>  #define TEMPMON_IMX7D			3
>  
> +/* Calibration offset limits (±20 °C in millicelsius) */
> +#define IMX_TEMP_CALIB_OFFSET_MIN	(-20000)
> +#define IMX_TEMP_CALIB_OFFSET_MAX	20000
> +
>  struct thermal_soc_data {
>  	u32 version;
>  
> @@ -207,6 +211,7 @@ struct imx_thermal_data {
>  	struct regmap *tempmon;
>  	u32 c1, c2; /* See formula in imx_init_calib() */
>  	int temp_max;
> +	s32 calibration_offset;
>  	int alarm_temp;
>  	int last_temp;
>  	bool irq_enabled;
> @@ -223,6 +228,7 @@ static void imx_set_panic_temp(struct imx_thermal_data *data,
>  	struct regmap *map = data->tempmon;
>  	int critical_value;
>  
> +	panic_temp -= data->calibration_offset;
>  	critical_value = (data->c2 - panic_temp) / data->c1;
>  
>  	regmap_write(map, soc_data->panic_alarm_ctrl + REG_CLR,
> @@ -239,6 +245,7 @@ static void imx_set_alarm_temp(struct imx_thermal_data *data,
>  	int alarm_value;
>  
>  	data->alarm_temp = alarm_temp;
> +	alarm_temp -= data->calibration_offset;
>  
>  	if (data->socdata->version == TEMPMON_IMX7D) {
>  		if (alarm_temp >= 0)
> @@ -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) {
> @@ -635,6 +643,25 @@ static int imx_thermal_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, data);
>  
> +	if (of_property_present(dev->of_node,
> +				"fsl,temp-calibration-offset-millicelsius")) {
> +		ret = of_property_read_s32(dev->of_node,
> +					   "fsl,temp-calibration-offset-millicelsius",
> +					   &data->calibration_offset);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "failed to read calibration offset\n");
> +
> +		if (data->calibration_offset < IMX_TEMP_CALIB_OFFSET_MIN ||
> +		    data->calibration_offset > IMX_TEMP_CALIB_OFFSET_MAX)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "calibration offset %d millicelsius out of range\n",
> +					     data->calibration_offset);
> +
> +		dev_dbg(dev, "calibration offset: %d millicelsius\n",
> +			data->calibration_offset);
> +	}
> +
>  	if (of_property_present(dev->of_node, "nvmem-cells")) {
>  		ret = imx_init_from_nvmem_cells(pdev);
>  		if (ret)
>