| 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: <6f62f3af-f697-4737-881b-38f60f373fab@gmx.de>
Date: Tue, 24 Dec 2024 14:09:29 +0100
From: Armin Wolf <W_Armin@....de>
To: Jackie EG1 Dong <dongeg1@...ovo.com>, Jackie Dong <xy-jackie@....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: Support
for mic and audio leds.
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.
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 :).
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