* [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs
2024-12-19 10:15 [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds Jackie Dong
@ 2024-12-19 10:15 ` Jackie Dong
2024-12-20 10:22 ` Takashi Iwai
2024-12-19 11:40 ` [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds Ilpo Järvinen
2024-12-22 22:34 ` Armin Wolf
2 siblings, 1 reply; 10+ messages in thread
From: Jackie Dong @ 2024-12-19 10:15 UTC (permalink / raw)
To: ike.pan, hdegoede, ilpo.jarvinen, perex, tiwai, bo.liu, kovalev,
me, jaroslaw.janik, cs, songxiebing, kailang, sbinding, simont,
josh, rf
Cc: linux-kernel, platform-driver-x86, linux-sound, mpearson-lenovo,
waterflowdeg, Jackie Dong, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs
2024-12-19 10:15 ` [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs Jackie Dong
@ 2024-12-20 10:22 ` Takashi Iwai
2024-12-22 8:49 ` Takashi Iwai
0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2024-12-20 10:22 UTC (permalink / raw)
To: Jackie Dong
Cc: ike.pan, hdegoede, ilpo.jarvinen, perex, tiwai, bo.liu, kovalev,
me, jaroslaw.janik, cs, songxiebing, kailang, sbinding, simont,
josh, rf, linux-kernel, platform-driver-x86, linux-sound,
mpearson-lenovo, waterflowdeg, Jackie Dong
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs
2024-12-20 10:22 ` Takashi Iwai
@ 2024-12-22 8:49 ` Takashi Iwai
0 siblings, 0 replies; 10+ messages in thread
From: Takashi Iwai @ 2024-12-22 8:49 UTC (permalink / raw)
To: Jackie Dong
Cc: ike.pan, hdegoede, ilpo.jarvinen, perex, tiwai, bo.liu, kovalev,
me, jaroslaw.janik, cs, songxiebing, kailang, sbinding, simont,
josh, rf, linux-kernel, platform-driver-x86, linux-sound,
mpearson-lenovo, waterflowdeg, Jackie Dong
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.
2024-12-19 10:15 [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds Jackie Dong
2024-12-19 10:15 ` [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs Jackie Dong
@ 2024-12-19 11:40 ` Ilpo Järvinen
2024-12-22 13:30 ` Jaroslav Kysela
2024-12-22 22:34 ` Armin Wolf
2 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2024-12-19 11:40 UTC (permalink / raw)
To: Jackie Dong
Cc: ike.pan, Hans de Goede, perex, tiwai, bo.liu, kovalev, me,
jaroslaw.janik, cs, songxiebing, kailang, sbinding, simont, josh,
rf, LKML, platform-driver-x86, linux-sound, Mark Pearson,
waterflowdeg, Jackie Dong
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.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.
2024-12-19 11:40 ` [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds Ilpo Järvinen
@ 2024-12-22 13:30 ` Jaroslav Kysela
0 siblings, 0 replies; 10+ messages in thread
From: Jaroslav Kysela @ 2024-12-22 13:30 UTC (permalink / raw)
To: Ilpo Järvinen, Jackie Dong
Cc: ike.pan, Hans de Goede, tiwai, bo.liu, kovalev, me,
jaroslaw.janik, cs, songxiebing, kailang, sbinding, simont, josh,
rf, LKML, platform-driver-x86, linux-sound, Mark Pearson,
waterflowdeg, Jackie Dong
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.
2024-12-19 10:15 [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds Jackie Dong
2024-12-19 10:15 ` [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs Jackie Dong
2024-12-19 11:40 ` [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds Ilpo Järvinen
@ 2024-12-22 22:34 ` Armin Wolf
2024-12-24 9:29 ` [External] " Jackie EG1 Dong
2 siblings, 1 reply; 10+ messages in thread
From: Armin Wolf @ 2024-12-22 22:34 UTC (permalink / raw)
To: Jackie Dong, ike.pan, hdegoede, ilpo.jarvinen, perex, tiwai,
bo.liu, kovalev, me, jaroslaw.janik, cs, songxiebing, kailang,
sbinding, simont, josh, rf
Cc: linux-kernel, platform-driver-x86, linux-sound, mpearson-lenovo,
waterflowdeg, Jackie Dong
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);
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [External] Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.
2024-12-22 22:34 ` Armin Wolf
@ 2024-12-24 9:29 ` Jackie EG1 Dong
2024-12-24 13:09 ` Armin Wolf
0 siblings, 1 reply; 10+ messages in thread
From: Jackie EG1 Dong @ 2024-12-24 9:29 UTC (permalink / raw)
To: Armin Wolf, Jackie Dong, 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
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);
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [External] Re: [PATCH 1/2] platform/x86: ideapad-laptop: Support for mic and audio leds.
2024-12-24 9:29 ` [External] " Jackie EG1 Dong
@ 2024-12-24 13:09 ` Armin Wolf
2024-12-25 5:03 ` [External] Re: [PATCH 1/2] platform/x86: ideapad-laptop: Supportfor " Jackie Dong
0 siblings, 1 reply; 10+ messages in thread
From: Armin Wolf @ 2024-12-24 13:09 UTC (permalink / raw)
To: Jackie EG1 Dong, Jackie Dong, 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
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);
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [External] Re: [PATCH 1/2] platform/x86: ideapad-laptop: Supportfor mic and audio leds.
2024-12-24 13:09 ` Armin Wolf
@ 2024-12-25 5:03 ` Jackie Dong
0 siblings, 0 replies; 10+ messages in thread
From: Jackie Dong @ 2024-12-25 5:03 UTC (permalink / raw)
To: Armin Wolf, Jackie EG1 Dong, 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
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);
^ permalink raw reply [flat|nested] 10+ messages in thread