mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] platform/x86: hp-wmi: Disable hwmon attributes when no fans are usable
@ 2026-10-17 13:24 Krishna Chomal
  2026-10-19  6:22 ` Ilpo Järvinen
  0 siblings, 1 reply; 3+ messages in thread
From: Krishna Chomal @ 2026-10-17 13:24 UTC (permalink / raw)
  To: ilpo.jarvinen, hansg
  Cc: platform-driver-x86, linux-kernel, Krishna Chomal, Lorenzo Collevecchio

Currently, hp_wmi_hwmon_is_visible() checks whether fan RPM queries
succeed before exposing fan1_input and fan2_input, but pwm1_enable
remains visible always, even when fan support is unavailable. On some
devices, such as certain Omnibook models, the firmware does not provide
functional WMI fan methods. Exposing pwm1_enable on these devices can
mislead userspace and cause fan mode changes to fail.

Additionally, some firmware implementations provide stub methods
that return an all-zero buffer instead of an error. In this case,
hp_wmi_get_active_fan_speed() and hp_wmi_get_fan_speed() can report 0
RPM even when the fan is physically spinning.

Use the result of fan count query (fetched from firmware once in
hp_wmi_setup_fan_settings(), and cached in hwmon private data) to
determine whether fan related attributes should be exposed.

The fan count query can have the following outcomes:
* Negative: The WMI query failed. Other fan-related queries are also
  expected to fail, so disable fan support.
* Zero (stub firmware): The methods return zero-filled responses without
  providing functional fan support, so disable fan support.
* Zero (genuine firmware): No fans are exposed through WMI, so disable
  fan support.
* Positive (1 or 2): Firmware reports one or more fans, so allow fan
  support. Per-channel RPM queries continue to determine which
  individual fan input attributes are exposed.

When fan support is unavailable, hide the fan and PWM attributes and
skip applying the initial fan settings.

Tested on: HP OmniBook 5 Laptop 16-ag1xxx (board: 8DF4)

Tested-by: Lorenzo Collevecchio <lorenzocollevecchio17@gmail.com>
Reported-by: Lorenzo Collevecchio <lorenzocollevecchio17@gmail.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222138
Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 0eac5e1d9cdb..52077833ac89 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -651,6 +651,7 @@ enum pwm_modes {
 
 struct hp_wmi_hwmon_priv {
 	struct mutex lock;	/* protects mode, pwm */
+	int fan_count;
 	u8 min_rpm;
 	u8 max_rpm;
 	u8 mode;
@@ -2950,12 +2951,18 @@ static umode_t hp_wmi_hwmon_is_visible(const void *data,
 				       enum hwmon_sensor_types type,
 				       u32 attr, int channel)
 {
+	const struct hp_wmi_hwmon_priv *priv = data;
+
 	switch (type) {
 	case hwmon_pwm:
+		if (priv->fan_count <= 0)
+			return 0;
 		if (attr == hwmon_pwm_input && !hp_wmi_fan_control_supported())
 			return 0;
 		return 0644;
 	case hwmon_fan:
+		if (priv->fan_count <= 0)
+			return 0;
 		if (hp_wmi_fan_control_supported()) {
 			if (hp_wmi_get_active_fan_speed(channel) >= 0)
 				return 0444;
@@ -3124,6 +3131,7 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
 
 	/* Default behaviour on hwmon init is automatic mode */
 	priv->mode = PWM_MODE_AUTO;
+	priv->fan_count = hp_wmi_get_fan_count_userdefine_trigger();
 
 	/* Bypass devices without fan control support. */
 	if (!hp_wmi_fan_table_supported())
@@ -3208,6 +3216,11 @@ static int hp_wmi_hwmon_init(void)
 		return PTR_ERR(hwmon);
 	}
 
+	if (priv->fan_count <= 0) {
+		dev_info(dev, "No usable fans reported by WMI, disabling fan support\n");
+		return 0;
+	}
+
 	ret = hp_wmi_apply_fan_settings(priv);
 	if (ret)
 		dev_warn(dev, "Failed to apply initial fan settings: %d\n", ret);
-- 
2.56.0


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

* Re: [PATCH] platform/x86: hp-wmi: Disable hwmon attributes when no fans are usable
  2026-10-17 13:24 [PATCH] platform/x86: hp-wmi: Disable hwmon attributes when no fans are usable Krishna Chomal
@ 2026-10-19  6:22 ` Ilpo Järvinen
  2026-10-19  7:40   ` Krishna Chomal
  0 siblings, 1 reply; 3+ messages in thread
From: Ilpo Järvinen @ 2026-10-19  6:22 UTC (permalink / raw)
  To: Krishna Chomal
  Cc: Hans de Goede, platform-driver-x86, LKML, Lorenzo Collevecchio

On Sat, 17 Oct 2026, Krishna Chomal wrote:

> Currently, hp_wmi_hwmon_is_visible() checks whether fan RPM queries
> succeed before exposing fan1_input and fan2_input, but pwm1_enable
> remains visible always, even when fan support is unavailable. 

Why does it yield different result from is_visible?

...Somehow I find starting with this more confusing than helpful.

> On some
> devices, such as certain Omnibook models, the firmware does not provide
> functional WMI fan methods. Exposing pwm1_enable on these devices can
> mislead userspace and cause fan mode changes to fail.
> 
> Additionally, some firmware implementations provide stub methods
> that return an all-zero buffer instead of an error. In this case,
> hp_wmi_get_active_fan_speed() and hp_wmi_get_fan_speed() can report 0
> RPM even when the fan is physically spinning.
> 
> Use the result of fan count query (fetched from firmware once in
> hp_wmi_setup_fan_settings(), and cached in hwmon private data) to
> determine whether fan related attributes should be exposed.
> 
> The fan count query can have the following outcomes:
> * Negative: The WMI query failed. Other fan-related queries are also
>   expected to fail, so disable fan support.
> * Zero (stub firmware): The methods return zero-filled responses without
>   providing functional fan support, so disable fan support.
> * Zero (genuine firmware): No fans are exposed through WMI, so disable
>   fan support.
> * Positive (1 or 2): Firmware reports one or more fans, so allow fan
>   support. Per-channel RPM queries continue to determine which
>   individual fan input attributes are exposed.
> 
> When fan support is unavailable, hide the fan and PWM attributes and
> skip applying the initial fan settings.
> 
> Tested on: HP OmniBook 5 Laptop 16-ag1xxx (board: 8DF4)
> 
> Tested-by: Lorenzo Collevecchio <lorenzocollevecchio17@gmail.com>
> Reported-by: Lorenzo Collevecchio <lorenzocollevecchio17@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222138
> Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 0eac5e1d9cdb..52077833ac89 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -651,6 +651,7 @@ enum pwm_modes {
>  
>  struct hp_wmi_hwmon_priv {
>  	struct mutex lock;	/* protects mode, pwm */
> +	int fan_count;
>  	u8 min_rpm;
>  	u8 max_rpm;
>  	u8 mode;
> @@ -2950,12 +2951,18 @@ static umode_t hp_wmi_hwmon_is_visible(const void *data,
>  				       enum hwmon_sensor_types type,
>  				       u32 attr, int channel)
>  {
> +	const struct hp_wmi_hwmon_priv *priv = data;
> +
>  	switch (type) {
>  	case hwmon_pwm:
> +		if (priv->fan_count <= 0)
> +			return 0;
>  		if (attr == hwmon_pwm_input && !hp_wmi_fan_control_supported())
>  			return 0;
>  		return 0644;
>  	case hwmon_fan:
> +		if (priv->fan_count <= 0)
> +			return 0;
>  		if (hp_wmi_fan_control_supported()) {
>  			if (hp_wmi_get_active_fan_speed(channel) >= 0)
>  				return 0444;
> @@ -3124,6 +3131,7 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
>  
>  	/* Default behaviour on hwmon init is automatic mode */
>  	priv->mode = PWM_MODE_AUTO;
> +	priv->fan_count = hp_wmi_get_fan_count_userdefine_trigger();
>  
>  	/* Bypass devices without fan control support. */
>  	if (!hp_wmi_fan_table_supported())
> @@ -3208,6 +3216,11 @@ static int hp_wmi_hwmon_init(void)
>  		return PTR_ERR(hwmon);
>  	}
>  
> +	if (priv->fan_count <= 0) {
> +		dev_info(dev, "No usable fans reported by WMI, disabling fan support\n");
> +		return 0;
> +	}
> +
>  	ret = hp_wmi_apply_fan_settings(priv);
>  	if (ret)
>  		dev_warn(dev, "Failed to apply initial fan settings: %d\n", ret);
> 

-- 
 i.


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

* Re: [PATCH] platform/x86: hp-wmi: Disable hwmon attributes when no fans are usable
  2026-10-19  6:22 ` Ilpo Järvinen
