[alsa-devel] [PATCH v4] ALSA: core api: define offsets for TLV items

[Takashi Sakamoto]

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 at 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 at 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