diff mbox series

[3/7] ASoC: soc-component: add mark for soc_pcm_components_open/close()

Message ID 87sgdcktzv.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show
Series ASoC: merge soc_pcm_open() rollback and soc_pcm_close() | expand

Commit Message

Kuninori Morimoto July 28, 2020, 6:57 a.m. UTC 
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_pcm_open() does rollback when failed (A),
but, it is almost same as soc_pcm_close().

	static int soc_pcm_open(xxx)
	{
		...
		if (ret < 0)
			goto xxx_err;
		...
		return 0;

 ^	config_err:
 |		...
 |	rtd_startup_err:
(A)		...
 |	component_err:
 |		...
 v		return ret;
	}

The difference is
soc_pcm_close() is for all dai/component/substream,
rollback        is for succeeded part only.

This kind of duplicated code can be a hotbed of bugs,
thus, we want to share soc_pcm_close() and rollback.

Now, soc_pcm_open/close() are handling
	1) snd_soc_dai_startup/shutdown()
	2) snd_soc_link_startup/shutdown()
=>	3) snd_soc_component_module_get/put()
=>	4) snd_soc_component_open/close()
	5) pm_runtime_put/get()

This patch is for 3) snd_soc_component_module_get/put()
4) snd_soc_component_open/close(),
and adds new substream mark.
It will mark substream when open was suceeded.
If rollback happen *after* that, it will check rollback flag
and marked substream.

It cares *previous* open() only now,
but we might want to check *whole* marked substream in the future.
This patch is using macro so that it can be easily adjust to it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc-component.h | 23 +++++++++++++++--------
 sound/soc/soc-component.c     | 35 +++++++++++++++++++++++++++++++++--
 sound/soc/soc-pcm.c           | 28 +++++++---------------------
 3 files changed, 55 insertions(+), 31 deletions(-)
diff mbox series

Patch

diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 8917b15eccae..8aabadfbdebc 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -217,6 +217,10 @@  struct snd_soc_component {
 	/* machine specific init */
 	int (*init)(struct snd_soc_component *component);
 
+	/* function mark */
+	struct snd_pcm_substream *mark_module;
+	struct snd_pcm_substream *mark_open;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *debugfs_root;
 	const char *debugfs_prefix;
@@ -373,17 +377,19 @@  void snd_soc_component_exit_regmap(struct snd_soc_component *component);
 #endif
 
 #define snd_soc_component_module_get_when_probe(component)\
-	snd_soc_component_module_get(component, 0)
-#define snd_soc_component_module_get_when_open(component)	\
-	snd_soc_component_module_get(component, 1)
+	snd_soc_component_module_get(component, NULL, 0)
+#define snd_soc_component_module_get_when_open(component, substream)	\
+	snd_soc_component_module_get(component, substream, 1)
 int snd_soc_component_module_get(struct snd_soc_component *component,
+				 struct snd_pcm_substream *substream,
 				 int upon_open);
 #define snd_soc_component_module_put_when_remove(component)	\
-	snd_soc_component_module_put(component, 0)
-#define snd_soc_component_module_put_when_close(component)	\
-	snd_soc_component_module_put(component, 1)
+	snd_soc_component_module_put(component, NULL, 0, 0)
+#define snd_soc_component_module_put_when_close(component, substream, rollback) \
+	snd_soc_component_module_put(component, substream, 1, rollback)
 void snd_soc_component_module_put(struct snd_soc_component *component,
-				  int upon_open);
+				  struct snd_pcm_substream *substream,
+				  int upon_open, int rollback);
 
 static inline void snd_soc_component_set_drvdata(struct snd_soc_component *c,
 						 void *data)
@@ -427,7 +433,8 @@  int snd_soc_component_force_enable_pin_unlocked(
 int snd_soc_component_open(struct snd_soc_component *component,
 			   struct snd_pcm_substream *substream);
 int snd_soc_component_close(struct snd_soc_component *component,
-			    struct snd_pcm_substream *substream);
+			    struct snd_pcm_substream *substream,
+			    int rollback);
 void snd_soc_component_suspend(struct snd_soc_component *component);
 void snd_soc_component_resume(struct snd_soc_component *component);
 int snd_soc_component_is_suspended(struct snd_soc_component *component);
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index dcc89fa8913a..6544e74cf7f7 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -33,6 +33,14 @@  static inline int _soc_component_ret(struct snd_soc_component *component,
 	return ret;
 }
 
