The patch ASoC: soc-pcm: fix state tracking error in snd_soc_component_open/close() has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From d2aaa8d8bfba93237ac944ee058fb98e2c2ef983 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> Date: Thu, 20 Feb 2020 11:49:55 +0200 Subject: [PATCH] ASoC: soc-pcm: fix state tracking error in snd_soc_component_open/close() 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. Fixes: dd03907bf129 ("ASoC: soc-pcm: call snd_soc_component_open/close() once") Signed-off-by: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> Tested-by: Dmitry Osipenko <digetx@xxxxxxxxx> Link: https://lore.kernel.org/r/20200220094955.16968-1-kai.vehmanen@xxxxxxxxxxxxxxx Signed-off-by: Mark Brown <broonie@xxxxxxxxxx> --- include/sound/soc-component.h | 7 ++----- sound/soc/soc-component.c | 35 +++++++---------------------------- sound/soc/soc-pcm.c | 27 +++++++++++++++++++++------ 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h index 1866ecc8e94b..154d02fbbfed 100644 --- a/include/sound/soc-component.h +++ b/include/sound/soc-component.h @@ -147,6 +147,8 @@ struct snd_soc_component { unsigned int active; + unsigned int suspended:1; /* is in suspend PM state */ + struct list_head list; struct list_head card_aux_list; /* for auxiliary bound components */ struct list_head card_list; @@ -180,11 +182,6 @@ struct snd_soc_component { struct dentry *debugfs_root; const char *debugfs_prefix; #endif - - /* bit field */ - unsigned int suspended:1; /* is in suspend PM state */ - unsigned int opened:1; - unsigned int module:1; }; #define for_each_component_dais(component, dai)\ diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index ee00c09df5e7..14e175cdeeb8 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -297,55 +297,34 @@ EXPORT_SYMBOL_GPL(snd_soc_component_set_jack); int snd_soc_component_module_get(struct snd_soc_component *component, int upon_open) { - if (component->module) - return 0; - if (component->driver->module_get_upon_open == !!upon_open && !try_module_get(component->dev->driver->owner)) return -ENODEV; - component->module = 1; - return 0; } void snd_soc_component_module_put(struct snd_soc_component *component, int upon_open) { - if (component->module && - component->driver->module_get_upon_open == !!upon_open) + if (component->driver->module_get_upon_open == !!upon_open) module_put(component->dev->driver->owner); - - component->module = 0; } int snd_soc_component_open(struct snd_soc_component *component, struct snd_pcm_substream *substream) { - int ret = 0; - - if (!component->opened && - component->driver->open) - ret = component->driver->open(component, substream); - - if (ret == 0) - component->opened = 1; - - return ret; + if (component->driver->open) + return component->driver->open(component, substream); + return 0; } int snd_soc_component_close(struct snd_soc_component *component, struct snd_pcm_substream *substream) { - int ret = 0; - - if (component->opened && - component->driver->close) - ret = component->driver->close(component, substream); - - component->opened = 0; - - return ret; + if (component->driver->close) + return component->driver->close(component, substream); + return 0; } int snd_soc_component_prepare(struct snd_soc_component *component, diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index aff27c8599ef..235baeb2d56a 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -469,28 +469,43 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream) static int soc_pcm_components_open(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_component *last = NULL; struct snd_soc_component *component; int i, ret = 0; for_each_rtd_components(rtd, i, component) { + last = component; + ret = snd_soc_component_module_get_when_open(component); if (ret < 0) { dev_err(component->dev, "ASoC: can't get module %s\n", component->name); - return ret; + break; } ret = snd_soc_component_open(component, substream); if (ret < 0) { + snd_soc_component_module_put_when_close(component); dev_err(component->dev, "ASoC: can't open component %s: %d\n", component->name, ret); - return ret; + break; } } - return 0; + if (ret < 0) { + /* rollback on error */ + for_each_rtd_components(rtd, i, component) { + if (component == last) + break; + + snd_soc_component_close(component, substream); + snd_soc_component_module_put_when_close(component); + } + } + + return ret; } static int soc_pcm_components_close(struct snd_pcm_substream *substream) @@ -585,7 +600,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) if (ret < 0) { pr_err("ASoC: %s startup failed: %d\n", rtd->dai_link->name, ret); - goto component_err; + goto rtd_startup_err; } /* startup the audio subsystem */ @@ -681,9 +696,9 @@ static int soc_pcm_open(struct snd_pcm_substream *substream) snd_soc_dai_shutdown(cpu_dai, substream); soc_rtd_shutdown(rtd, substream); -component_err: +rtd_startup_err: soc_pcm_components_close(substream); - +component_err: mutex_unlock(&rtd->card->pcm_mutex); for_each_rtd_components(rtd, i, component) { -- 2.20.1
