Re: [PATCH v2 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation

[Ilpo Järvinen]

On Sun, 22 Mar 2026, Emre Cecanpunar wrote:

> gpu_delta was declared as u8 and computed as the difference of two u8
> fan RPM values from the firmware fan table. If gpu_rpm < cpu_rpm, the
> subtraction wraps around modulo 256, producing a large positive value
> (e.g. 10 - 20 = 246 as u8). This value is then added to every
> requested fan speed in hp_wmi_fan_speed_set(), causing the GPU fan to
> be clamped to U8_MAX on almost every write.
> 
> Change gpu_delta to int in both struct hp_wmi_hwmon_priv and
> hp_wmi_setup_fan_settings(), and perform the subtraction in signed
> arithmetic. hp_wmi_fan_speed_set() already uses signed arithmetic with
> clamp_val(), so a negative delta correctly results in a lower GPU fan
> speed rather than saturating at U8_MAX.
> 
> Signed-off-by: Emre Cecanpunar <emreleno@xxxxxxxxx>
> ---
> Changes in v2:
> - Drop the if (gpu_delta < 0) guard and pr_warn. A negative delta is
>   valid firmware behavior on boards where CPU_RPM > GPU_RPM. Store
>   gpu_delta as int in struct hp_wmi_hwmon_priv so the existing signed
>   arithmetic and clamp_val() in hp_wmi_fan_speed_set() handle the
>   negative case correctly without saturating at U8_MAX.
> 
>  drivers/platform/x86/hp/hp-wmi.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index a29f34588055..af57cb0dbf9e 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -436,7 +436,7 @@ enum pwm_modes {
>  struct hp_wmi_hwmon_priv {
>  	u8 min_rpm;
>  	u8 max_rpm;
> -	u8 gpu_delta;
> +	int gpu_delta;
>  	u8 mode;
>  	u8 pwm;
>  	struct delayed_work keep_alive_dwork;
> @@ -2530,8 +2530,8 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  {
>  	u8 fan_data[128] = { 0 };
>  	struct victus_s_fan_table *fan_table;
> -	u8 min_rpm, max_rpm, gpu_delta;
> -	int ret;
> +	u8 min_rpm, max_rpm;
> +	int gpu_delta, ret;
> 
>  	/* Default behaviour on hwmon init is automatic mode */
>  	priv->mode = PWM_MODE_AUTO;
> @@ -2553,7 +2553,8 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
> 
>  	min_rpm = fan_table->entries[0].cpu_rpm;
>  	max_rpm = fan_table->entries[fan_table->header.num_entries - 1].cpu_rpm;
> -	gpu_delta = fan_table->entries[0].gpu_rpm - fan_table->entries[0].cpu_rpm;
> +	gpu_delta = (int)fan_table->entries[0].gpu_rpm -
> +		    (int)fan_table->entries[0].cpu_rpm;

Hi,

Doesn't C's implicit integer promotion rules already convert these to int 
prior to doing any arithmetic on them?

The underflow problem occurred only when storing that result back into u8 
value so the type change is still required.


-- 
 i.