Fix USB descriptors for multiple interface devices (#3193)

For simple 1-interface devices, there is a fixed position in the USB
interface descriptor where the interface lives and we can hardcode
an updater in the USB handler.  However, for devices like CDC(Serial)
or MIDI there are multiple interfaces and the interface IDs need to
be stuffed inside the actual descriptor.

To allow this, instead of just taking a fixed set of bytes and
blindly patching in USB, make all interfaces do their own memcpy
from a local version, updated with the new dynamic interface id,
via a callback.

Add a simple USBClass callback to handle the normal 1-interface
case.

Fix MSD (SingleFileDrive, FatFSUSB) to only request the correct, single
interface used.

Author: earlephilhower

cores/rp2040/SerialUSB.cpp

@@ -39,6 +39,7 @@ extern void serialEvent() __attribute__((weak));
 #define USBD_CDC_IN_OUT_MAX_SIZE (64)
 
 
+
 SerialUSB::SerialUSB() {
 }
 
@@ -50,17 +51,26 @@ void SerialUSB::begin(unsigned long baud) {
     }
 
     USB.disconnect();
-    static uint8_t cdc_desc[TUD_CDC_DESC_LEN] = {
-        // Interface number, string index, protocol, report descriptor len, EP In & Out address, size & polling interval
-        TUD_CDC_DESCRIPTOR(0 /* placeholder*/, USB.registerString("Pico Serial"), _epIn = USB.registerEndpointIn(), USBD_CDC_CMD_MAX_SIZE, _epOut = USB.registerEndpointOut(), USB.registerEndpointIn(), USBD_CDC_IN_OUT_MAX_SIZE)
-    };
 
-    _id = USB.registerInterface(2, cdc_desc, sizeof(cdc_desc), 1, 0);
+    _epIn = USB.registerEndpointIn();
+    _epOut = USB.registerEndpointOut();
+    _epIn2 = USB.registerEndpointIn();
+    _strID = USB.registerString("Pico Serial");
+
+    _id = USB.registerInterface(2, _cb, (void *)this, TUD_CDC_DESC_LEN, 1, 0);
 
     USB.connect();
     _running = true;
 }
 
