[PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.

[PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.

[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

Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Jackie Dong <xy-jackie@139.com>
Signed-off-by: Jackie Dong <dongeg1@lenovo.com>
---
 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);
-- 
2.34.1

[PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs

[Jackie Dong]

New ideapad helper file with support for handling FN key mute LEDS.
Update conexant and realtec codec to add LED support.

Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Signed-off-by: Jackie Dong  <xy-jackie@139.com>
Signed-off-by: Jackie Dong  <dongeg1@lenovo.com>
---
 sound/pci/hda/ideapad_hotkey_led_helper.c | 36 +++++++++++++++++++++++
 sound/pci/hda/patch_conexant.c            | 10 +++++++
 sound/pci/hda/patch_realtek.c             | 20 +++++++++++++
 3 files changed, 66 insertions(+)
 create mode 100644 sound/pci/hda/ideapad_hotkey_led_helper.c

diff --git a/sound/pci/hda/ideapad_hotkey_led_helper.c b/sound/pci/hda/ideapad_hotkey_led_helper.c
new file mode 100644
index 000000000000..e49765304cc0
--- /dev/null
+++ b/sound/pci/hda/ideapad_hotkey_led_helper.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Ideapad helper functions for Lenovo Ideapad LED control,
+ * It should be included from codec driver.
+ */
+
+#if IS_ENABLED(CONFIG_IDEAPAD_LAPTOP)
+
+#include <linux/acpi.h>
+#include <linux/leds.h>
+
+static bool is_ideapad(struct hda_codec *codec)
+{
+	return (codec->core.subsystem_id >> 16 == 0x17aa) &&
+	       (acpi_dev_found("LHK2019") || acpi_dev_found("VPC2004"));
+}
+
+static void hda_fixup_ideapad_acpi(struct hda_codec *codec,
+				   const struct hda_fixup *fix, int action)
+{
+	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
+		if (!is_ideapad(codec))
+			return;
+		snd_hda_gen_add_mute_led_cdev(codec, NULL);
+		snd_hda_gen_add_micmute_led_cdev(codec, NULL);
+	}
+}
+
+#else /* CONFIG_IDEAPAD_LAPTOP */
+
+static void hda_fixup_ideapad_acpi(struct hda_codec *codec,
+				   const struct hda_fixup *fix, int action)
+{
+}
+
+#endif /* CONFIG_IDEAPAD_LAPTOP */
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 538c37a78a56..127f9a9565c9 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -291,6 +291,7 @@ enum {
 	CXT_FIXUP_GPIO1,
 	CXT_FIXUP_ASPIRE_DMIC,
 	CXT_FIXUP_THINKPAD_ACPI,
+	CXT_FIXUP_IDEAPAD_ACPI,
 	CXT_FIXUP_OLPC_XO,
 	CXT_FIXUP_CAP_MIX_AMP,
 	CXT_FIXUP_TOSHIBA_P105,
@@ -313,6 +314,9 @@ enum {
 /* for hda_fixup_thinkpad_acpi() */
 #include "thinkpad_helper.c"
 
+/* for hda_fixup_ideapad_acpi() */
+#include "ideapad_hotkey_led_helper.c"
+
 static void cxt_fixup_stereo_dmic(struct hda_codec *codec,
 				  const struct hda_fixup *fix, int action)
 {
@@ -928,6 +932,10 @@ static const struct hda_fixup cxt_fixups[] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = hda_fixup_thinkpad_acpi,
 	},
+	[CXT_FIXUP_IDEAPAD_ACPI] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = hda_fixup_ideapad_acpi,
+	},
 	[CXT_FIXUP_OLPC_XO] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = cxt_fixup_olpc_xo,
@@ -1120,6 +1128,7 @@ static const struct hda_quirk cxt5066_fixups[] = {
 	SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo G50-70", CXT_FIXUP_STEREO_DMIC),
 	SND_PCI_QUIRK(0x17aa, 0x397b, "Lenovo S205", CXT_FIXUP_STEREO_DMIC),
 	SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", CXT_FIXUP_THINKPAD_ACPI),
+	SND_PCI_QUIRK_VENDOR(0x17aa, "IdeaPad", CXT_FIXUP_IDEAPAD_ACPI),
 	SND_PCI_QUIRK(0x1c06, 0x2011, "Lemote A1004", CXT_PINCFG_LEMOTE_A1004),
 	SND_PCI_QUIRK(0x1c06, 0x2012, "Lemote A1205", CXT_PINCFG_LEMOTE_A1205),
 	HDA_CODEC_QUIRK(0x2782, 0x12c3, "Sirius Gen1", CXT_PINCFG_TOP_SPEAKER),
@@ -1133,6 +1142,7 @@ static const struct hda_model_fixup cxt5066_fixup_models[] = {
 	{ .id = CXT_FIXUP_HEADPHONE_MIC_PIN, .name = "headphone-mic-pin" },
 	{ .id = CXT_PINCFG_LENOVO_TP410, .name = "tp410" },
 	{ .id = CXT_FIXUP_THINKPAD_ACPI, .name = "thinkpad" },
+	{ .id = CXT_FIXUP_IDEAPAD_ACPI, .name = "ideapad" },
 	{ .id = CXT_PINCFG_LEMOTE_A1004, .name = "lemote-a1004" },
 	{ .id = CXT_PINCFG_LEMOTE_A1205, .name = "lemote-a1205" },
 	{ .id = CXT_FIXUP_OLPC_XO, .name = "olpc-xo" },
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 61ba5dc35b8b..3b82be7469a8 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6934,6 +6934,16 @@ static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
 	hda_fixup_thinkpad_acpi(codec, fix, action);
 }
 
+/* for hda_fixup_ideapad_acpi() */
+#include "ideapad_hotkey_led_helper.c"
+
+static void alc_fixup_ideapad_acpi(struct hda_codec *codec,
+				   const struct hda_fixup *fix, int action)
+{
+	alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
+	hda_fixup_ideapad_acpi(codec, fix, action);
+}
+
 /* Fixup for Lenovo Legion 15IMHg05 speaker output on headset removal. */
 static void alc287_fixup_legion_15imhg05_speakers(struct hda_codec *codec,
 						  const struct hda_fixup *fix,
@@ -7556,6 +7566,7 @@ enum {
 	ALC290_FIXUP_SUBWOOFER,
 	ALC290_FIXUP_SUBWOOFER_HSJACK,
 	ALC269_FIXUP_THINKPAD_ACPI,
+	ALC269_FIXUP_IDEAPAD_ACPI,
 	ALC269_FIXUP_DMIC_THINKPAD_ACPI,
 	ALC269VB_FIXUP_INFINIX_ZERO_BOOK_13,
 	ALC269VC_FIXUP_INFINIX_Y4_MAX,
@@ -8327,6 +8338,12 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC269_FIXUP_SKU_IGNORE,
 	},
+	[ALC269_FIXUP_IDEAPAD_ACPI] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc_fixup_ideapad_acpi,
+		.chained = true,
+		.chain_id = ALC269_FIXUP_SKU_IGNORE,
+	},
 	[ALC269_FIXUP_DMIC_THINKPAD_ACPI] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc_fixup_inv_dmic,
@@ -10897,6 +10914,7 @@ static const struct hda_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x17aa, 0x3880, "Yoga S780-16 pro dual YC", ALC287_FIXUP_TAS2781_I2C),
 	SND_PCI_QUIRK(0x17aa, 0x3881, "YB9 dual power mode2 YC", ALC287_FIXUP_TAS2781_I2C),
 	SND_PCI_QUIRK(0x17aa, 0x3882, "Lenovo Yoga Pro 7 14APH8", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),
+	SND_PCI_QUIRK_VENDOR(0x17aa, "IdeaPad", ALC269_FIXUP_IDEAPAD_ACPI),
 	SND_PCI_QUIRK(0x17aa, 0x3884, "Y780 YG DUAL", ALC287_FIXUP_TAS2781_I2C),
 	SND_PCI_QUIRK(0x17aa, 0x3886, "Y780 VECO DUAL", ALC287_FIXUP_TAS2781_I2C),
 	SND_PCI_QUIRK(0x17aa, 0x3891, "Lenovo Yoga Pro 7 14AHP9", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),
@@ -11130,6 +11148,7 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
 	{.id = ALC290_FIXUP_MONO_SPEAKERS_HSJACK, .name = "mono-speakers"},
 	{.id = ALC290_FIXUP_SUBWOOFER_HSJACK, .name = "alc290-subwoofer"},
 	{.id = ALC269_FIXUP_THINKPAD_ACPI, .name = "thinkpad"},
+	{.id = ALC269_FIXUP_IDEAPAD_ACPI, .name = "ideapad-led"},
 	{.id = ALC269_FIXUP_DMIC_THINKPAD_ACPI, .name = "dmic-thinkpad"},
 	{.id = ALC255_FIXUP_ACER_MIC_NO_PRESENCE, .name = "alc255-acer"},
 	{.id = ALC255_FIXUP_ASUS_MIC_NO_PRESENCE, .name = "alc255-asus"},
-- 
2.34.1

Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.

[Ilpo Järvinen]

On Thu, 19 Dec 2024, 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
> 
> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Jackie Dong <xy-jackie@139.com>
> Signed-off-by: Jackie Dong <dongeg1@lenovo.com>

One signed off is enough from you. :-) Please use the one matching to 
From: address.

If you want mails automatically to some other address too, you can always 
add a Cc: tag so the git send-email and various other tools will included 
that email address too.

> ---
>  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
> + */

Fits to one line.

> +struct wmi_led_args {
> +	u8 ID;
> +	u8 SubID;
> +	u16 Value;

Use only lowercase in variable names.

> +};
> +
>  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)

Hmm, so you fix the math bug (2-1 is not 2 but 1) with that +1 in the end?

I think you would want something like this here (but I'm not entirely 
sure at this point of reading your change):

FIELD_GET(SNDRV_CTL_ELEM_ACCESS_MIC_LED, SNDRV_CTL_ELEM_ACCESS_MIC_LED)

(Remember to make sure you've include for FIELD_GET if that's the correct 
way to go here).

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

Use named defines instead of literals.

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

Don't leave empty line between call and its error handling.

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

This seems to init only a single LED at a time but the function name says 
"leds" which is plural.

> +{
> +	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;

Use switch/case/default

> +
> +	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);

Is this kfree() correct thing to do in case of error??

> +		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) {

These blocks too should use switch.

> +		if (led_version != WMI_LUD_SUPPORT_MICMUTE_LED_VER) {
> +			dev_info(dev, "This product doesn't support mic mute LED.\n");

If this is expected to trigger on some HW, it seems nuisance noise in the 
log for such machine.

Do you have a memleak here?

> +			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]);

This unregister could be put to a rollback path and you goto there. That 
way, both leds can share the unregister call.

> +		}
> +	} else {
> +		if (led_version != WMI_LUD_SUPPORT_AUDIOMUTE_LED_VER) {
> +			dev_info(dev, "This product doesn't support audio mute LED.\n");

Same here.

> +			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 */

How is this change related to adding LEDs ?

You can always do a patch series if you want to change unrelated things.

> +	{ "CE6C0974-0407-4F50-88BA-4FC3B6559AD8", &ideapad_wmi_context_LUD_keys }, /* Util data */
> +
>  	{},
>  };
>  MODULE_DEVICE_TABLE(wmi, ideapad_wmi_ids);
> 

-- 
 i.

Re: [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs

[Takashi Iwai]

On Thu, 19 Dec 2024 11:15:31 +0100,
Jackie Dong wrote:
> 
> New ideapad helper file with support for handling FN key mute LEDS.
> Update conexant and realtec codec to add LED support.
> 
> Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Jackie Dong  <xy-jackie@139.com>
> Signed-off-by: Jackie Dong  <dongeg1@lenovo.com>
> ---
>  sound/pci/hda/ideapad_hotkey_led_helper.c | 36 +++++++++++++++++++++++
>  sound/pci/hda/patch_conexant.c            | 10 +++++++
>  sound/pci/hda/patch_realtek.c             | 20 +++++++++++++
>  3 files changed, 66 insertions(+)
>  create mode 100644 sound/pci/hda/ideapad_hotkey_led_helper.c
> 
> diff --git a/sound/pci/hda/ideapad_hotkey_led_helper.c b/sound/pci/hda/ideapad_hotkey_led_helper.c
> new file mode 100644
> index 000000000000..e49765304cc0
> --- /dev/null
> +++ b/sound/pci/hda/ideapad_hotkey_led_helper.c
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Ideapad helper functions for Lenovo Ideapad LED control,
> + * It should be included from codec driver.
> + */
> +
> +#if IS_ENABLED(CONFIG_IDEAPAD_LAPTOP)
> +
> +#include <linux/acpi.h>
> +#include <linux/leds.h>
> +
> +static bool is_ideapad(struct hda_codec *codec)
> +{
> +	return (codec->core.subsystem_id >> 16 == 0x17aa) &&
> +	       (acpi_dev_found("LHK2019") || acpi_dev_found("VPC2004"));
> +}
> +
> +static void hda_fixup_ideapad_acpi(struct hda_codec *codec,
> +				   const struct hda_fixup *fix, int action)
> +{
> +	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> +		if (!is_ideapad(codec))
> +			return;
> +		snd_hda_gen_add_mute_led_cdev(codec, NULL);
> +		snd_hda_gen_add_micmute_led_cdev(codec, NULL);
> +	}
> +}
> +
> +#else /* CONFIG_IDEAPAD_LAPTOP */
> +
> +static void hda_fixup_ideapad_acpi(struct hda_codec *codec,
> +				   const struct hda_fixup *fix, int action)
> +{
> +}
> +
> +#endif /* CONFIG_IDEAPAD_LAPTOP */
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index 538c37a78a56..127f9a9565c9 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -291,6 +291,7 @@ enum {
>  	CXT_FIXUP_GPIO1,
>  	CXT_FIXUP_ASPIRE_DMIC,
>  	CXT_FIXUP_THINKPAD_ACPI,
> +	CXT_FIXUP_IDEAPAD_ACPI,
>  	CXT_FIXUP_OLPC_XO,
>  	CXT_FIXUP_CAP_MIX_AMP,
>  	CXT_FIXUP_TOSHIBA_P105,
> @@ -313,6 +314,9 @@ enum {
>  /* for hda_fixup_thinkpad_acpi() */
>  #include "thinkpad_helper.c"
>  
> +/* for hda_fixup_ideapad_acpi() */
> +#include "ideapad_hotkey_led_helper.c"
> +
>  static void cxt_fixup_stereo_dmic(struct hda_codec *codec,
>  				  const struct hda_fixup *fix, int action)
>  {
> @@ -928,6 +932,10 @@ static const struct hda_fixup cxt_fixups[] = {
>  		.type = HDA_FIXUP_FUNC,
>  		.v.func = hda_fixup_thinkpad_acpi,
>  	},
> +	[CXT_FIXUP_IDEAPAD_ACPI] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = hda_fixup_ideapad_acpi,
> +	},
>  	[CXT_FIXUP_OLPC_XO] = {
>  		.type = HDA_FIXUP_FUNC,
>  		.v.func = cxt_fixup_olpc_xo,
> @@ -1120,6 +1128,7 @@ static const struct hda_quirk cxt5066_fixups[] = {
>  	SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo G50-70", CXT_FIXUP_STEREO_DMIC),
>  	SND_PCI_QUIRK(0x17aa, 0x397b, "Lenovo S205", CXT_FIXUP_STEREO_DMIC),
>  	SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", CXT_FIXUP_THINKPAD_ACPI),
> +	SND_PCI_QUIRK_VENDOR(0x17aa, "IdeaPad", CXT_FIXUP_IDEAPAD_ACPI),

I'm afraid that this doesn't work.  The former entry with the vendor
17aa already hits and it's used as the matched quirk.

You'd need to create a chained quirk entry instead.

>  	SND_PCI_QUIRK(0x1c06, 0x2011, "Lemote A1004", CXT_PINCFG_LEMOTE_A1004),
>  	SND_PCI_QUIRK(0x1c06, 0x2012, "Lemote A1205", CXT_PINCFG_LEMOTE_A1205),
>  	HDA_CODEC_QUIRK(0x2782, 0x12c3, "Sirius Gen1", CXT_PINCFG_TOP_SPEAKER),
> @@ -1133,6 +1142,7 @@ static const struct hda_model_fixup cxt5066_fixup_models[] = {
>  	{ .id = CXT_FIXUP_HEADPHONE_MIC_PIN, .name = "headphone-mic-pin" },
>  	{ .id = CXT_PINCFG_LENOVO_TP410, .name = "tp410" },
>  	{ .id = CXT_FIXUP_THINKPAD_ACPI, .name = "thinkpad" },
> +	{ .id = CXT_FIXUP_IDEAPAD_ACPI, .name = "ideapad" },
>  	{ .id = CXT_PINCFG_LEMOTE_A1004, .name = "lemote-a1004" },
>  	{ .id = CXT_PINCFG_LEMOTE_A1205, .name = "lemote-a1205" },
>  	{ .id = CXT_FIXUP_OLPC_XO, .name = "olpc-xo" },
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 61ba5dc35b8b..3b82be7469a8 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6934,6 +6934,16 @@ static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
>  	hda_fixup_thinkpad_acpi(codec, fix, action);
>  }
>  
> +/* for hda_fixup_ideapad_acpi() */
> +#include "ideapad_hotkey_led_helper.c"
> +
> +static void alc_fixup_ideapad_acpi(struct hda_codec *codec,
> +				   const struct hda_fixup *fix, int action)
> +{
> +	alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
> +	hda_fixup_ideapad_acpi(codec, fix, action);
> +}
> +
>  /* Fixup for Lenovo Legion 15IMHg05 speaker output on headset removal. */
>  static void alc287_fixup_legion_15imhg05_speakers(struct hda_codec *codec,
>  						  const struct hda_fixup *fix,
> @@ -7556,6 +7566,7 @@ enum {
>  	ALC290_FIXUP_SUBWOOFER,
>  	ALC290_FIXUP_SUBWOOFER_HSJACK,
>  	ALC269_FIXUP_THINKPAD_ACPI,
> +	ALC269_FIXUP_IDEAPAD_ACPI,
>  	ALC269_FIXUP_DMIC_THINKPAD_ACPI,
>  	ALC269VB_FIXUP_INFINIX_ZERO_BOOK_13,
>  	ALC269VC_FIXUP_INFINIX_Y4_MAX,
> @@ -8327,6 +8338,12 @@ static const struct hda_fixup alc269_fixups[] = {
>  		.chained = true,
>  		.chain_id = ALC269_FIXUP_SKU_IGNORE,
>  	},
> +	[ALC269_FIXUP_IDEAPAD_ACPI] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = alc_fixup_ideapad_acpi,
> +		.chained = true,
> +		.chain_id = ALC269_FIXUP_SKU_IGNORE,
> +	},
>  	[ALC269_FIXUP_DMIC_THINKPAD_ACPI] = {
>  		.type = HDA_FIXUP_FUNC,
>  		.v.func = alc_fixup_inv_dmic,
> @@ -10897,6 +10914,7 @@ static const struct hda_quirk alc269_fixup_tbl[] = {
>  	SND_PCI_QUIRK(0x17aa, 0x3880, "Yoga S780-16 pro dual YC", ALC287_FIXUP_TAS2781_I2C),
>  	SND_PCI_QUIRK(0x17aa, 0x3881, "YB9 dual power mode2 YC", ALC287_FIXUP_TAS2781_I2C),
>  	SND_PCI_QUIRK(0x17aa, 0x3882, "Lenovo Yoga Pro 7 14APH8", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),
> +	SND_PCI_QUIRK_VENDOR(0x17aa, "IdeaPad", ALC269_FIXUP_IDEAPAD_ACPI),
>  	SND_PCI_QUIRK(0x17aa, 0x3884, "Y780 YG DUAL", ALC287_FIXUP_TAS2781_I2C),
>  	SND_PCI_QUIRK(0x17aa, 0x3886, "Y780 VECO DUAL", ALC287_FIXUP_TAS2781_I2C),
>  	SND_PCI_QUIRK(0x17aa, 0x3891, "Lenovo Yoga Pro 7 14AHP9", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),

Also, this doesn't look right.  The vendor match should be put after
all Lenovo entries.  I guess the similar chained quirk is needed like
for Conexant.


thanks,

Takashi

Re: [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs

[Takashi Iwai]

On Fri, 20 Dec 2024 11:22:40 +0100,
Takashi Iwai wrote:
> 
> On Thu, 19 Dec 2024 11:15:31 +0100,
> Jackie Dong wrote:
> > 
> > New ideapad helper file with support for handling FN key mute LEDS.
> > Update conexant and realtec codec to add LED support.
> > 
> > Suggested-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> > Signed-off-by: Jackie Dong  <xy-jackie@139.com>
> > Signed-off-by: Jackie Dong  <dongeg1@lenovo.com>
> > ---
> >  sound/pci/hda/ideapad_hotkey_led_helper.c | 36 +++++++++++++++++++++++
> >  sound/pci/hda/patch_conexant.c            | 10 +++++++
> >  sound/pci/hda/patch_realtek.c             | 20 +++++++++++++
> >  3 files changed, 66 insertions(+)
> >  create mode 100644 sound/pci/hda/ideapad_hotkey_led_helper.c
> > 
> > diff --git a/sound/pci/hda/ideapad_hotkey_led_helper.c b/sound/pci/hda/ideapad_hotkey_led_helper.c
> > new file mode 100644
> > index 000000000000..e49765304cc0
> > --- /dev/null
> > +++ b/sound/pci/hda/ideapad_hotkey_led_helper.c
> > @@ -0,0 +1,36 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Ideapad helper functions for Lenovo Ideapad LED control,
> > + * It should be included from codec driver.
> > + */
> > +
> > +#if IS_ENABLED(CONFIG_IDEAPAD_LAPTOP)
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/leds.h>
> > +
> > +static bool is_ideapad(struct hda_codec *codec)
> > +{
> > +	return (codec->core.subsystem_id >> 16 == 0x17aa) &&
> > +	       (acpi_dev_found("LHK2019") || acpi_dev_found("VPC2004"));
> > +}
> > +
> > +static void hda_fixup_ideapad_acpi(struct hda_codec *codec,
> > +				   const struct hda_fixup *fix, int action)
> > +{
> > +	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> > +		if (!is_ideapad(codec))
> > +			return;
> > +		snd_hda_gen_add_mute_led_cdev(codec, NULL);
> > +		snd_hda_gen_add_micmute_led_cdev(codec, NULL);
> > +	}
> > +}
> > +
> > +#else /* CONFIG_IDEAPAD_LAPTOP */
> > +
> > +static void hda_fixup_ideapad_acpi(struct hda_codec *codec,
> > +				   const struct hda_fixup *fix, int action)
> > +{
> > +}
> > +
> > +#endif /* CONFIG_IDEAPAD_LAPTOP */
> > diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> > index 538c37a78a56..127f9a9565c9 100644
> > --- a/sound/pci/hda/patch_conexant.c
> > +++ b/sound/pci/hda/patch_conexant.c
> > @@ -291,6 +291,7 @@ enum {
> >  	CXT_FIXUP_GPIO1,
> >  	CXT_FIXUP_ASPIRE_DMIC,
> >  	CXT_FIXUP_THINKPAD_ACPI,
> > +	CXT_FIXUP_IDEAPAD_ACPI,
> >  	CXT_FIXUP_OLPC_XO,
> >  	CXT_FIXUP_CAP_MIX_AMP,
> >  	CXT_FIXUP_TOSHIBA_P105,
> > @@ -313,6 +314,9 @@ enum {
> >  /* for hda_fixup_thinkpad_acpi() */
> >  #include "thinkpad_helper.c"
> >  
> > +/* for hda_fixup_ideapad_acpi() */
> > +#include "ideapad_hotkey_led_helper.c"
> > +
> >  static void cxt_fixup_stereo_dmic(struct hda_codec *codec,
> >  				  const struct hda_fixup *fix, int action)
> >  {
> > @@ -928,6 +932,10 @@ static const struct hda_fixup cxt_fixups[] = {
> >  		.type = HDA_FIXUP_FUNC,
> >  		.v.func = hda_fixup_thinkpad_acpi,
> >  	},
> > +	[CXT_FIXUP_IDEAPAD_ACPI] = {
> > +		.type = HDA_FIXUP_FUNC,
> > +		.v.func = hda_fixup_ideapad_acpi,
> > +	},
> >  	[CXT_FIXUP_OLPC_XO] = {
> >  		.type = HDA_FIXUP_FUNC,
> >  		.v.func = cxt_fixup_olpc_xo,
> > @@ -1120,6 +1128,7 @@ static const struct hda_quirk cxt5066_fixups[] = {
> >  	SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo G50-70", CXT_FIXUP_STEREO_DMIC),
> >  	SND_PCI_QUIRK(0x17aa, 0x397b, "Lenovo S205", CXT_FIXUP_STEREO_DMIC),
> >  	SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", CXT_FIXUP_THINKPAD_ACPI),
> > +	SND_PCI_QUIRK_VENDOR(0x17aa, "IdeaPad", CXT_FIXUP_IDEAPAD_ACPI),
> 
> I'm afraid that this doesn't work.  The former entry with the vendor
> 17aa already hits and it's used as the matched quirk.
> 
> You'd need to create a chained quirk entry instead.

That is, add a new enum CXT_FIXUP_LENOVO_XPAD_ACPI, and define an
entry like:

	[CXT_FIXUP_LENOVO_XPAD_ACPI] = {
		.type = HDA_FIXUP_FUNC,
		.v.func = hda_fixup_ideapad_acpi,
		.chained = true,
		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
	},

then replace the line
 	SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", CXT_FIXUP_THINKPAD_ACPI),
with
 	SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad/Ideapad", CXT_FIXUP_LENOVO_XPAD_ACPI),

Care to rewrite the patch in that way, and resubmit v2 patch?


thanks,

Takashi

Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.

[Jaroslav Kysela]

On 19. 12. 24 12:40, Ilpo Järvinen wrote:
> On Thu, 19 Dec 2024, Jackie Dong wrote:

...

>> +};
>> +
>>   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)
> 
> Hmm, so you fix the math bug (2-1 is not 2 but 1) with that +1 in the end?
> 
> I think you would want something like this here (but I'm not entirely
> sure at this point of reading your change):
> 
> FIELD_GET(SNDRV_CTL_ELEM_ACCESS_MIC_LED, SNDRV_CTL_ELEM_ACCESS_MIC_LED)
> 
> (Remember to make sure you've include for FIELD_GET if that's the correct
> way to go here).

There's no reason to use SNDRV_CTL_ELEM_ACCESS definitions here (no direct 
connection to the sound control API). I would use direct value 2 here, because 
this extension controls only 2 LEDs.

						Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.

[Armin Wolf]

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@squebb.ca>
> Signed-off-by: Jackie Dong <xy-jackie@139.com>
> Signed-off-by: Jackie Dong <dongeg1@lenovo.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);

RE: [External] Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.

[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

-----Original Message-----
From: Armin Wolf <W_Armin@gmx.de>
Sent: Monday, December 23, 2024 6:34 AM
To: Jackie Dong <xy-jackie@139.com>; ike.pan@canonical.com; hdegoede@redhat.com; ilpo.jarvinen@linux.intel.com; perex@perex.cz; tiwai@suse.com; bo.liu@senarytech.com; kovalev@altlinux.org; me@oldherl.one; jaroslaw.janik@gmail.com; cs@tuxedo.de; songxiebing@kylinos.cn; kailang@realtek.com; sbinding@opensource.cirrus.com; simont@opensource.cirrus.com; josh@joshuagrisham.com; rf@opensource.cirrus.com
Cc: linux-kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; linux-sound@vger.kernel.org; mpearson-lenovo@squebb.ca; waterflowdeg@gmail.com; Jackie EG1 Dong <dongeg1@lenovo.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@squebb.ca>
> Signed-off-by: Jackie Dong <xy-jackie@139.com>
> Signed-off-by: Jackie Dong <dongeg1@lenovo.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);

Re: [External] Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.

[Armin Wolf]

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@gmx.de>
> Sent: Monday, December 23, 2024 6:34 AM
> To: Jackie Dong <xy-jackie@139.com>; ike.pan@canonical.com; hdegoede@redhat.com; ilpo.jarvinen@linux.intel.com; perex@perex.cz; tiwai@suse.com; bo.liu@senarytech.com; kovalev@altlinux.org; me@oldherl.one; jaroslaw.janik@gmail.com; cs@tuxedo.de; songxiebing@kylinos.cn; kailang@realtek.com; sbinding@opensource.cirrus.com; simont@opensource.cirrus.com; josh@joshuagrisham.com; rf@opensource.cirrus.com
> Cc: linux-kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; linux-sound@vger.kernel.org; mpearson-lenovo@squebb.ca; waterflowdeg@gmail.com; Jackie EG1 Dong <dongeg1@lenovo.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@squebb.ca>
>> Signed-off-by: Jackie Dong <xy-jackie@139.com>
>> Signed-off-by: Jackie Dong <dongeg1@lenovo.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);

Re: [External] Re: [PATCH 1/2] platform/x86: ideapad-laptop: Supportfor mic and audio leds.

[Jackie Dong]

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@gmx.de>
>> Sent: Monday, December 23, 2024 6:34 AM
>> To: Jackie Dong <xy-jackie@139.com>; ike.pan@canonical.com; 
>> hdegoede@redhat.com; ilpo.jarvinen@linux.intel.com; perex@perex.cz; 
>> tiwai@suse.com; bo.liu@senarytech.com; kovalev@altlinux.org; 
>> me@oldherl.one; jaroslaw.janik@gmail.com; cs@tuxedo.de; 
>> songxiebing@kylinos.cn; kailang@realtek.com; 
>> sbinding@opensource.cirrus.com; simont@opensource.cirrus.com; 
>> josh@joshuagrisham.com; rf@opensource.cirrus.com
>> Cc: linux-kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; 
>> linux-sound@vger.kernel.org; mpearson-lenovo@squebb.ca; 
>> waterflowdeg@gmail.com; Jackie EG1 Dong <dongeg1@lenovo.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@squebb.ca>
>>> Signed-off-by: Jackie Dong <xy-jackie@139.com>
>>> Signed-off-by: Jackie Dong <dongeg1@lenovo.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);