diff mbox series

[v2,2/7] ASoC: soc-link: add mark for snd_soc_link_startup/shutdown()

Message ID 87tuwovoyt.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 2) snd_soc_link_startup/shutdown().

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 startup() was
succeeded. If rollback needed, it will check rollback flag and marked
substream pointer.

One note here is that it cares *current* startup() 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-link.h |  3 ++-
 include/sound/soc.h      |  3 +++
 sound/soc/soc-link.c     | 21 ++++++++++++++++++++-
 sound/soc/soc-pcm.c      |  4 ++--
 4 files changed, 27 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/include/sound/soc-link.h b/include/sound/soc-link.h
index 337ac5666757..dac6c0ce6ede 100644
--- a/include/sound/soc-link.h
+++ b/include/sound/soc-link.h
@@ -14,7 +14,8 @@  int snd_soc_link_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 				    struct snd_pcm_hw_params *params);
 
 int snd_soc_link_startup(struct snd_pcm_substream *substream);
-void snd_soc_link_shutdown(struct snd_pcm_substream *substream);
+void snd_soc_link_shutdown(struct snd_pcm_substream *substream,
+			   int rollback);
 int snd_soc_link_prepare(struct snd_pcm_substream *substream);
 int snd_soc_link_hw_params(struct snd_pcm_substream *substream,
 			   struct snd_pcm_hw_params *params);
diff --git a/include/sound/soc.h b/include/sound/soc.h
index b94ebff12350..7541c71c9eb8 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1159,6 +1159,9 @@  struct snd_soc_pcm_runtime {
 	unsigned int num; /* 0-based and monotonic increasing */
 	struct list_head list; /* rtd list of the soc card */
 
+	/* function mark */
+	struct snd_pcm_substream *mark_startup;
+
 	/* bit field */
 	unsigned int pop_wait:1;
 	unsigned int fe_compr:1; /* for Dynamic PCM */
diff --git a/sound/soc/soc-link.c b/sound/soc/soc-link.c
index cec70b19863e..2a8881978930 100644
--- a/sound/soc/soc-link.c
+++ b/sound/soc/soc-link.c
@@ -30,6 +30,14 @@  static inline int _soc_link_ret(struct snd_soc_pcm_runtime *rtd,
 	return ret;
 }
 
+/*
+ * We might want to check substream by using list.
+ * In such case, we can update these macros.
+ */
+#define soc_link_mark_push(rtd, substream, tgt)		((rtd)->mark_##tgt = substream)
+#define soc_link_mark_pop(rtd, substream, tgt)		((rtd)->mark_##tgt = NULL)
+#define soc_link_mark_match(rtd, substream, tgt)	((rtd)->mark_##tgt == substream)
+
 int snd_soc_link_init(struct snd_soc_pcm_runtime *rtd)
 {
 	int ret = 0;
@@ -66,16 +74,27 @@  int snd_soc_link_startup(struct snd_pcm_substream *substream)
 	    rtd->dai_link->ops->startup)
 		ret = rtd->dai_link->ops->startup(substream);
 
+	/* mark substream if succeeded */
+	if (ret == 0)
+		soc_link_mark_push(rtd, substream, startup);
+
 	return soc_link_ret(rtd, ret);
 }
 
-void snd_soc_link_shutdown(struct snd_pcm_substream *substream)
+void snd_soc_link_shutdown(struct snd_pcm_substream *substream,
+			   int rollback)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
 
+	if (rollback && !soc_link_mark_match(rtd, substream, startup))
+		return;
+
 	if (rtd->dai_link->ops &&
 	    rtd->dai_link->ops->shutdown)
 		rtd->dai_link->ops->shutdown(substream);
+
+	/* remove marked substream */
+	soc_link_mark_pop(rtd, substream, startup);
 }
 
 int snd_soc_link_prepare(struct snd_pcm_substream *substream)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index d269e2865013..047241589567 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -684,7 +684,7 @@  static int soc_pcm_close(struct snd_pcm_substream *substream)
 	for_each_rtd_dais(rtd, i, dai)
 		snd_soc_dai_shutdown(dai, substream, 0);
 
-	snd_soc_link_shutdown(substream);
+	snd_soc_link_shutdown(substream, 0);
 
 	soc_pcm_components_close(substream);
 
@@ -815,7 +815,7 @@  static int soc_pcm_open(struct snd_pcm_substream *substream)
 	for_each_rtd_dais(rtd, i, dai)
 		snd_soc_dai_shutdown(dai, substream, 1);
 
-	snd_soc_link_shutdown(substream);
+	snd_soc_link_shutdown(substream, 1);
 rtd_startup_err:
 	soc_pcm_components_close(substream);
 component_err: