From 51ccf8dc302ec0fcfbdeb50aae616405897110b5 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Mon, 19 Apr 2021 23:24:18 -0400 Subject: [PATCH] wip: revert usb_descriptor changes; use raw descriptors instead --- py/circuitpy_mpconfig.mk | 3 - shared-module/storage/__init__.c | 54 +++++++++++++- shared-module/storage/__init__.h | 6 ++ shared-module/usb_cdc/__init__.c | 4 +- shared-module/usb_cdc/__init__.h | 3 + shared-module/usb_midi/__init__.c | 107 +++++++++++++++++++++++++++- shared-module/usb_midi/__init__.h | 11 +++ supervisor/shared/usb/usb_desc.c | 114 +++++++++++++++++++++++++++++- supervisor/supervisor.mk | 50 +------------ tools/usb_descriptor | 2 +- 10 files changed, 294 insertions(+), 60 deletions(-) diff --git a/py/circuitpy_mpconfig.mk b/py/circuitpy_mpconfig.mk index 4538016a39..5dc59cc73b 100644 --- a/py/circuitpy_mpconfig.mk +++ b/py/circuitpy_mpconfig.mk @@ -251,9 +251,6 @@ CFLAGS += -DCIRCUITPY_REPL_BLE=$(CIRCUITPY_REPL_BLE) CIRCUITPY_REPL_UART ?= 0 CFLAGS += -DCIRCUITPY_REPL_UART=$(CIRCUITPY_REPL_UART) -CIRCUITPY_REPL_USB ?= 1 -CFLAGS += -DCIRCUITPY_REPL_USB=$(CIRCUITPY_REPL_USB) - # Should busio.I2C() check for pullups? # Some boards in combination with certain peripherals may not want this. CIRCUITPY_REQUIRE_I2C_PULLUPS ?= 1 diff --git a/shared-module/storage/__init__.c b/shared-module/storage/__init__.c index 04f64fda65..6c8e2a738b 100644 --- a/shared-module/storage/__init__.c +++ b/shared-module/storage/__init__.c @@ -41,8 +41,56 @@ #include "supervisor/usb.h" #include "tusb.h" +static uint8_t[] storage_usb_msc_descriptor[] = { + 0x09, // 0 bLength + 0x04, // 1 bDescriptorType (Interface) + 0xFF, // 2 bInterfaceNumber [SET AT RUNTIME] + 0x00, // 3 bAlternateSetting + 0x02, // 4 bNumEndpoints 2 + 0x08, // 5 bInterfaceClass: MSC + 0x06, // 6 bInterfaceSubClass: TRANSPARENT + 0x50, // 7 bInterfaceProtocol: BULK + 0x00, // 8 iInterface (String Index) [SET AT RUNTIME] + + 0x07, // 9 bLength + 0x05, // 10 bDescriptorType (Endpoint) + 0xFF, // 11 bEndpointAddress (IN/D2H) [SET AT RUNTIME: 0x8 | number] + 0x02, // 12 bmAttributes (Bulk) + 0x40, 0x00, // 13,14 wMaxPacketSize 64 + 0x00, // 15 bInterval 0 (unit depends on device speed) + + 0x07, // 16 bLength + 0x05, // 17 bDescriptorType (Endpoint) + 0xFF, // 18 bEndpointAddress (OUT/H2D) [SET AT RUNTIME] + 0x02, // 19 bmAttributes (Bulk) + 0x40, 0x00, // 20 wMaxPacketSize 64 + 0x00, // 21 bInterval 0 (unit depends on device speed) +}; + +// Indices into usb_msc_descriptor for values that must be set at runtime. + +#define MSC_INTERFACE_NUMBER_INDEX 2 +#define MSC_INTERFACE_STRING_INDEX 8 +#define MSC_IN_ENDPOINT_INDEX 11 +#define MSC_OUT_ENDPOINT_INDEX 118 + // Is the MSC device enabled? -static bool usb_storage_enabled; +bool storage_usb_enabled; + +size_t storage_usb_desc_length(void) { + return sizeof(usb_msc_descriptor); +} + +size_t storage_usb_add_desc(uint8_t *desc_buf, uint8_t interface_number, uint8_t in_endpoint_address, + uint8_t out_endpoint_address, uint8_t interface_string_index) { + memcpy(descriptor_buf, storage_usb_msc_descriptor, sizeof(storage_usb_msc_descriptor)); + desc_buf[MSC_INTERFACE_NUMBER_INDEX] = interface_number; + desc_buf[MSC_INTERFACE_STRING_INDEX] = interface_string_index; + desc_buf[MSC_IN_ENDPOINT_INDEX] = in_endpoint_address; + desc_buf[MSC_OUT_ENDPOINT_INDEX] = 0x80 | out_endpoint_address; + return sizeof(usb_msc_descriptor); +} + STATIC mp_obj_t mp_vfs_proxy_call(mp_vfs_mount_t *vfs, qstr meth_name, size_t n_args, const mp_obj_t *args) { if (vfs == MP_VFS_NONE) { @@ -62,7 +110,7 @@ STATIC mp_obj_t mp_vfs_proxy_call(mp_vfs_mount_t *vfs, qstr meth_name, size_t n_ } void storage_init(void) { - usb_storage_enabled = true; + storage_usb_enabled = true; } void common_hal_storage_mount(mp_obj_t vfs_obj, const char *mount_path, bool readonly) { @@ -180,6 +228,6 @@ bool common_hal_storage_configure_usb(bool enabled) { if (tud_connected()) { return false; } - usb_storage_enabled = enabled; + storage_usb_enabled = enabled; return true; } diff --git a/shared-module/storage/__init__.h b/shared-module/storage/__init__.h index 69e2a2da1c..e3cae1481a 100644 --- a/shared-module/storage/__init__.h +++ b/shared-module/storage/__init__.h @@ -27,6 +27,12 @@ #ifndef SHARED_MODULE_STORAGE___INIT___H #define SHARED_MODULE_STORAGE___INIT___H +extern bool storage_usb_enabled; + void storage_init(void); +size_t storage_usb_desc_length(void); +size_t storage_usb_add_desc(uint8_t *desc_buf, uint8_t interface_number, + uint8_t in_endpoint_address, uint8_t out_endpoint_address, + uint8_t interface_string_index); #endif // SHARED_MODULE_STORAGE___INIT___H diff --git a/shared-module/usb_cdc/__init__.c b/shared-module/usb_cdc/__init__.c index afd71feda5..d0930956cb 100644 --- a/shared-module/usb_cdc/__init__.c +++ b/shared-module/usb_cdc/__init__.c @@ -38,8 +38,8 @@ #error CFG_TUD_CDC must be exactly 2 #endif -static bool usb_cdc_repl_enabled; -static bool usb_cdc_data_enabled; +bool usb_cdc_repl_enabled; +bool usb_cdc_data_enabled; static usb_cdc_serial_obj_t usb_cdc_repl_obj = { .base.type = &usb_cdc_serial_type, diff --git a/shared-module/usb_cdc/__init__.h b/shared-module/usb_cdc/__init__.h index 321ee253bd..79a2e58a92 100644 --- a/shared-module/usb_cdc/__init__.h +++ b/shared-module/usb_cdc/__init__.h @@ -29,6 +29,9 @@ #include "py/objtuple.h" +extern bool usb_cdc_repl_enabled; +extern bool usb_cdc_data_enabled; + void usb_cdc_init(void); #endif /* SHARED_MODULE_USB_CDC___INIT___H */ diff --git a/shared-module/usb_midi/__init__.c b/shared-module/usb_midi/__init__.c index 7956fe2f0c..2869cc8b36 100644 --- a/shared-module/usb_midi/__init__.c +++ b/shared-module/usb_midi/__init__.c @@ -38,8 +38,113 @@ supervisor_allocation *usb_midi_allocation; +static uint8_t[] usb_midi_descriptor[] = { + 0x09, // 0 bLength + 0x04, // 1 bDescriptorType (Interface) + 0xFF, // 2 bInterfaceNumber [SET AT RUNTIME] + 0x00, // 3 bAlternateSetting + 0x00, // 4 bNumEndpoints 0 + 0x01, // 5 bInterfaceClass (Audio) + 0x01, // 6 bInterfaceSubClass (Audio Control) + 0x00, // 7 bInterfaceProtocol + 0x00, // 8 iInterface (String Index) [SET AT RUNTIME] + + 0x09, // 9 bLength + 0x24, // 10 bDescriptorType (See Next Line) + 0x01, // 11 bDescriptorSubtype (CS_INTERFACE -> HEADER) + 0x00, 0x01, // 12,13 bcdADC 1.00 + 0x09, 0x00, // 14,15 wTotalLength 9 + 0x01, // 16 binCollection 0x01 + 0xFF, // 17 baInterfaceNr [SET AT RUNTIME: one-element list: same as 20] + + 0x09, // 18 bLength + 0x04, // 19 bDescriptorType (Interface) + 0xFF, // 20 bInterfaceNumber [SET AT RUNTIME] + 0x00, // 21 bAlternateSetting + 0x02, // 22 bNumEndpoints 2 + 0x01, // 23 bInterfaceClass (Audio) + 0x03, // 24 bInterfaceSubClass (MIDI Streaming) + 0x00, // 25 bInterfaceProtocol + 0x0A, // 26 iInterface (String Index) [SET AT RUNTIME] + + 0x07, // 27 bLength + 0x24, // 28 bDescriptorType (See Next Line) + 0x01, 0x00, 0x01, 0x25, 0x00, // 29,30,31,32,33 + 0x06, // 34 bLength + 0x24, // 35 bDescriptorType (See Next Line) + 0x02, 0x01, 0x01, 0x08, // 36,37,38,39 + 0x06, // 40 bLength + 0x24, // 41 bDescriptorType (See Next Line) + 0x02, 0x02, 0x02, 0x00, // 42,43,44,45 + 0x09, // 46 bLength + 0x24, // 47 bDescriptorType (See Next Line) + 0x03, 0x01, 0x03, 0x01, 0x02, 0x01, 0x09, // 48,49,50,51,52,53,54 + 0x09, // 56 bLength + 0x24, // 57 bDescriptorType (See Next Line) + 0x03, 0x02, 0x04, 0x01, 0x01, 0x01, 0x00, // 58,59,60,61,62,63,64 + 0x07, // 65 bLength + 0x05, // 66 bDescriptorType (See Next Line) + 0xFF, // 67 bEndpointAddress (OUT/H2D) [SET AT RUNTIME] + 0x02, // 68 bmAttributes (Bulk) + 0x40, 0x00, // 69,70 wMaxPacketSize 64 + 0x00, // 71 bInterval 0 (unit depends on device speed) + + 0x05, // 72 bLength + 0x25, // 73 bDescriptorType (See Next Line) + 0x01, 0x01, 0x01, // 74,75,76 + 0x07, // 77 bLength + 0x05, // 78 bDescriptorType (See Next Line) + 0xFF, // 79 bEndpointAddress (IN/D2H) [SET AT RUNTIME: 0x8 | number] + 0x02, // 80 bmAttributes (Bulk) + 0x40, 0x00, // 81,82 wMaxPacketSize 64 + 0x00, // 83 bInterval 0 (unit depends on device speed) + + 0x05, // 84 bLength + 0x25, // 85 bDescriptorType (See Next Line) + 0x01, 0x01, 0x03, // 86,87,88 +}; + // Is the USB MIDI device enabled? -static bool usb_midi_enabled; +bool usb_midi_enabled; + +// Indices into usb_midi_descriptor for values that must be set at runtime. + +#define MIDI_AUDIO_CONTROL_INTERFACE_NUMBER_INDEX 2 +#define MIDI_AUDIO_CONTROL_INTERFACE_STRING_INDEX 8 + +// These two get the same value. +#define MIDI_MIDI_STREAMING_INTERFACE_NUMBER_INDEX 20 +#define MIDI_MIDI_STREAMING_INTERFACE_NUMBER_XREF_INDEX 17 + +#define MIDI_MIDI_STREAMING_INTERFACE_STRING_INDEX 26 + +#define MIDI_MIDI_STREAMING_OUT_ENDPOINT_INDEX 67 +#define MIDI_MIDI_STREAMING_IN_ENDPOINT_INDEX 79 + +size_t usb_midi_desc_length(void) { + return sizeof(usb_midi_descriptor); +} + +size_t usb_midi_add_desc(uint8_t *desc_buf, + uint8_t audio_control_interface_number, + uint8_t midi_streaming_interface_number, + uint8_t midi_streaming_in_endpoint_address, + uint8_t midi_streaming_out_endpoint_address, + uint8_t audio_control_interface_string, + uint8_t midi_streaming_interface_string) { + memcpy(descriptor_buf, usb_midi_descriptor, sizeof(usb_midi_descriptor)); + desc_buf[MIDI_AUDIO_CONTROL_INTERFACE_NUMBER_INDEX] = audio_control_interface_number; + desc_buf[MIDI_AUDIO_CONTROL_INTERFACE_STRING_INDEX] = audio_control_interface_string; + + desc_buf[MIDI_MIDI_STREAMING_INTERFACE_NUMBER_INDEX] = midi_streaming_interface_number; + desc_buf[MIDI_MIDI_STREAMING_INTERFACE_NUMBER_XREF_INDEX] = midi_streaming_interface_number; + desc_buf[MIDI_MIDI_STREAMING_INTERFACE_STRING_INDEX] = midi_streaming_interface_string; + + desc_buf[MSC_IN_ENDPOINT_INDEX] = midi_streaming_in_endpoint_address; + desc_buf[MSC_OUT_ENDPOINT_INDEX] = 0x80 | midi_streaming_out_endpoint_address; + return sizeof(usb_midi_descriptor); +} + void usb_midi_init(void) { usb_midi_enabled = true; diff --git a/shared-module/usb_midi/__init__.h b/shared-module/usb_midi/__init__.h index 9cba7fe6c4..75738515ba 100644 --- a/shared-module/usb_midi/__init__.h +++ b/shared-module/usb_midi/__init__.h @@ -27,7 +27,18 @@ #ifndef SHARED_MODULE_USB_MIDI___INIT___H #define SHARED_MODULE_USB_MIDI___INIT___H +extern bool usb_midi_enabled; + void usb_midi_init(void); void usb_midi_usb_init(void); +size_t usb_midi_desc_length(void); +size_t usb_midi_add_desc(uint8_t *desc_buf, + uint8_t audio_control_interface_number, + uint8_t midi_streaming_interface_number, + uint8_t midi_streaming_in_endpoint_address, + uint8_t midi_streaming_out_endpoint_address, + uint8_t audio_control_interface_string, + uint8_t midi_streaming_interface_string); + #endif /* SHARED_MODULE_USB_MIDI___INIT___H */ diff --git a/supervisor/shared/usb/usb_desc.c b/supervisor/shared/usb/usb_desc.c index 67df4f44bf..1de9c0a7a9 100644 --- a/supervisor/shared/usb/usb_desc.c +++ b/supervisor/shared/usb/usb_desc.c @@ -25,10 +25,29 @@ */ #include "lib/tinyusb/src/tusb.h" + +#if CIRCUITPY_USB_CDC +#include "shared-module/storage/__init__.h" +#endif + +#if CIRCUITPY_USB_HID +#include "shared-module/usb_hid/__init__.h" +#endif + +#if CIRCUITPY_USB_MIDI +#include "shared-module/usb_midi/__init__.h" +#endif + +#if CIRCUITPY_USB_MSC +#include "shared-module/storage/__init__.h" +#endif + #include "shared-module/usb_hid/Device.h" #include "genhdr/autogen_usb_descriptor.h" +static uint8_t *config_desc; + // Invoked when received GET DEVICE DESCRIPTOR // Application return pointer to descriptor uint8_t const *tud_descriptor_device_cb(void) { @@ -40,7 +59,100 @@ uint8_t const *tud_descriptor_device_cb(void) { // Descriptor contents must exist long enough for transfer to complete uint8_t const *tud_descriptor_configuration_cb(uint8_t index) { (void)index; // for multiple configurations - return usb_desc_cfg; + + size_t total_descriptor_length = 0; + + // CDC should be first, for compatibility with Adafruit Windows 7 drivers. + // In the past, the order has been CDC, MSC, MIDI, HID, so preserve + // that order. +#if CIRCUITPY_USB_CDC + if (usb_cdc_repl_enabled) { + total_descriptor_length += usb_cdc_desc_length(); + } + if (usb_cdc_data_enabled) { + total_descriptor_length += usb_cdc_desc_length(); + } +#endif + +#if CIRCUITPY_USB_MSC + if (storage_usb_enabled) { + total_descriptor_length += storage_usb_desc_length(); + } +#endif + +#if CIRCUITPY_USB_MIDI + if (usb_midi_enabled) { + total_descriptor_length += usb_midi_desc_length(); + } +#endif + +#if CIRCUITPY_USB_HID + if (usb_hid_enabled) { + total_descriptor_length += usb_hid_desc_length(); + } +#endif + + // Now we now how big the configuration descriptor will be. + config_desc = m_malloc(total_descriptor_length, false); + + // Number interfaces and endpoints. + // Endpoint 0 is already used for USB control + uint8_t current_interface = 0; + uint8_t current_endpoint = 1; + uint16_t current_interface_string = 1; + + uint8_t *desc_buf_remaining = config_desc; + +#if CIRCUITPY_USB_CDC + if (usb_cdc_repl_enabled) { + // Concatenate and fix up the CDC REPL descriptor. + } + if (usb_cdc_data_enabled) { + // Concatenate and fix up the CDC data descriptor. + } +#endif + +#if CIRCUITPY_USB_MSC + if (storage_usb_enabled) { + // Concatenate and fix up the MSC descriptor. + desc_buf_remaining += storage_usb_add_desc( + desc_buf_remaining, current_interface, + current_endpoint, // in + current_endpoint, // out + current_interface_string_index); + current_interface++; + current_endpoint++; + current_interface_string++; + } +#endif + +#if CIRCUITPY_USB_MIDI + if (usb_midi_enabled) { + // Concatenate and fix up the MIDI descriptor. + desc_buf_remaining += usb_midi_add_desc( + desc_buf_remaining, + current_interface, // audio control + current_interface + 1, // MIDI streaming + current_endpoint, // in + current_endpoint, // out + current_interface_string, // audio control + current_interface_string + 1 // MIDI streaming + ); + current_interface += 2; + current_endpoint++; + current_interface_string += 2; + + } +#endif + +#if CIRCUITPY_USB_HID + if (usb_hid_enabled) { + // Concatenate and fix up the HID descriptor (not the report descriptors). + } +#endif + + + return config_desc; } #if CIRCUITPY_USB_HID diff --git a/supervisor/supervisor.mk b/supervisor/supervisor.mk index 1744c4f47f..7fab262f64 100644 --- a/supervisor/supervisor.mk +++ b/supervisor/supervisor.mk @@ -149,57 +149,9 @@ endif # It gets added automatically. USB_WEBUSB_URL ?= "circuitpython.org" -ifeq ($(CIRCUITPY_REPL_USB),1) -USB_DEVICES += CDC -endif - -ifeq ($(CIRCUITPY_USB_HID),1) -USB_DEVICES += HID -endif -ifeq ($(CIRCUITPY_USB_MIDI),1) -USB_DEVICES += AUDIO -endif -ifeq ($(CIRCUITPY_USB_MSC),1) -USB_DEVICES += MSC -endif ifeq ($(CIRCUITPY_USB_CDC),1) # Inform TinyUSB there are two CDC devices. CFLAGS += -DCFG_TUD_CDC=2 -USB_DEVICES += CDC2 -endif -ifeq ($(CIRCUITPY_USB_VENDOR),1) -USB_DEVICES += VENDOR -endif - -USB_HID_DEVICES = -ifeq ($(CIRCUITPY_USB_HID_CONSUMER),1) -USB_HID_DEVICES += CONSUMER -endif -ifeq ($(CIRCUITPY_USB_HID_DIGITIZER),1) -USB_HID_DEVICES += DIGITIZER -endif -ifeq ($(CIRCUITPY_USB_HID_GAMEPAD),1) -USB_HID_DEVICES += GAMEPAD -endif -ifeq ($(CIRCUITPY_USB_HID_KEYBOARD),1) -USB_HID_DEVICES += KEYBOARD -endif -ifeq ($(CIRCUITPY_USB_HID_MOUSE),1) -USB_HID_DEVICES += MOUSE -endif -ifeq ($(CIRCUITPY_USB_HID_SYS_CONTROL),1) -USB_HID_DEVICES += SYS_CONTROL -endif -ifeq ($(CIRCUITPY_USB_HID_XAC_COMPATIBLE_GAMEPAD),1) -USB_HID_DEVICES += XAC_COMPATIBLE_GAMEPAD -endif - -# RAW is not compatible with other HID devices. -ifeq ($(CIRCUITPY_USB_HID_RAW),1) - ifneq ($(CIRCUITPY_USB_HID_DEVICES,) - $(error HID RAW must not be combined with other HID devices) -endif -USB_HID_DEVICES += MOUSE endif USB_HIGHSPEED ?= 0 @@ -221,7 +173,7 @@ USB_DESCRIPTOR_ARGS = \ --vid $(USB_VID)\ --pid $(USB_PID)\ --serial_number_length $(USB_SERIAL_NUMBER_LENGTH)\ - --interface_name $(USB_INTERFACE_NAME)\ + --interface_name_prefix $(USB_INTERFACE_NAME)\ --devices "$(USB_DEVICES)"\ --hid_devices "$(USB_HID_DEVICES)"\ --max_ep $(USB_NUM_EP) \ diff --git a/tools/usb_descriptor b/tools/usb_descriptor index 7275306928..2eaa6114b2 160000 --- a/tools/usb_descriptor +++ b/tools/usb_descriptor @@ -1 +1 @@ -Subproject commit 727530692805bb1c9d9d20d7477804a1799e358d +Subproject commit 2eaa6114b209fe7f0a795eda8d6a7b3b93d76d2e