[RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

This optional kcontrol_prefix allows to specify unique prefix for ALSA
control names for each DAI link. This makes possible to have a sound card
configuration with multiple DAIs and each of them using the same codec
driver without name collision.

Currently name collision would occur if a codec driver tries to register a
kcontrol with a same name for another DAI link.

Now it is possible to specify for instance "Front" and "Rear" prefixes and
a sound card can have two separate controls like "Front.PCM Playback Volume"
and "Rear.PCM Playback Volume". Those controls will then show as
"Front.PCM" and "Rear.PCM" in ALSA mixer application.

Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
---
 include/sound/soc.h  |    2 ++
 sound/soc/soc-core.c |   13 +++++++++++--
 sound/soc/soc-dapm.c |   28 ++++++++++++++++++++++------
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index d31e8b7..9b83f7e 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -423,6 +423,7 @@ struct snd_soc_ops {
 /* SoC Audio Codec device */
 struct snd_soc_codec {
 	const char *name;
+	const char *kcontrol_prefix;
 	int id;
 	struct device *dev;
 	struct snd_soc_codec_driver *driver;
@@ -539,6 +540,7 @@ struct snd_soc_dai_link {
 	const char *platform_name;	/* for multi-platform */
 	const char *cpu_dai_name;
 	const char *codec_dai_name;
+	const char *kcontrol_prefix;	/* kcontrol prefix for multi-codec */
 
 	/* Keep DAI active over suspend */
 	unsigned int ignore_suspend:1;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index a004876..f52eb2a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1289,6 +1289,7 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num)
 	/* config components */
 	codec_dai->codec = codec;
 	codec->card = card;
+	codec->kcontrol_prefix = dai_link->kcontrol_prefix;
 	cpu_dai->platform = platform;
 	rtd->card = card;
 	rtd->dev.parent = card->dev;
@@ -1903,14 +1904,22 @@ int snd_soc_add_controls(struct snd_soc_codec *codec,
 	const struct snd_kcontrol_new *controls, int num_controls)
 {
 	struct snd_card *card = codec->card->snd_card;
+	char prefixed_name[44], *name;
 	int err, i;
 
 	for (i = 0; i < num_controls; i++) {
 		const struct snd_kcontrol_new *control = &controls[i];
-		err = snd_ctl_add(card, snd_soc_cnew(control, codec, NULL));
+		if (codec->kcontrol_prefix) {
+			snprintf(prefixed_name, sizeof(prefixed_name), "%s.%s",
+				 codec->kcontrol_prefix, control->name);
+			name = prefixed_name;
+		} else {
+			name = control->name;
+		}
+		err = snd_ctl_add(card, snd_soc_cnew(control, codec, name));
 		if (err < 0) {
 			dev_err(codec->dev, "%s: Failed to add %s\n",
-				codec->name, control->name);
+				codec->name, name);
 			return err;
 		}
 	}
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 035cab8..5751f4f 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -327,6 +327,7 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 	int i, ret = 0;
 	size_t name_len;
 	struct snd_soc_dapm_path *path;
+	char prefix[10];
 
 	/* add kcontrol */
 	for (i = 0; i < w->num_kcontrols; i++) {
@@ -347,6 +348,13 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 			name_len = strlen(w->kcontrols[i].name) + 1;
 			if (w->id != snd_soc_dapm_mixer_named_ctl)
 				name_len += 1 + strlen(w->name);
+			if (codec->kcontrol_prefix) {
+				name_len += 1 + strlen(codec->kcontrol_prefix);
+				snprintf(prefix, sizeof(prefix), "%s.",
+					 codec->kcontrol_prefix);
+			} else {
+				prefix[0] = '\0';
+			}
 
 			path->long_name = kmalloc(name_len, GFP_KERNEL);
 
@@ -355,12 +363,12 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 
 			switch (w->id) {
 			default:
-				snprintf(path->long_name, name_len, "%s %s",
-					 w->name, w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s %s",
+					 prefix, w->name, w->kcontrols[i].name);
 				break;
 			case snd_soc_dapm_mixer_named_ctl:
-				snprintf(path->long_name, name_len, "%s",
-					 w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s",
+					 prefix, w->kcontrols[i].name);
 				break;
 			}
 
@@ -388,6 +396,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 {
 	struct snd_soc_dapm_path *path = NULL;
 	struct snd_kcontrol *kcontrol;
+	char prefixed_name[44], *name;
 	int ret = 0;
 
 	if (!w->num_kcontrols) {
@@ -395,7 +404,14 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 		return -EINVAL;
 	}
 
-	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, w->name);
+	if (codec->kcontrol_prefix) {
+		snprintf(prefixed_name, sizeof(prefixed_name), "%s.%s",
+			 codec->kcontrol_prefix, w->name);
+		name = prefixed_name;
+	} else {
+		name = w->name;
+	}
+	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, name);
 	ret = snd_ctl_add(codec->card->snd_card, kcontrol);
 	if (ret < 0)
 		goto err;
@@ -406,7 +422,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 	return ret;
 
 err:
-	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", w->name);
+	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", name);
 	return ret;
 }
 
-- 
1.7.1

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Mark Brown]

On Mon, Aug 16, 2010 at 10:29:30AM +0300, Jarkko Nikula wrote:
> This optional kcontrol_prefix allows to specify unique prefix for ALSA
> control names for each DAI link. This makes possible to have a sound card
> configuration with multiple DAIs and each of them using the same codec
> driver without name collision.

This isn't going to work in general - consider what happens for CODECs
with multiple DAIs, or for devices with no DAIs at all like external
analogue amps.  We do need to do this but it probably needs to be per
CODEC rather than per link I fear.

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Mon, 16 Aug 2010 11:07:05 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Mon, Aug 16, 2010 at 10:29:30AM +0300, Jarkko Nikula wrote:
> > This optional kcontrol_prefix allows to specify unique prefix for ALSA
> > control names for each DAI link. This makes possible to have a sound card
> > configuration with multiple DAIs and each of them using the same codec
> > driver without name collision.
> 
> This isn't going to work in general - consider what happens for CODECs
> with multiple DAIs, or for devices with no DAIs at all like external
> analogue amps.  We do need to do this but it probably needs to be per
> CODEC rather than per link I fear.

Yeah, true, this is not going to work if there are multiple amplifiers
that are registered for a same link.

Multi-DAI codecs are not so clear to me. I thought codecs are exporting
different controls for different DAIs? Like "foo Playback Volume" and
"bar Playback Volume".

But anyway, I'll try to look some better idea.


-- 
Jarkko

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Mark Brown]

On Mon, Aug 16, 2010 at 01:53:28PM +0300, Jarkko Nikula wrote:

> Multi-DAI codecs are not so clear to me. I thought codecs are exporting
> different controls for different DAIs? Like "foo Playback Volume" and
> "bar Playback Volume".

No, consider CODECs which support mixing signals between the two DAIs
like the WM9713 or WM8994.  You will normally have some controls for the
very edge of the CODEC but for at the parts of the CODEC after mixing
there's no way to distinguish based on the DAI.

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Mon, 16 Aug 2010 12:09:03 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Mon, Aug 16, 2010 at 01:53:28PM +0300, Jarkko Nikula wrote:
> 
> > Multi-DAI codecs are not so clear to me. I thought codecs are exporting
> > different controls for different DAIs? Like "foo Playback Volume" and
> > "bar Playback Volume".
> 
> No, consider CODECs which support mixing signals between the two DAIs
> like the WM9713 or WM8994.  You will normally have some controls for the
> very edge of the CODEC but for at the parts of the CODEC after mixing
> there's no way to distinguish based on the DAI.

Here's the work in progress version of this RFC before trying to patch
all the drivers. Previous version wasn't enough at all.

kcontrol or name prefix still comes from dai_link->kcontrol_prefix
(where it should be per codec as pointer by Mark). And no drivers are
patched for snd_soc_add_controls, snd_soc_dapm_new_control,
snd_soc_dapm_new_controls and snd_soc_dapm_add_routes API changes.

But core ideas are here how different drivers should call those
functions:

Codec:
- kcontrol prefix
- no widget name prefix (as they are per codec)
- no audio map prefix (as they are per codec)

Machine:
- no kcontrol prefix (they are anyway machine specific)
- no widget name prefix (they are anyway machine specific)
- no audio map prefix (since this is includes both codec widgets
  with no prefixes and extra driver widgets with prefixes)

Extra driver, e.g. amplifier registered to a codec in machine driver:
- kcontrol prefix (this must be different than codec->kcontrol_prefix)
- widget name prefix (ditto)
- audio map prefix (ditto)

So codec drivers would pass prefix to snd_soc_add_controls and NULL to
snd_soc_dapm_new_controls and snd_soc_dapm_add_routes.

Machine drivers will pass NULL to all.

Extra drivers will pass some own prefix from machine driver to all of
those functions so that multiple instances could be registered to
same codec.


-- 
Jarkko

============== HALF DONE RFC ================
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index c4a4456..b0e1323 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -315,16 +315,18 @@ int snd_soc_dapm_get_pin_switch(struct snd_kcontrol *kcontrol,
 int snd_soc_dapm_put_pin_switch(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *uncontrol);
 int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
-	const struct snd_soc_dapm_widget *widget);
+	const struct snd_soc_dapm_widget *widget,
+	const char *name_prefix);
 int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 	const struct snd_soc_dapm_widget *widget,
-	int num);
+	int num, const char *name_prefix);
 
 /* dapm path setup */
 int snd_soc_dapm_new_widgets(struct snd_soc_codec *codec);
 void snd_soc_dapm_free(struct snd_soc_codec *codec);
 int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
