diff mbox series

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

Message ID 87sgc8voyp.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 Aug. 27, 2020, 1:50 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().

The idea of having bit-flag or counter is not enough for this purpose.
For example if one DAI is used for 2xPlaybacks for some reasons,
and if 1st Playback was succeeded but 2nd Playback was failed,
2nd Playback rollback doesn't need to call shutdown.
But it has succeeded bit-flag or counter via 1st Playback,
thus, 2nd Playback rollback will call unneeded shutdown.
And 1st Playback's necessary shutdown will not be called,
because bit-flag or counter was cleared by wrong 2nd Playback rollback.

To avoid such case, this patch marks substream pointer when open() was
succeeded. If rollback needed, it will check rollback flag and marked
substream pointer.

One note here is that it cares *current* open() only now.
but we might want to check *whole* marked substream in the future.
This patch is using macro named "push/pop", so that it can be easily update.

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 089ea9441fd1..a812202ec595 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;
@@ -370,17 +374,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)
@@ -424,7 +430,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 5504b92946e3..147b46e29ff4 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)
+
 void snd_soc_component_set_aux(struct snd_soc_component *component,
 			       struct snd_soc_aux_dev *aux)
 {
@@ -238,6 +246,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;
@@ -246,14 +255,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,
@@ -264,17 +284,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 047241589567..b9d6ab301bda 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);