alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC: soc-core: add the driver component name to the component struc
@ 2021-12-06  9:59 Kory Maincent
  2021-12-06 19:33 ` Mark Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Kory Maincent @ 2021-12-06  9:59 UTC (permalink / raw)
  To: alsa-devel, linux-kernel
  Cc: alexandre.belloni, Takashi Iwai, Liam Girdwood, Mark Brown,
	thomas.petazzoni

The function fmt_single_name is using to sanitize namings, and it return
the name set to component->name.
This function is returning either the "device_name" or the
"device_name.driver_name" for i2c devices. There is no use of the component
driver name.
If a non i2c driver register two components the function will return the
same "device_name" for both components. This could cause unexpected issue,
in my case it is a debugfs error which tries to create two directory with
the same component name.

I have fixed it by prefixing the component name with the driver component
name.

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
 sound/soc/soc-core.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index dcf6be4c4aaa..21ff77b231b8 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2342,10 +2342,10 @@ EXPORT_SYMBOL_GPL(snd_soc_unregister_card);
  * Simplify DAI link configuration by removing ".-1" from device names
  * and sanitizing names.
  */
-static char *fmt_single_name(struct device *dev, int *id)
+static char *fmt_single_name(struct device *dev, const char *snd_drv_name, int *id)
 {
 	const char *devname = dev_name(dev);
-	char *found, *name;
+	char *found, *name, *tmp;
 	unsigned int id1, id2;
 
 	if (devname == NULL)
@@ -2380,6 +2380,14 @@ static char *fmt_single_name(struct device *dev, int *id)
 		*id = 0;
 	}
 
+	if (snd_drv_name != NULL) {
+		/* Add driver component name if present */
+		tmp = devm_kasprintf(dev, GFP_KERNEL, "%s.%s", snd_drv_name, name);
+		devm_kfree(dev, name);
+		name = devm_kstrdup(dev, tmp, GFP_KERNEL);
+		devm_kfree(dev, tmp);
+	}
+
 	return name;
 }
 
@@ -2444,7 +2452,7 @@ struct snd_soc_dai *snd_soc_register_dai(struct snd_soc_component *component,
 	 */
 	if (legacy_dai_naming &&
 	    (dai_drv->id == 0 || dai_drv->name == NULL)) {
-		dai->name = fmt_single_name(dev, &dai->id);
+		dai->name = fmt_single_name(dev, dai_drv->name, &dai->id);
 	} else {
 		dai->name = fmt_multiple_name(dev, dai_drv);
 		if (dai_drv->id)
@@ -2578,7 +2586,7 @@ int snd_soc_component_initialize(struct snd_soc_component *component,
 	INIT_LIST_HEAD(&component->list);
 	mutex_init(&component->io_mutex);
 
-	component->name = fmt_single_name(dev, &component->id);
+	component->name = fmt_single_name(dev, driver->name, &component->id);
 	if (!component->name) {
 		dev_err(dev, "ASoC: Failed to allocate name\n");
 		return -ENOMEM;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] ASoC: soc-core: add the driver component name to the component struc
  2021-12-06  9:59 [PATCH] ASoC: soc-core: add the driver component name to the component struc Kory Maincent
@ 2021-12-06 19:33 ` Mark Brown
  2021-12-07  8:47   ` Köry Maincent
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2021-12-06 19:33 UTC (permalink / raw)
  To: Kory Maincent
  Cc: alsa-devel, alexandre.belloni, Liam Girdwood, Takashi Iwai,
	linux-kernel, thomas.petazzoni

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

On Mon, Dec 06, 2021 at 10:59:20AM +0100, Kory Maincent wrote:

> If a non i2c driver register two components the function will return the
> same "device_name" for both components. This could cause unexpected issue,
> in my case it is a debugfs error which tries to create two directory with
> the same component name.

Why is one device registering multiple components in the first place?

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ASoC: soc-core: add the driver component name to the component struc
  2021-12-06 19:33 ` Mark Brown
@ 2021-12-07  8:47   ` Köry Maincent
  2021-12-07 13:08     ` Mark Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Köry Maincent @ 2021-12-07  8:47 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, alexandre.belloni, Liam Girdwood, Takashi Iwai,
	linux-kernel, thomas.petazzoni

Hello Mark,

On Mon, 6 Dec 2021 19:33:58 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Mon, Dec 06, 2021 at 10:59:20AM +0100, Kory Maincent wrote:
> 
> > If a non i2c driver register two components the function will return the
> > same "device_name" for both components. This could cause unexpected issue,
> > in my case it is a debugfs error which tries to create two directory with
> > the same component name.  
> 
> Why is one device registering multiple components in the first place?

Because the sound components are more and more complex. Why they shouldn't?

It seems to be already the case:
sound/soc/codecs/cros_ec_codec.c
sound/soc/fsl/fsl_easrc.c
sound/soc/mediatek/mt*/mt*-afe-pcm.c
sound/soc/sunxi/sun4i-codec.c
sound/soc/soc-utils.c

Regards,

Köry

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ASoC: soc-core: add the driver component name to the component struc
  2021-12-07  8:47   ` Köry Maincent
@ 2021-12-07 13:08     ` Mark Brown
  2021-12-07 15:16       ` Köry Maincent
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2021-12-07 13:08 UTC (permalink / raw)
  To: Köry Maincent
  Cc: alsa-devel, alexandre.belloni, Liam Girdwood, Takashi Iwai,
	linux-kernel, thomas.petazzoni

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

On Tue, Dec 07, 2021 at 09:47:32AM +0100, Köry Maincent wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > Why is one device registering multiple components in the first place?

> Because the sound components are more and more complex. Why they shouldn't?

In what way are they more complex?  

> It seems to be already the case:
> sound/soc/codecs/cros_ec_codec.c
> sound/soc/fsl/fsl_easrc.c
> sound/soc/mediatek/mt*/mt*-afe-pcm.c
> sound/soc/sunxi/sun4i-codec.c
> sound/soc/soc-utils.c

Quite a few (I think all?) of these are quite old and and are the result
of refactoring from pre-component code rather than modern drivers, it's
likely there is no concrete reason for them to behave as they do.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ASoC: soc-core: add the driver component name to the component struc
  2021-12-07 13:08     ` Mark Brown
@ 2021-12-07 15:16       ` Köry Maincent
  0 siblings, 0 replies; 5+ messages in thread
From: Köry Maincent @ 2021-12-07 15:16 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, alexandre.belloni, Liam Girdwood, Takashi Iwai,
	linux-kernel, thomas.petazzoni

Mark,

On Tue, 7 Dec 2021 13:08:33 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Tue, Dec 07, 2021 at 09:47:32AM +0100, Köry Maincent wrote:
> > Mark Brown <broonie@kernel.org> wrote:  
> 
> > > Why is one device registering multiple components in the first place?  
> 
> > Because the sound components are more and more complex. Why they shouldn't?
> >  
> 
> In what way are they more complex? 

The sound hardware components add more and more features.

> 
> > It seems to be already the case:
> > sound/soc/codecs/cros_ec_codec.c
> > sound/soc/fsl/fsl_easrc.c
> > sound/soc/mediatek/mt*/mt*-afe-pcm.c
> > sound/soc/sunxi/sun4i-codec.c
> > sound/soc/soc-utils.c  
> 
> Quite a few (I think all?) of these are quite old and and are the result
> of refactoring from pre-component code rather than modern drivers, it's
> likely there is no concrete reason for them to behave as they do.

I am a beginner in the kernel sound stack, alright then, the issue comes from
the drivers.

Thanks,

Regards

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-07 15:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-06  9:59 [PATCH] ASoC: soc-core: add the driver component name to the component struc Kory Maincent
2021-12-06 19:33 ` Mark Brown
2021-12-07  8:47   ` Köry Maincent
2021-12-07 13:08     ` Mark Brown
2021-12-07 15:16       ` Köry Maincent

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).