This series adds support for manual fan speed control and PWM reporting
for HP Victus S-style laptops.

The first patch refactors the hwmon implementation to use a per-device
private context for state tracking. It implements RPM-to-PWM conversion
using linear interpolation based on the laptop's internal fan tables
retrieved via WMI. It also introduces PWM_MODE_MANUAL, allowing users
to set specific fan speeds.

The second patch addresses a firmware-specific behavior where the
system reverts to "Auto" fan mode after a 120-second timeout if no WMI
fan commands are received. A delayed workqueue is implemented to act
as a keep-alive heartbeat, refreshing the fan state every 90 seconds
to ensure user-selected profiles remain active indefinitely.

Tested on: HP Omen 16-wf1xxx (board ID 8C78)

Krishna Chomal (2):
  platform/x86: hp-wmi: add manual fan control for Victus S models
  platform/x86: hp-wmi: implement fan keep-alive

 drivers/platform/x86/hp/hp-wmi.c | 287 ++++++++++++++++++++++++++-----
 1 file changed, 243 insertions(+), 44 deletions(-)

-- 
2.52.0

This series adds support for manual fan speed control and PWM reporting
for HP Victus S-style laptops.

The first patch refactors the hwmon implementation to use a per-device
private context for state tracking. It implements RPM-to-PWM conversion
using linear interpolation based on the laptop's internal fan tables
retrieved via WMI. It also introduces PWM_MODE_MANUAL, allowing users
to set specific fan speeds.

The second patch addresses a firmware-specific behavior where the
system reverts to "Auto" fan mode after a 120-second timeout if no WMI
fan commands are received. A delayed workqueue is implemented to act
as a keep-alive heartbeat, refreshing the fan state every 90 seconds
to ensure user-selected profiles remain active indefinitely.

Changes in v2:
- Refactored hp_wmi_apply_fan_settings() to use a 'ret' variable and use
  a common path to set fan settings and prepare for keep-alive logic.
- Replaced raw buffer casting with proper fan table structs.
- Converted RPM/PWM macros to static inline functions.
- Renamed internal context variable from 'ctx' to 'priv' for consistency.
- Renamed delay macro to KEEP_ALIVE_DELAY_SECS.
- Added missing headers and removed redundant NULL checks.

Tested on: HP Omen 16-wf1xxx (board ID 8C78)

Krishna Chomal (2):
  platform/x86: hp-wmi: add manual fan control for Victus S models
  platform/x86: hp-wmi: implement fan keep-alive

 drivers/platform/x86/hp/hp-wmi.c | 303 ++++++++++++++++++++++++++-----
 1 file changed, 258 insertions(+), 45 deletions(-)

-- 
2.52.0

This series adds support for manual fan speed control and PWM reporting
for HP Victus S-style laptops.

The first patch is a trivial fix for ordering the include headers.

The second patch refactors the hwmon implementation to use a per-device
private context for state tracking. It implements RPM-to-PWM conversion
using linear interpolation based on the laptop's internal fan tables
retrieved via WMI. It also introduces PWM_MODE_MANUAL, allowing users
to set specific fan speeds.

The third patch addresses a firmware-specific behavior where the
system reverts to "Auto" fan mode after a 120-second timeout if no WMI
fan commands are received. A delayed workqueue is implemented to act
as a keep-alive heartbeat, refreshing the fan state every 90 seconds
to ensure user-selected profiles remain active indefinitely.

Changes in v3:
- Added a new patch (1/3) to sort include headers alphabetically.
- Fixed alignment of multi-line function arguments.
- Refactored GPU fan speed calculation to use signed integers.
- Removed intermediate "ret" variables in hwmon read/write functions.
- Capitalized "PWM" in comments for consistency.
- Renamed "ctx" to "priv" for drvdata throughout the series.
Changes in v2:
- Refactored hp_wmi_apply_fan_settings() to use a "ret" variable and use
  a common path to set fan settings and prepare for keep-alive logic.
- Replaced raw buffer casting with proper fan table structs.
- Converted RPM/PWM macros to static inline functions.
- Renamed internal context variable from "ctx" to "priv" for consistency.
- Renamed delay macro to KEEP_ALIVE_DELAY_SECS.
- Added missing headers and removed redundant NULL checks.

Krishna Chomal (3):
  platform/x86: hp-wmi: order include headers
  platform/x86: hp-wmi: add manual fan control for Victus S models
  platform/x86: hp-wmi: implement fan keep-alive

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

-- 
2.52.0

On Tue, 13 Jan 2026 18:07:35 +0530, Krishna Chomal wrote:

> This series adds support for manual fan speed control and PWM reporting
> for HP Victus S-style laptops.
> 
> The first patch is a trivial fix for ordering the include headers.
> 
> The second patch refactors the hwmon implementation to use a per-device
> private context for state tracking. It implements RPM-to-PWM conversion
> using linear interpolation based on the laptop's internal fan tables
> retrieved via WMI. It also introduces PWM_MODE_MANUAL, allowing users
> to set specific fan speeds.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/3] platform/x86: hp-wmi: order include headers
      commit: 60f2d5d0f04365c41ad4f9eddf48c80dcd0b01c9
[2/3] platform/x86: hp-wmi: add manual fan control for Victus S models
      commit: 46be1453e6e61884b4840a768d1e8ffaf01a4c1c
[3/3] platform/x86: hp-wmi: implement fan keep-alive
      commit: c203c59fb5de1b1b8947d61176e868da1130cbeb

--
 i.

On Thu, Jan 15, 2026 at 03:45:36PM +0200, Ilpo Järvinen wrote:
>On Tue, 13 Jan 2026 18:07:35 +0530, Krishna Chomal wrote:
>
>> This series adds support for manual fan speed control and PWM reporting
>> for HP Victus S-style laptops.
>>
>> The first patch is a trivial fix for ordering the include headers.
>>
>> The second patch refactors the hwmon implementation to use a per-device
>> private context for state tracking. It implements RPM-to-PWM conversion
>> using linear interpolation based on the laptop's internal fan tables
>> retrieved via WMI. It also introduces PWM_MODE_MANUAL, allowing users
>> to set specific fan speeds.
>>
>> [...]
>
>
>Thank you for your contribution, it has been applied to my local
>review-ilpo-next branch. Note it will show up in the public
>platform-drivers-x86/review-ilpo-next branch only once I've pushed my
>local branch there, which might take a while.
>
>The list of commits applied:
>[1/3] platform/x86: hp-wmi: order include headers
>      commit: 60f2d5d0f04365c41ad4f9eddf48c80dcd0b01c9
>[2/3] platform/x86: hp-wmi: add manual fan control for Victus S models
>      commit: 46be1453e6e61884b4840a768d1e8ffaf01a4c1c
>[3/3] platform/x86: hp-wmi: implement fan keep-alive
>      commit: c203c59fb5de1b1b8947d61176e868da1130cbeb
>
>--
> i.
>

Thank you

The include headers in hp-wmi driver are currently not in any specific
order. As the driver continues to grow, keep the header block organized
by sorting them alphabetically.

Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
---
Changes in v3:
- New patch in this series
---
 drivers/platform/x86/hp/hp-wmi.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index f4ea1ea05997..fac8e227cee0 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -13,23 +13,23 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/cleanup.h>
+#include <linux/dmi.h>
+#include <linux/hwmon.h>
 #include <linux/init.h>
-#include <linux/slab.h>
-#include <linux/types.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h>
 #include <linux/platform_profile.h>
-#include <linux/hwmon.h>
-#include <linux/acpi.h>
-#include <linux/mutex.h>
-#include <linux/cleanup.h>
 #include <linux/power_supply.h>
 #include <linux/rfkill.h>
+#include <linux/slab.h>
 #include <linux/string.h>
-#include <linux/dmi.h>
+#include <linux/types.h>
 
 MODULE_AUTHOR("Matthew Garrett <mjg59@srcf.ucam.org>");
 MODULE_DESCRIPTION("HP laptop WMI driver");
-- 
2.52.0

Add manual fan speed control and PWM reporting for HP Victus S-series
laptops.

While HPWMI_FAN_SPEED_SET_QUERY was previously added to reset max fan
mode, it is actually capable of individual fan control. This patch
implements hp_wmi_fan_speed_set() to allow manual control and hides
PWM inputs for non-Victus devices as the query is Victus specific.

The existing hp_wmi_fan_speed_max_get() query is unreliable on Victus S
firmware, often incorrectly reporting "Auto" mode even when "Max" is
active. To resolve this synchronization issue, move state tracking to
a per-device private context and apply "Auto" mode during driver
initialization to ensure a consistent starting point.

Refactor hp_wmi_apply_fan_settings() to use an intermediate ret
variable. This prepares the switch block for keep-alive logic being
added in a later patch, avoiding the need for duplicated mode check.

Tested on: HP Omen 16-wf1xxx (board ID 8C78)

Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
---
Changes in v3:
- Fixed improper alignment of multi-line functions and if statements
- Fixed GPU fan speed calculation to use signed int to safely handle
  overflows/underflows
- Directly use rpm instead of intermediate ret in
  hp_wmi_hwmon_read/write
- Updated comments to use proper (uppercase) "PWM" terminology
- Split deeply nested pwm sanity check logic into separate statements
  for readability.
Changes in v2:
- Added limits.h and minmax.h headers
- Re-written pwm-to-rpm (and vice-versa) conversion from macros to
  static inline functions
- Removed redundant NULL checking of drvdata
- Made integer promotion explicit by casting intermediate result to
  unsigned int when calculating the GPU fan speed
- Renamed "enforce_ctx" to "apply_fan_settings" for clarity
- Removed unnecessary comments
- Renamed "ctx" to "priv" as it is more consistent for drvdata
- Added new struct victus_s_fan_table to avoid complex type casting
---
 drivers/platform/x86/hp/hp-wmi.c | 263 +++++++++++++++++++++++++------
 1 file changed, 217 insertions(+), 46 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index fac8e227cee0..d04e53ae1803 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -15,12 +15,16 @@
 
 #include <linux/acpi.h>
 #include <linux/cleanup.h>
+#include <linux/compiler_attributes.h>
 #include <linux/dmi.h>
+#include <linux/fixp-arith.h>
 #include <linux/hwmon.h>
 #include <linux/init.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
 #include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
