USB Audio Class 2 has 2 Configurations with the same Index

Sub forums for various specialist XMOS applications. e.g. USB audio, motor control and robotics.
Post Reply
rsdio
Junior Member
Posts: 4
Joined: Wed Jan 15, 2014 11:05 pm
Contact:

USB Audio Class 2 has 2 Configurations with the same Index

Post by rsdio »

First of all, I'm working with a couple of different xCORE 200 boards and I'm seeing what must be an error in the USB Descriptors. The Device Descriptor says there are 2 Configurations, but both Configurations have the same bConfigurationValue - which is surely an illegal combination. When the Host sends a SetConfiguration message with a non-zero Configuration value, there must be a way to distinguish between the Configurations. For the example of a USB Audio Class Device with two Configurations, those values should be 1 and 2.

Perhaps the xCORE firmware was configured incorrectly when it was compiled, and that might explain why the Descriptors are conflicted. However, scanning through the firmware source, it seems that the bConfigurationValue fields are hard-coded when they should use macros or some other technique to adapt to the various combinations. i.e. The second bConfigurationValue should be 2 instead of 1.

Second, both Configuration structures are identical, which makes it seem pointless to have two of them. However, there is a comment in the source that seems to explain why there are two. In the file:

sw_usb_audio-[sw]_6.15.2rc1/sc_usb_audio/module_usb_audio/endpoint0/descriptors.h

/* Device Descriptor for Audio Class 2.0 (Assumes High-Speed ) */
USB_Descriptor_Device_t devDesc_Audio2 =
{
.bLength = sizeof(USB_Descriptor_Device_t),
.bDescriptorType = USB_DESCTYPE_DEVICE,
.bcdUSB = 0x0200,
.bDeviceClass = 0xEF,
.bDeviceSubClass = 0x02,
.bDeviceProtocol = 0x01,
.bMaxPacketSize0 = 64,
.idVendor = VENDOR_ID,
.idProduct = PID_AUDIO_2,
.bcdDevice = BCD_DEVICE,
.iManufacturer = offsetof(StringDescTable_t, vendorStr)/sizeof(char *),
.iProduct = offsetof(StringDescTable_t, productStr_Audio2)/sizeof(char *),
.iSerialNumber = 0,
.bNumConfigurations = 0x02 /* Set to 2 such that windows does not load composite driver */
};

I am worried by this comment because it implies that XMOS is violating the USB Specifications in order to whip Windows into shape with regard to the composite driver, and that seems like a really bad idea. Can anyone explain the reasoning behind this comment and the associated Descriptors? It seems like there must be a way to honor the USB Specifications and still have Windows work correctly.

I am also testing with macOS.

Brian Willoughby
Sound Consulting


User avatar
Ross
XCore Expert
Posts: 962
Joined: Thu Dec 10, 2009 9:20 pm
Location: Bristol, UK

Post by Ross »

rsdio wrote: I am worried by this comment because it implies that XMOS is violating the USB Specifications in order to whip Windows into shape with regard to the composite driver, and that seems like a really bad idea.
Agree, welcome to working with Windows ;) In all seriousness we were advised by experts in the field and it hasn't caused an issue for the last ~6 years with plenty of device in customers hands.

It was intentional to workaround an issue with the UAC1 class driver in Win XP. Since XP is now defunct it can probably be safely removed.
rsdio
Junior Member
Posts: 4
Joined: Wed Jan 15, 2014 11:05 pm
Contact:

Post by rsdio »

Ross wrote:
rsdio wrote: I am worried by this comment because it implies that XMOS is violating the USB Specifications in order to whip Windows into shape with regard to the composite driver, and that seems like a really bad idea.
Agree, welcome to working with Windows ;) In all seriousness we were advised by experts in the field and it hasn't caused an issue for the last ~6 years with plenty of device in customers hands.

It was intentional to workaround an issue with the UAC1 class driver in Win XP. Since XP is now defunct it can probably be safely removed.
Is there any reason that the two Configurations must have the same index? That's the only part that violates the USB Specifications.

In other words, even though the ideal would be to have a single configuration that describes the USB Audio features, if it's necessary to put a second configuration in there for Windows then why not make it legal?

To rephrase this again: Does XP require that the second Configuration have an invalid index? Or does XP merely require that there be a second Configuration of some kind? Does XP even care what values are in the additional configuration?

An easy way to test this would be to alter the second copy of the Configuration so that it has an index of 2 instead of 1, and then test to see whether XP still works as desired. If that's the case, then modifying your example firmware to honor the USB Specifications would be a benefit to all platforms.
rsdio
Junior Member
Posts: 4
Joined: Wed Jan 15, 2014 11:05 pm
Contact:

Post by rsdio »

To be specific, one of the Descriptors should have .bConfigurationValue = 0x02 (instead of 0x01)
Most likely, that should probably be the cfgDesc_Null descriptor.

Here is one of the Configurations from descriptor.h:

.Config =
{
.bLength = sizeof(USB_Descriptor_Configuration_Header_t),
.bDescriptorType = USB_DESCTYPE_CONFIGURATION,
.wTotalLength = sizeof(USB_Config_Descriptor_Audio2_t),
.bNumInterfaces = INTERFACE_COUNT,
.bConfigurationValue = 0x01,
.iConfiguration = 0x00,
#ifdef SELF_POWERED
.bmAttributes = 192,
#else
.bmAttributes = 128,
#endif
.bMaxPower = BMAX_POWER,
},

... and the conflicting Configuration:

unsigned char cfgDesc_Null[] =
{
0x09, /* 0 bLength */
USB_DESCTYPE_CONFIGURATION, /* 1 bDescriptorType */
0x12, /* 2 wTotalLength */
0x00, /* 3 wTotalLength */
0x01, /* 4 bNumInterface: Number of interfaces*/
0x01, /* 5 bConfigurationValue */
0x00, /* 6 iConfiguration */
#ifdef SELF_POWERED
192, /* 7 bmAttributes */
#else
128,
#endif
BMAX_POWER, /* 8 bMaxPower */

0x09, /* 0 bLength : Size of this descriptor, in bytes. (field size 1 bytes) */
0x04, /* 1 bDescriptorType : INTERFACE descriptor. (field size 1 bytes) */
0x00, /* 2 bInterfaceNumber : Index of this interface. (field size 1 bytes) */
0x00, /* 3 bAlternateSetting : Index of this setting. (field size 1 bytes) */
0x00, /* 4 bNumEndpoints : 0 endpoints. (field size 1 bytes) */
0x00, /* 5 bInterfaceClass : */
0x00, /* 6 bInterfaceSubclass */
0x00, /* 7 bInterfaceProtocol : Unused. (field size 1 bytes) */
0x00, /* 8 iInterface : Unused. (field size 1 bytes) */
0x09, /* 0 bLength */
};

I suggest that:
0x02, /* 5 bConfigurationValue */
should replace the corresponding line in cfgDesc_Null[]
... but I haven't built and tested the firmware to confirm that this works as I expect.
Post Reply