Re: [PATCH 15/15] ASoC: soc-pcm: indicate warning if DPCM BE Codec has no settings

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

Hi Jerome

Thank you for your feedback

> I'm not quite sure what you mean by "should have validation" and what
> setting exactly we should validate ?
> 
> I know I should be able to able to understand that
> from the code below but, somehow I have trouble deciphering it.

Current ASoC have validation for ^^^ part

	DPCM
		[CPU/xxxx]-[xxxx/Codec]
		^^^^                   (A)
	Normal
		[CPU/Codec]
		^^^^^^^^^^^

(In many case, this "xxxx" is "dummy")
By this patch-set, It will check all cases

	DPCM
		[CPU/xxxx]-[xxxx/Codec]
		^^^^^^^^^   ^^^^^^^^^^ (B)
	Normal
		[CPU/Codec]
		^^^^^^^^^^^

At first, in [CPU/xxxx] case, "xxxx" part should be also checked
(in many case, this "xxxx" is "dummy").

And, because it didn't check (A) part before,
(B) part might be error on some board (at least Intel board).
To avoid such case, temporally it uses "dummy" instead of "Codec"
before [15/15]. This means (B) part checked as like below.

	[xxxx/Codec] -> [xxxx/dummy]

Because "dummy" will pass all cases, (B) part is almost same as no check.
Yes, it is no meaning, but the code will be simple.

> Where you have a CPU supporting both direction and 2 codecs, each
> supporting 1 stream direction ? This is a valid i2s configuration.
(snip)
> >  		/*
> > -		 * FIXME
> > +		 * FIXME / CLEAN-UP-ME
> >  		 *
> >  		 * DPCM BE Codec has been no checked before.
> >  		 * It should be checked, but it breaks compatibility.
> >  		 * It ignores BE Codec here, so far.
> >  		 */
> > -		if (dai_link->no_pcm)
> > -			codec_dai = dummy_dai;
> > +		if ((dai_link->no_pcm) &&
> > +		    ((cpu_play_t	&& !codec_play_t) ||
> > +		     (cpu_capture_t	&& !codec_capture_t))) {
> > +			dev_warn_once(rtd->dev, "DCPM BE Codec has no stream settings (%s)\n",
> > +				      codec_dai->name);
> 
> Taking one codec at a time, would you trigger a warning for the use case I
> described above ?

Oops, indeed it will indicate warning in your case.
How about this ?

	if ((dai_link->no_pcm) &&
	    (!codec_play_t && !codec_capture_t)) {
		dev_warn_once(...)
		...
	}

Thank you for your help !!

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