| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <241a9d1e-b62c-4442-9e9e-c289a3590f1d@139.com>
Date: Wed, 25 Dec 2024 13:03:33 +0800
From: Jackie Dong <xy-jackie@....com>
To: Armin Wolf <W_Armin@....de>, Jackie EG1 Dong <dongeg1@...ovo.com>,
"ike.pan@...onical.com" <ike.pan@...onical.com>,
"hdegoede@...hat.com" <hdegoede@...hat.com>,
"ilpo.jarvinen@...ux.intel.com" <ilpo.jarvinen@...ux.intel.com>,
"perex@...ex.cz" <perex@...ex.cz>, "tiwai@...e.com" <tiwai@...e.com>,
"bo.liu@...arytech.com" <bo.liu@...arytech.com>,
"kovalev@...linux.org" <kovalev@...linux.org>,
"me@...herl.one" <me@...herl.one>,
"jaroslaw.janik@...il.com" <jaroslaw.janik@...il.com>,
"cs@...edo.de" <cs@...edo.de>,
"songxiebing@...inos.cn" <songxiebing@...inos.cn>,
"kailang@...ltek.com" <kailang@...ltek.com>,
"sbinding@...nsource.cirrus.com" <sbinding@...nsource.cirrus.com>,
"simont@...nsource.cirrus.com" <simont@...nsource.cirrus.com>,
"josh@...huagrisham.com" <josh@...huagrisham.com>,
"rf@...nsource.cirrus.com" <rf@...nsource.cirrus.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
"mpearson-lenovo@...ebb.ca" <mpearson-lenovo@...ebb.ca>,
"waterflowdeg@...il.com" <waterflowdeg@...il.com>
Subject: Re: [External] Re: [PATCH 1/2] platform/x86: ideapad-laptop:
Supportfor mic and audio leds.
On 2024/12/24 21:09, Armin Wolf wrote:
> Am 24.12.24 um 10:29 schrieb Jackie EG1 Dong:
>
>> Dear Armin,
>> CE6C0974-0407-4F50-88BA-4FC3B6559AD8 is a WMI interface method.
>> From Lenovo keyboard WMI specification, I found it applicable to
>> LNB products, i.e. YOGA/XiaoXin/Gaming/ThinkBook etc and performs the
>> Lenovo customized hotkeys function for Consumer and SMB notebooks.
>> I implemented the audio mute LED and mic mute LED function of
>> IdeaPad notebook in ideapad_laptop driver, because I found the mute
>> LED function of Thinkpad notebook is implemented by thinkpad_acpi. And
>> ideapad_laptop driver has the similar function as thinkpad_acpi.
>>
>> Thanks,
>> Jackie Dong
>
> Please do not top-post, see https://subspace.kernel.org/etiquette.html
> for details.
Thanks Armin for your link, I have read and follow it in future.
>
> If the CE6C0974-0407-4F50-88BA-4FC3B6559AD8 is a WMI method device, when
> setting it up as a WMI event device is invalid. I will see if i can
> modify the WMI driver core to prevent
> this from happening in the future.
>
> Regarding the ideapad_laptop and thinkpad_acpi drivers: those drivers
> are using the legacy GUID-based WMI API, so they tend to handle multiple
> WMI GUIDs at the same time.
> This style of writing WMI drivers is deprecated, as it is prone to
> lifetime issues.
>
> I suggest you write a separate WMI driver for the
> CE6C0974-0407-4F50-88BA-4FC3B6559AD8 WMI device which just takes care of
> the LED devices. The documentation for writing WMI drivers
> also specifies a example driver which might be useful as a starting point.
>
> Looking forward for the second revision of the patch series :).I'll write a separate WMI driver for this function if I don't get any
other maintainers comment. Maybe it spend more time to finish it.
Many thanks,
Jackie Dong
>
> Thanks,
> Armin Wolf
>
>> -----Original Message-----
>> From: Armin Wolf <W_Armin@....de>
>> Sent: Monday, December 23, 2024 6:34 AM
>> To: Jackie Dong <xy-jackie@....com>; ike.pan@...onical.com;
>> hdegoede@...hat.com; ilpo.jarvinen@...ux.intel.com; perex@...ex.cz;
>> tiwai@...e.com; bo.liu@...arytech.com; kovalev@...linux.org;
>> me@...herl.one; jaroslaw.janik@...il.com; cs@...edo.de;
>> songxiebing@...inos.cn; kailang@...ltek.com;
>> sbinding@...nsource.cirrus.com; simont@...nsource.cirrus.com;
>> josh@...huagrisham.com; rf@...nsource.cirrus.com
>> Cc: linux-kernel@...r.kernel.org; platform-driver-x86@...r.kernel.org;
>> linux-sound@...r.kernel.org; mpearson-lenovo@...ebb.ca;
>> waterflowdeg@...il.com; Jackie EG1 Dong <dongeg1@...ovo.com>
>> Subject: [External] Re: [PATCH 1/2] platform/x86: ideapad-laptop:
>> Support for mic and audio leds.
>>
>> Am 19.12.24 um 11:15 schrieb Jackie Dong:
>>
>>> Implement Lenovo utility data WMI calls needed to make LEDs work on
>>> Ideapads that support this GUID.
>>> This enables the mic and audio LEDs to be updated correctly.
>>>
>>> Tested on below samples.
>>> ThinkBook 13X Gen4 IMH
>>> ThinkBook 14 G6 ABP
>>> ThinkBook 16p Gen4-21J8
>>> ThinkBook 16p Gen8-IRL
>>> ThinkBook 16p G7+ ASP
>> Hi,
>>
>> i am a bit confused regarding the role of the
>> CE6C0974-0407-4F50-88BA-4FC3B6559AD8 WMI device:
>>
>> - is it a event or a method block?
>>
>> - is it in some way connected with the remaining WMI devices supported
>> by the ideapad-laptop driver?
>>
>> Looking at the code it seems to me that the
>> CE6C0974-0407-4F50-88BA-4FC3B6559AD8 WMI device is not a event device
>> and is not directly connected with the remaining WMI devices (please
>> correct me if i am wrong).
>>
>> Can you please write a separate driver for this WMI device? Getting
>> the ideapad-laptop driver involved in this seems to be unreasonable
>> since the audio led functionality does not interact with the remaining
>> driver.
>>
>> This might be helpful:
>> https://docs.kernel.org/wmi/driver-development-guide.html.
>>
>>> Suggested-by: Mark Pearson <mpearson-lenovo@...ebb.ca>
>>> Signed-off-by: Jackie Dong <xy-jackie@....com>
>>> Signed-off-by: Jackie Dong <dongeg1@...ovo.com>
>> Please keep only the Signed-of tag with the email address used for
>> sending this patch.
>>
>> Besides that its always nice to see vendors getting involved with
>> upstream :).
>>
>> Thanks,
>> Armin Wolf
>>
>>> ---
>>> drivers/platform/x86/ideapad-laptop.c | 157
>>> +++++++++++++++++++++++++-
>>> 1 file changed, 156 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/platform/x86/ideapad-laptop.c
>>> b/drivers/platform/x86/ideapad-laptop.c
>>> index c64dfc56651d..acea4aa8eac3 100644
>>> --- a/drivers/platform/x86/ideapad-laptop.c
>>> +++ b/drivers/platform/x86/ideapad-laptop.c
>>> @@ -32,6 +32,7 @@
>>> #include <linux/sysfs.h>
>>> #include <linux/types.h>
>>> #include <linux/wmi.h>
>>> +#include <sound/control.h>
>>> #include "ideapad-laptop.h"
>>>
>>> #include <acpi/video.h>
>>> @@ -1298,6 +1299,15 @@ static const struct key_entry ideapad_keymap[]
>>> = {
>>> { KE_END },
>>> };
>>>
>>> +/*
>>> + * Input parameters to mute/unmute audio LED and Mic LED */ struct
>>> +wmi_led_args {
>>> + u8 ID;
>>> + u8 SubID;
>>> + u16 Value;
>>> +};
>>> +
>>> static int ideapad_input_init(struct ideapad_private *priv)
>>> {
>>> struct input_dev *inputdev;
>>> @@ -2023,15 +2033,145 @@ static void ideapad_check_features(struct
>>> ideapad_private *priv)
>>> /*
>>> * WMI driver
>>> */
>>> +#define IDEAPAD_ACPI_LED_MAX (((SNDRV_CTL_ELEM_ACCESS_MIC_LED -\
>>> + SNDRV_CTL_ELEM_ACCESS_SPK_LED) >>
>>> SNDRV_CTL_ELEM_ACCESS_LED_SHIFT)
>>> ++ 1)
>>> +
>>> enum ideapad_wmi_event_type {
>>> IDEAPAD_WMI_EVENT_ESC,
>>> IDEAPAD_WMI_EVENT_FN_KEYS,
>>> + IDEAPAD_WMI_EVENT_LUD_KEYS,
>>> };
>>>
>>> +#define WMI_LUD_GET_SUPPORT 1
>>> +#define WMI_LUD_SET_FEATURE 2
>>> +
>>> +#define WMI_LUD_GET_MICMUTE_LED_VER 20
>>> +#define WMI_LUD_GET_AUDIOMUTE_LED_VER 26
>>> +
>>> +#define WMI_LUD_SUPPORT_MICMUTE_LED_VER 25
>>> +#define WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER 27
>>> +
>>> struct ideapad_wmi_private {
>>> enum ideapad_wmi_event_type event;
>>> + struct led_classdev cdev[IDEAPAD_ACPI_LED_MAX];
>>> };
>>>
>>> +static struct wmi_device *led_wdev;
>>> +
>>> +enum mute_led_type {
>>> + MIC_MUTE,
>>> + AUDIO_MUTE,
>>> +};
>>> +
>>> +static int ideapad_wmi_mute_led_set(enum mute_led_type led_type,
>>> struct led_classdev *led_cdev,
>>> + enum led_brightness brightness)
>>> +
>>> +{
>>> + struct wmi_led_args led_arg = {0, 0, 0};
>>> + struct acpi_buffer input;
>>> + acpi_status status;
>>> +
>>> + if (led_type == MIC_MUTE)
>>> + led_arg.ID = brightness == LED_ON ? 1 : 2;
>>> + else if (led_type == AUDIO_MUTE)
>>> + led_arg.ID = brightness == LED_ON ? 4 : 5;
>>> + else
>>> + return -EINVAL;
>>> +
>>> + input.length = sizeof(struct wmi_led_args);
>>> + input.pointer = &led_arg;
>>> + status = wmidev_evaluate_method(led_wdev, 0, WMI_LUD_SET_FEATURE,
>>> +&input, NULL);
>>> +
>>> + if (ACPI_FAILURE(status))
>>> + return -EIO;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int ideapad_wmi_audiomute_led_set(struct led_classdev *led_cdev,
>>> + enum led_brightness brightness)
>>> +
>>> +{
>>> + return ideapad_wmi_mute_led_set(AUDIO_MUTE, led_cdev,
>>> brightness); }
>>> +
>>> +static int ideapad_wmi_micmute_led_set(struct led_classdev *led_cdev,
>>> + enum led_brightness brightness) {
>>> + return ideapad_wmi_mute_led_set(MIC_MUTE, led_cdev, brightness); }
>>> +
>>> +static int ideapad_wmi_leds_init(enum mute_led_type led_type, struct
>>> +device *dev) {
>>> + struct ideapad_wmi_private *wpriv = dev_get_drvdata(dev);
>>> + struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>> + struct acpi_buffer input;
>>> + union acpi_object *obj;
>>> + int led_version, err = 0;
>>> + unsigned int wmiarg;
>>> + acpi_status status;
>>> +
>>> + if (led_type == MIC_MUTE)
>>> + wmiarg = WMI_LUD_GET_MICMUTE_LED_VER;
>>> + else if (led_type == AUDIO_MUTE)
>>> + wmiarg = WMI_LUD_GET_AUDIOMUTE_LED_VER;
>>> + else
>>> + return -EINVAL;
>>> +
>>> + input.length = sizeof(wmiarg);
>>> + input.pointer = &wmiarg;
>>> + status = wmidev_evaluate_method(led_wdev, 0,
>>> WMI_LUD_GET_SUPPORT, &input, &output);
>>> + if (ACPI_FAILURE(status)) {
>>> + kfree(output.pointer);
>>> + return -EIO;
>>> + }
>>> + obj = output.pointer;
>>> + led_version = obj->integer.value;
>>> +
>>> + wpriv->cdev[led_type].max_brightness = LED_ON;
>>> + wpriv->cdev[led_type].dev = dev;
>>> + wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME;
>>> +
>>> + if (led_type == MIC_MUTE) {
>>> + if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) {
>>> + dev_info(dev, "This product doesn't support mic
>>> mute LED.\n");
>>> + return -EIO;
>>> + }
>>> + wpriv->cdev[led_type].name = "platform::micmute";
>>> + wpriv->cdev[led_type].brightness_set_blocking =
>>> &ideapad_wmi_micmute_led_set;
>>> + wpriv->cdev[led_type].default_trigger = "audio-micmute";
>>> +
>>> + err = devm_led_classdev_register(dev,
>>> &wpriv->cdev[led_type]);
>>> + if (err < 0) {
>>> + dev_err(dev, "Could not register mic mute LED :
>>> %d\n", err);
>>> + led_classdev_unregister(&wpriv->cdev[led_type]);
>>> + }
>>> + } else {
>>> + if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) {
>>> + dev_info(dev, "This product doesn't support
>>> audio mute LED.\n");
>>> + return -EIO;
>>> + }
>>> + wpriv->cdev[led_type].name = "platform::mute";
>>> + wpriv->cdev[led_type].brightness_set_blocking =
>>> &ideapad_wmi_audiomute_led_set;
>>> + wpriv->cdev[led_type].default_trigger = "audio-mute";
>>> +
>>> + err = devm_led_classdev_register(dev,
>>> &wpriv->cdev[led_type]);
>>> + if (err < 0) {
>>> + dev_err(dev, "Could not register audio mute
>>> LED: %d\n", err);
>>> + led_classdev_unregister(&wpriv->cdev[led_type]);
>>> + }
>>> + }
>>> +
>>> + kfree(obj);
>>> + return err;
>>> +}
>>> +
>>> +static void ideapad_wmi_leds_setup(struct device *dev) {
>>> + ideapad_wmi_leds_init(MIC_MUTE, dev);
>>> + ideapad_wmi_leds_init(AUDIO_MUTE, dev); }
>>> +
>>> static int ideapad_wmi_probe(struct wmi_device *wdev, const void
>>> *context)
>>> {
>>> struct ideapad_wmi_private *wpriv;
>>> @@ -2043,6 +2183,12 @@ static int ideapad_wmi_probe(struct wmi_device
>>> *wdev, const void *context)
>>> *wpriv = *(const struct ideapad_wmi_private *)context;
>>>
>>> dev_set_drvdata(&wdev->dev, wpriv);
>>> +
>>> + if (wpriv->event == IDEAPAD_WMI_EVENT_LUD_KEYS) {
>>> + led_wdev = wdev;
>>> + ideapad_wmi_leds_setup(&wdev->dev);
>>> + }
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -2088,6 +2234,9 @@ static void ideapad_wmi_notify(struct
>>> wmi_device *wdev, union acpi_object *data)
>>> data->integer.value |
>>> IDEAPAD_WMI_KEY);
>>>
>>> break;
>>> + case IDEAPAD_WMI_EVENT_LUD_KEYS:
>>> + break;
>>> +
>>> }
>>> }
>>>
>>> @@ -2099,10 +2248,16 @@ static const struct ideapad_wmi_private
>>> ideapad_wmi_context_fn_keys = {
>>> .event = IDEAPAD_WMI_EVENT_FN_KEYS
>>> };
>>>
>>> +static const struct ideapad_wmi_private ideapad_wmi_context_LUD_keys
>>> = {
>>> + .event = IDEAPAD_WMI_EVENT_LUD_KEYS
>>> +};
>>> +
>>> static const struct wmi_device_id ideapad_wmi_ids[] = {
>>> { "26CAB2E5-5CF1-46AE-AAC3-4A12B6BA50E6",
>>> &ideapad_wmi_context_esc }, /* Yoga 3 */
>>> { "56322276-8493-4CE8-A783-98C991274F5E",
>>> &ideapad_wmi_context_esc }, /* Yoga 700 */
>>> - { "8FC0DE0C-B4E4-43FD-B0F3-8871711C1294",
>>> &ideapad_wmi_context_fn_keys }, /* Legion 5 */
>>> + { "8FC0DE0C-B4E4-43FD-B0F3-8871711C1294",
>>> &ideapad_wmi_context_fn_keys }, /* FN keys */
>>> + { "CE6C0974-0407-4F50-88BA-4FC3B6559AD8",
>>> +&ideapad_wmi_context_LUD_keys }, /* Util data */
>>> +
>>> {},
>>> };
>>> MODULE_DEVICE_TABLE(wmi, ideapad_wmi_ids);
Powered by blists - more mailing lists