+/*
+ * We might want to check substream by using list.
+ * In such case, we can update these macros.
+ */
+#define soc_component_mark_push(component, substream, tgt)	((component)->mark_##tgt = substream)
+#define soc_component_mark_pop(component, substream, tgt)	((component)->mark_##tgt = NULL)
+#define soc_component_mark_match(component, substream, tgt)	((component)->mark_##tgt == substream)
+
 int snd_soc_component_initialize(struct snd_soc_component *component,
 				 const struct snd_soc_component_driver *driver,
 				 struct device *dev, const char *name)
@@ -254,6 +262,7 @@  int snd_soc_component_set_jack(struct snd_soc_component *component,
 EXPORT_SYMBOL_GPL(snd_soc_component_set_jack);
 
 int snd_soc_component_module_get(struct snd_soc_component *component,
+				 struct snd_pcm_substream *substream,
 				 int upon_open)
 {
 	int ret = 0;
@@ -262,14 +271,25 @@  int snd_soc_component_module_get(struct snd_soc_component *component,
 	    !try_module_get(component->dev->driver->owner))
 		ret = -ENODEV;
 
+	/* mark substream if succeeded */
+	if (ret == 0)
+		soc_component_mark_push(component, substream, module);
+
 	return soc_component_ret(component, ret);
 }
 
 void snd_soc_component_module_put(struct snd_soc_component *component,
-				  int upon_open)
+				  struct snd_pcm_substream *substream,
+				  int upon_open, int rollback)
 {
+	if (rollback && !soc_component_mark_match(component, substream, module))
+		return;
+
 	if (component->driver->module_get_upon_open == !!upon_open)
 		module_put(component->dev->driver->owner);
+
+	/* remove marked substream */
+	soc_component_mark_pop(component, substream, module);
 }
 
 int snd_soc_component_open(struct snd_soc_component *component,
@@ -280,17 +300,28 @@  int snd_soc_component_open(struct snd_soc_component *component,
 	if (component->driver->open)
 		ret = component->driver->open(component, substream);
 
+	/* mark substream if succeeded */
+	if (ret == 0)
+		soc_component_mark_push(component, substream, open);
+
 	return soc_component_ret(component, ret);
 }
 
 int snd_soc_component_close(struct snd_soc_component *component,
-			    struct snd_pcm_substream *substream)
+			    struct snd_pcm_substream *substream,
+			    int rollback)
 {
 	int ret = 0;
 
+	if (rollback && !soc_component_mark_match(component, substream, open))
+		return 0;
+
 	if (component->driver->close)
 		ret = component->driver->close(component, substream);
 
+	/* remove marked substream */
+	soc_component_mark_pop(component, substream, open);
+
 	return soc_component_ret(component, ret);
 }
 
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 7e809aa56e8d..c034b8a1e6f9 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -609,14 +609,11 @@  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 = asoc_substream_to_rtd(substream);
-	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);
+		ret = snd_soc_component_module_get_when_open(component, substream);
 		if (ret < 0) {
 			dev_err(component->dev,
 				"ASoC: can't get module %s\n",
@@ -626,7 +623,6 @@  static int soc_pcm_components_open(struct snd_pcm_substream *substream)
 
 		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);
@@ -634,32 +630,22 @@  static int soc_pcm_components_open(struct snd_pcm_substream *substream)
 		}
 	}
 
-	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)
+static int soc_pcm_components_close(struct snd_pcm_substream *substream,
+				    int rollback)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
 	struct snd_soc_component *component;
 	int i, r, ret = 0;
 
 	for_each_rtd_components(rtd, i, component) {
-		r = snd_soc_component_close(component, substream);
+		r = snd_soc_component_close(component, substream, rollback);
 		if (r < 0)
 			ret = r; /* use last ret */
 
-		snd_soc_component_module_put_when_close(component);
+		snd_soc_component_module_put_when_close(component, substream, rollback);
 	}
 
 	return ret;
@@ -686,7 +672,7 @@  static int soc_pcm_close(struct snd_pcm_substream *substream)
 
 	snd_soc_link_shutdown(substream, 0);
 
-	soc_pcm_components_close(substream);
+	soc_pcm_components_close(substream, 0);
 
 	snd_soc_dapm_stream_stop(rtd, substream->stream);
 
@@ -817,7 +803,7 @@  static int soc_pcm_open(struct snd_pcm_substream *substream)
 
 	snd_soc_link_shutdown(substream, 1);
 rtd_startup_err:
-	soc_pcm_components_close(substream);
+	soc_pcm_components_close(substream, 1);
 component_err:
 	mutex_unlock(&rtd->card->pcm_mutex);