From 092830cf550667d5fa6286605167d232f2c1f61e Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Tue, 30 May 2023 00:49:43 +0000 Subject: [PATCH 1/5] ASoC: soc-pcm.c: indicate error if stream has no playback no capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit soc_get_playback_capture() (A) returns number of substreams for playback/capture (B). ASoC will probe the Sound Card and mapps CPU<->Codec pair. (A) static int soc_get_playback_capture(..., (B) int *playback, int *capture) { ... if (rtd->dai_link->playback_only) { *playback = 1; *capture = 0; } if (rtd->dai_link->capture_only) { *playback = 0; *capture = 1; } (C) return 0; } But it might be no playback no capture if it returns playback=0, capture=0. It is very difficult to notice about it. This patch indicates error at (C) then. Signed-off-by: Kuninori Morimoto Reviewed-by: Amadeusz Sławiński Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/87y1l6zlqx.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 6365ad8ca7ef..f3ce825c0b20 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2814,6 +2814,13 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, *capture = 1; } + if (!*playback && !*capture) { + dev_err(rtd->dev, "substream %s has no playback, no capture\n", + rtd->dai_link->stream_name); + + return -EINVAL; + } + return 0; } From cfcb31c456b15e298f88fb5ebedf7b32b009d32d Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Tue, 30 May 2023 00:49:50 +0000 Subject: [PATCH 2/5] ASoC: soc-pcm.c: use dai_link on soc_get_playback_capture() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit soc_get_playback_capture() (A) is using rtd->dai_link->xxx everywhere. Because of that, 1 line is unnecessarily long and not readable. (A) static int soc_get_playback_capture(...) { if (rtd->dai_link->dynamic ...) { ^^^^^^^^^^^^^ ... } else { int cpu_capture = rtd->dai_link->c2c_params ? ^^^^^^^^^^^^^ ... } if (rtd->dai_link->playback_only) { ^^^^^^^^^^^^^ ... } ... } This patch uses variable "dai_link" to be clear code. Nothing changes the meanings. Signed-off-by: Kuninori Morimoto Reviewed-by: Amadeusz Sławiński Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/87wn0qzlqp.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index f3ce825c0b20..1e6d5da569a5 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2730,19 +2730,20 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream) static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, int *playback, int *capture) { + struct snd_soc_dai_link *dai_link = rtd->dai_link; struct snd_soc_dai *cpu_dai; int i; - if (rtd->dai_link->dynamic && rtd->dai_link->num_cpus > 1) { + if (dai_link->dynamic && dai_link->num_cpus > 1) { dev_err(rtd->dev, "DPCM doesn't support Multi CPU for Front-Ends yet\n"); return -EINVAL; } - if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) { + if (dai_link->dynamic || dai_link->no_pcm) { int stream; - if (rtd->dai_link->dpcm_playback) { + if (dai_link->dpcm_playback) { stream = SNDRV_PCM_STREAM_PLAYBACK; for_each_rtd_cpu_dais(rtd, i, cpu_dai) { @@ -2754,11 +2755,11 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, if (!*playback) { dev_err(rtd->card->dev, "No CPU DAIs support playback for stream %s\n", - rtd->dai_link->stream_name); + dai_link->stream_name); return -EINVAL; } } - if (rtd->dai_link->dpcm_capture) { + if (dai_link->dpcm_capture) { stream = SNDRV_PCM_STREAM_CAPTURE; for_each_rtd_cpu_dais(rtd, i, cpu_dai) { @@ -2771,7 +2772,7 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, if (!*capture) { dev_err(rtd->card->dev, "No CPU DAIs support capture for stream %s\n", - rtd->dai_link->stream_name); + dai_link->stream_name); return -EINVAL; } } @@ -2779,15 +2780,15 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dai *codec_dai; /* Adapt stream for codec2codec links */ - int cpu_capture = rtd->dai_link->c2c_params ? + int cpu_capture = dai_link->c2c_params ? SNDRV_PCM_STREAM_PLAYBACK : SNDRV_PCM_STREAM_CAPTURE; - int cpu_playback = rtd->dai_link->c2c_params ? + int cpu_playback = dai_link->c2c_params ? SNDRV_PCM_STREAM_CAPTURE : SNDRV_PCM_STREAM_PLAYBACK; for_each_rtd_codec_dais(rtd, i, codec_dai) { - if (rtd->dai_link->num_cpus == 1) { + if (dai_link->num_cpus == 1) { cpu_dai = asoc_rtd_to_cpu(rtd, 0); - } else if (rtd->dai_link->num_cpus == rtd->dai_link->num_codecs) { + } else if (dai_link->num_cpus == dai_link->num_codecs) { cpu_dai = asoc_rtd_to_cpu(rtd, i); } else { dev_err(rtd->card->dev, @@ -2804,19 +2805,19 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, } } - if (rtd->dai_link->playback_only) { + if (dai_link->playback_only) { *playback = 1; *capture = 0; } - if (rtd->dai_link->capture_only) { + if (dai_link->capture_only) { *playback = 0; *capture = 1; } if (!*playback && !*capture) { dev_err(rtd->dev, "substream %s has no playback, no capture\n", - rtd->dai_link->stream_name); + dai_link->stream_name); return -EINVAL; } From a1c0221fa5baeae6c9dc30294c2c6d01f1f4379b Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Tue, 30 May 2023 00:49:56 +0000 Subject: [PATCH 3/5] ASoC: soc-pcm.c: cleanup soc_get_playback_capture() error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit soc_get_playback_capture() (A) checks dai_link status, and indicate error if it was not matching (B). (A) static int soc_get_playback_capture(...) { ... ^ if (dai_link->dynamic && dai_link->num_cpus > 1) { | dev_err(rtd->dev, (B) "DPCM doesn't support Multi CPU for Front-Ends yet\n"); | return -EINVAL; v } ... } We can use 100 char for 1 line today. This patch cleanup error code line. Signed-off-by: Kuninori Morimoto Reviewed-by: Amadeusz Sławiński Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/87v8gazlqk.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 1e6d5da569a5..e752089a4227 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2735,8 +2735,7 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, int i; if (dai_link->dynamic && dai_link->num_cpus > 1) { - dev_err(rtd->dev, - "DPCM doesn't support Multi CPU for Front-Ends yet\n"); + dev_err(rtd->dev, "DPCM doesn't support Multi CPU for Front-Ends yet\n"); return -EINVAL; } From c3e9b6d6ef5a0a3e841c3aa29e7afc48a0b73806 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Tue, 30 May 2023 00:50:01 +0000 Subject: [PATCH 4/5] ASoC: soc-pcm.c: use temporary variable at soc_get_playback_capture() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit soc_get_playback_capture() (A) returns number of substreams for playback/capture (B). (A) static int soc_get_playback_capture(..., (B) int *playback, int *capture) { ... for_each_xxx(...) { if (xxx) return -EINVAL; => *playback = 1; ... => *capture = 1; ... } ... } But, it is directly updating playback/capture which is the result of this function even though it might be error. It should be updated in case of succeed only. This patch updates it. Signed-off-by: Kuninori Morimoto Reviewed-by: Amadeusz Sławiński Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/87ttvuzlqe.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index e752089a4227..765e43ca637d 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2732,6 +2732,8 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, { struct snd_soc_dai_link *dai_link = rtd->dai_link; struct snd_soc_dai *cpu_dai; + int has_playback = 0; + int has_capture = 0; int i; if (dai_link->dynamic && dai_link->num_cpus > 1) { @@ -2747,11 +2749,11 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, for_each_rtd_cpu_dais(rtd, i, cpu_dai) { if (snd_soc_dai_stream_valid(cpu_dai, stream)) { - *playback = 1; + has_playback = 1; break; } } - if (!*playback) { + if (!has_playback) { dev_err(rtd->card->dev, "No CPU DAIs support playback for stream %s\n", dai_link->stream_name); @@ -2763,12 +2765,12 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, for_each_rtd_cpu_dais(rtd, i, cpu_dai) { if (snd_soc_dai_stream_valid(cpu_dai, stream)) { - *capture = 1; + has_capture = 1; break; } } - if (!*capture) { + if (!has_capture) { dev_err(rtd->card->dev, "No CPU DAIs support capture for stream %s\n", dai_link->stream_name); @@ -2797,30 +2799,33 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) && snd_soc_dai_stream_valid(cpu_dai, cpu_playback)) - *playback = 1; + has_playback = 1; if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) && snd_soc_dai_stream_valid(cpu_dai, cpu_capture)) - *capture = 1; + has_capture = 1; } } if (dai_link->playback_only) { - *playback = 1; - *capture = 0; + has_playback = 1; + has_capture = 0; } if (dai_link->capture_only) { - *playback = 0; - *capture = 1; + has_playback = 0; + has_capture = 1; } - if (!*playback && !*capture) { + if (!has_playback && !has_capture) { dev_err(rtd->dev, "substream %s has no playback, no capture\n", dai_link->stream_name); return -EINVAL; } + *playback = has_playback; + *capture = has_capture; + return 0; } From e1f653ce847bab7285dd135cabe3ce544e574c75 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Tue, 30 May 2023 00:50:08 +0000 Subject: [PATCH 5/5] ASoC: soc-pcm.c: tidyup playback/capture_only at soc_get_playback_capture() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit soc_get_playback_capture() (A) returns number of substreams for playback/capture, and then, we can use playback/capture_only flag (X)(Y). (A) static int soc_get_playback_capture(...) { ... (X) if (dai_link->playback_only) { (*) *playback = 1; *capture = 0; } (Y) if (dai_link->capture_only) { *playback = 0; (*) *capture = 1; } ... } But this flag should not have effect to opposite side stream (*). This patch tidyup it. Signed-off-by: Kuninori Morimoto Reviewed-by: Amadeusz Sławiński Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/87sfbezlq8.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 765e43ca637d..fc0817dd0d83 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2806,15 +2806,11 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, } } - if (dai_link->playback_only) { - has_playback = 1; + if (dai_link->playback_only) has_capture = 0; - } - if (dai_link->capture_only) { + if (dai_link->capture_only) has_playback = 0; - has_capture = 1; - } if (!has_playback && !has_capture) { dev_err(rtd->dev, "substream %s has no playback, no capture\n",