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