Re: [PATCH] platform/x86: hp-wmi: Disable hwmon attributes when no fans are usable

[Krishna Chomal <krishna.chomal108@gmail.com>]

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