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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ed209k6l.wl-tiwai@suse.de>
Date: Sun, 22 Dec 2024 09:49:22 +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 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@...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.

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