| Message ID | 20250208144114.11685-1-xy-jackie@139.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v3] platform/x86: lenovo-super-hotkey-wmi.c: Support for mic and audio mute LEDs | expand |
Hi Jackie, I noticed a few things while going through your patch: On Sat Feb 8, 2025 at 9:41 AM -05, Jackie Dong wrote: > 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 > > Signed-off-by: Jackie Dong <xy-jackie@139.com> > Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca> > --- > Changes in v3: > - Changed the name of the Kconfig entry to LENOVO_SUPER_HOTKEY_WMI > - Renamed everything in this driver which contains the name "ideapad" > to instead contain the name of this driver. > - Moved struct wmi_device *led_wdev in lenovo_super_hotkey_wmi_private, > and use container_of() to the led_wdev pointer. > - Replaced sizeof(struct wmi_led_args) by sizeof(led_arg) > - Added condtions checking for obj && obj->type == ACPI_TYPE_INTEGER > and free the ACPI object after get the required value. > - Removed led_classdev_unregister() after led_reg_failed label, but > add lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) to free > resource. > - Removed IDEAPAD_WMI_EVENT_FN_KEYS/IDEAPAD_WMI_EVENT_LUD_KEYS related > source codes and only keep LUD_WMI_METHOD_GUID. > > Changes in v2: > - Update code layout and formatting as recommended in review > - Improved error handling in ideapad_wmi_led_init > - Separated a WMI driver named lenovo-super-hotkey-wmi.c from > ideapad-lap.c, it's only for Lenovo Super Hotkey WMI devices. > > drivers/platform/x86/Kconfig | 9 + > drivers/platform/x86/Makefile | 1 + > .../platform/x86/lenovo-super-hotkey-wmi.c | 248 ++++++++++++++++++ > 3 files changed, 258 insertions(+) > create mode 100644 drivers/platform/x86/lenovo-super-hotkey-wmi.c > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 0258dd879d64..c1792e8f04e1 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -475,6 +475,15 @@ config IDEAPAD_LAPTOP > This is a driver for Lenovo IdeaPad netbooks contains drivers for > rfkill switch, hotkey, fan control and backlight control. > > +config LENOVO_SUPER_HOTKEY_WMI > + tristate "Lenovo Super Hotkey Utility WMI extras driver" > + depends on ACPI_WMI > + depends on IDEAPAD_LAPTOP > + help > + This driver provides WMI support for Lenovo customized hotkeys function > + of Lenovo NoteBooks which are for Consumer and SMB customers, such as > + Ideapad/YOGA/XiaoXin/Gaming/ThinkBook and so on. > + > config LENOVO_YMC > tristate "Lenovo Yoga Tablet Mode Control" > depends on ACPI_WMI > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index e1b142947067..598335da9f55 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -61,6 +61,7 @@ obj-$(CONFIG_UV_SYSFS) += uv_sysfs.o > # IBM Thinkpad and Lenovo > obj-$(CONFIG_IBM_RTL) += ibm_rtl.o > obj-$(CONFIG_IDEAPAD_LAPTOP) += ideapad-laptop.o > +obj-$(CONFIG_LENOVO_SUPER_HOTKEY_WMI) += lenovo-super-hotkey-wmi.o > obj-$(CONFIG_LENOVO_YMC) += lenovo-ymc.o > obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o > obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o > diff --git a/drivers/platform/x86/lenovo-super-hotkey-wmi.c b/drivers/platform/x86/lenovo-super-hotkey-wmi.c > new file mode 100644 > index 000000000000..e677bd11fa2f > --- /dev/null > +++ b/drivers/platform/x86/lenovo-super-hotkey-wmi.c > @@ -0,0 +1,248 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop > + * > + * Copyright (C) 2025 Lenovo > + */ > + > +#include <linux/leds.h> > +#include <linux/wmi.h> > +#include "ideapad-laptop.h" > + > +/* Lenovo Super Hotkey WMI GUIDs */ > +#define LUD_WMI_METHOD_GUID "CE6C0974-0407-4F50-88BA-4FC3B6559AD8" > + > +/* Lenovo Utility Data WMI method_id */ > +#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 > + > +/* Input parameters to mute/unmute audio LED and Mic LED */ > +struct wmi_led_args { > + u8 id; > + u8 subid; > + u16 value; > +}; > + > +/* Values of input parameters to SetFeature of audio LED and Mic LED */ > +enum hotkey_set_feature { > + MIC_MUTE_LED_ON = 1, > + MIC_MUTE_LED_OFF, > + AUDIO_MUTE_LED_ON = 4, > + AUDIO_MUTE_LED_OFF, > +}; > + > +#define LSH_ACPI_LED_MAX 2 > + > +enum lenovo_super_hotkey_wmi_event_type { > + LSH_WMI_EVENT_LUD_KEYS = 2, > +}; > + > +struct lenovo_super_hotkey_wmi_private { > + enum lenovo_super_hotkey_wmi_event_type event; > + struct led_classdev cdev[LSH_ACPI_LED_MAX]; > + struct wmi_device *led_wdev; > +}; > + > +enum mute_led_type { > + MIC_MUTE, > + AUDIO_MUTE, > +}; > + > +static int lsh_wmi_mute_led_set(enum mute_led_type led_type, struct led_classdev *led_cdev, > + enum led_brightness brightness) > + > +{ > + struct lenovo_super_hotkey_wmi_private *wpriv = container_of(led_cdev, > + struct lenovo_super_hotkey_wmi_private, cdev[led_type]); > + struct wmi_led_args led_arg = {0, 0, 0}; > + struct acpi_buffer input; > + acpi_status status; > + > + switch (led_type) { > + case MIC_MUTE: > + led_arg.id = brightness == LED_ON ? MIC_MUTE_LED_ON : MIC_MUTE_LED_OFF; > + break; > + case AUDIO_MUTE: > + led_arg.id = brightness == LED_ON ? AUDIO_MUTE_LED_ON : AUDIO_MUTE_LED_OFF; > + break; > + default: > + return -EINVAL; > + } > + > + input.length = sizeof(led_arg); > + input.pointer = &led_arg; > + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_SET_FEATURE, &input, NULL); > + if (ACPI_FAILURE(status)) > + return -EIO; > + > + return 0; > +} > + > +static int lsh_wmi_audiomute_led_set(struct led_classdev *led_cdev, > + enum led_brightness brightness) > + > +{ > + return lsh_wmi_mute_led_set(AUDIO_MUTE, led_cdev, brightness); > +} > + > +static int lsh_wmi_micmute_led_set(struct led_classdev *led_cdev, > + enum led_brightness brightness) > +{ > + return lsh_wmi_mute_led_set(MIC_MUTE, led_cdev, brightness); > +} > + > +static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct device *dev) > +{ > + struct lenovo_super_hotkey_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; > + > + switch (led_type) { > + case MIC_MUTE: > + wmiarg = WMI_LUD_GET_MICMUTE_LED_VER; > + break; > + case AUDIO_MUTE: > + wmiarg = WMI_LUD_GET_AUDIOMUTE_LED_VER; > + break; > + default: > + return -EINVAL; > + } > + > + input.length = sizeof(wmiarg); > + input.pointer = &wmiarg; > + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_GET_SUPPORT, &input, &output); > + if (ACPI_FAILURE(status)) > + return -EIO; > + > + obj = output.pointer; > + if (obj && obj->type == ACPI_TYPE_INTEGER) { > + led_version = obj->integer.value; > + kfree(output.pointer); Maybe you should use <linux/cleanup.h> instead, to avoid memory bugs. Something like: union acpi_object *obj __free(kfree) = out.pointer > + } else { > + err = -EIO; > + goto led_error; > + } > + > + wpriv->cdev[led_type].max_brightness = LED_ON; > + wpriv->cdev[led_type].dev = dev; The .dev field in led_classdev is where the class device is stored by the LED core. Drivers should not set this. > + wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME; > + > + switch (led_type) { > + case MIC_MUTE: > + if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) { > + err = -EIO; > + goto led_error; Double free. > + } > + 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"; > + > + err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]); > + if (err < 0) > + goto led_reg_failed; Double free. > + > + break; > + case AUDIO_MUTE: > + if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) { > + err = -EIO; > + goto led_error; Double free. > + } > + wpriv->cdev[led_type].name = "platform::mute"; > + wpriv->cdev[led_type].brightness_set_blocking = &lsh_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) > + goto led_reg_failed; Double free. > + > + break; > + default: > + err = -EINVAL; > + dev_err(dev, "Unknown LED type %d\n", led_type); > + goto led_error; Double free. > + } > + > + return 0; > + > +led_reg_failed: > + dev_err(dev, "Could not register mute LED %d : %d\n", led_type, err); > + > +led_error: > + kfree(obj); > + return err; > +} > + > +static void lenovo_super_hotkey_wmi_leds_setup(struct device *dev) > +{ > + lenovo_super_hotkey_wmi_led_init(MIC_MUTE, dev); > + lenovo_super_hotkey_wmi_led_init(AUDIO_MUTE, dev); > +} > + > +static int lenovo_super_hotkey_wmi_probe(struct wmi_device *wdev, const void *context) > +{ > + struct lenovo_super_hotkey_wmi_private *wpriv; > + > + wpriv = devm_kzalloc(&wdev->dev, sizeof(*wpriv), GFP_KERNEL); > + if (!wpriv) > + return -ENOMEM; > + > + *wpriv = *(const struct lenovo_super_hotkey_wmi_private *)context; The .event context is only used inside this function, so I believe this copy is unnecessary. > + > + dev_set_drvdata(&wdev->dev, wpriv); > + > + if (wpriv->event == LSH_WMI_EVENT_LUD_KEYS) { This check is maybe unnecessary. > + wpriv->led_wdev = wdev; > + lenovo_super_hotkey_wmi_leds_setup(&wdev->dev); > + } > + > + return 0; > +} > + > +static void lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) > +{ > + struct lenovo_super_hotkey_wmi_private *wpriv = dev_get_drvdata(&wdev->dev); > + unsigned int i; > + > + for (i = 0; i < LSH_ACPI_LED_MAX; i++) > + led_classdev_unregister(&wpriv->cdev[i]); devm_led_classdev_register() was used so this is not needed. > + > + kfree(wpriv); devm_kzalloc() was used to allocate wpriv so this is not needed and probably double free. -- ~ Kurt > +} > +static const struct lenovo_super_hotkey_wmi_private lsk_wmi_context_lud_keys = { > + .event = LSH_WMI_EVENT_LUD_KEYS > +}; > + > +static const struct wmi_device_id lenovo_super_hotkey_wmi_id_table[] = { > + { LUD_WMI_METHOD_GUID, &lsk_wmi_context_lud_keys }, /* Utility data */ > + { } > +}; > + > +MODULE_DEVICE_TABLE(wmi, lenovo_super_hotkey_wmi_id_table); > + > +static struct wmi_driver lenovo_super_hotkey_wmi_driver = { > + .driver = { > + .name = "lenovo_super_hotkey_wmi", > + .probe_type = PROBE_PREFER_ASYNCHRONOUS > + }, > + .id_table = lenovo_super_hotkey_wmi_id_table, > + .probe = lenovo_super_hotkey_wmi_probe, > + .remove = lenovo_super_hotkey_wmi_remove, > + .no_singleton = true, > +}; > + > +module_wmi_driver(lenovo_super_hotkey_wmi_driver); > + > +MODULE_INFO(depends, "wmi,ideapad-laptop"); > +MODULE_AUTHOR("Jackie Dong <dongeg1@lenovo.com>"); > +MODULE_DESCRIPTION("Lenovo Super Hotkey Utility WMI extras driver"); > +MODULE_LICENSE("GPL");
On Sat, 8 Feb 2025, Jackie Dong wrote: > 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 > > Signed-off-by: Jackie Dong <xy-jackie@139.com> > Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca> > --- > Changes in v3: > - Changed the name of the Kconfig entry to LENOVO_SUPER_HOTKEY_WMI > - Renamed everything in this driver which contains the name "ideapad" > to instead contain the name of this driver. > - Moved struct wmi_device *led_wdev in lenovo_super_hotkey_wmi_private, > and use container_of() to the led_wdev pointer. > - Replaced sizeof(struct wmi_led_args) by sizeof(led_arg) > - Added condtions checking for obj && obj->type == ACPI_TYPE_INTEGER > and free the ACPI object after get the required value. > - Removed led_classdev_unregister() after led_reg_failed label, but > add lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) to free > resource. > - Removed IDEAPAD_WMI_EVENT_FN_KEYS/IDEAPAD_WMI_EVENT_LUD_KEYS related > source codes and only keep LUD_WMI_METHOD_GUID. > > Changes in v2: > - Update code layout and formatting as recommended in review > - Improved error handling in ideapad_wmi_led_init > - Separated a WMI driver named lenovo-super-hotkey-wmi.c from > ideapad-lap.c, it's only for Lenovo Super Hotkey WMI devices. > > drivers/platform/x86/Kconfig | 9 + > drivers/platform/x86/Makefile | 1 + > .../platform/x86/lenovo-super-hotkey-wmi.c | 248 ++++++++++++++++++ > 3 files changed, 258 insertions(+) > create mode 100644 drivers/platform/x86/lenovo-super-hotkey-wmi.c > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 0258dd879d64..c1792e8f04e1 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -475,6 +475,15 @@ config IDEAPAD_LAPTOP > This is a driver for Lenovo IdeaPad netbooks contains drivers for > rfkill switch, hotkey, fan control and backlight control. > > +config LENOVO_SUPER_HOTKEY_WMI > + tristate "Lenovo Super Hotkey Utility WMI extras driver" > + depends on ACPI_WMI > + depends on IDEAPAD_LAPTOP > + help > + This driver provides WMI support for Lenovo customized hotkeys function > + of Lenovo NoteBooks which are for Consumer and SMB customers, such as > + Ideapad/YOGA/XiaoXin/Gaming/ThinkBook and so on. > + > config LENOVO_YMC > tristate "Lenovo Yoga Tablet Mode Control" > depends on ACPI_WMI > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index e1b142947067..598335da9f55 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -61,6 +61,7 @@ obj-$(CONFIG_UV_SYSFS) += uv_sysfs.o > # IBM Thinkpad and Lenovo > obj-$(CONFIG_IBM_RTL) += ibm_rtl.o > obj-$(CONFIG_IDEAPAD_LAPTOP) += ideapad-laptop.o > +obj-$(CONFIG_LENOVO_SUPER_HOTKEY_WMI) += lenovo-super-hotkey-wmi.o > obj-$(CONFIG_LENOVO_YMC) += lenovo-ymc.o > obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o > obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o > diff --git a/drivers/platform/x86/lenovo-super-hotkey-wmi.c b/drivers/platform/x86/lenovo-super-hotkey-wmi.c > new file mode 100644 > index 000000000000..e677bd11fa2f > --- /dev/null > +++ b/drivers/platform/x86/lenovo-super-hotkey-wmi.c > @@ -0,0 +1,248 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop > + * > + * Copyright (C) 2025 Lenovo > + */ > + > +#include <linux/leds.h> > +#include <linux/wmi.h> > +#include "ideapad-laptop.h" Add an empty line before the local includes. > + > +/* Lenovo Super Hotkey WMI GUIDs */ > +#define LUD_WMI_METHOD_GUID "CE6C0974-0407-4F50-88BA-4FC3B6559AD8" > + > +/* Lenovo Utility Data WMI method_id */ > +#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 > + > +/* Input parameters to mute/unmute audio LED and Mic LED */ > +struct wmi_led_args { > + u8 id; > + u8 subid; > + u16 value; > +}; > + > +/* Values of input parameters to SetFeature of audio LED and Mic LED */ > +enum hotkey_set_feature { > + MIC_MUTE_LED_ON = 1, > + MIC_MUTE_LED_OFF, > + AUDIO_MUTE_LED_ON = 4, > + AUDIO_MUTE_LED_OFF, > +}; > + > +#define LSH_ACPI_LED_MAX 2 > + > +enum lenovo_super_hotkey_wmi_event_type { > + LSH_WMI_EVENT_LUD_KEYS = 2, > +}; > + > +struct lenovo_super_hotkey_wmi_private { > + enum lenovo_super_hotkey_wmi_event_type event; > + struct led_classdev cdev[LSH_ACPI_LED_MAX]; > + struct wmi_device *led_wdev; > +}; > + > +enum mute_led_type { > + MIC_MUTE, > + AUDIO_MUTE, > +}; > + > +static int lsh_wmi_mute_led_set(enum mute_led_type led_type, struct led_classdev *led_cdev, > + enum led_brightness brightness) > + > +{ > + struct lenovo_super_hotkey_wmi_private *wpriv = container_of(led_cdev, > + struct lenovo_super_hotkey_wmi_private, cdev[led_type]); > + struct wmi_led_args led_arg = {0, 0, 0}; > + struct acpi_buffer input; > + acpi_status status; > + > + switch (led_type) { > + case MIC_MUTE: > + led_arg.id = brightness == LED_ON ? MIC_MUTE_LED_ON : MIC_MUTE_LED_OFF; > + break; > + case AUDIO_MUTE: > + led_arg.id = brightness == LED_ON ? AUDIO_MUTE_LED_ON : AUDIO_MUTE_LED_OFF; > + break; > + default: > + return -EINVAL; > + } > + > + input.length = sizeof(led_arg); > + input.pointer = &led_arg; > + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_SET_FEATURE, &input, NULL); > + if (ACPI_FAILURE(status)) > + return -EIO; > + > + return 0; > +} > + > +static int lsh_wmi_audiomute_led_set(struct led_classdev *led_cdev, > + enum led_brightness brightness) > + > +{ > + return lsh_wmi_mute_led_set(AUDIO_MUTE, led_cdev, brightness); > +} > + > +static int lsh_wmi_micmute_led_set(struct led_classdev *led_cdev, > + enum led_brightness brightness) > +{ > + return lsh_wmi_mute_led_set(MIC_MUTE, led_cdev, brightness); > +} > + > +static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct device *dev) > +{ > + struct lenovo_super_hotkey_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; > + > + switch (led_type) { > + case MIC_MUTE: > + wmiarg = WMI_LUD_GET_MICMUTE_LED_VER; > + break; > + case AUDIO_MUTE: > + wmiarg = WMI_LUD_GET_AUDIOMUTE_LED_VER; > + break; > + default: > + return -EINVAL; > + } > + > + input.length = sizeof(wmiarg); > + input.pointer = &wmiarg; > + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_GET_SUPPORT, &input, &output); > + if (ACPI_FAILURE(status)) > + return -EIO; > + > + obj = output.pointer; > + if (obj && obj->type == ACPI_TYPE_INTEGER) { > + led_version = obj->integer.value; > + kfree(output.pointer); > + } else { > + err = -EIO; > + goto led_error; > + } > + > + wpriv->cdev[led_type].max_brightness = LED_ON; > + wpriv->cdev[led_type].dev = dev; > + wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME; > + > + switch (led_type) { > + case MIC_MUTE: > + if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) { > + err = -EIO; > + goto led_error; > + } > + 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"; > + > + err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]); > + if (err < 0) > + goto led_reg_failed; > + > + break; > + case AUDIO_MUTE: > + if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) { > + err = -EIO; > + goto led_error; > + } > + wpriv->cdev[led_type].name = "platform::mute"; > + wpriv->cdev[led_type].brightness_set_blocking = &lsh_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) > + goto led_reg_failed; > + > + break; > + default: > + err = -EINVAL; > + dev_err(dev, "Unknown LED type %d\n", led_type); Add include. > + goto led_error; > + } > + > + return 0; > + > +led_reg_failed: > + dev_err(dev, "Could not register mute LED %d : %d\n", led_type, err); > + > +led_error: > + kfree(obj); Add include. > + return err; > +} > + > +static void lenovo_super_hotkey_wmi_leds_setup(struct device *dev) > +{ > + lenovo_super_hotkey_wmi_led_init(MIC_MUTE, dev); > + lenovo_super_hotkey_wmi_led_init(AUDIO_MUTE, dev); > +} > + > +static int lenovo_super_hotkey_wmi_probe(struct wmi_device *wdev, const void *context) > +{ > + struct lenovo_super_hotkey_wmi_private *wpriv; > + > + wpriv = devm_kzalloc(&wdev->dev, sizeof(*wpriv), GFP_KERNEL); > + if (!wpriv) > + return -ENOMEM; > + > + *wpriv = *(const struct lenovo_super_hotkey_wmi_private *)context; > + > + dev_set_drvdata(&wdev->dev, wpriv); > + > + if (wpriv->event == LSH_WMI_EVENT_LUD_KEYS) { > + wpriv->led_wdev = wdev; > + lenovo_super_hotkey_wmi_leds_setup(&wdev->dev); > + } > + > + return 0; > +} > + > +static void lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) > +{ > + struct lenovo_super_hotkey_wmi_private *wpriv = dev_get_drvdata(&wdev->dev); > + unsigned int i; > + > + for (i = 0; i < LSH_ACPI_LED_MAX; i++) > + led_classdev_unregister(&wpriv->cdev[i]); > + > + kfree(wpriv); > +} > +static const struct lenovo_super_hotkey_wmi_private lsk_wmi_context_lud_keys = { > + .event = LSH_WMI_EVENT_LUD_KEYS > +}; > + > +static const struct wmi_device_id lenovo_super_hotkey_wmi_id_table[] = { > + { LUD_WMI_METHOD_GUID, &lsk_wmi_context_lud_keys }, /* Utility data */ > + { } > +}; > + > +MODULE_DEVICE_TABLE(wmi, lenovo_super_hotkey_wmi_id_table); Add include. > +static struct wmi_driver lenovo_super_hotkey_wmi_driver = { > + .driver = { > + .name = "lenovo_super_hotkey_wmi", > + .probe_type = PROBE_PREFER_ASYNCHRONOUS > + }, > + .id_table = lenovo_super_hotkey_wmi_id_table, > + .probe = lenovo_super_hotkey_wmi_probe, > + .remove = lenovo_super_hotkey_wmi_remove, > + .no_singleton = true, > +}; > + > +module_wmi_driver(lenovo_super_hotkey_wmi_driver); > + > +MODULE_INFO(depends, "wmi,ideapad-laptop"); > +MODULE_AUTHOR("Jackie Dong <dongeg1@lenovo.com>"); > +MODULE_DESCRIPTION("Lenovo Super Hotkey Utility WMI extras driver"); > +MODULE_LICENSE("GPL"); >
On 2/10/25 18:45, Ilpo Järvinen wrote: > On Sat, 8 Feb 2025, Jackie Dong wrote: > >> 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 >> >> Signed-off-by: Jackie Dong <xy-jackie@139.com> >> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca> >> --- >> Changes in v3: >> - Changed the name of the Kconfig entry to LENOVO_SUPER_HOTKEY_WMI >> - Renamed everything in this driver which contains the name "ideapad" >> to instead contain the name of this driver. >> - Moved struct wmi_device *led_wdev in lenovo_super_hotkey_wmi_private, >> and use container_of() to the led_wdev pointer. >> - Replaced sizeof(struct wmi_led_args) by sizeof(led_arg) >> - Added condtions checking for obj && obj->type == ACPI_TYPE_INTEGER >> and free the ACPI object after get the required value. >> - Removed led_classdev_unregister() after led_reg_failed label, but >> add lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) to free >> resource. >> - Removed IDEAPAD_WMI_EVENT_FN_KEYS/IDEAPAD_WMI_EVENT_LUD_KEYS related >> source codes and only keep LUD_WMI_METHOD_GUID. >> >> Changes in v2: >> - Update code layout and formatting as recommended in review >> - Improved error handling in ideapad_wmi_led_init >> - Separated a WMI driver named lenovo-super-hotkey-wmi.c from >> ideapad-lap.c, it's only for Lenovo Super Hotkey WMI devices. >> >> drivers/platform/x86/Kconfig | 9 + >> drivers/platform/x86/Makefile | 1 + >> .../platform/x86/lenovo-super-hotkey-wmi.c | 248 ++++++++++++++++++ >> 3 files changed, 258 insertions(+) >> create mode 100644 drivers/platform/x86/lenovo-super-hotkey-wmi.c >> >> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig >> index 0258dd879d64..c1792e8f04e1 100644 >> --- a/drivers/platform/x86/Kconfig >> +++ b/drivers/platform/x86/Kconfig >> @@ -475,6 +475,15 @@ config IDEAPAD_LAPTOP >> This is a driver for Lenovo IdeaPad netbooks contains drivers for >> rfkill switch, hotkey, fan control and backlight control. >> >> +config LENOVO_SUPER_HOTKEY_WMI >> + tristate "Lenovo Super Hotkey Utility WMI extras driver" >> + depends on ACPI_WMI >> + depends on IDEAPAD_LAPTOP >> + help >> + This driver provides WMI support for Lenovo customized hotkeys function >> + of Lenovo NoteBooks which are for Consumer and SMB customers, such as >> + Ideapad/YOGA/XiaoXin/Gaming/ThinkBook and so on. >> + >> config LENOVO_YMC >> tristate "Lenovo Yoga Tablet Mode Control" >> depends on ACPI_WMI >> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile >> index e1b142947067..598335da9f55 100644 >> --- a/drivers/platform/x86/Makefile >> +++ b/drivers/platform/x86/Makefile >> @@ -61,6 +61,7 @@ obj-$(CONFIG_UV_SYSFS) += uv_sysfs.o >> # IBM Thinkpad and Lenovo >> obj-$(CONFIG_IBM_RTL) += ibm_rtl.o >> obj-$(CONFIG_IDEAPAD_LAPTOP) += ideapad-laptop.o >> +obj-$(CONFIG_LENOVO_SUPER_HOTKEY_WMI) += lenovo-super-hotkey-wmi.o >> obj-$(CONFIG_LENOVO_YMC) += lenovo-ymc.o >> obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o >> obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o >> diff --git a/drivers/platform/x86/lenovo-super-hotkey-wmi.c b/drivers/platform/x86/lenovo-super-hotkey-wmi.c >> new file mode 100644 >> index 000000000000..e677bd11fa2f >> --- /dev/null >> +++ b/drivers/platform/x86/lenovo-super-hotkey-wmi.c >> @@ -0,0 +1,248 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop >> + * >> + * Copyright (C) 2025 Lenovo >> + */ >> + >> +#include <linux/leds.h> >> +#include <linux/wmi.h> >> +#include "ideapad-laptop.h" > > Add an empty line before the local includes. Hi Ilpo, Thanks for your review and comments. I'll update it in next version patch. > >> + >> +/* Lenovo Super Hotkey WMI GUIDs */ >> +#define LUD_WMI_METHOD_GUID "CE6C0974-0407-4F50-88BA-4FC3B6559AD8" >> + >> +/* Lenovo Utility Data WMI method_id */ >> +#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 >> + >> +/* Input parameters to mute/unmute audio LED and Mic LED */ >> +struct wmi_led_args { >> + u8 id; >> + u8 subid; >> + u16 value; >> +}; >> + >> +/* Values of input parameters to SetFeature of audio LED and Mic LED */ >> +enum hotkey_set_feature { >> + MIC_MUTE_LED_ON = 1, >> + MIC_MUTE_LED_OFF, >> + AUDIO_MUTE_LED_ON = 4, >> + AUDIO_MUTE_LED_OFF, >> +}; >> + >> +#define LSH_ACPI_LED_MAX 2 >> + >> +enum lenovo_super_hotkey_wmi_event_type { >> + LSH_WMI_EVENT_LUD_KEYS = 2, >> +}; >> + >> +struct lenovo_super_hotkey_wmi_private { >> + enum lenovo_super_hotkey_wmi_event_type event; >> + struct led_classdev cdev[LSH_ACPI_LED_MAX]; >> + struct wmi_device *led_wdev; >> +}; >> + >> +enum mute_led_type { >> + MIC_MUTE, >> + AUDIO_MUTE, >> +}; >> + >> +static int lsh_wmi_mute_led_set(enum mute_led_type led_type, struct led_classdev *led_cdev, >> + enum led_brightness brightness) >> + >> +{ >> + struct lenovo_super_hotkey_wmi_private *wpriv = container_of(led_cdev, >> + struct lenovo_super_hotkey_wmi_private, cdev[led_type]); >> + struct wmi_led_args led_arg = {0, 0, 0}; >> + struct acpi_buffer input; >> + acpi_status status; >> + >> + switch (led_type) { >> + case MIC_MUTE: >> + led_arg.id = brightness == LED_ON ? MIC_MUTE_LED_ON : MIC_MUTE_LED_OFF; >> + break; >> + case AUDIO_MUTE: >> + led_arg.id = brightness == LED_ON ? AUDIO_MUTE_LED_ON : AUDIO_MUTE_LED_OFF; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + input.length = sizeof(led_arg); >> + input.pointer = &led_arg; >> + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_SET_FEATURE, &input, NULL); >> + if (ACPI_FAILURE(status)) >> + return -EIO; >> + >> + return 0; >> +} >> + >> +static int lsh_wmi_audiomute_led_set(struct led_classdev *led_cdev, >> + enum led_brightness brightness) >> + >> +{ >> + return lsh_wmi_mute_led_set(AUDIO_MUTE, led_cdev, brightness); >> +} >> + >> +static int lsh_wmi_micmute_led_set(struct led_classdev *led_cdev, >> + enum led_brightness brightness) >> +{ >> + return lsh_wmi_mute_led_set(MIC_MUTE, led_cdev, brightness); >> +} >> + >> +static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct device *dev) >> +{ >> + struct lenovo_super_hotkey_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; >> + >> + switch (led_type) { >> + case MIC_MUTE: >> + wmiarg = WMI_LUD_GET_MICMUTE_LED_VER; >> + break; >> + case AUDIO_MUTE: >> + wmiarg = WMI_LUD_GET_AUDIOMUTE_LED_VER; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + input.length = sizeof(wmiarg); >> + input.pointer = &wmiarg; >> + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_GET_SUPPORT, &input, &output); >> + if (ACPI_FAILURE(status)) >> + return -EIO; >> + >> + obj = output.pointer; >> + if (obj && obj->type == ACPI_TYPE_INTEGER) { >> + led_version = obj->integer.value; >> + kfree(output.pointer); >> + } else { >> + err = -EIO; >> + goto led_error; >> + } >> + >> + wpriv->cdev[led_type].max_brightness = LED_ON; >> + wpriv->cdev[led_type].dev = dev; >> + wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME; >> + >> + switch (led_type) { >> + case MIC_MUTE: >> + if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) { >> + err = -EIO; >> + goto led_error; >> + } >> + 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"; >> + >> + err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]); >> + if (err < 0) >> + goto led_reg_failed; >> + >> + break; >> + case AUDIO_MUTE: >> + if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) { >> + err = -EIO; >> + goto led_error; >> + } >> + wpriv->cdev[led_type].name = "platform::mute"; >> + wpriv->cdev[led_type].brightness_set_blocking = &lsh_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) >> + goto led_reg_failed; >> + >> + break; >> + default: >> + err = -EINVAL; >> + dev_err(dev, "Unknown LED type %d\n", led_type); > > Add include. For this comment, I understand that it need to add include for dev_err(). If so, my comments is below. If you think that it still need to add the include include/linux/dev_printk.h, pls comfirm it again, I'll add it in next version patch. dev_err() is defined in include/linux/dev_printk.h which is included by include/linux/device.h which is included by include/linux/wmi.h which is included by this file as below. >> @@ -0,0 +1,248 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop >> + * >> + * Copyright (C) 2025 Lenovo >> + */ >> + >> +#include <linux/leds.h> >> +#include <linux/wmi.h> > >> + goto led_error; >> + } >> + >> + return 0; >> + >> +led_reg_failed: >> + dev_err(dev, "Could not register mute LED %d : %d\n", led_type, err); >> + >> +led_error: >> + kfree(obj); > > Add include. For this comment, I understand that it need to add include for kfree(). If so, my comments is below. If you think that it still need to add the include include/linux/slab.h, pls comfirm it again, I'll add it in next version patch or change to another function. kfree() is defined in include/linux/slab.h and the include chain is below. include/linux/slab.h -> include/acpi/platform/aclinux.h -> include/acpi/platform/acenv.h -> include/linux/acpi.h -> include/linux/wmi.h and wmi.h is included by this file as below. >> @@ -0,0 +1,248 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop >> + * >> + * Copyright (C) 2025 Lenovo >> + */ >> + >> +#include <linux/leds.h> >> +#include <linux/wmi.h> > >> + return err; >> +} >> + >> +static void lenovo_super_hotkey_wmi_leds_setup(struct device *dev) >> +{ >> + lenovo_super_hotkey_wmi_led_init(MIC_MUTE, dev); >> + lenovo_super_hotkey_wmi_led_init(AUDIO_MUTE, dev); >> +} >> + >> +static int lenovo_super_hotkey_wmi_probe(struct wmi_device *wdev, const void *context) >> +{ >> + struct lenovo_super_hotkey_wmi_private *wpriv; >> + >> + wpriv = devm_kzalloc(&wdev->dev, sizeof(*wpriv), GFP_KERNEL); >> + if (!wpriv) >> + return -ENOMEM; >> + >> + *wpriv = *(const struct lenovo_super_hotkey_wmi_private *)context; >> + >> + dev_set_drvdata(&wdev->dev, wpriv); >> + >> + if (wpriv->event == LSH_WMI_EVENT_LUD_KEYS) { >> + wpriv->led_wdev = wdev; >> + lenovo_super_hotkey_wmi_leds_setup(&wdev->dev); >> + } >> + >> + return 0; >> +} >> + >> +static void lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) >> +{ >> + struct lenovo_super_hotkey_wmi_private *wpriv = dev_get_drvdata(&wdev->dev); >> + unsigned int i; >> + >> + for (i = 0; i < LSH_ACPI_LED_MAX; i++) >> + led_classdev_unregister(&wpriv->cdev[i]); >> + >> + kfree(wpriv); >> +} >> +static const struct lenovo_super_hotkey_wmi_private lsk_wmi_context_lud_keys = { >> + .event = LSH_WMI_EVENT_LUD_KEYS >> +}; >> + >> +static const struct wmi_device_id lenovo_super_hotkey_wmi_id_table[] = { >> + { LUD_WMI_METHOD_GUID, &lsk_wmi_context_lud_keys }, /* Utility data */ >> + { } >> +}; >> + >> +MODULE_DEVICE_TABLE(wmi, lenovo_super_hotkey_wmi_id_table); > > Add include. For this comment, I understand that it need to add include for MODULE_DEVICE_TABLE(). If so, my comment is below. If you think that it still need to add the include/linux/module.h, pls comfirm it again, I'll add it in next version patch. MODULE_DEVICE_TABLE() is defined in include/linux/module.h which is included by include/linux/acpi.h which is included by include/linux/wmi.h which is included by this file as below. >> @@ -0,0 +1,248 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop >> + * >> + * Copyright (C) 2025 Lenovo >> + */ >> + >> +#include <linux/leds.h> >> +#include <linux/wmi.h> > >> +static struct wmi_driver lenovo_super_hotkey_wmi_driver = { >> + .driver = { >> + .name = "lenovo_super_hotkey_wmi", >> + .probe_type = PROBE_PREFER_ASYNCHRONOUS >> + }, >> + .id_table = lenovo_super_hotkey_wmi_id_table, >> + .probe = lenovo_super_hotkey_wmi_probe, >> + .remove = lenovo_super_hotkey_wmi_remove, >> + .no_singleton = true, >> +}; >> + >> +module_wmi_driver(lenovo_super_hotkey_wmi_driver); >> + >> +MODULE_INFO(depends, "wmi,ideapad-laptop"); >> +MODULE_AUTHOR("Jackie Dong <dongeg1@lenovo.com>"); >> +MODULE_DESCRIPTION("Lenovo Super Hotkey Utility WMI extras driver"); >> +MODULE_LICENSE("GPL"); >> >
On Tue, 11 Feb 2025, Jackie Dong wrote: > On 2/10/25 18:45, Ilpo Järvinen wrote: > > On Sat, 8 Feb 2025, Jackie Dong wrote: > > > > > 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 > > > > > > Signed-off-by: Jackie Dong <xy-jackie@139.com> > > > Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca> > > > --- > > > Changes in v3: > > > - Changed the name of the Kconfig entry to LENOVO_SUPER_HOTKEY_WMI > > > - Renamed everything in this driver which contains the name "ideapad" > > > to instead contain the name of this driver. > > > - Moved struct wmi_device *led_wdev in lenovo_super_hotkey_wmi_private, > > > and use container_of() to the led_wdev pointer. > > > - Replaced sizeof(struct wmi_led_args) by sizeof(led_arg) > > > - Added condtions checking for obj && obj->type == ACPI_TYPE_INTEGER > > > and free the ACPI object after get the required value. > > > - Removed led_classdev_unregister() after led_reg_failed label, but > > > add lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) to free > > > resource. > > > - Removed IDEAPAD_WMI_EVENT_FN_KEYS/IDEAPAD_WMI_EVENT_LUD_KEYS related > > > source codes and only keep LUD_WMI_METHOD_GUID. > > > > > > Changes in v2: > > > - Update code layout and formatting as recommended in review > > > - Improved error handling in ideapad_wmi_led_init > > > - Separated a WMI driver named lenovo-super-hotkey-wmi.c from > > > ideapad-lap.c, it's only for Lenovo Super Hotkey WMI devices. > > > > > > drivers/platform/x86/Kconfig | 9 + > > > drivers/platform/x86/Makefile | 1 + > > > .../platform/x86/lenovo-super-hotkey-wmi.c | 248 ++++++++++++++++++ > > > 3 files changed, 258 insertions(+) > > > create mode 100644 drivers/platform/x86/lenovo-super-hotkey-wmi.c > > > > > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > > index 0258dd879d64..c1792e8f04e1 100644 > > > --- a/drivers/platform/x86/Kconfig > > > +++ b/drivers/platform/x86/Kconfig > > > @@ -475,6 +475,15 @@ config IDEAPAD_LAPTOP > > > This is a driver for Lenovo IdeaPad netbooks contains drivers for > > > rfkill switch, hotkey, fan control and backlight control. > > > +config LENOVO_SUPER_HOTKEY_WMI > > > + tristate "Lenovo Super Hotkey Utility WMI extras driver" > > > + depends on ACPI_WMI > > > + depends on IDEAPAD_LAPTOP > > > + help > > > + This driver provides WMI support for Lenovo customized hotkeys > > > function > > > + of Lenovo NoteBooks which are for Consumer and SMB customers, such > > > as > > > + Ideapad/YOGA/XiaoXin/Gaming/ThinkBook and so on. > > > + > > > config LENOVO_YMC > > > tristate "Lenovo Yoga Tablet Mode Control" > > > depends on ACPI_WMI > > > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > > > index e1b142947067..598335da9f55 100644 > > > --- a/drivers/platform/x86/Makefile > > > +++ b/drivers/platform/x86/Makefile > > > @@ -61,6 +61,7 @@ obj-$(CONFIG_UV_SYSFS) += uv_sysfs.o > > > # IBM Thinkpad and Lenovo > > > obj-$(CONFIG_IBM_RTL) += ibm_rtl.o > > > obj-$(CONFIG_IDEAPAD_LAPTOP) += ideapad-laptop.o > > > +obj-$(CONFIG_LENOVO_SUPER_HOTKEY_WMI) += lenovo-super-hotkey-wmi.o > > > obj-$(CONFIG_LENOVO_YMC) += lenovo-ymc.o > > > obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o > > > obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o > > > diff --git a/drivers/platform/x86/lenovo-super-hotkey-wmi.c > > > b/drivers/platform/x86/lenovo-super-hotkey-wmi.c > > > new file mode 100644 > > > index 000000000000..e677bd11fa2f > > > --- /dev/null > > > +++ b/drivers/platform/x86/lenovo-super-hotkey-wmi.c > > > @@ -0,0 +1,248 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop > > > + * > > > + * Copyright (C) 2025 Lenovo > > > + */ > > > + > > > +#include <linux/leds.h> > > > +#include <linux/wmi.h> > > > +#include "ideapad-laptop.h" > > > > Add an empty line before the local includes. > Hi Ilpo, > Thanks for your review and comments. > I'll update it in next version patch. > > > > > + > > > +/* Lenovo Super Hotkey WMI GUIDs */ > > > +#define LUD_WMI_METHOD_GUID "CE6C0974-0407-4F50-88BA-4FC3B6559AD8" > > > + > > > +/* Lenovo Utility Data WMI method_id */ > > > +#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 > > > + > > > +/* Input parameters to mute/unmute audio LED and Mic LED */ > > > +struct wmi_led_args { > > > + u8 id; > > > + u8 subid; > > > + u16 value; > > > +}; > > > + > > > +/* Values of input parameters to SetFeature of audio LED and Mic LED */ > > > +enum hotkey_set_feature { > > > + MIC_MUTE_LED_ON = 1, > > > + MIC_MUTE_LED_OFF, > > > + AUDIO_MUTE_LED_ON = 4, > > > + AUDIO_MUTE_LED_OFF, > > > +}; > > > + > > > +#define LSH_ACPI_LED_MAX 2 > > > + > > > +enum lenovo_super_hotkey_wmi_event_type { > > > + LSH_WMI_EVENT_LUD_KEYS = 2, > > > +}; > > > + > > > +struct lenovo_super_hotkey_wmi_private { > > > + enum lenovo_super_hotkey_wmi_event_type event; > > > + struct led_classdev cdev[LSH_ACPI_LED_MAX]; > > > + struct wmi_device *led_wdev; > > > +}; > > > + > > > +enum mute_led_type { > > > + MIC_MUTE, > > > + AUDIO_MUTE, > > > +}; > > > + > > > +static int lsh_wmi_mute_led_set(enum mute_led_type led_type, struct > > > led_classdev *led_cdev, > > > + enum led_brightness brightness) > > > + > > > +{ > > > + struct lenovo_super_hotkey_wmi_private *wpriv = container_of(led_cdev, > > > + struct lenovo_super_hotkey_wmi_private, > > > cdev[led_type]); > > > + struct wmi_led_args led_arg = {0, 0, 0}; > > > + struct acpi_buffer input; > > > + acpi_status status; > > > + > > > + switch (led_type) { > > > + case MIC_MUTE: > > > + led_arg.id = brightness == LED_ON ? MIC_MUTE_LED_ON : > > > MIC_MUTE_LED_OFF; > > > + break; > > > + case AUDIO_MUTE: > > > + led_arg.id = brightness == LED_ON ? AUDIO_MUTE_LED_ON : > > > AUDIO_MUTE_LED_OFF; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + input.length = sizeof(led_arg); > > > + input.pointer = &led_arg; > > > + status = wmidev_evaluate_method(wpriv->led_wdev, 0, > > > WMI_LUD_SET_FEATURE, &input, NULL); > > > + if (ACPI_FAILURE(status)) > > > + return -EIO; > > > + > > > + return 0; > > > +} > > > + > > > +static int lsh_wmi_audiomute_led_set(struct led_classdev *led_cdev, > > > + enum led_brightness brightness) > > > + > > > +{ > > > + return lsh_wmi_mute_led_set(AUDIO_MUTE, led_cdev, brightness); > > > +} > > > + > > > +static int lsh_wmi_micmute_led_set(struct led_classdev *led_cdev, > > > + enum led_brightness brightness) > > > +{ > > > + return lsh_wmi_mute_led_set(MIC_MUTE, led_cdev, brightness); > > > +} > > > + > > > +static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, > > > struct device *dev) > > > +{ > > > + struct lenovo_super_hotkey_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; > > > + > > > + switch (led_type) { > > > + case MIC_MUTE: > > > + wmiarg = WMI_LUD_GET_MICMUTE_LED_VER; > > > + break; > > > + case AUDIO_MUTE: > > > + wmiarg = WMI_LUD_GET_AUDIOMUTE_LED_VER; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + input.length = sizeof(wmiarg); > > > + input.pointer = &wmiarg; > > > + status = wmidev_evaluate_method(wpriv->led_wdev, 0, > > > WMI_LUD_GET_SUPPORT, &input, &output); > > > + if (ACPI_FAILURE(status)) > > > + return -EIO; > > > + > > > + obj = output.pointer; > > > + if (obj && obj->type == ACPI_TYPE_INTEGER) { > > > + led_version = obj->integer.value; > > > + kfree(output.pointer); > > > + } else { > > > + err = -EIO; > > > + goto led_error; > > > + } > > > + > > > + wpriv->cdev[led_type].max_brightness = LED_ON; > > > + wpriv->cdev[led_type].dev = dev; > > > + wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME; > > > + > > > + switch (led_type) { > > > + case MIC_MUTE: > > > + if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) { > > > + err = -EIO; > > > + goto led_error; > > > + } > > > + 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"; > > > + > > > + err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]); > > > + if (err < 0) > > > + goto led_reg_failed; > > > + > > > + break; > > > + case AUDIO_MUTE: > > > + if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) { > > > + err = -EIO; > > > + goto led_error; > > > + } > > > + wpriv->cdev[led_type].name = "platform::mute"; > > > + wpriv->cdev[led_type].brightness_set_blocking = > > > &lsh_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) > > > + goto led_reg_failed; > > > + > > > + break; > > > + default: > > > + err = -EINVAL; > > > + dev_err(dev, "Unknown LED type %d\n", led_type); > > > > Add include. > For this comment, I understand that it need to add include for dev_err(). If > so, my comments is below. If you think that it still need to add the include > include/linux/dev_printk.h, pls comfirm it again, I'll add it in next version > patch. > dev_err() is defined in include/linux/dev_printk.h which is included by > include/linux/device.h which is included by include/linux/wmi.h which is > included by this file as below. Hi, Even if there's an indirect include path through other headers, you should include linux/dev_printk.h also here. There's nothing fundamendal that requires device.h to include dev_printk.h even if it currently does so relying on these indirect includes is fragile (and hopefully somebody eventually cleans up the entire include mess). Same applies to the other case I marked below so please add those as well. There are some guaranteed include paths through another header and I normally won't flag those, e.g., wmi.h will always have to include device.h because struct device resides within struct wmi_device (it won't build at all if device.h include is removed from wmi.h unless something else by chance included device.h before wmi.h).
On 2/11/25 22:00, Ilpo Järvinen wrote: > On Tue, 11 Feb 2025, Jackie Dong wrote: > >> On 2/10/25 18:45, Ilpo Järvinen wrote: >>> On Sat, 8 Feb 2025, Jackie Dong wrote: >>> >>>> 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 >>>> >>>> Signed-off-by: Jackie Dong <xy-jackie@139.com> >>>> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca> >>>> --- >>>> Changes in v3: >>>> - Changed the name of the Kconfig entry to LENOVO_SUPER_HOTKEY_WMI >>>> - Renamed everything in this driver which contains the name "ideapad" >>>> to instead contain the name of this driver. >>>> - Moved struct wmi_device *led_wdev in lenovo_super_hotkey_wmi_private, >>>> and use container_of() to the led_wdev pointer. >>>> - Replaced sizeof(struct wmi_led_args) by sizeof(led_arg) >>>> - Added condtions checking for obj && obj->type == ACPI_TYPE_INTEGER >>>> and free the ACPI object after get the required value. >>>> - Removed led_classdev_unregister() after led_reg_failed label, but >>>> add lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) to free >>>> resource. >>>> - Removed IDEAPAD_WMI_EVENT_FN_KEYS/IDEAPAD_WMI_EVENT_LUD_KEYS related >>>> source codes and only keep LUD_WMI_METHOD_GUID. >>>> >>>> Changes in v2: >>>> - Update code layout and formatting as recommended in review >>>> - Improved error handling in ideapad_wmi_led_init >>>> - Separated a WMI driver named lenovo-super-hotkey-wmi.c from >>>> ideapad-lap.c, it's only for Lenovo Super Hotkey WMI devices. >>>> >>>> drivers/platform/x86/Kconfig | 9 + >>>> drivers/platform/x86/Makefile | 1 + >>>> .../platform/x86/lenovo-super-hotkey-wmi.c | 248 ++++++++++++++++++ >>>> 3 files changed, 258 insertions(+) >>>> create mode 100644 drivers/platform/x86/lenovo-super-hotkey-wmi.c >>>> >>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig >>>> index 0258dd879d64..c1792e8f04e1 100644 >>>> --- a/drivers/platform/x86/Kconfig >>>> +++ b/drivers/platform/x86/Kconfig >>>> @@ -475,6 +475,15 @@ config IDEAPAD_LAPTOP >>>> This is a driver for Lenovo IdeaPad netbooks contains drivers for >>>> rfkill switch, hotkey, fan control and backlight control. >>>> +config LENOVO_SUPER_HOTKEY_WMI >>>> + tristate "Lenovo Super Hotkey Utility WMI extras driver" >>>> + depends on ACPI_WMI >>>> + depends on IDEAPAD_LAPTOP >>>> + help >>>> + This driver provides WMI support for Lenovo customized hotkeys >>>> function >>>> + of Lenovo NoteBooks which are for Consumer and SMB customers, such >>>> as >>>> + Ideapad/YOGA/XiaoXin/Gaming/ThinkBook and so on. >>>> + >>>> config LENOVO_YMC >>>> tristate "Lenovo Yoga Tablet Mode Control" >>>> depends on ACPI_WMI >>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile >>>> index e1b142947067..598335da9f55 100644 >>>> --- a/drivers/platform/x86/Makefile >>>> +++ b/drivers/platform/x86/Makefile >>>> @@ -61,6 +61,7 @@ obj-$(CONFIG_UV_SYSFS) += uv_sysfs.o >>>> # IBM Thinkpad and Lenovo >>>> obj-$(CONFIG_IBM_RTL) += ibm_rtl.o >>>> obj-$(CONFIG_IDEAPAD_LAPTOP) += ideapad-laptop.o >>>> +obj-$(CONFIG_LENOVO_SUPER_HOTKEY_WMI) += lenovo-super-hotkey-wmi.o >>>> obj-$(CONFIG_LENOVO_YMC) += lenovo-ymc.o >>>> obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o >>>> obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o >>>> diff --git a/drivers/platform/x86/lenovo-super-hotkey-wmi.c >>>> b/drivers/platform/x86/lenovo-super-hotkey-wmi.c >>>> new file mode 100644 >>>> index 000000000000..e677bd11fa2f >>>> --- /dev/null >>>> +++ b/drivers/platform/x86/lenovo-super-hotkey-wmi.c >>>> @@ -0,0 +1,248 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop >>>> + * >>>> + * Copyright (C) 2025 Lenovo >>>> + */ >>>> + >>>> +#include <linux/leds.h> >>>> +#include <linux/wmi.h> >>>> +#include "ideapad-laptop.h" >>> >>> Add an empty line before the local includes. >> Hi Ilpo, >> Thanks for your review and comments. >> I'll update it in next version patch. >>> >>>> + >>>> +/* Lenovo Super Hotkey WMI GUIDs */ >>>> +#define LUD_WMI_METHOD_GUID "CE6C0974-0407-4F50-88BA-4FC3B6559AD8" >>>> + >>>> +/* Lenovo Utility Data WMI method_id */ >>>> +#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 >>>> + >>>> +/* Input parameters to mute/unmute audio LED and Mic LED */ >>>> +struct wmi_led_args { >>>> + u8 id; >>>> + u8 subid; >>>> + u16 value; >>>> +}; >>>> + >>>> +/* Values of input parameters to SetFeature of audio LED and Mic LED */ >>>> +enum hotkey_set_feature { >>>> + MIC_MUTE_LED_ON = 1, >>>> + MIC_MUTE_LED_OFF, >>>> + AUDIO_MUTE_LED_ON = 4, >>>> + AUDIO_MUTE_LED_OFF, >>>> +}; >>>> + >>>> +#define LSH_ACPI_LED_MAX 2 >>>> + >>>> +enum lenovo_super_hotkey_wmi_event_type { >>>> + LSH_WMI_EVENT_LUD_KEYS = 2, >>>> +}; >>>> + >>>> +struct lenovo_super_hotkey_wmi_private { >>>> + enum lenovo_super_hotkey_wmi_event_type event; >>>> + struct led_classdev cdev[LSH_ACPI_LED_MAX]; >>>> + struct wmi_device *led_wdev; >>>> +}; >>>> + >>>> +enum mute_led_type { >>>> + MIC_MUTE, >>>> + AUDIO_MUTE, >>>> +}; >>>> + >>>> +static int lsh_wmi_mute_led_set(enum mute_led_type led_type, struct >>>> led_classdev *led_cdev, >>>> + enum led_brightness brightness) >>>> + >>>> +{ >>>> + struct lenovo_super_hotkey_wmi_private *wpriv = container_of(led_cdev, >>>> + struct lenovo_super_hotkey_wmi_private, >>>> cdev[led_type]); >>>> + struct wmi_led_args led_arg = {0, 0, 0}; >>>> + struct acpi_buffer input; >>>> + acpi_status status; >>>> + >>>> + switch (led_type) { >>>> + case MIC_MUTE: >>>> + led_arg.id = brightness == LED_ON ? MIC_MUTE_LED_ON : >>>> MIC_MUTE_LED_OFF; >>>> + break; >>>> + case AUDIO_MUTE: >>>> + led_arg.id = brightness == LED_ON ? AUDIO_MUTE_LED_ON : >>>> AUDIO_MUTE_LED_OFF; >>>> + break; >>>> + default: >>>> + return -EINVAL; >>>> + } >>>> + >>>> + input.length = sizeof(led_arg); >>>> + input.pointer = &led_arg; >>>> + status = wmidev_evaluate_method(wpriv->led_wdev, 0, >>>> WMI_LUD_SET_FEATURE, &input, NULL); >>>> + if (ACPI_FAILURE(status)) >>>> + return -EIO; >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int lsh_wmi_audiomute_led_set(struct led_classdev *led_cdev, >>>> + enum led_brightness brightness) >>>> + >>>> +{ >>>> + return lsh_wmi_mute_led_set(AUDIO_MUTE, led_cdev, brightness); >>>> +} >>>> + >>>> +static int lsh_wmi_micmute_led_set(struct led_classdev *led_cdev, >>>> + enum led_brightness brightness) >>>> +{ >>>> + return lsh_wmi_mute_led_set(MIC_MUTE, led_cdev, brightness); >>>> +} >>>> + >>>> +static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, >>>> struct device *dev) >>>> +{ >>>> + struct lenovo_super_hotkey_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; >>>> + >>>> + switch (led_type) { >>>> + case MIC_MUTE: >>>> + wmiarg = WMI_LUD_GET_MICMUTE_LED_VER; >>>> + break; >>>> + case AUDIO_MUTE: >>>> + wmiarg = WMI_LUD_GET_AUDIOMUTE_LED_VER; >>>> + break; >>>> + default: >>>> + return -EINVAL; >>>> + } >>>> + >>>> + input.length = sizeof(wmiarg); >>>> + input.pointer = &wmiarg; >>>> + status = wmidev_evaluate_method(wpriv->led_wdev, 0, >>>> WMI_LUD_GET_SUPPORT, &input, &output); >>>> + if (ACPI_FAILURE(status)) >>>> + return -EIO; >>>> + >>>> + obj = output.pointer; >>>> + if (obj && obj->type == ACPI_TYPE_INTEGER) { >>>> + led_version = obj->integer.value; >>>> + kfree(output.pointer); >>>> + } else { >>>> + err = -EIO; >>>> + goto led_error; >>>> + } >>>> + >>>> + wpriv->cdev[led_type].max_brightness = LED_ON; >>>> + wpriv->cdev[led_type].dev = dev; >>>> + wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME; >>>> + >>>> + switch (led_type) { >>>> + case MIC_MUTE: >>>> + if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) { >>>> + err = -EIO; >>>> + goto led_error; >>>> + } >>>> + 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"; >>>> + >>>> + err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]); >>>> + if (err < 0) >>>> + goto led_reg_failed; >>>> + >>>> + break; >>>> + case AUDIO_MUTE: >>>> + if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) { >>>> + err = -EIO; >>>> + goto led_error; >>>> + } >>>> + wpriv->cdev[led_type].name = "platform::mute"; >>>> + wpriv->cdev[led_type].brightness_set_blocking = >>>> &lsh_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) >>>> + goto led_reg_failed; >>>> + >>>> + break; >>>> + default: >>>> + err = -EINVAL; >>>> + dev_err(dev, "Unknown LED type %d\n", led_type); >>> >>> Add include. >> For this comment, I understand that it need to add include for dev_err(). If >> so, my comments is below. If you think that it still need to add the include >> include/linux/dev_printk.h, pls comfirm it again, I'll add it in next version >> patch. >> dev_err() is defined in include/linux/dev_printk.h which is included by >> include/linux/device.h which is included by include/linux/wmi.h which is >> included by this file as below. > > Hi, > > Even if there's an indirect include path through other headers, you should > include linux/dev_printk.h also here. There's nothing fundamendal that > requires device.h to include dev_printk.h even if it currently does so > relying on these indirect includes is fragile (and hopefully somebody > eventually cleans up the entire include mess). > > Same applies to the other case I marked below so please add those as > well. > > There are some guaranteed include paths through another header and > I normally won't flag those, e.g., wmi.h will always have to include > device.h because struct device resides within struct wmi_device (it won't > build at all if device.h include is removed from wmi.h unless something > else by chance included device.h before wmi.h). > Hi Ilpo, Thanks for your careful explaintion, I understand your view now and will add the include files in next version patch. Jackie Dong
On 2/10/25 12:00, Kurt Borja wrote: > Hi Jackie, > > I noticed a few things while going through your patch: > > On Sat Feb 8, 2025 at 9:41 AM -05, Jackie Dong wrote: >> 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 >> >> Signed-off-by: Jackie Dong <xy-jackie@139.com> >> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca> >> --- >> Changes in v3: >> - Changed the name of the Kconfig entry to LENOVO_SUPER_HOTKEY_WMI >> - Renamed everything in this driver which contains the name "ideapad" >> to instead contain the name of this driver. >> - Moved struct wmi_device *led_wdev in lenovo_super_hotkey_wmi_private, >> and use container_of() to the led_wdev pointer. >> - Replaced sizeof(struct wmi_led_args) by sizeof(led_arg) >> - Added condtions checking for obj && obj->type == ACPI_TYPE_INTEGER >> and free the ACPI object after get the required value. >> - Removed led_classdev_unregister() after led_reg_failed label, but >> add lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) to free >> resource. >> - Removed IDEAPAD_WMI_EVENT_FN_KEYS/IDEAPAD_WMI_EVENT_LUD_KEYS related >> source codes and only keep LUD_WMI_METHOD_GUID. >> >> Changes in v2: >> - Update code layout and formatting as recommended in review >> - Improved error handling in ideapad_wmi_led_init >> - Separated a WMI driver named lenovo-super-hotkey-wmi.c from >> ideapad-lap.c, it's only for Lenovo Super Hotkey WMI devices. >> >> drivers/platform/x86/Kconfig | 9 + >> drivers/platform/x86/Makefile | 1 + >> .../platform/x86/lenovo-super-hotkey-wmi.c | 248 ++++++++++++++++++ >> 3 files changed, 258 insertions(+) >> create mode 100644 drivers/platform/x86/lenovo-super-hotkey-wmi.c >> >> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig >> index 0258dd879d64..c1792e8f04e1 100644 >> --- a/drivers/platform/x86/Kconfig >> +++ b/drivers/platform/x86/Kconfig >> @@ -475,6 +475,15 @@ config IDEAPAD_LAPTOP >> This is a driver for Lenovo IdeaPad netbooks contains drivers for >> rfkill switch, hotkey, fan control and backlight control. >> >> +config LENOVO_SUPER_HOTKEY_WMI >> + tristate "Lenovo Super Hotkey Utility WMI extras driver" >> + depends on ACPI_WMI >> + depends on IDEAPAD_LAPTOP >> + help >> + This driver provides WMI support for Lenovo customized hotkeys function >> + of Lenovo NoteBooks which are for Consumer and SMB customers, such as >> + Ideapad/YOGA/XiaoXin/Gaming/ThinkBook and so on. >> + >> config LENOVO_YMC >> tristate "Lenovo Yoga Tablet Mode Control" >> depends on ACPI_WMI >> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile >> index e1b142947067..598335da9f55 100644 >> --- a/drivers/platform/x86/Makefile >> +++ b/drivers/platform/x86/Makefile >> @@ -61,6 +61,7 @@ obj-$(CONFIG_UV_SYSFS) += uv_sysfs.o >> # IBM Thinkpad and Lenovo >> obj-$(CONFIG_IBM_RTL) += ibm_rtl.o >> obj-$(CONFIG_IDEAPAD_LAPTOP) += ideapad-laptop.o >> +obj-$(CONFIG_LENOVO_SUPER_HOTKEY_WMI) += lenovo-super-hotkey-wmi.o >> obj-$(CONFIG_LENOVO_YMC) += lenovo-ymc.o >> obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o >> obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o >> diff --git a/drivers/platform/x86/lenovo-super-hotkey-wmi.c b/drivers/platform/x86/lenovo-super-hotkey-wmi.c >> new file mode 100644 >> index 000000000000..e677bd11fa2f >> --- /dev/null >> +++ b/drivers/platform/x86/lenovo-super-hotkey-wmi.c >> @@ -0,0 +1,248 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop >> + * >> + * Copyright (C) 2025 Lenovo >> + */ >> + >> +#include <linux/leds.h> >> +#include <linux/wmi.h> >> +#include "ideapad-laptop.h" >> + >> +/* Lenovo Super Hotkey WMI GUIDs */ >> +#define LUD_WMI_METHOD_GUID "CE6C0974-0407-4F50-88BA-4FC3B6559AD8" >> + >> +/* Lenovo Utility Data WMI method_id */ >> +#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 >> + >> +/* Input parameters to mute/unmute audio LED and Mic LED */ >> +struct wmi_led_args { >> + u8 id; >> + u8 subid; >> + u16 value; >> +}; >> + >> +/* Values of input parameters to SetFeature of audio LED and Mic LED */ >> +enum hotkey_set_feature { >> + MIC_MUTE_LED_ON = 1, >> + MIC_MUTE_LED_OFF, >> + AUDIO_MUTE_LED_ON = 4, >> + AUDIO_MUTE_LED_OFF, >> +}; >> + >> +#define LSH_ACPI_LED_MAX 2 >> + >> +enum lenovo_super_hotkey_wmi_event_type { >> + LSH_WMI_EVENT_LUD_KEYS = 2, >> +}; >> + >> +struct lenovo_super_hotkey_wmi_private { >> + enum lenovo_super_hotkey_wmi_event_type event; >> + struct led_classdev cdev[LSH_ACPI_LED_MAX]; >> + struct wmi_device *led_wdev; >> +}; >> + >> +enum mute_led_type { >> + MIC_MUTE, >> + AUDIO_MUTE, >> +}; >> + >> +static int lsh_wmi_mute_led_set(enum mute_led_type led_type, struct led_classdev *led_cdev, >> + enum led_brightness brightness) >> + >> +{ >> + struct lenovo_super_hotkey_wmi_private *wpriv = container_of(led_cdev, >> + struct lenovo_super_hotkey_wmi_private, cdev[led_type]); >> + struct wmi_led_args led_arg = {0, 0, 0}; >> + struct acpi_buffer input; >> + acpi_status status; >> + >> + switch (led_type) { >> + case MIC_MUTE: >> + led_arg.id = brightness == LED_ON ? MIC_MUTE_LED_ON : MIC_MUTE_LED_OFF; >> + break; >> + case AUDIO_MUTE: >> + led_arg.id = brightness == LED_ON ? AUDIO_MUTE_LED_ON : AUDIO_MUTE_LED_OFF; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + input.length = sizeof(led_arg); >> + input.pointer = &led_arg; >> + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_SET_FEATURE, &input, NULL); >> + if (ACPI_FAILURE(status)) >> + return -EIO; >> + >> + return 0; >> +} >> + >> +static int lsh_wmi_audiomute_led_set(struct led_classdev *led_cdev, >> + enum led_brightness brightness) >> + >> +{ >> + return lsh_wmi_mute_led_set(AUDIO_MUTE, led_cdev, brightness); >> +} >> + >> +static int lsh_wmi_micmute_led_set(struct led_classdev *led_cdev, >> + enum led_brightness brightness) >> +{ >> + return lsh_wmi_mute_led_set(MIC_MUTE, led_cdev, brightness); >> +} >> + >> +static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct device *dev) >> +{ >> + struct lenovo_super_hotkey_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; >> + >> + switch (led_type) { >> + case MIC_MUTE: >> + wmiarg = WMI_LUD_GET_MICMUTE_LED_VER; >> + break; >> + case AUDIO_MUTE: >> + wmiarg = WMI_LUD_GET_AUDIOMUTE_LED_VER; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + input.length = sizeof(wmiarg); >> + input.pointer = &wmiarg; >> + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_GET_SUPPORT, &input, &output); >> + if (ACPI_FAILURE(status)) >> + return -EIO; >> + >> + obj = output.pointer; >> + if (obj && obj->type == ACPI_TYPE_INTEGER) { >> + led_version = obj->integer.value; >> + kfree(output.pointer); > > Maybe you should use <linux/cleanup.h> instead, to avoid memory bugs. > Something like: > > union acpi_object *obj __free(kfree) = out.pointer > >> + } else { >> + err = -EIO; >> + goto led_error; >> + } >> + >> + wpriv->cdev[led_type].max_brightness = LED_ON; >> + wpriv->cdev[led_type].dev = dev; > > The .dev field in led_classdev is where the class device is stored by > the LED core. Drivers should not set this. > >> + wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME; >> + >> + switch (led_type) { >> + case MIC_MUTE: >> + if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) { >> + err = -EIO; >> + goto led_error; > > Double free. > >> + } >> + 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"; >> + >> + err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]); >> + if (err < 0) >> + goto led_reg_failed; > > Double free. > >> + >> + break; >> + case AUDIO_MUTE: >> + if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) { >> + err = -EIO; >> + goto led_error; > > Double free. > >> + } >> + wpriv->cdev[led_type].name = "platform::mute"; >> + wpriv->cdev[led_type].brightness_set_blocking = &lsh_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) >> + goto led_reg_failed; > > Double free. > >> + >> + break; >> + default: >> + err = -EINVAL; >> + dev_err(dev, "Unknown LED type %d\n", led_type); >> + goto led_error; > > Double free. > >> + } >> + >> + return 0; >> + >> +led_reg_failed: >> + dev_err(dev, "Could not register mute LED %d : %d\n", led_type, err); >> + >> +led_error: >> + kfree(obj); >> + return err; >> +} >> + >> +static void lenovo_super_hotkey_wmi_leds_setup(struct device *dev) >> +{ >> + lenovo_super_hotkey_wmi_led_init(MIC_MUTE, dev); >> + lenovo_super_hotkey_wmi_led_init(AUDIO_MUTE, dev); >> +} >> + >> +static int lenovo_super_hotkey_wmi_probe(struct wmi_device *wdev, const void *context) >> +{ >> + struct lenovo_super_hotkey_wmi_private *wpriv; >> + >> + wpriv = devm_kzalloc(&wdev->dev, sizeof(*wpriv), GFP_KERNEL); >> + if (!wpriv) >> + return -ENOMEM; >> + >> + *wpriv = *(const struct lenovo_super_hotkey_wmi_private *)context; > > The .event context is only used inside this function, so I believe this > copy is unnecessary. > >> + >> + dev_set_drvdata(&wdev->dev, wpriv); >> + >> + if (wpriv->event == LSH_WMI_EVENT_LUD_KEYS) { > > This check is maybe unnecessary. > >> + wpriv->led_wdev = wdev; >> + lenovo_super_hotkey_wmi_leds_setup(&wdev->dev); >> + } >> + >> + return 0; >> +} >> + >> +static void lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) >> +{ >> + struct lenovo_super_hotkey_wmi_private *wpriv = dev_get_drvdata(&wdev->dev); >> + unsigned int i; >> + >> + for (i = 0; i < LSH_ACPI_LED_MAX; i++) >> + led_classdev_unregister(&wpriv->cdev[i]); > > devm_led_classdev_register() was used so this is not needed. > >> + >> + kfree(wpriv); > > devm_kzalloc() was used to allocate wpriv so this is not needed and > probably double free. > > -- > ~ Kurt Hi Kurt, Thanks for your detail comments and professional guide. I have updated them based on your comments and sent mail with the version 4 patch, pls review it again. Jackie Dong > >> +} >> +static const struct lenovo_super_hotkey_wmi_private lsk_wmi_context_lud_keys = { >> + .event = LSH_WMI_EVENT_LUD_KEYS >> +}; >> + >> +static const struct wmi_device_id lenovo_super_hotkey_wmi_id_table[] = { >> + { LUD_WMI_METHOD_GUID, &lsk_wmi_context_lud_keys }, /* Utility data */ >> + { } >> +}; >> + >> +MODULE_DEVICE_TABLE(wmi, lenovo_super_hotkey_wmi_id_table); >> + >> +static struct wmi_driver lenovo_super_hotkey_wmi_driver = { >> + .driver = { >> + .name = "lenovo_super_hotkey_wmi", >> + .probe_type = PROBE_PREFER_ASYNCHRONOUS >> + }, >> + .id_table = lenovo_super_hotkey_wmi_id_table, >> + .probe = lenovo_super_hotkey_wmi_probe, >> + .remove = lenovo_super_hotkey_wmi_remove, >> + .no_singleton = true, >> +}; >> + >> +module_wmi_driver(lenovo_super_hotkey_wmi_driver); >> + >> +MODULE_INFO(depends, "wmi,ideapad-laptop"); >> +MODULE_AUTHOR("Jackie Dong <dongeg1@lenovo.com>"); >> +MODULE_DESCRIPTION("Lenovo Super Hotkey Utility WMI extras driver"); >> +MODULE_LICENSE("GPL"); >
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 0258dd879d64..c1792e8f04e1 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -475,6 +475,15 @@ config IDEAPAD_LAPTOP This is a driver for Lenovo IdeaPad netbooks contains drivers for rfkill switch, hotkey, fan control and backlight control. +config LENOVO_SUPER_HOTKEY_WMI + tristate "Lenovo Super Hotkey Utility WMI extras driver" + depends on ACPI_WMI + depends on IDEAPAD_LAPTOP + help + This driver provides WMI support for Lenovo customized hotkeys function + of Lenovo NoteBooks which are for Consumer and SMB customers, such as + Ideapad/YOGA/XiaoXin/Gaming/ThinkBook and so on. + config LENOVO_YMC tristate "Lenovo Yoga Tablet Mode Control" depends on ACPI_WMI diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index e1b142947067..598335da9f55 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -61,6 +61,7 @@ obj-$(CONFIG_UV_SYSFS) += uv_sysfs.o # IBM Thinkpad and Lenovo obj-$(CONFIG_IBM_RTL) += ibm_rtl.o obj-$(CONFIG_IDEAPAD_LAPTOP) += ideapad-laptop.o +obj-$(CONFIG_LENOVO_SUPER_HOTKEY_WMI) += lenovo-super-hotkey-wmi.o obj-$(CONFIG_LENOVO_YMC) += lenovo-ymc.o obj-$(CONFIG_SENSORS_HDAPS) += hdaps.o obj-$(CONFIG_THINKPAD_ACPI) += thinkpad_acpi.o diff --git a/drivers/platform/x86/lenovo-super-hotkey-wmi.c b/drivers/platform/x86/lenovo-super-hotkey-wmi.c new file mode 100644 index 000000000000..e677bd11fa2f --- /dev/null +++ b/drivers/platform/x86/lenovo-super-hotkey-wmi.c @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Lenovo Super Hotkey Utility WMI extras driver for Ideapad laptop + * + * Copyright (C) 2025 Lenovo + */ + +#include <linux/leds.h> +#include <linux/wmi.h> +#include "ideapad-laptop.h" + +/* Lenovo Super Hotkey WMI GUIDs */ +#define LUD_WMI_METHOD_GUID "CE6C0974-0407-4F50-88BA-4FC3B6559AD8" + +/* Lenovo Utility Data WMI method_id */ +#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 + +/* Input parameters to mute/unmute audio LED and Mic LED */ +struct wmi_led_args { + u8 id; + u8 subid; + u16 value; +}; + +/* Values of input parameters to SetFeature of audio LED and Mic LED */ +enum hotkey_set_feature { + MIC_MUTE_LED_ON = 1, + MIC_MUTE_LED_OFF, + AUDIO_MUTE_LED_ON = 4, + AUDIO_MUTE_LED_OFF, +}; + +#define LSH_ACPI_LED_MAX 2 + +enum lenovo_super_hotkey_wmi_event_type { + LSH_WMI_EVENT_LUD_KEYS = 2, +}; + +struct lenovo_super_hotkey_wmi_private { + enum lenovo_super_hotkey_wmi_event_type event; + struct led_classdev cdev[LSH_ACPI_LED_MAX]; + struct wmi_device *led_wdev; +}; + +enum mute_led_type { + MIC_MUTE, + AUDIO_MUTE, +}; + +static int lsh_wmi_mute_led_set(enum mute_led_type led_type, struct led_classdev *led_cdev, + enum led_brightness brightness) + +{ + struct lenovo_super_hotkey_wmi_private *wpriv = container_of(led_cdev, + struct lenovo_super_hotkey_wmi_private, cdev[led_type]); + struct wmi_led_args led_arg = {0, 0, 0}; + struct acpi_buffer input; + acpi_status status; + + switch (led_type) { + case MIC_MUTE: + led_arg.id = brightness == LED_ON ? MIC_MUTE_LED_ON : MIC_MUTE_LED_OFF; + break; + case AUDIO_MUTE: + led_arg.id = brightness == LED_ON ? AUDIO_MUTE_LED_ON : AUDIO_MUTE_LED_OFF; + break; + default: + return -EINVAL; + } + + input.length = sizeof(led_arg); + input.pointer = &led_arg; + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_SET_FEATURE, &input, NULL); + if (ACPI_FAILURE(status)) + return -EIO; + + return 0; +} + +static int lsh_wmi_audiomute_led_set(struct led_classdev *led_cdev, + enum led_brightness brightness) + +{ + return lsh_wmi_mute_led_set(AUDIO_MUTE, led_cdev, brightness); +} + +static int lsh_wmi_micmute_led_set(struct led_classdev *led_cdev, + enum led_brightness brightness) +{ + return lsh_wmi_mute_led_set(MIC_MUTE, led_cdev, brightness); +} + +static int lenovo_super_hotkey_wmi_led_init(enum mute_led_type led_type, struct device *dev) +{ + struct lenovo_super_hotkey_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; + + switch (led_type) { + case MIC_MUTE: + wmiarg = WMI_LUD_GET_MICMUTE_LED_VER; + break; + case AUDIO_MUTE: + wmiarg = WMI_LUD_GET_AUDIOMUTE_LED_VER; + break; + default: + return -EINVAL; + } + + input.length = sizeof(wmiarg); + input.pointer = &wmiarg; + status = wmidev_evaluate_method(wpriv->led_wdev, 0, WMI_LUD_GET_SUPPORT, &input, &output); + if (ACPI_FAILURE(status)) + return -EIO; + + obj = output.pointer; + if (obj && obj->type == ACPI_TYPE_INTEGER) { + led_version = obj->integer.value; + kfree(output.pointer); + } else { + err = -EIO; + goto led_error; + } + + wpriv->cdev[led_type].max_brightness = LED_ON; + wpriv->cdev[led_type].dev = dev; + wpriv->cdev[led_type].flags = LED_CORE_SUSPENDRESUME; + + switch (led_type) { + case MIC_MUTE: + if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) { + err = -EIO; + goto led_error; + } + 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"; + + err = devm_led_classdev_register(dev, &wpriv->cdev[led_type]); + if (err < 0) + goto led_reg_failed; + + break; + case AUDIO_MUTE: + if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) { + err = -EIO; + goto led_error; + } + wpriv->cdev[led_type].name = "platform::mute"; + wpriv->cdev[led_type].brightness_set_blocking = &lsh_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) + goto led_reg_failed; + + break; + default: + err = -EINVAL; + dev_err(dev, "Unknown LED type %d\n", led_type); + goto led_error; + } + + return 0; + +led_reg_failed: + dev_err(dev, "Could not register mute LED %d : %d\n", led_type, err); + +led_error: + kfree(obj); + return err; +} + +static void lenovo_super_hotkey_wmi_leds_setup(struct device *dev) +{ + lenovo_super_hotkey_wmi_led_init(MIC_MUTE, dev); + lenovo_super_hotkey_wmi_led_init(AUDIO_MUTE, dev); +} + +static int lenovo_super_hotkey_wmi_probe(struct wmi_device *wdev, const void *context) +{ + struct lenovo_super_hotkey_wmi_private *wpriv; + + wpriv = devm_kzalloc(&wdev->dev, sizeof(*wpriv), GFP_KERNEL); + if (!wpriv) + return -ENOMEM; + + *wpriv = *(const struct lenovo_super_hotkey_wmi_private *)context; + + dev_set_drvdata(&wdev->dev, wpriv); + + if (wpriv->event == LSH_WMI_EVENT_LUD_KEYS) { + wpriv->led_wdev = wdev; + lenovo_super_hotkey_wmi_leds_setup(&wdev->dev); + } + + return 0; +} + +static void lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) +{ + struct lenovo_super_hotkey_wmi_private *wpriv = dev_get_drvdata(&wdev->dev); + unsigned int i; + + for (i = 0; i < LSH_ACPI_LED_MAX; i++) + led_classdev_unregister(&wpriv->cdev[i]); + + kfree(wpriv); +} +static const struct lenovo_super_hotkey_wmi_private lsk_wmi_context_lud_keys = { + .event = LSH_WMI_EVENT_LUD_KEYS +}; + +static const struct wmi_device_id lenovo_super_hotkey_wmi_id_table[] = { + { LUD_WMI_METHOD_GUID, &lsk_wmi_context_lud_keys }, /* Utility data */ + { } +}; + +MODULE_DEVICE_TABLE(wmi, lenovo_super_hotkey_wmi_id_table); + +static struct wmi_driver lenovo_super_hotkey_wmi_driver = { + .driver = { + .name = "lenovo_super_hotkey_wmi", + .probe_type = PROBE_PREFER_ASYNCHRONOUS + }, + .id_table = lenovo_super_hotkey_wmi_id_table, + .probe = lenovo_super_hotkey_wmi_probe, + .remove = lenovo_super_hotkey_wmi_remove, + .no_singleton = true, +}; + +module_wmi_driver(lenovo_super_hotkey_wmi_driver); + +MODULE_INFO(depends, "wmi,ideapad-laptop"); +MODULE_AUTHOR("Jackie Dong <dongeg1@lenovo.com>"); +MODULE_DESCRIPTION("Lenovo Super Hotkey Utility WMI extras driver"); +MODULE_LICENSE("GPL");
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 Signed-off-by: Jackie Dong <xy-jackie@139.com> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca> --- Changes in v3: - Changed the name of the Kconfig entry to LENOVO_SUPER_HOTKEY_WMI - Renamed everything in this driver which contains the name "ideapad" to instead contain the name of this driver. - Moved struct wmi_device *led_wdev in lenovo_super_hotkey_wmi_private, and use container_of() to the led_wdev pointer. - Replaced sizeof(struct wmi_led_args) by sizeof(led_arg) - Added condtions checking for obj && obj->type == ACPI_TYPE_INTEGER and free the ACPI object after get the required value. - Removed led_classdev_unregister() after led_reg_failed label, but add lenovo_super_hotkey_wmi_remove(struct wmi_device *wdev) to free resource. - Removed IDEAPAD_WMI_EVENT_FN_KEYS/IDEAPAD_WMI_EVENT_LUD_KEYS related source codes and only keep LUD_WMI_METHOD_GUID. Changes in v2: - Update code layout and formatting as recommended in review - Improved error handling in ideapad_wmi_led_init - Separated a WMI driver named lenovo-super-hotkey-wmi.c from ideapad-lap.c, it's only for Lenovo Super Hotkey WMI devices. drivers/platform/x86/Kconfig | 9 + drivers/platform/x86/Makefile | 1 + .../platform/x86/lenovo-super-hotkey-wmi.c | 248 ++++++++++++++++++ 3 files changed, 258 insertions(+) create mode 100644 drivers/platform/x86/lenovo-super-hotkey-wmi.c