[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <87v7veu00f.wl-tiwai@suse.de>
Date: Fri, 20 Dec 2024 11:22:40 +0100
From: Takashi Iwai <tiwai@...e.de>
To: Jackie Dong <xy-jackie@....com>
Cc: ike.pan@...onical.com,
hdegoede@...hat.com,
ilpo.jarvinen@...ux.intel.com,
perex@...ex.cz,
tiwai@...e.com,
bo.liu@...arytech.com,
kovalev@...linux.org,
me@...herl.one,
jaroslaw.janik@...il.com,
cs@...edo.de,
songxiebing@...inos.cn,
kailang@...ltek.com,
sbinding@...nsource.cirrus.com,
simont@...nsource.cirrus.com,
josh@...huagrisham.com,
rf@...nsource.cirrus.com,
linux-kernel@...r.kernel.org,
platform-driver-x86@...r.kernel.org,
linux-sound@...r.kernel.org,
mpearson-lenovo@...ebb.ca,
waterflowdeg@...il.com,
Jackie Dong <dongeg1@...ovo.com>
Subject: Re: [PATCH 2/2] ALSA : hda : Support for Ideapad hotkey mute LEDs
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@...ebb.ca>
> Signed-off-by: Jackie Dong <xy-jackie@....com>
> Signed-off-by: Jackie Dong <dongeg1@...ovo.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
Powered by blists - more mailing lists