Re: [PATCH v2] lenovo-wmi-hotkey: Fixed a kernel error report for

From: Ilpo Järvinen
Date: Tue Jul 08 2025 - 04:09:15 EST


On Tue, 8 Jul 2025, Jackie Dong wrote:

Hi,

Shortlog in Subject seems incomplete

> 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

Add spaces after (

> 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@xxxxxxx>
> Suggested-by: Hans de Goede <hansg@xxxxxxxxxx>
>
> ---
> 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 | 30 +++++++++++++------
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
> index 89153afd7015..334c12f2896d 100644
> --- a/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
> +++ b/drivers/platform/x86/lenovo-wmi-hotkey-utilities.c
> @@ -122,26 +122,35 @@ static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct
> return -EIO;
>
> union acpi_object *obj __free(kfree) = output.pointer;
> - if (obj && obj->type == ACPI_TYPE_INTEGER)
> + if (obj && obj->type == ACPI_TYPE_INTEGER) {

Could you please reverse this logic and handle errors first.

> led_version = obj->integer.value;
> - 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;
> + } else {
> + return -EIO;
> + }
>
> 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("This device MIC_MUTE LED doesn't support 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("This device AUDIO_MUTE LED doesn't support now.\n");

Both of these warnings have the same grammar flaws and need to be
rephrased.

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