On Thu, 2018-05-10 at 03:06 +0900, Takashi Sakamoto wrote: > Hi, > > On May 8 2018 06:04, 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@xxxxxxxxxxxxxxx > > > > > --- > > > > Notes: > > v3: fix prefix for offset definitions > > > > 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..13f425c2abbd 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_TLVO_TYPE 0 > > +#define SNDRV_CTL_TLVO_LEN 1 > > + > > +/* 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 > > + > > #endif > > You added these macros in the end of this file. In my opinion, this > is > inconvenient for future when adding more macros for new types. It's > better to place the additional macro to places convenient to find > relevant lines such that: Thanks, Takashi. I've fixed the placement in v4. > - 'SNDRV_CTL_TLVO_TYPE' and 'SNDRV_CTL_TLVO_LEN' following > to comments about layout of TLV data. > - 'SNDRV_CTL_TLVO_DB_SCALE_MIN' and > 'SNDRV_CTL_TLVO_DB_SCALE_MUTE_AND_STEP' following to > 'SNDRV_CTL_TLVO_DB_SCALE_MUTE_AND_STEP'. > > diff --git a/include/uapi/sound/tlv.h b/include/uapi/sound/tlv.h > index be5371f09a62..b5f45b13077e 100644 > --- a/include/uapi/sound/tlv.h > +++ b/include/uapi/sound/tlv.h > @@ -37,6 +37,9 @@ > * block_length = (length + (sizeof(unsigned int) - 1)) & > * ~(sizeof(unsigned int) - 1)) .... > */ > +#define SNDRV_CTL_TLVO_TYPE 0 > +#define SNDRV_CTL_TLVO_LEN 1 > + > #define SNDRV_CTL_TLVD_ITEM(type, ...) \ > (type), SNDRV_CTL_TLVD_LENGTH(__VA_ARGS__), __VA_ARGS__ > #define SNDRV_CTL_TLVD_LENGTH(...) \ > @@ -61,6 +64,9 @@ > SNDRV_CTL_TLVD_DB_SCALE_ITEM(min, step, mute) \ > } > > +#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)) > > 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. > > > Regards > > Takashi Sakamoto _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
