From: Kai Vehmanen <kai.vehmanen@linux.intel.com>
To: broonie@kernel.org, alsa-devel@alsa-project.org,
kuninori.morimoto.gx@renesas.com, digetx@gmail.com
Cc: pierre-louis.bossart@linux.intel.com,
kai.vehmanen@linux.intel.com, ranjani.sridharan@linux.intel.com
Subject: [PATCH] ASoC: soc-pcm: fix state tracking error in snd_soc_component_open/close()
Date: Thu, 20 Feb 2020 11:49:55 +0200 [thread overview]
Message-ID: <20200220094955.16968-1-kai.vehmanen@linux.intel.com> (raw)
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@linux.intel.com>
---
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 63f67eb7c077..9e761f932887 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.17.1
next reply other threads:[~2020-02-20 9:51 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-20 9:49 Kai Vehmanen [this message]
2020-02-24 12:01 ` [PATCH] ASoC: soc-pcm: fix state tracking error in snd_soc_component_open/close() Mark Brown
2020-02-24 15:47 ` Dmitry Osipenko
2020-02-24 17:05 ` Pierre-Louis Bossart
2020-02-24 22:21 ` Applied "ASoC: soc-pcm: fix state tracking error in snd_soc_component_open/close()" to the asoc tree Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200220094955.16968-1-kai.vehmanen@linux.intel.com \
--to=kai.vehmanen@linux.intel.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=digetx@gmail.com \
--cc=kuninori.morimoto.gx@renesas.com \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=ranjani.sridharan@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox