Re: [PATCH 3/3] ALSA:hda: add support for Huawei WMI MicMute LED

[Takashi Iwai <tiwai@suse.de>]

On Wed, 31 Oct 2018 20:20:38 +0100,
Ayman Bagabas wrote:
> 
> Some of Huawei laptops come with a LED in the mic mute key. This patch
> enables and disable this LED when the internal microphone status is
> changed.
> 
> Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
> ---
>  include/linux/huawei_wmi.h        |  7 ++++
>  sound/pci/hda/huawei_wmi_helper.c | 66 +++++++++++++++++++++++++++++++
>  sound/pci/hda/patch_realtek.c     | 12 ++++++
>  3 files changed, 85 insertions(+)
>  create mode 100644 include/linux/huawei_wmi.h
>  create mode 100644 sound/pci/hda/huawei_wmi_helper.c
> 
> diff --git a/include/linux/huawei_wmi.h b/include/linux/huawei_wmi.h
> new file mode 100644
> index 000000000000..69b656c5029b
> --- /dev/null
> +++ b/include/linux/huawei_wmi.h
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __HUAWEI_WMI_H__
> +#define __HUAWEI_WMI_H__
> +
> +int huawei_wmi_micmute_led_set(bool on);
> +
> +#endif
> diff --git a/sound/pci/hda/huawei_wmi_helper.c b/sound/pci/hda/huawei_wmi_helper.c
> new file mode 100644
> index 000000000000..57256f51fd88
> --- /dev/null
> +++ b/sound/pci/hda/huawei_wmi_helper.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Helper functions for Huawei WMI Mic Mute LED;
> + * to be included from codec driver
> + */
> +
> +#if IS_ENABLED(CONFIG_HUAWEI_LAPTOP)
> +#include <linux/huawei_wmi.h>
> +
> +static int (*huawei_wmi_micmute_led_set_func)(bool);
> +
> +static void update_huawei_wmi_micmute_led(struct hda_codec *codec,
> +					  struct snd_kcontrol *kcontrol,
> +					  struct snd_ctl_elem_value *ucontrol)
> +{
> +	if (!ucontrol || !huawei_wmi_micmute_led_set_func)
> +		return;
> +	if (strcmp("Capture Switch", ucontrol->id.name) == 0 && ucontrol->id.index == 0) {
> +		/* TODO: How do I verify if it's a mono or stereo here? */
> +		bool val = ucontrol->value.integer.value[0] || ucontrol->value.integer.value[1];
> +		huawei_wmi_micmute_led_set_func(!val);
> +	}
> +}
> +
> +static void alc_fixup_huawei_wmi(struct hda_codec *codec,
> +				   const struct hda_fixup *fix, int action)
> +{
> +	struct hda_gen_spec *spec = codec->spec;
> +	bool removefunc = false;
> +
> +	codec_info(codec, "In alc_fixup_huawei_wmi\n");
> +
> +	if (action == HDA_FIXUP_ACT_PROBE) {
> +		if (!huawei_wmi_micmute_led_set_func)
> +			huawei_wmi_micmute_led_set_func = symbol_request(huawei_wmi_micmute_led_set);
> +		if (!huawei_wmi_micmute_led_set_func) {
> +			codec_warn(codec, "Failed to find huawei_wmi symbol huawei_wmi_micmute_led_set\n");
> +			return;
> +		}
> +
> +		removefunc = true;
> +		if (huawei_wmi_micmute_led_set_func(false) >= 0) {
> +			if (spec->num_adc_nids > 1 && !spec->dyn_adc_switch)
> +				codec_dbg(codec, "Skipping micmute LED control due to several ADCs");
> +			else {
> +				spec->cap_sync_hook = update_huawei_wmi_micmute_led;
> +				removefunc = false;
> +			}
> +		}
> +		codec_info(codec, "In alc_fixup_huawei_wmi IF\n");
> +
> +	}
> +
> +	if (huawei_wmi_micmute_led_set_func && (action == HDA_FIXUP_ACT_FREE || removefunc)) {
> +		symbol_put(huawei_wmi_micmute_led_set);
> +		huawei_wmi_micmute_led_set_func = NULL;
> +	}
> +}

There is a new snd_hda_gen_add_micmute_led() helper.  Please use it
instead.


> @@ -6610,6 +6620,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
>  	SND_PCI_QUIRK(0x17aa, 0x511e, "Thinkpad", ALC298_FIXUP_TPT470_DOCK),
>  	SND_PCI_QUIRK(0x17aa, 0x511f, "Thinkpad", ALC298_FIXUP_TPT470_DOCK),
>  	SND_PCI_QUIRK(0x19e5, 0x3204, "Huawei MBXP", ALC256_FIXUP_HUAWEI_MBXP_PINS),
> +	SND_PCI_QUIRK(0x19e5, 0x3204, "Huawei MBXP", ALC256_FIXUP_HUAWEI_WMI_MICMUTE_LED),

You must not have two entries with the very same ID.
And this implies that you didn't test the latest patches.  In the
quirk list, only the first matching is applied, so this WMI hook won't
be executed at all.

Anyways: the submission is confusing at this time since you posted in
may times by some reason, once mixed up all, once incomplete patchset,
etc.

At the next submission, could you give a proper cover letter (PATCH
0/3) and submit together with other patches?  git-format-patch will
give you a nice template with --cover-letter option.


thanks,

Takashi