-			    const struct snd_soc_dapm_route *route, int num);
+			    const struct snd_soc_dapm_route *route, int num,
+			    const char *name_prefix);
 
 /* dapm events */
 int snd_soc_dapm_stream_event(struct snd_soc_pcm_runtime *rtd,
@@ -412,6 +414,7 @@ struct snd_soc_dapm_path {
 struct snd_soc_dapm_widget {
 	enum snd_soc_dapm_type id;
 	char *name;		/* widget name */
+	bool prefixed;	/* set if widget name is prefixed */
 	char *sname;	/* stream name */
 	struct snd_soc_codec *codec;
 	struct list_head list;
diff --git a/include/sound/soc.h b/include/sound/soc.h
index d31e8b7..cc4acd9 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -311,7 +311,8 @@ void snd_soc_free_ac97_codec(struct snd_soc_codec *codec);
 struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template,
 	void *data, char *long_name);
 int snd_soc_add_controls(struct snd_soc_codec *codec,
-	const struct snd_kcontrol_new *controls, int num_controls);
+	const struct snd_kcontrol_new *controls, int num_controls,
+	const char *name_prefix);
 int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_info *uinfo);
 int snd_soc_info_enum_ext(struct snd_kcontrol *kcontrol,
@@ -423,6 +424,7 @@ struct snd_soc_ops {
 /* SoC Audio Codec device */
 struct snd_soc_codec {
 	const char *name;
+	const char *kcontrol_prefix;
 	int id;
 	struct device *dev;
 	struct snd_soc_codec_driver *driver;
@@ -539,6 +541,7 @@ struct snd_soc_dai_link {
 	const char *platform_name;	/* for multi-platform */
 	const char *cpu_dai_name;
 	const char *codec_dai_name;
+	const char *kcontrol_prefix;	/* kcontrol prefix for multi-codec */
 
 	/* Keep DAI active over suspend */
 	unsigned int ignore_suspend:1;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 7093c17..258cde4 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1280,6 +1280,7 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num)
 	/* config components */
 	codec_dai->codec = codec;
 	codec->card = card;
+	codec->kcontrol_prefix = dai_link->kcontrol_prefix;
 	cpu_dai->platform = platform;
 	rtd->card = card;
 	rtd->dev.parent = card->dev;
@@ -1888,21 +1889,31 @@ EXPORT_SYMBOL_GPL(snd_soc_cnew);
  * @codec: codec to add controls to
  * @controls: array of controls to add
  * @num_controls: number of elements in the array
+ * @name_prefix: prefix to kcontrol name or NULL
  *
  * Return 0 for success, else error.
  */
 int snd_soc_add_controls(struct snd_soc_codec *codec,
-	const struct snd_kcontrol_new *controls, int num_controls)
+	const struct snd_kcontrol_new *controls, int num_controls,
+	const char *name_prefix)
 {
 	struct snd_card *card = codec->card->snd_card;
+	char prefixed_name[44], *name;
 	int err, i;
 
 	for (i = 0; i < num_controls; i++) {
 		const struct snd_kcontrol_new *control = &controls[i];
-		err = snd_ctl_add(card, snd_soc_cnew(control, codec, NULL));
+		if (name_prefix) {
+			snprintf(prefixed_name, sizeof(prefixed_name), "%s.%s",
+				 name_prefix, control->name);
+			name = prefixed_name;
+		} else {
+			name = control->name;
+		}
+		err = snd_ctl_add(card, snd_soc_cnew(control, codec, name));
 		if (err < 0) {
 			dev_err(codec->dev, "%s: Failed to add %s\n",
-				codec->name, control->name);
+				codec->name, name);
 			return err;
 		}
 	}
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 035cab8..19c754f 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -327,6 +327,7 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 	int i, ret = 0;
 	size_t name_len;
 	struct snd_soc_dapm_path *path;
+	char prefix[10];
 
 	/* add kcontrol */
 	for (i = 0; i < w->num_kcontrols; i++) {
@@ -347,6 +348,13 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 			name_len = strlen(w->kcontrols[i].name) + 1;
 			if (w->id != snd_soc_dapm_mixer_named_ctl)
 				name_len += 1 + strlen(w->name);
+			if (codec->kcontrol_prefix && !w->prefixed) {
+				name_len += 1 + strlen(codec->kcontrol_prefix);
+				snprintf(prefix, sizeof(prefix), "%s.",
+					 codec->kcontrol_prefix);
+			} else {
+				prefix[0] = '\0';
+			}
 
 			path->long_name = kmalloc(name_len, GFP_KERNEL);
 
@@ -355,12 +363,12 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 
 			switch (w->id) {
 			default:
-				snprintf(path->long_name, name_len, "%s %s",
-					 w->name, w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s %s",
+					 prefix, w->name, w->kcontrols[i].name);
 				break;
 			case snd_soc_dapm_mixer_named_ctl:
-				snprintf(path->long_name, name_len, "%s",
-					 w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s",
+					 prefix, w->kcontrols[i].name);
 				break;
 			}
 
@@ -388,6 +396,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 {
 	struct snd_soc_dapm_path *path = NULL;
 	struct snd_kcontrol *kcontrol;
+	char prefixed_name[44], *name;
 	int ret = 0;
 
 	if (!w->num_kcontrols) {
@@ -395,7 +404,14 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 		return -EINVAL;
 	}
 
-	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, w->name);
+	if (codec->kcontrol_prefix && !w->prefixed) {
+		snprintf(prefixed_name, sizeof(prefixed_name), "%s.%s",
+			 codec->kcontrol_prefix, w->name);
+		name = prefixed_name;
+	} else {
+		name = w->name;
+	}
+	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, name);
 	ret = snd_ctl_add(codec->card->snd_card, kcontrol);
 	if (ret < 0)
 		goto err;
@@ -406,7 +422,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 	return ret;
 
 err:
-	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", w->name);
+	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", name);
 	return ret;
 }
 
@@ -1253,6 +1269,7 @@ static void dapm_free_widgets(struct snd_soc_codec *codec)
 
 	list_for_each_entry_safe(w, next_w, &codec->dapm_widgets, list) {
 		list_del(&w->list);
+		kfree(w->name);
 		kfree(w);
 	}
 
@@ -1299,15 +1316,30 @@ int snd_soc_dapm_sync(struct snd_soc_codec *codec)
 EXPORT_SYMBOL_GPL(snd_soc_dapm_sync);
 
 static int snd_soc_dapm_add_route(struct snd_soc_codec *codec,
-				  const struct snd_soc_dapm_route *route)
+				  const struct snd_soc_dapm_route *route,
+				  const char *name_prefix)
 {
 	struct snd_soc_dapm_path *path;
 	struct snd_soc_dapm_widget *wsource = NULL, *wsink = NULL, *w;
-	const char *sink = route->sink;
+	const char *sink;
 	const char *control = route->control;
-	const char *source = route->source;
+	const char *source;
+	char prefixed_sink[80];
+	char prefixed_source[80];
 	int ret = 0;
 
+	if (name_prefix) {
+		snprintf(prefixed_sink, sizeof(prefixed_sink), "%s.%s",
+			 name_prefix, route->sink);
+		sink = prefixed_sink;
+		snprintf(prefixed_source, sizeof(prefixed_source), "%s.%s",
+			 name_prefix, route->source);
+		source = prefixed_source;
+	} else {
+		sink = route->sink;
+		source = route->source;
+	}
+
 	/* find src and dest widgets */
 	list_for_each_entry(w, &codec->dapm_widgets, list) {
 
@@ -1416,6 +1448,7 @@ err:
  * @codec: codec
  * @route: audio routes
  * @num: number of routes
+ * @name_prefix: prefix to route name or NULL
  *
  * Connects 2 dapm widgets together via a named audio path. The sink is
  * the widget receiving the audio signal, whilst the source is the sender
@@ -1425,12 +1458,13 @@ err:
  * with a call to snd_soc_card_free().
  */
 int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
-			    const struct snd_soc_dapm_route *route, int num)
+			    const struct snd_soc_dapm_route *route, int num,
+			    const char *name_prefix)
 {
 	int i, ret;
 
 	for (i = 0; i < num; i++) {
-		ret = snd_soc_dapm_add_route(codec, route);
+		ret = snd_soc_dapm_add_route(codec, route, name_prefix);
 		if (ret < 0) {
 			printk(KERN_ERR "Failed to add route %s->%s\n",
 			       route->source,
@@ -1933,13 +1967,31 @@ EXPORT_SYMBOL_GPL(snd_soc_dapm_put_pin_switch);
  * Returns 0 for success else error.
  */
 int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
-	const struct snd_soc_dapm_widget *widget)
+	const struct snd_soc_dapm_widget *widget, const char *name_prefix)
 {
 	struct snd_soc_dapm_widget *w;
+	size_t name_len;
+	struct snd_soc_dapm_path *path;
 
 	if ((w = dapm_cnew_widget(widget)) == NULL)
 		return -ENOMEM;
 
+	name_len = strlen(widget->name) + 1;
+	if (name_prefix)
+		name_len += 1 + strlen(name_prefix);
+	w->name = kmalloc(name_len, GFP_KERNEL);
+	if (w->name == NULL) {
+		kfree(w);
+		return -ENOMEM;
+	}
+	if (name_prefix) {
+		snprintf(w->name, name_len, "%s.%s",
+			 name_prefix, widget->name);
+		w->prefixed = 1;
+	} else {
+		snprintf(w->name, name_len, "%s", widget->name);
+	}
+
 	w->codec = codec;
 	INIT_LIST_HEAD(&w->sources);
 	INIT_LIST_HEAD(&w->sinks);
@@ -1948,6 +2000,7 @@ int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
 
 	/* machine layer set ups unconnected pins and insertions */
 	w->connected = 1;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_soc_dapm_new_control);
@@ -1957,6 +2010,7 @@ EXPORT_SYMBOL_GPL(snd_soc_dapm_new_control);
  * @codec: audio codec
  * @widget: widget array
  * @num: number of widgets
+ * @name_prefix: prefix to widget name or NULL
  *
  * Creates new DAPM controls based upon the templates.
  *
@@ -1964,12 +2018,12 @@ EXPORT_SYMBOL_GPL(snd_soc_dapm_new_control);
  */
 int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 	const struct snd_soc_dapm_widget *widget,
-	int num)
+	int num, const char *name_prefix)
 {
 	int i, ret;
 
 	for (i = 0; i < num; i++) {
-		ret = snd_soc_dapm_new_control(codec, widget);
+		ret = snd_soc_dapm_new_control(codec, widget, name_prefix);
 		if (ret < 0) {
 			printk(KERN_ERR
 			       "ASoC: Failed to create DAPM control %s: %d\n",

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Mark Brown]

On Thu, Aug 19, 2010 at 02:44:51PM +0300, Jarkko Nikula wrote:

> kcontrol or name prefix still comes from dai_link->kcontrol_prefix
> (where it should be per codec as pointer by Mark). And no drivers are
> patched for snd_soc_add_controls, snd_soc_dapm_new_control,
> snd_soc_dapm_new_controls and snd_soc_dapm_add_routes API changes.

It seems inelegant to have to bounce the prefix information through the
CODEC driver - we're already supplying the CODEC when we register the
controls so it seems like the core ought to be able to work out which
controls need to be renamed from the machine description without needing
this.

It seems best to have the data come from machine-specific config so that
we can allow them to provide something that makes things clearer to
users on the particular board.

> Codec:
> - kcontrol prefix
> - no widget name prefix (as they are per codec)
> - no audio map prefix (as they are per codec)

Are you sure these are per CODEC?

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Thu, 19 Aug 2010 14:54:14 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Thu, Aug 19, 2010 at 02:44:51PM +0300, Jarkko Nikula wrote:
> 
> > kcontrol or name prefix still comes from dai_link->kcontrol_prefix
> > (where it should be per codec as pointer by Mark). And no drivers are
> > patched for snd_soc_add_controls, snd_soc_dapm_new_control,
> > snd_soc_dapm_new_controls and snd_soc_dapm_add_routes API changes.
> 
> It seems inelegant to have to bounce the prefix information through the
> CODEC driver - we're already supplying the CODEC when we register the
> controls so it seems like the core ought to be able to work out which
> controls need to be renamed from the machine description without needing
> this.
> 
Yeah, I agree. It would be best if there is no need to change API of
those functions but I haven't figured out yet how those functions
can see should they add prefix or not and what prefix.

So what we do in soc_probe_dai_link:

	cpu_dai->driver->probe
	codec->driver->probe
	-> Codec adds controls, widgets and routes (only controls
	are prefixed. E.g. "front.")
	platform->driver->probe
	codec_dai->driver->probe
	dai_link->init
	-> Machine adds controls, widgets and routes (no prefixes)
	-> Machine registers stuff from extra drivers (all
	controls, widgets and routes are prefixed per driver.
	E.g. "front-left-amp.", "front-right-amp." )

Codec and machine registrations are easy to separate e.g. by some flag
and use only codec->kcontrol_prefix and continue using unmodified API.

I think extra drivers could use own variants of those registration
functions that have the name_prefix argument (and core would call them
too). Then we don't need to patch all the codec and machine drivers.
Does this sound feasible?

> It seems best to have the data come from machine-specific config so that
> we can allow them to provide something that makes things clearer to
> users on the particular board.
> 
Pointer to some codec_name<->prefix table in struct snd_soc_card at
least eliminates the dai_link->kcontrol_prefix.

> > Codec:
> > - kcontrol prefix
> > - no widget name prefix (as they are per codec)
> > - no audio map prefix (as they are per codec)
> 
> Are you sure these are per CODEC?

I thought they and audio map of machine (registered in
dai_link->init) were per codec. Read that as I haven't tried with
second map yet in the test board :-)

Do you think there are some issues e.g. with multi-dai codecs that we
need to address?


-- 
Jarkko

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Thu, 19 Aug 2010 18:20:49 +0300
Jarkko Nikula <jhnikula@gmail.com> wrote:

> 	cpu_dai->driver->probe
> 	codec->driver->probe
> 	-> Codec adds controls, widgets and routes (only controls
> 	are prefixed. E.g. "front.")
> 	platform->driver->probe
> 	codec_dai->driver->probe
> 	dai_link->init
> 	-> Machine adds controls, widgets and routes (no prefixes)
> 	-> Machine registers stuff from extra drivers (all
> 	controls, widgets and routes are prefixed per driver.
> 	E.g. "front-left-amp.", "front-right-amp." )
> 
> Codec and machine registrations are easy to separate e.g. by some flag
> and use only codec->kcontrol_prefix and continue using unmodified API.
> 
> I think extra drivers could use own variants of those registration
> functions that have the name_prefix argument (and core would call them
> too). Then we don't need to patch all the codec and machine drivers.
> Does this sound feasible?
> 
Ok, this was easy. I added functions variants that take the prefix and
that core calls also. So _snd_soc_add_controls,
_snd_soc_dapm_new_control, _snd_soc_dapm_new_controls and
_snd_soc_dapm_add_routes.

This way there is no need to patch all existing drivers and core can
prefix nicely codec kcontrols based on codec->probed flag. Then
external drivers can use those function variants by passing custom 
prefix that is different than codec->kcontrol_prefix.

No other changes from previous version. I.e. dai_link->kcontrol_prefix
hack is still here.


-- 
Jarkko

=============== WORK IN PROGRESS RFC ==========
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index c4a4456..44516de 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -314,8 +314,14 @@ int snd_soc_dapm_get_pin_switch(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *uncontrol);
 int snd_soc_dapm_put_pin_switch(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *uncontrol);
+int _snd_soc_dapm_new_control(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget,
+	const char *name_prefix);
 int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
 	const struct snd_soc_dapm_widget *widget);
+int _snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget,
+	int num, const char *name_prefix);
 int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 	const struct snd_soc_dapm_widget *widget,
 	int num);
