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

Bug fixes for the Victus S manual fan control:
- Propagate fan_count_userdefine_trigger() errors
- Fix cancel_delayed_work_sync() deadlock from work handler
- Use mod_delayed_work() for efficient keep-alive timer resets
- Fix u8 underflow in gpu_delta calculation
- Add mutex for concurrent hwmon state access

Changes in v2:
- Patch 4: Store gpu_delta as 'int' to handle negative deltas correctly.

Changes in v3:
- Add Fixes: tags to all patches.
- Patch 4: Remove unnecessary casting.

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] 8+ messages in thread

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

hp_wmi_get_fan_count_userdefine_trigger() can fail, but its return
value was silently ignored in hp_wmi_apply_fan_settings() for
PWM_MODE_MAX/AUTO. Propagate these errors consistently.

Additionally, handle the return value of hp_wmi_apply_fan_settings()
in its callers by adding appropriate warnings on failure, and remove an
unreachable "return 0" at the end of the function.

Fixes: 46be1453e6e6 ("platform/x86: hp-wmi: add manual fan control for Victus S models")
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 988a0acc9622..eac39f68d762 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -2353,8 +2353,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;
@@ -2372,7 +2375,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);
@@ -2385,8 +2390,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,
@@ -2528,6 +2531,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);
@@ -2535,7 +2539,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)
@@ -2597,7 +2604,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] 8+ messages in thread

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

hp_wmi_apply_fan_settings() uses cancel_delayed_work_sync() to stop
the keep-alive timer in AUTO mode. However, since
hp_wmi_apply_fan_settings() is also called from the keep-alive
handler, a race condition with a sysfs write can cause the handler to
wait on itself, leading to a deadlock.

Replace cancel_delayed_work_sync() with cancel_delayed_work() in
hp_wmi_apply_fan_settings() to avoid the self-flush deadlock.

Fixes: c203c59fb5de ("platform/x86: hp-wmi: implement fan keep-alive")
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 eac39f68d762..79d6bc3cd223 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -2384,7 +2384,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] 8+ messages in thread