@@ -190,7 +194,8 @@ enum hp_wmi_gm_commandtype {
 	HPWMI_SET_GPU_THERMAL_MODES_QUERY	= 0x22,
 	HPWMI_SET_POWER_LIMITS_QUERY		= 0x29,
 	HPWMI_VICTUS_S_FAN_SPEED_GET_QUERY	= 0x2D,
-	HPWMI_FAN_SPEED_SET_QUERY		= 0x2E,
+	HPWMI_VICTUS_S_FAN_SPEED_SET_QUERY	= 0x2E,
+	HPWMI_VICTUS_S_GET_FAN_TABLE_QUERY	= 0x2F,
 };
 
 enum hp_wmi_command {
@@ -348,6 +353,51 @@ static const char * const tablet_chassis_types[] = {
 
 #define DEVICE_MODE_TABLET	0x06
 
+#define CPU_FAN 0
+#define GPU_FAN 1
+
+enum pwm_modes {
+	PWM_MODE_MAX		= 0,
+	PWM_MODE_MANUAL		= 1,
+	PWM_MODE_AUTO		= 2,
+};
+
+struct hp_wmi_hwmon_priv {
+	u8 min_rpm;
+	u8 max_rpm;
+	u8 gpu_delta;
+	u8 mode;
+	u8 pwm;
+};
+
+struct victus_s_fan_table_header {
+	u8 unknown;
+	u8 num_entries;
+} __packed;
+
+struct victus_s_fan_table_entry {
+	u8 cpu_rpm;
+	u8 gpu_rpm;
+	u8 unknown;
+} __packed;
+
+struct victus_s_fan_table {
+	struct victus_s_fan_table_header header;
+	struct victus_s_fan_table_entry entries[];
+} __packed;
+
+static inline u8 rpm_to_pwm(u8 rpm, struct hp_wmi_hwmon_priv *priv)
+{
+	return fixp_linear_interpolate(0, 0, priv->max_rpm, U8_MAX,
+				       clamp_val(rpm, 0, priv->max_rpm));
+}
+
+static inline u8 pwm_to_rpm(u8 pwm, struct hp_wmi_hwmon_priv *priv)
+{
+	return fixp_linear_interpolate(0, 0, U8_MAX, priv->max_rpm,
+				       clamp_val(pwm, 0, U8_MAX));
+}
+
 /* map output size to the corresponding WMI method id */
 static inline int encode_outsize_for_pvsz(int outsize)
 {
@@ -637,41 +687,53 @@ static int hp_wmi_fan_speed_max_set(int enabled)
 	return enabled;
 }
 
-static int hp_wmi_fan_speed_reset(void)
+static int hp_wmi_fan_speed_set(struct hp_wmi_hwmon_priv *priv, u8 speed)
 {
-	u8 fan_speed[2] = { HP_FAN_SPEED_AUTOMATIC, HP_FAN_SPEED_AUTOMATIC };
-	int ret;
-
-	ret = hp_wmi_perform_query(HPWMI_FAN_SPEED_SET_QUERY, HPWMI_GM,
-				   &fan_speed, sizeof(fan_speed), 0);
+	u8 fan_speed[2];
+	int gpu_speed, ret;
 
-	return ret;
-}
+	fan_speed[CPU_FAN] = speed;
+	fan_speed[GPU_FAN] = speed;
 
-static int hp_wmi_fan_speed_max_reset(void)
-{
-	int ret;
+	/*
+	 * GPU fan speed is always a little higher than CPU fan speed, we fetch
+	 * this delta value from the fan table during hwmon init.
+	 * Exception: Speed is set to HP_FAN_SPEED_AUTOMATIC, to revert to
+	 * automatic mode.
+	 */
+	if (speed != HP_FAN_SPEED_AUTOMATIC) {
+		gpu_speed = speed + priv->gpu_delta;
+		fan_speed[GPU_FAN] = clamp_val(gpu_speed, 0, U8_MAX);
+	}
 
+	ret = hp_wmi_get_fan_count_userdefine_trigger();
+	if (ret < 0)
+		return ret;
+	/* Max fans need to be explicitly disabled */
 	ret = hp_wmi_fan_speed_max_set(0);
-	if (ret)
+	if (ret < 0)
 		return ret;
+	ret = hp_wmi_perform_query(HPWMI_VICTUS_S_FAN_SPEED_SET_QUERY, HPWMI_GM,
+				   &fan_speed, sizeof(fan_speed), 0);
 
-	/* Disabling max fan speed on Victus s1xxx laptops needs a 2nd step: */
-	ret = hp_wmi_fan_speed_reset();
 	return ret;
 }
 
-static int hp_wmi_fan_speed_max_get(void)
+static int hp_wmi_fan_speed_reset(struct hp_wmi_hwmon_priv *priv)
 {
-	int val = 0, ret;
+	return hp_wmi_fan_speed_set(priv, HP_FAN_SPEED_AUTOMATIC);
+}
 
-	ret = hp_wmi_perform_query(HPWMI_FAN_SPEED_MAX_GET_QUERY, HPWMI_GM,
-				   &val, zero_if_sup(val), sizeof(val));
+static int hp_wmi_fan_speed_max_reset(struct hp_wmi_hwmon_priv *priv)
+{
+	int ret;
 
+	ret = hp_wmi_fan_speed_max_set(0);
 	if (ret)
-		return ret < 0 ? ret : -EINVAL;
+		return ret;
 
-	return val;
+	/* Disabling max fan speed on Victus s1xxx laptops needs a 2nd step: */
+	return hp_wmi_fan_speed_reset(priv);
 }
 
 static int __init hp_wmi_bios_2008_later(void)
@@ -2108,12 +2170,45 @@ static struct platform_driver hp_wmi_driver __refdata = {
 	.remove = __exit_p(hp_wmi_bios_remove),
 };
 
+static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
+{
+	int ret;
+
+	switch (priv->mode) {
+	case PWM_MODE_MAX:
+		if (is_victus_s_thermal_profile())
+			hp_wmi_get_fan_count_userdefine_trigger();
+		ret = hp_wmi_fan_speed_max_set(1);
+		return ret;
+	case PWM_MODE_MANUAL:
+		if (!is_victus_s_thermal_profile())
+			return -EOPNOTSUPP;
+		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
+		return ret;
+	case PWM_MODE_AUTO:
+		if (is_victus_s_thermal_profile()) {
+			hp_wmi_get_fan_count_userdefine_trigger();
+			ret = hp_wmi_fan_speed_max_reset(priv);
+		} else {
+			ret = hp_wmi_fan_speed_max_set(0);
+		}
+		return ret;
+	default:
+		/* shouldn't happen */
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static umode_t hp_wmi_hwmon_is_visible(const void *data,
 				       enum hwmon_sensor_types type,
 				       u32 attr, int channel)
 {
 	switch (type) {
 	case hwmon_pwm:
+		if (attr == hwmon_pwm_input && !is_victus_s_thermal_profile())
+			return 0;
 		return 0644;
 	case hwmon_fan:
 		if (is_victus_s_thermal_profile()) {
@@ -2134,8 +2229,10 @@ static umode_t hp_wmi_hwmon_is_visible(const void *data,
 static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 			     u32 attr, int channel, long *val)
 {
-	int ret;
+	struct hp_wmi_hwmon_priv *priv;
+	int rpm, ret;
 
+	priv = dev_get_drvdata(dev);
 	switch (type) {
 	case hwmon_fan:
 		if (is_victus_s_thermal_profile())
@@ -2147,16 +2244,21 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 		*val = ret;
 		return 0;
 	case hwmon_pwm:
-		switch (hp_wmi_fan_speed_max_get()) {
-		case 0:
-			/* 0 is automatic fan, which is 2 for hwmon */
-			*val = 2;
+		if (attr == hwmon_pwm_input) {
+			if (!is_victus_s_thermal_profile())
+				return -EOPNOTSUPP;
+
+			rpm = hp_wmi_get_fan_speed_victus_s(channel);
+			if (rpm < 0)
+				return rpm;
+			*val = rpm_to_pwm(rpm / 100, priv);
 			return 0;
-		case 1:
-			/* 1 is max fan, which is 0
-			 * (no fan speed control) for hwmon
-			 */
-			*val = 0;
+		}
+		switch (priv->mode) {
+		case PWM_MODE_MAX:
+		case PWM_MODE_MANUAL:
+		case PWM_MODE_AUTO:
+			*val = priv->mode;
 			return 0;
 		default:
 			/* shouldn't happen */
@@ -2170,23 +2272,46 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 			      u32 attr, int channel, long val)
 {
+	struct hp_wmi_hwmon_priv *priv;
+	int rpm;
+
+	priv = dev_get_drvdata(dev);
 	switch (type) {
 	case hwmon_pwm:
+		if (attr == hwmon_pwm_input) {
+			if (!is_victus_s_thermal_profile())
+				return -EOPNOTSUPP;
+			/* PWM input is invalid when not in manual mode */
+			if (priv->mode != PWM_MODE_MANUAL)
+				return -EINVAL;
+
+			/* ensure PWM input is within valid fan speeds */
+			rpm = pwm_to_rpm(val, priv);
+			rpm = clamp_val(rpm, priv->min_rpm, priv->max_rpm);
+			priv->pwm = rpm_to_pwm(rpm, priv);
+			return hp_wmi_apply_fan_settings(priv);
+		}
 		switch (val) {
-		case 0:
-			if (is_victus_s_thermal_profile())
-				hp_wmi_get_fan_count_userdefine_trigger();
-			/* 0 is no fan speed control (max), which is 1 for us */
-			return hp_wmi_fan_speed_max_set(1);
-		case 2:
-			/* 2 is automatic speed control, which is 0 for us */
-			if (is_victus_s_thermal_profile()) {
-				hp_wmi_get_fan_count_userdefine_trigger();
-				return hp_wmi_fan_speed_max_reset();
-			} else
-				return hp_wmi_fan_speed_max_set(0);
+		case PWM_MODE_MAX:
+			priv->mode = PWM_MODE_MAX;
+			return hp_wmi_apply_fan_settings(priv);
+		case PWM_MODE_MANUAL:
+			if (!is_victus_s_thermal_profile())
+				return -EOPNOTSUPP;
+			/*
+			 * When switching to manual mode, set fan speed to
+			 * current RPM values to ensure a smooth transition.
+			 */
+			rpm = hp_wmi_get_fan_speed_victus_s(channel);
+			if (rpm < 0)
+				return rpm;
+			priv->pwm = rpm_to_pwm(rpm / 100, priv);
+			priv->mode = PWM_MODE_MANUAL;
+			return hp_wmi_apply_fan_settings(priv);
+		case PWM_MODE_AUTO:
+			priv->mode = PWM_MODE_AUTO;
+			return hp_wmi_apply_fan_settings(priv);
 		default:
-			/* we don't support manual fan speed control */
 			return -EINVAL;
 		}
 	default:
@@ -2196,7 +2321,7 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 
 static const struct hwmon_channel_info * const info[] = {
 	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT),
-	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE),
+	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE | HWMON_PWM_INPUT),
 	NULL
 };
 
@@ -2211,12 +2336,56 @@ static const struct hwmon_chip_info chip_info = {
 	.info = info,
 };
 
+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;
+
+	/* Default behaviour on hwmon init is automatic mode */
+	priv->mode = PWM_MODE_AUTO;
+
+	/* Bypass all non-Victus S devices */
+	if (!is_victus_s_thermal_profile())
+		return 0;
+
+	ret = hp_wmi_perform_query(HPWMI_VICTUS_S_GET_FAN_TABLE_QUERY,
+				   HPWMI_GM, &fan_data, 4, sizeof(fan_data));
+	if (ret)
+		return ret;
+
+	fan_table = (struct victus_s_fan_table *)fan_data;
+	if (fan_table->header.num_entries == 0 ||
+	    sizeof(struct victus_s_fan_table_header) +
+	    sizeof(struct victus_s_fan_table_entry) * fan_table->header.num_entries > sizeof(fan_data))
+		return -EINVAL;
+
+	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;
+	priv->min_rpm = min_rpm;
+	priv->max_rpm = max_rpm;
+	priv->gpu_delta = gpu_delta;
+
+	return 0;
+}
+
 static int hp_wmi_hwmon_init(void)
 {
 	struct device *dev = &hp_wmi_platform_dev->dev;
+	struct hp_wmi_hwmon_priv *priv;
 	struct device *hwmon;
+	int ret;
 
-	hwmon = devm_hwmon_device_register_with_info(dev, "hp", &hp_wmi_driver,
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = hp_wmi_setup_fan_settings(priv);
+	if (ret)
+		return ret;
+	hwmon = devm_hwmon_device_register_with_info(dev, "hp", priv,
 						     &chip_info, NULL);
 
 	if (IS_ERR(hwmon)) {
@@ -2224,6 +2393,8 @@ static int hp_wmi_hwmon_init(void)
 		return PTR_ERR(hwmon);
 	}
 
+	hp_wmi_apply_fan_settings(priv);
+
 	return 0;
 }
 
-- 
2.52.0

The firmware on some HP laptops automatically reverts the fan speed
control to "Auto" mode after a 120 second timeout window.

To ensure that the user-selected fan profile (Max/Manual) persists,
implement a keep-alive mechanism that periodically refreshes the fan
mode trigger before the timeout occurs.

- Introduce a delayed workqueue to trigger the fan mode refresh every 90
  seconds, ensuring the system maintains the correct fan mode setting.
- Integrate the refresh mechanism into hp_wmi_hwmon_enforce_ctx() to
  start, update or cancel the keep-alive process based on the current
  fan mode.

This ensures that the driver stays in sync with the hardware.

Tested on: HP Omen 16-wf1xxx (board ID 8C78)

Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
---
Changes in v3:
- Rename "ctx" to "priv" for drvdata
Changes in v2:
- Explicitly specify time unit in KEEP_ALIVE_DELAY macro
- Handle work rescheduling directly in switch/case
---
 drivers/platform/x86/hp/hp-wmi.c | 46 +++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index d04e53ae1803..45ab644ff10e 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/types.h>
+#include <linux/workqueue.h>
 
 MODULE_AUTHOR("Matthew Garrett <mjg59@srcf.ucam.org>");
 MODULE_DESCRIPTION("HP laptop WMI driver");
@@ -368,6 +369,7 @@ struct hp_wmi_hwmon_priv {
 	u8 gpu_delta;
 	u8 mode;
 	u8 pwm;
+	struct delayed_work keep_alive_dwork;
 };
 
 struct victus_s_fan_table_header {
@@ -386,6 +388,12 @@ struct victus_s_fan_table {
 	struct victus_s_fan_table_entry entries[];
 } __packed;
 
+/*
+ * 90s delay to prevent the firmware from resetting fan mode after fixed
+ * 120s timeout
+ */
+#define KEEP_ALIVE_DELAY_SECS     90
+
 static inline u8 rpm_to_pwm(u8 rpm, struct hp_wmi_hwmon_priv *priv)
 {
 	return fixp_linear_interpolate(0, 0, priv->max_rpm, U8_MAX,
@@ -2093,6 +2101,7 @@ static int __init hp_wmi_bios_setup(struct platform_device *device)
 static void __exit hp_wmi_bios_remove(struct platform_device *device)
 {
 	int i;
+	struct hp_wmi_hwmon_priv *priv;
 
 	for (i = 0; i < rfkill2_count; i++) {
 		rfkill_unregister(rfkill2[i].rfkill);
@@ -2111,6 +2120,10 @@ static void __exit hp_wmi_bios_remove(struct platform_device *device)
 		rfkill_unregister(wwan_rfkill);
 		rfkill_destroy(wwan_rfkill);
 	}
+
+	priv = platform_get_drvdata(device);
+	if (priv)
+		cancel_delayed_work_sync(&priv->keep_alive_dwork);
 }
 
 static int hp_wmi_resume_handler(struct device *device)
@@ -2179,12 +2192,20 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 		if (is_victus_s_thermal_profile())
 			hp_wmi_get_fan_count_userdefine_trigger();
 		ret = hp_wmi_fan_speed_max_set(1);
-		return ret;
+		if (ret < 0)
+			return ret;
+		schedule_delayed_work(&priv->keep_alive_dwork,
+				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
+		return 0;
 	case PWM_MODE_MANUAL:
 		if (!is_victus_s_thermal_profile())
 			return -EOPNOTSUPP;
 		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
-		return ret;
+		if (ret < 0)
+			return ret;
+		schedule_delayed_work(&priv->keep_alive_dwork,
+				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
+		return 0;
 	case PWM_MODE_AUTO:
 		if (is_victus_s_thermal_profile()) {
 			hp_wmi_get_fan_count_userdefine_trigger();
@@ -2192,7 +2213,10 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 		} else {
 			ret = hp_wmi_fan_speed_max_set(0);
 		}
-		return ret;
+		if (ret < 0)
+			return ret;
+		cancel_delayed_work_sync(&priv->keep_alive_dwork);
+		return 0;
 	default:
 		/* shouldn't happen */
 		return -EINVAL;
@@ -2336,6 +2360,20 @@ static const struct hwmon_chip_info chip_info = {
 	.info = info,
 };
 
+static void hp_wmi_hwmon_keep_alive_handler(struct work_struct *work)
+{
+	struct delayed_work *dwork;
+	struct hp_wmi_hwmon_priv *priv;
+
+	dwork = to_delayed_work(work);
+	priv = container_of(dwork, struct hp_wmi_hwmon_priv, keep_alive_dwork);
+	/*
+	 * Re-apply the current hwmon context settings.
+	 * NOTE: hp_wmi_apply_fan_settings will handle the re-scheduling.
+	 */
+	hp_wmi_apply_fan_settings(priv);
+}
+
 static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
 {
 	u8 fan_data[128] = { 0 };
@@ -2393,6 +2431,8 @@ static int hp_wmi_hwmon_init(void)
 		return PTR_ERR(hwmon);
 	}
 
+	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);
 
 	return 0;
-- 
2.52.0

Add manual fan speed control and PWM reporting for HP Victus S-series
laptops.

While HPWMI_FAN_SPEED_SET_QUERY was previously added to reset max fan
mode, it is actually capable of individual fan control. This patch
implements hp_wmi_fan_speed_set() to allow manual control and hides
PWM inputs for non-Victus devices as the query is Victus specific.

The existing hp_wmi_fan_speed_max_get() query is unreliable on Victus S
firmware, often incorrectly reporting "Auto" mode even when "Max" is
active. To resolve this synchronization issue, move state tracking to
a per-device private context and apply "Auto" mode during driver
initialization to ensure a consistent starting point.

Refactor hp_wmi_apply_fan_settings() to use an intermediate ret
variable. This prepares the switch block for keep-alive logic being
added in a later patch, avoiding the need for duplicated mode check.

Tested on: HP Omen 16-wf1xxx (board ID 8C78)

Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
---
Changes in v2:
- Added limits.h and minmax.h headers
- Re-written pwm-to-rpm (and vice-versa) conversion from macros to
  static inline functions
- Removed redundant NULL checking of drvdata
- Made integer promotion explicit by casting intermediate result to
  unsigned int when calculating the GPU fan speed
- Renamed "enforce_ctx" to "apply_fan_settings" for clarity
- Removed unnecessary comments
- Renamed "ctx" to "priv" as it is more consistent for drvdata
- Added new struct victus_s_fan_table to avoid complex type casting
---
 drivers/platform/x86/hp/hp-wmi.c | 263 +++++++++++++++++++++++++------
 1 file changed, 218 insertions(+), 45 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index f4ea1ea05997..ef419575174c 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -30,6 +30,10 @@
 #include <linux/rfkill.h>
 #include <linux/string.h>
 #include <linux/dmi.h>
+#include <linux/fixp-arith.h>
+#include <linux/limits.h>
+#include <linux/minmax.h>
+#include <linux/compiler_attributes.h>
 
 MODULE_AUTHOR("Matthew Garrett <mjg59@srcf.ucam.org>");
 MODULE_DESCRIPTION("HP laptop WMI driver");
@@ -190,7 +194,8 @@ enum hp_wmi_gm_commandtype {
 	HPWMI_SET_GPU_THERMAL_MODES_QUERY	= 0x22,
 	HPWMI_SET_POWER_LIMITS_QUERY		= 0x29,
 	HPWMI_VICTUS_S_FAN_SPEED_GET_QUERY	= 0x2D,
-	HPWMI_FAN_SPEED_SET_QUERY		= 0x2E,
+	HPWMI_VICTUS_S_FAN_SPEED_SET_QUERY	= 0x2E,
+	HPWMI_VICTUS_S_GET_FAN_TABLE_QUERY	= 0x2F,
 };
 
 enum hp_wmi_command {
@@ -348,6 +353,51 @@ static const char * const tablet_chassis_types[] = {
 
 #define DEVICE_MODE_TABLET	0x06
 
+#define CPU_FAN 0
+#define GPU_FAN 1
+
+enum pwm_modes {
+	PWM_MODE_MAX		= 0,
+	PWM_MODE_MANUAL		= 1,
+	PWM_MODE_AUTO		= 2,
+};
+
+struct hp_wmi_hwmon_priv {
+	u8 min_rpm;
+	u8 max_rpm;
+	u8 gpu_delta;
+	u8 mode;
+	u8 pwm;
+};
+
+struct victus_s_fan_table_header {
+	u8 unknown;
+	u8 num_entries;
+} __packed;
+
+struct victus_s_fan_table_entry {
+	u8 cpu_rpm;
+	u8 gpu_rpm;
+	u8 unknown;
+} __packed;
+
+struct victus_s_fan_table {
+	struct victus_s_fan_table_header header;
+	struct victus_s_fan_table_entry entries[];
+} __packed;
+
+static inline u8 rpm_to_pwm(u8 rpm, struct hp_wmi_hwmon_priv *priv)
+{
+	return fixp_linear_interpolate(0, 0, priv->max_rpm, U8_MAX,
+					clamp_val(rpm, 0, priv->max_rpm));
+}
+
+static inline u8 pwm_to_rpm(u8 pwm, struct hp_wmi_hwmon_priv *priv)
+{
+	return fixp_linear_interpolate(0, 0, U8_MAX, priv->max_rpm,
+					clamp_val(pwm, 0, U8_MAX));
+}
+
 /* map output size to the corresponding WMI method id */
 static inline int encode_outsize_for_pvsz(int outsize)
 {
@@ -637,41 +687,53 @@ static int hp_wmi_fan_speed_max_set(int enabled)
 	return enabled;
 }
 
-static int hp_wmi_fan_speed_reset(void)
+static int hp_wmi_fan_speed_set(struct hp_wmi_hwmon_priv *priv, u8 speed)
 {
-	u8 fan_speed[2] = { HP_FAN_SPEED_AUTOMATIC, HP_FAN_SPEED_AUTOMATIC };
+	u8 fan_speed[2];
 	int ret;
 
-	ret = hp_wmi_perform_query(HPWMI_FAN_SPEED_SET_QUERY, HPWMI_GM,
-				   &fan_speed, sizeof(fan_speed), 0);
-
-	return ret;
-}
+	fan_speed[CPU_FAN] = speed;
+	fan_speed[GPU_FAN] = speed;
 
-static int hp_wmi_fan_speed_max_reset(void)
-{
-	int ret;
+	/*
+	 * GPU fan speed is always a little higher than CPU fan speed, we fetch
+	 * this delta value from the fan table during hwmon init.
+	 * Exception: Speed is set to HP_FAN_SPEED_AUTOMATIC, to revert to
+	 * automatic mode.
+	 */
+	if (speed != HP_FAN_SPEED_AUTOMATIC)
+		fan_speed[GPU_FAN] = clamp_val((unsigned int)speed +
+						(unsigned int)priv->gpu_delta,
+						0, U8_MAX);
 
+	ret = hp_wmi_get_fan_count_userdefine_trigger();
+	if (ret < 0)
+		return ret;
+	/* Max fans need to be explicitly disabled */
 	ret = hp_wmi_fan_speed_max_set(0);
-	if (ret)
+	if (ret < 0)
 		return ret;
+	ret = hp_wmi_perform_query(HPWMI_VICTUS_S_FAN_SPEED_SET_QUERY, HPWMI_GM,
+				   &fan_speed, sizeof(fan_speed), 0);
 
-	/* Disabling max fan speed on Victus s1xxx laptops needs a 2nd step: */
-	ret = hp_wmi_fan_speed_reset();
 	return ret;
 }
 
-static int hp_wmi_fan_speed_max_get(void)
+static int hp_wmi_fan_speed_reset(struct hp_wmi_hwmon_priv *priv)
 {
-	int val = 0, ret;
+	return hp_wmi_fan_speed_set(priv, HP_FAN_SPEED_AUTOMATIC);
+}
 
-	ret = hp_wmi_perform_query(HPWMI_FAN_SPEED_MAX_GET_QUERY, HPWMI_GM,
-				   &val, zero_if_sup(val), sizeof(val));
+static int hp_wmi_fan_speed_max_reset(struct hp_wmi_hwmon_priv *priv)
+{
+	int ret;
 
+	ret = hp_wmi_fan_speed_max_set(0);
 	if (ret)
-		return ret < 0 ? ret : -EINVAL;
+		return ret;
 
-	return val;
+	/* Disabling max fan speed on Victus s1xxx laptops needs a 2nd step: */
+	return hp_wmi_fan_speed_reset(priv);
 }
 
 static int __init hp_wmi_bios_2008_later(void)
@@ -2108,12 +2170,45 @@ static struct platform_driver hp_wmi_driver __refdata = {
 	.remove = __exit_p(hp_wmi_bios_remove),
 };
 
+static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
+{
+	int ret;
+
+	switch (priv->mode) {
+	case PWM_MODE_MAX:
+		if (is_victus_s_thermal_profile())
+			hp_wmi_get_fan_count_userdefine_trigger();
+		ret = hp_wmi_fan_speed_max_set(1);
+		return ret;
+	case PWM_MODE_MANUAL:
+		if (!is_victus_s_thermal_profile())
+			return -EOPNOTSUPP;
+		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
+		return ret;
+	case PWM_MODE_AUTO:
+		if (is_victus_s_thermal_profile()) {
+			hp_wmi_get_fan_count_userdefine_trigger();
+			ret = hp_wmi_fan_speed_max_reset(priv);
+		} else {
+			ret = hp_wmi_fan_speed_max_set(0);
+		}
+		return ret;
+	default:
+		/* shouldn't happen */
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static umode_t hp_wmi_hwmon_is_visible(const void *data,
 				       enum hwmon_sensor_types type,
 				       u32 attr, int channel)
 {
 	switch (type) {
 	case hwmon_pwm:
+		if (attr == hwmon_pwm_input && !is_victus_s_thermal_profile())
+			return 0;
 		return 0644;
 	case hwmon_fan:
 		if (is_victus_s_thermal_profile()) {
@@ -2134,8 +2229,10 @@ static umode_t hp_wmi_hwmon_is_visible(const void *data,
 static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 			     u32 attr, int channel, long *val)
 {
-	int ret;
+	struct hp_wmi_hwmon_priv *priv;
+	int current_rpm, ret;
 
+	priv = dev_get_drvdata(dev);
 	switch (type) {
 	case hwmon_fan:
 		if (is_victus_s_thermal_profile())
@@ -2147,16 +2244,21 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 		*val = ret;
 		return 0;
 	case hwmon_pwm:
-		switch (hp_wmi_fan_speed_max_get()) {
-		case 0:
-			/* 0 is automatic fan, which is 2 for hwmon */
-			*val = 2;
+		if (attr == hwmon_pwm_input) {
+			if (!is_victus_s_thermal_profile())
+				return -EOPNOTSUPP;
+			ret = hp_wmi_get_fan_speed_victus_s(channel);
+			if (ret < 0)
+				return ret;
+			current_rpm = ret;
+			*val = rpm_to_pwm(current_rpm / 100, priv);
 			return 0;
-		case 1:
-			/* 1 is max fan, which is 0
-			 * (no fan speed control) for hwmon
-			 */
-			*val = 0;
+		}
+		switch (priv->mode) {
+		case PWM_MODE_MAX:
+		case PWM_MODE_MANUAL:
+		case PWM_MODE_AUTO:
+			*val = priv->mode;
 			return 0;
 		default:
 			/* shouldn't happen */
@@ -2170,23 +2272,47 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 			      u32 attr, int channel, long val)
 {
+	struct hp_wmi_hwmon_priv *priv;
+	int current_rpm, ret;
+
+	priv = dev_get_drvdata(dev);
 	switch (type) {
 	case hwmon_pwm:
+		if (attr == hwmon_pwm_input) {
+			if (!is_victus_s_thermal_profile())
+				return -EOPNOTSUPP;
+			/* pwm input is invalid when not in manual mode */
+			if (priv->mode != PWM_MODE_MANUAL)
+				return -EINVAL;
+			/* ensure pwm input is within valid fan speeds */
+			priv->pwm = rpm_to_pwm(clamp_val(pwm_to_rpm(val, priv),
+							priv->min_rpm,
+							priv->max_rpm),
+						priv);
+			return hp_wmi_apply_fan_settings(priv);
+		}
 		switch (val) {
-		case 0:
-			if (is_victus_s_thermal_profile())
-				hp_wmi_get_fan_count_userdefine_trigger();
-			/* 0 is no fan speed control (max), which is 1 for us */
-			return hp_wmi_fan_speed_max_set(1);
-		case 2:
-			/* 2 is automatic speed control, which is 0 for us */
-			if (is_victus_s_thermal_profile()) {
-				hp_wmi_get_fan_count_userdefine_trigger();
-				return hp_wmi_fan_speed_max_reset();
-			} else
-				return hp_wmi_fan_speed_max_set(0);
+		case PWM_MODE_MAX:
+			priv->mode = PWM_MODE_MAX;
+			return hp_wmi_apply_fan_settings(priv);
+		case PWM_MODE_MANUAL:
+			if (!is_victus_s_thermal_profile())
+				return -EOPNOTSUPP;
+			/*
+			 * When switching to manual mode, set fan speed to
+			 * current RPM values to ensure a smooth transition.
+			 */
+			ret = hp_wmi_get_fan_speed_victus_s(channel);
+			if (ret < 0)
+				return ret;
+			current_rpm = ret;
+			priv->pwm = rpm_to_pwm(current_rpm / 100, priv);
+			priv->mode = PWM_MODE_MANUAL;
+			return hp_wmi_apply_fan_settings(priv);
+		case PWM_MODE_AUTO:
+			priv->mode = PWM_MODE_AUTO;
+			return hp_wmi_apply_fan_settings(priv);
 		default:
-			/* we don't support manual fan speed control */
 			return -EINVAL;
 		}
 	default:
@@ -2196,7 +2322,7 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 
 static const struct hwmon_channel_info * const info[] = {
 	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT),
-	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE),
+	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE | HWMON_PWM_INPUT),
 	NULL
 };
 
@@ -2211,12 +2337,57 @@ static const struct hwmon_chip_info chip_info = {
 	.info = info,
 };
 
+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;
+
+	/* Default behaviour on hwmon init is automatic mode */
+	priv->mode = PWM_MODE_AUTO;
+
+	/* Bypass all non-Victus S devices */
+	if (!is_victus_s_thermal_profile())
+		return 0;
+
+	ret = hp_wmi_perform_query(HPWMI_VICTUS_S_GET_FAN_TABLE_QUERY,
+				   HPWMI_GM, &fan_data, 4, sizeof(fan_data));
+	if (ret)
+		return ret;
+
+	fan_table = (struct victus_s_fan_table *)fan_data;
+	if (fan_table->header.num_entries == 0 ||
+		sizeof(struct victus_s_fan_table_header) +
+		sizeof(struct victus_s_fan_table_entry) * fan_table->header.num_entries >
+		sizeof(fan_data))
+		return -EINVAL;
+
+	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;
+	priv->min_rpm = min_rpm;
+	priv->max_rpm = max_rpm;
+	priv->gpu_delta = gpu_delta;
+
+	return 0;
+}
+
 static int hp_wmi_hwmon_init(void)
 {
 	struct device *dev = &hp_wmi_platform_dev->dev;
+	struct hp_wmi_hwmon_priv *priv;
 	struct device *hwmon;
+	int ret;
 
-	hwmon = devm_hwmon_device_register_with_info(dev, "hp", &hp_wmi_driver,
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = hp_wmi_setup_fan_settings(priv);
+	if (ret)
+		return ret;
+	hwmon = devm_hwmon_device_register_with_info(dev, "hp", priv,
 						     &chip_info, NULL);
 
 	if (IS_ERR(hwmon)) {
@@ -2224,6 +2395,8 @@ static int hp_wmi_hwmon_init(void)
 		return PTR_ERR(hwmon);
 	}
 
+	hp_wmi_apply_fan_settings(priv);
+
 	return 0;
 }
 
-- 
2.52.0

On Tue, 30 Dec 2025, Krishna Chomal wrote:

> Add manual fan speed control and PWM reporting for HP Victus S-series
> laptops.
> 
> While HPWMI_FAN_SPEED_SET_QUERY was previously added to reset max fan
> mode, it is actually capable of individual fan control. This patch
> implements hp_wmi_fan_speed_set() to allow manual control and hides
> PWM inputs for non-Victus devices as the query is Victus specific.
> 
> The existing hp_wmi_fan_speed_max_get() query is unreliable on Victus S
> firmware, often incorrectly reporting "Auto" mode even when "Max" is
> active. To resolve this synchronization issue, move state tracking to
> a per-device private context and apply "Auto" mode during driver
> initialization to ensure a consistent starting point.
> 
> Refactor hp_wmi_apply_fan_settings() to use an intermediate ret
> variable. This prepares the switch block for keep-alive logic being
> added in a later patch, avoiding the need for duplicated mode check.
> 
> Tested on: HP Omen 16-wf1xxx (board ID 8C78)
> 
> Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
> ---
> Changes in v2:
> - Added limits.h and minmax.h headers
> - Re-written pwm-to-rpm (and vice-versa) conversion from macros to
>   static inline functions
> - Removed redundant NULL checking of drvdata
> - Made integer promotion explicit by casting intermediate result to
>   unsigned int when calculating the GPU fan speed
> - Renamed "enforce_ctx" to "apply_fan_settings" for clarity
> - Removed unnecessary comments
> - Renamed "ctx" to "priv" as it is more consistent for drvdata
> - Added new struct victus_s_fan_table to avoid complex type casting
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 263 +++++++++++++++++++++++++------
>  1 file changed, 218 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index f4ea1ea05997..ef419575174c 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -30,6 +30,10 @@
>  #include <linux/rfkill.h>
>  #include <linux/string.h>
>  #include <linux/dmi.h>
> +#include <linux/fixp-arith.h>
> +#include <linux/limits.h>
> +#include <linux/minmax.h>
> +#include <linux/compiler_attributes.h>
>  
>  MODULE_AUTHOR("Matthew Garrett <mjg59@srcf.ucam.org>");
>  MODULE_DESCRIPTION("HP laptop WMI driver");
> @@ -190,7 +194,8 @@ enum hp_wmi_gm_commandtype {
>  	HPWMI_SET_GPU_THERMAL_MODES_QUERY	= 0x22,
>  	HPWMI_SET_POWER_LIMITS_QUERY		= 0x29,
>  	HPWMI_VICTUS_S_FAN_SPEED_GET_QUERY	= 0x2D,
> -	HPWMI_FAN_SPEED_SET_QUERY		= 0x2E,
> +	HPWMI_VICTUS_S_FAN_SPEED_SET_QUERY	= 0x2E,
> +	HPWMI_VICTUS_S_GET_FAN_TABLE_QUERY	= 0x2F,
>  };
>  
>  enum hp_wmi_command {
> @@ -348,6 +353,51 @@ static const char * const tablet_chassis_types[] = {
>  
>  #define DEVICE_MODE_TABLET	0x06
>  
> +#define CPU_FAN 0
> +#define GPU_FAN 1
> +
> +enum pwm_modes {
> +	PWM_MODE_MAX		= 0,
> +	PWM_MODE_MANUAL		= 1,
> +	PWM_MODE_AUTO		= 2,
> +};
> +
> +struct hp_wmi_hwmon_priv {
> +	u8 min_rpm;
> +	u8 max_rpm;
> +	u8 gpu_delta;
> +	u8 mode;
> +	u8 pwm;
> +};
> +
> +struct victus_s_fan_table_header {
> +	u8 unknown;
> +	u8 num_entries;
> +} __packed;

Please add #include for __packed.

> +struct victus_s_fan_table_entry {
> +	u8 cpu_rpm;
> +	u8 gpu_rpm;
> +	u8 unknown;
> +} __packed;
> +
> +struct victus_s_fan_table {
> +	struct victus_s_fan_table_header header;
> +	struct victus_s_fan_table_entry entries[];
> +} __packed;
> +
> +static inline u8 rpm_to_pwm(u8 rpm, struct hp_wmi_hwmon_priv *priv)
> +{
> +	return fixp_linear_interpolate(0, 0, priv->max_rpm, U8_MAX,
> +					clamp_val(rpm, 0, priv->max_rpm));

Please align the correctly.

> +}
> +
> +static inline u8 pwm_to_rpm(u8 pwm, struct hp_wmi_hwmon_priv *priv)
> +{
> +	return fixp_linear_interpolate(0, 0, U8_MAX, priv->max_rpm,
> +					clamp_val(pwm, 0, U8_MAX));
> +}
> +
>  /* map output size to the corresponding WMI method id */
>  static inline int encode_outsize_for_pvsz(int outsize)
>  {
> @@ -637,41 +687,53 @@ static int hp_wmi_fan_speed_max_set(int enabled)
>  	return enabled;
>  }
>  
> -static int hp_wmi_fan_speed_reset(void)
> +static int hp_wmi_fan_speed_set(struct hp_wmi_hwmon_priv *priv, u8 speed)
>  {
> -	u8 fan_speed[2] = { HP_FAN_SPEED_AUTOMATIC, HP_FAN_SPEED_AUTOMATIC };
> +	u8 fan_speed[2];
>  	int ret;
>  
> -	ret = hp_wmi_perform_query(HPWMI_FAN_SPEED_SET_QUERY, HPWMI_GM,
> -				   &fan_speed, sizeof(fan_speed), 0);
> -
> -	return ret;
> -}
> +	fan_speed[CPU_FAN] = speed;
> +	fan_speed[GPU_FAN] = speed;
>  
> -static int hp_wmi_fan_speed_max_reset(void)
> -{
> -	int ret;
> +	/*
> +	 * GPU fan speed is always a little higher than CPU fan speed, we fetch
> +	 * this delta value from the fan table during hwmon init.
> +	 * Exception: Speed is set to HP_FAN_SPEED_AUTOMATIC, to revert to
> +	 * automatic mode.
> +	 */
> +	if (speed != HP_FAN_SPEED_AUTOMATIC)
> +		fan_speed[GPU_FAN] = clamp_val((unsigned int)speed +
> +						(unsigned int)priv->gpu_delta,
> +						0, U8_MAX);

Add braces is it's multiline if.

If you use unsigned int, clamp to 0 makes no sense as those values have 
already underflowed.

You also have an alignment problem here, but this seems a cleaner way 
which doesn't have underflow issues:

	if (...) {
		int new_speed = speed + priv->gpu_delta;

		fan_speed[GPU_FAN] = clamp_val(new_speed, 0, U8_MAX);
	}

>  
> +	ret = hp_wmi_get_fan_count_userdefine_trigger();
> +	if (ret < 0)
> +		return ret;
> +	/* Max fans need to be explicitly disabled */
>  	ret = hp_wmi_fan_speed_max_set(0);
> -	if (ret)
> +	if (ret < 0)
>  		return ret;
> +	ret = hp_wmi_perform_query(HPWMI_VICTUS_S_FAN_SPEED_SET_QUERY, HPWMI_GM,
> +				   &fan_speed, sizeof(fan_speed), 0);
>  
> -	/* Disabling max fan speed on Victus s1xxx laptops needs a 2nd step: */
> -	ret = hp_wmi_fan_speed_reset();
>  	return ret;
>  }
>  
> -static int hp_wmi_fan_speed_max_get(void)
> +static int hp_wmi_fan_speed_reset(struct hp_wmi_hwmon_priv *priv)
>  {
> -	int val = 0, ret;
> +	return hp_wmi_fan_speed_set(priv, HP_FAN_SPEED_AUTOMATIC);
> +}
>  
> -	ret = hp_wmi_perform_query(HPWMI_FAN_SPEED_MAX_GET_QUERY, HPWMI_GM,
> -				   &val, zero_if_sup(val), sizeof(val));
> +static int hp_wmi_fan_speed_max_reset(struct hp_wmi_hwmon_priv *priv)
> +{
> +	int ret;
>  
> +	ret = hp_wmi_fan_speed_max_set(0);
>  	if (ret)
> -		return ret < 0 ? ret : -EINVAL;
> +		return ret;
>  
> -	return val;
> +	/* Disabling max fan speed on Victus s1xxx laptops needs a 2nd step: */
> +	return hp_wmi_fan_speed_reset(priv);
>  }
>  
>  static int __init hp_wmi_bios_2008_later(void)
> @@ -2108,12 +2170,45 @@ static struct platform_driver hp_wmi_driver __refdata = {
>  	.remove = __exit_p(hp_wmi_bios_remove),
>  };
>  
> +static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
> +{
> +	int ret;
> +
> +	switch (priv->mode) {
> +	case PWM_MODE_MAX:
> +		if (is_victus_s_thermal_profile())
> +			hp_wmi_get_fan_count_userdefine_trigger();
> +		ret = hp_wmi_fan_speed_max_set(1);
> +		return ret;
> +	case PWM_MODE_MANUAL:
> +		if (!is_victus_s_thermal_profile())
> +			return -EOPNOTSUPP;
> +		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
> +		return ret;
> +	case PWM_MODE_AUTO:
> +		if (is_victus_s_thermal_profile()) {
> +			hp_wmi_get_fan_count_userdefine_trigger();
> +			ret = hp_wmi_fan_speed_max_reset(priv);
> +		} else {
> +			ret = hp_wmi_fan_speed_max_set(0);
> +		}
> +		return ret;
> +	default:
> +		/* shouldn't happen */
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static umode_t hp_wmi_hwmon_is_visible(const void *data,
>  				       enum hwmon_sensor_types type,
>  				       u32 attr, int channel)
>  {
>  	switch (type) {
>  	case hwmon_pwm:
> +		if (attr == hwmon_pwm_input && !is_victus_s_thermal_profile())
> +			return 0;
>  		return 0644;
>  	case hwmon_fan:
>  		if (is_victus_s_thermal_profile()) {
> @@ -2134,8 +2229,10 @@ static umode_t hp_wmi_hwmon_is_visible(const void *data,
>  static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  			     u32 attr, int channel, long *val)
>  {
> -	int ret;
> +	struct hp_wmi_hwmon_priv *priv;
> +	int current_rpm, ret;
>  
> +	priv = dev_get_drvdata(dev);
>  	switch (type) {
>  	case hwmon_fan:
>  		if (is_victus_s_thermal_profile())
> @@ -2147,16 +2244,21 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  		*val = ret;
>  		return 0;
>  	case hwmon_pwm:
> -		switch (hp_wmi_fan_speed_max_get()) {
> -		case 0:
> -			/* 0 is automatic fan, which is 2 for hwmon */
> -			*val = 2;
> +		if (attr == hwmon_pwm_input) {
> +			if (!is_victus_s_thermal_profile())
> +				return -EOPNOTSUPP;

Add an empty line here.

> +			ret = hp_wmi_get_fan_speed_victus_s(channel);
> +			if (ret < 0)
> +				return ret;
> +			current_rpm = ret;

I'm not sure if using ret here makes things better or not, I'd prefer just 
assigning directly to current_rpm without ret as intermediate var.

> +			*val = rpm_to_pwm(current_rpm / 100, priv);
>  			return 0;
> -		case 1:
> -			/* 1 is max fan, which is 0
> -			 * (no fan speed control) for hwmon
> -			 */
> -			*val = 0;
> +		}
> +		switch (priv->mode) {
> +		case PWM_MODE_MAX:
> +		case PWM_MODE_MANUAL:
> +		case PWM_MODE_AUTO:
> +			*val = priv->mode;
>  			return 0;
>  		default:
>  			/* shouldn't happen */
> @@ -2170,23 +2272,47 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>  			      u32 attr, int channel, long val)
>  {
> +	struct hp_wmi_hwmon_priv *priv;
> +	int current_rpm, ret;
> +
> +	priv = dev_get_drvdata(dev);
>  	switch (type) {
>  	case hwmon_pwm:
> +		if (attr == hwmon_pwm_input) {
> +			if (!is_victus_s_thermal_profile())
> +				return -EOPNOTSUPP;
> +			/* pwm input is invalid when not in manual mode */

PWM (capitalize textual/comment "pwm"s correctly please).

> +			if (priv->mode != PWM_MODE_MANUAL)
> +				return -EINVAL;

ADd empty line here.

> +			/* ensure pwm input is within valid fan speeds */

PWM

> +			priv->pwm = rpm_to_pwm(clamp_val(pwm_to_rpm(val, priv),
> +							priv->min_rpm,
> +							priv->max_rpm),

These look misaligned.

I suggest you split this to multiple lines though, it will likely be 
easier to read that way.

> +						priv);
> +			return hp_wmi_apply_fan_settings(priv);
> +		}
>  		switch (val) {
> -		case 0:
> -			if (is_victus_s_thermal_profile())
> -				hp_wmi_get_fan_count_userdefine_trigger();
> -			/* 0 is no fan speed control (max), which is 1 for us */
> -			return hp_wmi_fan_speed_max_set(1);
> -		case 2:
> -			/* 2 is automatic speed control, which is 0 for us */
> -			if (is_victus_s_thermal_profile()) {
> -				hp_wmi_get_fan_count_userdefine_trigger();
> -				return hp_wmi_fan_speed_max_reset();
> -			} else
> -				return hp_wmi_fan_speed_max_set(0);
> +		case PWM_MODE_MAX:
> +			priv->mode = PWM_MODE_MAX;
> +			return hp_wmi_apply_fan_settings(priv);
> +		case PWM_MODE_MANUAL:
> +			if (!is_victus_s_thermal_profile())
> +				return -EOPNOTSUPP;
> +			/*
> +			 * When switching to manual mode, set fan speed to
> +			 * current RPM values to ensure a smooth transition.
> +			 */
> +			ret = hp_wmi_get_fan_speed_victus_s(channel);

Assign directly to current_rpm ?

> +			if (ret < 0)
> +				return ret;
> +			current_rpm = ret;
> +			priv->pwm = rpm_to_pwm(current_rpm / 100, priv);
> +			priv->mode = PWM_MODE_MANUAL;
> +			return hp_wmi_apply_fan_settings(priv);
> +		case PWM_MODE_AUTO:
> +			priv->mode = PWM_MODE_AUTO;
> +			return hp_wmi_apply_fan_settings(priv);
>  		default:
> -			/* we don't support manual fan speed control */
>  			return -EINVAL;
>  		}
>  	default:
> @@ -2196,7 +2322,7 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>  
>  static const struct hwmon_channel_info * const info[] = {
>  	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT),
> -	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE),
> +	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE | HWMON_PWM_INPUT),
>  	NULL
>  };
>  
> @@ -2211,12 +2337,57 @@ static const struct hwmon_chip_info chip_info = {
>  	.info = info,
>  };
>  
> +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;
> +
> +	/* Default behaviour on hwmon init is automatic mode */
> +	priv->mode = PWM_MODE_AUTO;
> +
> +	/* Bypass all non-Victus S devices */
> +	if (!is_victus_s_thermal_profile())
> +		return 0;
> +
> +	ret = hp_wmi_perform_query(HPWMI_VICTUS_S_GET_FAN_TABLE_QUERY,
> +				   HPWMI_GM, &fan_data, 4, sizeof(fan_data));
> +	if (ret)
> +		return ret;
> +
> +	fan_table = (struct victus_s_fan_table *)fan_data;
> +	if (fan_table->header.num_entries == 0 ||
> +		sizeof(struct victus_s_fan_table_header) +
> +		sizeof(struct victus_s_fan_table_entry) * fan_table->header.num_entries >
> +		sizeof(fan_data))

Badly misaligned.

Splitting at > is somewhat misleading so I'd prefer to avoid it (you can 
use up to 100 chars per line if needed).

> +		return -EINVAL;
> +
> +	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;
> +	priv->min_rpm = min_rpm;
> +	priv->max_rpm = max_rpm;
> +	priv->gpu_delta = gpu_delta;
> +
> +	return 0;
> +}
> +
>  static int hp_wmi_hwmon_init(void)
>  {
>  	struct device *dev = &hp_wmi_platform_dev->dev;
> +	struct hp_wmi_hwmon_priv *priv;
>  	struct device *hwmon;
> +	int ret;
>  
> -	hwmon = devm_hwmon_device_register_with_info(dev, "hp", &hp_wmi_driver,
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	ret = hp_wmi_setup_fan_settings(priv);
> +	if (ret)
> +		return ret;
> +	hwmon = devm_hwmon_device_register_with_info(dev, "hp", priv,
>  						     &chip_info, NULL);
>  
>  	if (IS_ERR(hwmon)) {
> @@ -2224,6 +2395,8 @@ static int hp_wmi_hwmon_init(void)
>  		return PTR_ERR(hwmon);
>  	}
>  
> +	hp_wmi_apply_fan_settings(priv);
> +
>  	return 0;
>  }
>  
> 

-- 
 i.

On Mon, Jan 12, 2026 at 05:13:05PM +0200, Ilpo Järvinen wrote:
>On Tue, 30 Dec 2025, Krishna Chomal wrote:
>
[snip]
>>  #include <linux/string.h>
>>  #include <linux/dmi.h>
>> +#include <linux/fixp-arith.h>
>> +#include <linux/limits.h>
>> +#include <linux/minmax.h>
>> +#include <linux/compiler_attributes.h>
>>
[snip]
>> +
>> +struct victus_s_fan_table_header {
>> +	u8 unknown;
>> +	u8 num_entries;
>> +} __packed;
>
>Please add #include for __packed.
>

__packed is defined in compiler_attributes.h, which is included in this
patch. Please let me know if there are any other headers that should be
included.

>> +struct victus_s_fan_table_entry {
>> +	u8 cpu_rpm;
>> +	u8 gpu_rpm;
>> +	u8 unknown;
>> +} __packed;
>> +
>> +struct victus_s_fan_table {
>> +	struct victus_s_fan_table_header header;
>> +	struct victus_s_fan_table_entry entries[];
>> +} __packed;
>> +
>> +static inline u8 rpm_to_pwm(u8 rpm, struct hp_wmi_hwmon_priv *priv)
>> +{
>> +	return fixp_linear_interpolate(0, 0, priv->max_rpm, U8_MAX,
>> +					clamp_val(rpm, 0, priv->max_rpm));
>
>Please align the correctly.
>

Apologies for the bad alignment of multi-line function calls and
if-statements in this patch. I have noted them and will fix them all in
v3.

[snip]
>> -static int hp_wmi_fan_speed_max_reset(void)
>> -{
>> -	int ret;
>> +	/*
>> +	 * GPU fan speed is always a little higher than CPU fan speed, we fetch
>> +	 * this delta value from the fan table during hwmon init.
>> +	 * Exception: Speed is set to HP_FAN_SPEED_AUTOMATIC, to revert to
>> +	 * automatic mode.
>> +	 */
>> +	if (speed != HP_FAN_SPEED_AUTOMATIC)
>> +		fan_speed[GPU_FAN] = clamp_val((unsigned int)speed +
>> +						(unsigned int)priv->gpu_delta,
>> +						0, U8_MAX);
>
>Add braces is it's multiline if.
>
>If you use unsigned int, clamp to 0 makes no sense as those values have
>already underflowed.
>
>You also have an alignment problem here, but this seems a cleaner way
>which doesn't have underflow issues:
>
>	if (...) {
>		int new_speed = speed + priv->gpu_delta;
>
>		fan_speed[GPU_FAN] = clamp_val(new_speed, 0, U8_MAX);
>	}
>

Understood. I will introduce a new "gpu_speed" variable and follow this
style in v3.

[snip]
>> @@ -2147,16 +2244,21 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>>  		*val = ret;
>>  		return 0;
>>  	case hwmon_pwm:
>> -		switch (hp_wmi_fan_speed_max_get()) {
>> -		case 0:
>> -			/* 0 is automatic fan, which is 2 for hwmon */
>> -			*val = 2;
>> +		if (attr == hwmon_pwm_input) {
>> +			if (!is_victus_s_thermal_profile())
>> +				return -EOPNOTSUPP;
>
>Add an empty line here.
>

Will do.

>> +			ret = hp_wmi_get_fan_speed_victus_s(channel);
>> +			if (ret < 0)
>> +				return ret;
>> +			current_rpm = ret;
>
>I'm not sure if using ret here makes things better or not, I'd prefer just
>assigning directly to current_rpm without ret as intermediate var.
>

Understood. I will directly use current_rpm then.

[snip]
>>  static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>>  			      u32 attr, int channel, long val)
>>  {
>> +	struct hp_wmi_hwmon_priv *priv;
>> +	int current_rpm, ret;
>> +
>> +	priv = dev_get_drvdata(dev);
>>  	switch (type) {
>>  	case hwmon_pwm:
>> +		if (attr == hwmon_pwm_input) {
>> +			if (!is_victus_s_thermal_profile())
>> +				return -EOPNOTSUPP;
>> +			/* pwm input is invalid when not in manual mode */
>
>PWM (capitalize textual/comment "pwm"s correctly please).
>

Corrected.

>> +			if (priv->mode != PWM_MODE_MANUAL)
>> +				return -EINVAL;
>
>ADd empty line here.
>

Added.

>> +			/* ensure pwm input is within valid fan speeds */
>
>PWM
>

Fixed.

>> +			priv->pwm = rpm_to_pwm(clamp_val(pwm_to_rpm(val, priv),
>> +							priv->min_rpm,
>> +							priv->max_rpm),
>
>These look misaligned.
>
>I suggest you split this to multiple lines though, it will likely be
>easier to read that way.
>

Agreed, this is too much nesting. I will split it into separate statements
for each function call.

>> +						priv);
>> +			return hp_wmi_apply_fan_settings(priv);
>> +		}
>>  		switch (val) {
>> -		case 0:
>> -			if (is_victus_s_thermal_profile())
>> -				hp_wmi_get_fan_count_userdefine_trigger();
>> -			/* 0 is no fan speed control (max), which is 1 for us */
>> -			return hp_wmi_fan_speed_max_set(1);
>> -		case 2:
>> -			/* 2 is automatic speed control, which is 0 for us */
>> -			if (is_victus_s_thermal_profile()) {
>> -				hp_wmi_get_fan_count_userdefine_trigger();
>> -				return hp_wmi_fan_speed_max_reset();
>> -			} else
>> -				return hp_wmi_fan_speed_max_set(0);
>> +		case PWM_MODE_MAX:
>> +			priv->mode = PWM_MODE_MAX;
>> +			return hp_wmi_apply_fan_settings(priv);
>> +		case PWM_MODE_MANUAL:
>> +			if (!is_victus_s_thermal_profile())
>> +				return -EOPNOTSUPP;
>> +			/*
>> +			 * When switching to manual mode, set fan speed to
>> +			 * current RPM values to ensure a smooth transition.
>> +			 */
>> +			ret = hp_wmi_get_fan_speed_victus_s(channel);
>
>Assign directly to current_rpm ?
>

Yes, will do in v3.

>> +			if (ret < 0)
>> +				return ret;
>> +			current_rpm = ret;
>> +			priv->pwm = rpm_to_pwm(current_rpm / 100, priv);
>> +			priv->mode = PWM_MODE_MANUAL;
>> +			return hp_wmi_apply_fan_settings(priv);
>> +		case PWM_MODE_AUTO:
>> +			priv->mode = PWM_MODE_AUTO;
>> +			return hp_wmi_apply_fan_settings(priv);
>>  		default:
>> -			/* we don't support manual fan speed control */
>>  			return -EINVAL;
>>  		}
>>  	default:
>> @@ -2196,7 +2322,7 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>>
>>  static const struct hwmon_channel_info * const info[] = {
>>  	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT),
>> -	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE),
>> +	HWMON_CHANNEL_INFO(pwm, HWMON_PWM_ENABLE | HWMON_PWM_INPUT),
>>  	NULL
>>  };
>>
>> @@ -2211,12 +2337,57 @@ static const struct hwmon_chip_info chip_info = {
>>  	.info = info,
>>  };
>>
>> +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;
>> +
>> +	/* Default behaviour on hwmon init is automatic mode */
>> +	priv->mode = PWM_MODE_AUTO;
>> +
>> +	/* Bypass all non-Victus S devices */
>> +	if (!is_victus_s_thermal_profile())
>> +		return 0;
>> +
>> +	ret = hp_wmi_perform_query(HPWMI_VICTUS_S_GET_FAN_TABLE_QUERY,
>> +				   HPWMI_GM, &fan_data, 4, sizeof(fan_data));
>> +	if (ret)
>> +		return ret;
>> +
>> +	fan_table = (struct victus_s_fan_table *)fan_data;
>> +	if (fan_table->header.num_entries == 0 ||
>> +		sizeof(struct victus_s_fan_table_header) +
>> +		sizeof(struct victus_s_fan_table_entry) * fan_table->header.num_entries >
>> +		sizeof(fan_data))
>
>Badly misaligned.
>
>Splitting at > is somewhat misleading so I'd prefer to avoid it (you can
>use up to 100 chars per line if needed).
>

Ok, will fix the alignment and ensure to keep the > in a single line.

On Mon, 12 Jan 2026, Krishna Chomal wrote:

> On Mon, Jan 12, 2026 at 05:13:05PM +0200, Ilpo Järvinen wrote:
> > On Tue, 30 Dec 2025, Krishna Chomal wrote:
> > 
> [snip]
> > >  #include <linux/string.h>
> > >  #include <linux/dmi.h>
> > > +#include <linux/fixp-arith.h>
> > > +#include <linux/limits.h>
> > > +#include <linux/minmax.h>
> > > +#include <linux/compiler_attributes.h>
> > > 
> [snip]

Hmm, these should be ordered alphabetically and if the existing one 
aren't, try your best (or add a patch to fix the order for all).

> > > +
> > > +struct victus_s_fan_table_header {
> > > +	u8 unknown;
> > > +	u8 num_entries;
> > > +} __packed;
> > 
> > Please add #include for __packed.
> > 
> 
> __packed is defined in compiler_attributes.h, which is included in this
> patch. Please let me know if there are any other headers that should be
> included.

Sorry, I don't know why I didn't notice it (too much reviewing in a row or 
too much multi-tasking I guess :-)).


-- 
 i.

On Mon, Jan 12, 2026 at 08:08:29PM +0200, Ilpo Järvinen wrote:
>On Mon, 12 Jan 2026, Krishna Chomal wrote:
>
>> On Mon, Jan 12, 2026 at 05:13:05PM +0200, Ilpo Järvinen wrote:
>> > On Tue, 30 Dec 2025, Krishna Chomal wrote:
>> >
>> [snip]
>> > >  #include <linux/string.h>
>> > >  #include <linux/dmi.h>
>> > > +#include <linux/fixp-arith.h>
>> > > +#include <linux/limits.h>
>> > > +#include <linux/minmax.h>
>> > > +#include <linux/compiler_attributes.h>
>> > >
>> [snip]
>
>Hmm, these should be ordered alphabetically and if the existing one
>aren't, try your best (or add a patch to fix the order for all).
>

Ok, then I will add a separate patch to this series to fix the ordering
for all includes.

>> > > +
>> > > +struct victus_s_fan_table_header {
>> > > +	u8 unknown;
>> > > +	u8 num_entries;
>> > > +} __packed;
>> >
>> > Please add #include for __packed.
>> >
>>
>> __packed is defined in compiler_attributes.h, which is included in this
>> patch. Please let me know if there are any other headers that should be
>> included.
>
>Sorry, I don't know why I didn't notice it (too much reviewing in a row or
>too much multi-tasking I guess :-)).
>

No problem, I understand. Thanks for the quick feedback.

The firmware on some HP laptops automatically reverts the fan speed
control to "Auto" mode after a 120 second timeout window.

To ensure that the user-selected fan profile (Max/Manual) persists,
implement a keep-alive mechanism that periodically refreshes the fan
mode trigger before the timeout occurs.

- Introduce a delayed workqueue to trigger the fan mode refresh every 90
  seconds, ensuring the system maintains the correct fan mode setting.
- Integrate the refresh mechanism into hp_wmi_hwmon_enforce_ctx() to
  start, update or cancel the keep-alive process based on the current
  fan mode.

This ensures that the driver stays in sync with the hardware.

Tested on: HP Omen 16-wf1xxx (board ID 8C78)

Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
---
Changes in v2:
- Explicitly specify time unit in KEEP_ALIVE_DELAY macro
- Handle work rescheduling directly in switch/case
---
 drivers/platform/x86/hp/hp-wmi.c | 46 +++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index ef419575174c..cf9327e1f40e 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -34,6 +34,7 @@
 #include <linux/limits.h>
 #include <linux/minmax.h>
 #include <linux/compiler_attributes.h>
+#include <linux/workqueue.h>
 
 MODULE_AUTHOR("Matthew Garrett <mjg59@srcf.ucam.org>");
 MODULE_DESCRIPTION("HP laptop WMI driver");
@@ -368,6 +369,7 @@ struct hp_wmi_hwmon_priv {
 	u8 gpu_delta;
 	u8 mode;
 	u8 pwm;
+	struct delayed_work keep_alive_dwork;
 };
 
 struct victus_s_fan_table_header {
@@ -386,6 +388,12 @@ struct victus_s_fan_table {
 	struct victus_s_fan_table_entry entries[];
 } __packed;
 
+/*
+ * 90s delay to prevent the firmware from resetting fan mode after fixed
+ * 120s timeout
+ */
+#define KEEP_ALIVE_DELAY_SECS     90
+
 static inline u8 rpm_to_pwm(u8 rpm, struct hp_wmi_hwmon_priv *priv)
 {
 	return fixp_linear_interpolate(0, 0, priv->max_rpm, U8_MAX,
@@ -2093,6 +2101,7 @@ static int __init hp_wmi_bios_setup(struct platform_device *device)
 static void __exit hp_wmi_bios_remove(struct platform_device *device)
 {
 	int i;
+	struct hp_wmi_hwmon_priv *ctx;
 
 	for (i = 0; i < rfkill2_count; i++) {
 		rfkill_unregister(rfkill2[i].rfkill);
@@ -2111,6 +2120,10 @@ static void __exit hp_wmi_bios_remove(struct platform_device *device)
 		rfkill_unregister(wwan_rfkill);
 		rfkill_destroy(wwan_rfkill);
 	}
+
+	ctx = platform_get_drvdata(device);
+	if (ctx)
+		cancel_delayed_work_sync(&ctx->keep_alive_dwork);
 }
 
 static int hp_wmi_resume_handler(struct device *device)
@@ -2179,12 +2192,20 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 		if (is_victus_s_thermal_profile())
 			hp_wmi_get_fan_count_userdefine_trigger();
 		ret = hp_wmi_fan_speed_max_set(1);
-		return ret;
+		if (ret < 0)
+			return ret;
+		schedule_delayed_work(&priv->keep_alive_dwork,
+				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
+		return 0;
 	case PWM_MODE_MANUAL:
 		if (!is_victus_s_thermal_profile())
 			return -EOPNOTSUPP;
 		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
-		return ret;
+		if (ret < 0)
+			return ret;
+		schedule_delayed_work(&priv->keep_alive_dwork,
+				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
+		return 0;
 	case PWM_MODE_AUTO:
 		if (is_victus_s_thermal_profile()) {
 			hp_wmi_get_fan_count_userdefine_trigger();
@@ -2192,7 +2213,10 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 		} else {
 			ret = hp_wmi_fan_speed_max_set(0);
 		}
-		return ret;
+		if (ret < 0)
+			return ret;
+		cancel_delayed_work_sync(&priv->keep_alive_dwork);
+		return 0;
 	default:
 		/* shouldn't happen */
 		return -EINVAL;
@@ -2337,6 +2361,20 @@ static const struct hwmon_chip_info chip_info = {
 	.info = info,
 };
 
+static void hp_wmi_hwmon_keep_alive_handler(struct work_struct *work)
+{
+	struct delayed_work *dwork;
+	struct hp_wmi_hwmon_priv *ctx;
+
+	dwork = to_delayed_work(work);
+	ctx = container_of(dwork, struct hp_wmi_hwmon_priv, keep_alive_dwork);
+	/*
+	 * Re-apply the current hwmon context settings.
+	 * NOTE: hp_wmi_apply_fan_settings will handle the re-scheduling.
+	 */
+	hp_wmi_apply_fan_settings(ctx);
+}
+
 static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
 {
 	u8 fan_data[128] = { 0 };
@@ -2395,6 +2433,8 @@ static int hp_wmi_hwmon_init(void)
 		return PTR_ERR(hwmon);
 	}
 
+	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);
 
 	return 0;
-- 
2.52.0

On Tue, 30 Dec 2025, Krishna Chomal wrote:

> The firmware on some HP laptops automatically reverts the fan speed
> control to "Auto" mode after a 120 second timeout window.
> 
> To ensure that the user-selected fan profile (Max/Manual) persists,
> implement a keep-alive mechanism that periodically refreshes the fan
> mode trigger before the timeout occurs.
> 
> - Introduce a delayed workqueue to trigger the fan mode refresh every 90
>   seconds, ensuring the system maintains the correct fan mode setting.
> - Integrate the refresh mechanism into hp_wmi_hwmon_enforce_ctx() to
>   start, update or cancel the keep-alive process based on the current
>   fan mode.
> 
> This ensures that the driver stays in sync with the hardware.
> 
> Tested on: HP Omen 16-wf1xxx (board ID 8C78)
> 
> Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
> ---
> Changes in v2:
> - Explicitly specify time unit in KEEP_ALIVE_DELAY macro
> - Handle work rescheduling directly in switch/case
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 46 +++++++++++++++++++++++++++++---
>  1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index ef419575174c..cf9327e1f40e 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -34,6 +34,7 @@
>  #include <linux/limits.h>
>  #include <linux/minmax.h>
>  #include <linux/compiler_attributes.h>
> +#include <linux/workqueue.h>
>  
>  MODULE_AUTHOR("Matthew Garrett <mjg59@srcf.ucam.org>");
>  MODULE_DESCRIPTION("HP laptop WMI driver");
> @@ -368,6 +369,7 @@ struct hp_wmi_hwmon_priv {
>  	u8 gpu_delta;
>  	u8 mode;
>  	u8 pwm;
> +	struct delayed_work keep_alive_dwork;
>  };
>  
>  struct victus_s_fan_table_header {
> @@ -386,6 +388,12 @@ struct victus_s_fan_table {
>  	struct victus_s_fan_table_entry entries[];
>  } __packed;
>  
> +/*
> + * 90s delay to prevent the firmware from resetting fan mode after fixed
> + * 120s timeout
> + */
> +#define KEEP_ALIVE_DELAY_SECS     90
> +
>  static inline u8 rpm_to_pwm(u8 rpm, struct hp_wmi_hwmon_priv *priv)
>  {
>  	return fixp_linear_interpolate(0, 0, priv->max_rpm, U8_MAX,
> @@ -2093,6 +2101,7 @@ static int __init hp_wmi_bios_setup(struct platform_device *device)
>  static void __exit hp_wmi_bios_remove(struct platform_device *device)
>  {
>  	int i;
> +	struct hp_wmi_hwmon_priv *ctx;

Can't we just call this priv everywhere as per the normal custom?

>  
>  	for (i = 0; i < rfkill2_count; i++) {
>  		rfkill_unregister(rfkill2[i].rfkill);
> @@ -2111,6 +2120,10 @@ static void __exit hp_wmi_bios_remove(struct platform_device *device)
>  		rfkill_unregister(wwan_rfkill);
>  		rfkill_destroy(wwan_rfkill);
>  	}
> +
> +	ctx = platform_get_drvdata(device);
> +	if (ctx)
> +		cancel_delayed_work_sync(&ctx->keep_alive_dwork);
>  }
>  
>  static int hp_wmi_resume_handler(struct device *device)
> @@ -2179,12 +2192,20 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  		if (is_victus_s_thermal_profile())
>  			hp_wmi_get_fan_count_userdefine_trigger();
>  		ret = hp_wmi_fan_speed_max_set(1);
> -		return ret;
> +		if (ret < 0)
> +			return ret;
> +		schedule_delayed_work(&priv->keep_alive_dwork,
> +				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
> +		return 0;
>  	case PWM_MODE_MANUAL:
>  		if (!is_victus_s_thermal_profile())
>  			return -EOPNOTSUPP;
>  		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
> -		return ret;
> +		if (ret < 0)
> +			return ret;
> +		schedule_delayed_work(&priv->keep_alive_dwork,
> +				      secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
> +		return 0;
>  	case PWM_MODE_AUTO:
>  		if (is_victus_s_thermal_profile()) {
>  			hp_wmi_get_fan_count_userdefine_trigger();
> @@ -2192,7 +2213,10 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  		} else {
>  			ret = hp_wmi_fan_speed_max_set(0);
>  		}
> -		return ret;
> +		if (ret < 0)
> +			return ret;
> +		cancel_delayed_work_sync(&priv->keep_alive_dwork);
> +		return 0;
>  	default:
>  		/* shouldn't happen */
>  		return -EINVAL;
> @@ -2337,6 +2361,20 @@ static const struct hwmon_chip_info chip_info = {
>  	.info = info,
>  };
>  
> +static void hp_wmi_hwmon_keep_alive_handler(struct work_struct *work)
> +{
> +	struct delayed_work *dwork;
> +	struct hp_wmi_hwmon_priv *ctx;
> +
> +	dwork = to_delayed_work(work);
> +	ctx = container_of(dwork, struct hp_wmi_hwmon_priv, keep_alive_dwork);
> +	/*
> +	 * Re-apply the current hwmon context settings.
> +	 * NOTE: hp_wmi_apply_fan_settings will handle the re-scheduling.
> +	 */
> +	hp_wmi_apply_fan_settings(ctx);
> +}
> +
>  static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  {
>  	u8 fan_data[128] = { 0 };
> @@ -2395,6 +2433,8 @@ static int hp_wmi_hwmon_init(void)
>  		return PTR_ERR(hwmon);
>  	}
>  
> +	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);
>  
>  	return 0;
> 

-- 
 i.

On Mon, Jan 12, 2026 at 05:14:12PM +0200, Ilpo Järvinen wrote:
[snip]
>>  static void __exit hp_wmi_bios_remove(struct platform_device *device)
>>  {
>>  	int i;
>> +	struct hp_wmi_hwmon_priv *ctx;
>
>Can't we just call this priv everywhere as per the normal custom?
>

Yes this was a mistake, I forgot to rename "ctx" to "priv" in v2, sorry.
I will ensure to fix this in v3.