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?