Re: [PATCH v2 09/16] ASoC: soc-core: Replace dpcm_playback/capture to playback/capture_only

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

Hi Pierre-Louis

Thank you for your feedback, but this is also my understanding is
not yet 100%

> >>>  			/* convert non BE into BE */
> >>> -			if (!dai_link->no_pcm) {
> >>> -				dai_link->no_pcm = 1;
> >>> -
> >>> -				if (dai_link->dpcm_playback)
> >>> -					dev_warn(card->dev,
> >>> -						 "invalid configuration, dailink %s has flags no_pcm=0 and dpcm_playback=1\n",
> >>> -						 dai_link->name);
> >>> -				if (dai_link->dpcm_capture)
> >>> -					dev_warn(card->dev,
> >>> -						 "invalid configuration, dailink %s has flags no_pcm=0 and dpcm_capture=1\n",
> >>> -						 dai_link->name);
> >>> -
> >>> -				/* convert normal link into DPCM one */
> >>> -				if (!(dai_link->dpcm_playback ||
> >>> -				      dai_link->dpcm_capture)) {
> >>> -					dai_link->dpcm_playback = !dai_link->capture_only;
> >>> -					dai_link->dpcm_capture = !dai_link->playback_only;
> >>> -				}
> >>> -			}
> >>> +			dai_link->no_pcm = 1;
> > (snip)
> >> It's not clear to me how this is related to the
> >> dpcm_playback/dpcm_capture removal.
> > 
> > In my understanding, if "dai_link->no_pcm" was 0, it sets no_pcm and
> > convert setting to BE. If no_pcm was 1, it is BE anyway. So no_pcm will
> > be 1 anyway after this code.
> > And then, dpcm_playback/capture is no longer needed.
> > So it just set no_pcm = 1 here. But am I wrong ??
> 
> The problem is that the patchset is supposed to be only about removal of
> flags to align on one set, but then we also have "simplifications" or
> removal of checks without explanations.

Do you mean it need to have/keep the comment on the code ??
And/or what does your "removal of checks" mean ?

I understand that patch should have enough explanation, and indeed above
code has if (dai_link->dpcm_xxx) checks, but dpcm_xxx are no longer
needed in new code. What kind of comment are you requesting to me ?

> It would be far less invasive if we only replaced flags and had
> iso-functionality. Then we can discuss the merits of simplifications.

This was the most difficult comment to understand for me...


Thank you for your help !!

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