Re: [PATCH v2 13/16] ASoC: remove snd_soc_dai_link_set_capabilities()

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

On 4/1/24 19:29, Kuninori Morimoto wrote:
> 
> Hi Pierre-Louis
> 
>>> snd_soc_dai_link_set_capabilities() checks all CPU/Codec DAI (Y)(Z)
>>> for Playback/Capture (X) and checks its validation (A), and setup
>>> dpcm_playback/capture flags (a).
>>>
>>> 	void snd_soc_dai_link_set_capabilities(...)
>>> 	{
>>> 		...
>>> (X)		for_each_pcm_streams(direction) {
>>> 			...
>>> (Y)			for_each_link_cpus(dai_link, i, cpu) {
>>> 				...
>>> (A)				if (... snd_soc_dai_stream_valid(...)) {
>>> 					...
>>> 				}
>>> 			}
>>> (Z)			for_each_link_codecs(dai_link, i, codec) {
>>> 				...
>>> (A)				if (... snd_soc_dai_stream_valid(...)) {
>>> 					...
>>> 				}
>>> 			}
>>> 			...
>>> 		}
>>>
>>> (a)		dai_link->dpcm_playback = supported[...];
>>> (a)		dai_link->dpcm_capture  = supported[...];
>>> 	}
>>>
>>> This validation check will be automatically done on new
>>> soc_get_playback_capture(). snd_soc_dai_link_set_capabilities() is no
>>> longer needed. Let's remove it.
>>
>> Humm, this is really hard to review.
>>
>> soc_get_playback_capture() used to do a verification of the match
>> between dailink and dais, and now it doesn't have it any longer and this
>> patch removes the checks?
> 
> Hmm..., Maybe I'm misunderstanding ?
> I think this patch is very clear to remove, because it is 100% duplicate
> code. Maybe this mutual misunderstanding is based [01/15] review ?
> I think we need to dig it first.

I agree this looks like duplicate code, but why can't we remove it first
*before* any code modification?

It's very hard to review because it comes as the 13th patch of a series
and you've already removed similar code earlier which precisely checked
the consistency between dailink and dais.

In this function, it's a similar case btw where the settings provided by
the machine drivers are overridden by the framework, so that's another
case of collision between machine driver and framework. Which of the two
should be trusted?