Re: [PATCH v2] ALSA: core api: define offsets for TLV items

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mar 29 2018 02:09, Ranjani Sridharan wrote:
This patch defines accessor offsets for the type, length, min,
mute and step items in TLV data. These will be used by drivers to
extract the TLV data while loading topology.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx>
---
V2: removed redundant redefinitions and updated prefix
---
  include/uapi/sound/tlv.h | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/include/uapi/sound/tlv.h b/include/uapi/sound/tlv.h
index be5371f09a62..1fd786a5e57b 100644
--- a/include/uapi/sound/tlv.h
+++ b/include/uapi/sound/tlv.h
@@ -98,4 +98,12 @@
#define SNDRV_CTL_TLVD_DB_GAIN_MUTE -9999999 +/* Accessor offsets for TLV data items */
+#define SNDRV_CTL_TLV_OFFSET_TYPE		0
+#define SNDRV_CTL_TLV_OFFSET_LEN		1

I suggest you to use these macros in declaration of 'SNDRV_CTL_TLVD_ITEM()' for self-described header. In short, I mean:

$ diff --git a/include/uapi/sound/tlv.h b/include/uapi/sound/tlv.h
index be5371f09a62..76598609f349 100644
--- a/include/uapi/sound/tlv.h
+++ b/include/uapi/sound/tlv.h
@@ -37,8 +37,15 @@
  *        block_length = (length + (sizeof(unsigned int) - 1)) &
  *                       ~(sizeof(unsigned int) - 1)) ....
  */
+
+/* Accessor offsets for TLV data items */
+#define SNDRV_CTL_TLV_OFFSET_TYPE      0
+#define SNDRV_CTL_TLV_OFFSET_LEN       1
+
 #define SNDRV_CTL_TLVD_ITEM(type, ...) \
-       (type), SNDRV_CTL_TLVD_LENGTH(__VA_ARGS__), __VA_ARGS__
+       [SNDRV_CTL_TLV_OFFSET_TYPE] = (type), \
+       [SNDRV_CTL_TLV_OFFSET_LEN] = SNDRV_CTL_TLVD_LENGTH(__VA_ARGS__), \
+       __VA_ARGS__
 #define SNDRV_CTL_TLVD_LENGTH(...) \
        ((unsigned int)sizeof((const unsigned int[]) { __VA_ARGS__ }))

+/* Accessor offsets for min, mute and step items in dB scale type TLV */
+#define SNDRV_CTL_TLV_OFFSET_DB_MIN		2
+#define SNDRV_CTL_TLV_OFFSET_DB_MUTE_AND_STEP	3
+
  #endif

I wonder why you add the above macros just for data of 'SNDRV_CTL_TLVT_DB_MINMAX_MUTE'. In fact, this header defines below types of TLV data.
 - SNDRV_CTL_TLVT_CONTAINER
 - SNDRV_CTL_TLVT_DB_SCALE
 - SNDRV_CTL_TLVT_DB_LINEAR
 - SNDRV_CTL_TLVT_DB_RANGE
 - SNDRV_CTL_TLVT_DB_MINMAX
 - SNDRV_CTL_TLVT_DB_MINMAX_MUTE

Would I request you to your reason not to add such offset macros for the others? At least, we should have a care for 'SNDRV_CTL_TLVT_DB_LINEAR' to keep consistency of defined macros.

Regards

Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux