All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes
@ 2016-09-28 23:57 Takashi Sakamoto
  2016-09-28 23:57 ` [PATCH v2 1/5] Update include/sound/tlv.h from 4.9-pre kernel uapi Takashi Sakamoto
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Takashi Sakamoto @ 2016-09-28 23:57 UTC (permalink / raw)
  To: clemens, tiwai, perex; +Cc: alsa-devel

Hi,

This patchset updates my former one:
[alsa-devel] [PATCH 0/4][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-September/112853.html

In this month, UAPI header of 4.9-pre kernel newly include existent macros
related to TLV operation for user land. This is to allow applications to
register TLV information to element set which they added.

This patchset is for related changes in ALSA user land library. The macros
are put into 'include/sound/tlv.h'. However corresponding macros are not
put into 'include/control.h' yet because currently I have no better idea
for library APIs to construct tlv information.

This patchset also improves some corrections for documentation which
committed with my misunderstandings.


Takashi Sakamoto (5):
  Update include/sound/tlv.h from 4.9-pre kernel uapi
  test: use actual information for TLV operation
  ctl: improve API documentation for TLV operation
  ctl: improve documentation about TLV-related APIs
  ctl: correct documentation about TLV feature

 include/sound/tlv.h         |  77 ++++++++++++++++
 src/control/control.c       |  47 +++++++---
 test/user-ctl-element-set.c | 214 ++++++++++++++++++++++++++++++++++++--------
 3 files changed, 289 insertions(+), 49 deletions(-)

-- 
2.7.4

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

* [PATCH v2 1/5] Update include/sound/tlv.h from 4.9-pre kernel uapi
  2016-09-28 23:57 [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes Takashi Sakamoto
@ 2016-09-28 23:57 ` Takashi Sakamoto
  2016-09-28 23:57 ` [PATCH v2 2/5] test: use actual information for TLV operation Takashi Sakamoto
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Sakamoto @ 2016-09-28 23:57 UTC (permalink / raw)
  To: clemens, tiwai, perex; +Cc: alsa-devel

The UAPI header in 4.9-pre kernel newly includes existent macros related
to tlv operation, mainly for layout of TLV packet payload.

This commit updates corresponding backport header in this library.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 include/sound/tlv.h | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/include/sound/tlv.h b/include/sound/tlv.h
index 33d747d..b4df440 100644
--- a/include/sound/tlv.h
+++ b/include/sound/tlv.h
@@ -20,4 +20,81 @@
 #define SNDRV_CTL_TLVT_DB_MINMAX 4	/* dB scale with min/max */
 #define SNDRV_CTL_TLVT_DB_MINMAX_MUTE 5	/* dB scale with min/max with mute */
 
+/*
+ * channel-mapping TLV items
+ *  TLV length must match with num_channels
+ */
+#define SNDRV_CTL_TLVT_CHMAP_FIXED	0x101	/* fixed channel position */
+#define SNDRV_CTL_TLVT_CHMAP_VAR	0x102	/* channels freely swappable */
+#define SNDRV_CTL_TLVT_CHMAP_PAIRED	0x103	/* pair-wise swappable */
+
+/*
+ * TLV structure is right behind the struct snd_ctl_tlv:
+ *   unsigned int type  	- see SNDRV_CTL_TLVT_*
+ *   unsigned int length
+ *   .... data aligned to sizeof(unsigned int), use
+ *        block_length = (length + (sizeof(unsigned int) - 1)) &
+ *                       ~(sizeof(unsigned int) - 1)) ....
+ */
+#define SNDRV_CTL_TLVD_ITEM(type, ...) \
+	(type), SNDRV_CTL_TLVD_LENGTH(__VA_ARGS__), __VA_ARGS__
+#define SNDRV_CTL_TLVD_LENGTH(...) \
+	((unsigned int)sizeof((const unsigned int[]) { __VA_ARGS__ }))
+
+#define SNDRV_CTL_TLVD_CONTAINER_ITEM(...) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_CONTAINER, __VA_ARGS__)
+#define SNDRV_CTL_TLVD_DECLARE_CONTAINER(name, ...) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_CONTAINER_ITEM(__VA_ARGS__) \
+	}
+
+#define SNDRV_CTL_TLVD_DB_SCALE_MASK	0xffff
+#define SNDRV_CTL_TLVD_DB_SCALE_MUTE	0x10000
+#define SNDRV_CTL_TLVD_DB_SCALE_ITEM(min, step, mute) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_SCALE, \
+			    (min), \
+			    ((step) & SNDRV_CTL_TLVD_DB_SCALE_MASK) | \
+			     ((mute) ? SNDRV_CTL_TLVD_DB_SCALE_MUTE : 0))
+#define SNDRV_CTL_TLVD_DECLARE_DB_SCALE(name, min, step, mute) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_SCALE_ITEM(min, step, mute) \
+	}
+
+/* dB scale specified with min/max values instead of step */
+#define SNDRV_CTL_TLVD_DB_MINMAX_ITEM(min_dB, max_dB) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_MINMAX, (min_dB), (max_dB))
+#define SNDRV_CTL_TLVD_DB_MINMAX_MUTE_ITEM(min_dB, max_dB) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_MINMAX_MUTE, (min_dB), (max_dB))
+#define SNDRV_CTL_TLVD_DECLARE_DB_MINMAX(name, min_dB, max_dB) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_MINMAX_ITEM(min_dB, max_dB) \
+	}
+#define SNDRV_CTL_TLVD_DECLARE_DB_MINMAX_MUTE(name, min_dB, max_dB) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_MINMAX_MUTE_ITEM(min_dB, max_dB) \
+	}
+
+/* linear volume between min_dB and max_dB (.01dB unit) */
+#define SNDRV_CTL_TLVD_DB_LINEAR_ITEM(min_dB, max_dB) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_LINEAR, (min_dB), (max_dB))
+#define SNDRV_CTL_TLVD_DECLARE_DB_LINEAR(name, min_dB, max_dB) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_LINEAR_ITEM(min_dB, max_dB) \
+	}
+
+/* dB range container:
+ * Items in dB range container must be ordered by their values and by their
+ * dB values. This implies that larger values must correspond with larger
+ * dB values (which is also required for all other mixer controls).
+ */
+/* Each item is: <min> <max> <TLV> */
+#define SNDRV_CTL_TLVD_DB_RANGE_ITEM(...) \
+	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_DB_RANGE, __VA_ARGS__)
+#define SNDRV_CTL_TLVD_DECLARE_DB_RANGE(name, ...) \
+	unsigned int name[] = { \
+		SNDRV_CTL_TLVD_DB_RANGE_ITEM(__VA_ARGS__) \
+	}
+
+#define SNDRV_CTL_TLVD_DB_GAIN_MUTE	-9999999
+
 #endif
-- 
2.7.4

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

* [PATCH v2 2/5] test: use actual information for TLV operation
  2016-09-28 23:57 [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes Takashi Sakamoto
  2016-09-28 23:57 ` [PATCH v2 1/5] Update include/sound/tlv.h from 4.9-pre kernel uapi Takashi Sakamoto
