fix #3596: big-endian host support for SETUP packet handling#3597
fix #3596: big-endian host support for SETUP packet handling#3597dangfan wants to merge 3 commits intohathach:masterfrom
Conversation
1. Add TU_LITTLE_ENDIAN_BITFIELD / TU_BIG_ENDIAN_BITFIELD macros in tusb_compiler.h (GCC and IAR), following Linux kernel style. 2. Update bmAttributes (tusb_desc_endpoint_t) and bmRequestType_bit (tusb_control_request_t) in tusb_types.h to use these macros with explicit #error fallback if undefined. 3. Add tu_le16toh() conversion in dcd_event_setup_received() for wValue/wIndex/wLength. Tested on CIU98320B (big-endian ARM Cortex-M, full-speed HID keyboard).
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| raspberrypi_zero/mtp | 60,160 → 60,184 (+24) | 4,603 → 4,571 (-32) | — | — | 64,763 → 64,755 (-8) | -0.0% |
There was a problem hiding this comment.
Pull request overview
This PR aims to make TinyUSB work correctly on big-endian CPUs by (1) converting multi-byte SETUP fields from USB wire (little-endian) to host byte order and (2) making USB bitfield parsing deterministic across endian/compilers.
Changes:
- Convert SETUP packet
wValue/wIndex/wLengthto host endianness indcd_event_setup_received(). - Add a
TU_BITFIELD_ORDERmechanism and use it to select correct bitfield layouts forbmRequestType_bitand endpointbmAttributes. - Introduce new bitfield-order constants in
tusb_compiler.hand setTU_BITFIELD_ORDERfor some toolchains.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/device/dcd.h |
Converts SETUP multi-byte fields to host order before dispatching the event. |
src/common/tusb_types.h |
Makes bmRequestType_bit and endpoint bmAttributes bitfields endian-order aware via TU_BITFIELD_ORDER. |
src/common/tusb_compiler.h |
Adds bitfield-order constants and defines TU_BITFIELD_ORDER for some compiler branches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| (void) memcpy(&event.setup_received, setup, sizeof(tusb_control_request_t)); | ||
| // USB wire format is little-endian. Convert multi-byte fields to host byte order | ||
| // so the stack always sees correct values regardless of CPU endianness. | ||
| event.setup_received.wValue = tu_le16toh(event.setup_received.wValue); | ||
| event.setup_received.wIndex = tu_le16toh(event.setup_received.wIndex); | ||
| event.setup_received.wLength = tu_le16toh(event.setup_received.wLength); |
There was a problem hiding this comment.
dcd_event_setup_received() now converts wValue/wIndex/wLength to host endianness. There is at least one in-tree caller that still applies tu_le16toh() to these fields (src/device/usbd.c passes tu_le16toh(p_request->wIndex) into tud_descriptor_string_cb()), which will double-swap on big-endian and break string descriptor requests (e.g., langid 0x0409 becomes 0x0904). After this change, the device stack should treat tusb_control_request_t fields as host-endian everywhere and remove any remaining tu_le16toh() uses on setup fields accordingly.
| // Endian conversion use well-known host to network (big endian) naming | ||
| // For TI ARM compiler, __BYTE_ORDER__ is not defined for MSP430 but still LE | ||
| #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ || defined(__MSP430__) | ||
| #define TU_BYTE_ORDER TU_LITTLE_ENDIAN | ||
| #define TU_BITFIELD_ORDER TU_BITFIELD_LE | ||
| #else | ||
| #define TU_BYTE_ORDER TU_BIG_ENDIAN | ||
| #define TU_BITFIELD_ORDER TU_BITFIELD_BE | ||
| #endif |
There was a problem hiding this comment.
TU_BITFIELD_ORDER is only defined in the GCC/TI and IAR compiler branches, but not in the __CCRX__ branch (which is still supported and defines TU_ATTR_BIT_FIELD_ORDER_BEGIN). With the new #error checks in tusb_types.h, CCRX builds will now fail because TU_BITFIELD_ORDER will be undefined. Please define TU_BITFIELD_ORDER for __CCRX__ as well (likely TU_BITFIELD_LE if #pragma bit_order right is intended to enforce LSB-first allocation).
| #define TU_LITTLE_ENDIAN (0x12u) | ||
| #define TU_BIG_ENDIAN (0x21u) | ||
|
|
||
| #define TU_BITFIELD_LE (0x34u) | ||
| #define TU_BITFIELD_BE (0x43u) | ||
|
|
There was a problem hiding this comment.
PR description mentions introducing TU_LITTLE_ENDIAN_BITFIELD / TU_BIG_ENDIAN_BITFIELD macros, but the implementation introduces TU_BITFIELD_LE / TU_BITFIELD_BE and TU_BITFIELD_ORDER instead. Please either align the macro names with the PR description or update the PR description to match the implemented API to avoid confusing downstream ports/users.
Summary
This PR fixes two issues that prevented TinyUSB from working correctly on big-endian hosts (#3596 ).
Problem 1: SETUP packet multi-byte fields not converted
USB wire format is little-endian. dcd_event_setup_received() did a raw
memcpyof the SETUP packet intotusb_control_request_t, leavingwValue,wIndex, andwLengthin wire byte order on big-endian hosts.Fix: Apply
tu_le16toh()on each field insidedcd_event_setup_received(). This is a no-op on little-endian hosts.Problem 2: Bit-field structure ordering is implementation-defined
C bit-field allocation order is implementation-defined (C11 §6.7.2.1p11). GCC on big-endian ARM allocates from MSB downward, opposite to the USB spec's LSB-first bit numbering. This caused
bmRequestType_bit.{direction,type,recipient}andbmAttributes.{xfer,sync,usage}to parse incorrectly.Fix: Following Linux kernel style (
__LITTLE_ENDIAN_BITFIELD/__BIG_ENDIAN_BITFIELD), defineTU_LITTLE_ENDIAN_BITFIELDandTU_BIG_ENDIAN_BITFIELDmacros intusb_compiler.h. Use explicit#ifdefblocks intusb_types.hto select correct field order. An#errordirective ensures new compiler/platform support must explicitly define the appropriate macro.Changes
src/common/tusb_compiler.h: AddTU_LITTLE_ENDIAN_BITFIELD/TU_BIG_ENDIAN_BITFIELDmacros (GCC and IAR)src/common/tusb_types.h: Use explicit bit-field order selection forbmAttributesandbmRequestType_bitsrc/device/dcd.h: Addtu_le16toh()conversion for SETUP packet fieldsTesting
Verified on CIU98320B (big-endian ARM Cortex-M0, full-speed USB device) with HID keyboard demo: