[PATCH v2 0/4] platform/x86: acer-wmi: Add fan control support

[PATCH v2 0/4] platform/x86: acer-wmi: Add fan control support

[Armin Wolf]

This patch series aims to add fan control support to the acer-wmi
driver. The patches are compile-tested only and need to be tested
on real hardware to verify that they actually work.

I CCed three users who requested support for this feature. I would be
very happy if one of you could test those patches and report back.

Changes since v1:
- remove unnecessary blank line

Changes since RFC v2:
- improve error handling when setting fan behavior
- Add Reviewed-by tags
- whitelist PHN16-72

Changes since RFC v1:
- remove duplicate include and replace hwmon_pwm_mode with
  hwmon_pwm_enable in second patch

Armin Wolf (4):
  platform/x86: acer-wmi: Fix setting of fan behavior
  platform/x86: acer-wmi: Add fan control support
  platform/x86: acer-wmi: Enable fan control for PH16-72 and PT14-51
  platform/x86: acer-wmi: Add support for PHN16-72

 drivers/platform/x86/acer-wmi.c | 308 +++++++++++++++++++++++++++++---
 1 file changed, 283 insertions(+), 25 deletions(-)

-- 
2.39.5

[PATCH v2 1/4] platform/x86: acer-wmi: Fix setting of fan behavior

[Armin Wolf]

After studying the linuwu_sense driver
(https://github.com/0x7375646F/Linuwu-Sense) i was able to understand
the meaning of the SetGamingFanBehavior() WMI method:

- the first 16-bit are a bitmap of all fans affected by a fan behavior
  change request.

- the next 8 bits contain four fan mode fields (2-bit), each being
  associated with a bit inside the fan bitmap.

There are three fan modes: auto, turbo and custom.

Use this newfound knowledge to fix the turbo fan handling by setting
the correct bits before calling SetGamingFanBehavior(). Also check
the result of the WMI method call and return an error should the ACPI
firmware signal failure.

Reviewed-by: Kurt Borja <kuurtb@gmail.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 76 +++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 23 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 69336bd778ee..a41555ee8589 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -68,10 +68,19 @@ MODULE_LICENSE("GPL");
 #define ACER_WMID_SET_GAMING_LED_METHODID 2
 #define ACER_WMID_GET_GAMING_LED_METHODID 4
 #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
-#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
+#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID 14
 #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
 #define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
 
+#define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
+#define ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK GENMASK_ULL(23, 16)
+
+#define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
+#define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
+
+#define ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK GENMASK(1, 0)
+#define ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK GENMASK(7, 6)
+
 #define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
 #define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
 #define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
@@ -121,6 +130,12 @@ enum acer_wmi_predator_v4_sensor_id {
 	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
 };
 
+enum acer_wmi_gaming_fan_mode {
+	ACER_WMID_FAN_MODE_AUTO		= 0x01,
+	ACER_WMID_FAN_MODE_TURBO	= 0x02,
+	ACER_WMID_FAN_MODE_CUSTOM	= 0x03,
+};
+
 enum acer_wmi_predator_v4_oc {
 	ACER_WMID_OC_NORMAL			= 0x0000,
 	ACER_WMID_OC_TURBO			= 0x0002,
@@ -1565,9 +1580,6 @@ static acpi_status WMID_gaming_set_u64(u64 value, u32 cap)
 	case ACER_CAP_TURBO_LED:
 		method_id = ACER_WMID_SET_GAMING_LED_METHODID;
 		break;
-	case ACER_CAP_TURBO_FAN:
-		method_id = ACER_WMID_SET_GAMING_FAN_BEHAVIOR;
-		break;
 	default:
 		return AE_BAD_PARAMETER;
 	}
@@ -1618,25 +1630,43 @@ static int WMID_gaming_get_sys_info(u32 command, u64 *out)
 	return 0;
 }
 
+static int WMID_gaming_set_fan_behavior(u16 fan_bitmap, u8 mode_bitmap)
+{
+	acpi_status status;
+	u64 input = 0;
+	u64 result;
+
+	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_ID_MASK, fan_bitmap);
+	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK, mode_bitmap);
+
+	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID, input,
+					&result);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	/* The return status must be zero for the operation to have succeeded */
+	if (FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK, result))
+		return -EIO;
+
+	return 0;
+}
+
 static void WMID_gaming_set_fan_mode(u8 fan_mode)
 {
-	/* fan_mode = 1 is used for auto, fan_mode = 2 used for turbo*/
-	u64 gpu_fan_config1 = 0, gpu_fan_config2 = 0;
-	int i;
-
-	if (quirks->cpu_fans > 0)
-		gpu_fan_config2 |= 1;
-	for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i)
-		gpu_fan_config2 |= 1 << (i + 1);
-	for (i = 0; i < quirks->gpu_fans; ++i)
-		gpu_fan_config2 |= 1 << (i + 3);
-	if (quirks->cpu_fans > 0)
-		gpu_fan_config1 |= fan_mode;
-	for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i)
-		gpu_fan_config1 |= fan_mode << (2 * i + 2);
-	for (i = 0; i < quirks->gpu_fans; ++i)
-		gpu_fan_config1 |= fan_mode << (2 * i + 6);
-	WMID_gaming_set_u64(gpu_fan_config2 | gpu_fan_config1 << 16, ACER_CAP_TURBO_FAN);
+	u16 mode_bitmap = 0;
+	u16 fan_bitmap = 0;
+
+	if (quirks->cpu_fans > 0) {
+		fan_bitmap |= ACER_GAMING_FAN_BEHAVIOR_CPU;
+		mode_bitmap |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK, fan_mode);
+	}
+
+	if (quirks->gpu_fans > 0) {
+		fan_bitmap |= ACER_GAMING_FAN_BEHAVIOR_GPU;
+		mode_bitmap |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK, fan_mode);
+	}
+
+	WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
 }
 
 static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
@@ -1923,7 +1953,7 @@ static int acer_toggle_turbo(void)
 		WMID_gaming_set_u64(0x1, ACER_CAP_TURBO_LED);
 
 		/* Set FAN mode to auto */
-		WMID_gaming_set_fan_mode(0x1);
+		WMID_gaming_set_fan_mode(ACER_WMID_FAN_MODE_AUTO);
 
 		/* Set OC to normal */
 		if (has_cap(ACER_CAP_TURBO_OC)) {
@@ -1937,7 +1967,7 @@ static int acer_toggle_turbo(void)
 		WMID_gaming_set_u64(0x10001, ACER_CAP_TURBO_LED);
 
 		/* Set FAN mode to turbo */
-		WMID_gaming_set_fan_mode(0x2);
+		WMID_gaming_set_fan_mode(ACER_WMID_FAN_MODE_TURBO);
 
 		/* Set OC to turbo mode */
 		if (has_cap(ACER_CAP_TURBO_OC)) {
-- 
2.39.5

Re: [PATCH v2 1/4] platform/x86: acer-wmi: Fix setting of fan behavior

[Ilpo Järvinen]

On Tue, 23 Sep 2025, Armin Wolf wrote:

> After studying the linuwu_sense driver
> (https://github.com/0x7375646F/Linuwu-Sense) i was able to understand
> the meaning of the SetGamingFanBehavior() WMI method:
> 
> - the first 16-bit are a bitmap of all fans affected by a fan behavior
>   change request.
> 
> - the next 8 bits contain four fan mode fields (2-bit), each being
>   associated with a bit inside the fan bitmap.
> 
> There are three fan modes: auto, turbo and custom.
> 
> Use this newfound knowledge to fix the turbo fan handling by setting
> the correct bits before calling SetGamingFanBehavior(). Also check
> the result of the WMI method call and return an error should the ACPI
> firmware signal failure.
> 
> Reviewed-by: Kurt Borja <kuurtb@gmail.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/acer-wmi.c | 76 +++++++++++++++++++++++----------
>  1 file changed, 53 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 69336bd778ee..a41555ee8589 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -68,10 +68,19 @@ MODULE_LICENSE("GPL");
>  #define ACER_WMID_SET_GAMING_LED_METHODID 2
>  #define ACER_WMID_GET_GAMING_LED_METHODID 4
>  #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
> -#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
> +#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID 14
>  #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
>  #define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
>  
> +#define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
> +#define ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK GENMASK_ULL(23, 16)
> +
> +#define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
> +#define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
> +
> +#define ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK GENMASK(1, 0)
> +#define ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK GENMASK(7, 6)
> +
>  #define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
>  #define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
>  #define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
> @@ -121,6 +130,12 @@ enum acer_wmi_predator_v4_sensor_id {
>  	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
>  };
>  
> +enum acer_wmi_gaming_fan_mode {
> +	ACER_WMID_FAN_MODE_AUTO		= 0x01,
> +	ACER_WMID_FAN_MODE_TURBO	= 0x02,
> +	ACER_WMID_FAN_MODE_CUSTOM	= 0x03,
> +};
> +
>  enum acer_wmi_predator_v4_oc {
>  	ACER_WMID_OC_NORMAL			= 0x0000,
>  	ACER_WMID_OC_TURBO			= 0x0002,
> @@ -1565,9 +1580,6 @@ static acpi_status WMID_gaming_set_u64(u64 value, u32 cap)
>  	case ACER_CAP_TURBO_LED:
>  		method_id = ACER_WMID_SET_GAMING_LED_METHODID;
>  		break;
> -	case ACER_CAP_TURBO_FAN:
> -		method_id = ACER_WMID_SET_GAMING_FAN_BEHAVIOR;
> -		break;
>  	default:
>  		return AE_BAD_PARAMETER;
>  	}
> @@ -1618,25 +1630,43 @@ static int WMID_gaming_get_sys_info(u32 command, u64 *out)
>  	return 0;
>  }
>  
> +static int WMID_gaming_set_fan_behavior(u16 fan_bitmap, u8 mode_bitmap)

Should the name be in plural as this sets all fans?

Please also consider what I noted to patch 2 about this nesting of 
FIELD_PREP()s. Getting rid of that effectively means the caller ORs all 
bits together but I don't think that would make the code harder to read.

> +{
> +	acpi_status status;
> +	u64 input = 0;
> +	u64 result;
> +
> +	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_ID_MASK, fan_bitmap);
> +	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK, mode_bitmap);
> +
> +	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID, input,
> +					&result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	/* The return status must be zero for the operation to have succeeded */
> +	if (FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK, result))
> +		return -EIO;
> +
> +	return 0;
> +}
> +
>  static void WMID_gaming_set_fan_mode(u8 fan_mode)
>  {
> -	/* fan_mode = 1 is used for auto, fan_mode = 2 used for turbo*/
> -	u64 gpu_fan_config1 = 0, gpu_fan_config2 = 0;
> -	int i;
> -
> -	if (quirks->cpu_fans > 0)
> -		gpu_fan_config2 |= 1;
> -	for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i)
> -		gpu_fan_config2 |= 1 << (i + 1);
> -	for (i = 0; i < quirks->gpu_fans; ++i)
> -		gpu_fan_config2 |= 1 << (i + 3);
> -	if (quirks->cpu_fans > 0)
> -		gpu_fan_config1 |= fan_mode;
> -	for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i)
> -		gpu_fan_config1 |= fan_mode << (2 * i + 2);
> -	for (i = 0; i < quirks->gpu_fans; ++i)
> -		gpu_fan_config1 |= fan_mode << (2 * i + 6);
> -	WMID_gaming_set_u64(gpu_fan_config2 | gpu_fan_config1 << 16, ACER_CAP_TURBO_FAN);
> +	u16 mode_bitmap = 0;
> +	u16 fan_bitmap = 0;
> +
> +	if (quirks->cpu_fans > 0) {
> +		fan_bitmap |= ACER_GAMING_FAN_BEHAVIOR_CPU;
> +		mode_bitmap |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK, fan_mode);
> +	}
> +
> +	if (quirks->gpu_fans > 0) {
> +		fan_bitmap |= ACER_GAMING_FAN_BEHAVIOR_GPU;
> +		mode_bitmap |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK, fan_mode);
> +	}
> +
> +	WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
>  }
>  
>  static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
> @@ -1923,7 +1953,7 @@ static int acer_toggle_turbo(void)
>  		WMID_gaming_set_u64(0x1, ACER_CAP_TURBO_LED);
>  
>  		/* Set FAN mode to auto */
> -		WMID_gaming_set_fan_mode(0x1);
> +		WMID_gaming_set_fan_mode(ACER_WMID_FAN_MODE_AUTO);
>  
>  		/* Set OC to normal */
>  		if (has_cap(ACER_CAP_TURBO_OC)) {
> @@ -1937,7 +1967,7 @@ static int acer_toggle_turbo(void)
>  		WMID_gaming_set_u64(0x10001, ACER_CAP_TURBO_LED);
>  
>  		/* Set FAN mode to turbo */
> -		WMID_gaming_set_fan_mode(0x2);
> +		WMID_gaming_set_fan_mode(ACER_WMID_FAN_MODE_TURBO);
>  
>  		/* Set OC to turbo mode */
>  		if (has_cap(ACER_CAP_TURBO_OC)) {
> 

-- 
 i.

Re: [PATCH v2 1/4] platform/x86: acer-wmi: Fix setting of fan behavior

[Armin Wolf]

Am 24.09.25 um 13:00 schrieb Ilpo Järvinen:

> On Tue, 23 Sep 2025, Armin Wolf wrote:
>
>> After studying the linuwu_sense driver
>> (https://github.com/0x7375646F/Linuwu-Sense) i was able to understand
>> the meaning of the SetGamingFanBehavior() WMI method:
>>
>> - the first 16-bit are a bitmap of all fans affected by a fan behavior
>>    change request.
>>
>> - the next 8 bits contain four fan mode fields (2-bit), each being
>>    associated with a bit inside the fan bitmap.
>>
>> There are three fan modes: auto, turbo and custom.
>>
>> Use this newfound knowledge to fix the turbo fan handling by setting
>> the correct bits before calling SetGamingFanBehavior(). Also check
>> the result of the WMI method call and return an error should the ACPI
>> firmware signal failure.
>>
>> Reviewed-by: Kurt Borja <kuurtb@gmail.com>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/platform/x86/acer-wmi.c | 76 +++++++++++++++++++++++----------
>>   1 file changed, 53 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
>> index 69336bd778ee..a41555ee8589 100644
>> --- a/drivers/platform/x86/acer-wmi.c
>> +++ b/drivers/platform/x86/acer-wmi.c
>> @@ -68,10 +68,19 @@ MODULE_LICENSE("GPL");
>>   #define ACER_WMID_SET_GAMING_LED_METHODID 2
>>   #define ACER_WMID_GET_GAMING_LED_METHODID 4
>>   #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
>> -#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
>> +#define ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID 14
>>   #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
>>   #define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
>>   
>> +#define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
>> +#define ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK GENMASK_ULL(23, 16)
>> +
>> +#define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
>> +#define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
>> +
>> +#define ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK GENMASK(1, 0)
>> +#define ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK GENMASK(7, 6)
>> +
>>   #define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
>>   #define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
>>   #define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
>> @@ -121,6 +130,12 @@ enum acer_wmi_predator_v4_sensor_id {
>>   	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
>>   };
>>   
>> +enum acer_wmi_gaming_fan_mode {
>> +	ACER_WMID_FAN_MODE_AUTO		= 0x01,
>> +	ACER_WMID_FAN_MODE_TURBO	= 0x02,
>> +	ACER_WMID_FAN_MODE_CUSTOM	= 0x03,
>> +};
>> +
>>   enum acer_wmi_predator_v4_oc {
>>   	ACER_WMID_OC_NORMAL			= 0x0000,
>>   	ACER_WMID_OC_TURBO			= 0x0002,
>> @@ -1565,9 +1580,6 @@ static acpi_status WMID_gaming_set_u64(u64 value, u32 cap)
>>   	case ACER_CAP_TURBO_LED:
>>   		method_id = ACER_WMID_SET_GAMING_LED_METHODID;
>>   		break;
>> -	case ACER_CAP_TURBO_FAN:
>> -		method_id = ACER_WMID_SET_GAMING_FAN_BEHAVIOR;
>> -		break;
>>   	default:
>>   		return AE_BAD_PARAMETER;
>>   	}
>> @@ -1618,25 +1630,43 @@ static int WMID_gaming_get_sys_info(u32 command, u64 *out)
>>   	return 0;
>>   }
>>   
>> +static int WMID_gaming_set_fan_behavior(u16 fan_bitmap, u8 mode_bitmap)
> Should the name be in plural as this sets all fans?

I don't think so, as this functions can also be used to control each fan
individually.

> Please also consider what I noted to patch 2 about this nesting of
> FIELD_PREP()s. Getting rid of that effectively means the caller ORs all
> bits together but I don't think that would make the code harder to read.

Alright, i will try to get rid of the nesting.

Thanks,
Armin Wolf

>> +{
>> +	acpi_status status;
>> +	u64 input = 0;
>> +	u64 result;
>> +
>> +	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_ID_MASK, fan_bitmap);
>> +	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK, mode_bitmap);
>> +
>> +	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID, input,
>> +					&result);
>> +	if (ACPI_FAILURE(status))
>> +		return -EIO;
>> +
>> +	/* The return status must be zero for the operation to have succeeded */
>> +	if (FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK, result))
>> +		return -EIO;
>> +
>> +	return 0;
>> +}
>> +
>>   static void WMID_gaming_set_fan_mode(u8 fan_mode)
>>   {
>> -	/* fan_mode = 1 is used for auto, fan_mode = 2 used for turbo*/
>> -	u64 gpu_fan_config1 = 0, gpu_fan_config2 = 0;
>> -	int i;
>> -
>> -	if (quirks->cpu_fans > 0)
>> -		gpu_fan_config2 |= 1;
>> -	for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i)
>> -		gpu_fan_config2 |= 1 << (i + 1);
>> -	for (i = 0; i < quirks->gpu_fans; ++i)
>> -		gpu_fan_config2 |= 1 << (i + 3);
>> -	if (quirks->cpu_fans > 0)
>> -		gpu_fan_config1 |= fan_mode;
>> -	for (i = 0; i < (quirks->cpu_fans + quirks->gpu_fans); ++i)
>> -		gpu_fan_config1 |= fan_mode << (2 * i + 2);
>> -	for (i = 0; i < quirks->gpu_fans; ++i)
>> -		gpu_fan_config1 |= fan_mode << (2 * i + 6);
>> -	WMID_gaming_set_u64(gpu_fan_config2 | gpu_fan_config1 << 16, ACER_CAP_TURBO_FAN);
>> +	u16 mode_bitmap = 0;
>> +	u16 fan_bitmap = 0;
>> +
>> +	if (quirks->cpu_fans > 0) {
>> +		fan_bitmap |= ACER_GAMING_FAN_BEHAVIOR_CPU;
>> +		mode_bitmap |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK, fan_mode);
>> +	}
>> +
>> +	if (quirks->gpu_fans > 0) {
>> +		fan_bitmap |= ACER_GAMING_FAN_BEHAVIOR_GPU;
>> +		mode_bitmap |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK, fan_mode);
>> +	}
>> +
>> +	WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
>>   }
>>   
>>   static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
>> @@ -1923,7 +1953,7 @@ static int acer_toggle_turbo(void)
>>   		WMID_gaming_set_u64(0x1, ACER_CAP_TURBO_LED);
>>   
>>   		/* Set FAN mode to auto */
>> -		WMID_gaming_set_fan_mode(0x1);
>> +		WMID_gaming_set_fan_mode(ACER_WMID_FAN_MODE_AUTO);
>>   
>>   		/* Set OC to normal */
>>   		if (has_cap(ACER_CAP_TURBO_OC)) {
>> @@ -1937,7 +1967,7 @@ static int acer_toggle_turbo(void)
>>   		WMID_gaming_set_u64(0x10001, ACER_CAP_TURBO_LED);
>>   
>>   		/* Set FAN mode to turbo */
>> -		WMID_gaming_set_fan_mode(0x2);
>> +		WMID_gaming_set_fan_mode(ACER_WMID_FAN_MODE_TURBO);
>>   
>>   		/* Set OC to turbo mode */
>>   		if (has_cap(ACER_CAP_TURBO_OC)) {
>>

[PATCH v2 2/4] platform/x86: acer-wmi: Add fan control support

[Armin Wolf]

Add support for controlling the fan speed using the
SetGamingFanSpeed() and GetGamingFanSpeed() WMI methods.

This feature is only enabled if the machine has ACER_CAP_PWM enabled
and depend on ACER_CAP_HWMON for detecting the number of available
fans.

Reviewed-by: Kurt Borja <kuurtb@gmail.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 221 +++++++++++++++++++++++++++++++-
 1 file changed, 219 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index a41555ee8589..345b6e24ae31 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -12,10 +12,12 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/dmi.h>
+#include <linux/fixp-arith.h>
 #include <linux/backlight.h>
 #include <linux/leds.h>
 #include <linux/platform_device.h>
@@ -69,11 +71,16 @@ MODULE_LICENSE("GPL");
 #define ACER_WMID_GET_GAMING_LED_METHODID 4
 #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
 #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID 14
+#define ACER_WMID_GET_GAMING_FAN_BEHAVIOR_METHODID 15
+#define ACER_WMID_SET_GAMING_FAN_SPEED_METHODID 16
+#define ACER_WMID_GET_GAMING_FAN_SPEED_METHODID 17
 #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
 #define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
 
+#define ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK GENMASK_ULL(7, 0)
 #define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
 #define ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK GENMASK_ULL(23, 16)
+#define ACER_GAMING_FAN_BEHAVIOR_GET_MODE_MASK GENMASK_ULL(23, 8)
 
 #define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
 #define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
@@ -81,6 +88,10 @@ MODULE_LICENSE("GPL");
 #define ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK GENMASK(1, 0)
 #define ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK GENMASK(7, 6)
 
+#define ACER_GAMING_FAN_SPEED_STATUS_MASK GENMASK_ULL(7, 0)
+#define ACER_GAMING_FAN_SPEED_ID_MASK GENMASK_ULL(7, 0)
+#define ACER_GAMING_FAN_SPEED_VALUE_MASK GENMASK_ULL(15, 8)
+
 #define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
 #define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
 #define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
@@ -130,6 +141,11 @@ enum acer_wmi_predator_v4_sensor_id {
 	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
 };
 
+enum acer_wmi_gaming_fan_id {
+	ACER_WMID_CPU_FAN	= 0x01,
+	ACER_WMID_GPU_FAN	= 0x04,
+};
+
 enum acer_wmi_gaming_fan_mode {
 	ACER_WMID_FAN_MODE_AUTO		= 0x01,
 	ACER_WMID_FAN_MODE_TURBO	= 0x02,
@@ -292,6 +308,7 @@ struct hotkey_function_type_aa {
 #define ACER_CAP_TURBO_FAN		BIT(9)
 #define ACER_CAP_PLATFORM_PROFILE	BIT(10)
 #define ACER_CAP_HWMON			BIT(11)
+#define ACER_CAP_PWM			BIT(12)
 
 /*
  * Interface type flags
@@ -386,6 +403,7 @@ struct quirk_entry {
 	u8 cpu_fans;
 	u8 gpu_fans;
 	u8 predator_v4;
+	u8 pwm;
 };
 
 static struct quirk_entry *quirks;
@@ -405,6 +423,9 @@ static void __init set_quirks(void)
 	if (quirks->predator_v4)
 		interface->capability |= ACER_CAP_PLATFORM_PROFILE |
 					 ACER_CAP_HWMON;
+
+	if (quirks->pwm)
+		interface->capability |= ACER_CAP_PWM;
 }
 
 static int __init dmi_matched(const struct dmi_system_id *dmi)
@@ -1651,6 +1672,27 @@ static int WMID_gaming_set_fan_behavior(u16 fan_bitmap, u8 mode_bitmap)
 	return 0;
 }
 
+static int WMID_gaming_get_fan_behavior(u16 fan_bitmap, u8 *mode_bitmap)
+{
+	acpi_status status;
+	u32 input = 0;
+	u64 result;
+
+	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_ID_MASK, fan_bitmap);
+	status = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_FAN_BEHAVIOR_METHODID, input,
+					    &result);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	/* The return status must be zero for the operation to have succeeded */
+	if (FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK, result))
+		return -EIO;
+
+	*mode_bitmap = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_GET_MODE_MASK, result);
+
+	return 0;
+}
+
 static void WMID_gaming_set_fan_mode(u8 fan_mode)
 {
 	u16 mode_bitmap = 0;
@@ -1669,6 +1711,55 @@ static void WMID_gaming_set_fan_mode(u8 fan_mode)
 	WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
 }
 
+static int WMID_gaming_set_gaming_fan_speed(u8 fan, u8 speed)
+{
+	acpi_status status;
+	u64 input = 0;
+	u64 result;
+
+	if (speed > 100)
+		return -EINVAL;
+
+	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_ID_MASK, fan);
+	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_VALUE_MASK, speed);
+
+	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_FAN_SPEED_METHODID, input, &result);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	switch (FIELD_GET(ACER_GAMING_FAN_SPEED_STATUS_MASK, result)) {
+	case 0x00:
+		return 0;
+	case 0x01:
+		return -ENODEV;
+	case 0x02:
+		return -EINVAL;
+	default:
+		return -ENXIO;
+	}
+}
+
+static int WMID_gaming_get_gaming_fan_speed(u8 fan, u8 *speed)
+{
+	acpi_status status;
+	u32 input = 0;
+	u64 result;
+
+	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_ID_MASK, fan);
+
+	status = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_FAN_SPEED_METHODID, input,
+					    &result);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	if (FIELD_GET(ACER_GAMING_FAN_SPEED_STATUS_MASK, result))
+		return -ENODEV;
+
+	*speed = FIELD_GET(ACER_GAMING_FAN_SPEED_VALUE_MASK, result);
+
+	return 0;
+}
+
 static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
 {
 	acpi_status status;
@@ -2840,6 +2931,16 @@ static const enum acer_wmi_predator_v4_sensor_id acer_wmi_fan_channel_to_sensor_
 	[1] = ACER_WMID_SENSOR_GPU_FAN_SPEED,
 };
 
