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

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

On 4/1/24 18:27, Kuninori Morimoto wrote:
> 
> Hi Pierre-Louis
> 
> Thank you for the feedback
> 
>>>  			/* 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.

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