Re: [PATCH v4 3/8] ASoC: core: Add initial support for DAI multicodec

[Lars-Peter Clausen <lars@metafoo.de>]

On 07/01/2014 09:47 AM, Benoit Cousson wrote:
> DAI link assumes a one to one mapping between CPU DAI and CODEC. In
> some cases, the same CPU DAI can be connected to several codecs.
> This is the case for example, if you connect two mono codecs to the
> same I2S link in order to have a stereo card.
> The current ASoC implementation does not allow such setup.
>
> Add support for DAI links composed of a single CPU DAI and multiple
> CODECs. Sound cards have to pass the CODECs array in the corresponding
> DAI link through a new 'snd_soc_dai_link_component' struct. Each CODEC in
> this array is described in the same manner single CODEC DAIs are
> (either DT/OF node or codec_name).
>
> Based on an original code done by Misael.
>
> Signed-off-by: Benoit Cousson <bcousson@baylibre.com>
> Signed-off-by: Misael Lopez Cruz <misael.lopez@ti.com>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>

Almost. There are two more serious issues, the rest is trivial.

> ---
[...]
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 37a965c..3764150 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -554,7 +554,7 @@ int snd_soc_suspend(struct device *dev)
>   {
>   	struct snd_soc_card *card = dev_get_drvdata(dev);
>   	struct snd_soc_codec *codec;
> -	int i;
> +	int i, j;
>
>   	/* If the initialization of this soc device failed, there is no codec
>   	 * associated with it. Just bail out in this case.
> @@ -574,14 +574,16 @@ int snd_soc_suspend(struct device *dev)
>
>   	/* mute any active DACs */
>   	for (i = 0; i < card->num_rtd; i++) {
> -		struct snd_soc_dai *dai = card->rtd[i].codec_dai;
> -		struct snd_soc_dai_driver *drv = dai->driver;
> +		for (j = 0; j < card->rtd[i].num_codecs; j++) {
> +			struct snd_soc_dai *dai = card->rtd[i].codec_dais[j];
> +			struct snd_soc_dai_driver *drv = dai->driver;
>
> -		if (card->rtd[i].dai_link->ignore_suspend)
> -			continue;
> +			if (card->rtd[i].dai_link->ignore_suspend)
> +				continue;

This check can actually stay outside the inner loop. We either want to mute 
all or none.

>
> -		if (drv->ops->digital_mute && dai->playback_active)
> -			drv->ops->digital_mute(dai, 1);
> +			if (drv->ops->digital_mute && dai->playback_active)
> +				drv->ops->digital_mute(dai, 1);
> +		}
>   	}
>
>   	/* suspend all pcms */
[...]
> @@ -697,7 +703,7 @@ static void soc_resume_deferred(struct work_struct *work)
>   	struct snd_soc_card *card =
>   			container_of(work, struct snd_soc_card, deferred_resume_work);
>   	struct snd_soc_codec *codec;
> -	int i;
> +	int i, j;
>
>   	/* our power state is still SNDRV_CTL_POWER_D3hot from suspend time,
>   	 * so userspace apps are blocked from touching us
> @@ -758,14 +764,17 @@ static void soc_resume_deferred(struct work_struct *work)
>
>   	/* unmute any active DACs */
>   	for (i = 0; i < card->num_rtd; i++) {
> -		struct snd_soc_dai *dai = card->rtd[i].codec_dai;
> -		struct snd_soc_dai_driver *drv = dai->driver;
>
> -		if (card->rtd[i].dai_link->ignore_suspend)
> -			continue;
> +		for (j = 0; j < card->rtd[i].num_codecs; j++) {
> +			struct snd_soc_dai *dai = card->rtd[i].codec_dais[j];
> +			struct snd_soc_dai_driver *drv = dai->driver;
> +
> +			if (card->rtd[i].dai_link->ignore_suspend)
> +				continue;

Same as with the mute loop.

>
> -		if (drv->ops->digital_mute && dai->playback_active)
> -			drv->ops->digital_mute(dai, 0);
> +			if (drv->ops->digital_mute && dai->playback_active)
> +				drv->ops->digital_mute(dai, 0);
> +		}
>   	}
>
>   	for (i = 0; i < card->num_rtd; i++) {
[...]			    int num)
> +static int soc_aux_dev_init(struct snd_soc_card *card, int num)
>   {
>   	struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num];
>   	struct snd_soc_pcm_runtime *rtd = &card->rtd_aux[num];
> +	struct snd_soc_codec *codec;
>   	int ret;
>
>   	rtd->card = card;
>
> +	codec = soc_find_codec(NULL, aux_dev->codec_name);

We actually have support for binding the aux dev by DT node now. But I have 
a patch in my componentization branch that cleans all this up. Let me send 
it to you and then you can rebase on-top of that.

> +	if (!codec)
> +		return -EPROBE_DEFER;
> +
>   	/* do machine specific initialization */
>   	if (aux_dev->init) {
>   		ret = aux_dev->init(&codec->dapm);
> @@ -1286,16 +1316,19 @@ static int soc_aux_dev_init(struct snd_soc_card *card,
>   	return 0;
>   }
>
> -static int soc_dai_link_init(struct snd_soc_card *card,
> -			     struct snd_soc_codec *codec,
> -			     int num)
> +static int soc_dai_link_init(struct snd_soc_card *card, int num)
>   {
>   	struct snd_soc_dai_link *dai_link =  &card->dai_link[num];
>   	struct snd_soc_pcm_runtime *rtd = &card->rtd[num];
> -	int ret;
> +	int i, ret;
>
>   	rtd->card = card;
>
> +	for (i = 0; i < rtd->num_codecs; i++) {
> +		/* Make sure all DAPM widgets are instantiated */
> +		snd_soc_dapm_new_widgets(rtd->codec_dais[i]->codec->dapm.card);
> +	}

This is still a left over from a very early revision of this patch. We 
removed this in upstream a while ago.

> +
>   	/* do machine specific initialization */
>   	if (dai_link->init) {
>   		ret = dai_link->init(rtd);
[...]