(X) part is for DPCM, and it checks playback/capture availability if dai_link has dpcm_playback/capture flag (a)(b). This availability check should be available not only for DPCM, but for all connections. But it is not mandatory option. Let's name it as assertion.
I don't follow the 'not mandatory option'. Why not make these 'assertions' mandatory? What happens in case the the option is not present?
In case of having assertion flag, non specific side will be disabled.
Not following the wording, multiple negatives and not clear on what 'side' refers to (direction or DPCM/non-DPCM).
For example if it has playback_assertion but not have capture_assertion, capture will be force disabled.
dpcm_playback -> playabck_assertion = 1
dpcm_capture -> capture_assertion = 1
playback_only -> playback_assertion = 1 capture_assertion = 0
capture_only -> playback_assertion = 0 capture_assertion = 1
By expanding this assertion to all connections, we can use same code for all connections, this means code can be more cleanup.
I see a contradiction between "expanding the assertion to all connections" and "not mandatory".
Current validation check on DPCM ignores Codec DAI, but there is no reason to do it. We should check both CPU/Codec in all connection.
"there's no reason to do so" ?
This patch expands validation check to all cases.
[CPU/xxxx]-[yyyy/Codec] *****
In many case (not all case), above [xxxx][yyyy] part are "dummy" DAI which is always valid for both playback/capture. But unfortunately DPCM BE Codec (**** part) had been no validation check before, and some Codec driver doesn't have necessary settings for it. This means all cases validation check breaks compatibility on some vender's drivers. Thus this patch temporary uses dummy DAI at BPCM BE
vendor
Codec part, and avoid compatibility error. But it should be removed in the future.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
include/sound/soc.h | 9 +++ sound/soc/soc-pcm.c | 143 +++++++++++++++++++++++++------------------- 2 files changed, 92 insertions(+), 60 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 0376f7e4c15d..e604d74f6e33 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -809,6 +809,15 @@ struct snd_soc_dai_link { unsigned int dpcm_capture:1; unsigned int dpcm_playback:1;
- /*
* Capture / Playback support assertion. Having assertion flag is not mandatory.* In case of having assertion flag, non specific side will be disabled.
again the 'not mandatory' and 'non specific side will be disabled' are confusing.
- /*
* Assertion check** playback_assertion = 0 No assertion check.* capture_assertion = 0 ASoC will use detected playback/capture as-is.* No playback, No capture will be error.
did you mean "this combination will be handled as an error" ?
It's probably best to have a different presentation, to avoid confusions. Using multiple lines without a separator isn't great.
Suggested example:
playback_assertion = 0 capture_assertion = 0 this combination will be handled as an error
playback_assertion = 1 capture_assertion = 0 the DAIs in this dailink must support playback. ASoC will disable capture. In other words "playback_only"
** playback_assertion = 1 DAI must playback available. ASoC will disable capture.* capture_assertion = 0 In other words "playback_only"** playback_assertion = 0 DAI must capture available. ASoC will disable playback.* capture_assertion = 1 In other words "capture_only"** playback_assertion = 1 DAI must both playback/capture available.* capture_assertion = 1
nit-pick: the 'must X available' does not read well, 'must support X' is probably what you meant.
*/- if (dai_link->playback_assertion) {
if (!has_playback) {dev_err(rtd->dev, "%s playback assertion check error\n", dai_link->stream_name);
"invalid playback_assertion" ?
return -EINVAL;}/* makes it plyaback only */
typo: playback
if (!dai_link->capture_assertion)has_capture = 0;- }
- if (dai_link->capture_assertion) {
if (!has_capture) {dev_err(rtd->dev, "%s capture assertion check error\n", dai_link->stream_name);return -EINVAL;}/* makes it capture only */if (!dai_link->playback_assertion)has_playback = 0;- }
we probably want a dev_ log when the has_playback/capture is overridden?
- /*
* Detect Mismatch*/
- return -EINVAL; }