Re: [PATCH v4 6/8] ASoC: compress: Add support for DAI multicodec

[Vinod Koul <vinod.koul@intel.com>]

On Wed, Jul 02, 2014 at 02:53:50PM +0200, Benoit Cousson wrote:
> >>@@ -622,23 +633,33 @@ int soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num)
> >>  {
> >>  	struct snd_soc_codec *codec = rtd->codec;
> >>  	struct snd_soc_platform *platform = rtd->platform;
> >>-	struct snd_soc_dai *codec_dai = rtd->codec_dai;
> >>  	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
> >>  	struct snd_compr *compr;
> >>  	struct snd_pcm *be_pcm;
> >>  	char new_name[64];
> >>-	int ret = 0, direction = 0;
> >>-
> >>-	/* check client and interface hw capabilities */
> >>-	snprintf(new_name, sizeof(new_name), "%s %s-%d",
> >>-			rtd->dai_link->stream_name, codec_dai->name, num);
> >>-
> >>-	if (codec_dai->driver->playback.channels_min)
> >>-		direction = SND_COMPRESS_PLAYBACK;
> >>-	else if (codec_dai->driver->capture.channels_min)
> >>-		direction = SND_COMPRESS_CAPTURE;
> >>-	else
> >>-		return -EINVAL;
> >>+	int i, ret = 0, direction = -1;
> >>+
> >>+	for (i = 0; i < rtd->num_codecs; i++) {
> >>+		struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
> >>+		/* check client and interface hw capabilities */
> >>+		snprintf(new_name, sizeof(new_name), "%s %s-%d",
> >>+			 rtd->dai_link->stream_name, codec_dai->name, num);
> >>+
> >>+		if (direction < 0) {
> >>+			if (codec_dai->driver->playback.channels_min)
> >>+				direction = SND_COMPRESS_PLAYBACK;
> >>+			else if (codec_dai->driver->capture.channels_min)
> >>+				direction = SND_COMPRESS_CAPTURE;
> >direction wont change with multiple codecs, so this loop makes no sense here.
> >For simplcity perhaps we can use cpu_dai?
> >>+			else
> >>+				return -EINVAL;
> >>+		} else {
> >when will this get executed? You have initialized direction to -1 and if case is
> >always true. I think compiler will simply remove the below sinppet.
> 
> direction is set to -1, then the first iteration will set it to the
> direction of the first codec_dai. The other iteration are just
> checking that there are all in the same direction and will fail
> otherwise.
> 
> >>+			if ((codec_dai->driver->playback.channels_min &&
> >>+					direction != SND_COMPRESS_PLAYBACK) ||
> >>+			    (codec_dai->driver->capture.channels_min &&
> >>+					direction != SND_COMPRESS_CAPTURE))
> >and what exactly are we trying to check here?
> 
> That every codec_dai are in the same direction.
> 
> If you think this is pointless since every DAI are always in the
> same direction, we can just remove it.
I think that will simplify it. we certainly dont expect different direction,
right?

-- 
~Vinod