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

[PATCH v2] 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>
---
Changes in v2:
 - Add a new enum CXT_FIXUP_LENOVO_XPAD_ACPI and define it as an entry in patch_conexant.c
 - Add a new enum ALC269_FIXUP_LENOVO_XPAD_ACPI and define it as an entry in patch_realtek.c

 sound/pci/hda/ideapad_hotkey_led_helper.c | 36 +++++++++++++++++++++++
 sound/pci/hda/patch_conexant.c            | 13 +++++++-
 sound/pci/hda/patch_realtek.c             | 20 ++++++++++++-
 3 files changed, 67 insertions(+), 2 deletions(-)
 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..c10d97964d49
--- /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..4985e72b9094 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_LENOVO_XPAD_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,12 @@ static const struct hda_fixup cxt_fixups[] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = hda_fixup_thinkpad_acpi,
 	},
+	[CXT_FIXUP_LENOVO_XPAD_ACPI] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = hda_fixup_ideapad_acpi,
+		.chained = true,
+		.chain_id = CXT_FIXUP_THINKPAD_ACPI,
+	},
 	[CXT_FIXUP_OLPC_XO] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = cxt_fixup_olpc_xo,
@@ -1119,7 +1129,7 @@ static const struct hda_quirk cxt5066_fixups[] = {
 	SND_PCI_QUIRK(0x17aa, 0x3977, "Lenovo IdeaPad U310", CXT_FIXUP_STEREO_DMIC),
 	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, "Thinkpad/Ideapad", CXT_FIXUP_LENOVO_XPAD_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 +1143,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_LENOVO_XPAD_ACPI, .name = "thinkpad-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..5f9927401322 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_LENOVO_XPAD_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_LENOVO_XPAD_ACPI] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc_fixup_ideapad_acpi,
+		.chained = true,
+		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
+	},
 	[ALC269_FIXUP_DMIC_THINKPAD_ACPI] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc_fixup_inv_dmic,
@@ -11065,7 +11082,7 @@ static const struct hda_quirk alc269_fixup_vendor_tbl[] = {
 	SND_PCI_QUIRK_VENDOR(0x1025, "Acer Aspire", ALC271_FIXUP_DMIC),
 	SND_PCI_QUIRK_VENDOR(0x103c, "HP", ALC269_FIXUP_HP_MUTE_LED),
 	SND_PCI_QUIRK_VENDOR(0x104d, "Sony VAIO", ALC269_FIXUP_SONY_VAIO),
-	SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", ALC269_FIXUP_THINKPAD_ACPI),
+	SND_PCI_QUIRK_VENDOR(0x17aa, "Lenovo XPAD", ALC269_FIXUP_LENOVO_XPAD_ACPI),
 	SND_PCI_QUIRK_VENDOR(0x19e5, "Huawei Matebook", ALC255_FIXUP_MIC_MUTE_LED),
 	{}
 };
@@ -11130,6 +11147,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_LENOVO_XPAD_ACPI, .name = "lenovo xpad 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 v2] ALSA: hda: Support for Ideapad hotkey mute LEDs

[Takashi Iwai]

On Tue, 24 Dec 2024 09:33:16 +0100,
Jackie Dong wrote:
> 
> --- 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);
> +}

So this unconditionally call alc_fixup_no_shutup(), and this
introduces another behavior to the existing entry -- i.e. there is a
chance of breakage.

If we want to be very conservative, this call should be limited to
Ideapad.


thanks,

Takashi

RE: [External] Re: [PATCH v2] ALSA: hda: Support for Ideapad hotkey mute LEDs

[Jackie EG1 Dong]

> On Tue, 24 Dec 2024 09:33:16 +0100,
 > Jackie Dong wrote:
 >>
 >> --- 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);
 >> +}
 >
 > So this unconditionally call alc_fixup_no_shutup(), and this  > introduces another behavior to the existing entry -- i.e. there is a  > chance of breakage.
 >
 > If we want to be very conservative, this call should be limited to  > Ideapad.
 > For alc_fixup_no_shutup(codec, fix, action),
 I want to keep same behavior with alc_fixup_thinkpad_apci() and alc_fixup_idea_acpi() for one sound card. So, I add alc_fixup_no_shutup() in alc_fixup_ideapad_acpi().
----------Related source code of patch_reatek.c are FYR as below.
static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
                                     const struct hda_fixup *fix, int
action)
{
         alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
         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);
}
Thanks, 
Jackie
> thanks, 
>  
> Takashi  
>

Re: [External] Re: [PATCH v2] ALSA: hda: Support for Ideapad hotkey mute LEDs

[Takashi Iwai]

On Mon, 30 Dec 2024 01:33:01 +0100,
Jackie EG1 Dong wrote:
> 
> > On Tue, 24 Dec 2024 09:33:16 +0100,
>  > Jackie Dong wrote:
>  >>
>  >> --- 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);
>  >> +}
>  >
>  > So this unconditionally call alc_fixup_no_shutup(), and this  > introduces another behavior to the existing entry -- i.e. there is a  > chance of breakage.
>  >
>  > If we want to be very conservative, this call should be limited to  > Ideapad.
>  > For alc_fixup_no_shutup(codec, fix, action),
>  I want to keep same behavior with alc_fixup_thinkpad_apci() and alc_fixup_idea_acpi() for one sound card. So, I add alc_fixup_no_shutup() in alc_fixup_ideapad_acpi().
> ----------Related source code of patch_reatek.c are FYR as below.
> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
>                                      const struct hda_fixup *fix, int
> action)
> {
>          alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
>          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);
> }

Oh yeah, but then it can be bad in other way round; the chain call of
alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the
alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup().
That is, alc_fixup_no_shutup() will be called twice for Thinkpad.

Instead, you can change like:

--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6925,11 +6925,16 @@ static void alc285_fixup_hp_envy_x360(struct hda_codec *codec,
 /* for hda_fixup_thinkpad_acpi() */
 #include "thinkpad_helper.c"
 
-static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
-				    const struct hda_fixup *fix, int action)
+/* for hda_fixup_ideapad_acpi() */
+#include "ideapad_hotkey_led_helper.c"
+
+/* generic fixup for both Lenovo Thinkpad and Ideapad */
+static void alc_fixup_xpad_acpi(struct hda_codec *codec,
+				const struct hda_fixup *fix, int action)
 {
 	alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
 	hda_fixup_thinkpad_acpi(codec, fix, action);
+	hda_fixup_ideapad_acpi(codec, fix, action);
 }
 
 /* Fixup for Lenovo Legion 15IMHg05 speaker output on headset removal. */
@@ -8321,7 +8326,7 @@ static const struct hda_fixup alc269_fixups[] = {
 	},
 	[ALC269_FIXUP_THINKPAD_ACPI] = {
 		.type = HDA_FIXUP_FUNC,
-		.v.func = alc_fixup_thinkpad_acpi,
+		.v.func = alc_fixup_xpad_acpi,
 		.chained = true,
 		.chain_id = ALC269_FIXUP_SKU_IGNORE,
 	},

Since hda_fixup_ideapad_acpi() is NOP except for Ideapad, this
shouldn't break other models, while it covers the Ideadpad now.


thanks,

Takashi

Re: [External] Re: [PATCH v2] ALSA: hda: Support for Ideapad hotkey mute LEDs

