Re: [PATCH v2 01/16] ASoC: soc-pcm.c: cleanup soc_get_playback_capture()

[Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>]

Hi Pierre-Louis

Thank you for your feedback.
I could understand your comment 80%, but not yet 100%

> With the older code, if the dpcm_playback was set for the dailink but
> there isn't any dai connected to support playback, an error was thrown.
> 
> With the new code, if playback_only is set but there isn't any dai
> connected, there is no error thrown, is there?
(snip)
> Again we had a verification that if the dpcm_playback was set at the
> dailink level, it was actually supported by the dais.
>
> We seem to have lost this verification. We only have an error when there
> are no settings at all.

Pseudo code of new soc_get_playback_capture() is like this

	soc_get_playback_capture(...)
	{
		...
 ^		for_each_rtd_ch_maps(...) {
 |			...
(A)			has_playback = xxx;
 |			has_capture  = xxx;
 v		}

 ^		if (dai_link->playback_only)
 |			has_capture = 0;
(B)
 |		if (dai_link->capture_only)
 v			has_playback = 0;

 ^		if (!has_playback && !has_capture) {
(C)			dev_err(...);
 v			return -EINVAL;
		}
		...
	}

In old/new soc_get_playback_capture(), has_xxx will be set at least
if one of rtd connected DAI can handle playback/capture.
In new code, it will be handled at (A).

And unneeded has_xxx will be removed if xxx_only was set (B)

Then, if neither has_xxx was set, it will be error (C)

	In new code, if playback_only is set but there isn't any dai
	connected, there is no error thrown, is there?

If playback_only was set, has_capture will be removed at (B).
And if DAI was not playback-able, this means has_playback was not set at (A).
In such case, (C) will indicate error. Same things happen if capture_only too.

So, old functions validation still exist in my opinion, but am I
misunderstanding ?

One note here is that in DPCM case, old function checks CPU only,
but new function checks both CPU and Codec.

2nd note is that in current version of patch-set, if dai_link doesn't
have xxx_only settings (= it should have both playback/capture), but if
DAI has has_playback or has_capture only, it can't detect about it.
I suggested it in previous mail, and will fix in v3

> The point is that these flags are sometimes set in the machine driver,
> sometimes set in the framework, and the open is which one has the priority.

I couldn't understand this.

I think "machine driver" = CPU/Codec driver, but what is "these flags"
which is sometimes set in machine driver, and sometimes set in framework ??
dpcm_xxx ? xxx_only ?? I don't think framework set these...

Or do you mean [09/16] patch (= it will set dai_link->no_pcm) ?



Thank you for your help !!

Best regards
---
Renesas Electronics
Ph.D. Kuninori Morimoto