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

[Krishna Chomal <krishna.chomal108@gmail.com>]

On Sat, Mar 21, 2026 at 02:55:56AM +0300, 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.
>
>The fan table originates from an undocumented WMI interface; a
>firmware bug or unexpected layout could produce this condition.
>
>Change gpu_delta to int and perform the subtraction in signed
>arithmetic. On underflow, emit a warning and treat the delta as zero
>so the GPU fan tracks the CPU fan directly rather than saturating.
>
>Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
>---
> drivers/platform/x86/hp/hp-wmi.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
>index a29f34588055..b8f22d70e996 100644
>--- a/drivers/platform/x86/hp/hp-wmi.c
>+++ b/drivers/platform/x86/hp/hp-wmi.c
>@@ -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,10 +2553,15 @@ 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;
>+	if (gpu_delta < 0) {
>+		pr_warn("fan table has gpu_rpm < cpu_rpm, ignoring gpu delta\n");

Since some boards have CPU_RPM > GPU_RPM in their firmware tables, this
subtraction can naturally result in a negative value. I don't think
we should pr_warn() what is essentially intended hardware behavior.

>+		gpu_delta = 0;
>+	}

Instead of capping gpu_delta to 0, I suggest we update the definition
of 'gpu_delta' in struct hp_wmi_hwmon_priv to an 'int'.

Since hp_wmi_fan_speed_set() already performs the calculation using
signed integer arithmetic:

     gpu_speed = speed + priv->gpu_delta;
     fan_speed[GPU_FAN] = clamp_val(gpu_speed, 0, U8_MAX);

Using a signed 'gpu_delta' will automatically result in the correct 
GPU_RPM without the risk of it clamping at U8_MAX.

> 	priv->min_rpm = min_rpm;
> 	priv->max_rpm = max_rpm;
>-	priv->gpu_delta = gpu_delta;
>+	priv->gpu_delta = (u8)gpu_delta;
>
> 	return 0;
> }
>-- 
>2.53.0
>