@ 2026-10-19  7:40   ` Krishna Chomal
  0 siblings, 0 replies; 3+ messages in thread
From: Krishna Chomal @ 2026-10-19  7:40 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Hans de Goede, platform-driver-x86, LKML, Lorenzo Collevecchio

On Mon, Oct 19, 2026 at 09:22:59AM +0300, Ilpo Järvinen wrote:
>On Sat, 17 Oct 2026, Krishna Chomal wrote:
>
>> Currently, hp_wmi_hwmon_is_visible() checks whether fan RPM queries
>> succeed before exposing fan1_input and fan2_input, but pwm1_enable
>> remains visible always, even when fan support is unavailable.
>
>Why does it yield different result from is_visible?
>
>...Somehow I find starting with this more confusing than helpful.

Hi,

Thanks for the feedback. Let me rephrase the two issues:

This is how our current switch-case looks like in is_visible:
	switch (type) {
	case hwmon_pwm:
		if (attr == hwmon_pwm_input && !hp_wmi_fan_control_supported())
			return 0;
		return 0644;
	case hwmon_fan:
		if (hp_wmi_fan_control_supported()) {
			if (hp_wmi_get_active_fan_speed(channel) >= 0)
				return 0444;
		} else {
			if (hp_wmi_get_fan_speed(channel) >= 0)
				return 0444;
		}
		break;
	default:
		return 0;
	}

For pwm1_enable we have:
- type = hwmon_pwm
- attr = hwmon_pwm_enable

which means we go straight to return 0644; exposing this field for all
devices. For OmniBook series, changing pwm modes would result in either:
* Failure with WMI error
* Silent failure (stub firmware, see below)

For fan attributes (fan1_input, fan2_input), the return value of
hp_wmi_get_fan_speed(channel) decides whether they are visible or not.
I believe, the idea originally was to expose these fields to userspace
only if the get fan speed queries success. But the problem is, certain
devices (OmniBook again) have stub methods in their firmware, returning
all-zero buffer instead of an error. This means: get fan speed query
returns 0, and is_visible ends up exposing this field to userspace. This
is problematic because hwmon will always return 0 RPM even when the
users have reported hearing the fan spinning.

Both issues arise because fan-related attributes are exposed even when
fan support is unavailable. To fix, we use the fan count query to check
whether fan support is available before exposing these attributes.

>> On some
>> devices, such as certain Omnibook models, the firmware does not provide
>> functional WMI fan methods. Exposing pwm1_enable on these devices can
>> mislead userspace and cause fan mode changes to fail.
>>
>> Additionally, some firmware implementations provide stub methods
>> that return an all-zero buffer instead of an error. In this case,
>> hp_wmi_get_active_fan_speed() and hp_wmi_get_fan_speed() can report 0
>> RPM even when the fan is physically spinning.
>>
>> Use the result of fan count query (fetched from firmware once in
>> hp_wmi_setup_fan_settings(), and cached in hwmon private data) to
>> determine whether fan related attributes should be exposed.
>>
>> The fan count query can have the following outcomes:
>> * Negative: The WMI query failed. Other fan-related queries are also
>>   expected to fail, so disable fan support.
>> * Zero (stub firmware): The methods return zero-filled responses without
>>   providing functional fan support, so disable fan support.
>> * Zero (genuine firmware): No fans are exposed through WMI, so disable
>>   fan support.
>> * Positive (1 or 2): Firmware reports one or more fans, so allow fan
>>   support. Per-channel RPM queries continue to determine which
>>   individual fan input attributes are exposed.
>>
>> When fan support is unavailable, hide the fan and PWM attributes and
>> skip applying the initial fan settings.
>>
>> Tested on: HP OmniBook 5 Laptop 16-ag1xxx (board: 8DF4)
>>
>> Tested-by: Lorenzo Collevecchio <lorenzocollevecchio17@gmail.com>
>> Reported-by: Lorenzo Collevecchio <lorenzocollevecchio17@gmail.com>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222138
>> Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
>> ---
>>  drivers/platform/x86/hp/hp-wmi.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
>> index 0eac5e1d9cdb..52077833ac89 100644
>> --- a/drivers/platform/x86/hp/hp-wmi.c
>> +++ b/drivers/platform/x86/hp/hp-wmi.c
>> @@ -651,6 +651,7 @@ enum pwm_modes {
>>
>>  struct hp_wmi_hwmon_priv {
>>  	struct mutex lock;	/* protects mode, pwm */
>> +	int fan_count;
>>  	u8 min_rpm;
>>  	u8 max_rpm;
>>  	u8 mode;
>> @@ -2950,12 +2951,18 @@ static umode_t hp_wmi_hwmon_is_visible(const void *data,
>>  				       enum hwmon_sensor_types type,
>>  				       u32 attr, int channel)
>>  {
>> +	const struct hp_wmi_hwmon_priv *priv = data;
>> +
>>  	switch (type) {
>>  	case hwmon_pwm:
>> +		if (priv->fan_count <= 0)
>> +			return 0;
>>  		if (attr == hwmon_pwm_input && !hp_wmi_fan_control_supported())
>>  			return 0;
>>  		return 0644;
>>  	case hwmon_fan:
>> +		if (priv->fan_count <= 0)
>> +			return 0;
>>  		if (hp_wmi_fan_control_supported()) {
>>  			if (hp_wmi_get_active_fan_speed(channel) >= 0)
>>  				return 0444;
>> @@ -3124,6 +3131,7 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
>>
>>  	/* Default behaviour on hwmon init is automatic mode */
>>  	priv->mode = PWM_MODE_AUTO;
>> +	priv->fan_count = hp_wmi_get_fan_count_userdefine_trigger();
>>
>>  	/* Bypass devices without fan control support. */
>>  	if (!hp_wmi_fan_table_supported())
>> @@ -3208,6 +3216,11 @@ static int hp_wmi_hwmon_init(void)
>>  		return PTR_ERR(hwmon);
>>  	}
>>
>> +	if (priv->fan_count <= 0) {
>> +		dev_info(dev, "No usable fans reported by WMI, disabling fan support\n");
>> +		return 0;
>> +	}
>> +
>>  	ret = hp_wmi_apply_fan_settings(priv);
>>  	if (ret)
>>  		dev_warn(dev, "Failed to apply initial fan settings: %d\n", ret);
>>
>
>-- 
> i.
>

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

end of thread, other threads:[~2026-10-19  7:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-17 13:24 [PATCH] platform/x86: hp-wmi: Disable hwmon attributes when no fans are usable Krishna Chomal
2026-10-19  6:22 ` Ilpo Järvinen
2026-10-19  7:40   ` Krishna Chomal

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

all inboxes | Powered by JetHome®