All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] ASoC: tidyup error message timing
@ 2021-03-15  0:57 Kuninori Morimoto
  2021-03-15  0:57 ` [PATCH 01/14] ASoC: soc-pcm: indicate error message at soc_pcm_open() Kuninori Morimoto
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:57 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


Hi Mark

Indicating error message when failed case is very useful for debuging.
In many case, it uses below style.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

I know many people have many opinion, but if function() indicates error
message, we can get same and detail information without forgot, and it is better.
This patch-set tidyup to do it.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}


Kuninori Morimoto (14):
  ASoC: soc-pcm: indicate error message at soc_pcm_open()
  ASoC: soc-pcm: indicate error message at soc_pcm_hw_params()
  ASoC: soc-pcm: indicate error message at soc_pcm_prepare()
  ASoC: soc-pcm: indicate error message at dpcm_path_get()
  ASoC: soc-pcm: indicate error message at dpcm_be_dai_trigger()
  ASoC: soc-pcm: indicate error message at dpcm_apply_symmetry()
  ASoC: soc-pcm: indicate error message at dpcm_run_update_startup/shutdown()
  ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_startup()
  ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_hw_params()
  ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_prepare()
  ASoC: soc-pcm: don't indicate error message for soc_pcm_hw_free()
  ASoC: soc-pcm: don't indicate error message for dpcm_be_dai_hw_free()
  ASoC: don't indicate error message for snd_soc_[pcm_]dai_xxx()
  ASoC: don't indicate error message for snd_soc_[pcm_]component_xxx()

 include/sound/soc-dpcm.h |   2 +-
 sound/soc/soc-compress.c |   9 +-
 sound/soc/soc-core.c     |  22 +----
 sound/soc/soc-dapm.c     |  24 ++---
 sound/soc/soc-pcm.c      | 197 +++++++++++++++++++--------------------
 5 files changed, 108 insertions(+), 146 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/14] ASoC: soc-pcm: indicate error message at soc_pcm_open()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
@ 2021-03-15  0:57 ` Kuninori Morimoto
  2021-03-15  0:57 ` [PATCH 02/14] ASoC: soc-pcm: indicate error message at soc_pcm_hw_params() Kuninori Morimoto
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:57 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

This patch follow above style at soc_pcm_open().

By this patch, dpcm_fe/be_dai_startup(...)
temporary lacks FE/BE error info, but it will reborn soon.

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

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index a27385ab7b55..ad4b67bd0306 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -792,8 +792,10 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 err:
 	mutex_unlock(&rtd->card->pcm_mutex);
 pm_err:
-	if (ret < 0)
+	if (ret < 0) {
 		soc_pcm_clean(substream, 1);
+		dev_err(rtd->dev, "%s() failed (%d)", __func__, ret);
+	}
 
 	return ret;
 }
@@ -1504,7 +1506,6 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream)
 		be_substream->runtime = be->dpcm[stream].runtime;
 		err = soc_pcm_open(be_substream);
 		if (err < 0) {
-			dev_err(be->dev, "ASoC: BE open failed %d\n", err);
 			be->dpcm[stream].users--;
 			if (be->dpcm[stream].users < 0)
 				dev_err(be->dev, "ASoC: no users %s at unwind %d\n",
@@ -1744,10 +1745,8 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
 
 	/* start the DAI frontend */
 	ret = soc_pcm_open(fe_substream);
-	if (ret < 0) {
-		dev_err(fe->dev,"ASoC: failed to start FE %d\n", ret);
+	if (ret < 0)
 		goto unwind;
-	}
 
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/14] ASoC: soc-pcm: indicate error message at soc_pcm_hw_params()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
  2021-03-15  0:57 ` [PATCH 01/14] ASoC: soc-pcm: indicate error message at soc_pcm_open() Kuninori Morimoto
@ 2021-03-15  0:57 ` Kuninori Morimoto
  2021-03-15  0:57 ` [PATCH 03/14] ASoC: soc-pcm: indicate error message at soc_pcm_prepare() Kuninori Morimoto
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:57 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

This patch follow above style at soc_pcm_hw_params().

By this patch, dpcm_fe/be_dai_hw_params(...)
temporary lacks FE/BE error info, but it will reborn soon.

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

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index ad4b67bd0306..a956d1852ade 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1001,8 +1001,10 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
 out:
 	mutex_unlock(&rtd->card->pcm_mutex);
 
-	if (ret < 0)
+	if (ret < 0) {
 		soc_pcm_hw_clean(substream, 1);
+		dev_err(rtd->dev, "ASoC: %s() failed (%d)\n", __func__, ret);
+	}
 
 	return ret;
 }
@@ -1905,11 +1907,8 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream)
 			be->dai_link->name);
 
 		ret = soc_pcm_hw_params(be_substream, &dpcm->hw_params);
-		if (ret < 0) {
-			dev_err(dpcm->be->dev,
-				"ASoC: hw_params BE failed %d\n", ret);
+		if (ret < 0)
 			goto unwind;
-		}
 
 		be->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_PARAMS;
 	}
@@ -1964,10 +1963,9 @@ static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream,
 
 	/* call hw_params on the frontend */
 	ret = soc_pcm_hw_params(substream, params);
-	if (ret < 0) {
-		dev_err(fe->dev,"ASoC: hw_params FE failed %d\n", ret);
+	if (ret < 0)
 		dpcm_be_dai_hw_free(fe, stream);
-	 } else
+	else
 		fe->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_PARAMS;
 
 out:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/14] ASoC: soc-pcm: indicate error message at soc_pcm_prepare()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
  2021-03-15  0:57 ` [PATCH 01/14] ASoC: soc-pcm: indicate error message at soc_pcm_open() Kuninori Morimoto
  2021-03-15  0:57 ` [PATCH 02/14] ASoC: soc-pcm: indicate error message at soc_pcm_hw_params() Kuninori Morimoto
@ 2021-03-15  0:57 ` Kuninori Morimoto
  2021-03-15  0:57 ` [PATCH 04/14] ASoC: soc-pcm: indicate error message at dpcm_path_get() Kuninori Morimoto
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:57 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

This patch follow above style at soc_pcm_prepare().

By this patch, dpcm_fe/be_dai_prepare(...)
temporary lacks FE/BE error info, but it will reborn soon.

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

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index a956d1852ade..82daf79f5b3f 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -852,6 +852,10 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
 
 out:
 	mutex_unlock(&rtd->card->pcm_mutex);
+
+	if (ret < 0)
+		dev_err(rtd->dev, "ASoC: %s() failed (%d)\n", __func__, ret);
+
 	return ret;
 }
 
@@ -2234,11 +2238,8 @@ int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream)
 			be->dai_link->name);
 
 		ret = soc_pcm_prepare(be_substream);
-		if (ret < 0) {
-			dev_err(be->dev, "ASoC: backend prepare failed %d\n",
-				ret);
+		if (ret < 0)
 			break;
-		}
 
 		be->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE;
 	}
@@ -2270,11 +2271,8 @@ static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream)
 
 	/* call prepare on the frontend */
 	ret = soc_pcm_prepare(substream);
-	if (ret < 0) {
-		dev_err(fe->dev,"ASoC: prepare FE %s failed\n",
-			fe->dai_link->name);
+	if (ret < 0)
 		goto out;
-	}
 
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/14] ASoC: soc-pcm: indicate error message at dpcm_path_get()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (2 preceding siblings ...)
  2021-03-15  0:57 ` [PATCH 03/14] ASoC: soc-pcm: indicate error message at soc_pcm_prepare() Kuninori Morimoto
@ 2021-03-15  0:57 ` Kuninori Morimoto
  2021-03-15  0:57 ` [PATCH 05/14] ASoC: soc-pcm: indicate error message at dpcm_be_dai_trigger() Kuninori Morimoto
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:57 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

Now, many place uses dpcm_path_get() like below

	ret = dpcm_path_get(...);
	if (ret < 0)
		goto error;
(A)	else if (ret == 0)
		dev_dbg(...)

But here, (A) part can be indicated at dpcm_path_get() not caller.
It is simple and readable code.

This patch do it.
Small detail behaviors will be exchanged by this patch.

	1) indicates debug info (= path numbers) if path > 0 case only
	   (It was *always* indicated).
	2) soc_dpcm_fe_runtime_update() is indicating error message
	   for paths < 0 case, but it is already done at dpcm_path_get().
	   Thus just remove it. but dev_dbg() vs dev_warn() is exchanged.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-compress.c |  4 +---
 sound/soc/soc-pcm.c      | 19 +++++++------------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 89445ba0e86b..94f1f7a9dd53 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -115,9 +115,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream)
 	ret = dpcm_path_get(fe, stream, &list);
 	if (ret < 0)
 		goto be_err;
-	else if (ret == 0)
-		dev_dbg(fe->dev, "Compress ASoC: %s no valid %s route\n",
-			fe->dai_link->name, stream ? "capture" : "playback");
+
 	/* calculate valid and active FE <-> BE dpcms */
 	dpcm_process_paths(fe, stream, &list, 1);
 	fe->dpcm[stream].runtime = fe_substream->runtime;
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 82daf79f5b3f..b21c53becd11 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1290,8 +1290,12 @@ int dpcm_path_get(struct snd_soc_pcm_runtime *fe,
 			fe->card->component_chaining ?
 				NULL : dpcm_end_walk_at_be);
 
-	dev_dbg(fe->dev, "ASoC: found %d audio %s paths\n", paths,
+	if (paths > 0)
+		dev_dbg(fe->dev, "ASoC: found %d audio %s paths\n", paths,
 			stream ? "capture" : "playback");
+	else if (paths == 0)
+		dev_dbg(fe->dev, "ASoC: %s no valid %s path\n", fe->dai_link->name,
+			 stream ? "capture" : "playback");
 
 	return paths;
 }
@@ -2457,13 +2461,8 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new)
 			continue;
 
 		paths = dpcm_path_get(fe, stream, &list);
-		if (paths < 0) {
-			dev_warn(fe->dev, "ASoC: %s no valid %s path\n",
-				 fe->dai_link->name,
-				 stream == SNDRV_PCM_STREAM_PLAYBACK ?
-				 "playback" : "capture");
+		if (paths < 0)
 			return paths;
-		}
 
 		/* update any playback/capture paths */
 		count = dpcm_process_paths(fe, stream, &list, new);
@@ -2556,12 +2555,8 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream)
 	fe->dpcm[stream].runtime = fe_substream->runtime;
 
 	ret = dpcm_path_get(fe, stream, &list);
-	if (ret < 0) {
+	if (ret < 0)
 		goto open_end;
-	} else if (ret == 0) {
-		dev_dbg(fe->dev, "ASoC: %s no valid %s route\n",
-			fe->dai_link->name, stream ? "capture" : "playback");
-	}
 
 	/* calculate valid and active FE <-> BE dpcms */
 	dpcm_process_paths(fe, stream, &list, 1);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/14] ASoC: soc-pcm: indicate error message at dpcm_be_dai_trigger()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (3 preceding siblings ...)
  2021-03-15  0:57 ` [PATCH 04/14] ASoC: soc-pcm: indicate error message at dpcm_path_get() Kuninori Morimoto
@ 2021-03-15  0:57 ` Kuninori Morimoto
  2021-03-15  0:58 ` [PATCH 06/14] ASoC: soc-pcm: indicate error message at dpcm_apply_symmetry() Kuninori Morimoto
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:57 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

Now, dpcm_be_dai_trigger() user uses it like below.

	err = dpcm_be_dai_trigger(...);
	if (err < 0)
		dev_err(..., "ASoC: trigger FE failed %d\n", err);

But we can get more detail information if dpcm_be_dai_trigger() itself
had dev_err(). And above error message is confusable,
failed is *BE*, not *FE*.

This patch indicates error message at dpcm_be_dai_trigger().

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

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index b21c53becd11..35c62062b8f8 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1985,14 +1985,15 @@ static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream,
 int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 			       int cmd)
 {
+	struct snd_soc_pcm_runtime *be;
 	struct snd_soc_dpcm *dpcm;
 	int ret = 0;
 
 	for_each_dpcm_be(fe, stream, dpcm) {
+		struct snd_pcm_substream *be_substream;
 
-		struct snd_soc_pcm_runtime *be = dpcm->be;
-		struct snd_pcm_substream *be_substream =
-			snd_soc_dpcm_get_substream(be, stream);
+		be = dpcm->be;
+		be_substream = snd_soc_dpcm_get_substream(be, stream);
 
 		/* is this op for this BE ? */
 		if (!snd_soc_dpcm_be_can_update(fe, be, stream))
@@ -2010,7 +2011,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				return ret;
+				goto end;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
 			break;
@@ -2020,7 +2021,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				return ret;
+				goto end;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
 			break;
@@ -2030,7 +2031,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				return ret;
+				goto end;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_START;
 			break;
@@ -2044,7 +2045,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				return ret;
+				goto end;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_STOP;
 			break;
@@ -2057,7 +2058,7 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				return ret;
+				goto end;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_SUSPEND;
 			break;
@@ -2070,13 +2071,16 @@ int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream,
 
 			ret = soc_pcm_trigger(be_substream, cmd);
 			if (ret)
-				return ret;
+				goto end;
 
 			be->dpcm[stream].state = SND_SOC_DPCM_STATE_PAUSED;
 			break;
 		}
 	}
-
+end:
+	if (ret < 0)
+		dev_err(fe->dev, "ASoC: %s() failed at %s (%d)\n",
+			__func__, be->dai_link->name, ret);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dpcm_be_dai_trigger);
@@ -2310,8 +2314,6 @@ static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream)
 			fe->dai_link->name);
 
 		err = dpcm_be_dai_trigger(fe, stream, SNDRV_PCM_TRIGGER_STOP);
-		if (err < 0)
-			dev_err(fe->dev,"ASoC: trigger FE failed %d\n", err);
 	}
 
 	err = dpcm_be_dai_hw_free(fe, stream);
@@ -2393,10 +2395,8 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream)
 
 		ret = dpcm_be_dai_trigger(fe, stream,
 					SNDRV_PCM_TRIGGER_START);
-		if (ret < 0) {
-			dev_err(fe->dev,"ASoC: trigger FE failed %d\n", ret);
+		if (ret < 0)
 			goto hw_free;
-		}
 	}
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/14] ASoC: soc-pcm: indicate error message at dpcm_apply_symmetry()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (4 preceding siblings ...)
  2021-03-15  0:57 ` [PATCH 05/14] ASoC: soc-pcm: indicate error message at dpcm_be_dai_trigger() Kuninori Morimoto
@ 2021-03-15  0:58 ` Kuninori Morimoto
  2021-03-15  0:58 ` [PATCH 07/14] ASoC: soc-pcm: indicate error message at dpcm_run_update_startup/shutdown() Kuninori Morimoto
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:58 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

This patch follow above style at dpcm_apply_symmetry(...)

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

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 35c62062b8f8..855904c0938d 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1706,7 +1706,7 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream,
 		/* Symmetry only applies if we've got an active stream. */
 		err = soc_pcm_apply_symmetry(fe_substream, fe_cpu_dai);
 		if (err < 0)
-			return err;
+			goto error;
 	}
 
 	/* apply symmetry for BE */
@@ -1731,11 +1731,14 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream,
 		for_each_rtd_dais(rtd, i, dai) {
 			err = soc_pcm_apply_symmetry(fe_substream, dai);
 			if (err < 0)
-				return err;
+				goto error;
 		}
 	}
+error:
+	if (err < 0)
+		dev_err(fe->dev, "ASoC: %s failed (%d)\n", __func__, err);
 
-	return 0;
+	return err;
 }
 
 static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
@@ -1767,9 +1770,6 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
 	dpcm_runtime_setup_be_rate(fe_substream);
 
 	ret = dpcm_apply_symmetry(fe_substream, stream);
-	if (ret < 0)
-		dev_err(fe->dev, "ASoC: failed to apply dpcm symmetry %d\n",
-			ret);
 
 unwind:
 	if (ret < 0)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/14] ASoC: soc-pcm: indicate error message at dpcm_run_update_startup/shutdown()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (5 preceding siblings ...)
  2021-03-15  0:58 ` [PATCH 06/14] ASoC: soc-pcm: indicate error message at dpcm_apply_symmetry() Kuninori Morimoto
