[PATCH v2] ASoC: SOF: topology: use new sound control LED layer

[Péter Ujfalusi]

On 29/03/2022 15:00, Jaroslav Kysela wrote:
> Use the new sound control LED layer instead the direct ledtrig_audio_set()
> call - see 22d8de62f11b ("ALSA: control - add generic LED trigger module
> as the new control layer").
> 
> Signed-off-by: Jaroslav Kysela <perex at perex.cz>
> Cc: Mark Brown <broonie at kernel.org>
> Cc: Ranjani Sridharan <ranjani.sridharan at linux.intel.com>
> Cc: Bard Liao <yung-chuan.liao at linux.intel.com>
> Cc: Péter Ujfalusi <peter.ujfalusi at linux.intel.com>
> Cc: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> 
> -----
> v2:
>   - add Mark to Cc:
>   - add snd_ctl_led_request() call
> ---
>  sound/soc/sof/control.c  | 33 ---------------------------------
>  sound/soc/sof/sof-priv.h |  1 +
>  sound/soc/sof/topology.c | 16 ++++++++++++++++
>  3 files changed, 17 insertions(+), 33 deletions(-)
> 
> diff --git a/sound/soc/sof/control.c b/sound/soc/sof/control.c
> index de1778c4002b..3b5718a3516d 100644
> --- a/sound/soc/sof/control.c
> +++ b/sound/soc/sof/control.c
> @@ -15,36 +15,6 @@
>  #include "sof-priv.h"
>  #include "sof-audio.h"
>  
> -static void update_mute_led(struct snd_sof_control *scontrol,
> -			    struct snd_kcontrol *kcontrol,
> -			    struct snd_ctl_elem_value *ucontrol)
> -{
> -	int temp = 0;
> -	int mask;
> -	int i;
> -
> -	mask = 1U << snd_ctl_get_ioffidx(kcontrol, &ucontrol->id);
> -
> -	for (i = 0; i < scontrol->num_channels; i++) {
> -		if (ucontrol->value.integer.value[i]) {
> -			temp |= mask;
> -			break;
> -		}
> -	}
> -
> -	if (temp == scontrol->led_ctl.led_value)
> -		return;
> -
> -	scontrol->led_ctl.led_value = temp;
> -
> -#if IS_REACHABLE(CONFIG_LEDS_TRIGGER_AUDIO)
> -	if (!scontrol->led_ctl.direction)
> -		ledtrig_audio_set(LED_AUDIO_MUTE, temp ? LED_OFF : LED_ON);
> -	else
> -		ledtrig_audio_set(LED_AUDIO_MICMUTE, temp ? LED_OFF : LED_ON);
> -#endif
> -}
> -
>  int snd_sof_volume_get(struct snd_kcontrol *kcontrol,
>  		       struct snd_ctl_elem_value *ucontrol)
>  {
> @@ -121,9 +91,6 @@ int snd_sof_switch_put(struct snd_kcontrol *kcontrol,
>  	struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
>  	const struct sof_ipc_tplg_ops *tplg_ops = sdev->ipc->ops->tplg;
>  
> -	if (scontrol->led_ctl.use_led)
> -		update_mute_led(scontrol, kcontrol, ucontrol);
> -
>  	if (tplg_ops->control->switch_put)
>  		return tplg_ops->control->switch_put(scontrol, ucontrol);
>  
> diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
> index 0d9b640ae24c..e537b1258aa8 100644
> --- a/sound/soc/sof/sof-priv.h
> +++ b/sound/soc/sof/sof-priv.h
> @@ -473,6 +473,7 @@ struct snd_sof_dev {
>  	struct list_head route_list;
>  	struct snd_soc_component *component;
>  	u32 enabled_cores_mask; /* keep track of enabled cores */
> +	bool led_present;
>  
>  	/* FW configuration */
>  	struct sof_ipc_window *info_window;
> diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
> index 9b11e9795a7a..e47a64934c04 100644
> --- a/sound/soc/sof/topology.c
> +++ b/sound/soc/sof/topology.c
> @@ -773,6 +773,7 @@ static int sof_control_load_volume(struct snd_soc_component *scomp,
>  	struct snd_soc_tplg_mixer_control *mc =
>  		container_of(hdr, struct snd_soc_tplg_mixer_control, hdr);
>  	int tlv[TLV_ITEMS];
> +	unsigned int mask;
>  	int ret;
>  
>  	/* validate topology data */
> @@ -821,6 +822,16 @@ static int sof_control_load_volume(struct snd_soc_component *scomp,
>  		goto err;
>  	}
>  
> +	if (scontrol->led_ctl.use_led) {
> +		mask = scontrol->led_ctl.direction ? SNDRV_CTL_ELEM_ACCESS_MIC_LED :
> +							SNDRV_CTL_ELEM_ACCESS_SPK_LED;
> +		scontrol->access &= ~SNDRV_CTL_ELEM_ACCESS_LED_MASK;
> +		scontrol->access |= mask;
> +		kc->access &= ~SNDRV_CTL_ELEM_ACCESS_LED_MASK;
> +		kc->access |= mask;
> +		sdev->led_present = true;
> +	}
> +
>  	dev_dbg(scomp->dev, "tplg: load kcontrol index %d chans %d\n",
>  		scontrol->comp_id, scontrol->num_channels);
>  
> @@ -2027,6 +2038,7 @@ static struct snd_soc_tplg_ops sof_tplg_ops = {
>  
>  int snd_sof_load_topology(struct snd_soc_component *scomp, const char *file)
>  {
> +	struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
>  	const struct firmware *fw;
>  	int ret;
>  
> @@ -2049,6 +2061,10 @@ int snd_sof_load_topology(struct snd_soc_component *scomp, const char *file)
>  	}

Nitpick, but I would probably done:
- 	}
+ 	} else if (sdev->led_present) {
+		ret = snd_ctl_led_request();
+ 	}

Reviewed-by: Peter Ujfalusi <peter.ujfalusi at linux.intel.com>

>  
>  	release_firmware(fw);
> +
> +	if (ret >= 0 && sdev->led_present)
> +		ret = snd_ctl_led_request();
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL(snd_sof_load_topology);

-- 
Péter