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@linux.intel.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
Link: https://lore.kernel.org/r/20200220094955.16968-1-kai.vehmanen@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>

diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 1866ecc8e94b2524f4b34c9b0be729287f70d198..154d02fbbfed69085e7d36c68856951f525f4c1f 100644 (file)
--- 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 ee00c09df5e791a850cca59a5f57b95b21732cf5..14e175cdeeb84b0336218c4fd190faf38947f04f 100644 (file)
--- 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 aff27c8599ef98950ad563d6f8563f8f845bfb17..235baeb2d56ab663229c4b6a3d38d968e85b5bce 100644 (file)
--- 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 @@ cpu_dai_err:
        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) {