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
