diff mbox series

[06/11] ASoC: soc-dapm.c: merge dapm_power_one_widget() and dapm_widget_set_power()

Message ID 87tu42owdd.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State Superseded
Headers show
Series ASoC: soc-dapm.c random cleanups | expand

Commit Message

Kuninori Morimoto Oct. 17, 2022, 11:37 p.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

dapm_widget_set_power() (= X) is called only from
dapm_power_one_widget() (= Y), and total purpose of these functions are
calling dapm_seq_insert() (= a) accordingly for each widget.

(X)	static void dapm_widget_set_power(...)
	{
		...
		if (power)
(a)			dapm_seq_insert(w, up_list, true);
		else
(a)			dapm_seq_insert(w, down_list, false);
	}

(Y)	static void dapm_power_one_widget(...)
	{
		..

		switch (w->id) {
		case snd_soc_dapm_pre:
(a)			dapm_seq_insert(w, down_list, false);
			break;
		case snd_soc_dapm_post:
(a)			dapm_seq_insert(w, up_list, true);
			break;

		default:
			power = dapm_widget_power_check(w);

(X)			dapm_widget_set_power(w, power, up_list, down_list);
			break;
		}
	}

It should be more simple, but the code is unnecessarily complicated,
and difficult to read/understand. This patch merge these into one.

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

Comments

Amadeusz Sławiński Oct. 18, 2022, 10:47 a.m. UTC | #1
On 10/18/2022 1:37 AM, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> dapm_widget_set_power() (= X) is called only from
> dapm_power_one_widget() (= Y), and total purpose of these functions are
> calling dapm_seq_insert() (= a) accordingly for each widget.
> 
> (X)	static void dapm_widget_set_power(...)
> 	{
> 		...
> 		if (power)
> (a)			dapm_seq_insert(w, up_list, true);
> 		else
> (a)			dapm_seq_insert(w, down_list, false);
> 	}
> 
> (Y)	static void dapm_power_one_widget(...)
> 	{
> 		..
> 
> 		switch (w->id) {
> 		case snd_soc_dapm_pre:
> (a)			dapm_seq_insert(w, down_list, false);
> 			break;
> 		case snd_soc_dapm_post:
> (a)			dapm_seq_insert(w, up_list, true);
> 			break;
> 
> 		default:
> 			power = dapm_widget_power_check(w);
> 
> (X)			dapm_widget_set_power(w, power, up_list, down_list);
> 			break;
> 		}
> 	}
> 
> It should be more simple, but the code is unnecessarily complicated,
> and difficult to read/understand. This patch merge these into one.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   sound/soc/soc-dapm.c | 39 +++++++++++++++------------------------
>   1 file changed, 15 insertions(+), 24 deletions(-)
> 
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index d4281e776e44..764830a51d2d 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -1873,11 +1873,24 @@ static void dapm_widget_set_peer_power(struct snd_soc_dapm_widget *peer,
>   		dapm_mark_dirty(peer, "peer state change");
>   }
>   
> -static void dapm_widget_set_power(struct snd_soc_dapm_widget *w, bool power,
> +static void dapm_power_one_widget(struct snd_soc_dapm_widget *w,
>   				  struct list_head *up_list,
>   				  struct list_head *down_list)
>   {
>   	struct snd_soc_dapm_path *path;
> +	int power;
> +
> +	switch (w->id) {
> +	case snd_soc_dapm_pre:
> +		power = 0;
> +		goto end;
> +	case snd_soc_dapm_post:
> +		power = 1;
> +		goto end;
> +	default:
> +	}

This introduces build error when applied:

sound/soc/soc-dapm.c: In function ‘dapm_power_one_widget’:
sound/soc/soc-dapm.c:1890:2: error: label at end of compound statement
  1890 |  default:
       |  ^~~~~~~

(May be because of CONFIG_WERROR, but still it would be a warning at 
least...)

> +
> +	power = dapm_widget_power_check(w);
>   
>   	if (w->power == power)
>   		return;
> @@ -1897,35 +1910,13 @@ static void dapm_widget_set_power(struct snd_soc_dapm_widget *w, bool power,
>   	if (!w->is_supply)
>   		snd_soc_dapm_widget_for_each_sink_path(w, path)
>   			dapm_widget_set_peer_power(path->sink, power, path->connect);
> -
> +end:
>   	if (power)
>   		dapm_seq_insert(w, up_list, true);
>   	else
>   		dapm_seq_insert(w, down_list, false);
>   }
>   
> -static void dapm_power_one_widget(struct snd_soc_dapm_widget *w,
> -				  struct list_head *up_list,
> -				  struct list_head *down_list)
> -{
> -	int power;
> -
> -	switch (w->id) {
> -	case snd_soc_dapm_pre:
> -		dapm_seq_insert(w, down_list, false);
> -		break;
> -	case snd_soc_dapm_post:
> -		dapm_seq_insert(w, up_list, true);
> -		break;
> -
> -	default:
> -		power = dapm_widget_power_check(w);
> -
> -		dapm_widget_set_power(w, power, up_list, down_list);
> -		break;
> -	}
> -}
> -
>   static bool dapm_idle_bias_off(struct snd_soc_dapm_context *dapm)
>   {
>   	if (dapm->idle_bias_off)
Mark Brown Oct. 18, 2022, 12:49 p.m. UTC | #2
On Tue, Oct 18, 2022 at 12:47:24PM +0200, Amadeusz Sławiński wrote:
> On 10/18/2022 1:37 AM, Kuninori Morimoto wrote:

> > +	default:
> > +	}
> 
> This introduces build error when applied:
> 
> sound/soc/soc-dapm.c: In function ‘dapm_power_one_widget’:
> sound/soc/soc-dapm.c:1890:2: error: label at end of compound statement
>  1890 |  default:
>       |  ^~~~~~~
> 
> (May be because of CONFIG_WERROR, but still it would be a warning at
> least...)

Probably also depends on toolchain.  In any case a break; is needed
there.
Kuninori Morimoto Oct. 19, 2022, 12:22 a.m. UTC | #3
Hi Amadeusz, Mark

> > > +	default:
> > > +	}
> > 
> > This introduces build error when applied:
> > 
> > sound/soc/soc-dapm.c: In function ‘dapm_power_one_widget’:
> > sound/soc/soc-dapm.c:1890:2: error: label at end of compound statement
> >  1890 |  default:
> >       |  ^~~~~~~
> > 
> > (May be because of CONFIG_WERROR, but still it would be a warning at
> > least...)
> 
> Probably also depends on toolchain.  In any case a break; is needed
> there.

Thank you for pointing it.
I didn't get such error/warning...
will send v2 patch, soon.

Thank you for your help !!

Best regards
---
Kuninori Morimoto
diff mbox series

Patch

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index d4281e776e44..764830a51d2d 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -1873,11 +1873,24 @@  static void dapm_widget_set_peer_power(struct snd_soc_dapm_widget *peer,
 		dapm_mark_dirty(peer, "peer state change");
 }
 
-static void dapm_widget_set_power(struct snd_soc_dapm_widget *w, bool power,
+static void dapm_power_one_widget(struct snd_soc_dapm_widget *w,
 				  struct list_head *up_list,
 				  struct list_head *down_list)
 {
 	struct snd_soc_dapm_path *path;
+	int power;
+
+	switch (w->id) {
+	case snd_soc_dapm_pre:
+		power = 0;
+		goto end;
+	case snd_soc_dapm_post:
+		power = 1;
+		goto end;
+	default:
+	}
+
+	power = dapm_widget_power_check(w);
 
 	if (w->power == power)
 		return;
@@ -1897,35 +1910,13 @@  static void dapm_widget_set_power(struct snd_soc_dapm_widget *w, bool power,
 	if (!w->is_supply)
 		snd_soc_dapm_widget_for_each_sink_path(w, path)
 			dapm_widget_set_peer_power(path->sink, power, path->connect);
-
+end:
 	if (power)
 		dapm_seq_insert(w, up_list, true);
 	else
 		dapm_seq_insert(w, down_list, false);
 }
 
-static void dapm_power_one_widget(struct snd_soc_dapm_widget *w,
-				  struct list_head *up_list,
-				  struct list_head *down_list)
-{
-	int power;
-
-	switch (w->id) {
-	case snd_soc_dapm_pre:
-		dapm_seq_insert(w, down_list, false);
-		break;
-	case snd_soc_dapm_post:
-		dapm_seq_insert(w, up_list, true);
-		break;
-
-	default:
-		power = dapm_widget_power_check(w);
-
-		dapm_widget_set_power(w, power, up_list, down_list);
-		break;
-	}
-}
-
 static bool dapm_idle_bias_off(struct snd_soc_dapm_context *dapm)
 {
 	if (dapm->idle_bias_off)