+static const enum acer_wmi_gaming_fan_id acer_wmi_fan_channel_to_fan_id[] = {
+	[0] = ACER_WMID_CPU_FAN,
+	[1] = ACER_WMID_GPU_FAN,
+};
+
+static const u16 acer_wmi_fan_channel_to_fan_bitmap[] = {
+	[0] = ACER_GAMING_FAN_BEHAVIOR_CPU,
+	[1] = ACER_GAMING_FAN_BEHAVIOR_GPU,
+};
+
 static umode_t acer_wmi_hwmon_is_visible(const void *data,
 					 enum hwmon_sensor_types type, u32 attr,
 					 int channel)
@@ -2851,6 +2952,11 @@ static umode_t acer_wmi_hwmon_is_visible(const void *data,
 	case hwmon_temp:
 		sensor_id = acer_wmi_temp_channel_to_sensor_id[channel];
 		break;
+	case hwmon_pwm:
+		if (!has_cap(ACER_CAP_PWM))
+			return 0;
+
+		fallthrough;
 	case hwmon_fan:
 		sensor_id = acer_wmi_fan_channel_to_sensor_id[channel];
 		break;
@@ -2858,8 +2964,12 @@ static umode_t acer_wmi_hwmon_is_visible(const void *data,
 		return 0;
 	}
 
-	if (*supported_sensors & BIT(sensor_id - 1))
+	if (*supported_sensors & BIT(sensor_id - 1)) {
+		if (type == hwmon_pwm)
+			return 0644;
+
 		return 0444;
+	}
 
 	return 0;
 }
@@ -2868,8 +2978,10 @@ static int acer_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 			       u32 attr, int channel, long *val)
 {
 	u64 command = ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING;
+	u8 fan, speed, mode_bitmap;
+	u16 fan_bitmap;
+	int mode, ret;
 	u64 result;
-	int ret;
 
 	switch (type) {
 	case hwmon_temp:
@@ -2893,6 +3005,106 @@ static int acer_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 
 		*val = FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
 		return 0;
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			fan = acer_wmi_fan_channel_to_fan_id[channel];
+			ret = WMID_gaming_get_gaming_fan_speed(fan, &speed);
+			if (ret < 0)
+				return ret;
+
+			*val = fixp_linear_interpolate(0, 0, 100, U8_MAX, speed);
+			return 0;
+		case hwmon_pwm_enable:
+			fan_bitmap = acer_wmi_fan_channel_to_fan_bitmap[channel];
+			ret = WMID_gaming_get_fan_behavior(fan_bitmap, &mode_bitmap);
+			if (ret < 0)
+				return ret;
+
+			switch (channel) {
+			case 0:
+				mode = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK,
+						 mode_bitmap);
+				break;
+			case 1:
+				mode = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK,
+						 mode_bitmap);
+				break;
+			default:
+				return -EINVAL;
+			}
+
+			switch (mode) {
+			case ACER_WMID_FAN_MODE_AUTO:
+				*val = 2;
+				return 0;
+			case ACER_WMID_FAN_MODE_TURBO:
+				*val = 0;
+				return 0;
+			case ACER_WMID_FAN_MODE_CUSTOM:
+				*val = 1;
+				return 0;
+			default:
+				return -ENXIO;
+			}
+		default:
+			return -EOPNOTSUPP;
+		}
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int acer_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
+				u32 attr, int channel, long val)
+{
+	u8 fan, speed, mode_bitmap;
+	u16 fan_bitmap;
+	int mode;
+
+	switch (type) {
+	case hwmon_pwm:
+		switch (attr) {
+		case hwmon_pwm_input:
+			fan = acer_wmi_fan_channel_to_fan_id[channel];
+			speed = fixp_linear_interpolate(0, 0, U8_MAX, 100,
+							clamp_val(val, 0, U8_MAX));
+
+			return WMID_gaming_set_gaming_fan_speed(fan, speed);
+		case hwmon_pwm_enable:
+			fan_bitmap = acer_wmi_fan_channel_to_fan_bitmap[channel];
+
+			switch (val) {
+			case 0:
+				mode = ACER_WMID_FAN_MODE_TURBO;
+				break;
+			case 1:
+				mode = ACER_WMID_FAN_MODE_CUSTOM;
+				break;
+			case 2:
+				mode = ACER_WMID_FAN_MODE_AUTO;
+				break;
+			default:
+				return -EINVAL;
+			}
+
+			switch (channel) {
+			case 0:
+				mode_bitmap = FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK,
+							 mode);
+				break;
+			case 1:
+				mode_bitmap = FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK,
+							 mode);
+				break;
+			default:
+				return -EINVAL;
+			}
+
+			return WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
+		default:
+			return -EOPNOTSUPP;
+		}
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -2908,11 +3120,16 @@ static const struct hwmon_channel_info *const acer_wmi_hwmon_info[] = {
 			   HWMON_F_INPUT,
 			   HWMON_F_INPUT
 			   ),
