[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <277b444e-5818-47ae-bf92-f31c25538572@139.com>
Date: Tue, 8 Jul 2025 17:41:57 +0800
From: Jackie Dong <xy-jackie@....com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: hansg@...nel.org, platform-driver-x86@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>, dongeg1@...ovo.com
Subject: Re: [PATCH v2] lenovo-wmi-hotkey: Fixed a kernel error report for
Hi Ilpo,
Thanks for your comments, I'll update it later.
Jackie Dong
On 7/8/25 16:07, Ilpo Järvinen wrote:
> 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@....com>
>> Suggested-by: Hans de Goede <hansg@...nel.org>
>>
>> ---
>> 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);
>>
>
Powered by blists - more mailing lists