Re: [PATCH] ASoC: soc-pcm: fix state tracking error in snd_soc_component_open/close()

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 2/24/20 9:47 AM, Dmitry Osipenko wrote:
> 24.02.2020 15:01, Mark Brown пишет:
>> On Thu, Feb 20, 2020 at 11:49:55AM +0200, Kai Vehmanen wrote:
>>> ASoC component open/close and snd_soc_component_module_get/put are called
>>> independently for each component-substream pair, so the logic added in
>>> commit dd03907bf129 ("ASoC: soc-pcm: call snd_soc_component_open/close()
>>> once") was not sufficient and led to PCM playback and module unload errors.
>>>
>>> Implement handling of failures directly in soc_pcm_components_open(),
>>> so that any successfully opened components are closed upon error with
>>> other components. This allows to clean up error handling in
>>> soc_pcm_open() without adding more state tracking.
>>
>> Do people have thoughts on this?  I do like this approach but can't
>> really test effectively myself.
>>
> 
> I haven't tried to review this patch, but it works fine on NVIDIA Tegra:
> 
> Tested-by: Dmitry Osipenko <digetx@gmail.com>

LGTM - I believe Kai tested for Intel platforms.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Note that the code could be simplified further with a 
for_each_rtd_components_rollback() macro, and we could also simplify the 
hw_free code below - the use of the 'last' pointer is not really 
necessary since we already have an index. That's for another cleanup though.

static int soc_pcm_components_hw_free(struct snd_pcm_substream *substream,
				      struct snd_soc_component *last)
{
	struct snd_soc_pcm_runtime *rtd = substream->private_data;
	struct snd_soc_component *component;
	int i, r, ret = 0;

	for_each_rtd_components(rtd, i, component) {
		if (component == last)
			break;