> (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@xxxxxxxxxxx> > --- > 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 > + */ > if (!has_playback && !has_capture) { > dev_err(rtd->dev, "substream %s has no playback, no capture\n", > dai_link->stream_name); > - > return -EINVAL; > } >