[Jackie Dong]

On 2025/1/3 23:17, Takashi Iwai wrote:
> On Mon, 30 Dec 2024 01:33:01 +0100,
> Jackie EG1 Dong wrote:
>>
>>> On Tue, 24 Dec 2024 09:33:16 +0100,
>>   > Jackie Dong wrote:
>>   >>
>>   >> --- 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);
>>   >> +}
>>   >
>>   > So this unconditionally call alc_fixup_no_shutup(), and this  > introduces another behavior to the existing entry -- i.e. there is a  > chance of breakage.
>>   >
>>   > If we want to be very conservative, this call should be limited to  > Ideapad.
>>   > For alc_fixup_no_shutup(codec, fix, action),
>>   I want to keep same behavior with alc_fixup_thinkpad_apci() and alc_fixup_idea_acpi() for one sound card. So, I add alc_fixup_no_shutup() in alc_fixup_ideapad_acpi().
>> ----------Related source code of patch_reatek.c are FYR as below.
>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
>>                                       const struct hda_fixup *fix, int
>> action)
>> {
>>           alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
>>           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);
>> }
> 
> Oh yeah, but then it can be bad in other way round; the chain call of
> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the
> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup().
> That is, alc_fixup_no_shutup() will be called twice for Thinkpad.
> 
Many thanks to Takashi for your detail comments  and sample code, I 
understand it now.

I'll check the logic of the code and update the patch later.

Best Regards,

Jackie Dong
> Instead, you can change like:
> 
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6925,11 +6925,16 @@ static void alc285_fixup_hp_envy_x360(struct hda_codec *codec,
>   /* for hda_fixup_thinkpad_acpi() */
>   #include "thinkpad_helper.c"
>   
> -static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
> -				    const struct hda_fixup *fix, int action)
> +/* for hda_fixup_ideapad_acpi() */
> +#include "ideapad_hotkey_led_helper.c"
> +
> +/* generic fixup for both Lenovo Thinkpad and Ideapad */
> +static void alc_fixup_xpad_acpi(struct hda_codec *codec,
> +				const struct hda_fixup *fix, int action)
>   {
>   	alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
>   	hda_fixup_thinkpad_acpi(codec, fix, action);
> +	hda_fixup_ideapad_acpi(codec, fix, action);
>   }
>   
>   /* Fixup for Lenovo Legion 15IMHg05 speaker output on headset removal. */
> @@ -8321,7 +8326,7 @@ static const struct hda_fixup alc269_fixups[] = {
>   	},
>   	[ALC269_FIXUP_THINKPAD_ACPI] = {
>   		.type = HDA_FIXUP_FUNC,
> -		.v.func = alc_fixup_thinkpad_acpi,
> +		.v.func = alc_fixup_xpad_acpi,
>   		.chained = true,
>   		.chain_id = ALC269_FIXUP_SKU_IGNORE,
>   	},
> 
> Since hda_fixup_ideapad_acpi() is NOP except for Ideapad, this
> shouldn't break other models, while it covers the Ideadpad now.
> 
> 
> thanks,
> 
> Takashi

Re: [External] Re: [PATCH v2] ALSA: hda: Support for Ideapad hotkeymute LEDs

[Jackie Dong]

On 1/6/25 20:49, Jackie Dong wrote:
> On 2025/1/3 23:17, Takashi Iwai wrote:
>> On Mon, 30 Dec 2024 01:33:01 +0100,
>> Jackie EG1 Dong wrote:
>>>
>>>> On Tue, 24 Dec 2024 09:33:16 +0100,
>>>   > Jackie Dong wrote:
>>>   >>
>>>   >> --- 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);
>>>   >> +}
>>>   >
>>>   > So this unconditionally call alc_fixup_no_shutup(), and this  > 
>>> introduces another behavior to the existing entry -- i.e. there is a  
>>> > chance of breakage.
>>>   >
>>>   > If we want to be very conservative, this call should be limited 
>>> to  > Ideapad.
>>>   > For alc_fixup_no_shutup(codec, fix, action),
>>>   I want to keep same behavior with alc_fixup_thinkpad_apci() and 
>>> alc_fixup_idea_acpi() for one sound card. So, I add 
>>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi().
>>> ----------Related source code of patch_reatek.c are FYR as below.
>>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
>>>                                       const struct hda_fixup *fix, int
>>> action)
>>> {
>>>           alc_fixup_no_shutup(codec, fix, action); /* reduce click 
>>> noise */
>>>           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);
>>> }
>>
>> Oh yeah, but then it can be bad in other way round; the chain call of
>> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the
>> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup().
>> That is, alc_fixup_no_shutup() will be called twice for Thinkpad.
>>
> Many thanks to Takashi for your detail comments  and sample code, I 
> understand it now.
> 
> I'll check the logic of the code and update the patch later.
> 
> Best Regards,
> 
> Jackie Dong

Hi Takashi,
   For this function, I added three debug message in patch_realtek.c as 
below. I find alc_fixup_no_shutup() only run once, no matter it's in 
alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some 
kernel log for your reference.
   So, I think the patch is ok for this concern.
   If you have any other concern for the patch, let me know.
   Thanks for your comment and guide in past.

