On 07/03/2014 01:39 PM, Benoit Cousson wrote:
On 03/07/2014 13:20, Lars-Peter Clausen wrote:
On 07/03/2014 01:09 PM, Benoit Cousson wrote:
On 03/07/2014 08:41, Vinod Koul wrote:
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?
Lars, Mark Any thought on that? Does that sound reasonable to you to use only the first codec_dai to figure out the direction?
I guess it is ok. But I'm wondering if the direction shouldn't depend on the CPU dai rather than the CODEC dai? E.g. it is possible to have a CODEC with both playback and capture support in two dai_links, one for capture, one for playback. The check above would create a SND_COMPRESS_PLAYBACK stream in both cases. Even if the CPU dai only supported capture in one of the cases.
OK, I'll use the cpu_dai then.
Lets wait for Vinod's feedback on this first. He knows this code best and what the intentions are.
- Lars