Re: [PATCH v3 2/4] thermal: renesas: rzg3s: Add thermal driver for the Renesas RZ/G3S SoC

[Claudiu Beznea <claudiu.beznea@tuxon.dev>]

Hi, Niklas,

On 05.07.2025 15:06, Niklas Söderlund wrote:
> Hi Claudiu,
> 
> Thanks for your work. 
> 
> Sorry for late review, Geert only alerted me to the series a few days 
> ago.
> 
> On 2025-03-24 15:56:59 +0200, Claudiu wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> The Renesas RZ/G3S SoC features a Thermal Sensor Unit (TSU) that reports
>> the junction temperature. The temperature is reported through a dedicated
>> ADC channel. Add a driver for the Renesas RZ/G3S TSU.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>

[ ...]

>> +static int rzg3s_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
>> +{
>> +	struct rzg3s_thermal_priv *priv = thermal_zone_device_priv(tz);
>> +	int ts_code_ave = 0;
>> +	int ret, val;
>> +
>> +	if (priv->mode != THERMAL_DEVICE_ENABLED)
>> +		return -EAGAIN;
>> +
>> +	for (u8 i = 0; i < TSU_READ_STEPS; i++) {
>> +		ret = iio_read_channel_raw(priv->channel, &val);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		ts_code_ave += val;
>> +		/*
>> +		 * According to the HW manual (section 40.4.4 Procedure for Measuring the
>> +		 * Temperature) we need to wait here at leat 3us.
>> +		 */
>> +		usleep_range(5, 10);
>> +	}
>> +
>> +	ret = 0;
>> +	ts_code_ave = DIV_ROUND_CLOSEST(MCELSIUS(ts_code_ave), TSU_READ_STEPS);
>> +
>> +	/*
>> +	 * According to the HW manual (section 40.4.4 Procedure for Measuring the Temperature)
>> +	 * the computation formula is as follows:
>> +	 *
>> +	 * Tj = (ts_code_ave - priv->calib1) * 165 / (priv->calib0 - priv->calib1) - 40
>> +	 *
>> +	 * Convert everything to mili Celsius before applying the formula to avoid
>> +	 * losing precision.
>> +	 */
>> +
>> +	*temp = DIV_ROUND_CLOSEST((s64)(ts_code_ave - MCELSIUS(priv->calib1)) * MCELSIUS(165),
>> +				  MCELSIUS(priv->calib0 - priv->calib1)) - MCELSIUS(40);
> 
> The issue Geert points out, can that not be solved by holding off 
> converting to MCELSIUS() to after you have done the calculation?

This method works as well, but at the cost of some precision. As of my
experiments, with it there will be no temperatures with .5 Celsius
resolution (e.g., 50.5, 51.5, 52.5, etc) reported.

Thank you,
Claudiu