Re: [PATCH v2 3/3] thermal: cv180x: Add cv180x thermal driver support

[Inochi Amaoto]

>On Fri, Jun 07, 2024 at 06:36:53AM +0800, Inochi Amaoto wrote:
>>> + *	Raw register value to temperature (mC) formula:
>>> + *
>>> + *		       read_val * 1000 * 716
>>> + *	Temperature = ----------------------- - 273000
>>> + *				divider
>>> + *
>>> + *	where divider should be ticks number of accumulation period,
>>> + *	e.g. 2048 for TEMPSEN_CTRL_ACCSEL_2048T
>>> + */
>>> +static int cv180x_calc_temp(struct cv180x_thermal_zone *ctz, u32 result)
>>> +{
>>> +	u32 divider = (u32)(512 * int_pow(2, ctz->accum_period));
>>> +
>>> +	return (result * 1000) * 716 / divider - 273000;
>>> +}
>>> +
>>
>> According to the staff from Sophgo, the original formula is just done 
>> by curve fitting. And the formula is not affected by any parameters.
>> Those parameters only affect timing.
>
> No, accumulation period does affect the raw register value of
> temperature. With the original formula where divider is a constant
> (2048), an accumulation period configuration differing from 2048T
> results in obviously wrong temperature.
>

Yes, you are right, I have confirmed. Thanks.

>> Although this is still uncertainty about how to calculate the temp,
>> I suspect you did not test your patch well, and the formula you wrote
>> is obviously broken. Please test your patch in all conditions.
>
> With the formula and different configuration, I get results with a range
> of about 1 Celsius degree, which should be enough for monitoring SoC
> status.

Excellect. I think it is pretty good.


And there are some extra suggestion for you:

Always reply e-mail in group reply mode, or others will lost your
reply.

Regards,
Inochi