On Tue, 27 Mar 2018 20:39:45 +0200,
Ranjani Sridharan wrote:
>
> This patch adds macros for accessing 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>
The subject is wrongly prefixed. This is ALSA core API stuff, not
about ASoC.
And about the change...
> --- a/include/sound/tlv.h
> +++ b/include/sound/tlv.h
> @@ -49,6 +49,11 @@
>
> #define TLV_DB_GAIN_MUTE SNDRV_CTL_TLVD_DB_GAIN_MUTE
>
> +#define TLV_ITEM_TYPE SNDRV_CTL_TLVD_ITEM_TYPE
> +#define TLV_ITEM_LEN SNDRV_CTL_TLVD_ITEM_LEN
> +#define TLV_DB_MIN SNDRV_CTL_TLVD_DB_SCALE_MIN
> +#define TLV_DB_MUTE_AND_STEP SNDRV_CTL_TLVD_DB_SCALE_MUTE_AND_STEP
What's the reason of these redundant redefinitions?
> --- a/include/uapi/sound/tlv.h
> +++ b/include/uapi/sound/tlv.h
> @@ -60,6 +60,13 @@
> unsigned int name[] = { \
> SNDRV_CTL_TLVD_DB_SCALE_ITEM(min, step, mute) \
> }
> +/* Accessor macros for TLV type and len */
> +#define SNDRV_CTL_TLVD_ITEM_TYPE 0
> +#define SNDRV_CTL_TLVD_ITEM_LEN 1
> +
> +/* Accessor macros for min, mute and step values from dB scale type TLV */
> +#define SNDRV_CTL_TLVD_DB_SCALE_MIN 2
> +#define SNDRV_CTL_TLVD_DB_SCALE_MUTE_AND_STEP 3
These are no "macros" but constants. And, they use the same prefix
SNDRV_CTL_TLVD_* as other existing macros. SNDTV_CTL_TLVD_*() is a
macro to define/expand a TLV descriptor. It's not for defining the
offset value as added in the patch.
That is, better to choose another unique prefix to distinguish the
definitions clearly.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