* [PATCH v3 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer
  2026-04-05 14:44 [PATCH v3 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
  2026-04-05 14:44 ` [PATCH v3 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
  2026-04-05 14:44 ` [PATCH v3 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler Emre Cecanpunar
@ 2026-04-05 14:44 ` Emre Cecanpunar
  2026-04-05 14:44 ` [PATCH v3 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
  2026-04-05 14:45 ` [PATCH v3 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
  4 siblings, 0 replies; 8+ messages in thread
From: Emre Cecanpunar @ 2026-04-05 14:44 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hansg, ilpo.jarvinen, linux-kernel, krishna.chomal108, edip,
	emreleno

Currently, schedule_delayed_work() is used to queue the 90s keep-alive
timer. If a user manually changes the fan speed at T=85s,
schedule_delayed_work() leaves the existing timer in place as it is a
no-op if the work is already pending. This results in the keep-alive
timer firing unnecessarily at T=90s, just 5 seconds after the user
action.

Replace schedule_delayed_work() with mod_delayed_work() to reset the
90s timer whenever fan settings are applied. This guarantees a full 90s
delay after every user interaction, preventing redundant keep-alive
executions and improving efficiency.

Fixes: c203c59fb5de ("platform/x86: hp-wmi: implement fan keep-alive")
Signed-off-by: Emre Cecanpunar <emreleno@gmail.com>
---
Changes in v3:
- Reworded commit message to clarify this is an inefficiency fix, not
  a functional bug.

 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 79d6bc3cd223..2932cab9aa78 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -2361,8 +2361,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())
@@ -2370,8 +2370,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] 8+ messages in thread

* [PATCH v3 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
  2026-04-05 14:44 [PATCH v3 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
                   ` (2 preceding siblings ...)
  2026-04-05 14:44 ` [PATCH v3 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer Emre Cecanpunar
@ 2026-04-05 14:44 ` Emre Cecanpunar
  2026-04-07  8:02   ` Ilpo Järvinen
  2026-04-05 14:45 ` [PATCH v3 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
  4 siblings, 1 reply; 8+ messages in thread
From: Emre Cecanpunar @ 2026-04-05 14:44 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hansg, ilpo.jarvinen, linux-kernel, krishna.chomal108, edip,
	emreleno

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


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

* [PATCH v3 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access
  2026-04-05 14:44 [PATCH v3 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
                   ` (3 preceding siblings ...)
  2026-04-05 14:44 ` [PATCH v3 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
@ 2026-04-05 14:45 ` Emre Cecanpunar
  2026-04-07  8:07   ` Ilpo Järvinen
  4 siblings, 1 reply; 8+ messages in thread
From: Emre Cecanpunar @ 2026-04-05 14:45 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: hansg, ilpo.jarvinen, linux-kernel, krishna.chomal108, edip,
	emreleno

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() in a
workqueue. A concurrent write and keep-alive expiry can observe an
inconsistent mode/pwm pair (e.g. mode=MANUAL with a stale pwm).

Add a mutex to hp_wmi_hwmon_priv protecting mode and pwm. Hold it in
hp_wmi_hwmon_write() across the field update and apply call, and in
hp_wmi_hwmon_keep_alive_handler() before calling apply.

In hp_wmi_hwmon_read(), only the pwm_enable path reads priv->mode; use
scoped_guard() there to avoid holding the lock across unrelated WMI
calls.

Fixes: c203c59fb5de ("platform/x86: hp-wmi: implement fan keep-alive")
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 db72ad9da0a5..1096fb46cbcd 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -453,6 +453,7 @@ enum pwm_modes {
 };
 
 struct hp_wmi_hwmon_priv {
+	struct mutex lock;	/* protects mode, pwm */
 	u8 min_rpm;
 	u8 max_rpm;
 	int gpu_delta;
@@ -2422,6 +2423,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) {
@@ -2445,11 +2447,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 */
@@ -2467,6 +2471,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) {
@@ -2535,6 +2540,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.
@@ -2592,6 +2599,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] 8+ messages in thread

* Re: [PATCH v3 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation
  2026-04-05 14:44 ` [PATCH v3 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
@ 2026-04-07  8:02   ` Ilpo Järvinen
  0 siblings, 0 replies; 8+ messages in thread
From: Ilpo Järvinen @ 2026-04-07  8:02 UTC (permalink / raw)
  To: Emre Cecanpunar
  Cc: platform-driver-x86, Hans de Goede, LKML, krishna.chomal108, edip

[-- Attachment #1: Type: text/plain, Size: 2756 bytes --]

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.

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

* Re: [PATCH v3 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access
  2026-04-05 14:45 ` [PATCH v3 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
@ 2026-04-07  8:07   ` Ilpo Järvinen
  0 siblings, 0 replies; 8+ messages in thread
From: Ilpo Järvinen @ 2026-04-07  8:07 UTC (permalink / raw)
  To: Emre Cecanpunar
  Cc: platform-driver-x86, Hans de Goede, LKML, krishna.chomal108, edip

On Sun, 5 Apr 2026, Emre Cecanpunar wrote:

> 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() in a
> workqueue. A concurrent write and keep-alive expiry can observe an
> inconsistent mode/pwm pair (e.g. mode=MANUAL with a stale pwm).
> 
> Add a mutex to hp_wmi_hwmon_priv protecting mode and pwm. Hold it in
> hp_wmi_hwmon_write() across the field update and apply call, and in
> hp_wmi_hwmon_keep_alive_handler() before calling apply.
> 
> In hp_wmi_hwmon_read(), only the pwm_enable path reads priv->mode; use
> scoped_guard() there to avoid holding the lock across unrelated WMI
> calls.
> 
> Fixes: c203c59fb5de ("platform/x86: hp-wmi: implement fan keep-alive")
> 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 db72ad9da0a5..1096fb46cbcd 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -453,6 +453,7 @@ enum pwm_modes {
>  };
>  
>  struct hp_wmi_hwmon_priv {
> +	struct mutex lock;	/* protects mode, pwm */
>  	u8 min_rpm;
>  	u8 max_rpm;
>  	int gpu_delta;
> @@ -2422,6 +2423,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) {
> @@ -2445,11 +2447,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 */
> @@ -2467,6 +2471,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) {
> @@ -2535,6 +2540,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.
> @@ -2592,6 +2599,8 @@ static int hp_wmi_hwmon_init(void)
>  	if (!priv)
>  		return -ENOMEM;
>  
> +	mutex_init(&priv->lock);

Please use devm_mutex_init() so you don't need to deinit it manually with 
mutex_destroy(). While changing that, don't forget that devm_*() can fail 
so remember to handle the error.

-- 
 i.


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

end of thread, other threads:[~2026-04-07  8:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-05 14:44 [PATCH v3 0/5] platform/x86: hp-wmi: Victus S fan control fixes Emre Cecanpunar
2026-04-05 14:44 ` [PATCH v3 1/5] platform/x86: hp-wmi: fix ignored return values in fan settings Emre Cecanpunar
2026-04-05 14:44 ` [PATCH v3 2/5] platform/x86: hp-wmi: avoid cancel_delayed_work_sync from work handler Emre Cecanpunar
2026-04-05 14:44 ` [PATCH v3 3/5] platform/x86: hp-wmi: use mod_delayed_work to reset keep-alive timer Emre Cecanpunar
2026-04-05 14:44 ` [PATCH v3 4/5] platform/x86: hp-wmi: fix u8 underflow in gpu_delta calculation Emre Cecanpunar
2026-04-07  8:02   ` Ilpo Järvinen
2026-04-05 14:45 ` [PATCH v3 5/5] platform/x86: hp-wmi: add locking for concurrent hwmon access Emre Cecanpunar
2026-04-07  8:07   ` 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