[PATCH v4 0/5] acer-wmi: Various improvements

[PATCH v4 0/5] acer-wmi: Various improvements

[Armin Wolf]

This patch series contains various improvements to the acer-wmi
driver based on user reports:

- adds support for the Acer PH14-51
- improves hwmon support
- ignores function 8 events

The changes where tested on a real machine and appear to work.

Changes since v3:
- fix duplicate definition of ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK
- add Tested-by tags

Changes since v2:
- fix compilation error after patch 3
- replace GENMASK() with GENMASK_ULL() and adjust bit numbers

Changes since v1:
- fix spelling issue in patch 2
- rework patch 3 and 4
- add Reviewed-by tag to patch 5

Armin Wolf (5):
  platform/x86: acer-wmi: Add support for Acer PH14-51
  platform/x86: acer-wmi: Rename ACER_CAP_FAN_SPEED_READ
  platform/x86: acer-wmi: Improve error handling when reading gaming
    system information
  platform/x86: acer-wmi: Implement proper hwmon support
  platform/x86: acer-wmi: Ignore AC events

 drivers/platform/x86/acer-wmi.c | 166 ++++++++++++++++++++++++--------
 1 file changed, 124 insertions(+), 42 deletions(-)

--
2.39.5

[PATCH v4 1/5] platform/x86: acer-wmi: Add support for Acer PH14-51

[Armin Wolf]

Add the Acer Predator PT14-51 to acer_quirks to provide support
for the turbo button and predator_v4 hwmon interface.

Reported-by: Rayan Margham <rayanmargham4@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/CACzB==6tUsCnr5musVMz-EymjTUCJfNtKzhMFYqMRU_h=kydXA@mail.gmail.com
Tested-by: Rayan Margham <rayanmargham4@gmail.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index d09baa3d3d90..5cff538ee67f 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -393,6 +393,13 @@ static struct quirk_entry quirk_acer_predator_ph315_53 = {
 	.gpu_fans = 1,
 };

+static struct quirk_entry quirk_acer_predator_pt14_51 = {
+	.turbo = 1,
+	.cpu_fans = 1,
+	.gpu_fans = 1,
+	.predator_v4 = 1,
+};
+
 static struct quirk_entry quirk_acer_predator_v4 = {
 	.predator_v4 = 1,
 };
@@ -600,6 +607,15 @@ static const struct dmi_system_id acer_quirks[] __initconst = {
 		},
 		.driver_data = &quirk_acer_predator_v4,
 	},
