[alsa-devel] [PATCH] ASoc: Handle multiple codecs with split playback / capture

[Mark Brown]

On Mon, Aug 17, 2015 at 10:02:29AM +0200, Ricard Wanderlof wrote:
> On Fri, 14 Aug 2015, Mark Brown wrote:
> > On Thu, Aug 13, 2015 at 04:12:56PM +0200, Ricard Wanderlof wrote:

> Should the commit message have any information as to which specific use 
> case led to the creation of the patch?

That's often good to mention, yes.

> > > +	return !!codec_stream->rates;

> > The !! is redundant here.

> I still felt it made sense as rates is an int and the function actually 
> returns a bool, i.e. the !! indicates to a human reader that the author of 
> the function is aware that rates is not a bool yet the function is 
> declared as one.

It's sufficiently obvious that you're just checking if rates has
anything set at all here I think.

> > > @@ -827,6 +859,10 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
> > >  		struct snd_soc_dai *codec_dai = rtd->codec_dais[i];
> > >  		struct snd_pcm_hw_params codec_params;

> > > +		/* Skip codecs which don't support the current stream type. */
> > > +		if (!snd_soc_dai_stream_valid(codec_dai, substream->stream))
> > > +			continue;

> > >  		/* copy params for each codec */
> > >  		codec_params = *params;

> > We're still calling hw_params() for all CODECs, even ones that happen to
> > not be suitable...  is that sensible? 

> Hm, how so, the soc_dai_hw_params(..., codec_dai) is further on inside the 
> loop so the continue skips over it?

> However, if nothing matches, soc_dai_hw_params(..., cpu_dai) would still 
> be called, just after the loop, which might not make sense. 

I think that's the bit I was thinking of, sorry.

> > It might be in a case where the CODEC for one direction clocks 
> > everything but I can see problems occurring if the transmit and receive 
> > paths are separately clocked which is going to be quite common for 
> > things like i.MX systems.  If the two are tied together there's going to 
> > be constraints between the two directions.

> > I'd expect that we could take a pretty good guess as to what the best
> > thing to do is based on the symmetry constraints we have for the link
> > but I think we do want to think about this early.

> I guess it would be better just to 'goto out' after the loop if a suitable 
> codec could not be found.

Well, there's two things here: one is if we've got a completely invalid
configuration with no CODECs being configured and the other is if we
should only be configuring matching CODECs in the first place.  For
systems where the LRCLK and BCLK are shared between playback and capture
we may want to always configure them even if the CODEC driving the
clocks isn't the one handling the audio.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20150817/702e1d6a/attachment.sig>