diff mbox series

[12/19] ASoC: soc-card: add probed bit field to snd_soc_card

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

Commit Message

Kuninori Morimoto May 26, 2020, 1:18 a.m. UTC 
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

We have bit field to control snd_soc_card.
Let's add probed field on it instead of local variable.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  |  1 +
 sound/soc/soc-core.c | 18 ++++++++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

Comments

Pierre-Louis Bossart May 26, 2020, 2:08 p.m. UTC | #1 
On 5/25/20 8:18 PM, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> We have bit field to control snd_soc_card.
> Let's add probed field on it instead of local variable.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   include/sound/soc.h  |  1 +
>   sound/soc/soc-core.c | 18 ++++++++----------
>   2 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 060c01c32547..74868436ac79 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -1096,6 +1096,7 @@ struct snd_soc_card {
>   	unsigned int topology_shortname_created:1;
>   	unsigned int fully_routed:1;
>   	unsigned int disable_route_checks:1;
> +	unsigned int probed:1;
>   
>   	void *drvdata;
>   };
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 02147acf6547..7cfb3ee6ff4f 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1723,8 +1723,7 @@ static void __soc_setup_card_name(char *name, int len,
>   	}
>   }
>   
> -static void soc_cleanup_card_resources(struct snd_soc_card *card,
> -				       int card_probed)
> +static void soc_cleanup_card_resources(struct snd_soc_card *card)
>   {
>   	struct snd_soc_pcm_runtime *rtd, *n;
>   
> @@ -1748,8 +1747,9 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card,
>   	soc_cleanup_card_debugfs(card);
>   
>   	/* remove the card */
> -	if (card_probed && card->remove)
> +	if (card->probed && card->remove)
>   		card->remove(card);
> +	card->probed = 0;
>   
>   	if (card->snd_card) {
>   		snd_card_free(card->snd_card);
> @@ -1760,12 +1760,10 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card,
>   static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister)
>   {
>   	if (card->instantiated) {
> -		int card_probed = 1;
> -

This looks like a change, now soc_cleanup_card_resources() is called 
without setting the card_probed bitfield?

everywhere else I see a 1:1 mapping between variable and bitfield usage, 
not here, is this intentional?

>   		card->instantiated = false;
>   		snd_soc_flush_all_delayed_work(card);
>   
> -		soc_cleanup_card_resources(card, card_probed);
> +		soc_cleanup_card_resources(card);
>   		if (!unregister)
>   			list_add(&card->list, &unbind_card_list);
>   	} else {
> @@ -1779,7 +1777,7 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
>   	struct snd_soc_pcm_runtime *rtd;
>   	struct snd_soc_component *component;
>   	struct snd_soc_dai_link *dai_link;
> -	int ret, i, card_probed = 0;
> +	int ret, i;
>   
>   	mutex_lock(&client_mutex);
>   	mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_INIT);
> @@ -1831,7 +1829,7 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
>   		ret = card->probe(card);
>   		if (ret < 0)
>   			goto probe_end;
> -		card_probed = 1;
> +		card->probed = 1;
>   	}
>   
>   	/* probe all components used by DAI links on this card */
> @@ -1923,7 +1921,7 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
>   			goto probe_end;
>   		}
>   	}
> -	card_probed = 1;
> +	card->probed = 1;
>   
>   	snd_soc_dapm_new_widgets(card);
>   
> @@ -1945,7 +1943,7 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
>   
>   probe_end:
>   	if (ret < 0)
> -		soc_cleanup_card_resources(card, card_probed);
> +		soc_cleanup_card_resources(card);
>   
>   	mutex_unlock(&card->mutex);
>   	mutex_unlock(&client_mutex);
>
Kuninori Morimoto May 27, 2020, 11:58 p.m. UTC | #2 
Hi Pierre-Louis

Thank you for the review

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > We have bit field to control snd_soc_card.
> > Let's add probed field on it instead of local variable.
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > ---
(snip)
> > @@ -1760,12 +1760,10 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card,
> >   static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister)
> >   {
> >   	if (card->instantiated) {
> > -		int card_probed = 1;
> > -
> 
> This looks like a change, now soc_cleanup_card_resources() is called
> without setting the card_probed bitfield?
> 
> everywhere else I see a 1:1 mapping between variable and bitfield
> usage, not here, is this intentional?

This is a little bit confusable point for reviewing.
I will explain, so, please double check it.

This "soc_cleanup_card_resources()" will be called from
snd_soc_unbind_card() as "formal cleanup" or,
snd_soc_bind_card()   as "error handling" (D).

For "error handling" purpose, it needed to have "card_probed"
flags as parameter.

This "card->instantiated" will be set at (A) if
all probe functions were called (B) without error.
By this patch, "probed" is handled by "card".
Thus, it already has card->probed flag if
card has instantiated flag.

	static int snd_soc_bind_card(struct snd_soc_card *card)
	{
		...
	(B)	ret = snd_soc_card_probe(card);
		if (ret < 0)
			goto probe_end;
		...
	(B)	ret = snd_soc_card_late_probe(card);
		if (ret < 0)
			goto probe_end;
		...
	(A)	card->instantiated = 1;
		...
	probe_end:
		if (ret < 0)
	(D)		soc_cleanup_card_resources();
		...
	}

But indeed patch and log are confusable.
I will try to add such explanation at git-log area.

Thank you for your help !!

Best regards
---
Kuninori Morimoto
diff mbox series

Patch

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 060c01c32547..74868436ac79 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1096,6 +1096,7 @@  struct snd_soc_card {
 	unsigned int topology_shortname_created:1;
 	unsigned int fully_routed:1;
 	unsigned int disable_route_checks:1;
+	unsigned int probed:1;
 
 	void *drvdata;
 };
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 02147acf6547..7cfb3ee6ff4f 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1723,8 +1723,7 @@  static void __soc_setup_card_name(char *name, int len,
 	}
 }
 
-static void soc_cleanup_card_resources(struct snd_soc_card *card,
-				       int card_probed)
+static void soc_cleanup_card_resources(struct snd_soc_card *card)
 {
 	struct snd_soc_pcm_runtime *rtd, *n;
 
@@ -1748,8 +1747,9 @@  static void soc_cleanup_card_resources(struct snd_soc_card *card,
 	soc_cleanup_card_debugfs(card);
 
 	/* remove the card */
-	if (card_probed && card->remove)
+	if (card->probed && card->remove)
 		card->remove(card);
+	card->probed = 0;
 
 	if (card->snd_card) {
 		snd_card_free(card->snd_card);
@@ -1760,12 +1760,10 @@  static void soc_cleanup_card_resources(struct snd_soc_card *card,
 static void snd_soc_unbind_card(struct snd_soc_card *card, bool unregister)
 {
 	if (card->instantiated) {
-		int card_probed = 1;
-
 		card->instantiated = false;
 		snd_soc_flush_all_delayed_work(card);
 
-		soc_cleanup_card_resources(card, card_probed);
+		soc_cleanup_card_resources(card);
 		if (!unregister)
 			list_add(&card->list, &unbind_card_list);
 	} else {
@@ -1779,7 +1777,7 @@  static int snd_soc_bind_card(struct snd_soc_card *card)
 	struct snd_soc_pcm_runtime *rtd;
 	struct snd_soc_component *component;
 	struct snd_soc_dai_link *dai_link;
-	int ret, i, card_probed = 0;
+	int ret, i;
 
 	mutex_lock(&client_mutex);
 	mutex_lock_nested(&card->mutex, SND_SOC_CARD_CLASS_INIT);
@@ -1831,7 +1829,7 @@  static int snd_soc_bind_card(struct snd_soc_card *card)
 		ret = card->probe(card);
 		if (ret < 0)
 			goto probe_end;
-		card_probed = 1;
+		card->probed = 1;
 	}
 
 	/* probe all components used by DAI links on this card */
@@ -1923,7 +1921,7 @@  static int snd_soc_bind_card(struct snd_soc_card *card)
 			goto probe_end;
 		}
 	}
-	card_probed = 1;
+	card->probed = 1;
 
 	snd_soc_dapm_new_widgets(card);
 
@@ -1945,7 +1943,7 @@  static int snd_soc_bind_card(struct snd_soc_card *card)
 
 probe_end:
 	if (ret < 0)
-		soc_cleanup_card_resources(card, card_probed);
+		soc_cleanup_card_resources(card);
 
 	mutex_unlock(&card->mutex);
 	mutex_unlock(&client_mutex);