Hello Morimoto-san, On Fri, 21 Feb 2020, Kuninori Morimoto wrote: > Kai Vehmanen wrote: > > So if the primary problem is the messy rollback in case one open fails, > > what if we do the rollback directly in soc_pcm_components_open() and do > > not add any additional tracking..? > > > > Let me send a proposal patch for that. > It seems the patch was not so good cleanup... > > Thank you for your proposal patch. I checked it. > But, if it rollback when error with *last, > my opinion is original code > (= soc_pcm_components_close() needs *last parameter) > (= same as just revert the patch) is better. well, I do think the original code could still need a cleanup, so the effort is very much welcome. Having to pass the "last" parameter to components_open() just so that we can clean up if errors happen, seems a bit out of place. But yeah, easier said than done. We have now three alternatives to solve this: 1. do the cleanup locally in soc_pcm_components_open() [PATCH] ASoC: soc-pcm: fix state tracking error in snd_soc_component_open/close() 2. revert to original implementation with "last" passed to components_open() [PATCH] ASoC: soc-pcm: Revert "call snd_soc_component_open/close() once" 3. implement opened/module counters [PATCH][RFC] ASoC: soc-component: count snd_soc_component_open/close() ... I have tested all three and they seem to work. I still don't really like (1), but I agree (2) adds more code in total. I worry (3) might backfire with some component-substream combinations. So maybe we'll go with (1)? We do need to merge one of them, because current merged code is broken on many platforms. Br, Kai