----------------------------------------------------------------------
./sound/pci/hda/patch_realtek.c
......
  6122 static void alc_fixup_no_shutup(struct hda_codec *codec,
  6123                                 const struct hda_fixup *fix, int 
action)
  6124 {
  6125         if (action == HDA_FIXUP_ACT_PRE_PROBE) {
  6126                 struct alc_spec *spec = codec->spec;
  6127                 spec->no_shutup_pins = 1;
  6128         }
  6129         printk("This is from 
alc_fixup_no_shutup++444444444444444444444444444444+-.\n");//Deg
  6130 }
......
6929 #include "thinkpad_helper.c"
  6930
  6931 static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
  6932                                     const struct hda_fixup *fix, 
int action)
  6933 {
  6934         alc_fixup_no_shutup(codec, fix, action); /* reduce click 
noise */ //Deg
  6935         printk("This is from 
alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.\n");//Deg
  6936         hda_fixup_thinkpad_acpi(codec, fix, action);
  6937 }
  6938
  6939 /* for hda_fixup_ideapad_acpi() */
  6940 #include "ideapad_hotkey_led_helper.c"
  6941
  6942 static void alc_fixup_ideapad_acpi(struct hda_codec *codec,
  6943                                    const struct hda_fixup *fix, 
int action)
  6944 {
  6945         alc_fixup_no_shutup(codec, fix, action); /* reduce click 
noise */ //Deg
  6946         printk("This is from 
alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.\n");//Deg
  6947         hda_fixup_ideapad_acpi(codec, fix, action);
  6948 }
......

----------------------------------------------------------------------
Some log from /var/log/kerlog of ThinkBook 16 G8 IRL.
......
2025-01-13T16:24:34.109926+08:00 test-ThinkBook-16-G8-IRL kernel: 
snd_hda_codec_realtek ehdaudio0D0: ALC257: picked fixup  for PCI SSID 
17aa:0000
2025-01-13T16:24:34.109927+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
2025-01-13T16:24:34.109927+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.
2025-01-13T16:24:34.109928+08:00 test-ThinkBook-16-G8-IRL kernel: 
snd_hda_codec_realtek ehdaudio0D0: autoconfig for ALC257: line_outs=1 
(0x14/0x0/0x0/0x0/0x0) type:speaker
2025-01-13T16:24:34.109928+08:00 test-ThinkBook-16-G8-IRL kernel: 
snd_hda_codec_realtek ehdaudio0D0:    speaker_outs=0 (0x0/0x0/0x0/0x0/0x0)
2025-01-13T16:24:34.109929+08:00 test-ThinkBook-16-G8-IRL kernel: 
snd_hda_codec_realtek ehdaudio0D0:    hp_outs=1 (0x21/0x0/0x0/0x0/0x0)
2025-01-13T16:24:34.109929+08:00 test-ThinkBook-16-G8-IRL kernel: 
snd_hda_codec_realtek ehdaudio0D0:    mono: mono_out=0x0
2025-01-13T16:24:34.109930+08:00 test-ThinkBook-16-G8-IRL kernel: 
snd_hda_codec_realtek ehdaudio0D0:    inputs:
2025-01-13T16:24:34.109930+08:00 test-ThinkBook-16-G8-IRL kernel: 
snd_hda_codec_realtek ehdaudio0D0:      Mic=0x19
2025-01-13T16:24:34.109931+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
2025-01-13T16:24:34.109931+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.
2025-01-13T16:24:34.109932+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
2025-01-13T16:24:34.109932+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.
2025-01-13T16:24:34.109933+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
2025-01-13T16:24:34.109933+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.
2025-01-13T16:24:34.109934+08:00 test-ThinkBook-16-G8-IRL kernel: 
skl_hda_dsp_generic skl_hda_dsp_generic: hda_dsp_hdmi_build_controls: no 
PCM in topology for HDMI converter 3
2025-01-13T16:24:34.109934+08:00 test-ThinkBook-16-G8-IRL kernel: 
iwlwifi 0000:00:14.3: base HW address: a8:59:5f:e8:58:dc
2025-01-13T16:24:34.109935+08:00 test-ThinkBook-16-G8-IRL kernel: input: 
sof-hda-dsp Mic as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input19
2025-01-13T16:24:34.109935+08:00 test-ThinkBook-16-G8-IRL kernel: input: 
sof-hda-dsp Headphone as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input20
2025-01-13T16:24:34.109936+08:00 test-ThinkBook-16-G8-IRL kernel: input: 
sof-hda-dsp HDMI/DP,pcm=3 as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input21
2025-01-13T16:24:34.109936+08:00 test-ThinkBook-16-G8-IRL kernel: input: 
sof-hda-dsp HDMI/DP,pcm=4 as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input22
2025-01-13T16:24:34.109937+08:00 test-ThinkBook-16-G8-IRL kernel: input: 
sof-hda-dsp HDMI/DP,pcm=5 as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input23
2025-01-13T16:24:34.109937+08:00 test-ThinkBook-16-G8-IRL kernel: 
iwlwifi 0000:00:14.3 wlp0s20f3: renamed from wlan0
2025-01-13T16:24:34.109938+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Waiting for firmware download to complete
2025-01-13T16:24:34.109938+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Firmware loaded in 1480525 usecs
2025-01-13T16:24:34.109939+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Waiting for device to boot
2025-01-13T16:24:34.109939+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Device booted in 15688 usecs
2025-01-13T16:24:34.109940+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-0040-0041.ddc
2025-01-13T16:24:34.109940+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Applying Intel DDC parameters completed
2025-01-13T16:24:34.109941+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Firmware timestamp 2023.48 buildtype 1 build 75324
2025-01-13T16:24:34.109941+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Firmware SHA1: 0x23bac558
2025-01-13T16:24:34.109942+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Fseq status: Success (0x00)
2025-01-13T16:24:34.109942+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Fseq executed: 00.00.02.41
2025-01-13T16:24:34.109942+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: hci0: Fseq BT Top: 00.00.02.41
2025-01-13T16:24:34.109943+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
2025-01-13T16:24:34.109943+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.
2025-01-13T16:24:34.109944+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: BNEP (Ethernet Emulation) ver 1.3
2025-01-13T16:24:34.109944+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: BNEP filters: protocol multicast
2025-01-13T16:24:34.109945+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: BNEP socket layer initialized
2025-01-13T16:24:34.109945+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: MGMT ver 1.23
2025-01-13T16:24:34.109946+08:00 test-ThinkBook-16-G8-IRL kernel: NET: 
Registered PF_ALG protocol family
2025-01-13T16:24:34.109946+08:00 test-ThinkBook-16-G8-IRL kernel: nvme 
nvme0: using unchecked data buffer
2025-01-13T16:24:34.132755+08:00 test-ThinkBook-16-G8-IRL kernel: NET: 
Registered PF_QIPCRTR protocol family
2025-01-13T16:24:34.796796+08:00 test-ThinkBook-16-G8-IRL kernel: 
iwlwifi 0000:00:14.3: WFPM_UMAC_PD_NOTIFICATION: 0x20
2025-01-13T16:24:34.796810+08:00 test-ThinkBook-16-G8-IRL kernel: 
iwlwifi 0000:00:14.3: WFPM_LMAC2_PD_NOTIFICATION: 0x1f
2025-01-13T16:24:34.796811+08:00 test-ThinkBook-16-G8-IRL kernel: 
iwlwifi 0000:00:14.3: WFPM_AUTH_KEY_0: 0x90
2025-01-13T16:24:34.796811+08:00 test-ThinkBook-16-G8-IRL kernel: 
iwlwifi 0000:00:14.3: CNVI_SCU_SEQ_DATA_DW9: 0x0
2025-01-13T16:24:34.796812+08:00 test-ThinkBook-16-G8-IRL kernel: 
iwlwifi 0000:00:14.3: RFIm is deactivated, reason = 4
2025-01-13T16:24:34.900941+08:00 test-ThinkBook-16-G8-IRL kernel: 
iwlwifi 0000:00:14.3: Registered PHC clock: iwlwifi-PTP, with index: 1
2025-01-13T16:24:35.092715+08:00 test-ThinkBook-16-G8-IRL kernel: 
loop13: detected capacity change from 0 to 8
2025-01-13T16:24:35.360758+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
2025-01-13T16:24:35.360768+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.
2025-01-13T16:24:35.476784+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: RFCOMM TTY layer initialized
2025-01-13T16:24:35.476792+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: RFCOMM socket layer initialized
2025-01-13T16:24:35.476793+08:00 test-ThinkBook-16-G8-IRL kernel: 
Bluetooth: RFCOMM ver 1.11
2025-01-13T16:24:36.000717+08:00 test-ThinkBook-16-G8-IRL kernel: 
rfkill: input handler disabled
2025-01-13T16:24:37.928735+08:00 test-ThinkBook-16-G8-IRL kernel: 
wlp0s20f3: authenticate with c2:95:70:cf:7f:31 (local 
address=a8:59:5f:e8:58:dc)
2025-01-13T16:24:37.928746+08:00 test-ThinkBook-16-G8-IRL kernel: 
wlp0s20f3: send auth to c2:95:70:cf:7f:31 (try 1/3)
2025-01-13T16:24:37.960811+08:00 test-ThinkBook-16-G8-IRL kernel: 
wlp0s20f3: authenticated
2025-01-13T16:24:37.960839+08:00 test-ThinkBook-16-G8-IRL kernel: 
wlp0s20f3: associate with c2:95:70:cf:7f:31 (try 1/3)
2025-01-13T16:24:37.964860+08:00 test-ThinkBook-16-G8-IRL kernel: 
wlp0s20f3: RX AssocResp from c2:95:70:cf:7f:31 (capab=0x531 status=0 aid=24)
2025-01-13T16:24:37.972839+08:00 test-ThinkBook-16-G8-IRL kernel: 
wlp0s20f3: associated
2025-01-13T16:25:20.616738+08:00 test-ThinkBook-16-G8-IRL kernel: 
kauditd_printk_skb: 156 callbacks suppressed
2025-01-13T16:25:20.616752+08:00 test-ThinkBook-16-G8-IRL kernel: audit: 
type=1400 audit(1736756720.611:168): apparmor="DENIED" 
operation="capable" class="cap" 
profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=2188 
comm="snap-confine" capability=12  capname="net_admin"
2025-01-13T16:25:20.616755+08:00 test-ThinkBook-16-G8-IRL kernel: audit: 
type=1400 audit(1736756720.611:169): apparmor="DENIED" 
operation="capable" class="cap" 
profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=2188 
comm="snap-confine" capability=38  capname="perfmon"
2025-01-13T16:25:20.860725+08:00 test-ThinkBook-16-G8-IRL kernel: 
rfkill: input handler enabled
2025-01-13T16:25:20.984744+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
2025-01-13T16:25:20.984754+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.
2025-01-13T16:25:21.588723+08:00 test-ThinkBook-16-G8-IRL kernel: 
rfkill: input handler disabled
2025-01-13T16:25:23.560752+08:00 test-ThinkBook-16-G8-IRL kernel: audit: 
type=1400 audit(1736756723.553:170): apparmor="DENIED" 
operation="capable" class="cap" 
profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=3165 
comm="snap-confine" capability=12  capname="net_admin"
2025-01-13T16:25:23.560790+08:00 test-ThinkBook-16-G8-IRL kernel: audit: 
type=1400 audit(1736756723.553:171): apparmor="DENIED" 
operation="capable" class="cap" 
profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=3165 
comm="snap-confine" capability=38  capname="perfmon"
2025-01-13T16:25:55.972879+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
2025-01-13T16:25:55.972922+08:00 test-ThinkBook-16-G8-IRL kernel: This 
is from alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.
2025-01-13T16:25:58.090951+08:00 test-ThinkBook-16-G8-IRL kernel: atkbd 
serio0: Unknown key pressed (translated set 2, code 0xac on isa0060/serio0).
......

----------------------------------------------------------------------
Some log from /var/log/kerlog of ThinkPad X1 Nano Gen1.
......
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.727147] This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.727150] This 
is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.727758] 
snd_hda_codec_realtek ehdaudio0D0: autoconfig for ALC287: line_outs=2 
(0x14/0x17/0x0/0x0/0x0) type:speaker
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.727764] 
snd_hda_codec_realtek ehdaudio0D0:    speaker_outs=0 (0x0/0x0/0x0/0x0/0x0)
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.727777] 
snd_hda_codec_realtek ehdaudio0D0:    hp_outs=1 (0x21/0x0/0x0/0x0/0x0)
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.727779] 
snd_hda_codec_realtek ehdaudio0D0:    mono: mono_out=0x0
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.727781] 
snd_hda_codec_realtek ehdaudio0D0:    inputs:
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.727782] 
snd_hda_codec_realtek ehdaudio0D0:      Mic=0x19
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.732045] This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.732059] This 
is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.775739] This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.775743] This 
is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.776998] This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.777000] This 
is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.777636] 
skl_hda_dsp_generic skl_hda_dsp_generic: hda_dsp_hdmi_build_controls: no 
PCM in topology for HDMI converter 3
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.812497] 
input: sof-hda-dsp Mic as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input23
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.812568] 
input: sof-hda-dsp Headphone as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input24
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.812826] 
input: sof-hda-dsp HDMI/DP,pcm=3 as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input25
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.812884] 
input: sof-hda-dsp HDMI/DP,pcm=4 as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input26
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.812930] 
input: sof-hda-dsp HDMI/DP,pcm=5 as 
/devices/pci0000:00/0000:00:1f.3/skl_hda_dsp_generic/sound/card0/input27
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.841455] 
Bluetooth: BNEP (Ethernet Emulation) ver 1.3
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.841460] 
Bluetooth: BNEP filters: protocol multicast
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    5.841464] 
Bluetooth: BNEP socket layer initialized
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    6.142838] 
iwlwifi 0000:00:14.3: Registered PHC clock: iwlwifi-PTP, with index: 0
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    6.241587] This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
Jan 13 16:30:16 test-ThinkPad-X1-Nano-Gen-1 kernel: [    6.241590] This 
is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.175456] 
loop14: detected capacity change from 0 to 8
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.234454] 
Bluetooth: hci0: Waiting for firmware download to complete
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.234793] 
Bluetooth: hci0: Firmware loaded in 1803474 usecs
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.234871] 
Bluetooth: hci0: Waiting for device to boot
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.249852] 
Bluetooth: hci0: Device booted in 14679 usecs
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.250205] 
Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-19-0-4.ddc
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.251900] 
Bluetooth: hci0: Applying Intel DDC parameters completed
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.252821] 
Bluetooth: hci0: Firmware revision 0.4 build 206 week 22 2023
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.254845] 
Bluetooth: hci0: HCI LE Coded PHY feature bit is set, but its usage is 
not supported.
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.319888] 
Bluetooth: MGMT ver 1.23
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.322558] NET: 
Registered PF_ALG protocol family
Jan 13 16:30:17 test-ThinkPad-X1-Nano-Gen-1 kernel: [    7.618988] 
rfkill: input handler disabled
Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [    9.150754] 
wlp0s20f3: authenticate with c2:95:70:cf:7f:31 (local 
address=dc:41:a9:86:dc:a1)
Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [    9.151962] 
wlp0s20f3: send auth to c2:95:70:cf:7f:31 (try 1/3)
Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [    9.180543] 
wlp0s20f3: authenticated
Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [    9.181813] 
wlp0s20f3: associate with c2:95:70:cf:7f:31 (try 1/3)
Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [    9.183387] 
wlp0s20f3: RX AssocResp from c2:95:70:cf:7f:31 (capab=0x531 status=0 aid=25)
Jan 13 16:30:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [    9.196275] 
wlp0s20f3: associated
Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [   29.438595] 
kauditd_printk_skb: 52 callbacks suppressed
Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [   29.438598] 
audit: type=1400 audit(1736757039.724:64): apparmor="DENIED" 
operation="capable" class="cap" 
profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=1542 
comm="snap-confine" capability=12  capname="net_admin"
Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [   29.438608] 
audit: type=1400 audit(1736757039.724:65): apparmor="DENIED" 
operation="capable" class="cap" 
profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=1542 
comm="snap-confine" capability=38  capname="perfmon"
Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [   29.484757] 
Bluetooth: RFCOMM TTY layer initialized
Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [   29.484764] 
Bluetooth: RFCOMM socket layer initialized
Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [   29.484768] 
Bluetooth: RFCOMM ver 1.11
Jan 13 16:30:39 test-ThinkPad-X1-Nano-Gen-1 kernel: [   29.660767] 
rfkill: input handler enabled
Jan 13 16:30:40 test-ThinkPad-X1-Nano-Gen-1 kernel: [   29.861572] This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
Jan 13 16:30:40 test-ThinkPad-X1-Nano-Gen-1 kernel: [   29.861577] This 
is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.
Jan 13 16:30:41 test-ThinkPad-X1-Nano-Gen-1 kernel: [   30.808052] 
rfkill: input handler disabled
Jan 13 16:30:42 test-ThinkPad-X1-Nano-Gen-1 kernel: [   32.255105] 
audit: type=1400 audit(1736757042.540:66): apparmor="DENIED" 
operation="capable" class="cap" 
profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=2163 
comm="snap-confine" capability=12  capname="net_admin"
Jan 13 16:30:42 test-ThinkPad-X1-Nano-Gen-1 kernel: [   32.255113] 
audit: type=1400 audit(1736757042.540:67): apparmor="DENIED" 
operation="capable" class="cap" 
profile="/snap/snapd/23545/usr/lib/snapd/snap-confine" pid=2163 
comm="snap-confine" capability=38  capname="perfmon"
Jan 13 16:31:35 test-ThinkPad-X1-Nano-Gen-1 kernel: [   85.593309] This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
Jan 13 16:31:35 test-ThinkPad-X1-Nano-Gen-1 kernel: [   85.593316] This 
is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.
Jan 13 19:06:02 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9352.480329] 
loop14: detected capacity change from 0 to 562880
Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.618422] 
audit: type=1400 audit(1736766365.698:68): apparmor="STATUS" 
operation="profile_replace" info="same as current profile, skipping" 
profile="unconfined" name="/snap/snapd/23545/usr/lib/snapd/snap-confine" 
pid=3116 comm="apparmor_parser"
Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.618433] 
audit: type=1400 audit(1736766365.698:69): apparmor="STATUS" 
operation="profile_replace" info="same as current profile, skipping" 
profile="unconfined" 
name="/snap/snapd/23545/usr/lib/snapd/snap-confine//mount-namespace-capture-helper" 
pid=3116 comm="apparmor_parser"
Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.730349] 
audit: type=1400 audit(1736766365.810:70): apparmor="STATUS" 
operation="profile_replace" profile="unconfined" 
name="snap.firefox.hook.configure" pid=3121 comm="apparmor_parser"
Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.764308] 
audit: type=1400 audit(1736766365.846:71): apparmor="STATUS" 
operation="profile_replace" profile="unconfined" 
name="snap.firefox.hook.post-refresh" pid=3123 comm="apparmor_parser"
Jan 13 19:06:05 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9355.766762] 
audit: type=1400 audit(1736766365.846:72): apparmor="STATUS" 
operation="profile_replace" profile="unconfined" 
name="snap.firefox.hook.disconnect-plug-host-hunspell" pid=3122 
comm="apparmor_parser"
Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.049245] 
audit: type=1400 audit(1736766366.130:73): apparmor="STATUS" 
operation="profile_replace" profile="unconfined" 
name="snap.firefox.firefox" pid=3119 comm="apparmor_parser"
Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.088026] 
audit: type=1400 audit(1736766366.170:74): apparmor="STATUS" 
operation="profile_replace" profile="unconfined" 
name="snap.firefox.geckodriver" pid=3120 comm="apparmor_parser"
Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.104996] 
audit: type=1400 audit(1736766366.186:75): apparmor="STATUS" 
operation="profile_replace" profile="unconfined" 
name="snap-update-ns.firefox" pid=3118 comm="apparmor_parser"
Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.294691] 
audit: type=1400 audit(1736766366.374:76): apparmor="STATUS" 
operation="profile_replace" info="same as current profile, skipping" 
profile="unconfined" name="/snap/snapd/23545/usr/lib/snapd/snap-confine" 
pid=3204 comm="apparmor_parser"
Jan 13 19:06:06 test-ThinkPad-X1-Nano-Gen-1 kernel: [ 9356.294696] 
audit: type=1400 audit(1736766366.374:77): apparmor="STATUS" 
operation="profile_replace" info="same as current profile, skipping" 
profile="unconfined" 
name="/snap/snapd/23545/usr/lib/snapd/snap-confine//mount-namespace-capture-helper" 
pid=3204 comm="apparmor_parser"
Jan 14 00:00:02 test-ThinkPad-X1-Nano-Gen-1 kernel: [26992.726977] 
kauditd_printk_skb: 7 callbacks suppressed
Jan 14 00:00:02 test-ThinkPad-X1-Nano-Gen-1 kernel: [26992.726981] 
audit: type=1400 audit(1736784002.633:85): apparmor="DENIED" 
operation="capable" class="cap" profile="/usr/sbin/cupsd" pid=4432 
comm="cupsd" capability=12  capname="net_admin"
Jan 14 03:54:13 test-ThinkPad-X1-Nano-Gen-1 kernel: [41043.929183] nvme 
nvme0: using unchecked data buffer
Jan 14 14:13:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [78189.824014] This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
Jan 14 14:13:19 test-ThinkPad-X1-Nano-Gen-1 kernel: [78189.824021] This 
is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.
Jan 14 14:37:32 test-ThinkPad-X1-Nano-Gen-1 kernel: [79643.222000] This 
is from alc_fixup_no_shutup++444444444444444444444444444444+-.
Jan 14 14:37:32 test-ThinkPad-X1-Nano-Gen-1 kernel: [79643.222006] This 
is from alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.
......

>> Instead, you can change like:
>>
>> --- a/sound/pci/hda/patch_realtek.c
>> +++ b/sound/pci/hda/patch_realtek.c
>> @@ -6925,11 +6925,16 @@ static void alc285_fixup_hp_envy_x360(struct 
>> hda_codec *codec,
>>   /* for hda_fixup_thinkpad_acpi() */
>>   #include "thinkpad_helper.c"
>> -static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
>> -                    const struct hda_fixup *fix, int action)
>> +/* for hda_fixup_ideapad_acpi() */
>> +#include "ideapad_hotkey_led_helper.c"
>> +
>> +/* generic fixup for both Lenovo Thinkpad and Ideapad */
>> +static void alc_fixup_xpad_acpi(struct hda_codec *codec,
>> +                const struct hda_fixup *fix, int action)
>>   {
>>       alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
>>       hda_fixup_thinkpad_acpi(codec, fix, action);
>> +    hda_fixup_ideapad_acpi(codec, fix, action);
>>   }
>>   /* Fixup for Lenovo Legion 15IMHg05 speaker output on headset 
>> removal. */
>> @@ -8321,7 +8326,7 @@ static const struct hda_fixup alc269_fixups[] = {
>>       },
>>       [ALC269_FIXUP_THINKPAD_ACPI] = {
>>           .type = HDA_FIXUP_FUNC,
>> -        .v.func = alc_fixup_thinkpad_acpi,
>> +        .v.func = alc_fixup_xpad_acpi,
>>           .chained = true,
>>           .chain_id = ALC269_FIXUP_SKU_IGNORE,
>>       },
>>
>> Since hda_fixup_ideapad_acpi() is NOP except for Ideapad, this
>> shouldn't break other models, while it covers the Ideadpad now.
>>
>>
>> thanks,
>>
>> Takashi
> 

