| Message ID | 20200427193306.31198-1-ranjani.sridharan@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: core: Make sure component driver names are unique | expand |
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.
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
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.
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
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.
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
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;
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(-)