All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] topology: Enhance support for private data
@ 2016-07-15 12:17 mengdong.lin
  2016-07-15 12:17 ` [PATCH v2 1/5] topology: An element can refer to multipe data sections in text conf file mengdong.lin
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: mengdong.lin @ 2016-07-15 12:17 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Mengdong Lin, tiwai, mengdong.lin, o-takashi, liam.r.girdwood,
	shreyas.nc

From: Mengdong Lin <mengdong.lin@linux.intel.com>

There is no ABI change in this series.

We finished verification on latest Intel platforms. There will be 4 or 5
series. Later series will come with some ABI update for topology. The size
check on ABI objects in current ASoC kernel will detect possible ABI
version mismatch between user space and kernel

History:
v2: fix compiler warnings. There are 2 warnings left in topology/pcm.c
    that are not caused by this series and will be fixed in later patches.

Mengdong Lin (5):
  topology: An element can refer to multipe data sections in text conf
    file
  topology: Merge lookup for data reference into tplg_copy_data()
  topology: Change uuid value to 16 separate characters in text conf
    file
  topology: Parse vendor private data for manifest
  topology: Tuple type can have an extenstion

 include/topology.h        |  69 ++++++++++-
 src/topology/ctl.c        |  52 +++-----
 src/topology/dapm.c       |  17 ++-
 src/topology/data.c       | 307 ++++++++++++++++++++++++++++++++++++++--------
 src/topology/elem.c       |   4 +
 src/topology/parser.c     |  28 ++++-
 src/topology/tplg_local.h |  15 ++-
 7 files changed, 395 insertions(+), 97 deletions(-)

-- 
2.5.0

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

* [PATCH v2 1/5] topology: An element can refer to multipe data sections in text conf file
  2016-07-15 12:17 [PATCH v2 0/5] topology: Enhance support for private data mengdong.lin
@ 2016-07-15 12:17 ` mengdong.lin
  2016-07-15 12:18 ` [PATCH v2 2/5] topology: Merge lookup for data reference into tplg_copy_data() mengdong.lin
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: mengdong.lin @ 2016-07-15 12:17 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Mengdong Lin, tiwai, mengdong.lin, o-takashi, liam.r.girdwood,
	shreyas.nc

From: Mengdong Lin <mengdong.lin@linux.intel.com>

Previously in text conf file, an element can only refer to one data
section (SectionData). Now it can also refer to a list of data sections.
Thus users can split groups of firmware parameters to multiple data
sections, and put them all in the reference list.

Finally, data of these data sections will be merged, in the same order as
they are in the reference list, as the element's private data for kernel.

We still support the original syntax of reference to a single data
section. The doc is updated for the syntax extension.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/include/topology.h b/include/topology.h
index 9d57ce3..d666505 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -213,6 +213,34 @@ extern "C" {
  * The keyword tuples is to define vendor specific tuples. Please refer to
  * section Vendor Tokens and Vendor tuples.
  *
+ * <h5>How to define an element with private data</h5>
+ * An element can refer to a single data section or multiple data
+ * sections.
+ *
+ * <h6>To refer to a single data section:</h6>
+ * <pre>
+ * Sectionxxx."element name" {
+ *    ...
+ *	data "name of data section"		# optional private data
+ * }
+ * </pre>
+ *
+ * <h6>To refer to multiple data sections:</h6>
+ * <pre>
+ * Sectionxxx."element name" {
+ *	...
+ *	data [						# optional private data
+ *		"name of 1st data section"
+ *		"name of 2nd data section"
+ *		...
+ *	]
+ * }
+ * </pre>
+ * And data of these sections will be merged in the same order as they are
+ * in the list, as the element's private data for kernel.
+ *
+ * </pre>
+ *
  *  <h6>Vendor Tokens</h6>
  * A vendor token list is defined as a new section. Each token element is
  * a pair of string ID and integer value. And both the ID and value are
diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index b948ac0..7ded0a4 100644
--- a/src/topology/ctl.c
+++ b/src/topology/ctl.c
@@ -447,11 +447,9 @@ int tplg_parse_control_bytes(snd_tplg_t *tplg,
 		}
 
 		if (strcmp(id, "data") == 0) {
-			if (snd_config_get_string(n, &val) < 0)
-				return -EINVAL;
-
-			tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val);
-			tplg_dbg("\t%s: %s\n", id, val);
+			err = tplg_parse_data_refs(n, elem);
+			if (err < 0)
+				return err;
 			continue;
 		}
 
@@ -587,11 +585,9 @@ int tplg_parse_control_enum(snd_tplg_t *tplg, snd_config_t *cfg,
 		}
 
 		if (strcmp(id, "data") == 0) {
-			if (snd_config_get_string(n, &val) < 0)
-				return -EINVAL;
-
-			tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val);
-			tplg_dbg("\t%s: %s\n", id, val);
+			err = tplg_parse_data_refs(n, elem);
+			if (err < 0)
+				return err;
 			continue;
 		}
 
@@ -725,11 +721,9 @@ int tplg_parse_control_mixer(snd_tplg_t *tplg,
 		}
 
 		if (strcmp(id, "data") == 0) {
-			if (snd_config_get_string(n, &val) < 0)
-				return -EINVAL;
-
-			tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val);
-			tplg_dbg("\t%s: %s\n", id, val);
+			err = tplg_parse_data_refs(n, elem);
+			if (err < 0)
+				return err;
 			continue;
 		}
 
diff --git a/src/topology/dapm.c b/src/topology/dapm.c
index 278d605..d8eb10c 100644
--- a/src/topology/dapm.c
+++ b/src/topology/dapm.c
@@ -598,11 +598,9 @@ int tplg_parse_dapm_widget(snd_tplg_t *tplg,
 		}
 
 		if (strcmp(id, "data") == 0) {
-			if (snd_config_get_string(n, &val) < 0)
-				return -EINVAL;
-
-			tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val);
-			tplg_dbg("\t%s: %s\n", id, val);
+			err = tplg_parse_data_refs(n, elem);
+			if (err < 0)
+				return err;
 			continue;
 		}
 	}
diff --git a/src/topology/data.c b/src/topology/data.c
index c0a098c..0c5469a 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -20,6 +20,36 @@
 #include "tplg_local.h"
 #include <ctype.h>
 
+/* Get private data buffer of an element */
+struct snd_soc_tplg_private *get_priv_data(struct tplg_elem *elem)
+{
+	struct snd_soc_tplg_private *priv = NULL;
+
+	switch (elem->type) {
+	case SND_TPLG_TYPE_MIXER:
+		priv = &elem->mixer_ctrl->priv;
+		break;
+
+	case SND_TPLG_TYPE_ENUM:
+		priv = &elem->enum_ctrl->priv;
+		break;
+
+	case SND_TPLG_TYPE_BYTES:
+		priv = &elem->bytes_ext->priv;
+		break;
+
+	case SND_TPLG_TYPE_DAPM_WIDGET:
+		priv = &elem->widget->priv;
+		break;
+
+	default:
+		SNDERR("error: '%s': no support for private data for type %d\n",
+			elem->id, elem->type);
+	}
+
+	return priv;
+}
+
 /* Get Private data from a file. */
 static int tplg_parse_data_file(snd_config_t *cfg, struct tplg_elem *elem)
 {
@@ -614,6 +644,48 @@ static int parse_tuple_sets(snd_tplg_t *tplg, snd_config_t *cfg,
 	return 0;
 }
 
+/* Parse private data references for the element, either a single data section
+ * or a list of data sections.
+ */
+int tplg_parse_data_refs(snd_config_t *cfg,
+	struct tplg_elem *elem)
+{
+	snd_config_type_t  type;
+	snd_config_iterator_t i, next;
+	snd_config_t *n;
+	const char *val = NULL;
+
+	type = snd_config_get_type(cfg);
+
+	/* refer to a single data section */
+	if (type == SND_CONFIG_TYPE_STRING) {
+		if (snd_config_get_string(cfg, &val) < 0)
+			return -EINVAL;
+
+		tplg_dbg("\tdata: %s\n", val);
+		return tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val);
+	}
+
+	if (type != SND_CONFIG_TYPE_COMPOUND) {
+		SNDERR("error: compound type expected for %s", elem->id);
+		return -EINVAL;
+	}
+
+	/* refer to a list of data sections */
+	snd_config_for_each(i, next, cfg) {
+		const char *val;
+
+		n = snd_config_iterator_entry(i);
+		if (snd_config_get_string(n, &val) < 0)
+			continue;
+
+		tplg_dbg("\tdata: %s\n", val);
+		tplg_ref_add(elem, SND_TPLG_TYPE_DATA, val);
+	}
+
+	return 0;
+}
+
 /* Parse vendor tokens
  */
 int tplg_parse_tokens(snd_tplg_t *tplg, snd_config_t *cfg,
@@ -817,11 +889,15 @@ int tplg_parse_data(snd_tplg_t *tplg, snd_config_t *cfg,
 	return err;
 }
 
-/* copy private data into the bytes extended control */
+/* Merge data from a referenced data element to the parent element's
+ * private data buffer.
+ * An element can refer to multiple data sections. Data of these sections
+ * will be merged in the their reference order.
+ */
 int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref)
 {
-	struct snd_soc_tplg_private *priv;
-	int priv_data_size;
+	struct snd_soc_tplg_private *priv, *old_priv;
+	int priv_data_size, old_priv_data_size;
 	void *obj;
 
 	if (!ref)
@@ -831,6 +907,11 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref)
 	if (!ref->data || !ref->data->size) /* overlook empty private data */
 		return 0;
 
+	old_priv = get_priv_data(elem);
+	if (!old_priv)
+		return -EINVAL;
+	old_priv_data_size = old_priv->size;
+
 	priv_data_size = ref->data->size;
 	obj = realloc(elem->obj,
 			elem->size + priv_data_size);
@@ -838,33 +919,16 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref)
 		return -ENOMEM;
 	elem->obj = obj;
 
-	switch (elem->type) {
-	case SND_TPLG_TYPE_MIXER:
-		priv = &elem->mixer_ctrl->priv;
-		break;
-
-	case SND_TPLG_TYPE_ENUM:
-		priv = &elem->enum_ctrl->priv;
-		break;
-
-	case SND_TPLG_TYPE_BYTES:
-		priv = &elem->bytes_ext->priv;
-		break;
-
-	case SND_TPLG_TYPE_DAPM_WIDGET:
-		priv = &elem->widget->priv;
-		break;
-
-	default:
-		SNDERR("error: elem '%s': type %d private data not supported \n",
-			elem->id, elem->type);
+	priv = get_priv_data(elem);
+	if (!priv)
 		return -EINVAL;
-	}
 
+	/* merge the new data block */
 	elem->size += priv_data_size;
-	priv->size = priv_data_size;
+	priv->size = priv_data_size + old_priv_data_size;
 	ref->compound_elem = 1;
-	memcpy(priv->data, ref->data->data, priv_data_size);
+	memcpy(priv->data + old_priv_data_size,
+	       ref->data->data, priv_data_size);
 	return 0;
 }
 
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 9239aef..4daa540 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -229,6 +229,7 @@ int tplg_build_routes(snd_tplg_t *tplg);
 int tplg_build_pcm_dai(snd_tplg_t *tplg, unsigned int type);
 
 int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref);
+int tplg_parse_data_refs(snd_config_t *cfg, struct tplg_elem *elem);
 
 int tplg_ref_add(struct tplg_elem *elem, int type, const char* id);
 int tplg_ref_add_elem(struct tplg_elem *elem, struct tplg_elem *elem_ref);
-- 
2.5.0

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

* [PATCH v2 2/5] topology: Merge lookup for data reference into tplg_copy_data()
  2016-07-15 12:17 [PATCH v2 0/5] topology: Enhance support for private data mengdong.lin
  2016-07-15 12:17 ` [PATCH v2 1/5] topology: An element can refer to multipe data sections in text conf file mengdong.lin
@ 2016-07-15 12:18 ` mengdong.lin
  2016-07-15 12:18 ` [PATCH v2 3/5] topology: Change uuid value to 16 separate characters in text conf file mengdong.lin
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: mengdong.lin @ 2016-07-15 12:18 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Mengdong Lin, tiwai, mengdong.lin, o-takashi, liam.r.girdwood,
	shreyas.nc

From: Mengdong Lin <mengdong.lin@linux.intel.com>

Code refactor to reduce function calls. Now tplg_copy_data() can look up
a referenced data element and merge its data.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/topology/ctl.c b/src/topology/ctl.c
index 7ded0a4..592dded 100644
--- a/src/topology/ctl.c
+++ b/src/topology/ctl.c
@@ -140,9 +140,9 @@ static int tplg_build_mixer_control(snd_tplg_t *tplg,
 				 err = copy_tlv(elem, ref->elem);
 
 		} else if (ref->type == SND_TPLG_TYPE_DATA) {
-			ref->elem = tplg_elem_lookup(&tplg->pdata_list,
-						ref->id, SND_TPLG_TYPE_DATA);
-			 err = tplg_copy_data(elem, ref->elem);
+			err = tplg_copy_data(tplg, elem, ref);
+			if (err < 0)
+				return err;
 		}
 
 		if (!ref->elem) {
@@ -188,9 +188,9 @@ static int tplg_build_enum_control(snd_tplg_t *tplg,
 				copy_enum_texts(elem, ref->elem);
 
 		} else if (ref->type == SND_TPLG_TYPE_DATA) {
-			ref->elem = tplg_elem_lookup(&tplg->pdata_list,
-						ref->id, SND_TPLG_TYPE_DATA);
-			err = tplg_copy_data(elem, ref->elem);
+			err = tplg_copy_data(tplg, elem, ref);
+			if (err < 0)
+				return err;
 		}
 		if (!ref->elem) {
 			SNDERR("error: cannot find '%s' referenced by"
@@ -208,6 +208,7 @@ static int tplg_build_bytes_control(snd_tplg_t *tplg, struct tplg_elem *elem)
 {
 	struct tplg_ref *ref;
 	struct list_head *base, *pos;
+	int err;
 
 	base = &elem->ref_list;
 
@@ -217,18 +218,11 @@ static int tplg_build_bytes_control(snd_tplg_t *tplg, struct tplg_elem *elem)
 		if (ref->id == NULL || ref->elem)
 			continue;
 
-		/* bytes control only reference one private data section */
-		ref->elem = tplg_elem_lookup(&tplg->pdata_list,
-			ref->id, SND_TPLG_TYPE_DATA);
-		if (!ref->elem) {
-			SNDERR("error: cannot find data '%s'"
-				" referenced by control '%s'\n",
-				ref->id, elem->id);
-			return -EINVAL;
+		 if (ref->type == SND_TPLG_TYPE_DATA) {
+			err = tplg_copy_data(tplg, elem, ref);
+			if (err < 0)
+				return err;
 		}
-
-		/* copy texts to enum elem */
-		return tplg_copy_data(elem, ref->elem);
 	}
 
 	return 0;
diff --git a/src/topology/dapm.c b/src/topology/dapm.c
index d8eb10c..4d343b2 100644
--- a/src/topology/dapm.c
+++ b/src/topology/dapm.c
@@ -191,12 +191,11 @@ static int tplg_build_widget(snd_tplg_t *tplg,
 			break;
 
 		case SND_TPLG_TYPE_DATA:
-			if (!ref->elem)
-				ref->elem = tplg_elem_lookup(&tplg->pdata_list,
-						ref->id, SND_TPLG_TYPE_DATA);
-			if (ref->elem)
-				err = tplg_copy_data(elem, ref->elem);
+			err = tplg_copy_data(tplg, elem, ref);
+			if (err < 0)
+				return err;
 			break;
+
 		default:
 			break;
 		}
diff --git a/src/topology/data.c b/src/topology/data.c
index 0c5469a..e60114e 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -889,22 +889,30 @@ int tplg_parse_data(snd_tplg_t *tplg, snd_config_t *cfg,
 	return err;
 }
 
-/* Merge data from a referenced data element to the parent element's
- * private data buffer.
+/* Find a referenced data element and copy its data to the parent
+ * element's private data buffer.
  * An element can refer to multiple data sections. Data of these sections
  * will be merged in the their reference order.
  */
-int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref)
+int tplg_copy_data(snd_tplg_t *tplg, struct tplg_elem *elem,
+		   struct tplg_ref *ref)
 {
+	struct tplg_elem *ref_elem;
 	struct snd_soc_tplg_private *priv, *old_priv;
 	int priv_data_size, old_priv_data_size;
 	void *obj;
 
-	if (!ref)
+	ref_elem = tplg_elem_lookup(&tplg->pdata_list,
+				     ref->id, SND_TPLG_TYPE_DATA);
+	if (!ref_elem) {
+		SNDERR("error: cannot find data '%s' referenced by"
+		" element '%s'\n", ref->id, elem->id);
 		return -EINVAL;
+	}
 
 	tplg_dbg("Data '%s' used by '%s'\n", ref->id, elem->id);
-	if (!ref->data || !ref->data->size) /* overlook empty private data */
+	/* overlook empty private data */
+	if (!ref_elem->data || !ref_elem->data->size)
 		return 0;
 
 	old_priv = get_priv_data(elem);
@@ -912,7 +920,7 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref)
 		return -EINVAL;
 	old_priv_data_size = old_priv->size;
 
-	priv_data_size = ref->data->size;
+	priv_data_size = ref_elem->data->size;
 	obj = realloc(elem->obj,
 			elem->size + priv_data_size);
 	if (!obj)
@@ -926,9 +934,9 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref)
 	/* merge the new data block */
 	elem->size += priv_data_size;
 	priv->size = priv_data_size + old_priv_data_size;
-	ref->compound_elem = 1;
+	ref_elem->compound_elem = 1;
 	memcpy(priv->data + old_priv_data_size,
-	       ref->data->data, priv_data_size);
+	       ref_elem->data->data, priv_data_size);
 	return 0;
 }
 
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 4daa540..518b81e 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -228,7 +228,9 @@ int tplg_build_widgets(snd_tplg_t *tplg);
 int tplg_build_routes(snd_tplg_t *tplg);
 int tplg_build_pcm_dai(snd_tplg_t *tplg, unsigned int type);
 
