pkg/tinyusb: add common USB descriptors implementation

[gschorcht]

Contribution description

This PR serves as a proof of concept and a starting point for discussion for a general implementation of the USB descriptors for the tinyUSB package.

Every application that uses the tinyUSB stack as a package has to implement the definition of the USB descriptors (device descriptor, configuration descriptors and interface descriptors) and their handling in the tinyUSB callback functions. See tests/pkg_tinyusb_cdc_msc/usb_descriptors.c and tests/pkg_tinyusb_cdc_acm_stdio/usb_descriptors.c in PR #18804 for example.

Wbile crawling through the tinyUSB examples, I realized that in tinyUSB the definition of the descriptors as well as their handling is always done in the same way. The idea is therefore to generalize the definition of the descriptors and their handling in the tinyUSB callback functions on the basis of the given configuration (selected device class modules and the Kconfig), at least for a set of standard applications. This avoids that the descriptors have to be redefined for each application and their handling has to be reimplemented again and again. Only for some special applications, the application itself must define the descriptors and implement their handling.

Testing procedure

tests/pkg_tinyusb_cdc_msc should still work.

Issues/PRs references

[benpicco]

Very nice, that makes using tinyusb much more seamless.
Works like a charm with #18804

[benpicco]

Eh, why WIP?
This is already much better than the status quo and I'd rather get this in before a copy & paste culture takes hold.

You could argue for allowing to opt out of it if the application really wants to provide their own usb_descriptors.c.

[gschorcht]

@benpicco I pushed a first version of the common USB descriptor definition and handling for the tinyUSB package. The concept is already working with CDC (1 or 2 interfaces), MSC and generic HID (1 or 2 interfaces) device classes. I would really appriciate if you would take a look whether this might be right way to avoid that each standard application has to define its own descriptors and their handling.

[gschorcht]

@benpicco Oops, you already approved that. The PR was intended only as a basis for discussion. Since the static tests fail, it can't be accidentally merged 😉

[gschorcht]

This is already much better than the status quo and I'd rather get this in before a copy & paste culture takes hold.

The approach has some disadvantages:

• Each interface blocks the IN and OUT endpoint instead of allocating only the EPs in the direction it requires as USBUS does
• HID devices are not specific (mouse, keyboard, game controler) but generic always using EP IN and EP OUT.
• It seems that Audio device class and ECM and RNDIS net device class can't be generalized.

Furthermore, I planned to implement Kconfig for all the configuration that use CONFIG_* defines.

[benpicco]

Could this be solved with XFA so each driver only defines the endpoints they want and the linker merges that into an array?

Still, those improvements / cleanups are all internal to RIOT, so they can be done incrementally.

[gschorcht]

Could this be solved with XFA so each driver only defines the endpoints they want and the linker merges that into an array?

Hm, I'm not sure how XFA could help here. EPs are just consecutive numbers.

[gschorcht]

Could this be solved with XFA so each driver only defines the endpoints they want and the linker merges that into an array?

Hm, I'm not sure how XFA could help here. EPs are just consecutive numbers.

@benpicco The combined configuration and interface descriptors as used by tinyUSB could probably be constructed using XFAs if each device class defines and handles its interface descriptors in separate modules. However, I have no idea how the Interface IDs and Endpoint IDs for the device classes could be defined depending on their usage if their definition is also done in separate modules. The Interface IDs and the Endpoint IDs are used in the interface descriptors of the according device class. The number of Interface IDs is used in the global configuration descriptor.

Another problem is the string descriptor, which contains a human readable description for each interface or different objects of an interface. The index in the string descriptor is used in interface descriptors.

Maybe, you have an idea how manage this.

[gschorcht]

The combined configuration and interface descriptors as used by tinyUSB could probably be constructed using XFAs if each device class defines and handles its interface descriptors in separate modules.

@benpicco I have experimented a bit with XFAs for the combined configuration/interface/endpoint descriptor. According to the USB specification, the configuration, interface and the endpoint descriptors are transferred in one piece as one transaction when the GET DESCRIPTOR request with DescriptorType=CONFIGURATION is recevied. Therefore, the tinyUSB stack handles these descriptors as a single byte array composed of the various descriptor macros.

My first idea was to define a couple of default descriptors as weak symbols in an XFA according to the device classes used, so that they can be overridden by the application if necessary. The Interface ID would be used as priority to ensure the correct order inside the XFA:

XFA_INIT_CONST(uint8_t, desc_fs_configuration);

/* FS configuration */
XFA_CONST(desc_fs_configuration, 0) uint8_t desc_dev[] = {
    /* Config number, interface count, string index, total length, attribute, power in mA */
    TUD_CONFIG_DESCRIPTOR(1, ITF_TOTAL, 0, DESC_TOTAL_LEN, DESC_DEV_ATTR, CONFIG_USB_MAX_POWER),
};

#if CFG_TUD_CDC > 0
__attribute__((weak))
XFA_CONST(desc_fs_configuration, ITF_CDC_0 + 1) uint8_t desc_cdc_0[] = {
    /* Interface number, string index, EP notification address and size, EP Data Out & In, EP size. */
    TUD_CDC_DESCRIPTOR(ITF_CDC_0, STR_IDX_CDC_0, EP_CDC_0_NOTIF, 8, EP_CDC_0_OUT, EP_CDC_0_IN, CONFIG_USB_FS_EP_SIZE),
};
#endif
...
#if CFG_TUD_HID > 0
__attribute__((weak))
XFA_CONST(desc_fs_configuration, ITF_HID_0 + 1) uint8_t desc_hid_0[] = {
    /* Interface number, string index, protocol, report descriptor len, EP Out & EP In address, EP size, polling interval */
    TUD_HID_INOUT_DESCRIPTOR(ITF_HID_0, STR_IDX_HID_0, HID_ITF_PROTOCOL_NONE, sizeof(desc_hid_0_report), EP_HID_0_OUT, EP_HID_0_IN, CFG_TUD_HID_EP_BUFSIZE, 10),
};
#endif
...

Unfortunatly, if the application defines its own desc_hid_0 using a different descriptor macro, e.g.

uint8_t const desc_hid_0_report[] =
{
  TUD_HID_REPORT_DESC_KEYBOARD()
};

__attribute__((weak))
XFA_CONST(desc_fs_configuration, ITF_HID_0 + 1) uint8_t desc_hid_0[] = {
    /* Interface number, string index, protocol, report descriptor len, EP In address, EP size & polling interval */
    TUD_HID_DESCRIPTOR(ITF_HID_0, 4, HID_ITF_PROTOCOL_NONE, sizeof(desc_hid_0_report), ITF_HID_0 + 0x81, CFG_TUD_HID_EP_BUFSIZE, 10),
};

desc_hid_0 as defined by the application as well as the weak symbol desc_hid_0 are linked as part of the combined descriptor desc_fs_configuration. I analyzed the linker mapping file but I have no idea why. The symbol desc_hid_0 is defined only once in the ELF file.

Another problem is that the configuration descriptor desc_dev contains the total length of the combined configuration/interface/endpoint descriptor desc_fs_configuration which can't be determined as constant initializer from the XFA. It could be determined during runtime, but this would place the descriptor in RAM.

So I'm afraid that XFA cannot be a mechanism for overridable default descriptors.

[riot-ci]

Murdock results

✔️ PASSED

028c220 pkg/tinyusb: add Kconfig variables for common tinyUSB descriptors

Success	Failures	Total	Runtime
2000	0	2000	06m:49s

Artifacts

• Documentation preview

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

large code changes

[gschorcht]

@benpicco I have finished the work on generic descriptors for now. I did my best but it is impossible to provide such generic descriptors for all possible devices classes and applications.

The approach now supports arbitrary combinations of

• up to two CDC ACM class interfaces
• up to two Generic In/Out HID class interfaces
• up to one MSC device interface and
• up to one Vendor device class interface.

Any other combination, either a different number of these supported device class interfaces or the use of a different device class interface, requires the implementation of custom descriptors.

All symbols of the generic descriptors and their handling are defined as weak symbols so that the application can override parts of the descriptors or the functions that handle them. For example, the tusb_desc_hid_0_report array which defines the HID report descriptor and makes the device to be a Generic In/Out HID device

__attribute__((weak))
uint8_t const tusb_desc_hid_0_report[] =
{
    TUD_HID_REPORT_DESC_GENERIC_INOUT(CONFIG_TUSBD_HID_EP_SIZE),
};

could be overriden by the application with

uint8_t const tusb_desc_hid_0_report[] =
{
    TUD_HID_REPORT_DESC_KEYBOARD(HID_REPORT_ID(REPORT_ID_KEYBOARD)),
    TUD_HID_REPORT_DESC_MOUSE(HID_REPORT_ID(REPORT_ID_MOUSE)),
};

to make the device to be a composite keyboard/mouse device.

The PR also integrates the tinyUSB configuration in Kconfig. For example, the number of used CDC interfaces is now configured by CONFIG_TUSBD_CDC_NUMOF in Kconfig and mapped to the CFG_TUD_CDC variable as used by tinyUSB.

[gschorcht]

This PR doesn't include your approach for the tusb_config.h and tinyusb_app_config.h files. This change can be kept as part of your PR #18804.

[benpicco]

Feel free to cherry-pick 8e3f40e if you prefer to have it all in one place.
(And squash, this is now pretty much a re-write anyway)

[benpicco]

Looks good to me and still works as expected.

CI failures are unrelated - www.dlbeer.co.nz was down (edhoc_c)

[gschorcht]

Thanks for reviewing and merging.