+	HWMON_CHANNEL_INFO(pwm,
+			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
+			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE
+			   ),
 	NULL
 };
 
 static const struct hwmon_ops acer_wmi_hwmon_ops = {
 	.read = acer_wmi_hwmon_read,
+	.write = acer_wmi_hwmon_write,
 	.is_visible = acer_wmi_hwmon_is_visible,
 };
 
-- 
2.39.5

Re: [PATCH v2 2/4] platform/x86: acer-wmi: Add fan control support

[Ilpo Järvinen]

On Tue, 23 Sep 2025, Armin Wolf wrote:

> Add support for controlling the fan speed using the
> SetGamingFanSpeed() and GetGamingFanSpeed() WMI methods.
> 
> This feature is only enabled if the machine has ACER_CAP_PWM enabled
> and depend on ACER_CAP_HWMON for detecting the number of available
> fans.
> 
> Reviewed-by: Kurt Borja <kuurtb@gmail.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/acer-wmi.c | 221 +++++++++++++++++++++++++++++++-
>  1 file changed, 219 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index a41555ee8589..345b6e24ae31 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -12,10 +12,12 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/kernel.h>
> +#include <linux/minmax.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/types.h>
>  #include <linux/dmi.h>
> +#include <linux/fixp-arith.h>
>  #include <linux/backlight.h>
>  #include <linux/leds.h>
>  #include <linux/platform_device.h>
> @@ -69,11 +71,16 @@ MODULE_LICENSE("GPL");
>  #define ACER_WMID_GET_GAMING_LED_METHODID 4
>  #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
>  #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID 14
> +#define ACER_WMID_GET_GAMING_FAN_BEHAVIOR_METHODID 15
> +#define ACER_WMID_SET_GAMING_FAN_SPEED_METHODID 16
> +#define ACER_WMID_GET_GAMING_FAN_SPEED_METHODID 17
>  #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
>  #define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
>  
> +#define ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK GENMASK_ULL(7, 0)
>  #define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
>  #define ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK GENMASK_ULL(23, 16)
> +#define ACER_GAMING_FAN_BEHAVIOR_GET_MODE_MASK GENMASK_ULL(23, 8)
>  
>  #define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
>  #define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
> @@ -81,6 +88,10 @@ MODULE_LICENSE("GPL");
>  #define ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK GENMASK(1, 0)
>  #define ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK GENMASK(7, 6)
>  
> +#define ACER_GAMING_FAN_SPEED_STATUS_MASK GENMASK_ULL(7, 0)
> +#define ACER_GAMING_FAN_SPEED_ID_MASK GENMASK_ULL(7, 0)
> +#define ACER_GAMING_FAN_SPEED_VALUE_MASK GENMASK_ULL(15, 8)
> +
>  #define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
>  #define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
>  #define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
> @@ -130,6 +141,11 @@ enum acer_wmi_predator_v4_sensor_id {
>  	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
>  };
>  
> +enum acer_wmi_gaming_fan_id {
> +	ACER_WMID_CPU_FAN	= 0x01,
> +	ACER_WMID_GPU_FAN	= 0x04,
> +};

In patch 1 you added these:

+#define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
...
+#define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
+#define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)

Are these numbers related (you even called the field "ID mask")? Could we 
do with less defines for the same thing if they're related?

>  enum acer_wmi_gaming_fan_mode {
>  	ACER_WMID_FAN_MODE_AUTO		= 0x01,
>  	ACER_WMID_FAN_MODE_TURBO	= 0x02,
> @@ -292,6 +308,7 @@ struct hotkey_function_type_aa {
>  #define ACER_CAP_TURBO_FAN		BIT(9)
>  #define ACER_CAP_PLATFORM_PROFILE	BIT(10)
>  #define ACER_CAP_HWMON			BIT(11)
> +#define ACER_CAP_PWM			BIT(12)
>  
>  /*
>   * Interface type flags
> @@ -386,6 +403,7 @@ struct quirk_entry {
>  	u8 cpu_fans;
>  	u8 gpu_fans;
>  	u8 predator_v4;
> +	u8 pwm;
>  };
>  
>  static struct quirk_entry *quirks;
> @@ -405,6 +423,9 @@ static void __init set_quirks(void)
>  	if (quirks->predator_v4)
>  		interface->capability |= ACER_CAP_PLATFORM_PROFILE |
>  					 ACER_CAP_HWMON;
> +
> +	if (quirks->pwm)
> +		interface->capability |= ACER_CAP_PWM;
>  }
>  
>  static int __init dmi_matched(const struct dmi_system_id *dmi)
> @@ -1651,6 +1672,27 @@ static int WMID_gaming_set_fan_behavior(u16 fan_bitmap, u8 mode_bitmap)
>  	return 0;
>  }
>  
> +static int WMID_gaming_get_fan_behavior(u16 fan_bitmap, u8 *mode_bitmap)
> +{
> +	acpi_status status;
> +	u32 input = 0;
> +	u64 result;
> +
> +	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_ID_MASK, fan_bitmap);
> +	status = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_FAN_BEHAVIOR_METHODID, input,
> +					    &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	/* The return status must be zero for the operation to have succeeded */
> +	if (FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK, result))
> +		return -EIO;
> +
> +	*mode_bitmap = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_GET_MODE_MASK, result);
> +
> +	return 0;
> +}
> +
>  static void WMID_gaming_set_fan_mode(u8 fan_mode)
>  {
>  	u16 mode_bitmap = 0;
> @@ -1669,6 +1711,55 @@ static void WMID_gaming_set_fan_mode(u8 fan_mode)
>  	WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
>  }
>  
> +static int WMID_gaming_set_gaming_fan_speed(u8 fan, u8 speed)
> +{
> +	acpi_status status;
> +	u64 input = 0;
> +	u64 result;
> +
> +	if (speed > 100)
> +		return -EINVAL;
> +
> +	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_ID_MASK, fan);
> +	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_VALUE_MASK, speed);
> +
> +	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_FAN_SPEED_METHODID, input, &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	switch (FIELD_GET(ACER_GAMING_FAN_SPEED_STATUS_MASK, result)) {
> +	case 0x00:
> +		return 0;
> +	case 0x01:
> +		return -ENODEV;
> +	case 0x02:
> +		return -EINVAL;
> +	default:
> +		return -ENXIO;
> +	}
> +}
> +
> +static int WMID_gaming_get_gaming_fan_speed(u8 fan, u8 *speed)
> +{
> +	acpi_status status;
> +	u32 input = 0;
> +	u64 result;
> +
> +	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_ID_MASK, fan);
> +
> +	status = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_FAN_SPEED_METHODID, input,
> +					    &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	if (FIELD_GET(ACER_GAMING_FAN_SPEED_STATUS_MASK, result))
> +		return -ENODEV;
> +
> +	*speed = FIELD_GET(ACER_GAMING_FAN_SPEED_VALUE_MASK, result);
> +
> +	return 0;
> +}
> +
>  static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
>  {
>  	acpi_status status;
> @@ -2840,6 +2931,16 @@ static const enum acer_wmi_predator_v4_sensor_id acer_wmi_fan_channel_to_sensor_
>  	[1] = ACER_WMID_SENSOR_GPU_FAN_SPEED,
>  };
>  
> +static const enum acer_wmi_gaming_fan_id acer_wmi_fan_channel_to_fan_id[] = {
> +	[0] = ACER_WMID_CPU_FAN,
> +	[1] = ACER_WMID_GPU_FAN,
> +};
> +
> +static const u16 acer_wmi_fan_channel_to_fan_bitmap[] = {
> +	[0] = ACER_GAMING_FAN_BEHAVIOR_CPU,
> +	[1] = ACER_GAMING_FAN_BEHAVIOR_GPU,
> +};
> +
>  static umode_t acer_wmi_hwmon_is_visible(const void *data,
>  					 enum hwmon_sensor_types type, u32 attr,
>  					 int channel)
> @@ -2851,6 +2952,11 @@ static umode_t acer_wmi_hwmon_is_visible(const void *data,
>  	case hwmon_temp:
>  		sensor_id = acer_wmi_temp_channel_to_sensor_id[channel];
>  		break;
> +	case hwmon_pwm:
> +		if (!has_cap(ACER_CAP_PWM))
> +			return 0;
> +
> +		fallthrough;
>  	case hwmon_fan:
>  		sensor_id = acer_wmi_fan_channel_to_sensor_id[channel];
>  		break;
> @@ -2858,8 +2964,12 @@ static umode_t acer_wmi_hwmon_is_visible(const void *data,
>  		return 0;
>  	}
>  
> -	if (*supported_sensors & BIT(sensor_id - 1))
> +	if (*supported_sensors & BIT(sensor_id - 1)) {
> +		if (type == hwmon_pwm)
> +			return 0644;
> +
>  		return 0444;
> +	}
>  
>  	return 0;
>  }
> @@ -2868,8 +2978,10 @@ static int acer_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  			       u32 attr, int channel, long *val)
>  {
>  	u64 command = ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING;
> +	u8 fan, speed, mode_bitmap;
> +	u16 fan_bitmap;
> +	int mode, ret;
>  	u64 result;
> -	int ret;
>  
>  	switch (type) {
>  	case hwmon_temp:
> @@ -2893,6 +3005,106 @@ static int acer_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  
>  		*val = FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
>  		return 0;
> +	case hwmon_pwm:
> +		switch (attr) {
> +		case hwmon_pwm_input:
> +			fan = acer_wmi_fan_channel_to_fan_id[channel];
> +			ret = WMID_gaming_get_gaming_fan_speed(fan, &speed);
> +			if (ret < 0)
> +				return ret;
> +
> +			*val = fixp_linear_interpolate(0, 0, 100, U8_MAX, speed);
> +			return 0;
> +		case hwmon_pwm_enable:
> +			fan_bitmap = acer_wmi_fan_channel_to_fan_bitmap[channel];
> +			ret = WMID_gaming_get_fan_behavior(fan_bitmap, &mode_bitmap);
> +			if (ret < 0)
> +				return ret;
> +
> +			switch (channel) {
> +			case 0:
> +				mode = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK,
> +						 mode_bitmap);
> +				break;
> +			case 1:
> +				mode = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK,
> +						 mode_bitmap);
> +				break;
> +			default:
> +				return -EINVAL;
> +			}

You've managed to create an internal API that resembles the WMI method 
input by working in terms of fan_bitmap and mode_bitmap which makes things 
complicated as then you have to build logic on top of it to do all the 
mapping in the caller...

I suggest changing this so you make WMID_gaming_get_fan_behavior() to 
input ACER_GAMING_FAN_BEHAVIOR_CPU / GPU (or ACER_WMID_CPU_FAN / GPU_FAN 
depending on the question I asked above) and perform the output mapping 
within that function. I think it would be much cleaner.

I didn't exactly like the internal API in patch 1 either and I'd prefer 
the field GENMASK()s to directly point to the correct bits instead of the 
nested FIELD_GET/PREP()s.

> +			switch (mode) {
> +			case ACER_WMID_FAN_MODE_AUTO:
> +				*val = 2;
> +				return 0;
> +			case ACER_WMID_FAN_MODE_TURBO:
> +				*val = 0;
> +				return 0;
> +			case ACER_WMID_FAN_MODE_CUSTOM:
> +				*val = 1;
> +				return 0;
> +			default:
> +				return -ENXIO;
> +			}
> +		default:
> +			return -EOPNOTSUPP;
> +		}
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int acer_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> +				u32 attr, int channel, long val)
> +{
> +	u8 fan, speed, mode_bitmap;
> +	u16 fan_bitmap;
> +	int mode;
> +
> +	switch (type) {
> +	case hwmon_pwm:
> +		switch (attr) {
> +		case hwmon_pwm_input:
> +			fan = acer_wmi_fan_channel_to_fan_id[channel];
> +			speed = fixp_linear_interpolate(0, 0, U8_MAX, 100,
> +							clamp_val(val, 0, U8_MAX));
> +
> +			return WMID_gaming_set_gaming_fan_speed(fan, speed);
> +		case hwmon_pwm_enable:
> +			fan_bitmap = acer_wmi_fan_channel_to_fan_bitmap[channel];
> +
> +			switch (val) {
> +			case 0:
> +				mode = ACER_WMID_FAN_MODE_TURBO;
> +				break;
> +			case 1:
> +				mode = ACER_WMID_FAN_MODE_CUSTOM;
> +				break;
> +			case 2:
> +				mode = ACER_WMID_FAN_MODE_AUTO;
> +				break;
> +			default:
> +				return -EINVAL;
> +			}
> +
> +			switch (channel) {
> +			case 0:
> +				mode_bitmap = FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK,
> +							 mode);
> +				break;
> +			case 1:
> +				mode_bitmap = FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK,
> +							 mode);
> +				break;
> +			default:
> +				return -EINVAL;
> +			}

These too look they could be mapped inside WMID_gaming_set_fan_behavior().

> +
> +			return WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
> +		default:
> +			return -EOPNOTSUPP;
> +		}
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> @@ -2908,11 +3120,16 @@ static const struct hwmon_channel_info *const acer_wmi_hwmon_info[] = {
>  			   HWMON_F_INPUT,
>  			   HWMON_F_INPUT
>  			   ),
> +	HWMON_CHANNEL_INFO(pwm,
> +			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
> +			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE
> +			   ),
>  	NULL
>  };
>  
>  static const struct hwmon_ops acer_wmi_hwmon_ops = {
>  	.read = acer_wmi_hwmon_read,
> +	.write = acer_wmi_hwmon_write,
>  	.is_visible = acer_wmi_hwmon_is_visible,
>  };
>  
> 

