From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lars-Peter Clausen Subject: Re: [PATCH v4 3/8] ASoC: core: Add initial support for DAI multicodec Date: Tue, 01 Jul 2014 15:19:33 +0200 Message-ID: <53B2B565.9080808@metafoo.de> References: <1404200881-32253-1-git-send-email-bcousson@baylibre.com> <1404200881-32253-4-git-send-email-bcousson@baylibre.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp-out-145.synserver.de (smtp-out-145.synserver.de [212.40.185.145]) by alsa0.perex.cz (Postfix) with ESMTP id 9109126504F for ; Tue, 1 Jul 2014 15:19:37 +0200 (CEST) In-Reply-To: <1404200881-32253-4-git-send-email-bcousson@baylibre.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Benoit Cousson , broonie@kernel.org, lgirdwood@gmail.com Cc: Fabien Parent , misael.lopez@ti.com, alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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 > Signed-off-by: Misael Lopez Cruz > Signed-off-by: Fabien Parent 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); [...]