diff mbox

[v4] ALSA: core api: define offsets for TLV items

Message ID 20180510005304.27903-1-ranjani.sridharan@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ranjani Sridharan May 10, 2018, 12:53 a.m. UTC
Currently, there are no pre-defined accessors for the elements
in topology TLV data. In the absence of such offsets, the
tlv data will have to be decoded using hardwired offset
numbers 0-N depending on the type of TLV. This patch defines
accessor offsets for the type, length, min and mute/step items
in TLV data for DB_SCALE type tlv's. These will be used by drivers to
decode the TLV data while loading topology thereby improving
code readability. The type and len offsets are common for all TLV
types. The min and step/mute offsets are specific to DB_SCALE tlv type.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---

Notes:
    v4: change placement of tlv accessor offsets

 include/uapi/sound/tlv.h | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Takashi Sakamoto May 10, 2018, 1:48 a.m. UTC | #1
On May 10 2018 09:53, Ranjani Sridharan wrote:
> Currently, there are no pre-defined accessors for the elements
> in topology TLV data. In the absence of such offsets, the
> tlv data will have to be decoded using hardwired offset
> numbers 0-N depending on the type of TLV. This patch defines
> accessor offsets for the type, length, min and mute/step items
> in TLV data for DB_SCALE type tlv's. These will be used by drivers to
> decode the TLV data while loading topology thereby improving
> code readability. The type and len offsets are common for all TLV
> types. The min and step/mute offsets are specific to DB_SCALE tlv type.
> 
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
> 
> Notes:
>      v4: change placement of tlv accessor offsets
> 
>   include/uapi/sound/tlv.h | 8 ++++++++
>   1 file changed, 8 insertions(+)

On May 10 2018 09:40, Ranjani Sridharan wrote:
 >>
 >> I have another concern that 'O' is hard to distinguish from 'D' as a
 >> quick glance and can easily leads developers and reviewers to
 >> misread 'SNDRV_CTL_TLVD_XXX' and 'SNDRV_CTL_TLVO_XXX'. Of cource, I
 >> know that you intend to use 'O' to express 'offset' and it's
 >> logically
 >> valid. My concern is just for visual.
 > I've left the prefix SNDRV_CTL_TLVO for lack of a better one and
 > adding OFFSET in the prefix would make the names very long. Hope that
 > is OK.

I agree with it.

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

> diff --git a/include/uapi/sound/tlv.h b/include/uapi/sound/tlv.h
> index be5371f09a62..e3437e96519a 100644
> --- a/include/uapi/sound/tlv.h
> +++ b/include/uapi/sound/tlv.h
> @@ -42,6 +42,10 @@
>   #define SNDRV_CTL_TLVD_LENGTH(...) \
>   	((unsigned int)sizeof((const unsigned int[]) { __VA_ARGS__ }))
>   
> +/* Accessor offsets for TLV data items */
> +#define SNDRV_CTL_TLVO_TYPE		0
> +#define SNDRV_CTL_TLVO_LEN		1
> +
>   #define SNDRV_CTL_TLVD_CONTAINER_ITEM(...) \
>   	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_CONTAINER, __VA_ARGS__)
>   #define SNDRV_CTL_TLVD_DECLARE_CONTAINER(name, ...) \
> @@ -61,6 +65,10 @@
>   		SNDRV_CTL_TLVD_DB_SCALE_ITEM(min, step, mute) \
>   	}
>   
> +/* Accessor offsets for min, mute and step items in dB scale type TLV */
> +#define SNDRV_CTL_TLVO_DB_SCALE_MIN		2
> +#define SNDRV_CTL_TLVO_DB_SCALE_MUTE_AND_STEP	3
> +
>   /* 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))

Later, I'll post patchset for alsa-lib to add these macros, but it'll be 
done after subsystem maintainer is back from his absence.


Thanks


Takashi Sakamoto
Takashi Iwai May 13, 2018, 10:32 a.m. UTC | #2
On Thu, 10 May 2018 02:53:04 +0200,
Ranjani Sridharan wrote:
> 
> Currently, there are no pre-defined accessors for the elements
> in topology TLV data. In the absence of such offsets, the
> tlv data will have to be decoded using hardwired offset
> numbers 0-N depending on the type of TLV. This patch defines
> accessor offsets for the type, length, min and mute/step items
> in TLV data for DB_SCALE type tlv's. These will be used by drivers to
> decode the TLV data while loading topology thereby improving
> code readability. The type and len offsets are common for all TLV
> types. The min and step/mute offsets are specific to DB_SCALE tlv type.
> 
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> ---
> 
> Notes:
>     v4: change placement of tlv accessor offsets

Applied now to for-next branch (with Sakamoto-san's reviewed-by tag).

But at the next time, please put the maintainers and some other
relevant people in Cc.


thanks,

Takashi
diff mbox

Patch

diff --git a/include/uapi/sound/tlv.h b/include/uapi/sound/tlv.h
index be5371f09a62..e3437e96519a 100644
--- a/include/uapi/sound/tlv.h
+++ b/include/uapi/sound/tlv.h
@@ -42,6 +42,10 @@ 
 #define SNDRV_CTL_TLVD_LENGTH(...) \
 	((unsigned int)sizeof((const unsigned int[]) { __VA_ARGS__ }))
 
+/* Accessor offsets for TLV data items */
+#define SNDRV_CTL_TLVO_TYPE		0
+#define SNDRV_CTL_TLVO_LEN		1
+
 #define SNDRV_CTL_TLVD_CONTAINER_ITEM(...) \
 	SNDRV_CTL_TLVD_ITEM(SNDRV_CTL_TLVT_CONTAINER, __VA_ARGS__)
 #define SNDRV_CTL_TLVD_DECLARE_CONTAINER(name, ...) \
@@ -61,6 +65,10 @@ 
 		SNDRV_CTL_TLVD_DB_SCALE_ITEM(min, step, mute) \
 	}
 
+/* Accessor offsets for min, mute and step items in dB scale type TLV */
+#define SNDRV_CTL_TLVO_DB_SCALE_MIN		2
+#define SNDRV_CTL_TLVO_DB_SCALE_MUTE_AND_STEP	3
+
 /* 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))