-- 
 i.

Re: [PATCH v2 2/4] platform/x86: acer-wmi: Add fan control support

[Armin Wolf]

Am 24.09.25 um 12:52 schrieb Ilpo Järvinen:

> On Tue, 23 Sep 2025, Armin Wolf wrote:
>
>> Add support for controlling the fan speed using the
>> SetGamingFanSpeed() and GetGamingFanSpeed() WMI methods.
>>
>> This feature is only enabled if the machine has ACER_CAP_PWM enabled
>> and depend on ACER_CAP_HWMON for detecting the number of available
>> fans.
>>
>> Reviewed-by: Kurt Borja <kuurtb@gmail.com>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/platform/x86/acer-wmi.c | 221 +++++++++++++++++++++++++++++++-
>>   1 file changed, 219 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
>> index a41555ee8589..345b6e24ae31 100644
>> --- a/drivers/platform/x86/acer-wmi.c
>> +++ b/drivers/platform/x86/acer-wmi.c
>> @@ -12,10 +12,12 @@
>>   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>   
>>   #include <linux/kernel.h>
>> +#include <linux/minmax.h>
>>   #include <linux/module.h>
>>   #include <linux/init.h>
>>   #include <linux/types.h>
>>   #include <linux/dmi.h>
>> +#include <linux/fixp-arith.h>
>>   #include <linux/backlight.h>
>>   #include <linux/leds.h>
>>   #include <linux/platform_device.h>
>> @@ -69,11 +71,16 @@ MODULE_LICENSE("GPL");
>>   #define ACER_WMID_GET_GAMING_LED_METHODID 4
>>   #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
>>   #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID 14
>> +#define ACER_WMID_GET_GAMING_FAN_BEHAVIOR_METHODID 15
>> +#define ACER_WMID_SET_GAMING_FAN_SPEED_METHODID 16
>> +#define ACER_WMID_GET_GAMING_FAN_SPEED_METHODID 17
>>   #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
>>   #define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
>>   
>> +#define ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK GENMASK_ULL(7, 0)
>>   #define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
>>   #define ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK GENMASK_ULL(23, 16)
>> +#define ACER_GAMING_FAN_BEHAVIOR_GET_MODE_MASK GENMASK_ULL(23, 8)
>>   
>>   #define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
>>   #define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
>> @@ -81,6 +88,10 @@ MODULE_LICENSE("GPL");
>>   #define ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK GENMASK(1, 0)
>>   #define ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK GENMASK(7, 6)
>>   
>> +#define ACER_GAMING_FAN_SPEED_STATUS_MASK GENMASK_ULL(7, 0)
>> +#define ACER_GAMING_FAN_SPEED_ID_MASK GENMASK_ULL(7, 0)
>> +#define ACER_GAMING_FAN_SPEED_VALUE_MASK GENMASK_ULL(15, 8)
>> +
>>   #define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
>>   #define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
>>   #define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
>> @@ -130,6 +141,11 @@ enum acer_wmi_predator_v4_sensor_id {
>>   	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
>>   };
>>   
>> +enum acer_wmi_gaming_fan_id {
>> +	ACER_WMID_CPU_FAN	= 0x01,
>> +	ACER_WMID_GPU_FAN	= 0x04,
>> +};
> In patch 1 you added these:
>
> +#define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
> ...
> +#define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
> +#define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
>
> Are these numbers related (you even called the field "ID mask")? Could we
> do with less defines for the same thing if they're related?

I honestly don't know if those IDs are related. I would like to play it safe
and keep them separate for now.

