[PATCH 0/5] ASoC: merge soc_compr_open() rollback and soc_compr_free()

[PATCH 0/5] ASoC: merge soc_compr_open() rollback and soc_compr_free()

[Kuninori Morimoto]

Hi Mark

soc_compr_open() does rollback when failed (A),
but, it is almost same as soc_compr_free().

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

 ^	machine_err:
 |		...
 |	out:
(A)		...
 |	pm_err:
 |		...
 v		return ret;
	}

This kind of duplicated code can be a hotbed of bugs,
thus, this patch-set share soc_compr_free() and rollback.

Kuninori Morimoto (5):
  ASoC: soc-compress: move soc_compr_free() next to soc_compr_open()
  ASoC: soc-dai: add mark for snd_soc_dai_compr_startup/shutdown()
  ASoC: soc-component: add mark for snd_soc_component_compr_open/free()
  ASoC: soc-component: add mark for snd_soc_link_compr_startup/shutdown()
  ASoC: soc-compress: add soc_compr_clean() and call it from soc_compr_open/free()

 include/sound/soc-component.h |   6 +-
 include/sound/soc-dai.h       |   4 +-
 include/sound/soc-link.h      |   3 +-
 include/sound/soc.h           |   1 +
 sound/soc/soc-component.c     |  17 +++--
 sound/soc/soc-compress.c      | 115 +++++++++++++++++-----------------
 sound/soc/soc-dai.c           |  13 +++-
 sound/soc/soc-link.c          |  11 +++-
 8 files changed, 95 insertions(+), 75 deletions(-)

-- 
2.25.1

[PATCH 1/5] ASoC: soc-compress: move soc_compr_free() next to soc_compr_open()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This patch moves soc_compr_free() next to soc_compr_open().
This is prepare for soc_compr_open() cleanup.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-compress.c | 68 ++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 5a751d5d3847..792c2aa1a3c0 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -22,6 +22,40 @@
 #include <sound/soc-link.h>
 #include <linux/pm_runtime.h>
 
+static int soc_compr_free(struct snd_compr_stream *cstream)
+{
+	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+	int stream = cstream->direction; /* SND_COMPRESS_xxx is same as SNDRV_PCM_STREAM_xxx */
+
+	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
+
+	snd_soc_runtime_deactivate(rtd, stream);
+
+	snd_soc_dai_digital_mute(codec_dai, 1, stream);
+
+	if (!snd_soc_dai_active(cpu_dai))
+		cpu_dai->rate = 0;
+
+	if (!snd_soc_dai_active(codec_dai))
+		codec_dai->rate = 0;
+
+	snd_soc_link_compr_shutdown(cstream);
+
+	snd_soc_component_compr_free(cstream, NULL);
+
+	snd_soc_dai_compr_shutdown(cpu_dai, cstream);
+
+	snd_soc_dapm_stream_stop(rtd, stream);
+
+	mutex_unlock(&rtd->card->pcm_mutex);
+
+	snd_soc_pcm_component_pm_runtime_put(rtd, cstream, 0);
+
+	return 0;
+}
+
 static int soc_compr_open(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
@@ -140,40 +174,6 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	return ret;
 }
 
-static int soc_compr_free(struct snd_compr_stream *cstream)
-{
-	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
-	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
-	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
-	int stream = cstream->direction; /* SND_COMPRESS_xxx is same as SNDRV_PCM_STREAM_xxx */
-
-	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
-
-	snd_soc_runtime_deactivate(rtd, stream);
-
-	snd_soc_dai_digital_mute(codec_dai, 1, stream);
-
-	if (!snd_soc_dai_active(cpu_dai))
-		cpu_dai->rate = 0;
-
-	if (!snd_soc_dai_active(codec_dai))
-		codec_dai->rate = 0;
-
-	snd_soc_link_compr_shutdown(cstream);
-
-	snd_soc_component_compr_free(cstream, NULL);
-
-	snd_soc_dai_compr_shutdown(cpu_dai, cstream);
-
-	snd_soc_dapm_stream_stop(rtd, stream);
-
-	mutex_unlock(&rtd->card->pcm_mutex);
-
-	snd_soc_pcm_component_pm_runtime_put(rtd, cstream, 0);
-
-	return 0;
-}
-
 static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
-- 
2.25.1

[PATCH 2/5] ASoC: soc-dai: add mark for snd_soc_dai_compr_startup/shutdown()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_compr_open() does rollback when failed (A),
but, it is almost same as soc_compr_free().

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

 ^	machine_err:
 |		...
 |	out:
(A)		...
 |	pm_err:
 |		...
 v		return ret;
	}

The difference is
soc_compr_free()  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_compr_free() and rollback.
=>	1) snd_soc_dai_compr_startup/shutdown()
	2) snd_soc_component_compr_open/free()
	3) snd_soc_link_compr_startup/shutdown()

This patch is for 1) snd_soc_dai_compr_startup/shutdown(),
and adds new cstream mark.
It will mark cstream when startup() was suceeded.
If rollback happen *after* that, it will check rollback flag
and marked cstream.

It cares *previous* startup() only now,
but we might want to check *whole* marked cstream 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-dai.h  |  4 +++-
 sound/soc/soc-compress.c |  8 ++++----
 sound/soc/soc-dai.c      | 13 ++++++++++++-
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 4bf759f025d2..6f54401d3de9 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -196,7 +196,8 @@ int snd_soc_pcm_dai_bespoke_trigger(struct snd_pcm_substream *substream,
 int snd_soc_dai_compr_startup(struct snd_soc_dai *dai,
 			      struct snd_compr_stream *cstream);
 void snd_soc_dai_compr_shutdown(struct snd_soc_dai *dai,
-				struct snd_compr_stream *cstream);
+				struct snd_compr_stream *cstream,
+				int rollback);
 int snd_soc_dai_compr_trigger(struct snd_soc_dai *dai,
 			      struct snd_compr_stream *cstream, int cmd);
 int snd_soc_dai_compr_set_params(struct snd_soc_dai *dai,
@@ -400,6 +401,7 @@ struct snd_soc_dai {
 	/* function mark */
 	struct snd_pcm_substream *mark_startup;
 	struct snd_pcm_substream *mark_hw_params;
+	struct snd_compr_stream  *mark_compr_startup;
 
 	/* bit field */
 	unsigned int probed:1;
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 792c2aa1a3c0..d85b6b174006 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -45,7 +45,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 
 	snd_soc_component_compr_free(cstream, NULL);
 
-	snd_soc_dai_compr_shutdown(cpu_dai, cstream);
+	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 0);
 
 	snd_soc_dapm_stream_stop(rtd, stream);
 
@@ -91,7 +91,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 machine_err:
 	snd_soc_component_compr_free(cstream, component);
 
-	snd_soc_dai_compr_shutdown(cpu_dai, cstream);
+	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 1);
 out:
 	mutex_unlock(&rtd->card->pcm_mutex);
 pm_err:
@@ -165,7 +165,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 machine_err:
 	snd_soc_component_compr_free(cstream, component);
 open_err:
-	snd_soc_dai_compr_shutdown(cpu_dai, cstream);
+	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 1);
 out:
 	dpcm_path_put(&list);
 be_err:
@@ -211,7 +211,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 
 	snd_soc_component_compr_free(cstream, NULL);
 
-	snd_soc_dai_compr_shutdown(cpu_dai, cstream);
+	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 0);
 
 	mutex_unlock(&fe->card->mutex);
 	return 0;
diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c
index 2686a566649b..9afc6e8c3f9f 100644
--- a/sound/soc/soc-dai.c
+++ b/sound/soc/soc-dai.c
@@ -612,16 +612,27 @@ int snd_soc_dai_compr_startup(struct snd_soc_dai *dai,
 	    dai->driver->cops->startup)
 		ret = dai->driver->cops->startup(cstream, dai);
 
+	/* mark cstream if succeeded */
+	if (ret == 0)
+		soc_dai_mark_push(dai, cstream, compr_startup);
+
 	return soc_dai_ret(dai, ret);
 }
 EXPORT_SYMBOL_GPL(snd_soc_dai_compr_startup);
 
 void snd_soc_dai_compr_shutdown(struct snd_soc_dai *dai,
-				struct snd_compr_stream *cstream)
+				struct snd_compr_stream *cstream,
+				int rollback)
 {
+	if (rollback && !soc_dai_mark_match(dai, cstream, compr_startup))
+		return;
+
 	if (dai->driver->cops &&
 	    dai->driver->cops->shutdown)
 		dai->driver->cops->shutdown(cstream, dai);
+
+	/* remove marked cstream */
+	soc_dai_mark_pop(dai, cstream, compr_startup);
 }
 EXPORT_SYMBOL_GPL(snd_soc_dai_compr_shutdown);
 
-- 
2.25.1

[PATCH 3/5] ASoC: soc-component: add mark for snd_soc_component_compr_open/free()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_compr_open() does rollback when failed (A),
but, it is almost same as soc_compr_free().

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

 ^	machine_err:
 |		...
 |	out:
(A)		...
 |	pm_err:
 |		...
 v		return ret;
	}

The difference is
soc_compr_free()  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_compr_free() and rollback.
	1) snd_soc_dai_compr_startup/shutdown()
=>	2) snd_soc_component_compr_open/free()
	3) snd_soc_link_compr_startup/shutdown()

This patch is for 2) snd_soc_component_compr_open/free(),
and adds new cstream mark.
It will mark cstream when startup() was suceeded.
If rollback happen *after* that, it will check rollback flag
and marked cstream.

It cares *previous* startup() only now,
but we might want to check *whole* marked cstream 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 |  6 +++---
 sound/soc/soc-component.c     | 17 ++++++++---------
 sound/soc/soc-compress.c      | 14 ++++++--------
 3 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 984dd7353066..9140b5fa19a4 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -221,6 +221,7 @@ struct snd_soc_component {
 	struct snd_pcm_substream *mark_module;
 	struct snd_pcm_substream *mark_open;
 	struct snd_pcm_substream *mark_hw_params;
+	struct snd_compr_stream  *mark_compr_open;
 	void *mark_pm;
 
 #ifdef CONFIG_DEBUG_FS
@@ -444,10 +445,9 @@ int snd_soc_component_of_xlate_dai_id(struct snd_soc_component *component,
 int snd_soc_component_of_xlate_dai_name(struct snd_soc_component *component,
 					struct of_phandle_args *args,
 					const char **dai_name);
-int snd_soc_component_compr_open(struct snd_compr_stream *cstream,
-				 struct snd_soc_component **last);
+int snd_soc_component_compr_open(struct snd_compr_stream *cstream);
 void snd_soc_component_compr_free(struct snd_compr_stream *cstream,
-				  struct snd_soc_component *last);
+				  int rollback);
 int snd_soc_component_compr_trigger(struct snd_compr_stream *cstream, int cmd);
 int snd_soc_component_compr_set_params(struct snd_compr_stream *cstream,
 				       struct snd_compr_params *params);
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index 5d3a8dcd8d49..434987a64353 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -421,8 +421,7 @@ EXPORT_SYMBOL_GPL(snd_soc_component_exit_regmap);
 
 #endif
 
-int snd_soc_component_compr_open(struct snd_compr_stream *cstream,
-				 struct snd_soc_component **last)
+int snd_soc_component_compr_open(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
@@ -432,32 +431,32 @@ int snd_soc_component_compr_open(struct snd_compr_stream *cstream,
 		if (component->driver->compress_ops &&
 		    component->driver->compress_ops->open) {
 			ret = component->driver->compress_ops->open(component, cstream);
-			if (ret < 0) {
-				*last = component;
+			if (ret < 0)
 				return soc_component_ret(component, ret);
-			}
 		}
+		soc_component_mark_push(component, cstream, compr_open);
 	}
 
-	*last = NULL;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_soc_component_compr_open);
 
 void snd_soc_component_compr_free(struct snd_compr_stream *cstream,
-				  struct snd_soc_component *last)
+				  int rollback)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_component *component;
 	int i;
 
 	for_each_rtd_components(rtd, i, component) {
-		if (component == last)
-			break;
+		if (rollback && !soc_component_mark_match(component, cstream, compr_open))
+			continue;
 
 		if (component->driver->compress_ops &&
 		    component->driver->compress_ops->free)
 			component->driver->compress_ops->free(component, cstream);
+
+		soc_component_mark_pop(component, cstream, compr_open);
 	}
 }
 EXPORT_SYMBOL_GPL(snd_soc_component_compr_free);
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index d85b6b174006..8b6a6bd482d1 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -43,7 +43,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 
 	snd_soc_link_compr_shutdown(cstream);
 
-	snd_soc_component_compr_free(cstream, NULL);
+	snd_soc_component_compr_free(cstream, 0);
 
 	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 0);
 
@@ -59,7 +59,6 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 static int soc_compr_open(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
-	struct snd_soc_component *component = NULL;
 	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
 	int stream = cstream->direction; /* SND_COMPRESS_xxx is same as SNDRV_PCM_STREAM_xxx */
 	int ret;
@@ -74,7 +73,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	if (ret < 0)
 		goto out;
 
-	ret = snd_soc_component_compr_open(cstream, &component);
+	ret = snd_soc_component_compr_open(cstream);
 	if (ret < 0)
 		goto machine_err;
 
@@ -89,7 +88,7 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 	return 0;
 
 machine_err:
-	snd_soc_component_compr_free(cstream, component);
+	snd_soc_component_compr_free(cstream, 1);
 
 	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 1);
 out:
@@ -105,7 +104,6 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	struct snd_soc_pcm_runtime *fe = cstream->private_data;
 	struct snd_pcm_substream *fe_substream =
 		 fe->pcm->streams[cstream->direction].substream;
-	struct snd_soc_component *component;
 	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(fe, 0);
 	struct snd_soc_dpcm *dpcm;
 	struct snd_soc_dapm_widget_list *list;
@@ -142,7 +140,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	if (ret < 0)
 		goto out;
 
-	ret = snd_soc_component_compr_open(cstream, &component);
+	ret = snd_soc_component_compr_open(cstream);
 	if (ret < 0)
 		goto open_err;
 
@@ -163,7 +161,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	return 0;
 
 machine_err:
-	snd_soc_component_compr_free(cstream, component);
+	snd_soc_component_compr_free(cstream, 1);
 open_err:
 	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 1);
 out:
@@ -209,7 +207,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 
 	snd_soc_link_compr_shutdown(cstream);
 
-	snd_soc_component_compr_free(cstream, NULL);
+	snd_soc_component_compr_free(cstream, 0);
 
 	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 0);
 
-- 
2.25.1

[PATCH 4/5] ASoC: soc-component: add mark for snd_soc_link_compr_startup/shutdown()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_compr_open() does rollback when failed (A),
but, it is almost same as soc_compr_free().

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

 ^	machine_err:
 |		...
 |	out:
(A)		...
 |	pm_err:
 |		...
 v		return ret;
	}

The difference is
soc_compr_free()  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_compr_free() and rollback.
	1) snd_soc_dai_compr_startup/shutdown()
	2) snd_soc_component_compr_open/free()
=>	3) snd_soc_link_compr_startup/shutdown()

This patch is for 3) snd_soc_link_compr_startup/shutdown()
and adds new cstream mark.
It will mark cstream when startup() was suceeded.
If rollback happen *after* that, it will check rollback flag
and marked cstream.

It cares *previous* startup() only now,
but we might want to check *whole* marked cstream 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-link.h |  3 ++-
 include/sound/soc.h      |  1 +
 sound/soc/soc-compress.c |  4 ++--
 sound/soc/soc-link.c     | 11 ++++++++++-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc-link.h b/include/sound/soc-link.h
index eff34fc7d3d3..4c68b06d6fe6 100644
--- a/include/sound/soc-link.h
+++ b/include/sound/soc-link.h
@@ -24,7 +24,8 @@ void snd_soc_link_hw_free(struct snd_pcm_substream *substream,
 int snd_soc_link_trigger(struct snd_pcm_substream *substream, int cmd);
 
 int snd_soc_link_compr_startup(struct snd_compr_stream *cstream);
-void snd_soc_link_compr_shutdown(struct snd_compr_stream *cstream);
+void snd_soc_link_compr_shutdown(struct snd_compr_stream *cstream,
+				 int rollback);
 int snd_soc_link_compr_set_params(struct snd_compr_stream *cstream);
 
 #endif /* __SOC_LINK_H */
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 4a9958b9b532..33c289f6097c 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1163,6 +1163,7 @@ struct snd_soc_pcm_runtime {
 	/* function mark */
 	struct snd_pcm_substream *mark_startup;
 	struct snd_pcm_substream *mark_hw_params;
+	struct snd_compr_stream  *mark_compr_startup;
 
 	/* bit field */
 	unsigned int pop_wait:1;
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 8b6a6bd482d1..6de22b2f8197 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -41,7 +41,7 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	if (!snd_soc_dai_active(codec_dai))
 		codec_dai->rate = 0;
 
-	snd_soc_link_compr_shutdown(cstream);
+	snd_soc_link_compr_shutdown(cstream, 0);
 
 	snd_soc_component_compr_free(cstream, 0);
 
@@ -205,7 +205,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 
 	fe->dpcm[stream].runtime = NULL;
 
-	snd_soc_link_compr_shutdown(cstream);
+	snd_soc_link_compr_shutdown(cstream, 0);
 
 	snd_soc_component_compr_free(cstream, 0);
 
diff --git a/sound/soc/soc-link.c b/sound/soc/soc-link.c
index 409ae4940da3..26cc60f8dcfb 100644
--- a/sound/soc/soc-link.c
+++ b/sound/soc/soc-link.c
@@ -162,17 +162,26 @@ int snd_soc_link_compr_startup(struct snd_compr_stream *cstream)
 	    rtd->dai_link->compr_ops->startup)
 		ret = rtd->dai_link->compr_ops->startup(cstream);
 
+	if (ret == 0)
+		soc_link_mark_push(rtd, cstream, compr_startup);
+
 	return soc_link_ret(rtd, ret);
 }
 EXPORT_SYMBOL_GPL(snd_soc_link_compr_startup);
 
-void snd_soc_link_compr_shutdown(struct snd_compr_stream *cstream)
+void snd_soc_link_compr_shutdown(struct snd_compr_stream *cstream,
+				 int rollback)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 
+	if (rollback && !soc_link_mark_match(rtd, cstream, compr_startup))
+		return;
+
 	if (rtd->dai_link->compr_ops &&
 	    rtd->dai_link->compr_ops->shutdown)
 		rtd->dai_link->compr_ops->shutdown(cstream);
+
+	soc_link_mark_pop(rtd, cstream, compr_startup);
 }
 EXPORT_SYMBOL_GPL(snd_soc_link_compr_shutdown);
 
-- 
2.25.1

[PATCH 5/5] ASoC: soc-compress: add soc_compr_clean() and call it from soc_compr_open/free()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_compr_open() does rollback when failed (A),
but, it is almost same as soc_compr_free().

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

 ^	machine_err:
 |		...
 |	out:
(A)		...
 |	pm_err:
 |		...
 v		return ret;
	}

The difference is
soc_compr_free()  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_compr_free() and rollback.

Now, soc_compr_open/free() are handling
	1) snd_soc_dai_compr_startup/shutdown()
	2) snd_soc_component_compr_open/free()
	3) snd_soc_link_compr_startup/shutdown()

Now, 1) to 3) are handled.
This patch adds new soc_compr_clean() and call it from
soc_compr_open() as rollback, and from soc_compr_free_free() as
normal close handler.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-compress.c | 45 ++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 6de22b2f8197..246a5e32e22a 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -22,7 +22,7 @@
 #include <sound/soc-link.h>
 #include <linux/pm_runtime.h>
 
-static int soc_compr_free(struct snd_compr_stream *cstream)
+static int soc_compr_clean(struct snd_compr_stream *cstream, int rollback)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
 	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
@@ -31,7 +31,8 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
-	snd_soc_runtime_deactivate(rtd, stream);
+	if (!rollback)
+		snd_soc_runtime_deactivate(rtd, stream);
 
 	snd_soc_dai_digital_mute(codec_dai, 1, stream);
 
@@ -41,21 +42,27 @@ static int soc_compr_free(struct snd_compr_stream *cstream)
 	if (!snd_soc_dai_active(codec_dai))
 		codec_dai->rate = 0;
 
-	snd_soc_link_compr_shutdown(cstream, 0);
+	snd_soc_link_compr_shutdown(cstream, rollback);
 
-	snd_soc_component_compr_free(cstream, 0);
+	snd_soc_component_compr_free(cstream, rollback);
 
-	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 0);
+	snd_soc_dai_compr_shutdown(cpu_dai, cstream, rollback);
 
-	snd_soc_dapm_stream_stop(rtd, stream);
+	if (!rollback)
+		snd_soc_dapm_stream_stop(rtd, stream);
 
 	mutex_unlock(&rtd->card->pcm_mutex);
 
-	snd_soc_pcm_component_pm_runtime_put(rtd, cstream, 0);
+	snd_soc_pcm_component_pm_runtime_put(rtd, cstream, rollback);
 
 	return 0;
 }
 
+static int soc_compr_free(struct snd_compr_stream *cstream)
+{
+	return soc_compr_clean(cstream, 0);
+}
+
 static int soc_compr_open(struct snd_compr_stream *cstream)
 {
 	struct snd_soc_pcm_runtime *rtd = cstream->private_data;
@@ -65,36 +72,28 @@ static int soc_compr_open(struct snd_compr_stream *cstream)
 
 	ret = snd_soc_pcm_component_pm_runtime_get(rtd, cstream);
 	if (ret < 0)
-		goto pm_err;
+		goto err_no_lock;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
 	ret = snd_soc_dai_compr_startup(cpu_dai, cstream);
 	if (ret < 0)
-		goto out;
+		goto err;
 
 	ret = snd_soc_component_compr_open(cstream);
 	if (ret < 0)
-		goto machine_err;
+		goto err;
 
 	ret = snd_soc_link_compr_startup(cstream);
 	if (ret < 0)
-		goto machine_err;
+		goto err;
 
 	snd_soc_runtime_activate(rtd, stream);
-
-	mutex_unlock(&rtd->card->pcm_mutex);
-
-	return 0;
-
-machine_err:
-	snd_soc_component_compr_free(cstream, 1);
-
-	snd_soc_dai_compr_shutdown(cpu_dai, cstream, 1);
-out:
+err:
 	mutex_unlock(&rtd->card->pcm_mutex);
-pm_err:
-	snd_soc_pcm_component_pm_runtime_put(rtd, cstream, 1);
+err_no_lock:
+	if (ret < 0)
+		soc_compr_clean(cstream, 1);
 
 	return ret;
 }
-- 
2.25.1

Re: [PATCH 0/5] ASoC: merge soc_compr_open() rollback and soc_compr_free()

[Mark Brown]

On 19 Nov 2020 08:49:32 +0900, Kuninori Morimoto wrote:
> soc_compr_open() does rollback when failed (A),
> but, it is almost same as soc_compr_free().
> 
> 	static int soc_compr_open(xxx)
> 	{
> 		...
> 		if (ret < 0)
> 			goto xxx_err;
> 		...
> 		return 0;
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] ASoC: soc-compress: move soc_compr_free() next to soc_compr_open()
      commit: 15a7b8c13653cc88de2db89af486e904aedc75ec
[2/5] ASoC: soc-dai: add mark for snd_soc_dai_compr_startup/shutdown()
      commit: 1e6a93cf74979e167cef8d29f6689705d9ec6735
[3/5] ASoC: soc-component: add mark for snd_soc_component_compr_open/free()
      commit: f94ba9ac20fab9af08240fde3741edf73655411d
[4/5] ASoC: soc-component: add mark for snd_soc_link_compr_startup/shutdown()
      commit: cd7c7d10e8f4ab1dac0bdb2019abc0fad995a788
[5/5] ASoC: soc-compress: add soc_compr_clean() and call it from soc_compr_open/free()
      commit: 453d32c2f7f7375c223eaf3a0a32efbb71bbd3f3

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark