On Thu, 4 Jun 2015, Lars-Peter Clausen wrote:
Ideally it would have been some sort of 'if (!codec_stream->defined)' but there isn't such a member in struct snd_soc_dai . I've gone with 'if (!codec_stream->rates && !codec_stream->formats)', thinking that if a codec doesn't support any rates or formats, it probably doesn't support that mode at all (else it's rather meaningless). In fact, one of these (rates or formats) would probably suffice, with a comment explaining what we're really trying to do.
The next problem is that when trying to set hw params, something in the framework or the individual codec driver hw_params() bails out saying it can't set the intended parameters. Looking at that right now to see if it can be solved in a similar way.
The best way to solve this is probably to introduce a helper function bool snd_soc_dai_stream_valid(struct snd_soc_dai *dai, int stream) that implements the logic for detecting whether a DAI supports a playback or capture stream. And then whenever iterating over the codec_dais field for stream operations skip those which don't support the stream.
Yes. The question is, exactly what should the helper function use to determine stream support. Is it sufficient to check for dai->driver->playback/capture->rates != 0 ?
I'm a bit worried at the 'whenever iterating over the codec_dais field'. There are 50+ places which iterate over codec_dais, mostly in soc-pcm.c and soc-core.c . Of course code could be added to all locations, but it starts to feel like the wrong approach, i.e. like the iteration itself should be done in a separate function (which doesn't really seem viable in itself; this isn't glib).
For the hardware I have here, there are two places where it is necessary; admittedly, this particular hardware is pretty simple (on the other hand, I would suppose that most situations with this type of split playback/capture would be for cases where there are simple codecs or I2S microphones that only support one of the modes).
What I'm getting at is that, disregarding the helper function which is needed anyway, perhaps I should just fix the two locations needed for now, and leave the rest to be fixed when the need arises? I'm not sure they all need to be modified in this way.
/Ricard