>>   enum acer_wmi_gaming_fan_mode {
>>   	ACER_WMID_FAN_MODE_AUTO		= 0x01,
>>   	ACER_WMID_FAN_MODE_TURBO	= 0x02,
>> @@ -292,6 +308,7 @@ struct hotkey_function_type_aa {
>>   #define ACER_CAP_TURBO_FAN		BIT(9)
>>   #define ACER_CAP_PLATFORM_PROFILE	BIT(10)
>>   #define ACER_CAP_HWMON			BIT(11)
>> +#define ACER_CAP_PWM			BIT(12)
>>   
>>   /*
>>    * Interface type flags
>> @@ -386,6 +403,7 @@ struct quirk_entry {
>>   	u8 cpu_fans;
>>   	u8 gpu_fans;
>>   	u8 predator_v4;
>> +	u8 pwm;
>>   };
>>   
>>   static struct quirk_entry *quirks;
>> @@ -405,6 +423,9 @@ static void __init set_quirks(void)
>>   	if (quirks->predator_v4)
>>   		interface->capability |= ACER_CAP_PLATFORM_PROFILE |
>>   					 ACER_CAP_HWMON;
>> +
>> +	if (quirks->pwm)
>> +		interface->capability |= ACER_CAP_PWM;
>>   }
>>   
>>   static int __init dmi_matched(const struct dmi_system_id *dmi)
>> @@ -1651,6 +1672,27 @@ static int WMID_gaming_set_fan_behavior(u16 fan_bitmap, u8 mode_bitmap)
>>   	return 0;
>>   }
>>   
>> +static int WMID_gaming_get_fan_behavior(u16 fan_bitmap, u8 *mode_bitmap)
>> +{
>> +	acpi_status status;
>> +	u32 input = 0;
>> +	u64 result;
>> +
>> +	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_ID_MASK, fan_bitmap);
>> +	status = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_FAN_BEHAVIOR_METHODID, input,
>> +					    &result);
>> +	if (ACPI_FAILURE(status))
>> +		return -EIO;
>> +
>> +	/* The return status must be zero for the operation to have succeeded */
>> +	if (FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK, result))
>> +		return -EIO;
>> +
>> +	*mode_bitmap = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_GET_MODE_MASK, result);
>> +
>> +	return 0;
>> +}
>> +
>>   static void WMID_gaming_set_fan_mode(u8 fan_mode)
>>   {
>>   	u16 mode_bitmap = 0;
>> @@ -1669,6 +1711,55 @@ static void WMID_gaming_set_fan_mode(u8 fan_mode)
>>   	WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
>>   }
>>   
>> +static int WMID_gaming_set_gaming_fan_speed(u8 fan, u8 speed)
>> +{
>> +	acpi_status status;
>> +	u64 input = 0;
>> +	u64 result;
>> +
>> +	if (speed > 100)
>> +		return -EINVAL;
>> +
>> +	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_ID_MASK, fan);
>> +	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_VALUE_MASK, speed);
>> +
>> +	status = WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_FAN_SPEED_METHODID, input, &result);
>> +	if (ACPI_FAILURE(status))
>> +		return -EIO;
>> +
>> +	switch (FIELD_GET(ACER_GAMING_FAN_SPEED_STATUS_MASK, result)) {
>> +	case 0x00:
>> +		return 0;
>> +	case 0x01:
>> +		return -ENODEV;
>> +	case 0x02:
>> +		return -EINVAL;
>> +	default:
>> +		return -ENXIO;
>> +	}
>> +}
>> +
>> +static int WMID_gaming_get_gaming_fan_speed(u8 fan, u8 *speed)
>> +{
>> +	acpi_status status;
>> +	u32 input = 0;
>> +	u64 result;
>> +
>> +	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_ID_MASK, fan);
>> +
>> +	status = WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_FAN_SPEED_METHODID, input,
>> +					    &result);
>> +	if (ACPI_FAILURE(status))
>> +		return -EIO;
>> +
>> +	if (FIELD_GET(ACER_GAMING_FAN_SPEED_STATUS_MASK, result))
>> +		return -ENODEV;
>> +
>> +	*speed = FIELD_GET(ACER_GAMING_FAN_SPEED_VALUE_MASK, result);
>> +
>> +	return 0;
>> +}
>> +
>>   static int WMID_gaming_set_misc_setting(enum acer_wmi_gaming_misc_setting setting, u8 value)
>>   {
>>   	acpi_status status;
>> @@ -2840,6 +2931,16 @@ static const enum acer_wmi_predator_v4_sensor_id acer_wmi_fan_channel_to_sensor_
>>   	[1] = ACER_WMID_SENSOR_GPU_FAN_SPEED,
>>   };
>>   
>> +static const enum acer_wmi_gaming_fan_id acer_wmi_fan_channel_to_fan_id[] = {
>> +	[0] = ACER_WMID_CPU_FAN,
>> +	[1] = ACER_WMID_GPU_FAN,
>> +};
>> +
>> +static const u16 acer_wmi_fan_channel_to_fan_bitmap[] = {
>> +	[0] = ACER_GAMING_FAN_BEHAVIOR_CPU,
>> +	[1] = ACER_GAMING_FAN_BEHAVIOR_GPU,
>> +};
>> +
>>   static umode_t acer_wmi_hwmon_is_visible(const void *data,
>>   					 enum hwmon_sensor_types type, u32 attr,
>>   					 int channel)
>> @@ -2851,6 +2952,11 @@ static umode_t acer_wmi_hwmon_is_visible(const void *data,
>>   	case hwmon_temp:
>>   		sensor_id = acer_wmi_temp_channel_to_sensor_id[channel];
>>   		break;
>> +	case hwmon_pwm:
>> +		if (!has_cap(ACER_CAP_PWM))
>> +			return 0;
>> +
>> +		fallthrough;
>>   	case hwmon_fan:
>>   		sensor_id = acer_wmi_fan_channel_to_sensor_id[channel];
>>   		break;
>> @@ -2858,8 +2964,12 @@ static umode_t acer_wmi_hwmon_is_visible(const void *data,
>>   		return 0;
>>   	}
>>   
>> -	if (*supported_sensors & BIT(sensor_id - 1))
>> +	if (*supported_sensors & BIT(sensor_id - 1)) {
>> +		if (type == hwmon_pwm)
>> +			return 0644;
>> +
>>   		return 0444;
>> +	}
>>   
>>   	return 0;
>>   }
>> @@ -2868,8 +2978,10 @@ static int acer_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>>   			       u32 attr, int channel, long *val)
>>   {
>>   	u64 command = ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING;
>> +	u8 fan, speed, mode_bitmap;
>> +	u16 fan_bitmap;
>> +	int mode, ret;
>>   	u64 result;
>> -	int ret;
>>   
>>   	switch (type) {
>>   	case hwmon_temp:
>> @@ -2893,6 +3005,106 @@ static int acer_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>>   
>>   		*val = FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
>>   		return 0;
>> +	case hwmon_pwm:
>> +		switch (attr) {
>> +		case hwmon_pwm_input:
>> +			fan = acer_wmi_fan_channel_to_fan_id[channel];
>> +			ret = WMID_gaming_get_gaming_fan_speed(fan, &speed);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			*val = fixp_linear_interpolate(0, 0, 100, U8_MAX, speed);
>> +			return 0;
>> +		case hwmon_pwm_enable:
>> +			fan_bitmap = acer_wmi_fan_channel_to_fan_bitmap[channel];
>> +			ret = WMID_gaming_get_fan_behavior(fan_bitmap, &mode_bitmap);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			switch (channel) {
>> +			case 0:
>> +				mode = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK,
>> +						 mode_bitmap);
>> +				break;
>> +			case 1:
>> +				mode = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK,
>> +						 mode_bitmap);
>> +				break;
>> +			default:
>> +				return -EINVAL;
>> +			}
> You've managed to create an internal API that resembles the WMI method
> input by working in terms of fan_bitmap and mode_bitmap which makes things
> complicated as then you have to build logic on top of it to do all the
> mapping in the caller...
>
> I suggest changing this so you make WMID_gaming_get_fan_behavior() to
> input ACER_GAMING_FAN_BEHAVIOR_CPU / GPU (or ACER_WMID_CPU_FAN / GPU_FAN
> depending on the question I asked above) and perform the output mapping
> within that function. I think it would be much cleaner.
>
> I didn't exactly like the internal API in patch 1 either and I'd prefer
> the field GENMASK()s to directly point to the correct bits instead of the
> nested FIELD_GET/PREP()s.

Ok, will do.

>> +			switch (mode) {
>> +			case ACER_WMID_FAN_MODE_AUTO:
>> +				*val = 2;
>> +				return 0;
>> +			case ACER_WMID_FAN_MODE_TURBO:
>> +				*val = 0;
>> +				return 0;
>> +			case ACER_WMID_FAN_MODE_CUSTOM:
>> +				*val = 1;
>> +				return 0;
>> +			default:
>> +				return -ENXIO;
>> +			}
>> +		default:
>> +			return -EOPNOTSUPP;
>> +		}
>> +	default:
>> +		return -EOPNOTSUPP;
>> +	}
>> +}
>> +
>> +static int acer_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>> +				u32 attr, int channel, long val)
>> +{
>> +	u8 fan, speed, mode_bitmap;
>> +	u16 fan_bitmap;
>> +	int mode;
>> +
>> +	switch (type) {
>> +	case hwmon_pwm:
>> +		switch (attr) {
>> +		case hwmon_pwm_input:
>> +			fan = acer_wmi_fan_channel_to_fan_id[channel];
>> +			speed = fixp_linear_interpolate(0, 0, U8_MAX, 100,
>> +							clamp_val(val, 0, U8_MAX));
>> +
>> +			return WMID_gaming_set_gaming_fan_speed(fan, speed);
>> +		case hwmon_pwm_enable:
>> +			fan_bitmap = acer_wmi_fan_channel_to_fan_bitmap[channel];
>> +
>> +			switch (val) {
>> +			case 0:
>> +				mode = ACER_WMID_FAN_MODE_TURBO;
>> +				break;
>> +			case 1:
>> +				mode = ACER_WMID_FAN_MODE_CUSTOM;
>> +				break;
>> +			case 2:
>> +				mode = ACER_WMID_FAN_MODE_AUTO;
>> +				break;
>> +			default:
>> +				return -EINVAL;
>> +			}
>> +
>> +			switch (channel) {
>> +			case 0:
>> +				mode_bitmap = FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK,
>> +							 mode);
>> +				break;
>> +			case 1:
>> +				mode_bitmap = FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK,
>> +							 mode);
>> +				break;
>> +			default:
>> +				return -EINVAL;
>> +			}
> These too look they could be mapped inside WMID_gaming_set_fan_behavior().

I don't think so, as the function also needs to handle the setting of multiple fans
at once.

Thanks,
Armin Wolf

>> +
>> +			return WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
>> +		default:
>> +			return -EOPNOTSUPP;
>> +		}
>>   	default:
>>   		return -EOPNOTSUPP;
>>   	}
>> @@ -2908,11 +3120,16 @@ static const struct hwmon_channel_info *const acer_wmi_hwmon_info[] = {
>>   			   HWMON_F_INPUT,
>>   			   HWMON_F_INPUT
>>   			   ),
>> +	HWMON_CHANNEL_INFO(pwm,
>> +			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
>> +			   HWMON_PWM_INPUT | HWMON_PWM_ENABLE
>> +			   ),
>>   	NULL
>>   };
>>   
>>   static const struct hwmon_ops acer_wmi_hwmon_ops = {
>>   	.read = acer_wmi_hwmon_read,
>> +	.write = acer_wmi_hwmon_write,
>>   	.is_visible = acer_wmi_hwmon_is_visible,
>>   };
>>   
>>

Re: [PATCH v2 2/4] platform/x86: acer-wmi: Add fan control support

[Ilpo Järvinen]

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

On Thu, 2 Oct 2025, Armin Wolf wrote:

> Am 24.09.25 um 12:52 schrieb Ilpo Järvinen:
> 
> > On Tue, 23 Sep 2025, Armin Wolf wrote:
> > 
> > > Add support for controlling the fan speed using the
> > > SetGamingFanSpeed() and GetGamingFanSpeed() WMI methods.
> > > 
> > > This feature is only enabled if the machine has ACER_CAP_PWM enabled
> > > and depend on ACER_CAP_HWMON for detecting the number of available
> > > fans.
> > > 
> > > Reviewed-by: Kurt Borja <kuurtb@gmail.com>
> > > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > > ---
> > >   drivers/platform/x86/acer-wmi.c | 221 +++++++++++++++++++++++++++++++-
> > >   1 file changed, 219 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/acer-wmi.c
> > > b/drivers/platform/x86/acer-wmi.c
> > > index a41555ee8589..345b6e24ae31 100644
> > > --- a/drivers/platform/x86/acer-wmi.c
> > > +++ b/drivers/platform/x86/acer-wmi.c
> > > @@ -12,10 +12,12 @@
> > >   #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > >     #include <linux/kernel.h>
> > > +#include <linux/minmax.h>
> > >   #include <linux/module.h>
> > >   #include <linux/init.h>
> > >   #include <linux/types.h>
> > >   #include <linux/dmi.h>
> > > +#include <linux/fixp-arith.h>
> > >   #include <linux/backlight.h>
> > >   #include <linux/leds.h>
> > >   #include <linux/platform_device.h>
> > > @@ -69,11 +71,16 @@ MODULE_LICENSE("GPL");
> > >   #define ACER_WMID_GET_GAMING_LED_METHODID 4
> > >   #define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
> > >   #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR_METHODID 14
> > > +#define ACER_WMID_GET_GAMING_FAN_BEHAVIOR_METHODID 15
> > > +#define ACER_WMID_SET_GAMING_FAN_SPEED_METHODID 16
> > > +#define ACER_WMID_GET_GAMING_FAN_SPEED_METHODID 17
> > >   #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
> > >   #define ACER_WMID_GET_GAMING_MISC_SETTING_METHODID 23
> > >   +#define ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK GENMASK_ULL(7, 0)
> > >   #define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
> > >   #define ACER_GAMING_FAN_BEHAVIOR_SET_MODE_MASK GENMASK_ULL(23, 16)
> > > +#define ACER_GAMING_FAN_BEHAVIOR_GET_MODE_MASK GENMASK_ULL(23, 8)
> > >     #define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
> > >   #define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
> > > @@ -81,6 +88,10 @@ MODULE_LICENSE("GPL");
> > >   #define ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK GENMASK(1, 0)
> > >   #define ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK GENMASK(7, 6)
> > >   +#define ACER_GAMING_FAN_SPEED_STATUS_MASK GENMASK_ULL(7, 0)
> > > +#define ACER_GAMING_FAN_SPEED_ID_MASK GENMASK_ULL(7, 0)
> > > +#define ACER_GAMING_FAN_SPEED_VALUE_MASK GENMASK_ULL(15, 8)
> > > +
> > >   #define ACER_GAMING_MISC_SETTING_STATUS_MASK GENMASK_ULL(7, 0)
> > >   #define ACER_GAMING_MISC_SETTING_INDEX_MASK GENMASK_ULL(7, 0)
> > >   #define ACER_GAMING_MISC_SETTING_VALUE_MASK GENMASK_ULL(15, 8)
> > > @@ -130,6 +141,11 @@ enum acer_wmi_predator_v4_sensor_id {
> > >   	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
> > >   };
> > >   +enum acer_wmi_gaming_fan_id {
> > > +	ACER_WMID_CPU_FAN	= 0x01,
> > > +	ACER_WMID_GPU_FAN	= 0x04,
> > > +};
> > In patch 1 you added these:
> > 
> > +#define ACER_GAMING_FAN_BEHAVIOR_ID_MASK GENMASK_ULL(15, 0)
> > ...
> > +#define ACER_GAMING_FAN_BEHAVIOR_CPU BIT(0)
> > +#define ACER_GAMING_FAN_BEHAVIOR_GPU BIT(3)
> > 
> > Are these numbers related (you even called the field "ID mask")? Could we
> > do with less defines for the same thing if they're related?
> 
> I honestly don't know if those IDs are related. I would like to play it safe
> and keep them separate for now.
> 
> > >   enum acer_wmi_gaming_fan_mode {
> > >   	ACER_WMID_FAN_MODE_AUTO		= 0x01,
> > >   	ACER_WMID_FAN_MODE_TURBO	= 0x02,
> > > @@ -292,6 +308,7 @@ struct hotkey_function_type_aa {
> > >   #define ACER_CAP_TURBO_FAN		BIT(9)
> > >   #define ACER_CAP_PLATFORM_PROFILE	BIT(10)
> > >   #define ACER_CAP_HWMON			BIT(11)
> > > +#define ACER_CAP_PWM			BIT(12)
> > >     /*
> > >    * Interface type flags
> > > @@ -386,6 +403,7 @@ struct quirk_entry {
> > >   	u8 cpu_fans;
> > >   	u8 gpu_fans;
> > >   	u8 predator_v4;
> > > +	u8 pwm;
> > >   };
> > >     static struct quirk_entry *quirks;
> > > @@ -405,6 +423,9 @@ static void __init set_quirks(void)
> > >   	if (quirks->predator_v4)
> > >   		interface->capability |= ACER_CAP_PLATFORM_PROFILE |
> > >   					 ACER_CAP_HWMON;
> > > +
> > > +	if (quirks->pwm)
> > > +		interface->capability |= ACER_CAP_PWM;
> > >   }
> > >     static int __init dmi_matched(const struct dmi_system_id *dmi)
> > > @@ -1651,6 +1672,27 @@ static int WMID_gaming_set_fan_behavior(u16
> > > fan_bitmap, u8 mode_bitmap)
> > >   	return 0;
> > >   }
> > >   +static int WMID_gaming_get_fan_behavior(u16 fan_bitmap, u8
> > > *mode_bitmap)
> > > +{
> > > +	acpi_status status;
> > > +	u32 input = 0;
> > > +	u64 result;
> > > +
> > > +	input |= FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_ID_MASK, fan_bitmap);
> > > +	status =
> > > WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_FAN_BEHAVIOR_METHODID,
> > > input,
> > > +					    &result);
> > > +	if (ACPI_FAILURE(status))
> > > +		return -EIO;
> > > +
> > > +	/* The return status must be zero for the operation to have succeeded
> > > */
> > > +	if (FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_STATUS_MASK, result))
> > > +		return -EIO;
> > > +
> > > +	*mode_bitmap = FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_GET_MODE_MASK,
> > > result);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   static void WMID_gaming_set_fan_mode(u8 fan_mode)
> > >   {
> > >   	u16 mode_bitmap = 0;
> > > @@ -1669,6 +1711,55 @@ static void WMID_gaming_set_fan_mode(u8 fan_mode)
> > >   	WMID_gaming_set_fan_behavior(fan_bitmap, mode_bitmap);
> > >   }
> > >   +static int WMID_gaming_set_gaming_fan_speed(u8 fan, u8 speed)
> > > +{
> > > +	acpi_status status;
> > > +	u64 input = 0;
> > > +	u64 result;
> > > +
> > > +	if (speed > 100)
> > > +		return -EINVAL;
> > > +
> > > +	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_ID_MASK, fan);
> > > +	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_VALUE_MASK, speed);
> > > +
> > > +	status =
> > > WMI_gaming_execute_u64(ACER_WMID_SET_GAMING_FAN_SPEED_METHODID, input,
> > > &result);
> > > +	if (ACPI_FAILURE(status))
> > > +		return -EIO;
> > > +
> > > +	switch (FIELD_GET(ACER_GAMING_FAN_SPEED_STATUS_MASK, result)) {
> > > +	case 0x00:
> > > +		return 0;
> > > +	case 0x01:
> > > +		return -ENODEV;
> > > +	case 0x02:
> > > +		return -EINVAL;
> > > +	default:
> > > +		return -ENXIO;
> > > +	}
> > > +}
> > > +
> > > +static int WMID_gaming_get_gaming_fan_speed(u8 fan, u8 *speed)
> > > +{
> > > +	acpi_status status;
> > > +	u32 input = 0;
> > > +	u64 result;
> > > +
> > > +	input |= FIELD_PREP(ACER_GAMING_FAN_SPEED_ID_MASK, fan);
> > > +
> > > +	status =
> > > WMI_gaming_execute_u32_u64(ACER_WMID_GET_GAMING_FAN_SPEED_METHODID, input,
> > > +					    &result);
> > > +	if (ACPI_FAILURE(status))
> > > +		return -EIO;
> > > +
> > > +	if (FIELD_GET(ACER_GAMING_FAN_SPEED_STATUS_MASK, result))
> > > +		return -ENODEV;
> > > +
> > > +	*speed = FIELD_GET(ACER_GAMING_FAN_SPEED_VALUE_MASK, result);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >   static int WMID_gaming_set_misc_setting(enum
> > > acer_wmi_gaming_misc_setting setting, u8 value)
> > >   {
> > >   	acpi_status status;
> > > @@ -2840,6 +2931,16 @@ static const enum acer_wmi_predator_v4_sensor_id
> > > acer_wmi_fan_channel_to_sensor_
> > >   	[1] = ACER_WMID_SENSOR_GPU_FAN_SPEED,
> > >   };
> > >   +static const enum acer_wmi_gaming_fan_id
> > > acer_wmi_fan_channel_to_fan_id[] = {
> > > +	[0] = ACER_WMID_CPU_FAN,
> > > +	[1] = ACER_WMID_GPU_FAN,
> > > +};
> > > +
> > > +static const u16 acer_wmi_fan_channel_to_fan_bitmap[] = {
> > > +	[0] = ACER_GAMING_FAN_BEHAVIOR_CPU,
> > > +	[1] = ACER_GAMING_FAN_BEHAVIOR_GPU,
> > > +};
> > > +
> > >   static umode_t acer_wmi_hwmon_is_visible(const void *data,
> > >   					 enum hwmon_sensor_types type, u32
> > > attr,
> > >   					 int channel)
> > > @@ -2851,6 +2952,11 @@ static umode_t acer_wmi_hwmon_is_visible(const void
> > > *data,
> > >   	case hwmon_temp:
> > >   		sensor_id = acer_wmi_temp_channel_to_sensor_id[channel];
> > >   		break;
> > > +	case hwmon_pwm:
> > > +		if (!has_cap(ACER_CAP_PWM))
> > > +			return 0;
> > > +
> > > +		fallthrough;
> > >   	case hwmon_fan:
> > >   		sensor_id = acer_wmi_fan_channel_to_sensor_id[channel];
> > >   		break;
> > > @@ -2858,8 +2964,12 @@ static umode_t acer_wmi_hwmon_is_visible(const void
> > > *data,
> > >   		return 0;
> > >   	}
> > >   -	if (*supported_sensors & BIT(sensor_id - 1))
> > > +	if (*supported_sensors & BIT(sensor_id - 1)) {
> > > +		if (type == hwmon_pwm)
> > > +			return 0644;
> > > +
> > >   		return 0444;
> > > +	}
> > >     	return 0;
> > >   }
> > > @@ -2868,8 +2978,10 @@ static int acer_wmi_hwmon_read(struct device *dev,
> > > enum hwmon_sensor_types type,
> > >   			       u32 attr, int channel, long *val)
> > >   {
> > >   	u64 command = ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING;
> > > +	u8 fan, speed, mode_bitmap;
> > > +	u16 fan_bitmap;
> > > +	int mode, ret;
> > >   	u64 result;
> > > -	int ret;
> > >     	switch (type) {
> > >   	case hwmon_temp:
> > > @@ -2893,6 +3005,106 @@ static int acer_wmi_hwmon_read(struct device *dev,
> > > enum hwmon_sensor_types type,
> > >     		*val =
> > > FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
> > >   		return 0;
> > > +	case hwmon_pwm:
> > > +		switch (attr) {
> > > +		case hwmon_pwm_input:
> > > +			fan = acer_wmi_fan_channel_to_fan_id[channel];
> > > +			ret = WMID_gaming_get_gaming_fan_speed(fan, &speed);
> > > +			if (ret < 0)
> > > +				return ret;
> > > +
> > > +			*val = fixp_linear_interpolate(0, 0, 100, U8_MAX,
> > > speed);
> > > +			return 0;
> > > +		case hwmon_pwm_enable:
> > > +			fan_bitmap =
> > > acer_wmi_fan_channel_to_fan_bitmap[channel];
> > > +			ret = WMID_gaming_get_fan_behavior(fan_bitmap,
> > > &mode_bitmap);
> > > +			if (ret < 0)
> > > +				return ret;
> > > +
> > > +			switch (channel) {
> > > +			case 0:
> > > +				mode =
> > > FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK,
> > > +						 mode_bitmap);
> > > +				break;
> > > +			case 1:
> > > +				mode =
> > > FIELD_GET(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK,
> > > +						 mode_bitmap);
> > > +				break;
> > > +			default:
> > > +				return -EINVAL;
> > > +			}
> > You've managed to create an internal API that resembles the WMI method
> > input by working in terms of fan_bitmap and mode_bitmap which makes things
> > complicated as then you have to build logic on top of it to do all the
> > mapping in the caller...
> > 
> > I suggest changing this so you make WMID_gaming_get_fan_behavior() to
> > input ACER_GAMING_FAN_BEHAVIOR_CPU / GPU (or ACER_WMID_CPU_FAN / GPU_FAN
> > depending on the question I asked above) and perform the output mapping
> > within that function. I think it would be much cleaner.
> > 
> > I didn't exactly like the internal API in patch 1 either and I'd prefer
> > the field GENMASK()s to directly point to the correct bits instead of the
> > nested FIELD_GET/PREP()s.
> 
> Ok, will do.
> 
> > > +			switch (mode) {
> > > +			case ACER_WMID_FAN_MODE_AUTO:
> > > +				*val = 2;
> > > +				return 0;
> > > +			case ACER_WMID_FAN_MODE_TURBO:
> > > +				*val = 0;
> > > +				return 0;
> > > +			case ACER_WMID_FAN_MODE_CUSTOM:
> > > +				*val = 1;
> > > +				return 0;
> > > +			default:
> > > +				return -ENXIO;
> > > +			}
> > > +		default:
> > > +			return -EOPNOTSUPP;
> > > +		}
> > > +	default:
> > > +		return -EOPNOTSUPP;
> > > +	}
> > > +}
> > > +
> > > +static int acer_wmi_hwmon_write(struct device *dev, enum
> > > hwmon_sensor_types type,
> > > +				u32 attr, int channel, long val)
> > > +{
> > > +	u8 fan, speed, mode_bitmap;
> > > +	u16 fan_bitmap;
> > > +	int mode;
> > > +
> > > +	switch (type) {
> > > +	case hwmon_pwm:
> > > +		switch (attr) {
> > > +		case hwmon_pwm_input:
> > > +			fan = acer_wmi_fan_channel_to_fan_id[channel];
> > > +			speed = fixp_linear_interpolate(0, 0, U8_MAX, 100,
> > > +							clamp_val(val, 0,
> > > U8_MAX));
> > > +
> > > +			return WMID_gaming_set_gaming_fan_speed(fan, speed);
> > > +		case hwmon_pwm_enable:
> > > +			fan_bitmap =
> > > acer_wmi_fan_channel_to_fan_bitmap[channel];
> > > +
> > > +			switch (val) {
> > > +			case 0:
> > > +				mode = ACER_WMID_FAN_MODE_TURBO;
> > > +				break;
> > > +			case 1:
> > > +				mode = ACER_WMID_FAN_MODE_CUSTOM;
> > > +				break;
> > > +			case 2:
> > > +				mode = ACER_WMID_FAN_MODE_AUTO;
> > > +				break;
> > > +			default:
> > > +				return -EINVAL;
> > > +			}
> > > +
> > > +			switch (channel) {
> > > +			case 0:
> > > +				mode_bitmap =
> > > FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_CPU_MODE_MASK,
> > > +							 mode);
> > > +				break;
> > > +			case 1:
> > > +				mode_bitmap =
> > > FIELD_PREP(ACER_GAMING_FAN_BEHAVIOR_GPU_MODE_MASK,
> > > +							 mode);
> > > +				break;
> > > +			default:
> > > +				return -EINVAL;
> > > +			}
> > These too look they could be mapped inside WMID_gaming_set_fan_behavior().
> 
> I don't think so, as the function also needs to handle the setting of multiple
> fans
> at once.

Oh, right. How about making it two functions then, one which is called 
from here to set one fan and which then internally calls into the 
function that can set any fans?


-- 
 i.

[PATCH v2 3/4] platform/x86: acer-wmi: Enable fan control for PH16-72 and PT14-51

[Armin Wolf]

Both machines support the necessary WMI methods, so enable fan control
for them.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 345b6e24ae31..d2cee0bdfd16 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -465,6 +465,7 @@ static struct quirk_entry quirk_acer_predator_ph16_72 = {
 	.cpu_fans = 1,
 	.gpu_fans = 1,
 	.predator_v4 = 1,
+	.pwm = 1,
 };
 
 static struct quirk_entry quirk_acer_predator_pt14_51 = {
@@ -472,6 +473,7 @@ static struct quirk_entry quirk_acer_predator_pt14_51 = {
 	.cpu_fans = 1,
 	.gpu_fans = 1,
 	.predator_v4 = 1,
+	.pwm = 1,
 };
 
 static struct quirk_entry quirk_acer_predator_v4 = {
-- 
2.39.5

[PATCH v2 4/4] platform/x86: acer-wmi: Add support for PHN16-72

[Armin Wolf]

A user reported that the config of the PH16-72 also works on
the PHN16-72. Add support for this new device as well.

Suggested-by: Fa-Iz Faadhillah Ibrahim <faiz.faadhillah@gmail.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index d2cee0bdfd16..485ed034651b 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -692,6 +692,15 @@ static const struct dmi_system_id acer_quirks[] __initconst = {
 		},
 		.driver_data = &quirk_acer_predator_ph16_72,
 	},
+	{
+		.callback = dmi_matched,
+		.ident = "Acer Predator Helios Neo 16",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Predator PHN16-72"),
+		},
+		.driver_data = &quirk_acer_predator_ph16_72,
+	},
 	{
 		.callback = dmi_matched,
 		.ident = "Acer Predator PH18-71",
-- 
2.39.5

Re: [PATCH v2 0/4] platform/x86: acer-wmi: Add fan control support

[Fa-Iz Faadhillah Ibrahim]

On Wednesday, September 24, 2025 4:52 AM Armin Wolf wrote:
> This patch series aims to add fan control support to the acer-wmi
> driver. The patches are compile-tested only and need to be tested
> on real hardware to verify that they actually work.
> 
> I CCed three users who requested support for this feature. I would be
> very happy if one of you could test those patches and report back.
> 
> Changes since v1:
> - remove unnecessary blank line
> 
> Changes since RFC v2:
> - improve error handling when setting fan behavior
> - Add Reviewed-by tags
> - whitelist PHN16-72
> 
> Changes since RFC v1:
> - remove duplicate include and replace hwmon_pwm_mode with
>   hwmon_pwm_enable in second patch
> 
> Armin Wolf (4):
>   platform/x86: acer-wmi: Fix setting of fan behavior
>   platform/x86: acer-wmi: Add fan control support
>   platform/x86: acer-wmi: Enable fan control for PH16-72 and PT14-51
>   platform/x86: acer-wmi: Add support for PHN16-72
> 
>  drivers/platform/x86/acer-wmi.c | 308 +++++++++++++++++++++++++++++---
>  1 file changed, 283 insertions(+), 25 deletions(-)

Hi,

I've tested your patch on my PHN16-72, Fan control works just fine, and it does 
detect my laptop properly.

Thanks,
Fa-Iz Faadhillah Ibrahim

Re: [PATCH v2 0/4] platform/x86: acer-wmi: Add fan control support

[Armin Wolf]

Am 29.09.25 um 04:04 schrieb Fa-Iz Faadhillah Ibrahim:

> On Wednesday, September 24, 2025 4:52 AM Armin Wolf wrote:
>> This patch series aims to add fan control support to the acer-wmi
>> driver. The patches are compile-tested only and need to be tested
>> on real hardware to verify that they actually work.
>>
>> I CCed three users who requested support for this feature. I would be
>> very happy if one of you could test those patches and report back.
>>
>> Changes since v1:
>> - remove unnecessary blank line
>>
>> Changes since RFC v2:
>> - improve error handling when setting fan behavior
>> - Add Reviewed-by tags
>> - whitelist PHN16-72
>>
>> Changes since RFC v1:
>> - remove duplicate include and replace hwmon_pwm_mode with
>>    hwmon_pwm_enable in second patch
>>
>> Armin Wolf (4):
>>    platform/x86: acer-wmi: Fix setting of fan behavior
>>    platform/x86: acer-wmi: Add fan control support
>>    platform/x86: acer-wmi: Enable fan control for PH16-72 and PT14-51
>>    platform/x86: acer-wmi: Add support for PHN16-72
>>
>>   drivers/platform/x86/acer-wmi.c | 308 +++++++++++++++++++++++++++++---
>>   1 file changed, 283 insertions(+), 25 deletions(-)
> Hi,
>
> I've tested your patch on my PHN16-72, Fan control works just fine, and it does
> detect my laptop properly.
>
> Thanks,
> Fa-Iz Faadhillah Ibrahim
>
Thank you, i will send a improved patch series in the coming days. I would be
very happy if you cloud test those too.

Thanks,
Armin Wolf