Re: [External] Re: [PATCH v2] ALSA: hda: Support for Ideapad hotkeymute LEDs

[Takashi Iwai]

On Tue, 14 Jan 2025 07:54:01 +0100,
Jackie Dong wrote:
> 
> On 1/6/25 20:49, Jackie Dong wrote:
> > On 2025/1/3 23:17, Takashi Iwai wrote:
> >> On Mon, 30 Dec 2024 01:33:01 +0100,
> >> Jackie EG1 Dong wrote:
> >>> 
> >>>> On Tue, 24 Dec 2024 09:33:16 +0100,
> >>>   > Jackie Dong wrote:
> >>>   >>
> >>>   >> --- 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);
> >>>   >> +}
> >>>   >
> >>>   > So this unconditionally call alc_fixup_no_shutup(), and this 
> >>> > introduces another behavior to the existing entry -- i.e. there
> >>> is a  > chance of breakage.
> >>>   >
> >>>   > If we want to be very conservative, this call should be
> >>> limited to  > Ideapad.
> >>>   > For alc_fixup_no_shutup(codec, fix, action),
> >>>   I want to keep same behavior with alc_fixup_thinkpad_apci() and
> >>> alc_fixup_idea_acpi() for one sound card. So, I add
> >>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi().
> >>> ----------Related source code of patch_reatek.c are FYR as below.
> >>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
> >>>                                       const struct hda_fixup *fix, int
> >>> action)
> >>> {
> >>>           alc_fixup_no_shutup(codec, fix, action); /* reduce click
> >>> noise */
> >>>           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);
> >>> }
> >> 
> >> Oh yeah, but then it can be bad in other way round; the chain call of
> >> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the
> >> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup().
> >> That is, alc_fixup_no_shutup() will be called twice for Thinkpad.
> >> 
> > Many thanks to Takashi for your detail comments  and sample code, I
> > understand it now.
> > 
> > I'll check the logic of the code and update the patch later.
> > 
> > Best Regards,
> > 
> > Jackie Dong
> 
> Hi Takashi,
>   For this function, I added three debug message in patch_realtek.c as
> below. I find alc_fixup_no_shutup() only run once, no matter it's in
> alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some
> kernel log for your reference.
>   So, I think the patch is ok for this concern.
>   If you have any other concern for the patch, let me know.
>   Thanks for your comment and guide in past.

