[PATCH v3] lenovo-wmi-hotkey: Fixed a kernel error report for some Lenovo non-ThinkPad devices

[PATCH v3] lenovo-wmi-hotkey: Fixed a kernel error report for some Lenovo non-ThinkPad devices

[Jackie Dong]

Not all of Lenovo non-ThinkPad devices support both mic mute LED (on F4)
and audio mute LED (on F1). Some of them only support one mute LED, some
of them don't have any mute LED. Add a decision to judge this device
support mute LED or not. Without this decision, not support both of mic
mute LED and audio mute LED Lenovo non-ThinkPad brand devices (including
Ideapad/Yoga/Xiaoxin/Gaming/ThinkBook, etc.) will report a failed message
with error -5.

Signed-off-by: Jackie Dong <xy-jackie@139.com>
Suggested-by: Hans de Goede <hansg@kernel.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

---
Changes in v3:
 - Reverse orignal logic  (obj && obj->type == ACPI_TYPE_INTEGER) 
   and add new decision for led_version == 0.
 - Optimize the descriptions based on reviewer comments.

Changes in v2:
 - Add warning message and then return 0 if the device support mute LED
   abnormaly, based on Hans suggestion and Armin previous patch.


 .../x86/lenovo-wmi-hotkey-utilities.c         | 24 ++++++++++++++-----
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
index 89153afd7015..1850992f2ea8 100644
--- a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
+++ b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
@@ -127,21 +127,30 @@ static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct
 	else
 		return -EIO;
 
-	wpriv->cdev[led_type].max_brightness = LED_ON;
-	wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME;
+	/*
+	 * Output parameters define: 0 means mute LED is not supported, Non-zero means
+	 * mute LED can be supported.
+	 */
+	if (led_version == 0)
+		return 0;
+
 
 	switch (led_type) {
 	case MIC_MUTE:
-		if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER)
-			return -EIO;
+		if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) {
+			pr_warn("The MIC_MUTE LED of this device isn't supported now.\n");
+			return 0;
+		}
 
 		wpriv->cdev[led_type].name = "platform::micmute";
 		wpriv->cdev[led_type].brightness_set_blocking = &lsh_wmi_micmute_led_set;
 		wpriv->cdev[led_type].default_trigger = "audio-micmute";
 		break;
 	case AUDIO_MUTE:
-		if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER)
-			return -EIO;
+		if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) {
+			pr_warn("The AUDIO_MUTE LED of this device isn't supported now.\n");
+			return 0;
+		}
 
 		wpriv->cdev[led_type].name = "platform::mute";
 		wpriv->cdev[led_type].brightness_set_blocking = &lsh_wmi_audiomute_led_set;
@@ -152,6 +161,9 @@ static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct
 		return -EINVAL;
 	}
 
