Re: [PATCH] cpufreq: amd_freq_sensitivity: Fix sensitivity clamping in amd_powersave_bias_target

[Thorsten Blum <thorsten.blum@linux.dev>]

On 19. Dec 2025, at 16:55, Thorsten Blum wrote:
> On 2. Dec 2025, at 20:09, David Laight wrote:
>> On Tue,  2 Dec 2025 13:44:28 +0100
>> Thorsten Blum <thorsten.blum@linux.dev> wrote:
>> 
>>> The local variable 'sensitivity' was never clamped to 0 or
>>> POWERSAVE_BIAS_MAX because the return value of clamp() was not used. Fix
>>> this by assigning the clamped value back to 'sensitivity'.
>> 
>> This actually makes no difference
>> (assuming od_tuners->powersave_bias <= POWERSAVE_BIAS_MAX).
>> The only use of 'sensitivity' is the test at the end of the diff.
>> 
>> So I think you could just delete the line.
> 
> The local variable 'sensitivity' is an 'int', while '->powersave_bias'
> is an 'unsigned int'. If 'sensitivity' were ever negative, it would be
> converted to an 'unsigned int', producing an incorrect result. That's
> probably what the clamping was meant to prevent.
> 
> However, calculating 'sensitivity' can be simplified from:
> 
> 	sensitivity = POWERSAVE_BIAS_MAX -
> 		(POWERSAVE_BIAS_MAX * (d_reference - d_actual) / d_reference);
> 
> to:
> 
> 	sensitivity = POWERSAVE_BIAS_MAX * d_actual / d_reference;
> 
> which makes it clearer that, in practice, 'sensitivity' is never
> negative. How about simplifying the formula as above, changing
> 'sensitivity' to 'unsigned int', and removing the clamping?

Hm, changing the formula could alter the integer arithmetic, potentially
producing slightly different results, and might even overflow.

I guess we should keep the formula as is and either defensively clamp
'sensitivity', as originally intended, or just remove the line (since
this seems to have been working since 2013 and as suggested by David).

I'm slightly in favor of clamping the value because the assumptions
aren't obvious to me.

Any preferences or other suggestions?

Thanks,
Thorsten