+// Need to define here so we don't have to include tusb.h in global header (causes problemw w/BT redefining things)
+void SerialUSB::interfaceCB(int itf, uint8_t *dst, int len) {
+    uint8_t desc[TUD_CDC_DESC_LEN] = {
+        TUD_CDC_DESCRIPTOR((uint8_t)itf, _strID, _epIn, USBD_CDC_CMD_MAX_SIZE, _epOut, _epIn2, USBD_CDC_IN_OUT_MAX_SIZE)
+    };
+    memcpy(dst, desc, len);
+};
+
 void SerialUSB::end() {
     if (_running) {
         USB.disconnect();

cores/rp2040/SerialUSB.h

@@ -57,11 +57,16 @@ class SerialUSB : public arduino::HardwareSerial {
     void tud_cdc_line_state_cb(uint8_t itf, bool dtr, bool rts);
     void tud_cdc_line_coding_cb(uint8_t itf, void const *p_line_coding); // Can't use cdc_line_coding_t const* p_line_coding, TinyUSB and BTStack conflict when we include tusb.h + BTStack.h
 
+    // USB interface descriptor CB
+    void interfaceCB(int itf, uint8_t *dst, int len);
+
 private:
     bool _running = false;
     uint8_t _id;
     uint8_t _epIn;
     uint8_t _epOut;
+    uint8_t _epIn2;
+    uint8_t _strID;
 
     typedef struct {
         unsigned int rebooting : 1;
@@ -73,6 +78,10 @@ class SerialUSB : public arduino::HardwareSerial {
     SyntheticState _ss = { 0, 0, 0, 0, 115200};
 
     void checkSerialReset();
+
+    static void _cb(int itf, uint8_t *dst, int len, void *param) {
+        ((SerialUSB *)param)->interfaceCB(itf, dst, len);
+    }
 };
 
 extern SerialUSB Serial;

cores/rp2040/USB.cpp

@@ -91,11 +91,12 @@ void USBClass::unregisterEndpointOut(int ep) {
     _endpointOut |= (1 << (ep - 0x80));
 }
 
-uint8_t USBClass::addEntry(Entry **head, int interfaces, const uint8_t *descriptor, size_t len, int ordering, uint32_t vidMask) {
+uint8_t USBClass::addEntry(Entry **head, int interfaces, void (*cb)(int itf, uint8_t *dst, int len, void *param), const void *param, size_t len, int ordering, uint32_t vidMask) {
     static uint8_t id = 1;
 
     Entry *n = (Entry *)malloc(sizeof(Entry));
-    n->descriptor = descriptor;
+    n->cb = cb;
+    n->param = param;
     n->len = len;
     n->interfaces = interfaces;
     n->order = ordering;
@@ -163,16 +164,16 @@ uint8_t USBClass::findInterfaceID(unsigned int localid) {
 
 // Called by a HID device to register a report.  Returns the *local* ID which must be mapped to the HID report ID
 uint8_t USBClass::registerHIDDevice(const uint8_t *descriptor, size_t len, int ordering, uint32_t vidMask) {
-    return addEntry(&_hids, 0, descriptor, len, ordering, vidMask);
+    return addEntry(&_hids, 0, nullptr, (const void *)descriptor, len, ordering, vidMask);
 }
 
 void USBClass::unregisterHIDDevice(unsigned int localid) {
     removeEntry(&_hids, localid);
 }
 
 // Called by an object at global init time to add a new interface (non-HID, like CDC or Picotool)
-uint8_t USBClass::registerInterface(int interfaces, const uint8_t *descriptor, size_t len, int ordering, uint32_t vidMask) {
-    return addEntry(&_interfaces, interfaces, descriptor, len, ordering, vidMask);
+uint8_t USBClass::registerInterface(int interfaces, void (*cb)(int itf, uint8_t *dst, int len, void *), void *param, size_t len, int ordering, uint32_t vidMask) {
+    return addEntry(&_interfaces, interfaces, cb, param, len, ordering, vidMask);
 }
 
 void USBClass::unregisterInterface(unsigned int localid) {
@@ -293,7 +294,7 @@ void USBClass::setupDescHIDReport() {
     uint8_t id = 1;
     h = _hids;
     while (h) {
-        memcpy(p, h->descriptor, h->len);
+        memcpy(p, h->param, h->len);
         // Need to update the report ID, a 2-byte value
         char buff[] = { HID_REPORT_ID(id) };
         memcpy(p + 6, buff, sizeof(buff));
@@ -340,12 +341,12 @@ void USBClass::setupUSBDescriptor() {
             // Interface number, string index, protocol, report descriptor len, EP In & Out address, size & polling interval
             TUD_HID_DESCRIPTOR(1 /* placeholder*/, 0, HID_ITF_PROTOCOL_NONE, hid_report_len, _hid_endpoint = USB.registerEndpointIn(), CFG_TUD_HID_EP_BUFSIZE, (uint8_t)usb_hid_poll_interval)
         };
-        _hid_interface = USB.registerInterface(1, hid_desc, sizeof(hid_desc), 10, 0);
+        _hid_interface = USB.registerInterface(1, simpleInterface, hid_desc, sizeof(hid_desc), 10, 0);
     }
 
 #ifdef ENABLE_PICOTOOL_USB
     uint8_t picotool_desc[] = { TUD_RPI_RESET_DESCRIPTOR(1, USB.registerString("Reset")) };
-    _picotool_itf_num = USB.registerInterface(1, picotool_desc, sizeof(picotool_desc), 100, 0);
+    _picotool_itf_num = USB.registerInterface(1, simpleInterface, picotool_desc, sizeof(picotool_desc), 100, 0);
 #endif
 
     usbd_desc_cfg_len = TUD_CONFIG_DESC_LEN; // Always have a config descriptor
@@ -361,19 +362,18 @@ void USBClass::setupUSBDescriptor() {
         TUD_CONFIG_DESCRIPTOR(1, interface_count, USB.registerString(""), usbd_desc_cfg_len, TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP, USBD_MAX_POWER_MA)
     };
 
-    // Allocate the "real" HID report descriptor
+    // Allocate the "real" report descriptor
     usbd_desc_cfg = (uint8_t *)malloc(usbd_desc_cfg_len);
     bzero(usbd_desc_cfg, usbd_desc_cfg_len);
 
-    // Now copy the descriptors
+    // Now copy the interface descriptors
     h = _interfaces;
     uint8_t *p = usbd_desc_cfg;
     memcpy(p, tud_cfg_desc, sizeof(tud_cfg_desc));
     p += sizeof(tud_cfg_desc);
     int id = 0;
     while (h) {
-        memcpy(p, h->descriptor, h->len);
-        p[2] = id; // Set the interface
+        h->cb(id, p, h->len, (void *)h->param);
         p += h->len;
         id += h->interfaces;
         h = h->next;

cores/rp2040/USB.h

@@ -39,7 +39,7 @@ class USBClass {
     void unregisterHIDDevice(unsigned int localid);
 
     // Called by an object at global init time to add a new interface (non-HID, like CDC or Picotool)
-    uint8_t registerInterface(int interfaces, const uint8_t *descriptor, size_t len, int ordering, uint32_t vidMask);
+    uint8_t registerInterface(int interfaces, void (*cb)(int itf, uint8_t *dst, int len, void *param), void *param, size_t len, int ordering, uint32_t vidMask);
 
     // Remove a USB interface from the USB descriptor.  Only call after usbDisconnect or results could be unpredictable!
     void unregisterInterface(unsigned int localid);
@@ -94,14 +94,21 @@ class USBClass {
     uint8_t usbTaskIRQ;
 #endif
 
+    // Simple 1-interface updated for "easy" interfaces like Picotool or HIF
+    static void simpleInterface(int itf, uint8_t *dst, int len, void *data) {
+        memcpy(dst, data, len);
+        dst[2] = itf; // Set the interface
+    }
+
 private:
     // We can't use non-trivial variables to hold the hid, interface, or string lists.  The global
     // initialization where things like the global Keyboard may be called before the non-trivial
     // objects (i.e. no std::vector).
 
     // Either a USB interface or HID device descriptor, kept in a linked list
     typedef struct Entry {
-        const uint8_t *descriptor;
+        void (*cb)(int itf, uint8_t *dst, int len, void *data); // unused for HID, only the report ID needs updating and we can do that inline
+        const void *param; // CB param or HID descriptor
         unsigned int len        : 12;
         unsigned int interfaces : 4;
         unsigned int order      : 18;
@@ -111,7 +118,7 @@ class USBClass {
     } Entry;
 
     // Add or remove Entry in a linked list, keeping things ordered by ordering
-    uint8_t addEntry(Entry **head, int interfaces, const uint8_t *descriptor, size_t len, int ordering, uint32_t vidMask);
+    uint8_t addEntry(Entry **head, int interfaces, void (*cb)(int itf, uint8_t *dst, int len, void *param), const void *param, size_t len, int ordering, uint32_t vidMask);
     void removeEntry(Entry **head, unsigned int localid);
 
     // Find the index (HID report ID or USB interface) of a given localid
@@ -124,7 +131,6 @@ class USBClass {
     // Gets a pointer to the HID report structure, optionally returning the size in len
     uint8_t *getDescHIDReport(int *len);
 
-private:
     Entry *_hids = nullptr;
     Entry *_interfaces = nullptr;
 

libraries/FatFSUSB/src/FatFSUSB.cpp

@@ -63,7 +63,7 @@ bool FatFSUSBClass::begin() {
     _epIn = USB.registerEndpointIn();
     _epOut = USB.registerEndpointOut();
     static uint8_t msd_desc[] = { TUD_MSC_DESCRIPTOR(1 /* placeholder */, 0, _epOut, _epIn, USBD_MSC_EPSIZE) };
-    _id = USB.registerInterface(2, msd_desc, sizeof(msd_desc), 2, 0);
+    _id = USB.registerInterface(1, USBClass::simpleInterface, msd_desc, sizeof(msd_desc), 2, 0);
     USB.connect();
 
     _started = true;

libraries/SingleFileDrive/src/SingleFileDrive.cpp

@@ -61,7 +61,7 @@ bool SingleFileDrive::begin(const char *localFile, const char *dosFile) {
     _epIn = USB.registerEndpointIn();
     _epOut = USB.registerEndpointOut();
     static uint8_t msd_desc[] = { TUD_MSC_DESCRIPTOR(1 /* placeholder */, 0, _epOut, _epIn, USBD_MSC_EPSIZE) };
-    _id = USB.registerInterface(2, msd_desc, sizeof(msd_desc), 2, 0);
+    _id = USB.registerInterface(1, USBClass::simpleInterface, msd_desc, sizeof(msd_desc), 2, 0);
     USB.connect();
     _localFile = strdup(localFile);
     _dosFile = strdup(dosFile);