+	wpriv->cdev[led_type].max_brightness = LED_ON;
+	wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME;
+
 	err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]);
 	if (err < 0) {
 		dev_err(dev, "Could not register mute LED %d : %d\n", led_type, err);
-- 
2.43.0

Re: [PATCH v3] lenovo-wmi-hotkey: Fixed a kernel error report for some Lenovo non-ThinkPad devices

[Ilpo Järvinen]

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

On Tue, 8 Jul 2025, Jackie Dong wrote:

> Not all of Lenovo non-ThinkPad devices support both mic mute LED (on F4)
> and audio mute LED (on F1). Some of them only support one mute LED, some
> of them don't have any mute LED. Add a decision to judge this device
> support mute LED or not. Without this decision, not support both of mic
> mute LED and audio mute LED Lenovo non-ThinkPad brand devices (including
> Ideapad/Yoga/Xiaoxin/Gaming/ThinkBook, etc.) will report a failed message
> with error -5.
> 
> Signed-off-by: Jackie Dong <xy-jackie@139.com>
> Suggested-by: Hans de Goede <hansg@kernel.org>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Hi Jackie,

Please don't add Reviewed-by nor Tested-by tags unless they were 
explicitly given to you.
 
> ---
> Changes in v3:
>  - Reverse orignal logic  (obj && obj->type == ACPI_TYPE_INTEGER) 
>    and add new decision for led_version == 0.
>  - Optimize the descriptions based on reviewer comments.
> 
> Changes in v2:
>  - Add warning message and then return 0 if the device support mute LED
>    abnormaly, based on Hans suggestion and Armin previous patch.
> 
> 
>  .../x86/lenovo-wmi-hotkey-utilities.c         | 24 ++++++++++++++-----
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
> index 89153afd7015..1850992f2ea8 100644
> --- a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
> +++ b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
> @@ -127,21 +127,30 @@ static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct
>  	else
>  		return -EIO;

The logic was not reversed as requested. Please change this code to:

	union acpi_object *obj __free(kfree) = output.pointer;
	if (!obj || obj->type != ACPI_TYPE_INTEGER)
		return -EIO;

	led_version = obj->integer.value;

> -	wpriv->cdev[led_type].max_brightness = LED_ON;
> -	wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME;
> +	/*
> +	 * Output parameters define: 0 means mute LED is not supported, Non-zero means
> +	 * mute LED can be supported.
> +	 */
> +	if (led_version == 0)
> +		return 0;
> +
>  
>  	switch (led_type) {
>  	case MIC_MUTE:
> -		if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER)
> -			return -EIO;
> +		if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) {
> +			pr_warn("The MIC_MUTE LED of this device isn't supported now.\n");

Drop "now" (or change it to more precise explanation why).

> +			return 0;
> +		}
>  
>  		wpriv->cdev[led_type].name = "platform::micmute";
>  		wpriv->cdev[led_type].brightness_set_blocking = &lsh_wmi_micmute_led_set;
>  		wpriv->cdev[led_type].default_trigger = "audio-micmute";
>  		break;
>  	case AUDIO_MUTE:
> -		if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER)
> -			return -EIO;
> +		if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) {
> +			pr_warn("The AUDIO_MUTE LED of this device isn't supported now.\n");

Ditto.

> +			return 0;
> +		}
>  
>  		wpriv->cdev[led_type].name = "platform::mute";
>  		wpriv->cdev[led_type].brightness_set_blocking = &lsh_wmi_audiomute_led_set;
> @@ -152,6 +161,9 @@ static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct
>  		return -EINVAL;
>  	}
>  
> +	wpriv->cdev[led_type].max_brightness = LED_ON;
> +	wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME;
> +
>  	err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]);
>  	if (err < 0) {
>  		dev_err(dev, "Could not register mute LED %d : %d\n", led_type, err);
> 

-- 
 i.

Re: [PATCH v3] lenovo-wmi-hotkey: Fixed a kernel error report for some Lenovo non-ThinkPad devices

[Ilpo Järvinen]

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

In addition, the shortlog should mention the error relates to not 
supported mute LEDs.

I'd also drop "some Lenovo" from to make it shorter, I think the 
explanation that is already in the changelog covers that (we cannot 
put everything to the shortlog to keep it relatively short).

-- 
 i.

On Tue, 8 Jul 2025, Ilpo Järvinen wrote:

> On Tue, 8 Jul 2025, Jackie Dong wrote:
> 
> > Not all of Lenovo non-ThinkPad devices support both mic mute LED (on F4)
> > and audio mute LED (on F1). Some of them only support one mute LED, some
> > of them don't have any mute LED. Add a decision to judge this device
> > support mute LED or not. Without this decision, not support both of mic
> > mute LED and audio mute LED Lenovo non-ThinkPad brand devices (including
> > Ideapad/Yoga/Xiaoxin/Gaming/ThinkBook, etc.) will report a failed message
> > with error -5.
> > 
> > Signed-off-by: Jackie Dong <xy-jackie@139.com>
> > Suggested-by: Hans de Goede <hansg@kernel.org>
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> Hi Jackie,
> 
> Please don't add Reviewed-by nor Tested-by tags unless they were 
> explicitly given to you.
>  
> > ---
> > Changes in v3:
> >  - Reverse orignal logic  (obj && obj->type == ACPI_TYPE_INTEGER) 
> >    and add new decision for led_version == 0.
> >  - Optimize the descriptions based on reviewer comments.
> > 
> > Changes in v2:
> >  - Add warning message and then return 0 if the device support mute LED
> >    abnormaly, based on Hans suggestion and Armin previous patch.
> > 
> > 
> >  .../x86/lenovo-wmi-hotkey-utilities.c         | 24 ++++++++++++++-----
> >  1 file changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
> > index 89153afd7015..1850992f2ea8 100644
> > --- a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
> > +++ b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
> > @@ -127,21 +127,30 @@ static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct
> >  	else
> >  		return -EIO;
> 
> The logic was not reversed as requested. Please change this code to:
> 
> 	union acpi_object *obj __free(kfree) = output.pointer;
> 	if (!obj || obj->type != ACPI_TYPE_INTEGER)
> 		return -EIO;
> 
> 	led_version = obj->integer.value;
> 
> > -	wpriv->cdev[led_type].max_brightness = LED_ON;
> > -	wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME;
> > +	/*
> > +	 * Output parameters define: 0 means mute LED is not supported, Non-zero means
> > +	 * mute LED can be supported.
> > +	 */
> > +	if (led_version == 0)
> > +		return 0;
> > +
> >  
> >  	switch (led_type) {
> >  	case MIC_MUTE:
> > -		if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER)
> > -			return -EIO;
> > +		if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) {
> > +			pr_warn("The MIC_MUTE LED of this device isn't supported now.\n");
> 
> Drop "now" (or change it to more precise explanation why).
> 
> > +			return 0;
> > +		}
> >  
> >  		wpriv->cdev[led_type].name = "platform::micmute";
> >  		wpriv->cdev[led_type].brightness_set_blocking = &lsh_wmi_micmute_led_set;
> >  		wpriv->cdev[led_type].default_trigger = "audio-micmute";
> >  		break;
> >  	case AUDIO_MUTE:
> > -		if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER)
> > -			return -EIO;
> > +		if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) {
> > +			pr_warn("The AUDIO_MUTE LED of this device isn't supported now.\n");
> 
> Ditto.
> 
> > +			return 0;
> > +		}
> >  
> >  		wpriv->cdev[led_type].name = "platform::mute";
> >  		wpriv->cdev[led_type].brightness_set_blocking = &lsh_wmi_audiomute_led_set;
> > @@ -152,6 +161,9 @@ static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct
> >  		return -EINVAL;
> >  	}
> >  
> > +	wpriv->cdev[led_type].max_brightness = LED_ON;
> > +	wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME;
> > +
> >  	err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]);
> >  	if (err < 0) {
> >  		dev_err(dev, "Could not register mute LED %d : %d\n", led_type, err);
> > 
> 
> 

Re: [PATCH v3] lenovo-wmi-hotkey: Fixed a kernel error report forsome Lenovo non-ThinkPad devices

[Jackie Dong]

On 7/8/25 18:07, Ilpo Järvinen wrote:
> On Tue, 8 Jul 2025, Jackie Dong wrote:
> 
>> Not all of Lenovo non-ThinkPad devices support both mic mute LED (on F4)
>> and audio mute LED (on F1). Some of them only support one mute LED, some
>> of them don't have any mute LED. Add a decision to judge this device
>> support mute LED or not. Without this decision, not support both of mic
>> mute LED and audio mute LED Lenovo non-ThinkPad brand devices (including
>> Ideapad/Yoga/Xiaoxin/Gaming/ThinkBook, etc.) will report a failed message
>> with error -5.
>>
>> Signed-off-by: Jackie Dong <xy-jackie@139.com>
>> Suggested-by: Hans de Goede <hansg@kernel.org>
>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> Hi Jackie,
> 
> Please don't add Reviewed-by nor Tested-by tags unless they were
> explicitly given to you.
>   
Hi Ilpo,
    I have deleted the Reviewed-by tag in next version patch.
>> ---
>> Changes in v3:
>>   - Reverse orignal logic  (obj && obj->type == ACPI_TYPE_INTEGER)
>>     and add new decision for led_version == 0.
>>   - Optimize the descriptions based on reviewer comments.
>>
>> Changes in v2:
>>   - Add warning message and then return 0 if the device support mute LED
>>     abnormaly, based on Hans suggestion and Armin previous patch.
>>
>>
>>   .../x86/lenovo-wmi-hotkey-utilities.c         | 24 ++++++++++++++-----
>>   1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
>> index 89153afd7015..1850992f2ea8 100644
>> --- a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
>> +++ b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
>> @@ -127,21 +127,30 @@ static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct
>>   	else
>>   		return -EIO;
> 
> The logic was not reversed as requested. Please change this code to:
> 
> 	union acpi_object *obj __free(kfree) = output.pointer;
> 	if (!obj || obj->type != ACPI_TYPE_INTEGER)
> 		return -EIO;
> 
> 	led_version = obj->integer.value;
> 
It's good suggestion, I'll update it.
>> -	wpriv->cdev[led_type].max_brightness = LED_ON;
>> -	wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME;
>> +	/*
>> +	 * Output parameters define: 0 means mute LED is not supported, Non-zero means
>> +	 * mute LED can be supported.
>> +	 */
>> +	if (led_version == 0)
>> +		return 0;
>> +
>>   
>>   	switch (led_type) {
>>   	case MIC_MUTE:
>> -		if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER)
>> -			return -EIO;
>> +		if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) {
>> +			pr_warn("The MIC_MUTE LED of this device isn't supported now.\n");
> 
> Drop "now" (or change it to more precise explanation why).
> 
I have deleted the "now" in next version patch.
>> +			return 0;
>> +		}
>>   
>>   		wpriv->cdev[led_type].name = "platform::micmute";
>>   		wpriv->cdev[led_type].brightness_set_blocking = &lsh_wmi_micmute_led_set;
>>   		wpriv->cdev[led_type].default_trigger = "audio-micmute";
>>   		break;
>>   	case AUDIO_MUTE:
>> -		if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER)
>> -			return -EIO;
>> +		if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) {
>> +			pr_warn("The AUDIO_MUTE LED of this device isn't supported now.\n");
> 
> Ditto.
> 
I have deleted the "now" in next version patch.
>> +			return 0;
>> +		}
>>   
>>   		wpriv->cdev[led_type].name = "platform::mute";
>>   		wpriv->cdev[led_type].brightness_set_blocking = &lsh_wmi_audiomute_led_set;
>> @@ -152,6 +161,9 @@ static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct
>>   		return -EINVAL;
>>   	}
>>   
>> +	wpriv->cdev[led_type].max_brightness = LED_ON;
>> +	wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME;
>> +
>>   	err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]);
>>   	if (err < 0) {
>>   		dev_err(dev, "Could not register mute LED %d : %d\n", led_type, err);
>>
> 
Thanks a lot,
Jackie