@ 2016-09-28 23:57 ` Takashi Sakamoto
  2016-09-28 23:57 ` [PATCH v2 3/5] ctl: improve API documentation " Takashi Sakamoto
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Sakamoto @ 2016-09-28 23:57 UTC (permalink / raw)
  To: clemens, tiwai, perex; +Cc: alsa-devel

Currently, this test program uses undefined type of TLV data. This can
bring confusions to userspace applications.

This commit replaces the array with valid information, constructed by newly
exported TLV macros from kernel land.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 test/user-ctl-element-set.c | 214 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 178 insertions(+), 36 deletions(-)

diff --git a/test/user-ctl-element-set.c b/test/user-ctl-element-set.c
index 9b9dc59..75083d2 100644
--- a/test/user-ctl-element-set.c
+++ b/test/user-ctl-element-set.c
@@ -8,6 +8,7 @@
  */
 
 #include "../include/asoundlib.h"
+#include <sound/tlv.h>
 
 struct elem_set_trial {
 	snd_ctl_t *handle;
@@ -25,8 +26,30 @@ struct elem_set_trial {
 				snd_ctl_elem_info_t *info);
 	void (*change_elem_members)(struct elem_set_trial *trial,
 				    snd_ctl_elem_value_t *elem_data);
+	int (*allocate_elem_set_tlv)(struct elem_set_trial *trial,
+				     unsigned int **tlv);
 };
 
+struct chmap_entry {
+	unsigned int type;
+	unsigned int length;
+	unsigned int maps[0];
+};
+
+/*
+ * History of TLV feature:
+ *
+ * 2016/09/15: 398fa4db6c69 ("ALSA: control: move layout of TLV payload to UAPI
+ *			      header")
+ * 2012/07/21: 2d3391ec0ecc ("ALSA: PCM: channel mapping API implementation")
+ * 2011/11/20: bf1d1c9b6179 ("ALSA: tlv: add DECLARE_TLV_DB_RANGE()")
+ * 2009/07/16: 085f30654175 ("ALSA: Add new TLV types for dBwith min/max")
+ * 2006/09/06: 55a29af5ed5d ("[ALSA] Add definition of TLV dB range compound")
+ * 2006/08/28: 063a40d9111c ("Add the definition of linear volume TLV")
+ * 2006/08/28: 42750b04c5ba ("[ALSA] Control API - TLV implementation for
+ *			      additional information like dB scale")
+ */
+
 /* Operations for elements in an element set with boolean type. */
 static int add_bool_elem_set(struct elem_set_trial *trial,
 			     snd_ctl_elem_info_t *info)
@@ -47,13 +70,30 @@ static void change_bool_elem_members(struct elem_set_trial *trial,
 	}
 }
 
+static int allocate_bool_elem_set_tlv(struct elem_set_trial *trial,
+				      unsigned int **tlv)
+{
+	/*
+	 * Performs like a toggle switch for attenuation, because they're bool
+	 * elements.
+	 */
+	static const SNDRV_CTL_TLVD_DECLARE_DB_MINMAX(range, -10000, 0);
+
+	*tlv = malloc(sizeof(range));
+	if (*tlv == NULL)
+		return -ENOMEM;
+	memcpy(*tlv, range, sizeof(range));
+
+	return 0;
+}
+
 /* Operations for elements in an element set with integer type. */
 static int add_int_elem_set(struct elem_set_trial *trial,
 			    snd_ctl_elem_info_t *info)
 {
 	return snd_ctl_add_integer_elem_set(trial->handle, info,
 				trial->element_count, trial->member_count,
-				0, 99, 1);
+				0, 25, 1);
 }
 
 static int check_int_elem_props(struct elem_set_trial *trial,
@@ -61,7 +101,7 @@ static int check_int_elem_props(struct elem_set_trial *trial,
 {
 	if (snd_ctl_elem_info_get_min(info) != 0)
 		return -EIO;
-	if (snd_ctl_elem_info_get_max(info) != 99)
+	if (snd_ctl_elem_info_get_max(info) != 25)
 		return -EIO;
 	if (snd_ctl_elem_info_get_step(info) != 1)
 		return -EIO;
@@ -81,6 +121,41 @@ static void change_int_elem_members(struct elem_set_trial *trial,
 	}
 }
 
+static int allocate_int_elem_set_tlv(struct elem_set_trial *trial,
+				     unsigned int **tlv)
+{
+	unsigned int len, pos;
+	unsigned int i, j;
+	struct chmap_entry *entry;
+
+	/* Calculate size of TLV packet for channel-mapping information. */
+	len = 0;
+	for (i = 1; i <= 25; ++i) {
+		len += sizeof(struct chmap_entry);
+		len += i * sizeof(unsigned int);
+	}
+
+	*tlv = malloc(len);
+	if (*tlv == NULL)
+		return -ENOMEM;
+
+	/*
+	 * Emulate channel-mapping information in in-kernel implementation.
+	 * Here, 25 entries are for each different channel.
+	 */
+	pos = 0;
+	for (i = 1; i <= 25 && pos < len; ++i) {
+		entry = (struct chmap_entry *)&(*tlv)[pos];
+		entry->type = SNDRV_CTL_TLVT_CHMAP_FIXED;
+		entry->length = i * sizeof(unsigned int);
+		for (j = 0; j < i; ++j)
+			entry->maps[j] = SND_CHMAP_MONO + j;
+		pos += sizeof(struct chmap_entry) + i * sizeof(unsigned int);
+	}
+
+	return 0;
+}
+
 /* Operations for elements in an element set with enumerated type. */
 static const char *const labels[] = {
 	"trusty",
@@ -158,6 +233,24 @@ static void change_bytes_elem_members(struct elem_set_trial *trial,
 	}
 }
 
+static int allocate_bytes_elem_set_tlv(struct elem_set_trial *trial,
+				       unsigned int **tlv)
+{
+	/*
+	 * Emulate AK4396.
+	 * 20 * log10(x/255) (dB)
+	 * Here, x is written value.
+	 */
+	static const SNDRV_CTL_TLVD_DECLARE_DB_LINEAR(range, -4813, 0);
+
+	*tlv = malloc(sizeof(range));
+	if (*tlv == NULL)
+		return -ENOMEM;
+	memcpy(*tlv, range, sizeof(range));
+
+	return 0;
+}
+
 /* Operations for elements in an element set with iec958 type. */
 static int add_iec958_elem_set(struct elem_set_trial *trial,
 			       snd_ctl_elem_info_t *info)
@@ -197,17 +290,17 @@ static int add_int64_elem_set(struct elem_set_trial *trial,
 {
 	return snd_ctl_add_integer64_elem_set(trial->handle, info,
 				trial->element_count, trial->member_count,
-				100, 10000, 30);
+				0, 10000, 1);
 }
 
 static int check_int64_elem_props(struct elem_set_trial *trial,
 				  snd_ctl_elem_info_t *info)
 {
-	if (snd_ctl_elem_info_get_min64(info) != 100)
+	if (snd_ctl_elem_info_get_min64(info) != 0)
 		return -EIO;
 	if (snd_ctl_elem_info_get_max64(info) != 10000)
 		return -EIO;
-	if (snd_ctl_elem_info_get_step64(info) != 30)
+	if (snd_ctl_elem_info_get_step64(info) != 1)
 		return -EIO;
 
 	return 0;
@@ -225,6 +318,45 @@ static void change_int64_elem_members(struct elem_set_trial *trial,
 	}
 }
 
+static int allocate_int64_elem_set_tlv(struct elem_set_trial *trial,
+				       unsigned int **tlv)
+{
+	/*
+	 * Use this fomula between linear/dB value:
+	 *
+	 *  Linear: dB range (coeff)
+	 *   0<-> 4: -59.40<->-56.36 (44)
+	 *   4<->22: -56.36<->-45.56 (60)
+	 *  22<->33: -45.56<->-40.72 (76)
+	 *  33<->37: -40.72<->-38.32 (44)
+	 *  37<->48: -38.32<->-29.96 (76)
+	 *  48<->66: -29.96<->-22.04 (60)
+	 *  66<->84: -22.04<-> -8.36 (44)
+	 *  84<->95:  -8.36<-> -1.76 (60)
+	 *  95<->99:  -1.76<->  0.00 (76)
+	 * 100<->..:   0.0
+	 */
+	static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(range,
+		 0,   4, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-5940, 44, 1),
+		 4,  22, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-5636, 60, 0),
+		22,  33, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-4556, 76, 0),
+		33,  37, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-4072, 44, 0),
+		37,  48, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-3832, 76, 0),
+		48,  66, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-2996, 60, 0),
+		66,  84, SNDRV_CTL_TLVD_DB_SCALE_ITEM(-2204, 44, 0),
+		84,  95, SNDRV_CTL_TLVD_DB_SCALE_ITEM( -836, 60, 0),
+		95,  99, SNDRV_CTL_TLVD_DB_SCALE_ITEM( -176, 76, 0),
+		100, 10000, SNDRV_CTL_TLVD_DB_SCALE_ITEM(0, 0, 0),
+	);
+
+	*tlv = malloc(sizeof(range));
+	if (*tlv == NULL)
+		return -ENOMEM;
+	memcpy(*tlv, range, sizeof(range));
+
+	return 0;
+}
+
 /* Common operations. */
 static int add_elem_set(struct elem_set_trial *trial)
 {
@@ -414,41 +546,41 @@ static int check_elems(struct elem_set_trial *trial)
 
 static int check_tlv(struct elem_set_trial *trial)
 {
-	unsigned int orig[8], curr[8];
+	unsigned int *tlv;
+	unsigned int len;
+	unsigned int *curr;
 	int err;
 
-	/*
-	 * See a layout of 'struct snd_ctl_tlv'. I don't know the reason to
-	 * construct this buffer with the same layout. It should be abstracted
-	 * inner userspace library...
-	 */
-	orig[0] = snd_ctl_elem_id_get_numid(trial->id);
-	orig[1] = 6 * sizeof(orig[0]);
-	orig[2] = 'a';
-	orig[3] = 'b';
-	orig[4] = 'c';
-	orig[5] = 'd';
-	orig[6] = 'e';
-	orig[7] = 'f';
+	err = trial->allocate_elem_set_tlv(trial, &tlv);
+	if (err < 0)
+		return err;
+
+	len = tlv[1] + sizeof(unsigned int) * 2;
+	curr = malloc(len);
+	if (curr == NULL) {
+		free(tlv);
+		return -ENOMEM;
+	}
 
 	/*
 	 * In in-kernel implementation, write and command operations are the
-	 * same  for an element set added by userspace applications. Here, I
+	 * same for an element set added by userspace applications. Here, I
 	 * use write.
 	 */
 	err = snd_ctl_elem_tlv_write(trial->handle, trial->id,
-				     (const unsigned int *)orig);
+				     (const unsigned int *)tlv);
 	if (err < 0)
-		return err;
+		goto end;
 
-	err = snd_ctl_elem_tlv_read(trial->handle, trial->id, curr,
-				    sizeof(curr));
+	err = snd_ctl_elem_tlv_read(trial->handle, trial->id, curr, len);
 	if (err < 0)
-		return err;
-
-	if (memcmp(curr, orig, sizeof(orig)) != 0)
-		return -EIO;
+		goto end;
 
+	if (memcmp(curr, tlv, len) != 0)
+		err = -EIO;
+end:
+	free(tlv);
+	free(curr);
 	return 0;
 }
 
@@ -484,6 +616,8 @@ int main(void)
 			trial.add_elem_set = add_bool_elem_set;
 			trial.check_elem_props = NULL;
 			trial.change_elem_members = change_bool_elem_members;
+			trial.allocate_elem_set_tlv =
+						allocate_bool_elem_set_tlv;
 			break;
 		case SND_CTL_ELEM_TYPE_INTEGER:
 			trial.element_count = 900;
@@ -495,6 +629,8 @@ int main(void)
 			trial.add_elem_set = add_int_elem_set;
 			trial.check_elem_props = check_int_elem_props;
 			trial.change_elem_members = change_int_elem_members;
+			trial.allocate_elem_set_tlv =
+						allocate_int_elem_set_tlv;
 			break;
 		case SND_CTL_ELEM_TYPE_ENUMERATED:
 			trial.element_count = 900;
@@ -506,6 +642,7 @@ int main(void)
 			trial.add_elem_set = add_enum_elem_set;
 			trial.check_elem_props = check_enum_elem_props;
 			trial.change_elem_members = change_enum_elem_members;
+			trial.allocate_elem_set_tlv = NULL;
 			break;
 		case SND_CTL_ELEM_TYPE_BYTES:
 			trial.element_count = 900;
@@ -517,6 +654,8 @@ int main(void)
 			trial.add_elem_set = add_bytes_elem_set;
 			trial.check_elem_props = NULL;
 			trial.change_elem_members = change_bytes_elem_members;
+			trial.allocate_elem_set_tlv =
+						allocate_bytes_elem_set_tlv;
 			break;
 		case SND_CTL_ELEM_TYPE_IEC958:
 			trial.element_count = 1;
@@ -528,6 +667,7 @@ int main(void)
 			trial.add_elem_set = add_iec958_elem_set;
 			trial.check_elem_props = NULL;
 			trial.change_elem_members = change_iec958_elem_members;
+			trial.allocate_elem_set_tlv = NULL;
 			break;
 		case SND_CTL_ELEM_TYPE_INTEGER64:
 		default:
@@ -540,6 +680,8 @@ int main(void)
 			trial.add_elem_set = add_int64_elem_set;
 			trial.check_elem_props = check_int64_elem_props;
 			trial.change_elem_members = change_int64_elem_members;
+			trial.allocate_elem_set_tlv =
+						allocate_int64_elem_set_tlv;
 			break;
 		}
 
@@ -589,22 +731,22 @@ int main(void)
 		}
 
 		/*
-		 * Test an operation to change threshold data of this element set,
-		 * except for IEC958 type.
+		 * Test an operation to change TLV data of this element set,
+		 * except for enumerated and IEC958 type.
 		 */
-		if (trial.type != SND_CTL_ELEM_TYPE_IEC958) {
+		if (trial.allocate_elem_set_tlv != NULL) {
 			err = check_tlv(&trial);
 			if (err < 0) {
-				printf("Fail to change threshold level of an "
-				       "element set with %s type.\n",
+				printf("Fail to change TLV data of an element "
+				       "set with %s type.\n",
 				       snd_ctl_elem_type_name(trial.type));
 				break;
 			}
 			err = check_event(&trial, SND_CTL_EVENT_MASK_TLV, 1);
 			if (err < 0) {
-				printf("Fail to check an event to change "
-				       "threshold level of an an element set "
-				       "with %s type.\n",
+				printf("Fail to check an event to change TLV"
+				       "data of an an element set with %s "
+				       "type.\n",
 				       snd_ctl_elem_type_name(trial.type));
 				break;
 			}
-- 
2.7.4

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

* [PATCH v2 3/5] ctl: improve API documentation for TLV operation
  2016-09-28 23:57 [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes Takashi Sakamoto
  2016-09-28 23:57 ` [PATCH v2 1/5] Update include/sound/tlv.h from 4.9-pre kernel uapi Takashi Sakamoto
  2016-09-28 23:57 ` [PATCH v2 2/5] test: use actual information for TLV operation Takashi Sakamoto
@ 2016-09-28 23:57 ` Takashi Sakamoto
  2016-09-28 23:57 ` [PATCH v2 4/5] ctl: improve documentation about TLV-related APIs Takashi Sakamoto
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Sakamoto @ 2016-09-28 23:57 UTC (permalink / raw)
  To: clemens, tiwai, perex; +Cc: alsa-devel

A commit fe1b08803db6 ("ctl: improve API documentation for threshold level
operations") changes documentations for some TLV-related APIs with wrong
explanations.

This commit fix it with better explanations.

Fixes: fe1b08803db6 ("ctl: improve API documentation for threshold level operations")
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 src/control/control.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/control/control.c b/src/control/control.c
index 6c00b8e..422582d 100644
--- a/src/control/control.c
+++ b/src/control/control.c
@@ -912,7 +912,7 @@ static int snd_ctl_tlv_do(snd_ctl_t *ctl, int op_flag,
 }
 
 /**
- * \brief Set given data to an element as threshold level.
+ * \brief Read structured data from an element set to given buffer.
  * \param ctl A handle of backend module for control interface.
  * \param id ID of an element.
  * \param tlv An array with members of unsigned int type.
@@ -940,7 +940,7 @@ int snd_ctl_elem_tlv_read(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 }
 
 /**
- * \brief Set given data to an element as threshold level.
+ * \brief Write structured data from given buffer to an element set.
  * \param ctl A handle of backend module for control interface.
  * \param id ID of an element.
  * \param tlv An array with members of unsigned int type. The second member
@@ -957,7 +957,7 @@ int snd_ctl_elem_tlv_write(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 }
 
 /**
- * \brief Set given data to an element as threshold level.
+ * \brief Process structured data from given buffer for an element set.
  * \param ctl A handle of backend module for control interface.
  * \param id ID of an element.
  * \param tlv An array with members of unsigned int type. The second member
-- 
2.7.4

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

* [PATCH v2 4/5] ctl: improve documentation about TLV-related APIs
  2016-09-28 23:57 [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes Takashi Sakamoto
                   ` (2 preceding siblings ...)
  2016-09-28 23:57 ` [PATCH v2 3/5] ctl: improve API documentation " Takashi Sakamoto
@ 2016-09-28 23:57 ` Takashi Sakamoto
  2016-09-28 23:57 ` [PATCH v2 5/5] ctl: correct documentation about TLV feature Takashi Sakamoto
  2016-09-30 15:13 ` [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes Takashi Iwai
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Sakamoto @ 2016-09-28 23:57 UTC (permalink / raw)
  To: clemens, tiwai, perex; +Cc: alsa-devel

The documentation gives no hints to users about format of TLV data.

This commit add hints to construct/parse the information.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 src/control/control.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/control/control.c b/src/control/control.c
index 422582d..40ee9b7 100644
--- a/src/control/control.c
+++ b/src/control/control.c
@@ -918,6 +918,13 @@ static int snd_ctl_tlv_do(snd_ctl_t *ctl, int op_flag,
  * \param tlv An array with members of unsigned int type.
  * \param tlv_size The length of the array.
  * \return 0 on success otherwise a negative error code
+ *
+ * The format of an array of \a tlv argument is:
+ *   tlv[0]:   Type. One of SND_CTL_TLVT_XXX.
+ *   tlv[1]:   Length. The length of value in units of byte.
+ *   tlv[2..]: Value. Depending on the type.
+ *
+ * Details are described in <sound/tlv.h>.
  */
 int snd_ctl_elem_tlv_read(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 			  unsigned int *tlv, unsigned int tlv_size)
@@ -948,6 +955,13 @@ int snd_ctl_elem_tlv_read(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
  * \retval 0 on success
  * \retval >0 on success when value was changed
  * \retval <0 a negative error code
+ *
+ * The format of an array of \a tlv argument is:
+ *   tlv[0]:   Type. One of SND_CTL_TLVT_XXX.
+ *   tlv[1]:   Length. The length of value in units of byte.
+ *   tlv[2..]: Value. Depending on the type.
+ *
+ * Details are described in <sound/tlv.h>.
  */
 int snd_ctl_elem_tlv_write(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 			   const unsigned int *tlv)
@@ -965,6 +979,13 @@ int snd_ctl_elem_tlv_write(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
  * \retval 0 on success
  * \retval >0 on success when value was changed
  * \retval <0 a negative error code
+ *
+ * The format of an array of \a tlv argument is:
+ *   tlv[0]:   Type. One of SND_CTL_TLVT_XXX.
+ *   tlv[1]:   Length. The length of value in units of byte.
+ *   tlv[2..]: Value. Depending on the type.
+ *
+ * Details are described in <sound/tlv.h>.
  */
 int snd_ctl_elem_tlv_command(snd_ctl_t *ctl, const snd_ctl_elem_id_t *id,
 			     const unsigned int *tlv)
-- 
2.7.4

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

* [PATCH v2 5/5] ctl: correct documentation about TLV feature
  2016-09-28 23:57 [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes Takashi Sakamoto
                   ` (3 preceding siblings ...)
  2016-09-28 23:57 ` [PATCH v2 4/5] ctl: improve documentation about TLV-related APIs Takashi Sakamoto
@ 2016-09-28 23:57 ` Takashi Sakamoto
  2016-09-30 15:13 ` [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes Takashi Iwai
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Sakamoto @ 2016-09-28 23:57 UTC (permalink / raw)
  To: clemens, tiwai, perex; +Cc: alsa-devel

>From my misunderstanding, some explanations are wrong. This commit
corrects them.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 src/control/control.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/control/control.c b/src/control/control.c
index 40ee9b7..134ba4c 100644
--- a/src/control/control.c
+++ b/src/control/control.c
@@ -59,27 +59,27 @@ elements included in the element set.
 When the value of member is changed, corresponding events are transferred to
 userspace applications. The applications should subscribe any events in advance.
 
-\section tlv_blob Thredshold level and arbitrary data
+\section tlv_blob Supplemental data for elements in an element set
 
-TLV feature is designed to transfer data about threshold level between a driver
-and any userspace applications. The data is for an element set.
+TLV feature is designed to transfer data in a shape of Type/Length/Value,
+between a driver and any userspace applications. The main purpose is to attach
+supplement information for elements to an element set; e.g. dB range.
 
 At first, this feature was implemented to add pre-defined data readable to
 userspace applications. Soon, it was extended to handle several operations;
 read, write and command. The original implementation remains as the read
 operation. The command operation allows drivers to have own implementations
-against requests from userspace applications. As of 2016, simple write operation
-is not supported yet.
+against requests from userspace applications.
 
 This feature was introduced to ALSA control feature in 2006, at commit
 c7a0708a2362, corresponding to a series of work for Linux kernel (42750b04c5ba
 and 8aa9b586e420).
 
-This feature can transfer arbitrary data in a shape of an array with members of
-unsigned int type, therefore it can be used to deliver quite large arbitrary
-data from userspace to in-kernel drivers via ALSA control character device.
-Focusing on this nature, some in-kernel implementations utilize this feature for
-I/O operations.
+There's no limitation about maximum size of the data, therefore it can be used
+to deliver quite large arbitrary data from userspace to in-kernel drivers via
+ALSA control character device. Focusing on this nature, as of 2016, some
+in-kernel implementations utilize this feature for I/O operations. This is
+against the original design.
 */
 
 #include <stdio.h>
-- 
2.7.4

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

* Re: [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes
  2016-09-28 23:57 [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes Takashi Sakamoto
                   ` (4 preceding siblings ...)
  2016-09-28 23:57 ` [PATCH v2 5/5] ctl: correct documentation about TLV feature Takashi Sakamoto
@ 2016-09-30 15:13 ` Takashi Iwai
  5 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2016-09-30 15:13 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Thu, 29 Sep 2016 01:57:19 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> This patchset updates my former one:
> [alsa-devel] [PATCH 0/4][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes
> http://mailman.alsa-project.org/pipermail/alsa-devel/2016-September/112853.html
> 
> In this month, UAPI header of 4.9-pre kernel newly include existent macros
> related to TLV operation for user land. This is to allow applications to
> register TLV information to element set which they added.
> 
> This patchset is for related changes in ALSA user land library. The macros
> are put into 'include/sound/tlv.h'. However corresponding macros are not
> put into 'include/control.h' yet because currently I have no better idea
> for library APIs to construct tlv information.
> 
> This patchset also improves some corrections for documentation which
> committed with my misunderstandings.

Applied all five patches now.  Thanks.


Takashi

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

end of thread, other threads:[~2016-09-30 15:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-28 23:57 [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes Takashi Sakamoto
2016-09-28 23:57 ` [PATCH v2 1/5] Update include/sound/tlv.h from 4.9-pre kernel uapi Takashi Sakamoto
2016-09-28 23:57 ` [PATCH v2 2/5] test: use actual information for TLV operation Takashi Sakamoto
2016-09-28 23:57 ` [PATCH v2 3/5] ctl: improve API documentation " Takashi Sakamoto
2016-09-28 23:57 ` [PATCH v2 4/5] ctl: improve documentation about TLV-related APIs Takashi Sakamoto
2016-09-28 23:57 ` [PATCH v2 5/5] ctl: correct documentation about TLV feature Takashi Sakamoto
2016-09-30 15:13 ` [PATCH v2 0/5][alsa-lib] ctl: sync UAPI header for TLV-related macros and apply misc changes 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.