@@ -323,6 +329,9 @@ int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 /* dapm path setup */
 int snd_soc_dapm_new_widgets(struct snd_soc_codec *codec);
 void snd_soc_dapm_free(struct snd_soc_codec *codec);
+int _snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
+			    const struct snd_soc_dapm_route *route, int num,
+			    const char *name_prefix);
 int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
 			    const struct snd_soc_dapm_route *route, int num);
 
@@ -412,6 +421,7 @@ struct snd_soc_dapm_path {
 struct snd_soc_dapm_widget {
 	enum snd_soc_dapm_type id;
 	char *name;		/* widget name */
+	bool prefixed;	/* set if widget name is prefixed */
 	char *sname;	/* stream name */
 	struct snd_soc_codec *codec;
 	struct list_head list;
diff --git a/include/sound/soc.h b/include/sound/soc.h
index d31e8b7..6ac9769 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -310,6 +310,9 @@ void snd_soc_free_ac97_codec(struct snd_soc_codec *codec);
  */
 struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template,
 	void *data, char *long_name);
+int _snd_soc_add_controls(struct snd_soc_codec *codec,
+	const struct snd_kcontrol_new *controls, int num_controls,
+	const char *name_prefix);
 int snd_soc_add_controls(struct snd_soc_codec *codec,
 	const struct snd_kcontrol_new *controls, int num_controls);
 int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol,
@@ -423,6 +426,7 @@ struct snd_soc_ops {
 /* SoC Audio Codec device */
 struct snd_soc_codec {
 	const char *name;
+	const char *kcontrol_prefix;
 	int id;
 	struct device *dev;
 	struct snd_soc_codec_driver *driver;
@@ -539,6 +543,7 @@ struct snd_soc_dai_link {
 	const char *platform_name;	/* for multi-platform */
 	const char *cpu_dai_name;
 	const char *codec_dai_name;
+	const char *kcontrol_prefix;	/* kcontrol prefix for multi-codec */
 
 	/* Keep DAI active over suspend */
 	unsigned int ignore_suspend:1;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 7093c17..b7e319d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1280,6 +1280,7 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num)
 	/* config components */
 	codec_dai->codec = codec;
 	codec->card = card;
+	codec->kcontrol_prefix = dai_link->kcontrol_prefix;
 	cpu_dai->platform = platform;
 	rtd->card = card;
 	rtd->dev.parent = card->dev;
@@ -1881,34 +1882,68 @@ struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template,
 EXPORT_SYMBOL_GPL(snd_soc_cnew);
 
 /**
- * snd_soc_add_controls - add an array of controls to a codec.
- * Convienience function to add a list of controls. Many codecs were
- * duplicating this code.
+ * _snd_soc_add_controls - add an array of controls to a codec.
+ * This varian of snd_soc_add_controls allow to specify custom name prefix to
+ * controls.
  *
  * @codec: codec to add controls to
  * @controls: array of controls to add
  * @num_controls: number of elements in the array
+ * @name_prefix: prefix to kcontrol name or NULL
  *
  * Return 0 for success, else error.
  */
-int snd_soc_add_controls(struct snd_soc_codec *codec,
-	const struct snd_kcontrol_new *controls, int num_controls)
+int _snd_soc_add_controls(struct snd_soc_codec *codec,
+	const struct snd_kcontrol_new *controls, int num_controls,
+	const char *name_prefix)
 {
 	struct snd_card *card = codec->card->snd_card;
+	char prefixed_name[44], *name;
 	int err, i;
 
 	for (i = 0; i < num_controls; i++) {
 		const struct snd_kcontrol_new *control = &controls[i];
-		err = snd_ctl_add(card, snd_soc_cnew(control, codec, NULL));
+		if (name_prefix) {
+			snprintf(prefixed_name, sizeof(prefixed_name), "%s.%s",
+				 name_prefix, control->name);
+			name = prefixed_name;
+		} else {
+			name = control->name;
+		}
+		err = snd_ctl_add(card, snd_soc_cnew(control, codec, name));
 		if (err < 0) {
 			dev_err(codec->dev, "%s: Failed to add %s\n",
-				codec->name, control->name);
+				codec->name, name);
 			return err;
 		}
 	}
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(_snd_soc_add_controls);
+
+/**
+ * snd_soc_add_controls - add an array of controls to a codec.
+ * Convienience function to add a list of controls. Many codecs were
+ * duplicating this code.
+ *
+ * @codec: codec to add controls to
+ * @controls: array of controls to add
+ * @num_controls: number of elements in the array
+ *
+ * Return 0 for success, else error.
+ */
+int snd_soc_add_controls(struct snd_soc_codec *codec,
+	const struct snd_kcontrol_new *controls, int num_controls)
+{
+	const char *prefix = NULL;
+
+	/* Only codec controls are prefixed */
+	if (!codec->probed)
+		prefix = codec->kcontrol_prefix;
+
+	return _snd_soc_add_controls(codec, controls, num_controls, prefix);
+}
 EXPORT_SYMBOL_GPL(snd_soc_add_controls);
 
 /**
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 035cab8..c2d9304 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -327,6 +327,7 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 	int i, ret = 0;
 	size_t name_len;
 	struct snd_soc_dapm_path *path;
+	char prefix[10];
 
 	/* add kcontrol */
 	for (i = 0; i < w->num_kcontrols; i++) {
@@ -347,6 +348,13 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 			name_len = strlen(w->kcontrols[i].name) + 1;
 			if (w->id != snd_soc_dapm_mixer_named_ctl)
 				name_len += 1 + strlen(w->name);
+			if (codec->kcontrol_prefix && !w->prefixed) {
+				name_len += 1 + strlen(codec->kcontrol_prefix);
+				snprintf(prefix, sizeof(prefix), "%s.",
+					 codec->kcontrol_prefix);
+			} else {
+				prefix[0] = '\0';
+			}
 
 			path->long_name = kmalloc(name_len, GFP_KERNEL);
 
@@ -355,12 +363,12 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 
 			switch (w->id) {
 			default:
-				snprintf(path->long_name, name_len, "%s %s",
-					 w->name, w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s %s",
+					 prefix, w->name, w->kcontrols[i].name);
 				break;
 			case snd_soc_dapm_mixer_named_ctl:
-				snprintf(path->long_name, name_len, "%s",
-					 w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s",
+					 prefix, w->kcontrols[i].name);
 				break;
 			}
 
@@ -388,6 +396,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 {
 	struct snd_soc_dapm_path *path = NULL;
 	struct snd_kcontrol *kcontrol;
+	char prefixed_name[44], *name;
 	int ret = 0;
 
 	if (!w->num_kcontrols) {
@@ -395,7 +404,14 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 		return -EINVAL;
 	}
 
-	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, w->name);
+	if (codec->kcontrol_prefix && !w->prefixed) {
+		snprintf(prefixed_name, sizeof(prefixed_name), "%s.%s",
+			 codec->kcontrol_prefix, w->name);
+		name = prefixed_name;
+	} else {
+		name = w->name;
+	}
+	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, name);
 	ret = snd_ctl_add(codec->card->snd_card, kcontrol);
 	if (ret < 0)
 		goto err;
@@ -406,7 +422,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 	return ret;
 
 err:
-	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", w->name);
+	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", name);
 	return ret;
 }
 
@@ -1253,6 +1269,7 @@ static void dapm_free_widgets(struct snd_soc_codec *codec)
 
 	list_for_each_entry_safe(w, next_w, &codec->dapm_widgets, list) {
 		list_del(&w->list);
+		kfree(w->name);
 		kfree(w);
 	}
 
@@ -1299,15 +1316,30 @@ int snd_soc_dapm_sync(struct snd_soc_codec *codec)
 EXPORT_SYMBOL_GPL(snd_soc_dapm_sync);
 
 static int snd_soc_dapm_add_route(struct snd_soc_codec *codec,
-				  const struct snd_soc_dapm_route *route)
+				  const struct snd_soc_dapm_route *route,
+				  const char *name_prefix)
 {
 	struct snd_soc_dapm_path *path;
 	struct snd_soc_dapm_widget *wsource = NULL, *wsink = NULL, *w;
-	const char *sink = route->sink;
+	const char *sink;
 	const char *control = route->control;
-	const char *source = route->source;
+	const char *source;
+	char prefixed_sink[80];
+	char prefixed_source[80];
 	int ret = 0;
 
+	if (name_prefix) {
+		snprintf(prefixed_sink, sizeof(prefixed_sink), "%s.%s",
+			 name_prefix, route->sink);
+		sink = prefixed_sink;
+		snprintf(prefixed_source, sizeof(prefixed_source), "%s.%s",
+			 name_prefix, route->source);
+		source = prefixed_source;
+	} else {
+		sink = route->sink;
+		source = route->source;
+	}
+
 	/* find src and dest widgets */
 	list_for_each_entry(w, &codec->dapm_widgets, list) {
 
@@ -1412,25 +1444,28 @@ err:
 }
 
 /**
- * snd_soc_dapm_add_routes - Add routes between DAPM widgets
+ * _snd_soc_dapm_add_routes - Add routes between DAPM widgets
  * @codec: codec
  * @route: audio routes
  * @num: number of routes
+ * @name_prefix: prefix to route name or NULL
  *
  * Connects 2 dapm widgets together via a named audio path. The sink is
  * the widget receiving the audio signal, whilst the source is the sender
- * of the audio signal.
+ * of the audio signal. This variant of snd_soc_dapm_add_routes allow to
+ * specify custom prefix to source and sink names of route.
  *
  * Returns 0 for success else error. On error all resources can be freed
  * with a call to snd_soc_card_free().
  */
-int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
-			    const struct snd_soc_dapm_route *route, int num)
+int _snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
+			    const struct snd_soc_dapm_route *route, int num,
+			    const char *name_prefix)
 {
 	int i, ret;
 
 	for (i = 0; i < num; i++) {
-		ret = snd_soc_dapm_add_route(codec, route);
+		ret = snd_soc_dapm_add_route(codec, route, name_prefix);
 		if (ret < 0) {
 			printk(KERN_ERR "Failed to add route %s->%s\n",
 			       route->source,
@@ -1442,6 +1477,26 @@ int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(_snd_soc_dapm_add_routes);
+
+/**
+ * snd_soc_dapm_add_routes - Add routes between DAPM widgets
+ * @codec: codec
+ * @route: audio routes
+ * @num: number of routes
+ *
+ * Connects 2 dapm widgets together via a named audio path. The sink is
+ * the widget receiving the audio signal, whilst the source is the sender
+ * of the audio signal.
+ *
+ * Returns 0 for success else error. On error all resources can be freed
+ * with a call to snd_soc_card_free().
+ */
+int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
+			    const struct snd_soc_dapm_route *route, int num)
+{
+	return _snd_soc_dapm_add_routes(codec, route, num, NULL);
+}
 EXPORT_SYMBOL_GPL(snd_soc_dapm_add_routes);
 
 /**
@@ -1924,22 +1979,41 @@ int snd_soc_dapm_put_pin_switch(struct snd_kcontrol *kcontrol,
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_pin_switch);
 
 /**
- * snd_soc_dapm_new_control - create new dapm control
+ * _snd_soc_dapm_new_control - create new dapm control
  * @codec: audio codec
  * @widget: widget template
  *
- * Creates a new dapm control based upon the template.
+ * Creates a new dapm control based upon the template. This variant of
+ * snd_soc_dapm_new_control allow to specify custom prefix to widget name.
  *
  * Returns 0 for success else error.
  */
-int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
-	const struct snd_soc_dapm_widget *widget)
+int _snd_soc_dapm_new_control(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget, const char *name_prefix)
 {
 	struct snd_soc_dapm_widget *w;
+	size_t name_len;
+	struct snd_soc_dapm_path *path;
 
 	if ((w = dapm_cnew_widget(widget)) == NULL)
 		return -ENOMEM;
 
+	name_len = strlen(widget->name) + 1;
+	if (name_prefix)
+		name_len += 1 + strlen(name_prefix);
+	w->name = kmalloc(name_len, GFP_KERNEL);
+	if (w->name == NULL) {
+		kfree(w);
+		return -ENOMEM;
+	}
+	if (name_prefix) {
+		snprintf(w->name, name_len, "%s.%s",
+			 name_prefix, widget->name);
+		w->prefixed = 1;
+	} else {
+		snprintf(w->name, name_len, "%s", widget->name);
+	}
+
 	w->codec = codec;
 	INIT_LIST_HEAD(&w->sources);
 	INIT_LIST_HEAD(&w->sinks);
@@ -1948,28 +2022,47 @@ int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
 
 	/* machine layer set ups unconnected pins and insertions */
 	w->connected = 1;
+
 	return 0;
 }
+EXPORT_SYMBOL_GPL(_snd_soc_dapm_new_control);
+
+/**
+ * snd_soc_dapm_new_control - create new dapm control
+ * @codec: audio codec
+ * @widget: widget template
+ *
+ * Creates a new dapm control based upon the template.
+ *
+ * Returns 0 for success else error.
+ */
+int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget)
+{
+	return _snd_soc_dapm_new_control(codec, widget, NULL);
+}
 EXPORT_SYMBOL_GPL(snd_soc_dapm_new_control);
 
 /**
- * snd_soc_dapm_new_controls - create new dapm controls
+ * _snd_soc_dapm_new_controls - create new dapm controls
  * @codec: audio codec
  * @widget: widget array
  * @num: number of widgets
+ * @name_prefix: prefix to widget name or NULL
  *
- * Creates new DAPM controls based upon the templates.
+ * Creates new DAPM controls based upon the templates. This variant of
+ * snd_soc_dapm_new_controls allow to specify custom prefix to widget names.
  *
  * Returns 0 for success else error.
  */
-int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
+int _snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 	const struct snd_soc_dapm_widget *widget,
-	int num)
+	int num, const char *name_prefix)
 {
 	int i, ret;
 
 	for (i = 0; i < num; i++) {
-		ret = snd_soc_dapm_new_control(codec, widget);
+		ret = _snd_soc_dapm_new_control(codec, widget, name_prefix);
 		if (ret < 0) {
 			printk(KERN_ERR
 			       "ASoC: Failed to create DAPM control %s: %d\n",
@@ -1980,8 +2073,25 @@ int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 	}
 	return 0;
 }
-EXPORT_SYMBOL_GPL(snd_soc_dapm_new_controls);
+EXPORT_SYMBOL_GPL(_snd_soc_dapm_new_controls);
 
+/**
+ * snd_soc_dapm_new_controls - create new dapm controls
+ * @codec: audio codec
+ * @widget: widget array
+ * @num: number of widgets
+ *
+ * Creates new DAPM controls based upon the templates.
+ *
+ * Returns 0 for success else error.
+ */
+int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget,
+	int num)
+{
+	return _snd_soc_dapm_new_controls(codec, widget, num, NULL);
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_new_controls);
 
 /**
  * snd_soc_dapm_stream_event - send a stream event to the dapm core

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Fri, 20 Aug 2010 11:51:44 +0300
Jarkko Nikula <jhnikula@gmail.com> wrote:

> On Thu, 19 Aug 2010 18:20:49 +0300
> Jarkko Nikula <jhnikula@gmail.com> wrote:
> 
> > 	cpu_dai->driver->probe
> > 	codec->driver->probe
> > 	-> Codec adds controls, widgets and routes (only controls
> > 	are prefixed. E.g. "front.")
> > 	platform->driver->probe
> > 	codec_dai->driver->probe
> > 	dai_link->init
> > 	-> Machine adds controls, widgets and routes (no prefixes)
> > 	-> Machine registers stuff from extra drivers (all
> > 	controls, widgets and routes are prefixed per driver.
> > 	E.g. "front-left-amp.", "front-right-amp." )
> > 
> > Codec and machine registrations are easy to separate e.g. by some flag
> > and use only codec->kcontrol_prefix and continue using unmodified API.
> > 
> > I think extra drivers could use own variants of those registration
> > functions that have the name_prefix argument (and core would call them
> > too). Then we don't need to patch all the codec and machine drivers.
> > Does this sound feasible?
> > 
> Ok, this was easy. I added functions variants that take the prefix and
> that core calls also. So _snd_soc_add_controls,
> _snd_soc_dapm_new_control, _snd_soc_dapm_new_controls and
> _snd_soc_dapm_add_routes.
> 
> This way there is no need to patch all existing drivers and core can
> prefix nicely codec kcontrols based on codec->probed flag. Then
> external drivers can use those function variants by passing custom 
> prefix that is different than codec->kcontrol_prefix.
> 
> No other changes from previous version. I.e. dai_link->kcontrol_prefix
> hack is still here.
> 
Here's the recent version.

I removed dai_link->kcontrol_prefix hack from this version, removed one
unused spuriously added variable and added pointer to optional name
prefix map into struct snd_soc_card that is used to associate prefix to
codec.

So if there's a machine with two bar-codecs, following code associated
prefix 'b' to kcontrols of second codec.

static struct snd_soc_prefix_map foo_codec_prefix[] = {
	{
		.codec_name = "bar-codec.2",
		.kcontrol_prefix = "b",
	},
};

static struct snd_soc_card foo_sound_card = {
	.name = "FOO,
	.dai_link = foo_dai,
	.num_links = ARRAY_SIZE(foo_dai),
	.prefix_map = foo_codec_prefix,
	.num_prefixes = ARRAY_SIZE(foo_codec_prefix),
};


-- 
Jarkko

================== RFC ===============
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index c4a4456..44516de 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -314,8 +314,14 @@ int snd_soc_dapm_get_pin_switch(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *uncontrol);
 int snd_soc_dapm_put_pin_switch(struct snd_kcontrol *kcontrol,
 	struct snd_ctl_elem_value *uncontrol);
+int _snd_soc_dapm_new_control(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget,
+	const char *name_prefix);
 int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
 	const struct snd_soc_dapm_widget *widget);
+int _snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget,
+	int num, const char *name_prefix);
 int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 	const struct snd_soc_dapm_widget *widget,
 	int num);
@@ -323,6 +329,9 @@ int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 /* dapm path setup */
 int snd_soc_dapm_new_widgets(struct snd_soc_codec *codec);
 void snd_soc_dapm_free(struct snd_soc_codec *codec);
+int _snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
+			    const struct snd_soc_dapm_route *route, int num,
+			    const char *name_prefix);
 int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
 			    const struct snd_soc_dapm_route *route, int num);
 
@@ -412,6 +421,7 @@ struct snd_soc_dapm_path {
 struct snd_soc_dapm_widget {
 	enum snd_soc_dapm_type id;
 	char *name;		/* widget name */
+	bool prefixed;	/* set if widget name is prefixed */
 	char *sname;	/* stream name */
 	struct snd_soc_codec *codec;
 	struct list_head list;
diff --git a/include/sound/soc.h b/include/sound/soc.h
index d31e8b7..06c13ec 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -310,6 +310,9 @@ void snd_soc_free_ac97_codec(struct snd_soc_codec *codec);
  */
 struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template,
 	void *data, char *long_name);
+int _snd_soc_add_controls(struct snd_soc_codec *codec,
+	const struct snd_kcontrol_new *controls, int num_controls,
+	const char *name_prefix);
 int snd_soc_add_controls(struct snd_soc_codec *codec,
 	const struct snd_kcontrol_new *controls, int num_controls);
 int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol,
@@ -423,6 +426,7 @@ struct snd_soc_ops {
 /* SoC Audio Codec device */
 struct snd_soc_codec {
 	const char *name;
+	const char *kcontrol_prefix;
 	int id;
 	struct device *dev;
 	struct snd_soc_codec_driver *driver;
@@ -553,6 +557,11 @@ struct snd_soc_dai_link {
 	struct snd_soc_ops *ops;
 };
 
+struct snd_soc_prefix_map {
+	const char *codec_name;
+	const char *kcontrol_prefix;
+};
+
 /* SoC card */
 struct snd_soc_card {
 	const char *name;
@@ -587,6 +596,10 @@ struct snd_soc_card {
 	struct snd_soc_pcm_runtime *rtd;
 	int num_rtd;
 
+	/* optional map of name prefixes that are associated per codec */
+	struct snd_soc_prefix_map *prefix_map;
+	int num_prefixes;
+
 	struct work_struct deferred_resume_work;
 
 	/* lists of probed devices belonging to this card */
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 65352c7..5162bbe 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1266,6 +1266,23 @@ static void soc_remove_dai_link(struct snd_soc_card *card, int num)
 
 static void rtd_release(struct device *dev) {}
 
+static void soc_set_name_prefix(struct snd_soc_card *card,
+				struct snd_soc_codec *codec)
+{
+	int i;
+
+	if (card->prefix_map == NULL)
+		return;
+
+	for (i = 0; i < card->num_prefixes; i++) {
+		struct snd_soc_prefix_map *map = &card->prefix_map[i];
+		if (map->codec_name && !strcmp(codec->name, map->codec_name)) {
+			codec->kcontrol_prefix = map->kcontrol_prefix;
+			break;
+		}
+	}
+}
+
 static int soc_probe_dai_link(struct snd_soc_card *card, int num)
 {
 	struct snd_soc_dai_link *dai_link = &card->dai_link[num];
@@ -1306,6 +1323,7 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num)
 
 	/* probe the CODEC */
 	if (!codec->probed) {
+		soc_set_name_prefix(card, codec);
 		if (codec->driver->probe) {
 			ret = codec->driver->probe(codec);
 			if (ret < 0) {
@@ -1881,34 +1899,68 @@ struct snd_kcontrol *snd_soc_cnew(const struct snd_kcontrol_new *_template,
 EXPORT_SYMBOL_GPL(snd_soc_cnew);
 
 /**
- * snd_soc_add_controls - add an array of controls to a codec.
- * Convienience function to add a list of controls. Many codecs were
- * duplicating this code.
+ * _snd_soc_add_controls - add an array of controls to a codec.
+ * This varian of snd_soc_add_controls allow to specify custom name prefix to
+ * controls.
  *
  * @codec: codec to add controls to
  * @controls: array of controls to add
  * @num_controls: number of elements in the array
+ * @name_prefix: prefix to kcontrol name or NULL
  *
  * Return 0 for success, else error.
  */
-int snd_soc_add_controls(struct snd_soc_codec *codec,
-	const struct snd_kcontrol_new *controls, int num_controls)
+int _snd_soc_add_controls(struct snd_soc_codec *codec,
+	const struct snd_kcontrol_new *controls, int num_controls,
+	const char *name_prefix)
 {
 	struct snd_card *card = codec->card->snd_card;
+	char prefixed_name[44], *name;
 	int err, i;
 
 	for (i = 0; i < num_controls; i++) {
 		const struct snd_kcontrol_new *control = &controls[i];
-		err = snd_ctl_add(card, snd_soc_cnew(control, codec, NULL));
+		if (name_prefix) {
+			snprintf(prefixed_name, sizeof(prefixed_name), "%s.%s",
+				 name_prefix, control->name);
+			name = prefixed_name;
+		} else {
+			name = control->name;
+		}
+		err = snd_ctl_add(card, snd_soc_cnew(control, codec, name));
 		if (err < 0) {
 			dev_err(codec->dev, "%s: Failed to add %s\n",
-				codec->name, control->name);
+				codec->name, name);
 			return err;
 		}
 	}
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(_snd_soc_add_controls);
+
+/**
+ * snd_soc_add_controls - add an array of controls to a codec.
+ * Convienience function to add a list of controls. Many codecs were
+ * duplicating this code.
+ *
+ * @codec: codec to add controls to
+ * @controls: array of controls to add
+ * @num_controls: number of elements in the array
+ *
+ * Return 0 for success, else error.
+ */
+int snd_soc_add_controls(struct snd_soc_codec *codec,
+	const struct snd_kcontrol_new *controls, int num_controls)
+{
+	const char *prefix = NULL;
+
+	/* Only codec controls are prefixed */
+	if (!codec->probed)
+		prefix = codec->kcontrol_prefix;
+
+	return _snd_soc_add_controls(codec, controls, num_controls, prefix);
+}
 EXPORT_SYMBOL_GPL(snd_soc_add_controls);
 
 /**
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 035cab8..9a419b2 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -327,6 +327,7 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 	int i, ret = 0;
 	size_t name_len;
 	struct snd_soc_dapm_path *path;
+	char prefix[10];
 
 	/* add kcontrol */
 	for (i = 0; i < w->num_kcontrols; i++) {
@@ -347,6 +348,13 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 			name_len = strlen(w->kcontrols[i].name) + 1;
 			if (w->id != snd_soc_dapm_mixer_named_ctl)
 				name_len += 1 + strlen(w->name);
+			if (codec->kcontrol_prefix && !w->prefixed) {
+				name_len += 1 + strlen(codec->kcontrol_prefix);
+				snprintf(prefix, sizeof(prefix), "%s.",
+					 codec->kcontrol_prefix);
+			} else {
+				prefix[0] = '\0';
+			}
 
 			path->long_name = kmalloc(name_len, GFP_KERNEL);
 
@@ -355,12 +363,12 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 
 			switch (w->id) {
 			default:
-				snprintf(path->long_name, name_len, "%s %s",
-					 w->name, w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s %s",
+					 prefix, w->name, w->kcontrols[i].name);
 				break;
 			case snd_soc_dapm_mixer_named_ctl:
-				snprintf(path->long_name, name_len, "%s",
-					 w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s",
+					 prefix, w->kcontrols[i].name);
 				break;
 			}
 
@@ -388,6 +396,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 {
 	struct snd_soc_dapm_path *path = NULL;
 	struct snd_kcontrol *kcontrol;
+	char prefixed_name[44], *name;
 	int ret = 0;
 
 	if (!w->num_kcontrols) {
@@ -395,7 +404,14 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 		return -EINVAL;
 	}
 
-	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, w->name);
+	if (codec->kcontrol_prefix && !w->prefixed) {
+		snprintf(prefixed_name, sizeof(prefixed_name), "%s.%s",
+			 codec->kcontrol_prefix, w->name);
+		name = prefixed_name;
+	} else {
+		name = w->name;
+	}
+	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, name);
 	ret = snd_ctl_add(codec->card->snd_card, kcontrol);
 	if (ret < 0)
 		goto err;
@@ -406,7 +422,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 	return ret;
 
 err:
-	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", w->name);
+	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", name);
 	return ret;
 }
 
@@ -1253,6 +1269,7 @@ static void dapm_free_widgets(struct snd_soc_codec *codec)
 
 	list_for_each_entry_safe(w, next_w, &codec->dapm_widgets, list) {
 		list_del(&w->list);
+		kfree(w->name);
 		kfree(w);
 	}
 
@@ -1299,15 +1316,30 @@ int snd_soc_dapm_sync(struct snd_soc_codec *codec)
 EXPORT_SYMBOL_GPL(snd_soc_dapm_sync);
 
 static int snd_soc_dapm_add_route(struct snd_soc_codec *codec,
-				  const struct snd_soc_dapm_route *route)
+				  const struct snd_soc_dapm_route *route,
+				  const char *name_prefix)
 {
 	struct snd_soc_dapm_path *path;
 	struct snd_soc_dapm_widget *wsource = NULL, *wsink = NULL, *w;
-	const char *sink = route->sink;
+	const char *sink;
 	const char *control = route->control;
-	const char *source = route->source;
+	const char *source;
+	char prefixed_sink[80];
+	char prefixed_source[80];
 	int ret = 0;
 
+	if (name_prefix) {
+		snprintf(prefixed_sink, sizeof(prefixed_sink), "%s.%s",
+			 name_prefix, route->sink);
+		sink = prefixed_sink;
+		snprintf(prefixed_source, sizeof(prefixed_source), "%s.%s",
+			 name_prefix, route->source);
+		source = prefixed_source;
+	} else {
+		sink = route->sink;
+		source = route->source;
+	}
+
 	/* find src and dest widgets */
 	list_for_each_entry(w, &codec->dapm_widgets, list) {
 
@@ -1412,25 +1444,28 @@ err:
 }
 
 /**
- * snd_soc_dapm_add_routes - Add routes between DAPM widgets
+ * _snd_soc_dapm_add_routes - Add routes between DAPM widgets
  * @codec: codec
  * @route: audio routes
  * @num: number of routes
+ * @name_prefix: prefix to route name or NULL
  *
  * Connects 2 dapm widgets together via a named audio path. The sink is
  * the widget receiving the audio signal, whilst the source is the sender
- * of the audio signal.
+ * of the audio signal. This variant of snd_soc_dapm_add_routes allow to
+ * specify custom prefix to source and sink names of route.
  *
  * Returns 0 for success else error. On error all resources can be freed
  * with a call to snd_soc_card_free().
  */
-int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
-			    const struct snd_soc_dapm_route *route, int num)
+int _snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
+			    const struct snd_soc_dapm_route *route, int num,
+			    const char *name_prefix)
 {
 	int i, ret;
 
 	for (i = 0; i < num; i++) {
-		ret = snd_soc_dapm_add_route(codec, route);
+		ret = snd_soc_dapm_add_route(codec, route, name_prefix);
 		if (ret < 0) {
 			printk(KERN_ERR "Failed to add route %s->%s\n",
 			       route->source,
@@ -1442,6 +1477,26 @@ int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(_snd_soc_dapm_add_routes);
+
+/**
+ * snd_soc_dapm_add_routes - Add routes between DAPM widgets
+ * @codec: codec
+ * @route: audio routes
+ * @num: number of routes
+ *
+ * Connects 2 dapm widgets together via a named audio path. The sink is
+ * the widget receiving the audio signal, whilst the source is the sender
+ * of the audio signal.
+ *
+ * Returns 0 for success else error. On error all resources can be freed
+ * with a call to snd_soc_card_free().
+ */
+int snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
+			    const struct snd_soc_dapm_route *route, int num)
+{
+	return _snd_soc_dapm_add_routes(codec, route, num, NULL);
+}
 EXPORT_SYMBOL_GPL(snd_soc_dapm_add_routes);
 
 /**
@@ -1924,22 +1979,40 @@ int snd_soc_dapm_put_pin_switch(struct snd_kcontrol *kcontrol,
 EXPORT_SYMBOL_GPL(snd_soc_dapm_put_pin_switch);
 
 /**
- * snd_soc_dapm_new_control - create new dapm control
+ * _snd_soc_dapm_new_control - create new dapm control
  * @codec: audio codec
  * @widget: widget template
  *
- * Creates a new dapm control based upon the template.
+ * Creates a new dapm control based upon the template. This variant of
+ * snd_soc_dapm_new_control allow to specify custom prefix to widget name.
  *
  * Returns 0 for success else error.
  */
-int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
-	const struct snd_soc_dapm_widget *widget)
+int _snd_soc_dapm_new_control(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget, const char *name_prefix)
 {
 	struct snd_soc_dapm_widget *w;
+	size_t name_len;
 
 	if ((w = dapm_cnew_widget(widget)) == NULL)
 		return -ENOMEM;
 
+	name_len = strlen(widget->name) + 1;
+	if (name_prefix)
+		name_len += 1 + strlen(name_prefix);
+	w->name = kmalloc(name_len, GFP_KERNEL);
+	if (w->name == NULL) {
+		kfree(w);
+		return -ENOMEM;
+	}
+	if (name_prefix) {
+		snprintf(w->name, name_len, "%s.%s",
+			 name_prefix, widget->name);
+		w->prefixed = 1;
+	} else {
+		snprintf(w->name, name_len, "%s", widget->name);
+	}
+
 	w->codec = codec;
 	INIT_LIST_HEAD(&w->sources);
 	INIT_LIST_HEAD(&w->sinks);
@@ -1948,28 +2021,47 @@ int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
 
 	/* machine layer set ups unconnected pins and insertions */
 	w->connected = 1;
+
 	return 0;
 }
+EXPORT_SYMBOL_GPL(_snd_soc_dapm_new_control);
+
+/**
+ * snd_soc_dapm_new_control - create new dapm control
+ * @codec: audio codec
+ * @widget: widget template
+ *
+ * Creates a new dapm control based upon the template.
+ *
+ * Returns 0 for success else error.
+ */
+int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget)
+{
+	return _snd_soc_dapm_new_control(codec, widget, NULL);
+}
 EXPORT_SYMBOL_GPL(snd_soc_dapm_new_control);
 
 /**
- * snd_soc_dapm_new_controls - create new dapm controls
+ * _snd_soc_dapm_new_controls - create new dapm controls
  * @codec: audio codec
  * @widget: widget array
  * @num: number of widgets
+ * @name_prefix: prefix to widget name or NULL
  *
- * Creates new DAPM controls based upon the templates.
+ * Creates new DAPM controls based upon the templates. This variant of
+ * snd_soc_dapm_new_controls allow to specify custom prefix to widget names.
  *
  * Returns 0 for success else error.
  */
-int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
+int _snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 	const struct snd_soc_dapm_widget *widget,
-	int num)
+	int num, const char *name_prefix)
 {
 	int i, ret;
 
 	for (i = 0; i < num; i++) {
-		ret = snd_soc_dapm_new_control(codec, widget);
+		ret = _snd_soc_dapm_new_control(codec, widget, name_prefix);
 		if (ret < 0) {
 			printk(KERN_ERR
 			       "ASoC: Failed to create DAPM control %s: %d\n",
@@ -1980,8 +2072,25 @@ int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
 	}
 	return 0;
 }
-EXPORT_SYMBOL_GPL(snd_soc_dapm_new_controls);
+EXPORT_SYMBOL_GPL(_snd_soc_dapm_new_controls);
 
+/**
+ * snd_soc_dapm_new_controls - create new dapm controls
+ * @codec: audio codec
+ * @widget: widget array
+ * @num: number of widgets
+ *
+ * Creates new DAPM controls based upon the templates.
+ *
+ * Returns 0 for success else error.
+ */
+int snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
+	const struct snd_soc_dapm_widget *widget,
+	int num)
+{
+	return _snd_soc_dapm_new_controls(codec, widget, num, NULL);
+}
+EXPORT_SYMBOL_GPL(snd_soc_dapm_new_controls);
 
 /**
  * snd_soc_dapm_stream_event - send a stream event to the dapm core

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Mark Brown]

On Fri, Aug 20, 2010 at 11:51:44AM +0300, Jarkko Nikula wrote:
> Jarkko Nikula <jhnikula@gmail.com> wrote:

> +int _snd_soc_dapm_new_control(struct snd_soc_codec *codec,
> +	const struct snd_soc_dapm_widget *widget,
> +	const char *name_prefix);
>  int snd_soc_dapm_new_control(struct snd_soc_codec *codec,
>  	const struct snd_soc_dapm_widget *widget);
> +int _snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
> +	const struct snd_soc_dapm_widget *widget,
> +	int num, const char *name_prefix);

Best not in the header; these are not things individual drivers should
be worrying their pretty little heads about.  If they should be used by
individual drivers then we need better names than just _.

> @@ -539,6 +543,7 @@ struct snd_soc_dai_link {
>  	const char *platform_name;	/* for multi-platform */
>  	const char *cpu_dai_name;
>  	const char *codec_dai_name;
> +	const char *kcontrol_prefix;	/* kcontrol prefix for multi-codec */
>  
>  	/* Keep DAI active over suspend */
>  	unsigned int ignore_suspend:1;

I don't see how a DAI link can ever be used to configure prefix names -
there's just not any real association between DAI links and controls,
and as soon as you hit mixing any that does exist gets lost.  Probably a
table of CODEC to prefix mappings would be better.

> +			if (codec->kcontrol_prefix && !w->prefixed) {
> +				name_len += 1 + strlen(codec->kcontrol_prefix);
> +				snprintf(prefix, sizeof(prefix), "%s.",
> +					 codec->kcontrol_prefix);

A space would probably be more idiomatic for the separator.

> +int _snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
> +			    const struct snd_soc_dapm_route *route, int num,
> +			    const char *name_prefix)
>  {
>  	int i, ret;
>  
>  	for (i = 0; i < num; i++) {
> -		ret = snd_soc_dapm_add_route(codec, route);
> +		ret = snd_soc_dapm_add_route(codec, route, name_prefix);

This one is a bit more fun.  For this to work properly we need to
consider what happens with the cross-device links in the DAI maps which
means we need to able to cope with separate prefixes for the source and
the sink.

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Mon, 23 Aug 2010 16:21:45 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > +int _snd_soc_dapm_new_controls(struct snd_soc_codec *codec,
> > +	const struct snd_soc_dapm_widget *widget,
> > +	int num, const char *name_prefix);
> 
> Best not in the header; these are not things individual drivers should
> be worrying their pretty little heads about.  If they should be used by
> individual drivers then we need better names than just _.
> 
Yeah, I picked up _ prefix so that name indicates that these functions
are more like for internal use and drivers should use them only
exceptionally. What I'm thinking if we can rid of them completely.

>From your commit 26b01cc it looks like there's a work in progress to
support DAI-less codecs/amplifiers. If that would be possible then
there is no need to register controls from other drivers in machine DAI
init. Well, CPU DAI controls are possible but they don't need a prefix
I think.

> > @@ -539,6 +543,7 @@ struct snd_soc_dai_link {
> >  	const char *platform_name;	/* for multi-platform */
> >  	const char *cpu_dai_name;
> >  	const char *codec_dai_name;
> > +	const char *kcontrol_prefix;	/* kcontrol prefix for multi-codec */
> >  
> >  	/* Keep DAI active over suspend */
> >  	unsigned int ignore_suspend:1;
> 
> I don't see how a DAI link can ever be used to configure prefix names -
> there's just not any real association between DAI links and controls,
> and as soon as you hit mixing any that does exist gets lost.  Probably a
> table of CODEC to prefix mappings would be better.
> 
Sorry, I didn't emphasis this well enough that this hack was
temporary just after your comment to first version and it got finally
removed in yesterday's version :-)

> > +			if (codec->kcontrol_prefix && !w->prefixed) {
> > +				name_len += 1 + strlen(codec->kcontrol_prefix);
> > +				snprintf(prefix, sizeof(prefix), "%s.",
> > +					 codec->kcontrol_prefix);
> 
> A space would probably be more idiomatic for the separator.
> 
Ok, makes sense.

> > +int _snd_soc_dapm_add_routes(struct snd_soc_codec *codec,
> > +			    const struct snd_soc_dapm_route *route, int num,
> > +			    const char *name_prefix)
> >  {
> >  	int i, ret;
> >  
> >  	for (i = 0; i < num; i++) {
> > -		ret = snd_soc_dapm_add_route(codec, route);
> > +		ret = snd_soc_dapm_add_route(codec, route, name_prefix);
> 
> This one is a bit more fun.  For this to work properly we need to
> consider what happens with the cross-device links in the DAI maps which
> means we need to able to cope with separate prefixes for the source and
> the sink.

Prefixing is not problem I think since we can specify them in
machine's audio map (like two mono amplifiers registered to 1st codec
are prefixed below) but how to link DAPMs of two codec together?

static const struct snd_soc_dapm_route audio_map1[] = {
	{"Left AMP input", NULL, "LLOUT"},
	{"Right AMP input, NULL, "RLOUT"},
	{"Speaker", NULL, "Left AMP output"},
	{"Speaker, NULL, "Right AMP output"},

	{"Codec B Left input"}, NULL, "LLOUT"},
	{"Codec B Right input"}, NULL, "RLOUT"},
};

static const struct snd_soc_dapm_route audio_map2[] = {
	{"foo bar", NULL, "Codec B Left input"},
	{"foo bar", NULL, "Codec B Right input"},
};


-- 
Jarkko

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Mark Brown]

On Tue, Aug 24, 2010 at 10:23:43AM +0300, Jarkko Nikula wrote:
> Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> > Best not in the header; these are not things individual drivers should
> > be worrying their pretty little heads about.  If they should be used by
> > individual drivers then we need better names than just _.

> Yeah, I picked up _ prefix so that name indicates that these functions
> are more like for internal use and drivers should use them only
> exceptionally. What I'm thinking if we can rid of them completely.

I figured.

> From your commit 26b01cc it looks like there's a work in progress to
> support DAI-less codecs/amplifiers. If that would be possible then
> there is no need to register controls from other drivers in machine DAI

DAIless devices should work already.

> init. Well, CPU DAI controls are possible but they don't need a prefix
> I think.

There might be an issue disambiguating against collisions with other
drivers in the system, I guess.

> > I don't see how a DAI link can ever be used to configure prefix names -
> > there's just not any real association between DAI links and controls,
> > and as soon as you hit mixing any that does exist gets lost.  Probably a
> > table of CODEC to prefix mappings would be better.

> Sorry, I didn't emphasis this well enough that this hack was
> temporary just after your comment to first version and it got finally
> removed in yesterday's version :-)

Yeah, I was writing my reply as you sent that.

> > This one is a bit more fun.  For this to work properly we need to
> > consider what happens with the cross-device links in the DAI maps which
> > means we need to able to cope with separate prefixes for the source and
> > the sink.

> Prefixing is not problem I think since we can specify them in
> machine's audio map (like two mono amplifiers registered to 1st codec
> are prefixed below) but how to link DAPMs of two codec together?

We'll be fine just using the prefixed name in the machine drivers I
think and not advertising the prefix-adding route add function.

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Tue, 24 Aug 2010 11:10:32 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> DAIless devices should work already.
...
> We'll be fine just using the prefixed name in the machine drivers I
> think and not advertising the prefix-adding route add function.
>
I went back to original idea that prefixes only kcontrols of codec and
doesn't add any new API. So if we can have DAIless codec drivers (i.e.
amplifiers) then there is no immediate need for prefixing widgets and
routes.

Cross-device links and DAPM between them remains unsolved as this tries
to solve only kcontrol name issue with multiple codecs only.

-- 
Jarkko
=============== CUT HERE ================
>From 5b4db56e01fc7c19011dfa83d11b1e6d55681dd5 Mon Sep 17 00:00:00 2001
From: Jarkko Nikula <jhnikula@gmail.com>
Date: Wed, 25 Aug 2010 12:59:01 +0300
Subject: [PATCH] ASoC: Add optional prefix_map to snd_soc_card for prefixing codec kcontrols

This optional prefix_map allows to specify unique ALSA control name prefixes
for maching codec names. This makes possible to have a sound card
configuration using multiple codecs without a name collision that would
occur if a codec driver tries to register a kcontrol with an existing name.

Now it is possible to specify for instance "Front" and "Rear" prefixes and
a sound card can have two separate controls like "Front PCM Playback Volume"
and "Rear PCM Playback Volume". Those controls will then show as
"Front PCM" and "Rear PCM" in ALSA mixer application.

Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
---
 include/sound/soc.h  |   10 ++++++++++
 sound/soc/soc-core.c |   35 +++++++++++++++++++++++++++++++++--
 sound/soc/soc-dapm.c |   28 ++++++++++++++++++++++------
 3 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index d31e8b7..cc309af 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -423,6 +423,7 @@ struct snd_soc_ops {
 /* SoC Audio Codec device */
 struct snd_soc_codec {
 	const char *name;
+	const char *kcontrol_prefix;
 	int id;
 	struct device *dev;
 	struct snd_soc_codec_driver *driver;
@@ -553,6 +554,11 @@ struct snd_soc_dai_link {
 	struct snd_soc_ops *ops;
 };
 
+struct snd_soc_prefix_map {
+	const char *codec_name;
+	const char *kcontrol_prefix;
+};
+
 /* SoC card */
 struct snd_soc_card {
 	const char *name;
@@ -587,6 +593,10 @@ struct snd_soc_card {
 	struct snd_soc_pcm_runtime *rtd;
 	int num_rtd;
 
+	/* optional map of name prefixes that are associated per codec */
+	struct snd_soc_prefix_map *prefix_map;
+	int num_prefixes;
+
 	struct work_struct deferred_resume_work;
 
 	/* lists of probed devices belonging to this card */
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 65352c7..b17596f 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1266,6 +1266,23 @@ static void soc_remove_dai_link(struct snd_soc_card *card, int num)
 
 static void rtd_release(struct device *dev) {}
 
+static void soc_set_name_prefix(struct snd_soc_card *card,
+				struct snd_soc_codec *codec)
+{
+	int i;
+
+	if (card->prefix_map == NULL)
+		return;
+
+	for (i = 0; i < card->num_prefixes; i++) {
+		struct snd_soc_prefix_map *map = &card->prefix_map[i];
+		if (map->codec_name && !strcmp(codec->name, map->codec_name)) {
+			codec->kcontrol_prefix = map->kcontrol_prefix;
+			break;
+		}
+	}
+}
+
 static int soc_probe_dai_link(struct snd_soc_card *card, int num)
 {
 	struct snd_soc_dai_link *dai_link = &card->dai_link[num];
@@ -1306,6 +1323,7 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num)
 
 	/* probe the CODEC */
 	if (!codec->probed) {
+		soc_set_name_prefix(card, codec);
 		if (codec->driver->probe) {
 			ret = codec->driver->probe(codec);
 			if (ret < 0) {
@@ -1895,14 +1913,27 @@ int snd_soc_add_controls(struct snd_soc_codec *codec,
 	const struct snd_kcontrol_new *controls, int num_controls)
 {
 	struct snd_card *card = codec->card->snd_card;
+	char prefixed_name[44], *name;
+	const char *prefix = NULL;
 	int err, i;
 
+	/* Only codec controls are prefixed */
+	if (!codec->probed)
+		prefix = codec->kcontrol_prefix;
+
 	for (i = 0; i < num_controls; i++) {
 		const struct snd_kcontrol_new *control = &controls[i];
-		err = snd_ctl_add(card, snd_soc_cnew(control, codec, NULL));
+		if (prefix) {
+			snprintf(prefixed_name, sizeof(prefixed_name), "%s %s",
+				 prefix, control->name);
+			name = prefixed_name;
+		} else {
+			name = control->name;
+		}
+		err = snd_ctl_add(card, snd_soc_cnew(control, codec, name));
 		if (err < 0) {
 			dev_err(codec->dev, "%s: Failed to add %s\n",
-				codec->name, control->name);
+				codec->name, name);
 			return err;
 		}
 	}
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 035cab8..412f0bf 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -327,6 +327,7 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 	int i, ret = 0;
 	size_t name_len;
 	struct snd_soc_dapm_path *path;
+	char prefix[10];
 
 	/* add kcontrol */
 	for (i = 0; i < w->num_kcontrols; i++) {
@@ -347,6 +348,13 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 			name_len = strlen(w->kcontrols[i].name) + 1;
 			if (w->id != snd_soc_dapm_mixer_named_ctl)
 				name_len += 1 + strlen(w->name);
+			if (codec->kcontrol_prefix) {
+				name_len += 1 + strlen(codec->kcontrol_prefix);
+				snprintf(prefix, sizeof(prefix), "%s ",
+					 codec->kcontrol_prefix);
+			} else {
+				prefix[0] = '\0';
+			}
 
 			path->long_name = kmalloc(name_len, GFP_KERNEL);
 
@@ -355,12 +363,12 @@ static int dapm_new_mixer(struct snd_soc_codec *codec,
 
 			switch (w->id) {
 			default:
-				snprintf(path->long_name, name_len, "%s %s",
-					 w->name, w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s %s",
+					 prefix, w->name, w->kcontrols[i].name);
 				break;
 			case snd_soc_dapm_mixer_named_ctl:
-				snprintf(path->long_name, name_len, "%s",
-					 w->kcontrols[i].name);
+				snprintf(path->long_name, name_len, "%s%s",
+					 prefix, w->kcontrols[i].name);
 				break;
 			}
 
@@ -388,6 +396,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 {
 	struct snd_soc_dapm_path *path = NULL;
 	struct snd_kcontrol *kcontrol;
+	char prefixed_name[44], *name;
 	int ret = 0;
 
 	if (!w->num_kcontrols) {
@@ -395,7 +404,14 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 		return -EINVAL;
 	}
 
-	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, w->name);
+	if (codec->kcontrol_prefix) {
+		snprintf(prefixed_name, sizeof(prefixed_name), "%s %s",
+			 codec->kcontrol_prefix, w->name);
+		name = prefixed_name;
+	} else {
+		name = w->name;
+	}
+	kcontrol = snd_soc_cnew(&w->kcontrols[0], w, name);
 	ret = snd_ctl_add(codec->card->snd_card, kcontrol);
 	if (ret < 0)
 		goto err;
@@ -406,7 +422,7 @@ static int dapm_new_mux(struct snd_soc_codec *codec,
 	return ret;
 
 err:
-	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", w->name);
+	printk(KERN_ERR "asoc: failed to add kcontrol %s\n", name);
 	return ret;
 }
 
-- 
1.7.1

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Mark Brown]

On Wed, Aug 25, 2010 at 01:59:22PM +0300, Jarkko Nikula wrote:

> I went back to original idea that prefixes only kcontrols of codec and
> doesn't add any new API. So if we can have DAIless codec drivers (i.e.
> amplifiers) then there is no immediate need for prefixing widgets and
> routes.

Note that even DAIless drivers can have routing in them - see WM9090 for
example.

> Cross-device links and DAPM between them remains unsolved as this tries
> to solve only kcontrol name issue with multiple codecs only.

I really think anything along these lines needs to handle DAPM somehow -
given that virtually every driver has DAPM support of some kind and
there are some fairly standard pin names for things it's likely that
you'll get the same issues there.

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Thu, 26 Aug 2010 14:32:29 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Wed, Aug 25, 2010 at 01:59:22PM +0300, Jarkko Nikula wrote:
> 
> > I went back to original idea that prefixes only kcontrols of codec and
> > doesn't add any new API. So if we can have DAIless codec drivers (i.e.
> > amplifiers) then there is no immediate need for prefixing widgets and
> > routes.
> 
> Note that even DAIless drivers can have routing in them - see WM9090 for
> example.
> 
I don't see it are there any problems. For me it looks the routes in
WM9090 are unique and registered to own codec instance so there should
not be route prefixing needed.

How these amplifier drivers are actually meant to be probed? Currently
struct snd_soc_codec_driver->probe is called only from
soc_probe_dai_link.


-- 
Jarkko

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Mark Brown]

On Mon, Aug 30, 2010 at 02:17:29PM +0300, Jarkko Nikula wrote:

> I don't see it are there any problems. For me it looks the routes in
> WM9090 are unique and registered to own codec instance so there should
> not be route prefixing needed.

Meh, right.  DAPM isn't coping with things that cross CODECs really.
This will need to be looked at.

> How these amplifier drivers are actually meant to be probed? Currently
> struct snd_soc_codec_driver->probe is called only from
> soc_probe_dai_link.

We need to set up a list of anciliary devices which are registered
without DAIs for this.

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Thu, 2 Sep 2010 15:25:18 +0100
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Mon, Aug 30, 2010 at 02:17:29PM +0300, Jarkko Nikula wrote:
> 
> > I don't see it are there any problems. For me it looks the routes in
> > WM9090 are unique and registered to own codec instance so there should
> > not be route prefixing needed.
> 
> Meh, right.  DAPM isn't coping with things that cross CODECs really.
> This will need to be looked at.
> 
> > How these amplifier drivers are actually meant to be probed? Currently
> > struct snd_soc_codec_driver->probe is called only from
> > soc_probe_dai_link.
> 
> We need to set up a list of anciliary devices which are registered
> without DAIs for this.

So we need to split these into three separate problems:

- Prefixing
- DAPM linking between codecs
- Registering DAIless codecs in machine drivers

What you think: is it better to hold my prefixing patch until DAPM
linking is solved or can it be applied before? I mean if we need to go
some global, not codec based DAPM, then there is need to prefix but not
if DAPM remains per codec.


-- 
Jarkko

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Mark Brown]

On Fri, Sep 03, 2010 at 10:55:32AM +0300, Jarkko Nikula wrote:

> So we need to split these into three separate problems:

> - Prefixing
> - DAPM linking between codecs
> - Registering DAIless codecs in machine drivers

> What you think: is it better to hold my prefixing patch until DAPM
> linking is solved or can it be applied before? I mean if we need to go
> some global, not codec based DAPM, then there is need to prefix but not
> if DAPM remains per codec.

Well, the DAPM map needs to be global in some sense since it needs to
flow between CODECs and we need board widgets which are definitely not
fixed on an individual chip.  

I think I'd like a firm idea of how the cross device DAPM works before
we do anything with the prefixes since there's some overlap there (if
only due to the fact that half the controls come from DAPM), even if the
cross device DAPM doesn't actually work yet.  I think your current code
is probably going to work well with DAPM but I need to have a bit of a
think about that over the weekend.

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Liam Girdwood]

On Fri, 2010-09-03 at 10:33 +0100, Mark Brown wrote:
> On Fri, Sep 03, 2010 at 10:55:32AM +0300, Jarkko Nikula wrote:
> 
> > So we need to split these into three separate problems:
> 
> > - Prefixing
> > - DAPM linking between codecs
> > - Registering DAIless codecs in machine drivers
> 
> > What you think: is it better to hold my prefixing patch until DAPM
> > linking is solved or can it be applied before? I mean if we need to go
> > some global, not codec based DAPM, then there is need to prefix but not
> > if DAPM remains per codec.
> 
> Well, the DAPM map needs to be global in some sense since it needs to
> flow between CODECs and we need board widgets which are definitely not
> fixed on an individual chip.  
> 
> I think I'd like a firm idea of how the cross device DAPM works before
> we do anything with the prefixes since there's some overlap there (if
> only due to the fact that half the controls come from DAPM), even if the
> cross device DAPM doesn't actually work yet.  I think your current code
> is probably going to work well with DAPM but I need to have a bit of a
> think about that over the weekend.

I've made some initial progress here with DAPM by de-coupling it from
CODEC drivers. So I now have a struct dapm_context * in the codec,
platform and machine drivers. I just now need to get some time to
experiment with the best way to hook them all up.

It's currently in my L24.9 branch, although the patches are in need of
clean up and squashing so are not ready for upstream yet.

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [RFC] ASoC: multi-component: Add optional kcontrol prefix name for a DAI link

[Jarkko Nikula]

On Fri, 03 Sep 2010 11:00:57 +0100
Liam Girdwood <lrg@slimlogic.co.uk> wrote:

> > I think I'd like a firm idea of how the cross device DAPM works before
> > we do anything with the prefixes since there's some overlap there (if
> > only due to the fact that half the controls come from DAPM), even if the
> > cross device DAPM doesn't actually work yet.  I think your current code
> > is probably going to work well with DAPM but I need to have a bit of a
> > think about that over the weekend.
> 
Ok, sounds good. Doing DAPM first avoids that there is no then need to
go back and forth with prefixing.

> I've made some initial progress here with DAPM by de-coupling it from
> CODEC drivers. So I now have a struct dapm_context * in the codec,
> platform and machine drivers. I just now need to get some time to
> experiment with the best way to hook them all up.
> 
> It's currently in my L24.9 branch, although the patches are in need of
> clean up and squashing so are not ready for upstream yet.
> 
Thanks for info. Have to give a try to it :-)


-- 
Jarkko