diff mbox series

[v2,resend,07/18] ASoC: soc-core: move snd_soc_lookup_component()

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

Commit Message

Kuninori Morimoto Oct. 30, 2019, 1:26 a.m. UTC 
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This patch moves snd_soc_lookup_component() to upper side.
This is prepare for snd_soc_unregister_component()

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

Comments

Pierre-Louis Bossart Nov. 5, 2019, 2:22 a.m. UTC | #1 
> +struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
> +						   const char *driver_name)
> +{
> +	struct snd_soc_component *component;
> +	struct snd_soc_component *ret;
> +
> +	ret = NULL;
> +	mutex_lock(&client_mutex);
> +	for_each_component(component) {
> +		if (dev != component->dev)
> +			continue;
> +
> +		if (driver_name &&
> +		    (driver_name != component->driver->name) &&
> +		    (strcmp(component->driver->name, driver_name) != 0))
> +			continue;
> +
> +		ret = component;
> +		break;

The mix of continue and break in the same loop is odd.

can this be done with break only, e.g. (check the logic)

for_each_component(component) {
	if (dev == component->dev &&
	   (!driver_name ||
	    (driver_name == component->driver->name) ||
	    (strcmp(component->driver->name, driver_name) == 0))
		ret = component;
		break;
}

> +	mutex_unlock(&client_mutex);
> +
> +	return ret;

usually 'return ret' is for an error code. It's odd to use it for a pointer.

> +}
> +EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
> +
>   struct snd_pcm_substream *snd_soc_get_dai_substream(struct snd_soc_card *card,
>   		const char *dai_link, int stream)
>   {
> @@ -2889,32 +2915,6 @@ void snd_soc_unregister_component(struct device *dev)
>   }
>   EXPORT_SYMBOL_GPL(snd_soc_unregister_component);
>   
> -struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
> -						   const char *driver_name)
> -{
> -	struct snd_soc_component *component;
> -	struct snd_soc_component *ret;
> -
> -	ret = NULL;
> -	mutex_lock(&client_mutex);
> -	for_each_component(component) {
> -		if (dev != component->dev)
> -			continue;
> -
> -		if (driver_name &&
> -		    (driver_name != component->driver->name) &&
> -		    (strcmp(component->driver->name, driver_name) != 0))
> -			continue;
> -
> -		ret = component;
> -		break;
> -	}
> -	mutex_unlock(&client_mutex);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
> -
>   /* Retrieve a card's name from device tree */
>   int snd_soc_of_parse_card_name(struct snd_soc_card *card,
>   			       const char *propname)
>
Kuninori Morimoto Nov. 5, 2019, 4:10 a.m. UTC | #2 
Hi Pierre-Louis

> > +struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
> > +						   const char *driver_name)
> > +{
> > +	struct snd_soc_component *component;
> > +	struct snd_soc_component *ret;
> > +
> > +	ret = NULL;
> > +	mutex_lock(&client_mutex);
> > +	for_each_component(component) {
> > +		if (dev != component->dev)
> > +			continue;
> > +
> > +		if (driver_name &&
> > +		    (driver_name != component->driver->name) &&
> > +		    (strcmp(component->driver->name, driver_name) != 0))
> > +			continue;
> > +
> > +		ret = component;
> > +		break;
> 
> The mix of continue and break in the same loop is odd.
> 
> can this be done with break only, e.g. (check the logic)
> 
> for_each_component(component) {
> 	if (dev == component->dev &&
> 	   (!driver_name ||
> 	    (driver_name == component->driver->name) ||
> 	    (strcmp(component->driver->name, driver_name) == 0))
> 		ret = component;
> 		break;
> }

Indeed !!
Will post follow-up patch

> > +	mutex_unlock(&client_mutex);
> > +
> > +	return ret;
> 
> usually 'return ret' is for an error code. It's odd to use it for a pointer.

Will fix.

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

Patch

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index fa837c0..72eb59c 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -356,6 +356,32 @@  struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
 }
 EXPORT_SYMBOL_GPL(snd_soc_rtdcom_lookup);
 
+struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
+						   const char *driver_name)
+{
+	struct snd_soc_component *component;
+	struct snd_soc_component *ret;
+
+	ret = NULL;
+	mutex_lock(&client_mutex);
+	for_each_component(component) {
+		if (dev != component->dev)
+			continue;
+
+		if (driver_name &&
+		    (driver_name != component->driver->name) &&
+		    (strcmp(component->driver->name, driver_name) != 0))
+			continue;
+
+		ret = component;
+		break;
+	}
+	mutex_unlock(&client_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
+
 struct snd_pcm_substream *snd_soc_get_dai_substream(struct snd_soc_card *card,
 		const char *dai_link, int stream)
 {
@@ -2889,32 +2915,6 @@  void snd_soc_unregister_component(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(snd_soc_unregister_component);
 
-struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
-						   const char *driver_name)
-{
-	struct snd_soc_component *component;
-	struct snd_soc_component *ret;
-
-	ret = NULL;
-	mutex_lock(&client_mutex);
-	for_each_component(component) {
-		if (dev != component->dev)
-			continue;
-
-		if (driver_name &&
-		    (driver_name != component->driver->name) &&
-		    (strcmp(component->driver->name, driver_name) != 0))
-			continue;
-
-		ret = component;
-		break;
-	}
-	mutex_unlock(&client_mutex);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(snd_soc_lookup_component);
-
 /* Retrieve a card's name from device tree */
 int snd_soc_of_parse_card_name(struct snd_soc_card *card,
 			       const char *propname)