* [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control
@ 2026-03-20 23:54 Emre Cecanpunar
0 siblings, 0 replies; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-20 23:54 UTC (permalink / raw)
To: platform-driver-x86
Cc: hansg, ilpo.jarvinen, krishna.chomal108, linux-kernel,
Emre Cecanpunar
This series fixes five bugs in the hwmon fan control code added for
HP Victus S laptops:
- Return values from WMI calls were silently discarded in two paths
of hp_wmi_apply_fan_settings(), and the function's own return value
was ignored by both of its callers.
- cancel_delayed_work_sync() was called from inside the work handler
it was trying to cancel, causing a potential deadlock.
- schedule_delayed_work() does not reset a pending timer, so rapid
back-to-back fan updates did not extend the keep-alive window.
- u8 arithmetic was used to compute a signed delta, producing silent
unsigned wrap-around on malformed firmware fan tables.
- mode and pwm fields in hp_wmi_hwmon_priv were accessed from both
sysfs and workqueue context without any synchronisation.
Patches are ordered so that each one applies cleanly on top of the
previous. Patches 1-4 are independent fixes; patch 5 (locking) builds
on patch 2 (cancel_delayed_work) to avoid a lock-ordering issue.
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 | 59 ++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 18 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control
@ 2026-03-20 23:55 Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 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-20 23:55 UTC (permalink / raw)
To: platform-driver-x86
Cc: hansg, ilpo.jarvinen, krishna.chomal108, linux-kernel,
Emre Cecanpunar
This series fixes five bugs in the hwmon fan control code added for
HP Victus S laptops:
- Return values from WMI calls were silently discarded in two paths
of hp_wmi_apply_fan_settings(), and the function's own return value
was ignored by both of its callers.
- cancel_delayed_work_sync() was called from inside the work handler
it was trying to cancel, causing a potential deadlock.
- schedule_delayed_work() does not reset a pending timer, so rapid
back-to-back fan updates did not extend the keep-alive window.
- u8 arithmetic was used to compute a signed delta, producing silent
unsigned wrap-around on malformed firmware fan tables.
- mode and pwm fields in hp_wmi_hwmon_priv were accessed from both
sysfs and workqueue context without any synchronisation.
Patches are ordered so that each one applies cleanly on top of the
previous. Patches 1-4 are independent fixes; patch 5 (locking) builds
on patch 2 (cancel_delayed_work) to avoid a lock-ordering issue.
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 | 59 ++++++++++++++++++++++----------
1 file changed, 41 insertions(+), 18 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings
2026-03-20 23:55 [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Emre Cecanpunar
@ 2026-03-20 23:55 ` Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 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-20 23:55 UTC (permalink / raw)
To: platform-driver-x86
Cc: hansg, ilpo.jarvinen, krishna.chomal108, linux-kernel,
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 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler
2026-03-20 23:55 [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
@ 2026-03-20 23:55 ` Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 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-20 23:55 UTC (permalink / raw)
To: platform-driver-x86
Cc: hansg, ilpo.jarvinen, krishna.chomal108, linux-kernel,
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 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer
2026-03-20 23:55 [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler Emre Cecanpunar
@ 2026-03-20 23:55 ` Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-20 23:55 UTC (permalink / raw)
To: platform-driver-x86
Cc: hansg, ilpo.jarvinen, krishna.chomal108, linux-kernel,
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 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
2026-03-20 23:55 [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Emre Cecanpunar
` (2 preceding siblings ...)
2026-03-20 23:55 ` [PATCH 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer Emre Cecanpunar
@ 2026-03-20 23:55 ` Emre Cecanpunar
2026-03-22 17:14 ` Krishna Chomal
2026-03-20 23:55 ` [PATCH 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
2026-03-22 16:27 ` [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Krishna Chomal
5 siblings, 1 reply; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-20 23:55 UTC (permalink / raw)
To: platform-driver-x86
Cc: hansg, ilpo.jarvinen, krishna.chomal108, linux-kernel,
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.
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");
+ gpu_delta = 0;
+ }
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access
2026-03-20 23:55 [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Emre Cecanpunar
` (3 preceding siblings ...)
2026-03-20 23:55 ` [PATCH 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
@ 2026-03-20 23:55 ` Emre Cecanpunar
2026-03-22 16:27 ` [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Krishna Chomal
5 siblings, 0 replies; 9+ messages in thread
From: Emre Cecanpunar @ 2026-03-20 23:55 UTC (permalink / raw)
To: platform-driver-x86
Cc: hansg, ilpo.jarvinen, krishna.chomal108, linux-kernel,
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 b8f22d70e996..cfb207f09e7b 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;
u8 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.
@@ -2577,6 +2584,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 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control
2026-03-20 23:55 [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Emre Cecanpunar
` (4 preceding siblings ...)
2026-03-20 23:55 ` [PATCH 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
@ 2026-03-22 16:27 ` Krishna Chomal
5 siblings, 0 replies; 9+ messages in thread
From: Krishna Chomal @ 2026-03-22 16:27 UTC (permalink / raw)
To: Emre Cecanpunar; +Cc: hansg, ilpo.jarvinen, linux-kernel, platform-driver-x86
On Sat, Mar 21, 2026 at 02:55:52AM +0300, Emre Cecanpunar wrote:
>This series fixes five bugs in the hwmon fan control code added for
>HP Victus S laptops:
>
>- Return values from WMI calls were silently discarded in two paths
> of hp_wmi_apply_fan_settings(), and the function's own return value
> was ignored by both of its callers.
>- cancel_delayed_work_sync() was called from inside the work handler
> it was trying to cancel, causing a potential deadlock.
>- schedule_delayed_work() does not reset a pending timer, so rapid
> back-to-back fan updates did not extend the keep-alive window.
>- u8 arithmetic was used to compute a signed delta, producing silent
> unsigned wrap-around on malformed firmware fan tables.
>- mode and pwm fields in hp_wmi_hwmon_priv were accessed from both
> sysfs and workqueue context without any synchronisation.
>
>Patches are ordered so that each one applies cleanly on top of the
>previous. Patches 1-4 are independent fixes; patch 5 (locking) builds
>on patch 2 (cancel_delayed_work) to avoid a lock-ordering issue.
>
>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 | 59 ++++++++++++++++++++++----------
> 1 file changed, 41 insertions(+), 18 deletions(-)
>
>--
>2.53.0
>
Hi Emre,
Thank you for reviewing and catching these bugs. As this was my first
attempt with delayed workqueues, I appreciate the bug fixes, specially
regarding the deadlock.
I have tested this series on my hardware (board ID: 8C78) and can
confirm that the fans still behave correctly with the improvements.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
2026-03-20 23:55 ` [PATCH 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
@ 2026-03-22 17:14 ` Krishna Chomal
0 siblings, 0 replies; 9+ messages in thread
From: Krishna Chomal @ 2026-03-22 17:14 UTC (permalink / raw)
To: Emre Cecanpunar; +Cc: hansg, ilpo.jarvinen, linux-kernel, platform-driver-x86
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-03-22 17:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20 23:55 [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer Emre Cecanpunar
2026-03-20 23:55 ` [PATCH 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
2026-03-22 17:14 ` Krishna Chomal
2026-03-20 23:55 ` [PATCH 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
2026-03-22 16:27 ` [PATCH 0/5] platform/x86: hp-wmi: fix bugs in Victus S fan control Krishna Chomal
-- strict thread matches above, loose matches on Subject: below --
2026-03-20 23:54 Emre Cecanpunar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox