From e4b044f4582366de10b8b28614c24ac4ff22b299 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:57:37 +0900 Subject: ASoC: soc-pcm: indicate error message at soc_pcm_open() 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 Link: https://lore.kernel.org/r/87r1khutby.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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 @@ dynamic: 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; -- cgit v1.2.3 From cb11f79b4af65005584880eb408f9748c32661d0 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:57:42 +0900 Subject: ASoC: soc-pcm: indicate error message at soc_pcm_hw_params() 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 Link: https://lore.kernel.org/r/87pn01utbt.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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: -- cgit v1.2.3 From dab7eeb4045cce074e083be1f3092d7390d6cfb2 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:57:48 +0900 Subject: ASoC: soc-pcm: indicate error message at soc_pcm_prepare() 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 Link: https://lore.kernel.org/r/87o8flutbn.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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; -- cgit v1.2.3 From d479f00b795ac62b24ef90f4ec421e65c3178ca7 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:57:52 +0900 Subject: ASoC: soc-pcm: indicate error message at dpcm_path_get() 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 Link: https://lore.kernel.org/r/87mtv5utbj.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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); -- cgit v1.2.3 From db3aa39c91068424407f71d23b028493eac994a1 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:57:57 +0900 Subject: ASoC: soc-pcm: indicate error message at dpcm_be_dai_trigger() 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 Link: https://lore.kernel.org/r/87lfaputbe.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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 @@ out: 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; -- cgit v1.2.3 From bbd2bac8d6ca00ee0b032d3c03100328131425ac Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:58:02 +0900 Subject: ASoC: soc-pcm: indicate error message at dpcm_apply_symmetry() 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 Link: https://lore.kernel.org/r/87k0q9utb9.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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) -- cgit v1.2.3 From 81c82a9edbddc4cd97e4d974dfd7f2689ee63474 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:58:08 +0900 Subject: ASoC: soc-pcm: indicate error message at dpcm_run_update_startup/shutdown() 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 Link: https://lore.kernel.org/r/87im5tutb3.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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 @@ disconnect: } 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); -- cgit v1.2.3 From 06aaeb874256a10fe5b84f511da3c65f548a43b9 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:58:13 +0900 Subject: ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_startup() 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 Link: https://lore.kernel.org/r/87h7ldutay.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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 @@ unwind: 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; } -- cgit v1.2.3 From 33b6b94f55ec60517ce71ca2bc9c03a6d337c805 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:58:18 +0900 Subject: ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_hw_params() 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 Link: https://lore.kernel.org/r/87ft0xutat.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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; } -- cgit v1.2.3 From 273db971cf833d62c9e2d9381d190e14b1cd3641 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:58:22 +0900 Subject: ASoC: soc-pcm: indicate error message at dpcm_fe/be_dai_prepare() 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 Link: https://lore.kernel.org/r/87eeghutap.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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 @@ 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; } -- cgit v1.2.3 From e20c9c4f96d79aa48eb3649c57f1f2784f92b838 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:58:28 +0900 Subject: ASoC: soc-pcm: don't indicate error message for soc_pcm_hw_free() 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 Link: https://lore.kernel.org/r/87czw1utaj.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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 */ -- cgit v1.2.3 From f52366e6831ecb2da133b6ecfc7c69266086660c Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:58:32 +0900 Subject: ASoC: soc-pcm: don't indicate error message for dpcm_be_dai_hw_free() dpcm_be_dai_hw_free() never fail, error message is not needed. Signed-off-by: Kuninori Morimoto Link: https://lore.kernel.org/r/87blblutaf.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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); -- cgit v1.2.3 From 62462e018220895267450155b188f5804f54c202 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:58:37 +0900 Subject: ASoC: don't indicate error message for snd_soc_[pcm_]dai_xxx() 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 Link: https://lore.kernel.org/r/87a6r5utaa.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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); -- cgit v1.2.3 From 60adbd8fbf486214f4ae1946e61df69c3867e20b Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 15 Mar 2021 09:58:41 +0900 Subject: ASoC: don't indicate error message for snd_soc_[pcm_]component_xxx() 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 Link: https://lore.kernel.org/r/878s6puta6.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- 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: -- cgit v1.2.3