diff mbox series

[5/7] ASoC: soc-pcm: goto error after trying all component open

Message ID 87zhe9mzx3.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show
Series ASoC: soc-pcm cleanup step2 | expand

Commit Message

Kuninori Morimoto Jan. 27, 2020, 1:49 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_pcm_components_open() might goto error process *during* opening
component loop.
In such case, fallback process need to care about operated/non-operated
component.

But, if it goto error process *after* loop even though error happen
during loop, it don't need to care about operated/non-operated.
In such case code can be more simple.
This patch do it. And this is prepare for soc_snd_open() cleanup

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

Comments

Sridharan, Ranjani Jan. 27, 2020, 6:34 p.m. UTC | #1
On Sun, Jan 26, 2020 at 5:54 PM Kuninori Morimoto <
kuninori.morimoto.gx@renesas.com> wrote:

>
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> soc_pcm_components_open() might goto error process *during* opening
> component loop.
> In such case, fallback process need to care about operated/non-operated
> component.
>
> But, if it goto error process *after* loop even though error happen
> during loop, it don't need to care about operated/non-operated.
> In such case code can be more simple.
> This patch do it. And this is prepare for soc_snd_open() cleanup
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  sound/soc/soc-pcm.c | 41 +++++++++++++----------------------------
>  1 file changed, 13 insertions(+), 28 deletions(-)
>
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 57d2f00..1e370ef 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -463,47 +463,32 @@ static void soc_pcm_init_runtime_hw(struct
> snd_pcm_substream *substream)
>         hw->rate_max = min_not_zero(hw->rate_max, rate_max);
>  }
>
> -static int soc_pcm_components_open(struct snd_pcm_substream *substream,
> -                                  struct snd_soc_component **last)
> +static int soc_pcm_components_open(struct snd_pcm_substream *substream)
>  {
>         struct snd_soc_pcm_runtime *rtd = substream->private_data;
>         struct snd_soc_component *component;
>         int i, ret = 0;
>
>         for_each_rtd_components(rtd, i, component) {
> -               *last = component;
> +               ret |= snd_soc_component_module_get_when_open(component);
> +               ret |= snd_soc_component_open(component, substream);
> +       }
>
> -               ret = snd_soc_component_module_get_when_open(component);
> -               if (ret < 0) {
> -                       dev_err(component->dev,
> -                               "ASoC: can't get module %s\n",
> -                               component->name);
> -                       return ret;
> -               }
> +       if (ret < 0)
> +               dev_err(component->dev,
> +                       "ASoC: error happen during open component %s:
> %d\n",
> +                       component->name, ret);
>
Hi Morimoto-san,

Wouldn't the component here always be the last component in the list of rtd
components? Should this error log be moved inside
the for_each_rtd_components() {} above?

Thanks,
Ranjani
Kuninori Morimoto Jan. 28, 2020, 12:31 a.m. UTC | #2
Hi Sridharan

Thank you for your feedback

>     @@ -463,47 +463,32 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
>             hw->rate_max = min_not_zero(hw->rate_max, rate_max);
>      }
>    
>     -static int soc_pcm_components_open(struct snd_pcm_substream *substream,
>     -                                  struct snd_soc_component **last)
>     +static int soc_pcm_components_open(struct snd_pcm_substream *substream)
>      {
>             struct snd_soc_pcm_runtime *rtd = substream->private_data;
>             struct snd_soc_component *component;
>             int i, ret = 0;
>    
>             for_each_rtd_components(rtd, i, component) {
>     -               *last = component;
>     +               ret |= snd_soc_component_module_get_when_open(component);
>     +               ret |= snd_soc_component_open(component, substream);
>     +       }
>    
>     -               ret = snd_soc_component_module_get_when_open(component);
>     -               if (ret < 0) {
>     -                       dev_err(component->dev,
>     -                               "ASoC: can't get module %s\n",
>     -                               component->name);
>     -                       return ret;
>     -               }
>     +       if (ret < 0)
>     +               dev_err(component->dev,
>     +                       "ASoC: error happen during open component %s: %d\n",
>     +                       component->name, ret);
> 
> Hi Morimoto-san,
> 
> Wouldn't the component here always be the last component in the list of rtd components? Should this error log be moved inside
> the for_each_rtd_components() {} above?

Yeah, indeed.
Will fix

Thank you for your help !!
Best regards
---
Kuninori Morimoto
diff mbox series

Patch

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 57d2f00..1e370ef 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -463,47 +463,32 @@  static void soc_pcm_init_runtime_hw(struct snd_pcm_substream *substream)
 	hw->rate_max = min_not_zero(hw->rate_max, rate_max);
 }
 
-static int soc_pcm_components_open(struct snd_pcm_substream *substream,
-				   struct snd_soc_component **last)
+static int soc_pcm_components_open(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
 	int i, ret = 0;
 
 	for_each_rtd_components(rtd, i, component) {
-		*last = component;
+		ret |= snd_soc_component_module_get_when_open(component);
+		ret |= snd_soc_component_open(component, substream);
+	}
 
-		ret = snd_soc_component_module_get_when_open(component);
-		if (ret < 0) {
-			dev_err(component->dev,
-				"ASoC: can't get module %s\n",
-				component->name);
-			return ret;
-		}
+	if (ret < 0)
+		dev_err(component->dev,
+			"ASoC: error happen during open component %s: %d\n",
+			component->name, ret);
 
-		ret = snd_soc_component_open(component, substream);
-		if (ret < 0) {
-			dev_err(component->dev,
-				"ASoC: can't open component %s: %d\n",
-				component->name, ret);
-			return ret;
-		}
-	}
-	*last = NULL;
-	return 0;
+	return ret;
 }
 
-static int soc_pcm_components_close(struct snd_pcm_substream *substream,
-				    struct snd_soc_component *last)
+static int soc_pcm_components_close(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
 	int i, ret = 0;
 
 	for_each_rtd_components(rtd, i, component) {
-		if (component == last)
-			break;
-
 		ret |= snd_soc_component_close(component, substream);
 		snd_soc_component_module_put_when_close(component);
 	}
@@ -542,7 +527,7 @@  static int soc_pcm_open(struct snd_pcm_substream *substream)
 		goto out;
 	}
 
-	ret = soc_pcm_components_open(substream, &component);
+	ret = soc_pcm_components_open(substream);
 	if (ret < 0)
 		goto component_err;
 
@@ -638,7 +623,7 @@  static int soc_pcm_open(struct snd_pcm_substream *substream)
 		snd_soc_dai_shutdown(codec_dai, substream);
 
 component_err:
-	soc_pcm_components_close(substream, component);
+	soc_pcm_components_close(substream);
 
 	snd_soc_dai_shutdown(cpu_dai, substream);
 out:
@@ -692,7 +677,7 @@  static int soc_pcm_close(struct snd_pcm_substream *substream)
 
 	soc_rtd_shutdown(rtd, substream);
 
-	soc_pcm_components_close(substream, NULL);
+	soc_pcm_components_close(substream);
 
 	snd_soc_dapm_stream_stop(rtd, substream->stream);