-int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref);
+int tplg_copy_data(snd_tplg_t *tplg, struct tplg_elem *elem,
+		   struct tplg_ref *ref);
+
 int tplg_parse_data_refs(snd_config_t *cfg, struct tplg_elem *elem);
 
 int tplg_ref_add(struct tplg_elem *elem, int type, const char* id);
-- 
2.5.0

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

* [PATCH v2 3/5] topology: Change uuid value to 16 separate characters in text conf file
  2016-07-15 12:17 [PATCH v2 0/5] topology: Enhance support for private data mengdong.lin
  2016-07-15 12:17 ` [PATCH v2 1/5] topology: An element can refer to multipe data sections in text conf file mengdong.lin
  2016-07-15 12:18 ` [PATCH v2 2/5] topology: Merge lookup for data reference into tplg_copy_data() mengdong.lin
@ 2016-07-15 12:18 ` mengdong.lin
  2016-07-15 12:19 ` [PATCH v2 4/5] topology: Parse vendor private data for manifest mengdong.lin
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: mengdong.lin @ 2016-07-15 12:18 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Mengdong Lin, tiwai, mengdong.lin, o-takashi, liam.r.girdwood,
	shreyas.nc

From: Mengdong Lin <mengdong.lin@linux.intel.com>

Previously in text conf file, the uuid value of vendor tuples is a
16-characer string. Now change it to 16 characters separated by commas,
easier for users to edit it manually.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/include/topology.h b/include/topology.h
index d666505..89bed6c 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -273,8 +273,8 @@ extern "C" {
  *		...
  *	}
  *
- *	tuples."uuid" {
- *		VENDOR_TOKEN_ID2 "16 character uuid"
+ *	tuples."uuid" {			# 16 characters separated by commas
+ *		VENDOR_TOKEN_ID2 "0x01,0x02,...,0x0f"
  *		...
  *	}
  *
diff --git a/src/topology/data.c b/src/topology/data.c
index e60114e..a0c5ea2 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -176,6 +176,49 @@ static int get_hex_num(const char *str)
 	return values;
 }
 
+/* get uuid from a string made by 16 characters separated by commas */
+static int get_uuid(const char *str, unsigned char *uuid_le)
+{
+	unsigned long int  val;
+	char *tmp, *s = NULL;
+	int values = 0, ret = 0;
+
+	tmp = strdup(str);
+	if (tmp == NULL)
+		return -ENOMEM;
+
+	s = strtok(tmp, ",");
+
+	while (s != NULL) {
+		errno = 0;
+		val = strtoul(s, NULL, 0);
+		if ((errno == ERANGE && val == ULONG_MAX)
+			|| (errno != 0 && val == 0)
+			|| (val > UCHAR_MAX)) {
+			SNDERR("error: invalid value for uuid\n");
+			ret = -EINVAL;
+			goto out;
+		}
+
+		 *(uuid_le + values) = (unsigned char)val;
+
+		values++;
+		if (values >= 16)
+			break;
+
+		s = strtok(NULL, ",");
+	}
+
+	if (values < 16) {
+		SNDERR("error: less than 16 integers for uuid\n");
+		ret = -EINVAL;
+	}
+
+out:
+	free(tmp);
+	return ret;
+}
+
 static int write_hex(char *buf, char *str, int width)
 {
 	long val;
@@ -474,7 +517,7 @@ static int build_tuples(snd_tplg_t *tplg, struct tplg_elem *elem)
 	return 0;
 }
 
-static int parse_tuple_set(snd_tplg_t *tplg, snd_config_t *cfg,
+static int parse_tuple_set(snd_config_t *cfg,
 	struct tplg_tuple_set **s)
 {
 	snd_config_iterator_t i, next;
@@ -484,7 +527,6 @@ static int parse_tuple_set(snd_tplg_t *tplg, snd_config_t *cfg,
 	unsigned int type, num_tuples = 0;
 	struct tplg_tuple *tuple;
 	unsigned long int tuple_val;
-	int len;
 
 	snd_config_get_id(cfg, &id);
 
@@ -535,14 +577,8 @@ static int parse_tuple_set(snd_tplg_t *tplg, snd_config_t *cfg,
 
 		switch (type) {
 		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
-			len = strlen(value);
-			if (len > 16 || len == 0) {
-				SNDERR("error: tuple %s: invalid uuid\n", id);
+			if (get_uuid(value, tuple->uuid) < 0)
 				goto err;
-			}
-
-			memcpy(tuple->uuid, value, len);
-			tplg_dbg("\t\t%s = %s\n", tuple->token, tuple->uuid);
 			break;
 
 		case SND_SOC_TPLG_TUPLE_TYPE_STRING:
@@ -598,7 +634,7 @@ err:
 	return -EINVAL;
 }
 
-static int parse_tuple_sets(snd_tplg_t *tplg, snd_config_t *cfg,
+static int parse_tuple_sets(snd_config_t *cfg,
 	struct tplg_vendor_tuples *tuples)
 {
 	snd_config_iterator_t i, next;
@@ -632,7 +668,7 @@ static int parse_tuple_sets(snd_tplg_t *tplg, snd_config_t *cfg,
 			return -EINVAL;
 		}
 
-		err = parse_tuple_set(tplg, n, &tuples->set[tuples->num_sets]);
+		err = parse_tuple_set(n, &tuples->set[tuples->num_sets]);
 		if (err < 0)
 			return err;
 
@@ -774,7 +810,7 @@ int tplg_parse_tuples(snd_tplg_t *tplg, snd_config_t *cfg,
 		}
 
 		if (strcmp(id, "tuples") == 0) {
-			err = parse_tuple_sets(tplg, n, tuples);
+			err = parse_tuple_sets(n, tuples);
 			if (err < 0)
 				return err;
 		}
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 518b81e..48db813 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -104,7 +104,7 @@ struct tplg_tuple {
 	char token[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
 	union {
 		char string[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
-		char uuid[16];
+		unsigned char uuid[16];
 		unsigned int value;
 	};
 };
-- 
2.5.0

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

* [PATCH v2 4/5] topology: Parse vendor private data for manifest
  2016-07-15 12:17 [PATCH v2 0/5] topology: Enhance support for private data mengdong.lin
                   ` (2 preceding siblings ...)
  2016-07-15 12:18 ` [PATCH v2 3/5] topology: Change uuid value to 16 separate characters in text conf file mengdong.lin
@ 2016-07-15 12:19 ` mengdong.lin
  2016-07-15 12:19 ` [PATCH v2 5/5] topology: Tuple type can have an extenstion mengdong.lin
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: mengdong.lin @ 2016-07-15 12:19 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Mengdong Lin, tiwai, mengdong.lin, o-takashi, liam.r.girdwood,
	shreyas.nc

From: Mengdong Lin <mengdong.lin@linux.intel.com>

In text conf file, user can define a manifest section and let it refer
to private data sections, in the same syntax as other element types.

The text conf file can have at most 1 manifest section.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/include/topology.h b/include/topology.h
index 89bed6c..644e548 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -586,6 +586,20 @@ extern "C" {
  * }
  * </pre>
  *
+ * <h4>Manifest Private Data</h4>
+ * Manfiest may have private data. Users need to define a manifest section
+ * and add the references to 1 or multiple data sections. Please refer to
+ * section 'How to define an element with private data'. <br>
+ * And the text conf file can have at most 1 manifest section. <br><br>
+ *
+ * Manifest section is defined as follows :-
+ *
+ * <pre>
+ * SectionManifest"name" {
+ *
+ *	data "name"			# optional private data
+ * }
+ * </pre>
  */
 
 /** Maximum number of channels supported in one control */
diff --git a/src/topology/data.c b/src/topology/data.c
index a0c5ea2..245a834 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -26,6 +26,10 @@ struct snd_soc_tplg_private *get_priv_data(struct tplg_elem *elem)
 	struct snd_soc_tplg_private *priv = NULL;
 
 	switch (elem->type) {
+	case SND_TPLG_TYPE_MANIFEST:
+		priv = &elem->manifest->priv;
+		break;
+
 	case SND_TPLG_TYPE_MIXER:
 		priv = &elem->mixer_ctrl->priv;
 		break;
@@ -834,6 +838,103 @@ void tplg_free_tuples(void *obj)
 	free(tuples->set);
 }
 
+/* Parse manifest's data references
+ */
+int tplg_parse_manifest_data(snd_tplg_t *tplg, snd_config_t *cfg,
+	void *private ATTRIBUTE_UNUSED)
+{
+	struct snd_soc_tplg_manifest *manifest;
+	struct tplg_elem *elem;
+	snd_config_iterator_t i, next;
+	snd_config_t *n;
+	const char *id;
+	int err;
+
+	if (!list_empty(&tplg->manifest_list)) {
+		SNDERR("error: already has manifest data\n");
+		return -EINVAL;
+	}
+
+	elem = tplg_elem_new_common(tplg, cfg, NULL, SND_TPLG_TYPE_MANIFEST);
+	if (!elem)
+		return -ENOMEM;
+
+	manifest = elem->manifest;
+	manifest->size = elem->size;
+
+	tplg_dbg(" Manifest: %s\n", elem->id);
+
+	snd_config_for_each(i, next, cfg) {
+		n = snd_config_iterator_entry(i);
+		if (snd_config_get_id(n, &id) < 0)
+			continue;
+
+		/* skip comments */
+		if (strcmp(id, "comment") == 0)
+			continue;
+		if (id[0] == '#')
+			continue;
+
+
+		if (strcmp(id, "data") == 0) {
+			err = tplg_parse_data_refs(n, elem);
+			if (err < 0)
+				return err;
+			continue;
+		}
+	}
+
+	return 0;
+}
+
+/* merge private data of manifest */
+int tplg_build_manifest_data(snd_tplg_t *tplg)
+{
+	struct list_head *base, *pos;
+	struct tplg_elem *elem = NULL;
+	struct tplg_ref *ref;
+	struct snd_soc_tplg_manifest *manifest;
+	int err = 0;
+
+	base = &tplg->manifest_list;
+	list_for_each(pos, base) {
+
+		elem = list_entry(pos, struct tplg_elem, list);
+		break;
+	}
+
+	if (!elem) /* no manifest data */
+		return 0;
+
+	base = &elem->ref_list;
+
+	/* for each ref in this manifest elem */
+	list_for_each(pos, base) {
+
+		ref = list_entry(pos, struct tplg_ref, list);
+		if (ref->id == NULL || ref->elem)
+			continue;
+
+		if (ref->type == SND_TPLG_TYPE_DATA) {
+			err = tplg_copy_data(tplg, elem, ref);
+			if (err < 0)
+				return err;
+		}
+	}
+
+	manifest = elem->manifest;
+	if (!manifest->priv.size) /* no manifest data */
+		return 0;
+
+	tplg->manifest_pdata = malloc(manifest->priv.size);
+	if (!tplg->manifest_pdata)
+		return -ENOMEM;
+
+	tplg->manifest.priv.size = manifest->priv.size;
+	memcpy(tplg->manifest_pdata, manifest->priv.data, manifest->priv.size);
+	return 0;
+}
+
 /* Parse Private data.
  *
  * Object private data can either be from file or defined as bytes, shorts,
diff --git a/src/topology/elem.c b/src/topology/elem.c
index 50414f0..029c9ab 100644
--- a/src/topology/elem.c
+++ b/src/topology/elem.c
@@ -150,6 +150,10 @@ struct tplg_elem* tplg_elem_new_common(snd_tplg_t *tplg,
 	case SND_TPLG_TYPE_DATA:
 		list_add_tail(&elem->list, &tplg->pdata_list);
 		break;
+	case SND_TPLG_TYPE_MANIFEST:
+		list_add_tail(&elem->list, &tplg->manifest_list);
+		obj_size = sizeof(struct snd_soc_tplg_manifest);
+		break;
 	case SND_TPLG_TYPE_TEXT:
 		list_add_tail(&elem->list, &tplg->text_list);
 		break;
diff --git a/src/topology/parser.c b/src/topology/parser.c
index f6fc944..3ab64f4 100644
--- a/src/topology/parser.c
+++ b/src/topology/parser.c
@@ -189,6 +189,15 @@ static int tplg_parse_config(snd_tplg_t *tplg, snd_config_t *cfg)
 			continue;
 		}
 
+		if (strcmp(id, "SectionManifest") == 0) {
+			err = tplg_parse_compound(tplg, n,
+						  tplg_parse_manifest_data,
+				NULL);
+			if (err < 0)
+				return err;
+			continue;
+		}
+
 		SNDERR("error: unknown section %s\n", id);
 	}
 	return 0;
@@ -246,6 +255,10 @@ static int tplg_build_integ(snd_tplg_t *tplg)
 	if (err <  0)
 		return err;
 
+	err = tplg_build_manifest_data(tplg);
+	if (err <  0)
+		return err;
+
 	err = tplg_build_controls(tplg);
 	if (err <  0)
 		return err;
@@ -374,8 +387,16 @@ out:
 
 int snd_tplg_set_manifest_data(snd_tplg_t *tplg, const void *data, int len)
 {
+	if (len <= 0)
+		return 0;
+
 	tplg->manifest.priv.size = len;
-	tplg->manifest_pdata = data;
+
+	tplg->manifest_pdata = malloc(len);
+	if (!tplg->manifest_pdata)
+		return -ENOMEM;
+
+	memcpy(tplg->manifest_pdata, data, len);
 	return 0;
 }
 
@@ -423,6 +444,7 @@ snd_tplg_t *snd_tplg_new(void)
 	INIT_LIST_HEAD(&tplg->cc_list);
 	INIT_LIST_HEAD(&tplg->route_list);
 	INIT_LIST_HEAD(&tplg->pdata_list);
+	INIT_LIST_HEAD(&tplg->manifest_list);
 	INIT_LIST_HEAD(&tplg->text_list);
 	INIT_LIST_HEAD(&tplg->pcm_config_list);
 	INIT_LIST_HEAD(&tplg->pcm_caps_list);
@@ -437,6 +459,9 @@ snd_tplg_t *snd_tplg_new(void)
 
 void snd_tplg_free(snd_tplg_t *tplg)
 {
+	if (tplg->manifest_pdata)
+		free(tplg->manifest_pdata);
+
 	tplg_elem_free_list(&tplg->tlv_list);
 	tplg_elem_free_list(&tplg->widget_list);
 	tplg_elem_free_list(&tplg->pcm_list);
@@ -444,6 +469,7 @@ void snd_tplg_free(snd_tplg_t *tplg)
 	tplg_elem_free_list(&tplg->cc_list);
 	tplg_elem_free_list(&tplg->route_list);
 	tplg_elem_free_list(&tplg->pdata_list);
+	tplg_elem_free_list(&tplg->manifest_list);
 	tplg_elem_free_list(&tplg->text_list);
 	tplg_elem_free_list(&tplg->pcm_config_list);
 	tplg_elem_free_list(&tplg->pcm_caps_list);
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 48db813..4d79aa7 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -58,7 +58,7 @@ struct snd_tplg {
 
 	/* manifest */
 	struct snd_soc_tplg_manifest manifest;
-	const void *manifest_pdata;	/* copied by builder at file write */
+	void *manifest_pdata;	/* copied by builder at file write */
 
 	/* list of each element type */
 	struct list_head tlv_list;
@@ -71,6 +71,7 @@ struct snd_tplg {
 	struct list_head pdata_list;
 	struct list_head token_list;
 	struct list_head tuple_list;
+	struct list_head manifest_list;
 	struct list_head pcm_config_list;
 	struct list_head pcm_caps_list;
 
@@ -154,6 +155,7 @@ struct tplg_elem {
 		struct snd_soc_tplg_private *data;
 		struct tplg_vendor_tokens *tokens;
 		struct tplg_vendor_tuples *tuples;
+		struct snd_soc_tplg_manifest *manifest;
 	};
 
 	/* an element may refer to other elements:
@@ -195,6 +197,9 @@ int tplg_parse_tuples(snd_tplg_t *tplg, snd_config_t *cfg,
 
 void tplg_free_tuples(void *obj);
 
+int tplg_parse_manifest_data(snd_tplg_t *tplg, snd_config_t *cfg,
+	void *private ATTRIBUTE_UNUSED);
+
 int tplg_parse_control_bytes(snd_tplg_t *tplg,
 	snd_config_t *cfg, void *private ATTRIBUTE_UNUSED);
 
@@ -223,6 +228,7 @@ int tplg_parse_cc(snd_tplg_t *tplg,
 	snd_config_t *cfg, void *private ATTRIBUTE_UNUSED);
 
 int tplg_build_data(snd_tplg_t *tplg);
+int tplg_build_manifest_data(snd_tplg_t *tplg);
 int tplg_build_controls(snd_tplg_t *tplg);
 int tplg_build_widgets(snd_tplg_t *tplg);
 int tplg_build_routes(snd_tplg_t *tplg);
-- 
2.5.0

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

* [PATCH v2 5/5] topology: Tuple type can have an extenstion
  2016-07-15 12:17 [PATCH v2 0/5] topology: Enhance support for private data mengdong.lin
                   ` (3 preceding siblings ...)
  2016-07-15 12:19 ` [PATCH v2 4/5] topology: Parse vendor private data for manifest mengdong.lin
@ 2016-07-15 12:19 ` mengdong.lin
  2016-07-16 12:58 ` [PATCH v2 0/5] topology: Enhance support for private data Takashi Sakamoto
  2016-07-17  8:03 ` Takashi Iwai
  6 siblings, 0 replies; 14+ messages in thread
From: mengdong.lin @ 2016-07-15 12:19 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: Mengdong Lin, tiwai, mengdong.lin, o-takashi, liam.r.girdwood,
	shreyas.nc

From: Mengdong Lin <mengdong.lin@linux.intel.com>

After the type specific string ("uuid", "string", "byte", "short" and
"word"), users may append a string, like  "uuidxxx". The topology parser
will check the first few characters to get the tuple type.

This can allow users to put multiple tuples of the same type into one
vendor tuple section (SectionVendorTuples), e.g. parameters of multiple
firmware modules.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/include/topology.h b/include/topology.h
index 644e548..0675b52 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -302,6 +302,29 @@ extern "C" {
  *	}
  * }
  * </pre>
+ * To define multiple vendor tuples of same type, please append some
+ * characters after the type string ("string", "uuid", "bool", "byte", "short"
+ * or "word"), to avoid ID duplication in the SectionVendorTuples.<br>
+ * The parser will check the first few characters in ID to get the tuple type.
+ * Here is an example:
+ * <pre>
+ * SectionVendorTuples."id of the vendor tuples" {
+ *    ...
+ *	tuples."word.module0" {
+ *		VENDOR_TOKEN_PARAM_ID1 "0x00112233"
+ *		VENDOR_TOKEN_PARAM_ID2 "0x44556677"
+ *		...
+ *	}
+ *
+ *	tuples."word.module2" {
+ *		VENDOR_TOKEN_PARAM_ID1 "0x11223344"
+ *		VENDOR_TOKEN_PARAM_ID2 "0x55667788"
+ *		...
+ *	}
+ *	...
+ * }
+ *
+ * </pre>
  *
  * <h5>Mixer Controls</h5>
  * A mixer control is defined as a new section that can include channel mapping,
diff --git a/src/topology/data.c b/src/topology/data.c
index 245a834..e81b7f1 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -534,17 +534,17 @@ static int parse_tuple_set(snd_config_t *cfg,
 
 	snd_config_get_id(cfg, &id);
 
-	if (strcmp(id, "uuid") == 0)
+	if (strncmp(id, "uuid", 4) == 0)
 		type = SND_SOC_TPLG_TUPLE_TYPE_UUID;
-	else if (strcmp(id, "string") == 0)
+	else if (strncmp(id, "string", 5) == 0)
 		type = SND_SOC_TPLG_TUPLE_TYPE_STRING;
-	else if (strcmp(id, "bool") == 0)
+	else if (strncmp(id, "bool", 4) == 0)
 		type = SND_SOC_TPLG_TUPLE_TYPE_BOOL;
-	else if (strcmp(id, "byte") == 0)
+	else if (strncmp(id, "byte", 4) == 0)
 		type = SND_SOC_TPLG_TUPLE_TYPE_BYTE;
-	else if (strcmp(id, "short") == 0)
+	else if (strncmp(id, "short", 5) == 0)
 		type = SND_SOC_TPLG_TUPLE_TYPE_SHORT;
-	else if (strcmp(id, "word") == 0)
+	else if (strncmp(id, "word", 4) == 0)
 		type = SND_SOC_TPLG_TUPLE_TYPE_WORD;
 	else {
 		SNDERR("error: invalid tuple type '%s'\n", id);
-- 
2.5.0

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

* Re: [PATCH v2 0/5] topology: Enhance support for private data
  2016-07-15 12:17 [PATCH v2 0/5] topology: Enhance support for private data mengdong.lin
                   ` (4 preceding siblings ...)
  2016-07-15 12:19 ` [PATCH v2 5/5] topology: Tuple type can have an extenstion mengdong.lin
@ 2016-07-16 12:58 ` Takashi Sakamoto
  2016-07-18  2:47   ` Mengdong Lin
  2016-07-17  8:03 ` Takashi Iwai
  6 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2016-07-16 12:58 UTC (permalink / raw)
  To: mengdong.lin, alsa-devel, broonie
  Cc: tiwai, mengdong.lin, liam.r.girdwood, shreyas.nc

On Jul 15 2016 21:17, mengdong.lin@linux.intel.com wrote:
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> There is no ABI change in this series.
> 
> We finished verification on latest Intel platforms. There will be 4 or 5
> series. Later series will come with some ABI update for topology. The size
> check on ABI objects in current ASoC kernel will detect possible ABI
> version mismatch between user space and kernel
> 
> History:
> v2: fix compiler warnings. There are 2 warnings left in topology/pcm.c
>     that are not caused by this series and will be fixed in later patches.
> 
> Mengdong Lin (5):
>   topology: An element can refer to multipe data sections in text conf
>     file
>   topology: Merge lookup for data reference into tplg_copy_data()
>   topology: Change uuid value to 16 separate characters in text conf
>     file
>   topology: Parse vendor private data for manifest
>   topology: Tuple type can have an extenstion
> 
>  include/topology.h        |  69 ++++++++++-
>  src/topology/ctl.c        |  52 +++-----
>  src/topology/dapm.c       |  17 ++-
>  src/topology/data.c       | 307 ++++++++++++++++++++++++++++++++++++++--------
>  src/topology/elem.c       |   4 +
>  src/topology/parser.c     |  28 ++++-
>  src/topology/tplg_local.h |  15 ++-
>  7 files changed, 395 insertions(+), 97 deletions(-)

Reviewed-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

Current implementation of ALSA SoC part in kernel has no call
corresponding to sys_ioctl(SNDRV_CTL_IOCTL_ELEM_ADD) or something like
that, thus I recommend you to drop this line by an additional patch:
http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b948ac021ceecc116bc1087075743243e4b14055;hb=HEAD#l44


Regards

Takashi Sakamoto

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

* Re: [PATCH v2 0/5] topology: Enhance support for private data
  2016-07-15 12:17 [PATCH v2 0/5] topology: Enhance support for private data mengdong.lin
                   ` (5 preceding siblings ...)
  2016-07-16 12:58 ` [PATCH v2 0/5] topology: Enhance support for private data Takashi Sakamoto
@ 2016-07-17  8:03 ` Takashi Iwai
  6 siblings, 0 replies; 14+ messages in thread
From: Takashi Iwai @ 2016-07-17  8:03 UTC (permalink / raw)
  To: mengdong.lin
  Cc: alsa-devel, mengdong.lin, broonie, liam.r.girdwood, shreyas.nc,
	o-takashi

On Fri, 15 Jul 2016 14:17:21 +0200,
mengdong.lin@linux.intel.com wrote:
> 
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> There is no ABI change in this series.
> 
> We finished verification on latest Intel platforms. There will be 4 or 5
> series. Later series will come with some ABI update for topology. The size
> check on ABI objects in current ASoC kernel will detect possible ABI
> version mismatch between user space and kernel
> 
> History:
> v2: fix compiler warnings. There are 2 warnings left in topology/pcm.c
>     that are not caused by this series and will be fixed in later patches.
> 
> Mengdong Lin (5):
>   topology: An element can refer to multipe data sections in text conf
>     file
>   topology: Merge lookup for data reference into tplg_copy_data()
>   topology: Change uuid value to 16 separate characters in text conf
>     file
>   topology: Parse vendor private data for manifest
>   topology: Tuple type can have an extenstion

Applied, thanks.


Takashi

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

* Re: [PATCH v2 0/5] topology: Enhance support for private data
  2016-07-16 12:58 ` [PATCH v2 0/5] topology: Enhance support for private data Takashi Sakamoto
@ 2016-07-18  2:47   ` Mengdong Lin
  2016-07-18  3:43     ` Takashi Sakamoto
  0 siblings, 1 reply; 14+ messages in thread
From: Mengdong Lin @ 2016-07-18  2:47 UTC (permalink / raw)
  To: Takashi Sakamoto, alsa-devel, broonie
  Cc: tiwai, mengdong.lin, liam.r.girdwood, shreyas.nc



On 07/16/2016 08:58 PM, Takashi Sakamoto wrote:

>
> Current implementation of ALSA SoC part in kernel has no call
> corresponding to sys_ioctl(SNDRV_CTL_IOCTL_ELEM_ADD) or something like
> that, thus I recommend you to drop this line by an additional patch:
> http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b948ac021ceecc116bc1087075743243e4b14055;hb=HEAD#l44
>

Thanks for your suggestions. But this tool is not for directly adding 
controls from the user space.

It's for ADSP firmware vendors to describe the controls/widgets/pcms for 
a specific version of firmware. Previously, these components are hard 
coded in ASoC platforms drivers and not so convenient to share a common 
driver for different firmwares. The topology binary generated by this 
tool will be loaded by ASoC platform driver with a specific firmware 
binary, and so ASoC will create the controls/widgets/pcms that match the 
firmware automatically.

Thanks
Mengdong

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

* Re: [PATCH v2 0/5] topology: Enhance support for private data
  2016-07-18  2:47   ` Mengdong Lin
@ 2016-07-18  3:43     ` Takashi Sakamoto
  2016-07-18 15:06       ` Lin, Mengdong
  0 siblings, 1 reply; 14+ messages in thread
From: Takashi Sakamoto @ 2016-07-18  3:43 UTC (permalink / raw)
  To: Mengdong Lin, alsa-devel, broonie
  Cc: tiwai, mengdong.lin, liam.r.girdwood, shreyas.nc

On Jul 18 2016 11:47, Mengdong Lin wrote:
> Thanks for your suggestions. But this tool is not for directly adding
> controls from the user space.
> 
> It's for ADSP firmware vendors to describe the controls/widgets/pcms for
> a specific version of firmware. Previously, these components are hard
> coded in ASoC platforms drivers and not so convenient to share a common
> driver for different firmwares. The topology binary generated by this
> tool will be loaded by ASoC platform driver with a specific firmware
> binary, and so ASoC will create the controls/widgets/pcms that match the
> firmware automatically.

I know all of what you explained. Therefore, You misunderstand about my
suggestion.

What I suggested is to drop a support of SNDRV_CTL_ELEM_ACCESS_USER flag
from current topology implementation in userspace library, because
current implementation of ALSA SoC part doesn't support it.

In ALSA SoC part, snd_ctl_new1() is called to add a control element set
as a result of parsing binary blob generated by topology implementation.

In snd_ctl_new1(), SNDRV_CTL_ELEM_ACCESS_USER flag is dropped, in these
lines:
http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/control.c#n263

Totally, the flag cannot be supported by current implementation of
topology in kernel. Thus, in userspace, we should drop this line to
prevent users from confusions:
http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b948ac021ceecc116bc1087075743243e4b14055;hb=HEAD#l44

That's all what I mentioned. If something unclear, please tell it to me.


Regards

Takashi Sakamoto

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

* Re: [PATCH v2 0/5] topology: Enhance support for private data
  2016-07-18  3:43     ` Takashi Sakamoto
@ 2016-07-18 15:06       ` Lin, Mengdong
  2016-07-19  3:59         ` Vinod Koul
  0 siblings, 1 reply; 14+ messages in thread
From: Lin, Mengdong @ 2016-07-18 15:06 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: alsa-devel, Mengdong Lin, tiwai, broonie, Girdwood, Liam R, Nc, Shreyas


> -----Original Message-----
> From: Takashi Sakamoto [mailto:o-takashi@sakamocchi.jp]
> Sent: Monday, July 18, 2016 11:43 AM
 
> I know all of what you explained. Therefore, You misunderstand about my
> suggestion.
> 
> What I suggested is to drop a support of SNDRV_CTL_ELEM_ACCESS_USER
> flag from current topology implementation in userspace library, because
> current implementation of ALSA SoC part doesn't support it.
> 
> In ALSA SoC part, snd_ctl_new1() is called to add a control element set as a
> result of parsing binary blob generated by topology implementation.
> 
> In snd_ctl_new1(), SNDRV_CTL_ELEM_ACCESS_USER flag is dropped, in these
> lines:
> http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/c
> ontrol.c#n263
> 
> Totally, the flag cannot be supported by current implementation of topology
> in kernel. Thus, in userspace, we should drop this line to prevent users from
> confusions:
> http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b94
> 8ac021ceecc116bc1087075743243e4b14055;hb=HEAD#l44
> 
> That's all what I mentioned. If something unclear, please tell it to me.

Yes, you're right. snd_ctl_new1() dropped SNDRV_CTL_ELEM_ACCESS_USER flag.
I'll remove this from the topology user space tool to avoid confusion.

Thanks again
Mengdong

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

* Re: [PATCH v2 0/5] topology: Enhance support for private data
  2016-07-18 15:06       ` Lin, Mengdong
@ 2016-07-19  3:59         ` Vinod Koul
  2016-07-19  5:14           ` Takashi Sakamoto
  2016-07-19  8:44           ` Lin, Mengdong
  0 siblings, 2 replies; 14+ messages in thread
From: Vinod Koul @ 2016-07-19  3:59 UTC (permalink / raw)
  To: Lin, Mengdong
  Cc: alsa-devel, Mengdong Lin, tiwai, Takashi Sakamoto, broonie,
	Girdwood, Liam R, Nc, Shreyas

On Mon, Jul 18, 2016 at 03:06:24PM +0000, Lin, Mengdong wrote:
> 
> > -----Original Message-----
> > From: Takashi Sakamoto [mailto:o-takashi@sakamocchi.jp]
> > Sent: Monday, July 18, 2016 11:43 AM
>  
> > I know all of what you explained. Therefore, You misunderstand about my
> > suggestion.
> > 
> > What I suggested is to drop a support of SNDRV_CTL_ELEM_ACCESS_USER
> > flag from current topology implementation in userspace library, because
> > current implementation of ALSA SoC part doesn't support it.
> > 
> > In ALSA SoC part, snd_ctl_new1() is called to add a control element set as a
> > result of parsing binary blob generated by topology implementation.
> > 
> > In snd_ctl_new1(), SNDRV_CTL_ELEM_ACCESS_USER flag is dropped, in these
> > lines:
> > http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sound/core/c
> > ontrol.c#n263
> > 
> > Totally, the flag cannot be supported by current implementation of topology
> > in kernel. Thus, in userspace, we should drop this line to prevent users from
> > confusions:
> > http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ctl.c;h=b94
> > 8ac021ceecc116bc1087075743243e4b14055;hb=HEAD#l44
> > 
> > That's all what I mentioned. If something unclear, please tell it to me.
> 
> Yes, you're right. snd_ctl_new1() dropped SNDRV_CTL_ELEM_ACCESS_USER flag.
> I'll remove this from the topology user space tool to avoid confusion.

Oh no, that won't be a good idea.

We would like to specify the access for controls from topology. Some
controls can be read only and some write only :)

For example, any VU-meter controls should be read-only. Similarly if we have
some user data being sent to some modules which can do do all fancy audio
detection then these controls should be write-only.

Yes it seems to be broken by this, but we should fix it rather than remove.

-- 
~Vinod

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

* Re: [PATCH v2 0/5] topology: Enhance support for private data
  2016-07-19  3:59         ` Vinod Koul
@ 2016-07-19  5:14           ` Takashi Sakamoto
  2016-07-19  8:44           ` Lin, Mengdong
  1 sibling, 0 replies; 14+ messages in thread
From: Takashi Sakamoto @ 2016-07-19  5:14 UTC (permalink / raw)
  To: Vinod Koul, Lin, Mengdong
  Cc: alsa-devel, Mengdong Lin, tiwai, broonie, Girdwood, Liam R, Nc, Shreyas

On Jul 19 2016 12:59, Vinod Koul wrote:
>> Yes, you're right. snd_ctl_new1() dropped SNDRV_CTL_ELEM_ACCESS_USER flag.
>> I'll remove this from the topology user space tool to avoid confusion.
>
> Oh no, that won't be a good idea.
>
> We would like to specify the access for controls from topology. Some
> controls can be read only and some write only :)
>
> For example, any VU-meter controls should be read-only. Similarly if we have
> some user data being sent to some modules which can do do all fancy audio
> detection then these controls should be write-only.

?

I just suggested to remove 'user' from topology implementation in 
userspace library. You can still control access rights of control 
elements by adding enough flags; 'read' or 'write' 
(SNDRV_CTL_ELEM_ACCESS_READ or SNDRV_CTL_ELEM_ACCESS_WRITE). 
SNDRV_CTL_ELEM_ACCESS_USER has no relationship to your purposes.

When a control element has SNDRV_CTL_ELEM_ACCESS_USER flag, a control 
element set including the element has below operations;
  - struct snd_kcontrol.info = snd_ctl_elem_user_enum_info or
                               snd_ctl_elem_user_info
  - struct snd_kcontrol.get = snd_ctl_elem_user_get
  - struct snd_kcontrol.put = snd_ctl_elem_user_put
  - struct snd_kcontrol.tlv.c = snd_ctl_elem_user_tlv

When userland applications call ioctl(2) with SNDRV_CTL_IOCTL_ELEM_ADD, 
then ALSA ctl core adds new control element set. The 'USER' flag is used 
only for this case.

I think it better for you to read 'sound/core/control.c' or my test 
program in alsa-lib source.
http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=test/user-ctl-element-set.c


Regards

Takashi Sakamoto

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

* Re: [PATCH v2 0/5] topology: Enhance support for private data
  2016-07-19  3:59         ` Vinod Koul
  2016-07-19  5:14           ` Takashi Sakamoto
@ 2016-07-19  8:44           ` Lin, Mengdong
  1 sibling, 0 replies; 14+ messages in thread
From: Lin, Mengdong @ 2016-07-19  8:44 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: alsa-devel, Mengdong Lin, tiwai, Takashi Sakamoto, broonie,
	Girdwood, Liam R, Nc, Shreyas

> -----Original Message-----
> From: alsa-devel-bounces@alsa-project.org
> [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Vinod Koul
> Sent: Tuesday, July 19, 2016 12:00 PM
> To: Lin, Mengdong
> Cc: alsa-devel@alsa-project.org; Mengdong Lin; tiwai@suse.de; Takashi
> Sakamoto; broonie@kernel.org; Girdwood, Liam R; Nc, Shreyas
> Subject: Re: [alsa-devel] [PATCH v2 0/5] topology: Enhance support for
> private data
> 
> On Mon, Jul 18, 2016 at 03:06:24PM +0000, Lin, Mengdong wrote:
> >
> > > -----Original Message-----
> > > From: Takashi Sakamoto [mailto:o-takashi@sakamocchi.jp]
> > > Sent: Monday, July 18, 2016 11:43 AM
> >
> > > I know all of what you explained. Therefore, You misunderstand about
> > > my suggestion.
> > >
> > > What I suggested is to drop a support of
> SNDRV_CTL_ELEM_ACCESS_USER
> > > flag from current topology implementation in userspace library,
> > > because current implementation of ALSA SoC part doesn't support it.
> > >
> > > In ALSA SoC part, snd_ctl_new1() is called to add a control element
> > > set as a result of parsing binary blob generated by topology
> implementation.
> > >
> > > In snd_ctl_new1(), SNDRV_CTL_ELEM_ACCESS_USER flag is dropped, in
> > > these
> > > lines:
> > > http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/tree/sou
> > > nd/core/c
> > > ontrol.c#n263
> > >
> > > Totally, the flag cannot be supported by current implementation of
> > > topology in kernel. Thus, in userspace, we should drop this line to
> > > prevent users from
> > > confusions:
> > > http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/topology/ct
> > > l.c;h=b94
> > > 8ac021ceecc116bc1087075743243e4b14055;hb=HEAD#l44
> > >
> > > That's all what I mentioned. If something unclear, please tell it to me.
> >
> > Yes, you're right. snd_ctl_new1() dropped
> SNDRV_CTL_ELEM_ACCESS_USER flag.
> > I'll remove this from the topology user space tool to avoid confusion.
> 
> Oh no, that won't be a good idea.

Hi Vinod,

I think topology will drop support for only one flag "SNDRV_CTL_ELEM_ACCESS_USER". All other access flags are still supported.

Thanks
Mengdong

> 
> We would like to specify the access for controls from topology. Some controls
> can be read only and some write only :)
> 
> For example, any VU-meter controls should be read-only. Similarly if we have
> some user data being sent to some modules which can do do all fancy audio
> detection then these controls should be write-only.
> 
> Yes it seems to be broken by this, but we should fix it rather than remove.
> 
> --
> ~Vinod
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

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

end of thread, other threads:[~2016-07-19  8:44 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-15 12:17 [PATCH v2 0/5] topology: Enhance support for private data mengdong.lin
2016-07-15 12:17 ` [PATCH v2 1/5] topology: An element can refer to multipe data sections in text conf file mengdong.lin
2016-07-15 12:18 ` [PATCH v2 2/5] topology: Merge lookup for data reference into tplg_copy_data() mengdong.lin
2016-07-15 12:18 ` [PATCH v2 3/5] topology: Change uuid value to 16 separate characters in text conf file mengdong.lin
2016-07-15 12:19 ` [PATCH v2 4/5] topology: Parse vendor private data for manifest mengdong.lin
2016-07-15 12:19 ` [PATCH v2 5/5] topology: Tuple type can have an extenstion mengdong.lin
2016-07-16 12:58 ` [PATCH v2 0/5] topology: Enhance support for private data Takashi Sakamoto
2016-07-18  2:47   ` Mengdong Lin
2016-07-18  3:43     ` Takashi Sakamoto
2016-07-18 15:06       ` Lin, Mengdong
2016-07-19  3:59         ` Vinod Koul
2016-07-19  5:14           ` Takashi Sakamoto
2016-07-19  8:44           ` Lin, Mengdong
2016-07-17  8:03 ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.