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