@ 2021-03-15  0:58 ` Kuninori Morimoto
  2021-03-15  0:58 ` [PATCH 08/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_startup() Kuninori Morimoto
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:58 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

This patch also
do below to dpcm_run_update_startup()
	1) remove duplicated ret = -EINVAL
	2) remove blank line
do below to dpcm_run_update_shutdown()
	1) remove unused ret

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

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 855904c0938d..a34b1fb9967a 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2325,7 +2325,10 @@ static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream)
 	/* run the stream event for each BE */
 	dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_NOP);
 
-	return 0;
+	if (err < 0)
+		dev_err(fe->dev, "ASoC: %s() failed (%d)\n", __func__, err);
+
+	return err;
 }
 
 static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream)
@@ -2366,7 +2369,6 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream)
 	if (fe->dpcm[stream].state == SND_SOC_DPCM_STATE_HW_PARAMS)
 		return 0;
 
-
 	ret = dpcm_be_dai_prepare(fe, stream);
 	if (ret < 0)
 		goto hw_free;
@@ -2421,6 +2423,9 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream)
 	}
 	spin_unlock_irqrestore(&fe->card->dpcm_lock, flags);
 
+	if (ret < 0)
+		dev_err(fe->dev, "ASoC: %s() failed (%d)\n", __func__, ret);
+
 	return ret;
 }
 
@@ -2429,7 +2434,6 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new)
 	struct snd_soc_dapm_widget_list *list;
 	int stream;
 	int count, paths;
-	int ret;
 
 	if (!fe->dai_link->dynamic)
 		return 0;
@@ -2469,11 +2473,9 @@ static int soc_dpcm_fe_runtime_update(struct snd_soc_pcm_runtime *fe, int new)
 		if (count) {
 			dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_BE);
 			if (new)
-				ret = dpcm_run_update_startup(fe, stream);
+				dpcm_run_update_startup(fe, stream);
 			else
-				ret = dpcm_run_update_shutdown(fe, stream);
-			if (ret < 0)
-				dev_err(fe->dev, "ASoC: failed to shutdown some BEs\n");
+				dpcm_run_update_shutdown(fe, stream);
 			dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
 
 			dpcm_clear_pending_state(fe, stream);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_startup()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (6 preceding siblings ...)
  2021-03-15  0:58 ` [PATCH 07/14] ASoC: soc-pcm: indicate error message at dpcm_run_update_startup/shutdown() Kuninori Morimoto
@ 2021-03-15  0:58 ` Kuninori Morimoto
  2021-03-15  0:58 ` [PATCH 09/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_hw_params() Kuninori Morimoto
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:58 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

This patch follow above style at dpcm_fe/be_dai_startup().

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

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index a34b1fb9967a..9e3ecf788a74 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1475,15 +1475,16 @@ void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream,
 
 int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream)
 {
+	struct snd_soc_pcm_runtime *be;
 	struct snd_soc_dpcm *dpcm;
 	int err, count = 0;
 
 	/* only startup BE DAIs that are either sinks or sources to this FE DAI */
 	for_each_dpcm_be(fe, stream, dpcm) {
+		struct snd_pcm_substream *be_substream;
 
-		struct snd_soc_pcm_runtime *be = dpcm->be;
-		struct snd_pcm_substream *be_substream =
-			snd_soc_dpcm_get_substream(be, stream);
+		be = dpcm->be;
+		be_substream = snd_soc_dpcm_get_substream(be, stream);
 
 		if (!be_substream) {
 			dev_err(be->dev, "ASoC: no backend %s stream\n",
@@ -1535,6 +1536,9 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream)
 unwind:
 	dpcm_be_dai_startup_rollback(fe, stream, dpcm);
 
+	dev_err(fe->dev, "ASoC: %s() failed at %s (%d)\n",
+		__func__, be->dai_link->name, err);
+
 	return err;
 }
 
@@ -1749,10 +1753,8 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
 	dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
 
 	ret = dpcm_be_dai_startup(fe, stream);
-	if (ret < 0) {
-		dev_err(fe->dev,"ASoC: failed to start some BEs %d\n", ret);
+	if (ret < 0)
 		goto be_err;
-	}
 
 	dev_dbg(fe->dev, "ASoC: open FE %s\n", fe->dai_link->name);
 
@@ -1776,6 +1778,10 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream)
 		dpcm_be_dai_startup_unwind(fe, stream);
 be_err:
 	dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
+
+	if (ret < 0)
+		dev_err(fe->dev, "%s() failed (%d)\n", __func__, ret);
+
 	return ret;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_hw_params()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (7 preceding siblings ...)
  2021-03-15  0:58 ` [PATCH 08/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_startup() Kuninori Morimoto
@ 2021-03-15  0:58 ` Kuninori Morimoto
  2021-03-15  0:58 ` [PATCH 10/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_prepare() Kuninori Morimoto
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:58 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

This patch follow above style at dpcm_fe/be_dai_hw_params()

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

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 9e3ecf788a74..e141d0658279 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1882,14 +1882,14 @@ static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream)
 
 int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream)
 {
+	struct snd_soc_pcm_runtime *be;
+	struct snd_pcm_substream *be_substream;
 	struct snd_soc_dpcm *dpcm;
 	int ret;
 
 	for_each_dpcm_be(fe, stream, dpcm) {
-
-		struct snd_soc_pcm_runtime *be = dpcm->be;
-		struct snd_pcm_substream *be_substream =
-			snd_soc_dpcm_get_substream(be, stream);
+		be = dpcm->be;
+		be_substream = snd_soc_dpcm_get_substream(be, stream);
 
 		/* is this op for this BE ? */
 		if (!snd_soc_dpcm_be_can_update(fe, be, stream))
@@ -1929,11 +1929,13 @@ int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int stream)
 	return 0;
 
 unwind:
+	dev_dbg(fe->dev, "ASoC: %s() failed at %s (%d)\n",
+		__func__, be->dai_link->name, ret);
+
 	/* disable any enabled and non active backends */
 	for_each_dpcm_be_rollback(fe, stream, dpcm) {
-		struct snd_soc_pcm_runtime *be = dpcm->be;
-		struct snd_pcm_substream *be_substream =
-			snd_soc_dpcm_get_substream(be, stream);
+		be = dpcm->be;
+		be_substream = snd_soc_dpcm_get_substream(be, stream);
 
 		if (!snd_soc_dpcm_be_can_update(fe, be, stream))
 			continue;
@@ -1966,10 +1968,8 @@ static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream,
 	memcpy(&fe->dpcm[stream].hw_params, params,
 			sizeof(struct snd_pcm_hw_params));
 	ret = dpcm_be_dai_hw_params(fe, stream);
-	if (ret < 0) {
-		dev_err(fe->dev,"ASoC: hw_params BE failed %d\n", ret);
+	if (ret < 0)
 		goto out;
-	}
 
 	dev_dbg(fe->dev, "ASoC: hw_params FE %s rate %d chan %x fmt %d\n",
 			fe->dai_link->name, params_rate(params),
@@ -1985,6 +1985,10 @@ static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream,
 out:
 	dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
 	mutex_unlock(&fe->card->mutex);
+
+	if (ret < 0)
+		dev_err(fe->dev, "ASoC: %s failed (%d)\n", __func__, ret);
+
 	return ret;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_prepare()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (8 preceding siblings ...)
  2021-03-15  0:58 ` [PATCH 09/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_hw_params() Kuninori Morimoto
@ 2021-03-15  0:58 ` Kuninori Morimoto
  2021-03-15  0:58 ` [PATCH 11/14] ASoC: soc-pcm: don't indicate error message for soc_pcm_hw_free() Kuninori Morimoto
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:58 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

Indicating error message when failed case is very useful for debuging.
In many case, its style is like below.

	int function(...)
	{
		...
		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		if (ret < 0)
			dev_err(...)
		...
	}

This is not so bad, but in this style *each caller* needs to indicate
duplicate same error message, and some caller is forgetting to do it.
And caller can't indicate detail function() error information.

If function() indicates error message, we can get same and
detail information without forgot.

	int function(...)
	{
		...
		if (ret < 0)
			dev_err(...)

		return ret;
	}

	int caller(...)
	{
		...
		ret = function(...);
		...
	}

This patch follow above style at dpcm_fe/be_dai_prepare()

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

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index e141d0658279..4a01a3925ab6 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2261,6 +2261,10 @@ int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream)
 
 		be->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE;
 	}
+
+	if (ret < 0)
+		dev_err(fe->dev, "ASoC: %s() failed (%d)\n", __func__, ret);
+
 	return ret;
 }
 
@@ -2298,6 +2302,9 @@ static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream)
 	dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
 	mutex_unlock(&fe->card->mutex);
 
+	if (ret < 0)
+		dev_err(fe->dev, "ASoC: %s() failed (%d)\n", __func__, ret);
+
 	return ret;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 11/14] ASoC: soc-pcm: don't indicate error message for soc_pcm_hw_free()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (9 preceding siblings ...)
  2021-03-15  0:58 ` [PATCH 10/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_prepare() Kuninori Morimoto
@ 2021-03-15  0:58 ` Kuninori Morimoto
  2021-03-15  0:58 ` [PATCH 12/14] ASoC: soc-pcm: don't indicate error message for dpcm_be_dai_hw_free() Kuninori Morimoto
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:58 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

soc_pcm_hw_free() never fail, error message is not needed.
We can't use void function for it, because it is used
part of struct snd_pcm_ops :: hw_free.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-pcm.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 4a01a3925ab6..eb52f78ca053 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1862,10 +1862,7 @@ static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream)
 	dev_dbg(fe->dev, "ASoC: hw_free FE %s\n", fe->dai_link->name);
 
 	/* call hw_free on the frontend */
-	err = soc_pcm_hw_free(substream);
-	if (err < 0)
-		dev_err(fe->dev,"ASoC: hw_free FE %s failed\n",
-			fe->dai_link->name);
+	soc_pcm_hw_free(substream);
 
 	/* only hw_params backends that are either sinks or sources
 	 * to this frontend DAI */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 12/14] ASoC: soc-pcm: don't indicate error message for dpcm_be_dai_hw_free()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (10 preceding siblings ...)
  2021-03-15  0:58 ` [PATCH 11/14] ASoC: soc-pcm: don't indicate error message for soc_pcm_hw_free() Kuninori Morimoto
@ 2021-03-15  0:58 ` Kuninori Morimoto
  2021-03-15  0:58 ` [PATCH 13/14] ASoC: don't indicate error message for snd_soc_[pcm_]dai_xxx() Kuninori Morimoto
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:58 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

dpcm_be_dai_hw_free() never fail, error message is not needed.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc-dpcm.h |  2 +-
 sound/soc/soc-compress.c |  5 +----
 sound/soc/soc-pcm.c      | 14 ++++----------
 3 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/include/sound/soc-dpcm.h b/include/sound/soc-dpcm.h
index d76cb1eeeaca..e296a3949b18 100644
--- a/include/sound/soc-dpcm.h
+++ b/include/sound/soc-dpcm.h
@@ -153,7 +153,7 @@ void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream,
 		      int do_hw_free, struct snd_soc_dpcm *last);
 void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream);
 void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream);
-int dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream);
+void dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream);
 int dpcm_be_dai_hw_params(struct snd_soc_pcm_runtime *fe, int tream);
 int dpcm_be_dai_trigger(struct snd_soc_pcm_runtime *fe, int stream, int cmd);
 int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream);
diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index 94f1f7a9dd53..83b511f8b8c9 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -175,7 +175,6 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(fe, 0);
 	struct snd_soc_dpcm *dpcm;
 	int stream = cstream->direction; /* SND_COMPRESS_xxx is same as SNDRV_PCM_STREAM_xxx */
-	int ret;
 
 	mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
 
@@ -183,9 +182,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_FE;
 
-	ret = dpcm_be_dai_hw_free(fe, stream);
-	if (ret < 0)
-		dev_err(fe->dev, "Compressed ASoC: hw_free failed: %d\n", ret);
+	dpcm_be_dai_hw_free(fe, stream);
 
 	dpcm_be_dai_shutdown(fe, stream);
 
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index eb52f78ca053..2a126840677c 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1808,7 +1808,7 @@ static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-int dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream)
+void dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream)
 {
 	struct snd_soc_dpcm *dpcm;
 
@@ -1847,14 +1847,12 @@ int dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream)
 
 		be->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_FREE;
 	}
-
-	return 0;
 }
 
 static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream);
-	int err, stream = substream->stream;
+	int stream = substream->stream;
 
 	mutex_lock_nested(&fe->card->mutex, SND_SOC_CARD_CLASS_RUNTIME);
 	dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE);
@@ -1866,9 +1864,7 @@ static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream)
 
 	/* only hw_params backends that are either sinks or sources
 	 * to this frontend DAI */
-	err = dpcm_be_dai_hw_free(fe, stream);
-	if (err < 0)
-		dev_err(fe->dev, "ASoC: hw_free BE failed %d\n", err);
+	dpcm_be_dai_hw_free(fe, stream);
 
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_HW_FREE;
 	dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_NO);
@@ -2330,9 +2326,7 @@ static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream)
 		err = dpcm_be_dai_trigger(fe, stream, SNDRV_PCM_TRIGGER_STOP);
 	}
 
-	err = dpcm_be_dai_hw_free(fe, stream);
-	if (err < 0)
-		dev_err(fe->dev,"ASoC: hw_free FE failed %d\n", err);
+	dpcm_be_dai_hw_free(fe, stream);
 
 	dpcm_be_dai_shutdown(fe, stream);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 13/14] ASoC: don't indicate error message for snd_soc_[pcm_]dai_xxx()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (11 preceding siblings ...)
  2021-03-15  0:58 ` [PATCH 12/14] ASoC: soc-pcm: don't indicate error message for dpcm_be_dai_hw_free() Kuninori Morimoto
@ 2021-03-15  0:58 ` Kuninori Morimoto
  2021-03-15  0:58 ` [PATCH 14/14] ASoC: don't indicate error message for snd_soc_[pcm_]component_xxx() Kuninori Morimoto
  2021-03-19 16:37 ` [PATCH 00/14] ASoC: tidyup error message timing Mark Brown
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:58 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

All snd_soc_dai_xxx() and snd_soc_pcm_dai_xxx() itself
indicate error message if failed.
Its caller doesn't need to indicate duplicated error message.
This patch removes it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 16 +++-------------
 sound/soc/soc-dapm.c | 24 ++++++------------------
 sound/soc/soc-pcm.c  | 10 ++--------
 3 files changed, 11 insertions(+), 39 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c7e4600b2dd4..f12c763973fa 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1088,12 +1088,8 @@ static int soc_init_pcm_runtime(struct snd_soc_card *card,
 
 	/* create compress_device if possible */
 	ret = snd_soc_dai_compress_new(cpu_dai, rtd, num);
-	if (ret != -ENOTSUPP) {
-		if (ret < 0)
-			dev_err(card->dev, "ASoC: can't create compress %s\n",
-				dai_link->stream_name);
+	if (ret != -ENOTSUPP)
 		return ret;
-	}
 
 	/* create the pcm */
 	ret = soc_new_pcm(rtd, num);
@@ -1422,11 +1418,8 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd,
 
 	for_each_rtd_codec_dais(rtd, i, codec_dai) {
 		ret = snd_soc_dai_set_fmt(codec_dai, dai_fmt);
-		if (ret != 0 && ret != -ENOTSUPP) {
-			dev_warn(codec_dai->dev,
-				 "ASoC: Failed to set DAI format: %d\n", ret);
+		if (ret != 0 && ret != -ENOTSUPP)
 			return ret;
-		}
 	}
 
 	/*
@@ -1455,11 +1448,8 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd,
 			fmt = inv_dai_fmt;
 
 		ret = snd_soc_dai_set_fmt(cpu_dai, fmt);
-		if (ret != 0 && ret != -ENOTSUPP) {
-			dev_warn(cpu_dai->dev,
-				 "ASoC: Failed to set DAI format: %d\n", ret);
+		if (ret != 0 && ret != -ENOTSUPP)
 			return ret;
-		}
 	}
 
 	return 0;
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index b005f9eadd71..91bf939d5233 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3831,11 +3831,9 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
 		source = path->source->priv;
 
 		ret = snd_soc_dai_startup(source, substream);
-		if (ret < 0) {
-			dev_err(source->dev,
-				"ASoC: startup() failed: %d\n", ret);
+		if (ret < 0)
 			goto out;
-		}
+
 		snd_soc_dai_activate(source, substream->stream);
 	}
 
@@ -3844,11 +3842,9 @@ snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
 		sink = path->sink->priv;
 
 		ret = snd_soc_dai_startup(sink, substream);
-		if (ret < 0) {
-			dev_err(sink->dev,
-				"ASoC: startup() failed: %d\n", ret);
+		if (ret < 0)
 			goto out;
-		}
+
 		snd_soc_dai_activate(sink, substream->stream);
 	}
 
@@ -3943,11 +3939,7 @@ static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w,
 		snd_soc_dapm_widget_for_each_sink_path(w, path) {
 			sink = path->sink->priv;
 
-			ret = snd_soc_dai_digital_mute(sink, 0,
-						       SNDRV_PCM_STREAM_PLAYBACK);
-			if (ret != 0 && ret != -ENOTSUPP)
-				dev_warn(sink->dev,
-					 "ASoC: Failed to unmute: %d\n", ret);
+			snd_soc_dai_digital_mute(sink, 0, SNDRV_PCM_STREAM_PLAYBACK);
 			ret = 0;
 		}
 		break;
@@ -3956,11 +3948,7 @@ static int snd_soc_dai_link_event(struct snd_soc_dapm_widget *w,
 		snd_soc_dapm_widget_for_each_sink_path(w, path) {
 			sink = path->sink->priv;
 
-			ret = snd_soc_dai_digital_mute(sink, 1,
-						       SNDRV_PCM_STREAM_PLAYBACK);
-			if (ret != 0 && ret != -ENOTSUPP)
-				dev_warn(sink->dev,
-					 "ASoC: Failed to mute: %d\n", ret);
+			snd_soc_dai_digital_mute(sink, 1, SNDRV_PCM_STREAM_PLAYBACK);
 			ret = 0;
 		}
 
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 2a126840677c..ecf2bfa9640b 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -832,10 +832,8 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
 		goto out;
 
 	ret = snd_soc_pcm_dai_prepare(substream);
-	if (ret < 0) {
-		dev_err(rtd->dev, "ASoC: DAI prepare error: %d\n", ret);
+	if (ret < 0)
 		goto out;
-	}
 
 	/* cancel any delayed stream shutdown that is pending */
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
@@ -2317,8 +2315,6 @@ static int dpcm_run_update_shutdown(struct snd_soc_pcm_runtime *fe, int stream)
 				fe->dai_link->name);
 
 		err = snd_soc_pcm_dai_bespoke_trigger(substream, SNDRV_PCM_TRIGGER_STOP);
-		if (err < 0)
-			dev_err(fe->dev,"ASoC: trigger FE failed %d\n", err);
 	} else {
 		dev_dbg(fe->dev, "ASoC: trigger FE %s cmd stop\n",
 			fe->dai_link->name);
@@ -2395,10 +2391,8 @@ static int dpcm_run_update_startup(struct snd_soc_pcm_runtime *fe, int stream)
 				fe->dai_link->name);
 
 		ret = snd_soc_pcm_dai_bespoke_trigger(substream, SNDRV_PCM_TRIGGER_START);
-		if (ret < 0) {
-			dev_err(fe->dev,"ASoC: bespoke trigger FE failed %d\n", ret);
+		if (ret < 0)
 			goto hw_free;
-		}
 	} else {
 		dev_dbg(fe->dev, "ASoC: trigger FE %s cmd start\n",
 			fe->dai_link->name);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 14/14] ASoC: don't indicate error message for snd_soc_[pcm_]component_xxx()
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (12 preceding siblings ...)
  2021-03-15  0:58 ` [PATCH 13/14] ASoC: don't indicate error message for snd_soc_[pcm_]dai_xxx() Kuninori Morimoto
@ 2021-03-15  0:58 ` Kuninori Morimoto
  2021-03-19 16:37 ` [PATCH 00/14] ASoC: tidyup error message timing Mark Brown
  14 siblings, 0 replies; 16+ messages in thread
From: Kuninori Morimoto @ 2021-03-15  0:58 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA

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

All snd_soc_component_xxx() and snd_soc_pcm_component_xxx() itself
indicate error message if failed.
Its caller doesn't need to indicate duplicated error message.
This patch removes it.

All snd_soc_component_xxx() indicate error message if failed.
Its caller doesn't need to indicate duplicated error message.
This patch removes it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 6 ++----
 sound/soc/soc-pcm.c  | 5 +----
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f12c763973fa..522bae73640a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1203,11 +1203,9 @@ static int soc_probe_component(struct snd_soc_card *card,
 	}
 
 	ret = snd_soc_component_probe(component);
-	if (ret < 0) {
-		dev_err(component->dev,
-			"ASoC: failed to probe component %d\n", ret);
+	if (ret < 0)
 		goto err_probe;
-	}
+
 	WARN(dapm->idle_bias_off &&
 	     dapm->bias_level != SND_SOC_BIAS_OFF,
 	     "codec %s can not start from non-off bias with idle_bias_off==1\n",
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index ecf2bfa9640b..2df70ab851ea 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -2781,11 +2781,8 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
 		snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &rtd->ops);
 
 	ret = snd_soc_pcm_component_new(rtd);
-	if (ret < 0) {
-		dev_err(rtd->dev, "ASoC: pcm constructor failed for dailink %s: %d\n",
-			rtd->dai_link->name, ret);
+	if (ret < 0)
 		return ret;
-	}
 
 	pcm->no_device_suspend = true;
 out:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/14] ASoC: tidyup error message timing
  2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
                   ` (13 preceding siblings ...)
  2021-03-15  0:58 ` [PATCH 14/14] ASoC: don't indicate error message for snd_soc_[pcm_]component_xxx() Kuninori Morimoto
@ 2021-03-19 16:37 ` Mark Brown
  14 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2021-03-19 16:37 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown

On 15 Mar 2021 09:57:14 +0900, Kuninori Morimoto wrote:
> Indicating error message when failed case is very useful for debuging.
> In many case, it uses below style.
> 
> 	int function(...)
> 	{
> 		...
> 		return ret;
> 	}
> 
> [...]

Applied to

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

Thanks!

[01/14] ASoC: soc-pcm: indicate error message at soc_pcm_open()
        commit: e4b044f4582366de10b8b28614c24ac4ff22b299
[02/14] ASoC: soc-pcm: indicate error message at soc_pcm_hw_params()
        commit: cb11f79b4af65005584880eb408f9748c32661d0
[03/14] ASoC: soc-pcm: indicate error message at soc_pcm_prepare()
        commit: dab7eeb4045cce074e083be1f3092d7390d6cfb2
[04/14] ASoC: soc-pcm: indicate error message at dpcm_path_get()
        commit: d479f00b795ac62b24ef90f4ec421e65c3178ca7
[05/14] ASoC: soc-pcm: indicate error message at dpcm_be_dai_trigger()
        commit: db3aa39c91068424407f71d23b028493eac994a1
[06/14] ASoC: soc-pcm: indicate error message at dpcm_apply_symmetry()
        commit: bbd2bac8d6ca00ee0b032d3c03100328131425ac
[07/14] ASoC: soc-pcm: indicate error message at dpcm_run_update_startup/shutdown()
        commit: 81c82a9edbddc4cd97e4d974dfd7f2689ee63474
[08/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_startup()
        commit: 06aaeb874256a10fe5b84f511da3c65f548a43b9
[09/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_hw_params()
        commit: 33b6b94f55ec60517ce71ca2bc9c03a6d337c805
[10/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_prepare()
        commit: 273db971cf833d62c9e2d9381d190e14b1cd3641
[11/14] ASoC: soc-pcm: don't indicate error message for soc_pcm_hw_free()
        commit: e20c9c4f96d79aa48eb3649c57f1f2784f92b838
[12/14] ASoC: soc-pcm: don't indicate error message for dpcm_be_dai_hw_free()
        commit: f52366e6831ecb2da133b6ecfc7c69266086660c
[13/14] ASoC: don't indicate error message for snd_soc_[pcm_]dai_xxx()
        commit: 62462e018220895267450155b188f5804f54c202
[14/14] ASoC: don't indicate error message for snd_soc_[pcm_]component_xxx()
        commit: 60adbd8fbf486214f4ae1946e61df69c3867e20b

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-03-19 16:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15  0:57 [PATCH 00/14] ASoC: tidyup error message timing Kuninori Morimoto
2021-03-15  0:57 ` [PATCH 01/14] ASoC: soc-pcm: indicate error message at soc_pcm_open() Kuninori Morimoto
2021-03-15  0:57 ` [PATCH 02/14] ASoC: soc-pcm: indicate error message at soc_pcm_hw_params() Kuninori Morimoto
2021-03-15  0:57 ` [PATCH 03/14] ASoC: soc-pcm: indicate error message at soc_pcm_prepare() Kuninori Morimoto
2021-03-15  0:57 ` [PATCH 04/14] ASoC: soc-pcm: indicate error message at dpcm_path_get() Kuninori Morimoto
2021-03-15  0:57 ` [PATCH 05/14] ASoC: soc-pcm: indicate error message at dpcm_be_dai_trigger() Kuninori Morimoto
2021-03-15  0:58 ` [PATCH 06/14] ASoC: soc-pcm: indicate error message at dpcm_apply_symmetry() Kuninori Morimoto
2021-03-15  0:58 ` [PATCH 07/14] ASoC: soc-pcm: indicate error message at dpcm_run_update_startup/shutdown() Kuninori Morimoto
2021-03-15  0:58 ` [PATCH 08/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_startup() Kuninori Morimoto
2021-03-15  0:58 ` [PATCH 09/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_hw_params() Kuninori Morimoto
2021-03-15  0:58 ` [PATCH 10/14] ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_prepare() Kuninori Morimoto
2021-03-15  0:58 ` [PATCH 11/14] ASoC: soc-pcm: don't indicate error message for soc_pcm_hw_free() Kuninori Morimoto
2021-03-15  0:58 ` [PATCH 12/14] ASoC: soc-pcm: don't indicate error message for dpcm_be_dai_hw_free() Kuninori Morimoto
2021-03-15  0:58 ` [PATCH 13/14] ASoC: don't indicate error message for snd_soc_[pcm_]dai_xxx() Kuninori Morimoto
2021-03-15  0:58 ` [PATCH 14/14] ASoC: don't indicate error message for snd_soc_[pcm_]component_xxx() Kuninori Morimoto
2021-03-19 16:37 ` [PATCH 00/14] ASoC: tidyup error message timing Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.