That's really weird.  Are you testing your v2 patch, right?
(That is, the ALC269_FIXUP_LENOVO_XPAD_ACPI entry calls
alc_fixup_ideadpad_acpi() and is chained with
ALC269_FIXUP_THINKPAD_ACPI.  If this entry is really used, it *must*
call the alc_fixup_thinkpad_acpi() as well.

Please double-check.


Takashi

Re: [External] Re: [PATCH v2] ALSA: hda: Support for Ideapad hotkeymute LEDs

[Takashi Iwai]

On Tue, 14 Jan 2025 08:19:17 +0100,
Takashi Iwai wrote:
> 
> On Tue, 14 Jan 2025 07:54:01 +0100,
> Jackie Dong wrote:
> > 
> > On 1/6/25 20:49, Jackie Dong wrote:
> > > On 2025/1/3 23:17, Takashi Iwai wrote:
> > >> On Mon, 30 Dec 2024 01:33:01 +0100,
> > >> Jackie EG1 Dong wrote:
> > >>> 
> > >>>> On Tue, 24 Dec 2024 09:33:16 +0100,
> > >>>   > Jackie Dong wrote:
> > >>>   >>
> > >>>   >> --- 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);
> > >>>   >> +}
> > >>>   >
> > >>>   > So this unconditionally call alc_fixup_no_shutup(), and this 
> > >>> > introduces another behavior to the existing entry -- i.e. there
> > >>> is a  > chance of breakage.
> > >>>   >
> > >>>   > If we want to be very conservative, this call should be
> > >>> limited to  > Ideapad.
> > >>>   > For alc_fixup_no_shutup(codec, fix, action),
> > >>>   I want to keep same behavior with alc_fixup_thinkpad_apci() and
> > >>> alc_fixup_idea_acpi() for one sound card. So, I add
> > >>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi().
> > >>> ----------Related source code of patch_reatek.c are FYR as below.
> > >>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
> > >>>                                       const struct hda_fixup *fix, int
> > >>> action)
> > >>> {
> > >>>           alc_fixup_no_shutup(codec, fix, action); /* reduce click
> > >>> noise */
> > >>>           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);
> > >>> }
> > >> 
> > >> Oh yeah, but then it can be bad in other way round; the chain call of
> > >> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the
> > >> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup().
> > >> That is, alc_fixup_no_shutup() will be called twice for Thinkpad.
> > >> 
> > > Many thanks to Takashi for your detail comments  and sample code, I
> > > understand it now.
> > > 
> > > I'll check the logic of the code and update the patch later.
> > > 
> > > Best Regards,
> > > 
> > > Jackie Dong
> > 
> > Hi Takashi,
> >   For this function, I added three debug message in patch_realtek.c as
> > below. I find alc_fixup_no_shutup() only run once, no matter it's in
> > alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some
> > kernel log for your reference.
> >   So, I think the patch is ok for this concern.
> >   If you have any other concern for the patch, let me know.
> >   Thanks for your comment and guide in past.
> 
> That's really weird.  Are you testing your v2 patch, right?
> (That is, the ALC269_FIXUP_LENOVO_XPAD_ACPI entry calls
> alc_fixup_ideadpad_acpi() and is chained with
> ALC269_FIXUP_THINKPAD_ACPI.  If this entry is really used, it *must*
> call the alc_fixup_thinkpad_acpi() as well.
> 
> Please double-check.

On the second thought, alc_fixup_no_shutup() itself is mostly harmless
even if it's called multiple times, as it sets only the flag.
But, unifying the quirk function makes more sense as it results in
smaller changes.

In anyway, the check of the alc_fixup_no_shutup() should be done
again; if a test is negative, it doesn't mean it's OK but it's
something wrong.


thanks,

Takashi

Re: [External] Re: [PATCH v2] ALSA: hda: Support for Ideapad hotkeymute LEDs

[Jackie Dong]

On 1/14/25 16:28, Takashi Iwai wrote:
> On Tue, 14 Jan 2025 08:19:17 +0100,
> Takashi Iwai wrote:
>>
>> On Tue, 14 Jan 2025 07:54:01 +0100,
>> Jackie Dong wrote:
>>>
>>> On 1/6/25 20:49, Jackie Dong wrote:
>>>> On 2025/1/3 23:17, Takashi Iwai wrote:
>>>>> On Mon, 30 Dec 2024 01:33:01 +0100,
>>>>> Jackie EG1 Dong wrote:
>>>>>>
>>>>>>> On Tue, 24 Dec 2024 09:33:16 +0100,
>>>>>>    > Jackie Dong wrote:
>>>>>>    >>
>>>>>>    >> --- 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);
>>>>>>    >> +}
>>>>>>    >
>>>>>>    > So this unconditionally call alc_fixup_no_shutup(), and this
>>>>>>> introduces another behavior to the existing entry -- i.e. there
>>>>>> is a  > chance of breakage.
>>>>>>    >
>>>>>>    > If we want to be very conservative, this call should be
>>>>>> limited to  > Ideapad.
>>>>>>    > For alc_fixup_no_shutup(codec, fix, action),
>>>>>>    I want to keep same behavior with alc_fixup_thinkpad_apci() and
>>>>>> alc_fixup_idea_acpi() for one sound card. So, I add
>>>>>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi().
>>>>>> ----------Related source code of patch_reatek.c are FYR as below.
>>>>>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
>>>>>>                                        const struct hda_fixup *fix, int
>>>>>> action)
>>>>>> {
>>>>>>            alc_fixup_no_shutup(codec, fix, action); /* reduce click
>>>>>> noise */
>>>>>>            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);
>>>>>> }
>>>>>
>>>>> Oh yeah, but then it can be bad in other way round; the chain call of
>>>>> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the
>>>>> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup().
>>>>> That is, alc_fixup_no_shutup() will be called twice for Thinkpad.
>>>>>
>>>> Many thanks to Takashi for your detail comments  and sample code, I
>>>> understand it now.
>>>>
>>>> I'll check the logic of the code and update the patch later.
>>>>
>>>> Best Regards,
>>>>
>>>> Jackie Dong
>>>
>>> Hi Takashi,
>>>    For this function, I added three debug message in patch_realtek.c as
>>> below. I find alc_fixup_no_shutup() only run once, no matter it's in
>>> alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some
>>> kernel log for your reference.
>>>    So, I think the patch is ok for this concern.
>>>    If you have any other concern for the patch, let me know.
>>>    Thanks for your comment and guide in past.
>>
>> That's really weird.  Are you testing your v2 patch, right?
>> (That is, the ALC269_FIXUP_LENOVO_XPAD_ACPI entry calls
>> alc_fixup_ideadpad_acpi() and is chained with
>> ALC269_FIXUP_THINKPAD_ACPI.  If this entry is really used, it *must*
>> call the alc_fixup_thinkpad_acpi() as well.
>>
>> Please double-check.
Hi Takashi,
   You're right.
   I commented two lines in [ALC269_FIXUP_LENOVO_XPAD_ACPI] and got the 
result of previous mail. I try to look for which funcion call 
alc_fixup_thinkpad_acpi() after add below patch. And I hope to impeleted 
the function with minimum changes.
   Many thanks,
-----------------------------------------------------------------------
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6126,6 +6126,7 @@ static void alc_fixup_no_shutup(struct hda_codec 
*codec,
                 struct alc_spec *spec = codec->spec;
                 spec->no_shutup_pins = 1;
         }
+       printk("This is from 
alc_fixup_no_shutup++444444444444444444444444444444+-.\n");//Deg
  }

  static void alc_fixup_disable_aamix(struct hda_codec *codec,
@@ -6930,10 +6931,22 @@ static void alc285_fixup_hp_envy_x360(struct 
hda_codec *codec,
  static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
                                     const struct hda_fixup *fix, int 
action)
  {
-       alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
+       alc_fixup_no_shutup(codec, fix, action); /* reduce click noise 
*/ //Deg
+       printk("This is from 
alc_fixup_no_shutup++TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.\n");//Deg
         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 
*/ //Deg
+       printk("This is from 
alc_fixup_no_shutup++IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.\n");//Deg
+       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 +7569,7 @@ enum {
         ALC290_FIXUP_SUBWOOFER,
         ALC290_FIXUP_SUBWOOFER_HSJACK,
         ALC269_FIXUP_THINKPAD_ACPI,
+       ALC269_FIXUP_LENOVO_XPAD_ACPI,
         ALC269_FIXUP_DMIC_THINKPAD_ACPI,
         ALC269VB_FIXUP_INFINIX_ZERO_BOOK_13,
         ALC269VC_FIXUP_INFINIX_Y4_MAX,
@@ -8327,6 +8341,12 @@ static const struct hda_fixup alc269_fixups[] = {
                 .chained = true,
                 .chain_id = ALC269_FIXUP_SKU_IGNORE,
         },
+       [ALC269_FIXUP_LENOVO_XPAD_ACPI] = {
+               .type = HDA_FIXUP_FUNC,
+               .v.func = alc_fixup_ideapad_acpi,
+//             .chained = true,
+//             .chain_id = ALC269_FIXUP_THINKPAD_ACPI,
+       },
         [ALC269_FIXUP_DMIC_THINKPAD_ACPI] = {
                 .type = HDA_FIXUP_FUNC,
                 .v.func = alc_fixup_inv_dmic,
@@ -11065,7 +11085,7 @@ static const struct hda_quirk 
alc269_fixup_vendor_tbl[] = {
         SND_PCI_QUIRK_VENDOR(0x1025, "Acer Aspire", ALC271_FIXUP_DMIC),
         SND_PCI_QUIRK_VENDOR(0x103c, "HP", ALC269_FIXUP_HP_MUTE_LED),
         SND_PCI_QUIRK_VENDOR(0x104d, "Sony VAIO", ALC269_FIXUP_SONY_VAIO),
-       SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", 
ALC269_FIXUP_THINKPAD_ACPI),
+       SND_PCI_QUIRK_VENDOR(0x17aa, "Lenovo XPAD", 
ALC269_FIXUP_LENOVO_XPAD_ACPI),
         SND_PCI_QUIRK_VENDOR(0x19e5, "Huawei Matebook", 
ALC255_FIXUP_MIC_MUTE_LED),
         {}
  };
@@ -11130,6 +11150,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_LENOVO_XPAD_ACPI, .name = "lenovo xpad 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.43.0


> 
> On the second thought, alc_fixup_no_shutup() itself is mostly harmless
> even if it's called multiple times, as it sets only the flag.
> But, unifying the quirk function makes more sense as it results in
> smaller changes.
> 
> In anyway, the check of the alc_fixup_no_shutup() should be done
> again; if a test is negative, it doesn't mean it's OK but it's
> something wrong.
> 
> 
> thanks,
> 
> Takashi

Re: [External] Re: [PATCH v2] ALSA: hda: Support for Ideapad hotkeymute LEDs

[Jackie Dong]

On 1/14/25 23:47, Jackie Dong wrote:
> On 1/14/25 16:28, Takashi Iwai wrote:
>> On Tue, 14 Jan 2025 08:19:17 +0100,
>> Takashi Iwai wrote:
>>>
>>> On Tue, 14 Jan 2025 07:54:01 +0100,
>>> Jackie Dong wrote:
>>>>
>>>> On 1/6/25 20:49, Jackie Dong wrote:
>>>>> On 2025/1/3 23:17, Takashi Iwai wrote:
>>>>>> On Mon, 30 Dec 2024 01:33:01 +0100,
>>>>>> Jackie EG1 Dong wrote:
>>>>>>>
>>>>>>>> On Tue, 24 Dec 2024 09:33:16 +0100,
>>>>>>>    > Jackie Dong wrote:
>>>>>>>    >>
>>>>>>>    >> --- 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);
>>>>>>>    >> +}
>>>>>>>    >
>>>>>>>    > So this unconditionally call alc_fixup_no_shutup(), and this
>>>>>>>> introduces another behavior to the existing entry -- i.e. there
>>>>>>> is a  > chance of breakage.
>>>>>>>    >
>>>>>>>    > If we want to be very conservative, this call should be
>>>>>>> limited to  > Ideapad.
>>>>>>>    > For alc_fixup_no_shutup(codec, fix, action),
>>>>>>>    I want to keep same behavior with alc_fixup_thinkpad_apci() and
>>>>>>> alc_fixup_idea_acpi() for one sound card. So, I add
>>>>>>> alc_fixup_no_shutup() in alc_fixup_ideapad_acpi().
>>>>>>> ----------Related source code of patch_reatek.c are FYR as below.
>>>>>>> static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
>>>>>>>                                        const struct hda_fixup 
>>>>>>> *fix, int
>>>>>>> action)
>>>>>>> {
>>>>>>>            alc_fixup_no_shutup(codec, fix, action); /* reduce click
>>>>>>> noise */
>>>>>>>            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);
>>>>>>> }
>>>>>>
>>>>>> Oh yeah, but then it can be bad in other way round; the chain call of
>>>>>> alc_fixup_thinkpad_acpi() contains alc_fixup_no_shutup() and the
>>>>>> alc_fixup_ideadpad_acpi() also contains alc_fixup_no_shutup().
>>>>>> That is, alc_fixup_no_shutup() will be called twice for Thinkpad.
>>>>>>
>>>>> Many thanks to Takashi for your detail comments  and sample code, I
>>>>> understand it now.
>>>>>
>>>>> I'll check the logic of the code and update the patch later.
>>>>>
>>>>> Best Regards,
>>>>>
>>>>> Jackie Dong
>>>>
>>>> Hi Takashi,
>>>>    For this function, I added three debug message in patch_realtek.c as
>>>> below. I find alc_fixup_no_shutup() only run once, no matter it's in
>>>> alc_fixup_thinkpad_acpi(), or it's in alc_fixup_ideadpad_acpi(). Some
>>>> kernel log for your reference.
>>>>    So, I think the patch is ok for this concern.
>>>>    If you have any other concern for the patch, let me know.
>>>>    Thanks for your comment and guide in past.
>>>
>>> That's really weird.  Are you testing your v2 patch, right?
>>> (That is, the ALC269_FIXUP_LENOVO_XPAD_ACPI entry calls
>>> alc_fixup_ideadpad_acpi() and is chained with
>>> ALC269_FIXUP_THINKPAD_ACPI.  If this entry is really used, it *must*
>>> call the alc_fixup_thinkpad_acpi() as well.
>>>
>>> Please double-check.
> Hi Takashi,
>    You're right.
>    I commented two lines in [ALC269_FIXUP_LENOVO_XPAD_ACPI] and got the 
> result of previous mail. I try to look for which funcion call 
> alc_fixup_thinkpad_acpi() after add below patch. And I hope to impeleted 
> the function with minimum changes.
>    Many thanks,
> -----------------------------------------------------------------------
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6126,6 +6126,7 @@ static void alc_fixup_no_shutup(struct hda_codec 
> *codec,
>                  struct alc_spec *spec = codec->spec;
>                  spec->no_shutup_pins = 1;
>          }
> +       printk("This is from alc_fixup_no_shutup+ 
> +444444444444444444444444444444+-.\n");//Deg
>   }
> 
>   static void alc_fixup_disable_aamix(struct hda_codec *codec,
> @@ -6930,10 +6931,22 @@ static void alc285_fixup_hp_envy_x360(struct 
> hda_codec *codec,
>   static void alc_fixup_thinkpad_acpi(struct hda_codec *codec,
>                                      const struct hda_fixup *fix, int 
> action)
>   {
> -       alc_fixup_no_shutup(codec, fix, action); /* reduce click noise */
> +       alc_fixup_no_shutup(codec, fix, action); /* reduce click noise 
> */ //Deg
> +       printk("This is from alc_fixup_no_shutup+ 
> +TTTTTTTTTTTTTTTTTTTTTTTTTTTTTTT+-.\n");//Deg
>          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 
> */ //Deg
> +       printk("This is from alc_fixup_no_shutup+ 
> +IIIIIIIIIIIIIIIIIIIIIIIIIIIIII+-.\n");//Deg
> +       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 +7569,7 @@ enum {
>          ALC290_FIXUP_SUBWOOFER,
>          ALC290_FIXUP_SUBWOOFER_HSJACK,
>          ALC269_FIXUP_THINKPAD_ACPI,
> +       ALC269_FIXUP_LENOVO_XPAD_ACPI,
>          ALC269_FIXUP_DMIC_THINKPAD_ACPI,
>          ALC269VB_FIXUP_INFINIX_ZERO_BOOK_13,
>          ALC269VC_FIXUP_INFINIX_Y4_MAX,
> @@ -8327,6 +8341,12 @@ static const struct hda_fixup alc269_fixups[] = {
>                  .chained = true,
>                  .chain_id = ALC269_FIXUP_SKU_IGNORE,
>          },
> +       [ALC269_FIXUP_LENOVO_XPAD_ACPI] = {
> +               .type = HDA_FIXUP_FUNC,
> +               .v.func = alc_fixup_ideapad_acpi,
> +//             .chained = true,
> +//             .chain_id = ALC269_FIXUP_THINKPAD_ACPI,
> +       },
>          [ALC269_FIXUP_DMIC_THINKPAD_ACPI] = {
>                  .type = HDA_FIXUP_FUNC,
>                  .v.func = alc_fixup_inv_dmic,
> @@ -11065,7 +11085,7 @@ static const struct hda_quirk 
> alc269_fixup_vendor_tbl[] = {
>          SND_PCI_QUIRK_VENDOR(0x1025, "Acer Aspire", ALC271_FIXUP_DMIC),
>          SND_PCI_QUIRK_VENDOR(0x103c, "HP", ALC269_FIXUP_HP_MUTE_LED),
>          SND_PCI_QUIRK_VENDOR(0x104d, "Sony VAIO", ALC269_FIXUP_SONY_VAIO),
> -       SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", 
> ALC269_FIXUP_THINKPAD_ACPI),
> +       SND_PCI_QUIRK_VENDOR(0x17aa, "Lenovo XPAD", 
> ALC269_FIXUP_LENOVO_XPAD_ACPI),
>          SND_PCI_QUIRK_VENDOR(0x19e5, "Huawei Matebook", 
> ALC255_FIXUP_MIC_MUTE_LED),
>          {}
>   };
> @@ -11130,6 +11150,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_LENOVO_XPAD_ACPI, .name = "lenovo xpad 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"},
Hi Takashi,
    I study for the logic of ALC269_FIXUP_THINKPAD_ACPI call and test 
many times. From my view, only removed alc_fixup_no_shutup() from 
alc_fixup_ideapad_acpi() is the best way which has no impact for other 
functions call ALC269_FIXUP_THINKPAD_ACPI.
    I have submitted the v3 patch which only removed 
alc_fixup_no_shutup() from alc_fixup_ideapad_acpi() to make
    sure alc_fixup_no_shutup() only be called once for Thinkpad and
    Ideapad.
    Pls review it again and give your comments.

  Best Regards,
Jackie Dong