On 4/1/24 19:21, Kuninori Morimoto wrote: > > Hi Pierre-Louis > > Thank you for your review > >> The problem I have is with the following code (not shown with diff) >> >> if (dai_link->playback_only) >> has_capture = 0; >> >> if (dai_link->capture_only) >> has_playback = 0; >> >> So with this grand unification, all the loops above may make a decision >> that could be overridden by these two branches. >> >> This was not the case before for DPCM, all the 'has_capture' and >> 'has_playback' variables were used as a verification of the dai_link >> settings with an error thrown e.g. if the dpcm_playback was set without >> any DAIs supporting playback. > > I could understand so far. > >> Now the dailink settings are used unconditionally. There is one warning >> added if there are no settings for a dailink, but we've lost the >> detection of a mismatch between dailink and the set of cpu/codec dais >> that are part of this dailink. > > But sorry I could understand this. > > "There is one warning added if there are no settings for a dailink" > > By [01/16] patch ? I think no warning is added. Or do you mean by [15/16] > patch ? Yes I looked at the entire series, it's just too complicated to look with diff. > > "we've lost the detection of a mismatch between dailink and the > set of cpu/codec dais that are part of this dailink" > > Sorry I couldn't understand about this. > Which mismatch detection we lost ?? Concrete sample / code / image > is very helpful for me to well understanding. With the older code, if the dpcm_playback was set for the dailink but there isn't any dai connected to support playback, an error was thrown. With the new code, if playback_only is set but there isn't any dai connected, there is no error thrown, is there? The point is that these flags are sometimes set in the machine driver, sometimes set in the framework, and the open is which one has the priority.
