[PATCH] ASoC: core: Make sure component driver names are unique

[PATCH] ASoC: core: Make sure component driver names are unique

[Ranjani Sridharan]

When registering a component, make sure that the driver names
are unique. This will ensure that the snd_soc_rtdcom_lookup()
function returns the right component based on the name.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/soc-core.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 8321e75ff244..00c1f8ce46af 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -286,14 +286,6 @@ struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
 	if (!driver_name)
 		return NULL;
 
-	/*
-	 * NOTE
-	 *
-	 * snd_soc_rtdcom_lookup() will find component from rtd by using
-	 * specified driver name.
-	 * But, if many components which have same driver name are connected
-	 * to 1 rtd, this function will return 1st found component.
-	 */
 	for_each_rtd_components(rtd, i, component) {
 		const char *component_name = component->driver->name;
 
@@ -2640,6 +2632,22 @@ int snd_soc_add_component(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(snd_soc_add_component);
 
+static bool
+snd_soc_component_driver_name_is_unique(const struct snd_soc_component_driver *component_driver)
+{
+	struct snd_soc_component *component;
+
+	mutex_lock(&client_mutex);
+	for_each_component(component)
+		if (!strcmp(component->driver->name, component_driver->name)) {
+			mutex_unlock(&client_mutex);
+			return false;
+		}
+
+	mutex_unlock(&client_mutex);
+	return true;
+}
+
 int snd_soc_register_component(struct device *dev,
 			const struct snd_soc_component_driver *component_driver,
 			struct snd_soc_dai_driver *dai_drv,
@@ -2647,6 +2655,13 @@ int snd_soc_register_component(struct device *dev,
 {
 	struct snd_soc_component *component;
 
+	if (component_driver->name &&
+	    !snd_soc_component_driver_name_is_unique(component_driver)) {
+		dev_err(dev, "component driver name %s is not unique\n",
+			component_driver->name);
+		return -EINVAL;
+	}
+
 	component = devm_kzalloc(dev, sizeof(*component), GFP_KERNEL);
 	if (!component)
 		return -ENOMEM;
-- 
2.17.1

Re: [PATCH] ASoC: core: Make sure component driver names are unique

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Mon, Apr 27, 2020 at 12:33:06PM -0700, Ranjani Sridharan wrote:

> When registering a component, make sure that the driver names
> are unique. This will ensure that the snd_soc_rtdcom_lookup()
> function returns the right component based on the name.

I would not expect driver names to be unique, you can have multiple
instances of the same device on a board for example when two mono
speaker drivers are used for stereo playback.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: core: Make sure component driver names are unique

[Ranjani Sridharan]

On Tue, 2020-04-28 at 12:40 +0100, Mark Brown wrote:
> On Mon, Apr 27, 2020 at 12:33:06PM -0700, Ranjani Sridharan wrote:
> 
> > When registering a component, make sure that the driver names
> > are unique. This will ensure that the snd_soc_rtdcom_lookup()
> > function returns the right component based on the name.
> 
> I would not expect driver names to be unique, you can have multiple
> instances of the same device on a board for example when two mono
> speaker drivers are used for stereo playback.
Maybe I misunderstood your comment in the previous thread then, Mark.

https://mailman.alsa-project.org/pipermail/alsa-devel/2020-April/166665.html

Did you mean to say that the individual drivers should do this check
before registering multiple platform components to make sure they are
unique?

Thanks,
Ranjani

Re: [PATCH] ASoC: core: Make sure component driver names are unique

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

On Tue, Apr 28, 2020 at 09:07:04AM -0700, Ranjani Sridharan wrote:
> On Tue, 2020-04-28 at 12:40 +0100, Mark Brown wrote:

> > I would not expect driver names to be unique, you can have multiple
> > instances of the same device on a board for example when two mono
> > speaker drivers are used for stereo playback.

> Maybe I misunderstood your comment in the previous thread then, Mark.

> https://mailman.alsa-project.org/pipermail/alsa-devel/2020-April/166665.html

> Did you mean to say that the individual drivers should do this check
> before registering multiple platform components to make sure they are
> unique?

That was in the context of a single DAI link, not the system as a whole,
and only for platform drivers not DAIs.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: core: Make sure component driver names are unique

[Ranjani Sridharan]

On Tue, 2020-04-28 at 17:14 +0100, Mark Brown wrote:
> On Tue, Apr 28, 2020 at 09:07:04AM -0700, Ranjani Sridharan wrote:
> > On Tue, 2020-04-28 at 12:40 +0100, Mark Brown wrote:
> > > I would not expect driver names to be unique, you can have
> > > multiple
> > > instances of the same device on a board for example when two mono
> > > speaker drivers are used for stereo playback.
> > Maybe I misunderstood your comment in the previous thread then,
> > Mark.
> > 
https://mailman.alsa-project.org/pipermail/alsa-devel/2020-April/166665.html
> > Did you mean to say that the individual drivers should do this
> > check
> > before registering multiple platform components to make sure they
> > are
> > unique?
> 
> That was in the context of a single DAI link, not the system as a
> whole,
> and only for platform drivers not DAIs.
Ahh, got it. So, maybe I should add this check when adding platform
components to the pcm runtime for the DAI link?

Thanks,
Ranjani

Re: [PATCH] ASoC: core: Make sure component driver names are unique

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

On Tue, Apr 28, 2020 at 09:26:13AM -0700, Ranjani Sridharan wrote:
> On Tue, 2020-04-28 at 17:14 +0100, Mark Brown wrote:

> > That was in the context of a single DAI link, not the system as a
> > whole,
> > and only for platform drivers not DAIs.

> Ahh, got it. So, maybe I should add this check when adding platform
> components to the pcm runtime for the DAI link?

Yes, checking to see if there's any platform component already would be
good - perhaps we might have to relax that in future but right now
there's no clear use case.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: core: Make sure component driver names are unique

[Kuninori Morimoto]

Hi Ranjani

> When registering a component, make sure that the driver names
> are unique. This will ensure that the snd_soc_rtdcom_lookup()
> function returns the right component based on the name.
> 
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
(snip)
> +static bool
> +snd_soc_component_driver_name_is_unique(const struct snd_soc_component_driver *component_driver)
> +{
> +	struct snd_soc_component *component;
> +
> +	mutex_lock(&client_mutex);
> +	for_each_component(component)
> +		if (!strcmp(component->driver->name, component_driver->name)) {
> +			mutex_unlock(&client_mutex);
> +			return false;
> +		}
> +
> +	mutex_unlock(&client_mutex);
> +	return true;
> +}

I think it will be more readable if it doesn't have
multiple mutex_unlock().

	{
		int ret = true;

		mutex_lock();
		for_each_component()
			if (...) {
				...
				ret = false;
			}
		mutex_unlock();
		return ret;
	}

Thank you for your help !!

Best regards
---
Kuninori Morimoto