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

Emre Cecanpunar posted 5 patches 1 day, 20 hours ago
[PATCH v3 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
Posted by Emre Cecanpunar 1 day, 20 hours ago
gpu_delta was declared as u8. If the firmware specifies a GPU RPM
lower than the CPU RPM, subtracting them causes an underflow
(e.g. 10 - 20 = 246), which forces the GPU fan to remain clamped at
U8_MAX (100% speed) during operation.

Change gpu_delta to int and use signed arithmetic. Existing signed logic
in hp_wmi_fan_speed_set() correctly handles negative deltas.

Fixes: 46be1453e6e6 ("platform/x86: hp-wmi: add manual fan control for Victus S models")
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
---
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.

Changes in v3:
- Removed unnecessary explicit (int) casts. C's integer promotion rules
  already convert u8 operands to int for arithmetic; the fix only
  requires storing the result in an int variable.

 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 2932cab9aa78..db72ad9da0a5 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -455,7 +455,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;
@@ -2549,8 +2549,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;
@@ -2572,7 +2572,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 = fan_table->entries[0].gpu_rpm -
+		    fan_table->entries[0].cpu_rpm;
 	priv->min_rpm = min_rpm;
 	priv->max_rpm = max_rpm;
 	priv->gpu_delta = gpu_delta;
-- 
2.53.0

Re: [PATCH v3 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
Posted by Ilpo Järvinen 3 hours ago
On Sun, 5 Apr 2026, Emre Cecanpunar wrote:

> gpu_delta was declared as u8. If the firmware specifies a GPU RPM
> lower than the CPU RPM, subtracting them causes an underflow
> (e.g. 10 - 20 = 246), which forces the GPU fan to remain clamped at
> U8_MAX (100% speed) during operation.
> 
> Change gpu_delta to int and use signed arithmetic. Existing signed logic
> in hp_wmi_fan_speed_set() correctly handles negative deltas.
> 
> Fixes: 46be1453e6e6 ("platform/x86: hp-wmi: add manual fan control for Victus S models")
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
> ---
> 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.
> 
> Changes in v3:
> - Removed unnecessary explicit (int) casts. C's integer promotion rules
>   already convert u8 operands to int for arithmetic; the fix only
>   requires storing the result in an int variable.
> 
>  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 2932cab9aa78..db72ad9da0a5 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -455,7 +455,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;
> @@ -2549,8 +2549,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;
> @@ -2572,7 +2572,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 = fan_table->entries[0].gpu_rpm -
> +		    fan_table->entries[0].cpu_rpm;

So this part has now become pure newline change (which is unrelated to 
the intent of this patch)?

>  	priv->min_rpm = min_rpm;
>  	priv->max_rpm = max_rpm;
>  	priv->gpu_delta = gpu_delta;
> 

-- 
 i.