[PATCH] usb: common: Detect USB storage media with "miscellaneous" USB devices

[Marek Vasut]

On 9/30/22 23:58, Christian Kohlschütter wrote:
>> On 30. Sep 2022, at 04:42, Marek Vasut <marex at denx.de> wrote:
>>
>> On 9/26/22 23:55, Christian Kohlschütter wrote:
>>> On 26. Sep 2022, at 01:45, Marek Vasut <marex at denx.de> wrote:
>>>>
>>>> On 9/25/22 16:46, Christian Kohlschütter wrote:
>>>>> When detecting USB storage devices, we currently skip everything that is
>>>>> not marked as "undefined device class".
>>>>> Composite devices such as tinyusb's CDC+MSC identify as "miscellaneous"
>>>>> (class 0xEF).
>>>>> Introduce a new constant, USB_CLASS_MISC (0xEF), and allow the detection
>>>>> process to proceed for USB devices with this device class.
>>>>
>>>> What does Linux do with such a device , is there a specific quirk or does it simply not check the bDeviceClass at all ?
>>> I didn't see any  relevant checks in drivers/usb/storage, however changing the bDeviceClass from "MISC" (0xEF) to "UNSPECIFIED"/"PER_INTERFACE" (0x00) causes the device to be no longer detected by either Linux or macOS, but it would work with U-Boot (i.e., without the change).
>>> macOS' USB prober would report the USB device as "unconfigured". On Linux, using usbutils but not busybox, lsusb would actually hang.
>>> I think it's a good idea to keep the set of permitted device classes to a minimum (0x00 / 0xEF) and abort early in all other cases. This way, we can skip the more elaborate checks in usb_storage_probe for devices that are definitely not supported.
>>
>> I think the check in the usb_storage.c is actually correct as-is and the device you have might be odd.
>>
>> https://www.usb.org/sites/default/files/usbmassbulk_10.pdf
>>
>> "
>> 4.1 Device Descriptor
>> Each USB device has one device descriptor (per USB Specification). The device shall specify the device class and subclass codes in the interface descriptor, and not in the device descriptor.
>> ...
>> 4 | bDeviceClass | Byte | 00h | Class is specified in the interface descriptor.
>> ...
>> NOTE: Information in this table is from the USB Specification version 1.1 table 9-7. Bold text has been added for clarifications when using these descriptors with this specification.
>> "
>>
>> And then
>>
>> "
>> 4.3 Interface Descriptors
>> The device shall support at least one interface, known herein as the Bulk-Only Data Interface. The Bulk-Only Data Interface uses three endpoints.
>> Composite mass storage devices may support additional interfaces, to provide other features such as audio or video capabilities. This specification does not define such interfaces.
>> The interface may have multiple alternate settings. The host shall examine each of the alternate settings to look for the bInterfaceProtocol and bInterfaceSubClass it supports optimally.
>> "
>>
>> If I read this right, then such a composite device like you have should have bDeviceClass=0x00 and two Interface descriptors, one for the mass storage device and one for CDC ?
> 
> See https://github.com/hathach/tinyusb/blob/master/examples/device/cdc_msc/src/usb_descriptors.c for an example of such a device. It has one mass storage device and one CDC.
> 
> Changing the device class to 0x00 breaks support on Linux and macOS (I didn't try Windows).

The question is -- does the aforementioned device conform to the USB-IF 
specification ? I think it does not, but luckily it could be fixed in 
software on the tinyusb side.