public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes
@ 2026-03-22 19:06 Emre Cecanpunar
  2026-03-22 19:06 ` [PATCH v2 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-22 19:06 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hansg, ilpo.jarvinen, linux-kernel, krishna.chomal108,
	Emre Cecanpunar

This series fixes several bugs in the Victus S fan control code.

Changes in v2:
- Patch 4: 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 that the
  existing signed arithmetic and clamp_val() in hp_wmi_fan_speed_set()
  handle the negative case correctly without saturating at U8_MAX.

Emre Cecanpunar (5):
  platform/x86: hp-wmi: fix ignored return values in fan settings
  platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler
  platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer
  platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
  platform/x86: hp-wmi: add locking for concurrent hwmon access

 drivers/platform/x86/hp/hp-wmi.c | 55 +++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 18 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings
  2026-03-22 19:06 [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
@ 2026-03-22 19:06 ` Emre Cecanpunar
  2026-03-22 19:06 ` [PATCH v2 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler Emre Cecanpunar
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-22 19:06 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hansg, ilpo.jarvinen, linux-kernel, krishna.chomal108,
	Emre Cecanpunar

hp_wmi_get_fan_count_userdefine_trigger() can fail, but its return
value was silently discarded in the PWM_MODE_MAX and PWM_MODE_AUTO
cases of hp_wmi_apply_fan_settings(). The same function's return value
is already checked in hp_wmi_fan_speed_set(). Propagate errors
consistently.

hp_wmi_apply_fan_settings() itself returned an error code in all
paths, but its callers hp_wmi_hwmon_keep_alive_handler() and
hp_wmi_hwmon_init() both discarded the return value. Log failures via
pr_warn_ratelimited() in the keep-alive handler (rate-limited because
it fires every 90 seconds) and dev_warn() in init (non-fatal: the
hwmon device is already registered).

Also remove the unreachable "return 0" after the switch statement in
hp_wmi_apply_fan_settings(): every case branch already returns
explicitly.

Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 68ede7e5757a..0cb2a2b31998 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -2334,8 +2334,11 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 
 	switch (priv->mode) {
 	case PWM_MODE_MAX:
-		if (is_victus_s_thermal_profile())
-			hp_wmi_get_fan_count_userdefine_trigger();
+		if (is_victus_s_thermal_profile()) {
+			ret = hp_wmi_get_fan_count_userdefine_trigger();
+			if (ret < 0)
+				return ret;
+		}
 		ret = hp_wmi_fan_speed_max_set(1);
 		if (ret < 0)
 			return ret;
@@ -2353,7 +2356,9 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 		return 0;
 	case PWM_MODE_AUTO:
 		if (is_victus_s_thermal_profile()) {
-			hp_wmi_get_fan_count_userdefine_trigger();
+			ret = hp_wmi_get_fan_count_userdefine_trigger();
+			if (ret < 0)
+				return ret;
 			ret = hp_wmi_fan_speed_max_reset(priv);
 		} else {
 			ret = hp_wmi_fan_speed_max_set(0);
@@ -2366,8 +2371,6 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 		/* shouldn't happen */
 		return -EINVAL;
 	}
-
-	return 0;
 }
 
 static umode_t hp_wmi_hwmon_is_visible(const void *data,
@@ -2509,6 +2512,7 @@ static void hp_wmi_hwmon_keep_alive_handler(struct work_struct *work)
 {
 	struct delayed_work *dwork;
 	struct hp_wmi_hwmon_priv *priv;
+	int ret;
 
 	dwork = to_delayed_work(work);
 	priv = container_of(dwork, struct hp_wmi_hwmon_priv, keep_alive_dwork);
@@ -2516,7 +2520,10 @@ static void hp_wmi_hwmon_keep_alive_handler(struct work_struct *work)
 	 * Re-apply the current hwmon context settings.
 	 * NOTE: hp_wmi_apply_fan_settings will handle the re-scheduling.
 	 */
-	hp_wmi_apply_fan_settings(priv);
+	ret = hp_wmi_apply_fan_settings(priv);
+	if (ret)
+		pr_warn_ratelimited("keep-alive failed to refresh fan settings: %d\n",
+				    ret);
 }
 
 static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
@@ -2578,7 +2585,9 @@ static int hp_wmi_hwmon_init(void)
 
 	INIT_DELAYED_WORK(&priv->keep_alive_dwork, hp_wmi_hwmon_keep_alive_handler);
 	platform_set_drvdata(hp_wmi_platform_dev, priv);
-	hp_wmi_apply_fan_settings(priv);
+	ret = hp_wmi_apply_fan_settings(priv);
+	if (ret)
+		dev_warn(dev, "Failed to apply initial fan settings: %d\n", ret);
 
 	return 0;
 }
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler
  2026-03-22 19:06 [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
  2026-03-22 19:06 ` [PATCH v2 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
@ 2026-03-22 19:06 ` Emre Cecanpunar
  2026-03-22 19:06 ` [PATCH v2 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer Emre Cecanpunar
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-22 19:06 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hansg, ilpo.jarvinen, linux-kernel, krishna.chomal108,
	Emre Cecanpunar

hp_wmi_apply_fan_settings() calls cancel_delayed_work_sync() in the
PWM_MODE_AUTO case to stop the keep-alive timer. This function is also
called from hp_wmi_hwmon_keep_alive_handler(), which is itself a
delayed work handler.

If the keep-alive work is executing and the fan mode happens to be
AUTO (e.g. a concurrent sysfs write changed it just before the handler
read priv->mode), cancel_delayed_work_sync() tries to flush the work
item it is already running inside, causing a deadlock.

The workqueue documentation explicitly requires callers to ensure this
self-flush situation does not arise.

Replace cancel_delayed_work_sync() with cancel_delayed_work() in
hp_wmi_apply_fan_settings(). The non-synchronous variant cancels
pending work without waiting for a running instance to complete,
avoiding the deadlock. The synchronous cancel in hp_wmi_bios_remove()
is unaffected and still provides the final cleanup guarantee at driver
removal time.

Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 0cb2a2b31998..41769c0861e5 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -2365,7 +2365,7 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 		}
 		if (ret < 0)
 			return ret;
-		cancel_delayed_work_sync(&priv->keep_alive_dwork);
+		cancel_delayed_work(&priv->keep_alive_dwork);
 		return 0;
 	default:
 		/* shouldn't happen */
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer
  2026-03-22 19:06 [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
  2026-03-22 19:06 ` [PATCH v2 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
  2026-03-22 19:06 ` [PATCH v2 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler Emre Cecanpunar
@ 2026-03-22 19:06 ` Emre Cecanpunar
  2026-03-23  9:51   ` Ilpo Järvinen
  2026-03-22 19:06 ` [PATCH v2 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-22 19:06 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hansg, ilpo.jarvinen, linux-kernel, krishna.chomal108,
	Emre Cecanpunar

The keep-alive mechanism re-applies fan settings every 90 seconds to
prevent the firmware from reverting to automatic mode after its 120 s
timeout. Fan settings are also applied immediately when the user writes
to the hwmon sysfs interface.

schedule_delayed_work() is a no-op when the work item is already
pending. If the user updates fan settings at T=85s, the keep-alive
still fires at T=90s (5 s after the update) instead of 90 s later.
This shortens the effective keep-alive window and may allow the
firmware timeout to expire between cycles.

Replace schedule_delayed_work() with mod_delayed_work() in both the
PWM_MODE_MAX and PWM_MODE_MANUAL cases. mod_delayed_work() resets the
delay to KEEP_ALIVE_DELAY_SECS from the time of each call, whether or
not the work is already pending, ensuring each user action and each
keep-alive expiry restarts the full 90 s window.

Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 41769c0861e5..a29f34588055 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -2342,8 +2342,8 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 		ret = hp_wmi_fan_speed_max_set(1);
 		if (ret < 0)
 			return ret;
-		schedule_delayed_work(&priv->keep_alive_dwork,
-				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
+		mod_delayed_work(system_wq, &priv->keep_alive_dwork,
+				 secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
 		return 0;
 	case PWM_MODE_MANUAL:
 		if (!is_victus_s_thermal_profile())
@@ -2351,8 +2351,8 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
 		if (ret < 0)
 			return ret;
-		schedule_delayed_work(&priv->keep_alive_dwork,
-				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
+		mod_delayed_work(system_wq, &priv->keep_alive_dwork,
+				 secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
 		return 0;
 	case PWM_MODE_AUTO:
 		if (is_victus_s_thermal_profile()) {
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
  2026-03-22 19:06 [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
                   ` (2 preceding siblings ...)
  2026-03-22 19:06 ` [PATCH v2 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer Emre Cecanpunar
@ 2026-03-22 19:06 ` Emre Cecanpunar
  2026-03-23  9:42   ` Ilpo Järvinen
  2026-03-22 19:06 ` [PATCH v2 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
  2026-03-24 15:46 ` [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Ilpo Järvinen
  5 siblings, 1 reply; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-22 19:06 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hansg, ilpo.jarvinen, linux-kernel, krishna.chomal108,
	Emre Cecanpunar

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@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.

 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;
 	priv->min_rpm = min_rpm;
 	priv->max_rpm = max_rpm;
 	priv->gpu_delta = gpu_delta;
--
2.53.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access
  2026-03-22 19:06 [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
                   ` (3 preceding siblings ...)
  2026-03-22 19:06 ` [PATCH v2 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
@ 2026-03-22 19:06 ` Emre Cecanpunar
  2026-03-24 15:46 ` [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Ilpo Järvinen
  5 siblings, 0 replies; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-22 19:06 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hansg, ilpo.jarvinen, linux-kernel, krishna.chomal108,
	Emre Cecanpunar

hp_wmi_hwmon_priv.mode and .pwm are written by hp_wmi_hwmon_write()
in sysfs context and read by hp_wmi_hwmon_keep_alive_handler() running
in a workqueue. There is no synchronisation between them, so a
concurrent sysfs write and keep-alive expiry can observe an
inconsistent combination of mode and pwm (e.g. mode=MANUAL with an
stale pwm value from the previous mode).

Add a mutex to hp_wmi_hwmon_priv that protects mode and pwm. Acquire
it in hp_wmi_hwmon_write() for the full write operation (field update
plus hp_wmi_apply_fan_settings() call), and in
hp_wmi_hwmon_keep_alive_handler() before calling
hp_wmi_apply_fan_settings().

In hp_wmi_hwmon_read(), only the pwm_enable attribute reads priv->mode
and requires the lock. Use scoped_guard() to hold the mutex only for
the mode read, avoiding holding it across unrelated WMI hardware calls.

Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index af57cb0dbf9e..0947aaeabff3 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -434,6 +434,7 @@ enum pwm_modes {
 };
 
 struct hp_wmi_hwmon_priv {
+	struct mutex lock;	/* protects mode, pwm */
 	u8 min_rpm;
 	u8 max_rpm;
 	int gpu_delta;
@@ -2403,6 +2404,7 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 {
 	struct hp_wmi_hwmon_priv *priv;
 	int rpm, ret;
+	u8 mode;
 
 	priv = dev_get_drvdata(dev);
 	switch (type) {
@@ -2426,11 +2428,13 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 			*val = rpm_to_pwm(rpm / 100, priv);
 			return 0;
 		}
-		switch (priv->mode) {
+		scoped_guard(mutex, &priv->lock)
+			mode = priv->mode;
+		switch (mode) {
 		case PWM_MODE_MAX:
 		case PWM_MODE_MANUAL:
 		case PWM_MODE_AUTO:
-			*val = priv->mode;
+			*val = mode;
 			return 0;
 		default:
 			/* shouldn't happen */
@@ -2448,6 +2452,7 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 	int rpm;
 
 	priv = dev_get_drvdata(dev);
+	guard(mutex)(&priv->lock);
 	switch (type) {
 	case hwmon_pwm:
 		if (attr == hwmon_pwm_input) {
@@ -2516,6 +2521,8 @@ static void hp_wmi_hwmon_keep_alive_handler(struct work_struct *work)
 
 	dwork = to_delayed_work(work);
 	priv = container_of(dwork, struct hp_wmi_hwmon_priv, keep_alive_dwork);
+
+	guard(mutex)(&priv->lock);
 	/*
 	 * Re-apply the current hwmon context settings.
 	 * NOTE: hp_wmi_apply_fan_settings will handle the re-scheduling.
@@ -2573,6 +2580,8 @@ static int hp_wmi_hwmon_init(void)
 	if (!priv)
 		return -ENOMEM;
 
+	mutex_init(&priv->lock);
+
 	ret = hp_wmi_setup_fan_settings(priv);
 	if (ret)
 		return ret;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
  2026-03-22 19:06 ` [PATCH v2 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
@ 2026-03-23  9:42   ` Ilpo Järvinen
  0 siblings, 0 replies; 9+ messages in thread
From: Ilpo Järvinen @ 2026-03-23  9:42 UTC (permalink / raw)
  To: Emre Cecanpunar
  Cc: platform-driver-x86, Hans de Goede, LKML, krishna.chomal108

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@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.
> 
>  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.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer
  2026-03-22 19:06 ` [PATCH v2 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer Emre Cecanpunar
@ 2026-03-23  9:51   ` Ilpo Järvinen
  0 siblings, 0 replies; 9+ messages in thread
From: Ilpo Järvinen @ 2026-03-23  9:51 UTC (permalink / raw)
  To: Emre Cecanpunar
  Cc: platform-driver-x86, Hans de Goede, LKML, krishna.chomal108

On Sun, 22 Mar 2026, Emre Cecanpunar wrote:

> The keep-alive mechanism re-applies fan settings every 90 seconds to
> prevent the firmware from reverting to automatic mode after its 120 s
> timeout. Fan settings are also applied immediately when the user writes
> to the hwmon sysfs interface.
> 
> schedule_delayed_work() is a no-op when the work item is already
> pending. If the user updates fan settings at T=85s, the keep-alive
> still fires at T=90s (5 s after the update) instead of 90 s later.
> This shortens the effective keep-alive window and may allow the
> firmware timeout to expire between cycles.

Hi,

Unfortunately, I don't understand this explanation.

Won't the keepalive expiry at T=90s just result in adding the next 
keepaline event to T=90s + 90s, and the resulting intervals never exceeds 
120s?

It may be inefficient to apply fan settings at T=85s and then fire 
keepalive at T=90s, that much I can see but if you think there's a problem 
beyond that, this explanation doesn't convince me so please elaborate.

If the only problem is inefficiency, this explanation did not convey 
that meaning so in that case, please rephrase.

> Replace schedule_delayed_work() with mod_delayed_work() in both the
> PWM_MODE_MAX and PWM_MODE_MANUAL cases. mod_delayed_work() resets the
> delay to KEEP_ALIVE_DELAY_SECS from the time of each call, whether or
> not the work is already pending, ensuring each user action and each
> keep-alive expiry restarts the full 90 s window.
> 
> Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 41769c0861e5..a29f34588055 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -2342,8 +2342,8 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  		ret = hp_wmi_fan_speed_max_set(1);
>  		if (ret < 0)
>  			return ret;
> -		schedule_delayed_work(&priv->keep_alive_dwork,
> -				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
> +		mod_delayed_work(system_wq, &priv->keep_alive_dwork,
> +				 secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
>  		return 0;
>  	case PWM_MODE_MANUAL:
>  		if (!is_victus_s_thermal_profile())
> @@ -2351,8 +2351,8 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
>  		if (ret < 0)
>  			return ret;
> -		schedule_delayed_work(&priv->keep_alive_dwork,
> -				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
> +		mod_delayed_work(system_wq, &priv->keep_alive_dwork,
> +				 secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
>  		return 0;
>  	case PWM_MODE_AUTO:
>  		if (is_victus_s_thermal_profile()) {
> 

-- 
 i.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes
  2026-03-22 19:06 [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
                   ` (4 preceding siblings ...)
  2026-03-22 19:06 ` [PATCH v2 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
@ 2026-03-24 15:46 ` Ilpo Järvinen
  5 siblings, 0 replies; 9+ messages in thread
From: Ilpo Järvinen @ 2026-03-24 15:46 UTC (permalink / raw)
  To: Emre Cecanpunar
  Cc: platform-driver-x86, Hans de Goede, LKML, krishna.chomal108

On Sun, 22 Mar 2026, Emre Cecanpunar wrote:

> This series fixes several bugs in the Victus S fan control code.
> 
> Changes in v2:
> - Patch 4: 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 that the
>   existing signed arithmetic and clamp_val() in hp_wmi_fan_speed_set()
>   handle the negative case correctly without saturating at U8_MAX.
> 
> Emre Cecanpunar (5):
>   platform/x86: hp-wmi: fix ignored return values in fan settings
>   platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler
>   platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer
>   platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
>   platform/x86: hp-wmi: add locking for concurrent hwmon access

You should all so add Fixes tags to these changes, they're currently 
missing from all of them.

-- 
 i.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-03-24 15:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-22 19:06 [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
2026-03-22 19:06 ` [PATCH v2 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
2026-03-22 19:06 ` [PATCH v2 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler Emre Cecanpunar
2026-03-22 19:06 ` [PATCH v2 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer Emre Cecanpunar
2026-03-23  9:51   ` Ilpo Järvinen
2026-03-22 19:06 ` [PATCH v2 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
2026-03-23  9:42   ` Ilpo Järvinen
2026-03-22 19:06 ` [PATCH v2 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
2026-03-24 15:46 ` [PATCH v2 0/5] platform/x86: hp-wmi: Victus S fan control fixes Ilpo Järvinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox