Re: [PATCH v1 2/8] lsusb: Add declarative definitions for UAC1 and UAC2 descriptors.

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

 



On Tue, Dec 05, 2017 at 04:14:25PM +0000, Michael Drake wrote:
> These descriptor definitions descibe how raw descriptor data
> should be interpreted.
> ---
>  desc-defs.c | 647 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  desc-defs.h | 153 ++++++++++++++
>  2 files changed, 800 insertions(+)

Don't we need to add the file to the build here?

>  create mode 100644 desc-defs.c
>  create mode 100644 desc-defs.h
> 
> diff --git a/desc-defs.c b/desc-defs.c
> new file mode 100644
> index 0000000..6bc558e
> --- /dev/null
> +++ b/desc-defs.c
> @@ -0,0 +1,647 @@
> +/*****************************************************************************/
> +
> +/*
> + *      desc-defs.c  --  USB descriptor definitions
> + *
> + *      Copyright (C) 2017 Michael Drake <michael.drake@xxxxxxxxxxxxxxx>
> + *
> + *      This program is free software; you can redistribute it and/or modify
> + *      it under the terms of the GNU General Public License as published by
> + *      the Free Software Foundation; either version 2 of the License, or
> + *      (at your option) any later version.
> + *
> + *      This program is distributed in the hope that it will be useful,
> + *      but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *      MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *      GNU General Public License for more details.
> + *
> + */
> +
> +/*****************************************************************************/

Wow that's a large boiler plate of a header.  Not your fault, you are
copying lsusb.c, I'll fix that up later :)

> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif

Why #ifdef this?

> +
> +#include <stdbool.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "desc-defs.h"
> +
> +static const char * const uac2_interface_header_bmcontrols[] = {
> +	"Latency control",
> +	NULL
> +};

All of these arrays have to be in the specific order, right?  Do we need
a comment somewhere saying this?

> +const struct desc * const desc_audio_ac_processing_unit[3] = {
> +	desc_audio_1_ac_processing_unit,
> +	desc_audio_2_ac_processing_unit,
> +	NULL, /* UAC3 not implemented yet */
> +};
> +
> +
> +/** UAC1: 4.3.2.5 Feature Unit Descriptor; Table 4-7. */
> +static const struct desc desc_audio_1_ac_feature_unit[] = {
> +	{ .field = "bUnitID",      .size = 1,                    .type = DESC_NUMBER },
> +	{ .field = "bSourceID",    .size = 1,                    .type = DESC_CONSTANT },
> +	{ .field = "bControlSize", .size = 1,                    .type = DESC_NUMBER },
> +	{ .field = "bmaControls",  .size_field = "bControlSize", .type = DESC_BMCONTROL_1,
> +			.bmcontrol = uac_feature_unit_bmcontrols, .array = { .array = true } },
> +	{ .field = "iFeature",     .size = 1,                    .type = DESC_STR_DESC_INDEX },
> +	{ .field = NULL }
> +};
> +/** UAC2: 4.7.2.8 Feature Unit Descriptor; Table 4-13. */
> +static const struct desc desc_audio_2_ac_feature_unit[] = {
> +	{ .field = "bUnitID",     .size = 1, .type = DESC_NUMBER },
> +	{ .field = "bSourceID",   .size = 1, .type = DESC_CONSTANT },
> +	{ .field = "bmaControls", .size = 4, .type = DESC_BMCONTROL_2,
> +			.bmcontrol = uac_feature_unit_bmcontrols, .array = { .array = true } },
> +	{ .field = "iFeature",    .size = 1, .type = DESC_STR_DESC_INDEX },
> +	{ .field = NULL }
> +};
> +const struct desc * const desc_audio_ac_feature_unit[3] = {
> +	desc_audio_1_ac_feature_unit,
> +	desc_audio_2_ac_feature_unit,
> +	NULL, /* UAC3 not implemented yet */
> +};
> +
> +

Just put 1 blank line between everything, makes it simpler to manage
over time and to read :)

> +/** UAC1: 4.3.2.7 Extension Unit Descriptor; Table 4-15. */
> +static const struct desc desc_audio_1_ac_extension_unit[] = {
> +	{ .field = "bUnitID",        .size = 1, .type = DESC_NUMBER },
> +	{ .field = "wExtensionCode", .size = 2, .type = DESC_CONSTANT },
> +	{ .field = "bNrInPins",      .size = 1, .type = DESC_NUMBER },
> +	{ .field = "baSourceID",     .size = 1, .type = DESC_NUMBER,
> +			.array = { .array = true, .length_field1 = "bNrInPins" } },
> +	{ .field = "bNrChannels",    .size = 1, .type = DESC_NUMBER },
> +	{ .field = "wChannelConfig", .size = 2, .type = DESC_BITMAP_STRINGS,
> +			.bitmap_strings = { .strings = uac1_channel_names, .count = 12 } },
> +	{ .field = "iChannelNames",  .size = 1, .type = DESC_STR_DESC_INDEX },
> +	{ .field = "bControlSize",   .size = 1, .type = DESC_NUMBER },
> +	{ .field = "bmControls",     .size = 1, .type = DESC_BITMAP,
> +			.array = { .array = true, .length_field1 = "bControlSize" } },
> +	{ .field = "iExtension",     .size = 1, .type = DESC_STR_DESC_INDEX },
> +	{ .field = NULL }
> +};
> +/** UAC2: 4.7.2.12 Extension Unit Descriptor; Table 4-24. */
> +static const struct desc desc_audio_2_ac_extension_unit[] = {
> +	{ .field = "bUnitID",         .size = 1, .type = DESC_NUMBER },
> +	{ .field = "wExtensionCode",  .size = 2, .type = DESC_CONSTANT },
> +	{ .field = "bNrInPins",       .size = 1, .type = DESC_NUMBER },
> +	{ .field = "baSourceID",      .size = 1, .type = DESC_NUMBER,
> +			.array = { .array = true, .length_field1 = "bNrInPins" } },
> +	{ .field = "bNrChannels",     .size = 1, .type = DESC_NUMBER },
> +	{ .field = "bmChannelConfig", .size = 4, .type = DESC_BITMAP_STRINGS,
> +			.bitmap_strings = { .strings = uac2_channel_names, .count = 26 } },
> +	{ .field = "iChannelNames",   .size = 1, .type = DESC_STR_DESC_INDEX },
> +	{ .field = "bmControls",      .size = 1, .type = DESC_BMCONTROL_2,
> +			.bmcontrol = uac2_extension_unit_bmcontrols },
> +	{ .field = "iExtension",      .size = 1, .type = DESC_STR_DESC_INDEX },
> +	{ .field = NULL }
> +};
> +const struct desc * const desc_audio_ac_extension_unit[3] = {
> +	desc_audio_1_ac_extension_unit,
> +	desc_audio_2_ac_extension_unit,
> +	NULL, /* UAC3 not implemented yet */
> +};
> +
> +
> +static const char * const uac2_clk_src_bmattr[] = {
> +	"External",
> +	"Internal fixed",
> +	"Internal variable",
> +	"Internal programmable"
> +};
> +static const char * const uac3_clk_src_bmattr[] = {
> +	"External",
> +	"Internal",
> +	"(asynchronous)",
> +	"(synchronized to SOF)"
> +};
> +
> +/** Special rendering function for UAC2 clock source bmAttributes */
> +static void desc_snowflake_dump_uac2_clk_src_bmattr(
> +		unsigned long long value,
> +		unsigned int indent)
> +{
> +	printf(" %s clock %s\n",
> +			uac2_clk_src_bmattr[value & 0x3],
> +			(value & 0x4) ? uac3_clk_src_bmattr[3] : "");
> +}
> +
> +/** UAC2: 4.7.2.1 Clock Source Descriptor; Table 4-6. */
> +static const struct desc desc_audio_2_ac_clock_source[] = {
> +	{ .field = "bClockID",       .size = 1, .type = DESC_CONSTANT },
> +	{ .field = "bmAttributes",   .size = 1, .type = DESC_SNOWFLAKE,
> +			.snowflake = desc_snowflake_dump_uac2_clk_src_bmattr },
> +	{ .field = "bmControls",     .size = 1, .type = DESC_BMCONTROL_2,
> +			.bmcontrol = uac2_clock_source_bmcontrols },
> +	{ .field = "bAssocTerminal", .size = 1, .type = DESC_CONSTANT },
> +	{ .field = "iClockSource",   .size = 1, .type = DESC_STR_DESC_INDEX },
> +	{ .field = NULL }
> +};
> +const struct desc * const desc_audio_ac_clock_source[3] = {
> +	NULL, /* UAC1 not supported */
> +	desc_audio_2_ac_clock_source,
> +	NULL, /* UAC3 not implemented yet */
> +};
> +
> +
> +/** UAC2: 4.7.2.2 Clock Selector Descriptor; Table 4-7. */
> +static const struct desc desc_audio_2_ac_clock_selector[] = {
> +	{ .field = "bClockID",       .size = 1, .type = DESC_NUMBER },
> +	{ .field = "bNrInPins",      .size = 1, .type = DESC_NUMBER },
> +	{ .field = "baCSourceID",    .size = 1, .type = DESC_NUMBER,
> +			.array = { .array = true, .length_field1 = "bNrInPins" } },
> +	{ .field = "bmControls",     .size = 1, .type = DESC_BMCONTROL_2,
> +			.bmcontrol = uac2_clock_selector_bmcontrols },
> +	{ .field = "iClockSelector", .size = 1, .type = DESC_STR_DESC_INDEX },
> +	{ .field = NULL }
> +};
> +const struct desc * const desc_audio_ac_clock_selector[3] = {
> +	NULL, /* UAC1 not supported */
> +	desc_audio_2_ac_clock_selector,
> +	NULL, /* UAC3 not implemented yet */
> +};
> +
> +
> +/** UAC2: 4.7.2.3 Clock Multiplier Descriptor; Table 4-8. */
> +static const struct desc desc_audio_2_ac_clock_multiplier[] = {
> +	{ .field = "bClockID",         .size = 1, .type = DESC_CONSTANT },
> +	{ .field = "bCSourceID",       .size = 1, .type = DESC_NUMBER },
> +	{ .field = "bmControls",       .size = 1, .type = DESC_BMCONTROL_2,
> +			.bmcontrol = uac2_clock_multiplier_bmcontrols },
> +	{ .field = "iClockMultiplier", .size = 1, .type = DESC_STR_DESC_INDEX },
> +	{ .field = NULL }
> +};
> +const struct desc * const desc_audio_ac_clock_multiplier[3] = {
> +	NULL, /* UAC1 not supported */
> +	desc_audio_2_ac_clock_multiplier,
> +	NULL, /* UAC3 not implemented yet */
> +};
> +
> +
> +/** UAC2: 4.7.2.9 Sampling Rate Converter Descriptor; Table 4-14. */
> +static const struct desc desc_audio_2_ac_sample_rate_converter[] = {
> +	{ .field = "bUnitID",       .size = 1, .type = DESC_CONSTANT },
> +	{ .field = "bSourceID",     .size = 1, .type = DESC_CONSTANT },
> +	{ .field = "bCSourceInID",  .size = 1, .type = DESC_CONSTANT },
> +	{ .field = "bCSourceOutID", .size = 1, .type = DESC_CONSTANT },
> +	{ .field = "iSRC",          .size = 1, .type = DESC_STR_DESC_INDEX },
> +	{ .field = NULL }
> +};
> +const struct desc * const desc_audio_ac_sample_rate_converter[3] = {
> +	NULL, /* UAC1 not supported */
> +	desc_audio_2_ac_sample_rate_converter,
> +	NULL, /* UAC3 not implemented yet */
> +};
> +
> +
> +static const char * const uac2_as_interface_bmcontrols[] = {
> +	"Active Alternate Setting",
> +	"Valid Alternate Setting",
> +	NULL
> +};
> +static const char * const audio_data_format_type_i[] = {
> +	"TYPE_I_UNDEFINED",
> +	"PCM",
> +	"PCM8",
> +	"IEEE_FLOAT",
> +	"ALAW",
> +	"MULAW"
> +};
> +static const char * const audio_data_format_type_ii[] = {
> +	"TYPE_II_UNDEFINED",
> +	"MPEG",
> +	"AC-3"
> +};
> +static const char * const audio_data_format_type_iii[] = {
> +	"TYPE_III_UNDEFINED",
> +	"IEC1937_AC-3",
> +	"IEC1937_MPEG-1_Layer1",
> +	"IEC1937_MPEG-Layer2/3/NOEXT",
> +	"IEC1937_MPEG-2_EXT",
> +	"IEC1937_MPEG-2_Layer1_LS",
> +	"IEC1937_MPEG-2_Layer2/3_LS"
> +};
> +
> +/** Special rendering function for UAC1 AS interface wFormatTag */
> +static void desc_snowflake_dump_uac1_as_interface_wformattag(
> +		unsigned long long value,
> +		unsigned int indent)
> +{
> +	const char *format_string = "undefined";
> +
> +	if (value <= 5) {

Isn't that 5 a #define somewhere?

> +		format_string = audio_data_format_type_i[value];
> +
> +	} else if (value >= 0x1000 && value <= 0x1002) {
> +		format_string = audio_data_format_type_ii[value & 0xfff];
> +
> +	} else if (value >= 0x2000 && value <= 0x2006) {

Same for 0x1000, 0x1002, 0x2000, and 0x2006, aren't they defined
somewhere?

> --- /dev/null
> +++ b/desc-defs.h
> @@ -0,0 +1,153 @@
> +/*****************************************************************************/
> +
> +/*
> + *      desc-defs.h  --  USB descriptor definitions
> + *
> + *      Copyright (C) 2017 Michael Drake <michael.drake@xxxxxxxxxxxxxxx>
> + *
> + *      This program is free software; you can redistribute it and/or modify
> + *      it under the terms of the GNU General Public License as published by
> + *      the Free Software Foundation; either version 2 of the License, or
> + *      (at your option) any later version.
> + *
> + *      This program is distributed in the hope that it will be useful,
> + *      but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *      MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *      GNU General Public License for more details.
> + *
> + */
> +
> +/*****************************************************************************/
> +
> +#ifndef _DESC_DEFS_H
> +#define _DESC_DEFS_H
> +
> +/* ---------------------------------------------------------------------- */
> +
> +/**
> + * Descriptor field value type.
> + *
> + * Note that there are more types here than exist in the descriptor definitions
> + * in the specifications.  This is because the type here is used to do `lsusb`
> + * specific rendering of certain fields.
> + *
> + * Note that the use of certain types mandates the setting of various entries
> + * in the type-specific anonymous union in `struct desc`.
> + */
> +enum desc_type {
> +	DESC_CONSTANT,       /** Plain numerical value; no annotation. */
> +	DESC_NUMBER,         /** Plain numerical value; no annotation. */
> +	DESC_NUMBER_POSTFIX, /**< Number with a postfix string. */
> +	DESC_BITMAP,         /**< Plain hex rendered value; no annotation. */
> +	DESC_BCD,            /**< Binary coded decimal */
> +	DESC_BMCONTROL_1,    /**< UAC1 style bmControl field */
> +	DESC_BMCONTROL_2,    /**< UAC2/UAC3 style bmControl field */
> +	DESC_STR_DESC_INDEX, /**< String index. */
> +	DESC_TERMINAL_STR,   /**< Audio terminal string. */
> +	DESC_BITMAP_STRINGS, /**< Bitfield with string per bit. */
> +	DESC_NUMBER_STRINGS, /**< Use for enum-style value to string. */
> +	DESC_SNOWFLAKE,  /**< Value with custom annotation callback function. */
> +};
> +
> +/**
> + * Callback function for the DESC_SNOWFLAKE descriptor field value type.
> + *
> + * This is used when some special rendering of the value is required, which
> + * is specific to the field in question, so no generic type's rendering is
> + * suitable.
> + *
> + * The core descriptor dumping code will have already dumped the numerical
> + * value for the field, but not the trailing newline character.  It is up
> + * to the callback function to ensure it always finishes by writing a '\n'
> + * character to stdout.
> + *
> + * \param[in] value    The value to dump a human-readable representation of.
> + * \param[in] indent   The current indent level.
> + */

What is the style you are using for this formatting of the
documentation?  I don't think we use anything today, is it really
needed?

> +typedef void (*desc_snowflake_dump_fn)(
> +		unsigned long long value,
> +		unsigned int indent);
> +
> +
> +/**
> + * Descriptor field definition.
> + *
> + * Whole descriptors can be defined as NULL terminated arrays of these
> + * structures.
> + */
> +struct desc {
> +	const char *field;   /**< Field's name */
> +	unsigned int size;   /**< Byte size of field, if (size_field == NULL) */
> +	const char *size_field; /**< Name of field specifying field size. */
> +	enum desc_type type; /**< Field's value type. */
> +
> +	/** Anonymous union containing type-specific data. */
> +	union {
> +		/**
> +		 * Corresponds to types DESC_BMCONTROL_1 and DESC_BMCONTROL_2.
> +		 *
> +		 * Must be a NULL terminated array of '\0' terminated strings.
> +		 */
> +		const char * const *bmcontrol;
> +		/** Corresponds to type DESC_BITMAP_STRINGS */
> +		struct {
> +			/** Must contain '\0' terminated strings. */
> +			const char * const *strings;
> +			/** Number of strings in strings array. */
> +			unsigned int count;
> +		} bitmap_strings;
> +		/**
> +		 * Corresponds to type DESC_NUMBER_STRINGS.
> +		 *
> +		 * Must be a NULL terminated array of '\0' terminated strings.
> +		 */
> +		const char * const *number_strings;
> +		/**
> +		 * Corresponds to type DESC_NUMBER_POSTFIX.
> +		 *
> +		 * Must be a '\0' terminated string.
> +		 */
> +		const char *number_postfix;
> +		/**
> +		 * Corresponds to type DESC_SNOWFLAKE.
> +		 *
> +		 * Callback function called to annotate snowflake value type.
> +		 */
> +		desc_snowflake_dump_fn snowflake;
> +	};
> +
> +	/** Grouping of array-specific fields. */
> +	struct {
> +		bool array; /**< True if entry is an array. */
> +		bool bits;  /**< True if array length is specified in bits */

You use 1 for these two values, shouldn't you use 'true' instead?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux