Re: [PATCH 5/9] thermal/drivers/mediatek/lvts: Add lvts_temp_to_raw variant for positive temp_factor

[Laura Nao <laura.nao@collabora.com>]

On 7/21/25 13:12, Chen-Yu Tsai wrote:
> On Mon, Jul 21, 2025 at 4:31 PM Laura Nao <laura.nao@collabora.com> wrote:
>>
>> The current lvts_temp_to_raw() implementation assumes a negative
>> temperature-to-raw slope (temp_factor), which holds for the SoCs
>> currently supported by the driver. However, this assumption breaks on
>> MT8196/MT6991, where the slope is positive.
>
> I don't think that's really a problem. The formula is:
>
>     temp = (raw * factor) >> 14 + golden
>
> If we move the terms around we get
>
>     ((temp - golden) << 14) / factor = raw
>
> Or
>
>     raw = ((golden - temp) << 14) / -factor
>
>
> The calculations should work regardless of whether the factor is positive
> or negative, as long as the intermediate and final values are within
> the range of s64.
>
>> Add a variant of the function that inverts the calculation logic
>> accordingly. This ensures accurate raw value generation for temperature
>> thresholds,avoiding spurious thermal interrupts or unintended hardware
>> resets on MT8196/MT6991.
>>
>> Signed-off-by: Laura Nao <laura.nao@collabora.com>
>> ---
>>  drivers/thermal/mediatek/lvts_thermal.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
>> index db83137c7537..3c34956e37c1 100644
>> --- a/drivers/thermal/mediatek/lvts_thermal.c
>> +++ b/drivers/thermal/mediatek/lvts_thermal.c
>> @@ -296,6 +296,18 @@ static u32 lvts_temp_to_raw(int temperature, int temp_factor)
>>         return div_s64(raw_temp, -temp_factor);
>>  }
>>
>> +static u32 lvts_temp_to_raw_v2(int temperature, int temp_factor)
>> +{
>> +       u32 raw_temp;
>> +
>> +       if (temp_factor == 0)
>> +               return temperature;
>> +
>> +       raw_temp = temperature - golden_temp_offset;
>> +
>> +       return div_s64((s64)temp_factor << 14, raw_temp);
>> +}
>
> Here you have
>
>     raw = (factor << 14) / (temp - golden)
>
> which, barring integer arithmetic limitations, is actually the
> multiplicative inverse of the original version.
>
> So I think the commit message is misleading. It's not negative or
> positive that matters, but that the hardware expects the
> multiplicative inverse in this version.
>
> (or the downstream code is just botched.)
>

Right - the positive temp_factor on MT8196 is one distinction from older 
SoCs, and I initially assumed that was the reason a different conversion 
formula was needed. That assumption was partly based on the original 
lvts_temp_to_raw() implementation, which assigns 
((s64)(golden_temp_offset - temperature) << 14) to a u32, losing the 
sign when golden_temp_offset is negative. However, even correcting that 
to preserve the sign doesn't make the function work on MT8196 as it 
still requires using the multiplicative inverse form of the formula. 

I'll reword the commit message to clarify this.

Thanks!

Laura

> ChenYu
>
>> +
>>  static int lvts_get_temp(struct thermal_zone_device *tz, int *temp)
>>  {
>>         struct lvts_sensor *lvts_sensor = thermal_zone_device_priv(tz);
>> --
>> 2.39.5
>>
>>