Hi Pierre-Louis
(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?
The big reason why "assertion flag" is not mandatory is that non-DPCM doesn't have such flag before. I can't add such flags to all of non-DPCM, because I don't know which direction (playback/capture) is available on each DAIs.
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).
How about this ?
If either playback or capture assertion flag was presented, not presented direction will be disabled by ASoC even if it was available.
I see a contradiction between "expanding the assertion to all connections" and "not mandatory".
(snip)
again the 'not mandatory' and 'non specific side will be disabled' are confusing.
(snip)
- /*
* 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" ?
Hmm... is it word selection issue ?? is "must_support" better ?
(playback_xxx, capture_xxx)
(0, 0) : Both are not must item. available direction is used as-is. But it will be error if nothing was available.
(1, 0) : DAI must support selected direction. (0, 1) Not selected direction will be disabled even though it was available
(1, 1) : Both must be supported.
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"
Yeah, I agree
* 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.
Thanks. will fix in v4
- 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?
OK, will do in v4
Thank you for your help !!
Best regards --- Renesas Electronics Ph.D. Kuninori Morimoto