[PATCH] usb: common: Detect USB storage media with "miscellaneous" USB devices
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.
Signed-off-by: Christian Kohlschütter christian@kohlschutter.com --- common/usb_storage.c | 3 ++- include/usb_defs.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/usb_storage.c b/common/usb_storage.c index eaa31374ef..727b364b3d 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -1322,7 +1322,8 @@ int usb_storage_probe(struct usb_device *dev, unsigned int ifnum, /* let's examine the device now */ iface = &dev->config.if_desc[ifnum];
- if (dev->descriptor.bDeviceClass != 0 || + if ((dev->descriptor.bDeviceClass != 0 && dev->descriptor.bDeviceClass + != USB_CLASS_MISC) || iface->desc.bInterfaceClass != USB_CLASS_MASS_STORAGE || iface->desc.bInterfaceSubClass < US_SC_MIN || iface->desc.bInterfaceSubClass > US_SC_MAX) { diff --git a/include/usb_defs.h b/include/usb_defs.h index 6dd2c997f9..f8f87d6a8a 100644 --- a/include/usb_defs.h +++ b/include/usb_defs.h @@ -19,6 +19,7 @@ #define USB_CLASS_MASS_STORAGE 8 #define USB_CLASS_HUB 9 #define USB_CLASS_DATA 10 +#define USB_CLASS_MISC 0xef #define USB_CLASS_VENDOR_SPEC 0xff
/* some HID sub classes */
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 ?
On 26. Sep 2022, at 01:45, Marek Vasut marex@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.
On 9/26/22 23:55, Christian Kohlschütter wrote:
On 26. Sep 2022, at 01:45, Marek Vasut marex@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 ?
On 30. Sep 2022, at 04:42, Marek Vasut marex@denx.de wrote:
On 9/26/22 23:55, Christian Kohlschütter wrote:
On 26. Sep 2022, at 01:45, Marek Vasut marex@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/u... 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).
On 9/30/22 23:58, Christian Kohlschütter wrote:
On 30. Sep 2022, at 04:42, Marek Vasut marex@denx.de wrote:
On 9/26/22 23:55, Christian Kohlschütter wrote:
On 26. Sep 2022, at 01:45, Marek Vasut marex@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/u... 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.
See https://github.com/hathach/tinyusb/blob/master/examples/device/cdc_msc/src/u... 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.
I'm not familiar enough with the available USB specs to say what is "right" but I think the problem is that "fixing" it by changing the device class breaks support on major operating systems.
On 1. Oct 2022, at 00:21, Christian Kohlschütter christian@kohlschutter.com wrote:
See https://github.com/hathach/tinyusb/blob/master/examples/device/cdc_msc/src/u... 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.
I'm not familiar enough with the available USB specs to say what is "right" but I think the problem is that "fixing" it by changing the device class breaks support on major operating systems.
OK, looking through the tinyUSB example source code [1] and USB docs [2], it looks like the reason why changing the device class from 0xEF (misc) to 0x00 is that the combination deviceClass:0xEF + deviceSubClass:0x02 + deviceProtocol:0x01 indicates the presence of an "Interface Association Descriptor":
// Use Interface Association Descriptor (IAD) for CDC // As required by USB Specs IAD's subclass must be common class (2) and protocol must be IAD (1) .bDeviceClass = TUSB_CLASS_MISC, .bDeviceSubClass = MISC_SUBCLASS_COMMON, .bDeviceProtocol = MISC_PROTOCOL_IAD,
So yeah, it looks like it has to be like that.
[1] https://github.com/hathach/tinyusb/blob/master/examples/device/cdc_msc/src/u... [2] https://www.usb.org/defined-class-codes
On 1. Oct 2022, at 00:58, Christian Kohlschütter christian@kohlschutter.com wrote:
On 1. Oct 2022, at 00:21, Christian Kohlschütter christian@kohlschutter.com wrote:
See https://github.com/hathach/tinyusb/blob/master/examples/device/cdc_msc/src/u... 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.
I'm not familiar enough with the available USB specs to say what is "right" but I think the problem is that "fixing" it by changing the device class breaks support on major operating systems.
OK, looking through the tinyUSB example source code [1] and USB docs [2], it looks like the reason why changing the device class from 0xEF (misc) to 0x00 is that the combination deviceClass:0xEF + deviceSubClass:0x02 + deviceProtocol:0x01 indicates the presence of an "Interface Association Descriptor":
// Use Interface Association Descriptor (IAD) for CDC // As required by USB Specs IAD's subclass must be common class (2) and protocol must be IAD (1) .bDeviceClass = TUSB_CLASS_MISC, .bDeviceSubClass = MISC_SUBCLASS_COMMON, .bDeviceProtocol = MISC_PROTOCOL_IAD,
So yeah, it looks like it has to be like that.
[1] https://github.com/hathach/tinyusb/blob/master/examples/device/cdc_msc/src/u... [2] https://www.usb.org/defined-class-codes
More insights:
According to [3], setting all three values (class/subclass/protocol) to zero (0x00/0x00/0x00) may also indicate a composite device (and I can confirm it working as well, so that could be a workaround if you can control the USB device's software stack). However this approach seems to require the device to have exactly one USB configuration. This may limit the utility of such a device.
Moreover, for many real-world devices, like the ESP32, the documentation calls for 0xEF/0x02/0x01 [4].
[3] https://learn.microsoft.com/en-us/windows-hardware/drivers/usbcon/enumeratio... [4] https://docs.espressif.com/projects/esp-idf/en/latest/esp32s2/api-reference/...
participants (3)
-
Christian Kohlschütter -
Christian Kohlschütter
-
Marek Vasut