+	{
+		.callback = dmi_matched,
+		.ident = "Acer Predator PT14-51",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Predator PT14-51"),
+		},
+		.driver_data = &quirk_acer_predator_pt14_51,
+	},
 	{
 		.callback = set_force_caps,
 		.ident = "Acer Aspire Switch 10E SW3-016",
--
2.39.5

Re: [PATCH v4 1/5] platform/x86: acer-wmi: Add support for Acer PH14-51

[Kurt Borja]

On Tue, Dec 10, 2024 at 01:16:53AM +0100, Armin Wolf wrote:
> Add the Acer Predator PT14-51 to acer_quirks to provide support
> for the turbo button and predator_v4 hwmon interface.

Reviewed-by: Kurt Borja <kuurtb@gmail.com>

> 
> Reported-by: Rayan Margham <rayanmargham4@gmail.com>
> Closes: https://lore.kernel.org/platform-driver-x86/CACzB==6tUsCnr5musVMz-EymjTUCJfNtKzhMFYqMRU_h=kydXA@mail.gmail.com
> Tested-by: Rayan Margham <rayanmargham4@gmail.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/acer-wmi.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index d09baa3d3d90..5cff538ee67f 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -393,6 +393,13 @@ static struct quirk_entry quirk_acer_predator_ph315_53 = {
>  	.gpu_fans = 1,
>  };
> 
> +static struct quirk_entry quirk_acer_predator_pt14_51 = {
> +	.turbo = 1,
> +	.cpu_fans = 1,
> +	.gpu_fans = 1,
> +	.predator_v4 = 1,
> +};
> +
>  static struct quirk_entry quirk_acer_predator_v4 = {
>  	.predator_v4 = 1,
>  };
> @@ -600,6 +607,15 @@ static const struct dmi_system_id acer_quirks[] __initconst = {
>  		},
>  		.driver_data = &quirk_acer_predator_v4,
>  	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "Acer Predator PT14-51",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Predator PT14-51"),
> +		},
> +		.driver_data = &quirk_acer_predator_pt14_51,
> +	},
>  	{
>  		.callback = set_force_caps,
>  		.ident = "Acer Aspire Switch 10E SW3-016",
> --
> 2.39.5
> 

[PATCH v4 2/5] platform/x86: acer-wmi: Rename ACER_CAP_FAN_SPEED_READ

[Armin Wolf]

Rename ACER_CAP_FAN_SPEED_READ to ACER_CAP_HWMON to prepare for
upcoming changes in the hwmon handling code.

Tested-by: Rayan Margham <rayanmargham4@gmail.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 5cff538ee67f..dd57787466b9 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -246,7 +246,7 @@ struct hotkey_function_type_aa {
 #define ACER_CAP_TURBO_LED		BIT(8)
 #define ACER_CAP_TURBO_FAN		BIT(9)
 #define ACER_CAP_PLATFORM_PROFILE	BIT(10)
-#define ACER_CAP_FAN_SPEED_READ		BIT(11)
+#define ACER_CAP_HWMON			BIT(11)

 /*
  * Interface type flags
@@ -358,7 +358,7 @@ static void __init set_quirks(void)

 	if (quirks->predator_v4)
 		interface->capability |= ACER_CAP_PLATFORM_PROFILE |
-					 ACER_CAP_FAN_SPEED_READ;
+					 ACER_CAP_HWMON;
 }

 static int __init dmi_matched(const struct dmi_system_id *dmi)
@@ -2551,7 +2551,7 @@ static int acer_platform_probe(struct platform_device *device)
 			goto error_platform_profile;
 	}

-	if (has_cap(ACER_CAP_FAN_SPEED_READ)) {
+	if (has_cap(ACER_CAP_HWMON)) {
 		err = acer_wmi_hwmon_init();
 		if (err)
 			goto error_hwmon;
--
2.39.5

Re: [PATCH v4 2/5] platform/x86: acer-wmi: Rename ACER_CAP_FAN_SPEED_READ

[Kurt Borja]

On Tue, Dec 10, 2024 at 01:16:54AM +0100, Armin Wolf wrote:
> Rename ACER_CAP_FAN_SPEED_READ to ACER_CAP_HWMON to prepare for
> upcoming changes in the hwmon handling code.

Reviewed-by: Kurt Borja <kuurtb@gmail.com>

> 
> Tested-by: Rayan Margham <rayanmargham4@gmail.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/acer-wmi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 5cff538ee67f..dd57787466b9 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -246,7 +246,7 @@ struct hotkey_function_type_aa {
>  #define ACER_CAP_TURBO_LED		BIT(8)
>  #define ACER_CAP_TURBO_FAN		BIT(9)
>  #define ACER_CAP_PLATFORM_PROFILE	BIT(10)
> -#define ACER_CAP_FAN_SPEED_READ		BIT(11)
> +#define ACER_CAP_HWMON			BIT(11)
> 
>  /*
>   * Interface type flags
> @@ -358,7 +358,7 @@ static void __init set_quirks(void)
> 
>  	if (quirks->predator_v4)
>  		interface->capability |= ACER_CAP_PLATFORM_PROFILE |
> -					 ACER_CAP_FAN_SPEED_READ;
> +					 ACER_CAP_HWMON;
>  }
> 
>  static int __init dmi_matched(const struct dmi_system_id *dmi)
> @@ -2551,7 +2551,7 @@ static int acer_platform_probe(struct platform_device *device)
>  			goto error_platform_profile;
>  	}
> 
> -	if (has_cap(ACER_CAP_FAN_SPEED_READ)) {
> +	if (has_cap(ACER_CAP_HWMON)) {
>  		err = acer_wmi_hwmon_init();
>  		if (err)
>  			goto error_hwmon;
> --
> 2.39.5
> 

[PATCH v4 3/5] platform/x86: acer-wmi: Improve error handling when reading gaming system information

[Armin Wolf]

If a call to ACER_WMID_GET_GAMING_SYS_INFO_METHODID fails, the lower
8 bits will be non-zero. Introduce a helper function to check this and
use it when reading gaming system information.

Tested-by: Rayan Margham <rayanmargham4@gmail.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 55 +++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index dd57787466b9..ac4500f33b8c 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -70,6 +70,7 @@ MODULE_LICENSE("GPL");

 #define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54

+#define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
 #define ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK GENMASK(20, 8)

 /*
@@ -1513,6 +1514,24 @@ static acpi_status WMID_gaming_get_u64(u64 *value, u32 cap)
 	return status;
 }

+static int WMID_gaming_get_sys_info(u32 command, u64 *out)
+{
+	acpi_status status;
+	u64 result;
+
+	status = WMI_gaming_execute_u64(ACER_WMID_GET_GAMING_SYS_INFO_METHODID, command, &result);
+	if (ACPI_FAILURE(status))
+		return -EIO;
+
+	/* The return status must be zero for the operation to have succeeded */
+	if (FIELD_GET(ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK, result))
+		return -EIO;
+
+	*out = result;
+
+	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*/
@@ -1762,22 +1781,23 @@ static int acer_gsensor_event(void)

 static int acer_get_fan_speed(int fan)
 {
-	if (quirks->predator_v4) {
-		acpi_status status;
-		u64 fanspeed;
+	u64 fanspeed;
+	u32 command;
+	int ret;

-		status = WMI_gaming_execute_u64(
-			ACER_WMID_GET_GAMING_SYS_INFO_METHODID,
-			fan == 0 ? ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED :
-				   ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED,
-			&fanspeed);
+	if (!quirks->predator_v4)
+		return -EOPNOTSUPP;

-		if (ACPI_FAILURE(status))
-			return -EIO;
+	if (fan == 0)
+		command = ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED;
+	else
+		command = ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED;

-		return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
-	}
-	return -EOPNOTSUPP;
+	ret = WMID_gaming_get_sys_info(command, &fanspeed);
+	if (ret < 0)
+		return ret;
+
+	return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
 }

 /*
@@ -1942,12 +1962,9 @@ static int acer_thermal_profile_change(void)
 			return err;

 		/* Check power source */
-		status = WMI_gaming_execute_u64(
-			ACER_WMID_GET_GAMING_SYS_INFO_METHODID,
-			ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
-
-		if (ACPI_FAILURE(status))
-			return -EIO;
+		err = WMID_gaming_get_sys_info(ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
+		if (err < 0)
+			return err;

 		switch (current_tp) {
 		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
--
2.39.5

Re: [PATCH v4 3/5] platform/x86: acer-wmi: Improve error handling when reading gaming system information

[Kurt Borja]

On Tue, Dec 10, 2024 at 01:16:55AM +0100, Armin Wolf wrote:
> If a call to ACER_WMID_GET_GAMING_SYS_INFO_METHODID fails, the lower
> 8 bits will be non-zero. Introduce a helper function to check this and
> use it when reading gaming system information.
> 
> Tested-by: Rayan Margham <rayanmargham4@gmail.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/acer-wmi.c | 55 +++++++++++++++++++++------------
>  1 file changed, 36 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index dd57787466b9..ac4500f33b8c 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -70,6 +70,7 @@ MODULE_LICENSE("GPL");
> 
>  #define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
> 
> +#define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
>  #define ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK GENMASK(20, 8)
> 
>  /*
> @@ -1513,6 +1514,24 @@ static acpi_status WMID_gaming_get_u64(u64 *value, u32 cap)
>  	return status;
>  }
> 
> +static int WMID_gaming_get_sys_info(u32 command, u64 *out)
> +{
> +	acpi_status status;
> +	u64 result;
> +
> +	status = WMI_gaming_execute_u64(ACER_WMID_GET_GAMING_SYS_INFO_METHODID, command, &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	/* The return status must be zero for the operation to have succeeded */
> +	if (FIELD_GET(ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK, result))
> +		return -EIO;
> +
> +	*out = result;
> +
> +	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*/
> @@ -1762,22 +1781,23 @@ static int acer_gsensor_event(void)
> 
>  static int acer_get_fan_speed(int fan)
>  {
> -	if (quirks->predator_v4) {
> -		acpi_status status;
> -		u64 fanspeed;
> +	u64 fanspeed;
> +	u32 command;
> +	int ret;
> 
> -		status = WMI_gaming_execute_u64(
> -			ACER_WMID_GET_GAMING_SYS_INFO_METHODID,
> -			fan == 0 ? ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED :
> -				   ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED,
> -			&fanspeed);
> +	if (!quirks->predator_v4)
> +		return -EOPNOTSUPP;
> 
> -		if (ACPI_FAILURE(status))
> -			return -EIO;
> +	if (fan == 0)
> +		command = ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED;
> +	else
> +		command = ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED;
> 
> -		return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
> -	}
> -	return -EOPNOTSUPP;
> +	ret = WMID_gaming_get_sys_info(command, &fanspeed);
> +	if (ret < 0)
> +		return ret;
> +
> +	return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
>  }

I wonder if it's necessary to refactor this function if you are going to
drop it on Patch 4.

> 
>  /*
> @@ -1942,12 +1962,9 @@ static int acer_thermal_profile_change(void)
>  			return err;
> 
>  		/* Check power source */
> -		status = WMI_gaming_execute_u64(
> -			ACER_WMID_GET_GAMING_SYS_INFO_METHODID,
> -			ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
> -
> -		if (ACPI_FAILURE(status))
> -			return -EIO;
> +		err = WMID_gaming_get_sys_info(ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
> +		if (err < 0)
> +			return err;
> 
>  		switch (current_tp) {
>  		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
> --
> 2.39.5
> 

Re: [PATCH v4 3/5] platform/x86: acer-wmi: Improve error handling when reading gaming system information

[Armin Wolf]

Am 10.12.24 um 03:06 schrieb Kurt Borja:

> On Tue, Dec 10, 2024 at 01:16:55AM +0100, Armin Wolf wrote:
>> If a call to ACER_WMID_GET_GAMING_SYS_INFO_METHODID fails, the lower
>> 8 bits will be non-zero. Introduce a helper function to check this and
>> use it when reading gaming system information.
>>
>> Tested-by: Rayan Margham <rayanmargham4@gmail.com>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/platform/x86/acer-wmi.c | 55 +++++++++++++++++++++------------
>>   1 file changed, 36 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
>> index dd57787466b9..ac4500f33b8c 100644
>> --- a/drivers/platform/x86/acer-wmi.c
>> +++ b/drivers/platform/x86/acer-wmi.c
>> @@ -70,6 +70,7 @@ MODULE_LICENSE("GPL");
>>
>>   #define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
>>
>> +#define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
>>   #define ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK GENMASK(20, 8)
>>
>>   /*
>> @@ -1513,6 +1514,24 @@ static acpi_status WMID_gaming_get_u64(u64 *value, u32 cap)
>>   	return status;
>>   }
>>
>> +static int WMID_gaming_get_sys_info(u32 command, u64 *out)
>> +{
>> +	acpi_status status;
>> +	u64 result;
>> +
>> +	status = WMI_gaming_execute_u64(ACER_WMID_GET_GAMING_SYS_INFO_METHODID, command, &result);
>> +	if (ACPI_FAILURE(status))
>> +		return -EIO;
>> +
>> +	/* The return status must be zero for the operation to have succeeded */
>> +	if (FIELD_GET(ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK, result))
>> +		return -EIO;
>> +
>> +	*out = result;
>> +
>> +	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*/
>> @@ -1762,22 +1781,23 @@ static int acer_gsensor_event(void)
>>
>>   static int acer_get_fan_speed(int fan)
>>   {
>> -	if (quirks->predator_v4) {
>> -		acpi_status status;
>> -		u64 fanspeed;
>> +	u64 fanspeed;
>> +	u32 command;
>> +	int ret;
>>
>> -		status = WMI_gaming_execute_u64(
>> -			ACER_WMID_GET_GAMING_SYS_INFO_METHODID,
>> -			fan == 0 ? ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED :
>> -				   ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED,
>> -			&fanspeed);
>> +	if (!quirks->predator_v4)
>> +		return -EOPNOTSUPP;
>>
>> -		if (ACPI_FAILURE(status))
>> -			return -EIO;
>> +	if (fan == 0)
>> +		command = ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED;
>> +	else
>> +		command = ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED;
>>
>> -		return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
>> -	}
>> -	return -EOPNOTSUPP;
>> +	ret = WMID_gaming_get_sys_info(command, &fanspeed);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
>>   }
> I wonder if it's necessary to refactor this function if you are going to
> drop it on Patch 4.

I did it so each patch has a single well-defined scope. And refactoring this function was necessary for
the patch being complete.

Thanks,
Armin Wolf

>>   /*
>> @@ -1942,12 +1962,9 @@ static int acer_thermal_profile_change(void)
>>   			return err;
>>
>>   		/* Check power source */
>> -		status = WMI_gaming_execute_u64(
>> -			ACER_WMID_GET_GAMING_SYS_INFO_METHODID,
>> -			ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
>> -
>> -		if (ACPI_FAILURE(status))
>> -			return -EIO;
>> +		err = WMID_gaming_get_sys_info(ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
>> +		if (err < 0)
>> +			return err;
>>
>>   		switch (current_tp) {
>>   		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
>> --
>> 2.39.5
>>

Re: [PATCH v4 3/5] platform/x86: acer-wmi: Improve error handling when reading gaming system information

[Kurt Borja]

On Tue, Dec 10, 2024 at 05:08:01AM +0100, Armin Wolf wrote:
> Am 10.12.24 um 03:06 schrieb Kurt Borja:
> 
> > On Tue, Dec 10, 2024 at 01:16:55AM +0100, Armin Wolf wrote:
> > > If a call to ACER_WMID_GET_GAMING_SYS_INFO_METHODID fails, the lower
> > > 8 bits will be non-zero. Introduce a helper function to check this and
> > > use it when reading gaming system information.
> > > 
> > > Tested-by: Rayan Margham <rayanmargham4@gmail.com>
> > > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > > ---
> > >   drivers/platform/x86/acer-wmi.c | 55 +++++++++++++++++++++------------
> > >   1 file changed, 36 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> > > index dd57787466b9..ac4500f33b8c 100644
> > > --- a/drivers/platform/x86/acer-wmi.c
> > > +++ b/drivers/platform/x86/acer-wmi.c
> > > @@ -70,6 +70,7 @@ MODULE_LICENSE("GPL");
> > > 
> > >   #define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
> > > 
> > > +#define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
> > >   #define ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK GENMASK(20, 8)
> > > 
> > >   /*
> > > @@ -1513,6 +1514,24 @@ static acpi_status WMID_gaming_get_u64(u64 *value, u32 cap)
> > >   	return status;
> > >   }
> > > 
> > > +static int WMID_gaming_get_sys_info(u32 command, u64 *out)
> > > +{
> > > +	acpi_status status;
> > > +	u64 result;
> > > +
> > > +	status = WMI_gaming_execute_u64(ACER_WMID_GET_GAMING_SYS_INFO_METHODID, command, &result);
> > > +	if (ACPI_FAILURE(status))
> > > +		return -EIO;
> > > +
> > > +	/* The return status must be zero for the operation to have succeeded */
> > > +	if (FIELD_GET(ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK, result))
> > > +		return -EIO;
> > > +
> > > +	*out = result;
> > > +
> > > +	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*/
> > > @@ -1762,22 +1781,23 @@ static int acer_gsensor_event(void)
> > > 
> > >   static int acer_get_fan_speed(int fan)
> > >   {
> > > -	if (quirks->predator_v4) {
> > > -		acpi_status status;
> > > -		u64 fanspeed;
> > > +	u64 fanspeed;
> > > +	u32 command;
> > > +	int ret;
> > > 
> > > -		status = WMI_gaming_execute_u64(
> > > -			ACER_WMID_GET_GAMING_SYS_INFO_METHODID,
> > > -			fan == 0 ? ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED :
> > > -				   ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED,
> > > -			&fanspeed);
> > > +	if (!quirks->predator_v4)
> > > +		return -EOPNOTSUPP;
> > > 
> > > -		if (ACPI_FAILURE(status))
> > > -			return -EIO;
> > > +	if (fan == 0)
> > > +		command = ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED;
> > > +	else
> > > +		command = ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED;
> > > 
> > > -		return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
> > > -	}
> > > -	return -EOPNOTSUPP;
> > > +	ret = WMID_gaming_get_sys_info(command, &fanspeed);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
> > >   }
> > I wonder if it's necessary to refactor this function if you are going to
> > drop it on Patch 4.
> 
> I did it so each patch has a single well-defined scope. And refactoring this function was necessary for
> the patch being complete.

I'll keep that in mind when submitting.

In that case:

Reviewed-by: Kurt Borja <kuurtb@gmail.com>

I apologize If this was out of place, as I'm still learning!

~ Kurt

> 
> Thanks,
> Armin Wolf
> 
> > >   /*
> > > @@ -1942,12 +1962,9 @@ static int acer_thermal_profile_change(void)
> > >   			return err;
> > > 
> > >   		/* Check power source */
> > > -		status = WMI_gaming_execute_u64(
> > > -			ACER_WMID_GET_GAMING_SYS_INFO_METHODID,
> > > -			ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
> > > -
> > > -		if (ACPI_FAILURE(status))
> > > -			return -EIO;
> > > +		err = WMID_gaming_get_sys_info(ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS, &on_AC);
> > > +		if (err < 0)
> > > +			return err;
> > > 
> > >   		switch (current_tp) {
> > >   		case ACER_PREDATOR_V4_THERMAL_PROFILE_TURBO:
> > > --
> > > 2.39.5
> > > 

[PATCH v4 4/5] platform/x86: acer-wmi: Implement proper hwmon support

[Armin Wolf]

After looking at the ACPI AML code, it seems that the command 0x0000
used with ACER_WMID_GET_GAMING_SYS_INFO_METHODID returns a bitmap of
all supported sensor indices available through the 0x0001 command.

Those sensor indices seem to include both temperature and fan speed
sensors, with only the fan speed sensors being currently supported.

Use the output of this new command to implement reliable sensor
detection. This fixes detection of fans which do not spin during
probe, as fans are currently being ignored if their speed is 0.

Also add support for the new temperature sensor ids.

Tested-by: Rayan Margham <rayanmargham4@gmail.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 113 ++++++++++++++++++++++----------
 1 file changed, 79 insertions(+), 34 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index ac4500f33b8c..aad8eb0ddae5 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -30,6 +30,7 @@
 #include <linux/input/sparse-keymap.h>
 #include <acpi/video.h>
 #include <linux/hwmon.h>
+#include <linux/units.h>
 #include <linux/bitfield.h>

 MODULE_AUTHOR("Carlos Corbacho");
@@ -71,7 +72,9 @@ MODULE_LICENSE("GPL");
 #define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54

 #define ACER_PREDATOR_V4_RETURN_STATUS_BIT_MASK GENMASK_ULL(7, 0)
-#define ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK GENMASK(20, 8)
+#define ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK GENMASK_ULL(15, 8)
+#define ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK GENMASK_ULL(23, 8)
+#define ACER_PREDATOR_V4_SUPPORTED_SENSORS_BIT_MASK GENMASK_ULL(39, 24)

 /*
  * Acer ACPI method GUIDs
@@ -99,9 +102,17 @@ enum acer_wmi_event_ids {
 };

 enum acer_wmi_predator_v4_sys_info_command {
-	ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS = 0x02,
-	ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED = 0x0201,
-	ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED = 0x0601,
+	ACER_WMID_CMD_GET_PREDATOR_V4_SUPPORTED_SENSORS = 0x0000,
+	ACER_WMID_CMD_GET_PREDATOR_V4_SENSOR_READING	= 0x0001,
+	ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS	= 0x0002,
+};
+
+enum acer_wmi_predator_v4_sensor_id {
+	ACER_WMID_SENSOR_CPU_TEMPERATURE	= 0x01,
+	ACER_WMID_SENSOR_CPU_FAN_SPEED		= 0x02,
+	ACER_WMID_SENSOR_EXTERNAL_TEMPERATURE_2 = 0x03,
+	ACER_WMID_SENSOR_GPU_FAN_SPEED		= 0x06,
+	ACER_WMID_SENSOR_GPU_TEMPERATURE	= 0x0A,
 };

 static const struct key_entry acer_wmi_keymap[] __initconst = {
@@ -272,6 +283,7 @@ static u16 commun_func_bitmap;
 static u8 commun_fn_key_number;
 static bool cycle_gaming_thermal_profile = true;
 static bool predator_v4;
+static u64 supported_sensors;

 module_param(mailled, int, 0444);
 module_param(brightness, int, 0444);
@@ -1779,27 +1791,6 @@ static int acer_gsensor_event(void)
 	return 0;
 }

-static int acer_get_fan_speed(int fan)
-{
-	u64 fanspeed;
-	u32 command;
-	int ret;
-
-	if (!quirks->predator_v4)
-		return -EOPNOTSUPP;
-
-	if (fan == 0)
-		command = ACER_WMID_CMD_GET_PREDATOR_V4_CPU_FAN_SPEED;
-	else
-		command = ACER_WMID_CMD_GET_PREDATOR_V4_GPU_FAN_SPEED;
-
-	ret = WMID_gaming_get_sys_info(command, &fanspeed);
-	if (ret < 0)
-		return ret;
-
-	return FIELD_GET(ACER_PREDATOR_V4_FAN_SPEED_READ_BIT_MASK, fanspeed);
-}
-
 /*
  *  Predator series turbo button
  */
@@ -2688,43 +2679,86 @@ static void __init create_debugfs(void)
 			   &interface->debug.wmid_devices);
 }

+static const enum acer_wmi_predator_v4_sensor_id acer_wmi_temp_channel_to_sensor_id[] = {
+	[0] = ACER_WMID_SENSOR_CPU_TEMPERATURE,
+	[1] = ACER_WMID_SENSOR_GPU_TEMPERATURE,
+	[2] = ACER_WMID_SENSOR_EXTERNAL_TEMPERATURE_2,
+};
+
+static const enum acer_wmi_predator_v4_sensor_id acer_wmi_fan_channel_to_sensor_id[] = {
+	[0] = ACER_WMID_SENSOR_CPU_FAN_SPEED,
+	[1] = ACER_WMID_SENSOR_GPU_FAN_SPEED,
+};
+
 static umode_t acer_wmi_hwmon_is_visible(const void *data,
 					 enum hwmon_sensor_types type, u32 attr,
 					 int channel)
 {
+	enum acer_wmi_predator_v4_sensor_id sensor_id;
+	const u64 *supported_sensors = data;
+
 	switch (type) {
+	case hwmon_temp:
+		sensor_id = acer_wmi_temp_channel_to_sensor_id[channel];
+		break;
 	case hwmon_fan:
-		if (acer_get_fan_speed(channel) >= 0)
-			return 0444;
+		sensor_id = acer_wmi_fan_channel_to_sensor_id[channel];
 		break;
 	default:
 		return 0;
 	}

+	if (*supported_sensors & BIT(sensor_id - 1))
+		return 0444;
+
 	return 0;
 }

 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;
+	u64 result;
 	int ret;

 	switch (type) {
+	case hwmon_temp:
+		command |= FIELD_PREP(ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK,
+				      acer_wmi_temp_channel_to_sensor_id[channel]);
+
+		ret = WMID_gaming_get_sys_info(command, &result);
+		if (ret < 0)
+			return ret;
+
+		result = FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
+		*val = result * MILLIDEGREE_PER_DEGREE;
+		return 0;
 	case hwmon_fan:
-		ret = acer_get_fan_speed(channel);
+		command |= FIELD_PREP(ACER_PREDATOR_V4_SENSOR_INDEX_BIT_MASK,
+				      acer_wmi_fan_channel_to_sensor_id[channel]);
+
+		ret = WMID_gaming_get_sys_info(command, &result);
 		if (ret < 0)
 			return ret;
-		*val = ret;
-		break;
+
+		*val = FIELD_GET(ACER_PREDATOR_V4_SENSOR_READING_BIT_MASK, result);
+		return 0;
 	default:
 		return -EOPNOTSUPP;
 	}
-
-	return 0;
 }

 static const struct hwmon_channel_info *const acer_wmi_hwmon_info[] = {
-	HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT, HWMON_F_INPUT), NULL
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT,
+			   HWMON_T_INPUT
+			   ),
+	HWMON_CHANNEL_INFO(fan,
+			   HWMON_F_INPUT,
+			   HWMON_F_INPUT
+			   ),
+	NULL
 };

 static const struct hwmon_ops acer_wmi_hwmon_ops = {
@@ -2741,9 +2775,20 @@ static int acer_wmi_hwmon_init(void)
 {
 	struct device *dev = &acer_platform_device->dev;
 	struct device *hwmon;
+	u64 result;
+	int ret;
+
+	ret = WMID_gaming_get_sys_info(ACER_WMID_CMD_GET_PREDATOR_V4_SUPPORTED_SENSORS, &result);
+	if (ret < 0)
+		return ret;
+
+	/* Return early if no sensors are available */
+	supported_sensors = FIELD_GET(ACER_PREDATOR_V4_SUPPORTED_SENSORS_BIT_MASK, result);
+	if (!supported_sensors)
+		return 0;

 	hwmon = devm_hwmon_device_register_with_info(dev, "acer",
-						     &acer_platform_driver,
+						     &supported_sensors,
 						     &acer_wmi_hwmon_chip_info,
 						     NULL);

--
2.39.5

[PATCH v4 5/5] platform/x86: acer-wmi: Ignore AC events

[Armin Wolf]

On the Acer Swift SFG14-41, the events 8 - 1 and 8 - 0 are printed on
AC connect/disconnect. Ignore those events to avoid spamming the
kernel log with error messages.

Reported-by: Farhan Anwar <farhan.anwar8@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/2ffb529d-e7c8-4026-a3b8-120c8e7afec8@gmail.com
Tested-by: Rayan Margham <rayanmargham4@gmail.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/acer-wmi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index aad8eb0ddae5..ef26ec8d90ea 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -99,6 +99,7 @@ enum acer_wmi_event_ids {
 	WMID_HOTKEY_EVENT = 0x1,
 	WMID_ACCEL_OR_KBD_DOCK_EVENT = 0x5,
 	WMID_GAMING_TURBO_KEY_EVENT = 0x7,
+	WMID_AC_EVENT = 0x8,
 };

 enum acer_wmi_predator_v4_sys_info_command {
@@ -2304,6 +2305,9 @@ static void acer_wmi_notify(union acpi_object *obj, void *context)
 		if (return_value.key_num == 0x5 && has_cap(ACER_CAP_PLATFORM_PROFILE))
 			acer_thermal_profile_change();
 		break;
+	case WMID_AC_EVENT:
+		/* We ignore AC events here */
+		break;
 	default:
 		pr_warn("Unknown function number - %d - %d\n",
 			return_value.function, return_value.key_num);
--
2.39.5

Re: [PATCH v4 5/5] platform/x86: acer-wmi: Ignore AC events

[Kurt Borja]

On Tue, Dec 10, 2024 at 01:16:57AM +0100, Armin Wolf wrote:
> On the Acer Swift SFG14-41, the events 8 - 1 and 8 - 0 are printed on
> AC connect/disconnect. Ignore those events to avoid spamming the
> kernel log with error messages.

I noticed acer_thermal_profile_change() behavior changed whether the
laptop was on AC or not. Maybe users expect some kind of behavior around
thermal profiles with this event? Like switching to low-power when not
on AC or something like that.

~ Kurt

> 
> Reported-by: Farhan Anwar <farhan.anwar8@gmail.com>
> Closes: https://lore.kernel.org/platform-driver-x86/2ffb529d-e7c8-4026-a3b8-120c8e7afec8@gmail.com
> Tested-by: Rayan Margham <rayanmargham4@gmail.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/platform/x86/acer-wmi.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index aad8eb0ddae5..ef26ec8d90ea 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -99,6 +99,7 @@ enum acer_wmi_event_ids {
>  	WMID_HOTKEY_EVENT = 0x1,
>  	WMID_ACCEL_OR_KBD_DOCK_EVENT = 0x5,
>  	WMID_GAMING_TURBO_KEY_EVENT = 0x7,
> +	WMID_AC_EVENT = 0x8,
>  };
> 
>  enum acer_wmi_predator_v4_sys_info_command {
> @@ -2304,6 +2305,9 @@ static void acer_wmi_notify(union acpi_object *obj, void *context)
>  		if (return_value.key_num == 0x5 && has_cap(ACER_CAP_PLATFORM_PROFILE))
>  			acer_thermal_profile_change();
>  		break;
> +	case WMID_AC_EVENT:
> +		/* We ignore AC events here */
> +		break;
>  	default:
>  		pr_warn("Unknown function number - %d - %d\n",
>  			return_value.function, return_value.key_num);
> --
> 2.39.5
> 

Re: [PATCH v4 5/5] platform/x86: acer-wmi: Ignore AC events

[Armin Wolf]

Am 10.12.24 um 03:16 schrieb Kurt Borja:

> On Tue, Dec 10, 2024 at 01:16:57AM +0100, Armin Wolf wrote:
>> On the Acer Swift SFG14-41, the events 8 - 1 and 8 - 0 are printed on
>> AC connect/disconnect. Ignore those events to avoid spamming the
>> kernel log with error messages.
> I noticed acer_thermal_profile_change() behavior changed whether the
> laptop was on AC or not. Maybe users expect some kind of behavior around
> thermal profiles with this event? Like switching to low-power when not
> on AC or something like that.
>
> ~ Kurt

Good question, i will ask the person with the Acer notebook to test if the platform
profile changes automatically when switching to AC.

The other however patches can be taken regardless of this patch.

Thanks,
Armin Wolf

>> Reported-by: Farhan Anwar <farhan.anwar8@gmail.com>
>> Closes: https://lore.kernel.org/platform-driver-x86/2ffb529d-e7c8-4026-a3b8-120c8e7afec8@gmail.com
>> Tested-by: Rayan Margham <rayanmargham4@gmail.com>
>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/platform/x86/acer-wmi.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
>> index aad8eb0ddae5..ef26ec8d90ea 100644
>> --- a/drivers/platform/x86/acer-wmi.c
>> +++ b/drivers/platform/x86/acer-wmi.c
>> @@ -99,6 +99,7 @@ enum acer_wmi_event_ids {
>>   	WMID_HOTKEY_EVENT = 0x1,
>>   	WMID_ACCEL_OR_KBD_DOCK_EVENT = 0x5,
>>   	WMID_GAMING_TURBO_KEY_EVENT = 0x7,
>> +	WMID_AC_EVENT = 0x8,
>>   };
>>
>>   enum acer_wmi_predator_v4_sys_info_command {
>> @@ -2304,6 +2305,9 @@ static void acer_wmi_notify(union acpi_object *obj, void *context)
>>   		if (return_value.key_num == 0x5 && has_cap(ACER_CAP_PLATFORM_PROFILE))
>>   			acer_thermal_profile_change();
>>   		break;
>> +	case WMID_AC_EVENT:
>> +		/* We ignore AC events here */
>> +		break;
>>   	default:
>>   		pr_warn("Unknown function number - %d - %d\n",
>>   			return_value.function, return_value.key_num);
>> --
>> 2.39.5
>>

Re: [PATCH v4 5/5] platform/x86: acer-wmi: Ignore AC events

[Ilpo Järvinen]

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

On Tue, 10 Dec 2024, Armin Wolf wrote:

> Am 10.12.24 um 03:16 schrieb Kurt Borja:
> 
> > On Tue, Dec 10, 2024 at 01:16:57AM +0100, Armin Wolf wrote:
> > > On the Acer Swift SFG14-41, the events 8 - 1 and 8 - 0 are printed on
> > > AC connect/disconnect. Ignore those events to avoid spamming the
> > > kernel log with error messages.
> > I noticed acer_thermal_profile_change() behavior changed whether the
> > laptop was on AC or not. Maybe users expect some kind of behavior around
> > thermal profiles with this event? Like switching to low-power when not
> > on AC or something like that.
> > 
> > ~ Kurt
> 
> Good question, i will ask the person with the Acer notebook to test if the
> platform
> profile changes automatically when switching to AC.
> 
> The other however patches can be taken regardless of this patch.

I took patches now 1-4 but left the 5th for now.

-- 
 i.

> Thanks,
> Armin Wolf
> 
> > > Reported-by: Farhan Anwar <farhan.anwar8@gmail.com>
> > > Closes:
> > > https://lore.kernel.org/platform-driver-x86/2ffb529d-e7c8-4026-a3b8-120c8e7afec8@gmail.com
> > > Tested-by: Rayan Margham <rayanmargham4@gmail.com>
> > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > > ---
> > >   drivers/platform/x86/acer-wmi.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/platform/x86/acer-wmi.c
> > > b/drivers/platform/x86/acer-wmi.c
> > > index aad8eb0ddae5..ef26ec8d90ea 100644
> > > --- a/drivers/platform/x86/acer-wmi.c
> > > +++ b/drivers/platform/x86/acer-wmi.c
> > > @@ -99,6 +99,7 @@ enum acer_wmi_event_ids {
> > >   	WMID_HOTKEY_EVENT = 0x1,
> > >   	WMID_ACCEL_OR_KBD_DOCK_EVENT = 0x5,
> > >   	WMID_GAMING_TURBO_KEY_EVENT = 0x7,
> > > +	WMID_AC_EVENT = 0x8,
> > >   };
> > > 
> > >   enum acer_wmi_predator_v4_sys_info_command {
> > > @@ -2304,6 +2305,9 @@ static void acer_wmi_notify(union acpi_object *obj,
> > > void *context)
> > >   		if (return_value.key_num == 0x5 &&
> > > has_cap(ACER_CAP_PLATFORM_PROFILE))
> > >   			acer_thermal_profile_change();
> > >   		break;
> > > +	case WMID_AC_EVENT:
> > > +		/* We ignore AC events here */
> > > +		break;
> > >   	default:
> > >   		pr_warn("Unknown function number - %d - %d\n",
> > >   			return_value.function, return_value.key_num);
> > > --
> > > 2.39.5
> > > 
> 

Re: [PATCH v4 5/5] platform/x86: acer-wmi: Ignore AC events

[David Wang]

May I suggest pr_info() this WMID_AC_EVENT event, there were discussions
about possible unstable adapter cord causing flood of events 8-0/1. The
logs may help identify the cause.


Thanks
David

 

Re: [PATCH v4 5/5] platform/x86: acer-wmi: Ignore AC events

[Ilpo Järvinen]

On Sat, 14 Dec 2024, David Wang wrote:

> May I suggest pr_info() this WMID_AC_EVENT event, there were discussions
> about possible unstable adapter cord causing flood of events 8-0/1. The
> logs may help identify the cause.

I don't think flood of pr_info() would be desired if that's the case. 
While one could ratelimit the print out, it still seems useless noise for 
normal operation which this patch aimed to eliminate.

It sounds more like you'd want to print something if and only if a flood 
is detected (how to determine what is "flood", I don't know enough details 
of this particular case)?

In any case, if feels something that would be better put into own a patch 
with a proper justification and explanation what constitutes as "flood".

-- 
 i.