Re: [PATCH] ALSA: hda: generic: Always add LED controls

[Takashi Iwai <tiwai@xxxxxxx>]

On Thu, 04 Jan 2024 16:58:38 +0100,
Bernhard Seibold wrote:
> 
> LEDs for mute and microphone mute can be added to any system via the
> audio-mute and audio-micmute triggers, e.g. via USB HID devices.
> Therefore, add the LED controls unconditionally.

Hmm...  Won't your change lead to the sysfs entries always,
i.e. creating sysfs entries even if the device has no LED at all?


Takashi

> 
> Signed-off-by: Bernhard Seibold <mail@xxxxxxxxxxxxxxxxxxx>
> ---
>  sound/pci/hda/hda_generic.c | 46 ++++++++++++++++++-------------------
>  sound/pci/hda/hda_generic.h |  2 --
>  2 files changed, 22 insertions(+), 26 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
> index bf685d01259d..e78c0dd17c42 100644
> --- a/sound/pci/hda/hda_generic.c
> +++ b/sound/pci/hda/hda_generic.c
> @@ -3644,8 +3644,7 @@ static int add_single_cap_ctl(struct hda_codec *codec, const char *label,
>  		return -ENOMEM;
>  	if (is_switch) {
>  		knew->put = cap_single_sw_put;
> -		if (spec->mic_mute_led)
> -			knew->access |= SNDRV_CTL_ELEM_ACCESS_MIC_LED;
> +		knew->access |= SNDRV_CTL_ELEM_ACCESS_MIC_LED;
>  	}
>  	if (!inv_dmic)
>  		return 0;
> @@ -3663,8 +3662,7 @@ static int add_single_cap_ctl(struct hda_codec *codec, const char *label,
>  		return -ENOMEM;
>  	if (is_switch) {
>  		knew->put = cap_single_sw_put;
> -		if (spec->mic_mute_led)
> -			knew->access |= SNDRV_CTL_ELEM_ACCESS_MIC_LED;
> +		knew->access |= SNDRV_CTL_ELEM_ACCESS_MIC_LED;
>  	}
>  	return 0;
>  }
> @@ -3706,8 +3704,7 @@ static int create_bind_cap_vol_ctl(struct hda_codec *codec, int idx,
>  		knew->index = idx;
>  		knew->private_value = sw_ctl;
>  		knew->subdevice = HDA_SUBDEV_AMP_FLAG;
> -		if (spec->mic_mute_led)
> -			knew->access |= SNDRV_CTL_ELEM_ACCESS_MIC_LED;
> +		knew->access |= SNDRV_CTL_ELEM_ACCESS_MIC_LED;
>  	}
>  	return 0;
>  }
> @@ -3979,7 +3976,6 @@ int snd_hda_gen_add_mute_led_cdev(struct hda_codec *codec,
>  	if (spec->vmaster_mute.hook)
>  		codec_err(codec, "vmaster hook already present before cdev!\n");
>  
> -	spec->vmaster_mute_led = 1;
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(snd_hda_gen_add_mute_led_cdev);
> @@ -4002,7 +3998,6 @@ int snd_hda_gen_add_micmute_led_cdev(struct hda_codec *codec,
>  				     int (*callback)(struct led_classdev *,
>  						     enum led_brightness))
>  {
> -	struct hda_gen_spec *spec = codec->spec;
>  	int err;
>  
>  	if (callback) {
> @@ -4013,7 +4008,6 @@ int snd_hda_gen_add_micmute_led_cdev(struct hda_codec *codec,
>  		}
>  	}
>  
> -	spec->mic_mute_led = 1;
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(snd_hda_gen_add_micmute_led_cdev);
> @@ -4973,8 +4967,7 @@ int snd_hda_gen_parse_auto_config(struct hda_codec *codec,
>  
>  	parse_user_hints(codec);
>  
> -	if (spec->vmaster_mute_led || spec->mic_mute_led)
> -		snd_ctl_led_request();
> +	snd_ctl_led_request();
>  
>  	if (spec->mixer_nid && !spec->mixer_merge_nid)
>  		spec->mixer_merge_nid = spec->mixer_nid;
> @@ -5173,6 +5166,7 @@ static const char * const follower_pfxs[] = {
>  int snd_hda_gen_build_controls(struct hda_codec *codec)
>  {
>  	struct hda_gen_spec *spec = codec->spec;
> +	struct snd_kcontrol *kctl;
>  	int err;
>  
>  	if (spec->kctls.used) {
> @@ -5211,19 +5205,23 @@ int snd_hda_gen_build_controls(struct hda_codec *codec)
>  		if (err < 0)
>  			return err;
>  	}
> -	if (!spec->no_analog && !spec->suppress_vmaster &&
> -	    !snd_hda_find_mixer_ctl(codec, "Master Playback Switch")) {
> -		err = __snd_hda_add_vmaster(codec, "Master Playback Switch",
> -					    NULL, follower_pfxs,
> -					    "Playback Switch", true,
> -					    spec->vmaster_mute_led ?
> -						SNDRV_CTL_ELEM_ACCESS_SPK_LED : 0,
> -					    &spec->vmaster_mute.sw_kctl);
> -		if (err < 0)
> -			return err;
> -		if (spec->vmaster_mute.hook) {
> -			snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute);
> -			snd_hda_sync_vmaster_hook(&spec->vmaster_mute);
> +	if (!spec->no_analog && !spec->suppress_vmaster) {
> +
> +		kctl = snd_hda_find_mixer_ctl(codec, "Master Playback Switch");
> +		if (!kctl) {
> +			err = __snd_hda_add_vmaster(codec, "Master Playback Switch",
> +						    NULL, follower_pfxs,
> +						    "Playback Switch", true,
> +						    SNDRV_CTL_ELEM_ACCESS_SPK_LED,
> +						    &spec->vmaster_mute.sw_kctl);
> +			if (err < 0)
> +				return err;
> +			if (spec->vmaster_mute.hook) {
> +				snd_hda_add_vmaster_hook(codec, &spec->vmaster_mute);
> +				snd_hda_sync_vmaster_hook(&spec->vmaster_mute);
> +			}
> +		} else {
> +			kctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_SPK_LED;
>  		}
>  	}
>  
> diff --git a/sound/pci/hda/hda_generic.h b/sound/pci/hda/hda_generic.h
> index a8eea8367629..13db36528041 100644
> --- a/sound/pci/hda/hda_generic.h
> +++ b/sound/pci/hda/hda_generic.h
> @@ -223,8 +223,6 @@ struct hda_gen_spec {
>  	unsigned int inv_dmic_split:1; /* inverted dmic w/a for conexant */
>  	unsigned int own_eapd_ctl:1; /* set EAPD by own function */
>  	unsigned int keep_eapd_on:1; /* don't turn off EAPD automatically */
> -	unsigned int vmaster_mute_led:1; /* add SPK-LED flag to vmaster mute switch */
> -	unsigned int mic_mute_led:1; /* add MIC-LED flag to capture mute switch */
>  	unsigned int indep_hp:1; /* independent HP supported */
>  	unsigned int prefer_hp_amp:1; /* enable HP amp for speaker if any */
>  	unsigned int add_stereo_mix_input:2; /* add aamix as a capture src */
> -- 
> 2.43.0
>