diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/class/audio/audio.h | 65 | ||||
| -rw-r--r-- | src/common/tusb_mcu.h | 19 | ||||
| -rw-r--r-- | src/device/dcd.h | 26 | ||||
| -rw-r--r-- | src/device/usbd.c | 35 | ||||
| -rw-r--r-- | src/portable/chipidea/ci_hs/ci_hs_type.h | 8 | ||||
| -rw-r--r-- | src/portable/chipidea/ci_hs/dcd_ci_hs.c | 243 | ||||
| -rw-r--r-- | src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c | 106 |
7 files changed, 390 insertions, 112 deletions
diff --git a/src/class/audio/audio.h b/src/class/audio/audio.h index 7981396c2..82db9ed9e 100644 --- a/src/class/audio/audio.h +++ b/src/class/audio/audio.h @@ -83,6 +83,71 @@ typedef enum { AUDIO_TERM_TYPE_OUT_LOW_FRQ_EFFECTS_SPEAKER = 0x0307, } audio_terminal_output_type_t; +/// 2.4 - Audio Class-Bi-directional Terminal Types UAC2 +typedef enum +{ + AUDIO_TERM_TYPE_BI_DIRECTIONAL_UNDEFINED = 0x0400, + AUDIO_TERM_TYPE_BI_DIRECTIONAL_HEADSET_HAND_HELD = 0x0401, + AUDIO_TERM_TYPE_BI_DIRECTIONAL_HEADSET_MOUNTED = 0x0402, + AUDIO_TERM_TYPE_BI_DIRECTIONAL_SPEAKERPHONE = 0x0403, + AUDIO_TERM_TYPE_BI_DIRECTIONAL_SPEAKERPHONE_ECHO_SUPPRESS = 0x0404, + AUDIO_TERM_TYPE_BI_DIRECTIONAL_SPEAKERPHONE_ECHO_CANCEL = 0x0405, +} audio_terminal_bi_directional_type_t; + +/// 2.5 - Audio Class-Telephone Terminal Types UAC2 +typedef enum +{ + AUDIO_TERM_TYPE_TELEPHONE_UNDEFINED = 0x0500, + AUDIO_TERM_TYPE_TELEPHONE_PHONE_LINE = 0x0501, + AUDIO_TERM_TYPE_TELEPHONE_TELEPHONE = 0x0502, + AUDIO_TERM_TYPE_TELEPHONE_DOWN_LINE_PHONE = 0x0503, +} audio_terminal_telephony_type_t; + +/// 2.6 - Audio Class-External Types UAC2 +typedef enum +{ + AUDIO_TERM_TYPE_EXTERNAL_UNDEFINED = 0x0600, + AUDIO_TERM_TYPE_EXTERNAL_ANALOG_CONNECTOR = 0x0601, + AUDIO_TERM_TYPE_EXTERNAL_DIGITAL_AUDIO = 0x0602, + AUDIO_TERM_TYPE_EXTERNAL_LINE_CONNECTOR = 0x0603, + AUDIO_TERM_TYPE_EXTERNAL_LEGACY_AUDIO_CONNECTOR = 0x0604, + AUDIO_TERM_TYPE_EXTERNAL_SPDIF_INTERFACE = 0x0605, + AUDIO_TERM_TYPE_EXTERNAL_1394_DA_STREAM = 0x0606, + AUDIO_TERM_TYPE_EXTERNAL_1394_DV_STREAM_SOUNDTRACK = 0x0607, + AUDIO_TERM_TYPE_EXTERNAL_ADAT_LIGHTPIPE = 0x0608, + AUDIO_TERM_TYPE_EXTERNAL_TDIF = 0x0609, + AUDIO_TERM_TYPE_EXTERNAL_MADI = 0x060A, +} audio_terminal_external_type_t; + +/// 2.7 - Audio Class-Embedded Types UAC2 +typedef enum +{ + AUDIO_TERM_TYPE_EMBEDDED_UNDEFINED = 0x0700, + AUDIO_TERM_TYPE_EMBEDDED_LEVEL_CALIBRATION_NOISE_SOURCE = 0x0701, + AUDIO_TERM_TYPE_EMBEDDED_EQUALIZATION_NOISE = 0x0702, + AUDIO_TERM_TYPE_EMBEDDED_CD_PLAYER = 0x0703, + AUDIO_TERM_TYPE_EMBEDDED_DAT = 0x0704, + AUDIO_TERM_TYPE_EMBEDDED_DCC = 0x0705, + AUDIO_TERM_TYPE_EMBEDDED_COMPRESSED_AUDIO_PLAYER = 0x0706, + AUDIO_TERM_TYPE_EMBEDDED_ANALOG_TAPE = 0x0707, + AUDIO_TERM_TYPE_EMBEDDED_PHONOGRAPH = 0x0708, + AUDIO_TERM_TYPE_EMBEDDED_VCR_AUDIO = 0x0709, + AUDIO_TERM_TYPE_EMBEDDED_VIDEO_DISC_AUDIO = 0x070A, + AUDIO_TERM_TYPE_EMBEDDED_DVD_AUDIO = 0x070B, + AUDIO_TERM_TYPE_EMBEDDED_TV_TUNER_AUDIO = 0x070C, + AUDIO_TERM_TYPE_EMBEDDED_SATELLITE_RECEIVER_AUDIO = 0x070D, + AUDIO_TERM_TYPE_EMBEDDED_CABLE_TUNER_AUDIO = 0x070E, + AUDIO_TERM_TYPE_EMBEDDED_DSS_AUDIO = 0x070F, + AUDIO_TERM_TYPE_EMBEDDED_RADIO_RECEIVER = 0x0710, + AUDIO_TERM_TYPE_EMBEDDED_RADIO_TRANSMITTER = 0x0711, + AUDIO_TERM_TYPE_EMBEDDED_MULTI_TRACK_RECORDER = 0x0712, + AUDIO_TERM_TYPE_EMBEDDED_SYNTHESIZER = 0x0713, + AUDIO_TERM_TYPE_EMBEDDED_PIANO = 0x0714, + AUDIO_TERM_TYPE_EMBEDDED_GUITAR = 0x0715, + AUDIO_TERM_TYPE_EMBEDDED_DRUMS = 0x0716, + AUDIO_TERM_TYPE_EMBEDDED_OTHER_MUSICAL_INSTRUMENT = 0x0717, +} audio_terminal_embedded_type_t; + /// Rest is yet to be implemented //--------------------------------------------------------------------+ diff --git a/src/common/tusb_mcu.h b/src/common/tusb_mcu.h index 93b4a2ee9..af43dfb12 100644 --- a/src/common/tusb_mcu.h +++ b/src/common/tusb_mcu.h @@ -126,6 +126,14 @@ #define CFG_TUSB_MEM_DCACHE_LINE_SIZE_DEFAULT 32 #endif + // Errata ERR050101, listed for RT1015/RT1020/RT1024/RT1050 (no fix scheduled) and for + // RT1060/RT1064 rev A (fixed in rev B); not listed for RT1010 or the RT11xx family. + #if defined(MIMXRT1015_SERIES) || defined(MIMXRT1021_SERIES) || defined(MIMXRT1024_SERIES) || \ + defined(MIMXRT1051_SERIES) || defined(MIMXRT1052_SERIES) || defined(MIMXRT1061_SERIES) || \ + defined(MIMXRT1062_SERIES) || defined(MIMXRT1064_SERIES) + #define CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 1 + #endif + #elif TU_CHECK_MCU(OPT_MCU_KINETIS_KL, OPT_MCU_KINETIS_K32L, OPT_MCU_KINETIS_K) #define TUP_USBIP_CHIPIDEA_FS #define TUP_USBIP_CHIPIDEA_FS_KINETIS @@ -768,6 +776,17 @@ #define TUP_DCD_EDPT_ISO_ALLOC #endif +// Set by silicon whose isochronous IN endpoint can be unprimed by an IN token sent to that same +// endpoint number on ANOTHER device sharing the host, taking one of this device's OUT endpoints +// down with it - undetectable in software. Descriptors must then give an isochronous IN endpoint +// a number no other device on the bus uses; a number is only safe while it stays unique, so two +// affected boards on one hub must not pick the same one. Default 0 (no such conflict). Set it to +// 0 by hand on RT1060/RT1064 rev B, which carry the fix - the revision cannot be told apart at +// compile time, so the affected parts are assumed to be rev A. +#ifndef CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 + #define CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 0 +#endif + // Some USBIPs (SAMG, SAMX7X, PIC32, MAX3266x/MAX78002) cannot assign the same endpoint // number to both IN and OUT. Default to 0 (same endpoint number may be used for IN and OUT). #ifndef CFG_TUD_ENDPOINT_ONE_DIRECTION_ONLY diff --git a/src/device/dcd.h b/src/device/dcd.h index f005e9620..a4006ae0c 100644 --- a/src/device/dcd.h +++ b/src/device/dcd.h @@ -20,19 +20,27 @@ // MACRO CONSTANT TYPEDEF PROTYPES //--------------------------------------------------------------------+ +// Bus reset is reported as two edges. BUS_RESET_START is optional: a controller that +// cannot tell the edges apart emits only BUS_RESET_END, which stays self-sufficient (it +// performs the full teardown with or without a preceding START). Emit START when reset +// signaling is detected - the link is unusable and the speed is not negotiated yet - so +// the stack stops using endpoints immediately instead of at the end of the reset. typedef enum { - DCD_EVENT_INVALID = 0, // 0 - DCD_EVENT_BUS_RESET, // 1 - DCD_EVENT_UNPLUGGED, // 2 - DCD_EVENT_SOF, // 3 - DCD_EVENT_SUSPEND, // 4 TODO LPM Sleep L1 support - DCD_EVENT_RESUME, // 5 - DCD_EVENT_SETUP_RECEIVED, // 6 - DCD_EVENT_XFER_COMPLETE, // 7 - USBD_EVENT_FUNC_CALL, // 8 Not an DCD event, just a convenient way to defer ISR function + DCD_EVENT_INVALID = 0, // 0 + DCD_EVENT_BUS_RESET_START, // 1 + DCD_EVENT_BUS_RESET_END, // 2 with negotiated speed + DCD_EVENT_UNPLUGGED, // 3 + DCD_EVENT_SOF, // 4 + DCD_EVENT_SUSPEND, // 5 TODO LPM Sleep L1 support + DCD_EVENT_RESUME, // 6 + DCD_EVENT_SETUP_RECEIVED, // 7 + DCD_EVENT_XFER_COMPLETE, // 8 + USBD_EVENT_FUNC_CALL, // 9 Not an DCD event, just a convenient way to defer ISR function DCD_EVENT_COUNT } dcd_eventid_t; +#define DCD_EVENT_BUS_RESET DCD_EVENT_BUS_RESET_END // backward compatibility + typedef struct TU_ATTR_ALIGNED(4) { uint8_t rhport; uint8_t event_id; diff --git a/src/device/usbd.c b/src/device/usbd.c index f5c3046d6..e84d72fa4 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -456,7 +456,8 @@ TU_ATTR_WEAK bool dcd_configure(uint8_t rhport, uint32_t cfg_id, const void* cfg #if CFG_TUSB_DEBUG >= CFG_TUD_LOG_LEVEL static char const *const _usbd_event_str[DCD_EVENT_COUNT] = { "Invalid", - "Bus Reset", + "Bus Reset Start", + "Bus Reset End", "Unplugged", "SOF", "Suspend", @@ -697,8 +698,15 @@ void tud_task_ext(uint32_t timeout_ms, bool in_isr) { #endif switch (event.event_id) { - case DCD_EVENT_BUS_RESET: + case DCD_EVENT_BUS_RESET_START: + TU_LOG_USBD("\r\n"); + usbd_reset(event.rhport); + break; + + case DCD_EVENT_BUS_RESET_END: TU_LOG_USBD(": %s Speed\r\n", tu_str_speed[event.bus_reset.speed]); + // TODO a DCD that reports both edges pays for two teardowns: track a per-rhport + // "start seen" flag and skip this reset, keeping it for the single-event DCDs. usbd_reset(event.rhport); _usbd_dev.speed = event.bus_reset.speed; break; @@ -749,7 +757,14 @@ void tud_task_ext(uint32_t timeout_ms, bool in_isr) { _usbd_dev.ep_status[epnum][ep_dir] &= (uint8_t) ~(TU_EDPT_STATE_BUSY | TU_EDPT_STATE_CLAIMED); if (0 == epnum) { - usbd_control_xfer_cb(event.rhport, ep_addr, (xfer_result_t) event.xfer_complete.result, event.xfer_complete.len); + // Not stalled on failure: a DCD refuses an EP0 prime when a newer setup is already + // latched, and EP0 stalls are cleared by hardware when that setup arrives - so a stall + // issued here lands after the auto-clear and would stall the transfer that superseded + // this one. The pending setup re-drives EP0 by itself. + if (!usbd_control_xfer_cb(event.rhport, ep_addr, (xfer_result_t) event.xfer_complete.result, + event.xfer_complete.len)) { + TU_LOG_USBD(" Control stage not continued\r\n"); + } } else { usbd_class_driver_t const* driver = get_driver(_usbd_dev.ep2drv[epnum][ep_dir]); TU_ASSERT(driver,); @@ -867,10 +882,10 @@ bool tud_control_xfer(uint8_t rhport, const tusb_control_request_t* request, voi if (ctrl_xfer->data_len > 0U) { TU_ASSERT(buffer); } - TU_ASSERT(data_stage_xact(rhport)); + TU_VERIFY(data_stage_xact(rhport)); } else { // wLength == 0: Status stage is always IN per USB 2.0 ยง9.3.1 - TU_ASSERT(status_stage_xact(rhport, TU_EP0_IN)); + TU_VERIFY(status_stage_xact(rhport, TU_EP0_IN)); } return true; @@ -921,7 +936,7 @@ static bool usbd_control_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t } if (is_ok) { - TU_ASSERT(status_stage_xact(rhport, ep_status)); + TU_VERIFY(status_stage_xact(rhport, ep_status)); } else { // Stall both IN and OUT control endpoint dcd_edpt_stall(rhport, TU_EP0_OUT); @@ -929,7 +944,7 @@ static bool usbd_control_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t } } else { // More data to transfer - TU_ASSERT(data_stage_xact(rhport)); + TU_VERIFY(data_stage_xact(rhport)); } return true; @@ -1600,10 +1615,12 @@ bool usbd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t* buffer, uint16_t t if (dcd_edpt_xfer(rhport, ep_addr, buffer, total_bytes, is_isr)) { return true; } else { - // DCD error, mark endpoint as ready to allow next transfer + // Driver refused the transfer, mark endpoint as ready to allow next transfer. This is a + // recoverable condition (e.g. a new setup superseding a control response), not a bug, so + // do not break into the debugger - TU_BREAKPOINT() halts the CPU whenever a probe is + // attached, which on a test rig is always. _usbd_dev.ep_status[epnum][dir] &= (uint8_t) ~(TU_EDPT_STATE_BUSY | TU_EDPT_STATE_CLAIMED); TU_LOG_USBD("FAILED\r\n"); - TU_BREAKPOINT(); return false; } } diff --git a/src/portable/chipidea/ci_hs/ci_hs_type.h b/src/portable/chipidea/ci_hs/ci_hs_type.h index b209c7545..5baa14821 100644 --- a/src/portable/chipidea/ci_hs/ci_hs_type.h +++ b/src/portable/chipidea/ci_hs/ci_hs_type.h @@ -36,10 +36,18 @@ enum { PORTSC1_CURRENT_CONNECT_STATUS = TU_BIT(0), PORTSC1_FORCE_PORT_RESUME = TU_BIT(6), PORTSC1_SUSPEND = TU_BIT(7), + PORTSC1_PORT_RESET = TU_BIT(8), // read-only in device mode: a reset is being driven PORTSC1_FORCE_FULL_SPEED = TU_BIT(24), PORTSC1_PORT_SPEED = TU_BIT(26) | TU_BIT(27) }; +// PORTSC1 PSPD field values, once shifted down by PORTSC1_PORT_SPEED_POS. 3 is undefined. +enum { + PORTSC1_PORT_SPEED_FULL = 0, + PORTSC1_PORT_SPEED_LOW = 1, + PORTSC1_PORT_SPEED_HIGH = 2, +}; + // OTGSC enum { OTGSC_VBUS_DISCHARGE = TU_BIT(0), diff --git a/src/portable/chipidea/ci_hs/dcd_ci_hs.c b/src/portable/chipidea/ci_hs/dcd_ci_hs.c index 8c08c6bd5..6ab28e0be 100644 --- a/src/portable/chipidea/ci_hs/dcd_ci_hs.c +++ b/src/portable/chipidea/ci_hs/dcd_ci_hs.c @@ -154,6 +154,14 @@ TU_VERIFY_STATIC(sizeof(dcd_qhd_t) == 64, "size is not correct"); #define QTD_NEXT_INVALID 0x01 +// Bounded spin for register waits. The longest legitimate wait is a flush held off by a packet +// already in progress: ~50 us for a full-speed 64-byte packet, a low thousands of dependent +// register reads, so healthy hardware never approaches this bound. Exceeding it means the +// controller has stopped responding, and the spin then only serves to keep an ISR (or an +// IRQ-masked caller) from hanging outright - the 3 ms reset-cleanup window of IMXRT1060RM 42.5.6.2.1 (p.2394) +// is already unreachable in that state, and the manual's remedy there is a controller reset. +#define CI_HS_BUSY_SPIN 10000u + typedef struct { // Must be at 2K alignment // Each endpoint with direction (IN/OUT) occupies a queue head @@ -164,6 +172,17 @@ typedef struct { CFG_TUD_MEM_SECTION TU_ATTR_ALIGNED(2048) static dcd_data_t _dcd_data; +// What the next Port Change Detect will be. Each one is preceded by the interrupt that causes it: +// a reset interrupt for the end of a bus reset - where the speed first becomes final - or a +// suspend interrupt for the resume that ends the suspend. A suspend itself raises no port change, +// which is why there is no such value here. Indexed by rhport, which is 0 or 1 on every ci_hs +// variant (NOT the controller count: mcx/rw61x map rhport 1 to controller 0). +enum { + PORT_CHANGE_REASON_RESET = 0, + PORT_CHANGE_REASON_RESUME = 1, +}; +static volatile uint8_t _port_change_reason[2]; + //--------------------------------------------------------------------+ // Prototypes and Helper Functions //--------------------------------------------------------------------+ @@ -172,12 +191,37 @@ TU_ATTR_ALWAYS_INLINE static inline uint8_t ci_ep_count(const ci_hs_regs_t *dcd_ return dcd_reg->DCCPARAMS & DCCPARAMS_DEN_MASK; } +static bool controller_reset(uint8_t rhport); + //--------------------------------------------------------------------+ // Controller API //--------------------------------------------------------------------+ -/// follows LPC43xx User Manual 23.10.3 -static void bus_reset(uint8_t rhport) { +// Flush endpoint buffers, following IMXRT1060RM 42.5.6.6.5 Flushing/De-priming an Endpoint +// (p.2413): write ENDPTFLUSH, wait for the controller +// to acknowledge, then confirm ENDPTSTAT went to zero. The controller refuses the flush when a +// packet is in progress, and the manual requires the procedure be repeated until it takes. +// Callers proceed regardless of the result; the bound only prevents an ISR-context hang on dead +// hardware. +static bool flush_endpoints(ci_hs_regs_t *dcd_reg, uint32_t mask) { + uint32_t guard = CI_HS_BUSY_SPIN; + do { + dcd_reg->ENDPTFLUSH = mask; + while (dcd_reg->ENDPTFLUSH & mask) { + if (!guard--) { + return false; + } + } + } while ((dcd_reg->ENDPTSTAT & mask) && guard--); + + return !(dcd_reg->ENDPTSTAT & mask); +} + +/// Everything the manual asks of the DCD when a reset is detected, in its order: clear the setup +/// and completion semaphores, cancel every prime, check the reset is still being driven, and free +/// the dTDs. All of it belongs inside the reset window (IMXRT1060RM 42.5.6.2.1, p.2394); nothing +/// is left for the port change that ends the reset, which only reports the negotiated speed. +static void bus_reset_begin(uint8_t rhport) { ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); // The reset value for all endpoint types is the control endpoint. If one endpoint @@ -193,17 +237,24 @@ static void bus_reset(uint8_t rhport) { //------------- Clear All Registers -------------// dcd_reg->ENDPTNAK = dcd_reg->ENDPTNAK; dcd_reg->ENDPTNAKEN = 0; - dcd_reg->USBSTS = dcd_reg->USBSTS; dcd_reg->ENDPTSETUPSTAT = dcd_reg->ENDPTSETUPSTAT; dcd_reg->ENDPTCOMPLETE = dcd_reg->ENDPTCOMPLETE; - while (dcd_reg->ENDPTPRIME) {} - dcd_reg->ENDPTFLUSH = 0xFFFFFFFF; - while (dcd_reg->ENDPTFLUSH) {} + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTPRIME && guard--) {} + dcd_reg->ENDPTFLUSH = 0xFFFFFFFFUL; - // read reset bit in portsc + // All of the above must land while the reset is still being driven - it lasts at least 3 ms. + // Arriving late leaves the controller in an undefined state, and the manual's remedy is to + // hardware-reset it. That clears Run/Stop, so the device detaches and the host will drive a + // fresh reset and enumeration - which is why nothing below this point is worth doing here. + if (!(dcd_reg->PORTSC1 & PORTSC1_PORT_RESET)) { + TU_LOG1("ci_hs: reset cleanup ran past the end of the reset, resetting controller\r\n"); + controller_reset(rhport); + return; // the controller detached; the host's next reset redoes everything below + } - //------------- Queue Head & Queue TD -------------// + //------------- Free all allocated dTDs: the controller will not execute them again -------------// tu_memclr(&_dcd_data, sizeof(dcd_data_t)); //------------- Set up Control Endpoints (0 OUT, 1 IN) -------------// @@ -216,21 +267,19 @@ static void bus_reset(uint8_t rhport) { dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); } -bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { - (void)rh_init; - tu_memclr(&_dcd_data, sizeof(dcd_data_t)); - +/// Reset the controller and bring it back up in device mode. Also the manual's remedy when the +/// reset cleanup misses its window: the controller reset clears Run/Stop and detaches the device, +/// so it must be re-initialised completely afterwards (IMXRT1060RM 42.5.6.2.1, p.2394). +static bool controller_reset(uint8_t rhport) { ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); - TU_ASSERT(ci_ep_count(dcd_reg) <= TUP_DCD_ENDPOINT_MAX); - - #if TU_CHECK_MCU(OPT_MCU_HPM) - usb_phy_init((USB_Type *)dcd_reg, false); - #endif + tu_memclr(&_dcd_data, sizeof(dcd_data_t)); // Reset controller dcd_reg->USBCMD |= USBCMD_RESET; - while (dcd_reg->USBCMD & USBCMD_RESET) {} + uint32_t guard = CI_HS_BUSY_SPIN; + while ((dcd_reg->USBCMD & USBCMD_RESET) && guard--) {} + TU_VERIFY(!(dcd_reg->USBCMD & USBCMD_RESET)); // reached from the ISR too, so never halt here // Set mode to device, must be set immediately after reset uint32_t usbmode = dcd_reg->USBMODE & ~USBMOD_CM_MASK; @@ -257,9 +306,11 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESET; + dcd_reg->ENDPTLISTADDR = (uint32_t)_dcd_data.qhd; // Endpoint List Address has to be 2K alignment dcd_reg->USBSTS = dcd_reg->USBSTS; - dcd_reg->USBINTR = INTR_USB | INTR_ERROR | INTR_PORT_CHANGE | INTR_SUSPEND; + dcd_reg->USBINTR = INTR_USB | INTR_ERROR | INTR_PORT_CHANGE | INTR_RESET | INTR_SUSPEND; uint32_t usbcmd = dcd_reg->USBCMD; usbcmd &= ~USBCMD_INTR_THRESHOLD_MASK; // Interrupt Threshold Interval = 0 @@ -270,8 +321,22 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { return true; } +bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { + (void)rh_init; + ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); + + TU_ASSERT(ci_ep_count(dcd_reg) <= TUP_DCD_ENDPOINT_MAX); + + #if TU_CHECK_MCU(OPT_MCU_HPM) + usb_phy_init((USB_Type *)dcd_reg, false); + #endif + + return controller_reset(rhport); +} + bool dcd_deinit(uint8_t rhport) { ci_hs_regs_t* dcd_reg = CI_HS_REG(rhport); + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESET; // disable all interrupt dcd_reg->USBINTR = 0; @@ -280,9 +345,9 @@ bool dcd_deinit(uint8_t rhport) { dcd_reg->USBCMD &= ~USBCMD_RUN_STOP; // flush all endpoints - while (dcd_reg->ENDPTPRIME) {} - dcd_reg->ENDPTFLUSH = 0xFFFFFFFF; - while (dcd_reg->ENDPTFLUSH) {} + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTPRIME && guard--) {} + flush_endpoints(dcd_reg, 0xFFFFFFFF); return true; } @@ -296,11 +361,13 @@ void dcd_int_disable(uint8_t rhport) { } void dcd_set_address(uint8_t rhport, uint8_t dev_addr) { - // Response with status first before changing device address - dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0, false); - - ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); - dcd_reg->DEVICEADDR = (dev_addr << 25) | TU_BIT(24); + // Response with status first before changing device address. A refused prime means a new + // setup superseded this transfer; staging an address whose ACK will never arrive would + // leave the device answering on it, so only arm the address when the status went out. + if (dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0, false)) { + ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); + dcd_reg->DEVICEADDR = (dev_addr << 25) | TU_BIT(24); + } } void dcd_remote_wakeup(uint8_t rhport) { @@ -468,9 +535,7 @@ bool dcd_edpt_iso_activate(uint8_t rhport, const tusb_desc_endpoint_t *desc_ep) // dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); // Flush EP - const uint32_t flush_mask = TU_BIT(epnum + (dir ? 16 : 0)); - dcd_reg->ENDPTFLUSH = flush_mask; - while (dcd_reg->ENDPTFLUSH & flush_mask) {} + flush_endpoints(dcd_reg, TU_BIT(epnum + (dir ? 16 : 0))); // disable to change max packet size ep_ctrl_clear(endptctrl, dir, ENDPTCTRL_ENABLE); @@ -496,7 +561,7 @@ void dcd_edpt_close_all(uint8_t rhport) { } } -static void qhd_start_xfer(uint8_t rhport, uint8_t epnum, uint8_t dir) { +static bool qhd_start_xfer(uint8_t rhport, uint8_t epnum, uint8_t dir) { ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); dcd_qhd_t *p_qhd = &_dcd_data.qhd[epnum][dir]; dcd_qtd_t *p_qtd = &_dcd_data.qtd[epnum][dir]; @@ -509,13 +574,22 @@ static void qhd_start_xfer(uint8_t rhport, uint8_t epnum, uint8_t dir) { dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); if (epnum == 0) { - // follows UM 24.10.8.1.1 Setup packet handling using setup lockout mechanism - // wait until ENDPTSETUPSTAT before priming data/status in response TODO add time out - while (dcd_reg->ENDPTSETUPSTAT & TU_BIT(0)) {} + // Setup lockout (IMXRT1060RM 42.5.6.4.2.1 Setup Phase, p.2403): never prime EP0 while a new + // SETUP is pending. The ISR + // normally consumes ENDPTSETUPSTAT quickly; if the guard trips, fail the transfer so usbd + // releases the endpoint (a pending SETUP supersedes this response anyway; without one, usbd + // stalls EP0 and the host recovers with a fresh control transfer). + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTSETUPSTAT & TU_BIT(0)) { + if (!guard--) { + return false; + } + } } // start transfer dcd_reg->ENDPTPRIME = TU_BIT(epnum + (dir ? 16 : 0)); + return true; } bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_t total_bytes, bool is_isr) { @@ -531,9 +605,7 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t *buffer, uint16_t to // Start qhd transfer p_qhd->ff = NULL; - qhd_start_xfer(rhport, epnum, dir); - - return true; + return qhd_start_xfer(rhport, epnum, dir); } #if !CFG_TUD_MEM_DCACHE_ENABLE @@ -584,9 +656,7 @@ bool dcd_edpt_xfer_fifo(uint8_t rhport, uint8_t ep_addr, tu_fifo_t *ff, uint16_t // Start qhd transfer p_qhd->ff = ff; - qhd_start_xfer(rhport, epnum, dir); - - return true; + return qhd_start_xfer(rhport, epnum, dir); } #endif @@ -634,43 +704,43 @@ void dcd_int_handler(uint8_t rhport) { return; } - // Set if the port controller enters the full or high-speed operational state. - // either from Bus Reset or Suspended state - if (int_status & INTR_PORT_CHANGE) { - // TU_LOG2("PortChange %08lx\r\n", dcd_reg->PORTSC1); - - // Reset interrupt is not enabled, we manually check if Port Change is due - // to connection / disconnection - if (dcd_reg->USBSTS & INTR_RESET) { - dcd_reg->USBSTS = INTR_RESET; + const uint8_t pci_reason = _port_change_reason[rhport]; // save current pci_reason - if (dcd_reg->PORTSC1 & PORTSC1_CURRENT_CONNECT_STATUS) { - const uint32_t speed = (dcd_reg->PORTSC1 & PORTSC1_PORT_SPEED) >> PORTSC1_PORT_SPEED_POS; - bus_reset(rhport); - dcd_event_bus_reset(rhport, (tusb_speed_t)speed, true); - } else { - dcd_event_bus_signal(rhport, DCD_EVENT_UNPLUGGED, true); - } - } else { - // Triggered by resuming from suspended state - if (!(dcd_reg->PORTSC1 & PORTSC1_SUSPEND)) { - dcd_event_bus_signal(rhport, DCD_EVENT_RESUME, true); - } - } + if (int_status & INTR_SUSPEND) { + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESUME; // next PCI is resume + dcd_event_bus_signal(rhport, DCD_EVENT_SUSPEND, true); } - if (int_status & INTR_SUSPEND) { - // TU_LOG2("Suspend %08lx\r\n", dcd_reg->PORTSC1); + // USB Reset Received: register cleanup runs here within the reset window (IMXRT1060RM 42.5.6.2.1, p.2394) + // and BUS_RESET_START fires now; BUS_RESET_END, with the final speed, is triggered later by PCI. + if (int_status & INTR_RESET) { + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESET; + bus_reset_begin(rhport); + dcd_event_bus_signal(rhport, DCD_EVENT_BUS_RESET_START, true); + } - if (dcd_reg->PORTSC1 & PORTSC1_SUSPEND) { - // Note: Host may delay more than 3 ms before and/or after bus reset before doing enumeration. - // Skip suspend event if we are not addressed - if ((dcd_reg->DEVICEADDR >> 25) & 0x0f) { - dcd_event_bus_signal(rhport, DCD_EVENT_SUSPEND, true); - } + // Port entered the full/high-speed operational state: the end of a bus reset, or a resume. + if (int_status & INTR_PORT_CHANGE) { + if (pci_reason == PORT_CHANGE_REASON_RESUME) { + dcd_event_bus_signal(rhport, DCD_EVENT_RESUME, true); + } else { + // the undefined encoding falls back to full speed + const uint32_t pspd = (dcd_reg->PORTSC1 & PORTSC1_PORT_SPEED) >> PORTSC1_PORT_SPEED_POS; + const tusb_speed_t speed = (pspd == PORTSC1_PORT_SPEED_LOW) ? TUSB_SPEED_LOW : + (pspd == PORTSC1_PORT_SPEED_HIGH) ? TUSB_SPEED_HIGH : TUSB_SPEED_FULL; + dcd_event_bus_reset(rhport, speed, true); + // This reset is over, so the next port change is a resume. Leaving it at RESET instead would + // dispatch every later resume as another end-of-reset, clearing the queue heads mid-session. + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESUME; } } + // No unplug detection yet, by the manual rather than by omission: IMXRT1060RM 42.7.31 (p.2470) says a zero + // Current Connect Status means the device "did not attach successfully or was forcibly + // disconnected by the software writing a zero to the Run bit ... It does not state the device + // being disconnected or suspended", so a cable pull raises no port change at all. VBUS via + // OTGSC BSV is the manual's disconnect indicator, and it is board dependent. + if (int_status & INTR_USB) { // Make sure we read the latest version of _dcd_data. dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); @@ -678,7 +748,7 @@ void dcd_int_handler(uint8_t rhport) { const uint32_t edpt_complete = dcd_reg->ENDPTCOMPLETE; dcd_reg->ENDPTCOMPLETE = edpt_complete; // acknowledge - // 23.10.12.3 Failed QTD also get ENDPTCOMPLETE set + // 42.5.6.6.4 Transfer Completion (p.2413): a failed dTD also sets ENDPTCOMPLETE // nothing to do, we will submit xfer as error to usbd // if (int_status & INTR_ERROR) { } @@ -694,12 +764,39 @@ void dcd_int_handler(uint8_t rhport) { } // Set up Received - // 23.10.10.2 Operational model for setup transfers + // 42.5.6.4.2 Control Endpoint Operation Model (p.2403) // Must be after normal transfer complete since it is possible to have both previous control status + new setup // in the same frame and we should handle previous status first. if (dcd_reg->ENDPTSETUPSTAT) { + // 42.5.6.4.2.1 Setup Phase (p.2403) steps 1-2: duplicate the setup payload BEFORE clearing + // ENDPTSETUPSTAT - + // the clear releases the setup lockout and a back-to-back SETUP (usbtest case 10) can + // overwrite the queue-head buffer immediately after. The copy is read through the volatile + // qualifier rather than memcpy'd because C orders volatile accesses only against each + // other: a plain copy may legally be sunk past the lockout-releasing store below. + union { + tusb_control_request_t request; + uint8_t byte[8]; + } setup; + const volatile uint8_t *setup_src = (const volatile uint8_t *)&_dcd_data.qhd[0][0].setup_request; + for (uint8_t i = 0; i < sizeof(setup.request); i++) { + setup.byte[i] = setup_src[i]; + } dcd_reg->ENDPTSETUPSTAT = dcd_reg->ENDPTSETUPSTAT; - dcd_event_setup_received(rhport, (uint8_t *)(uintptr_t)&_dcd_data.qhd[0][0].setup_request, true); + + // Retire a status/handshake phase left primed by the previous control sequence + // (IMXRT1060RM 42.5.6.4.2.1, p.2403), which would otherwise retire the response the task is about to + // prime for this setup. Skipped when EP0 has nothing primed or priming, since the manual + // does not want the flush wait in an interrupt handler when it has nothing to do. + // One volatile read per statement: C leaves their order unspecified within a single + // expression, which IAR rejects outright (Pa082). + const uint32_t ep0_mask = TU_BIT(0) | TU_BIT(16); + const uint32_t ep0_stat = dcd_reg->ENDPTSTAT; + const uint32_t ep0_prime = dcd_reg->ENDPTPRIME; + if ((ep0_stat | ep0_prime) & ep0_mask) { + flush_endpoints(dcd_reg, ep0_mask); + } + dcd_event_setup_received(rhport, setup.byte, true); } } diff --git a/src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c b/src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c index d5b03e4b1..42f6750b1 100644 --- a/src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c +++ b/src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c @@ -87,6 +87,10 @@ enum { DEVCMDSTAT_SUSPEND_CHANGE_MASK = TU_BIT(25), DEVCMDSTAT_RESET_CHANGE_MASK = TU_BIT(26), DEVCMDSTAT_VBUS_DEBOUNCED_MASK = TU_BIT(28), + + // write-1-to-clear latches + DEVCMDSTAT_W1C_MASK = DEVCMDSTAT_SETUP_RECEIVED_MASK | DEVCMDSTAT_CONNECT_CHANGE_MASK | + DEVCMDSTAT_SUSPEND_CHANGE_MASK | DEVCMDSTAT_RESET_CHANGE_MASK, }; enum { @@ -171,7 +175,9 @@ typedef struct ep_cmd_sts_t ep[2*MAX_EP_PAIRS][2]; xfer_dma_t dma[2*MAX_EP_PAIRS]; - TU_ATTR_ALIGNED(64) uint8_t setup_packet[8]; + // volatile: the controller DMAs a new setup packet into this buffer as soon as the SETUP + // latch is cleared, so reads of it must stay ordered against the register accesses around them + TU_ATTR_ALIGNED(64) volatile uint8_t setup_packet[8]; }dcd_data_t; // EP list must be 256-byte aligned @@ -180,8 +186,12 @@ typedef struct // Use CFG_TUD_MEM_SECTION to place it accordingly. CFG_TUD_MEM_SECTION TU_ATTR_ALIGNED(256) static dcd_data_t _dcd; -// Dummy buffer to fix ZLPs overwriting the buffer (probably an USB/DMA controller bug) -// TODO find way to save memory +// Dummy buffer to fix ZLPs overwriting the buffer: Errata LPC55S6x USB.5 / LPC55S2x USB.4 - the +// HS device controller always DMA-writes OUT data in 8-byte units, so up to 7 bytes land past the +// received length. This redirects the ZLP case; the general short-OUT case is unhandled here +// (TinyUSB's own endpoint buffers are sized/aligned so the spill stays inside them, but a tight +// caller buffer can be overrun by up to 7 bytes - the SDK's documented workaround is a bounce +// buffer). TODO find way to save memory CFG_TUD_MEM_SECTION TU_ATTR_ALIGNED(64) static uint8_t dummy[8]; //--------------------------------------------------------------------+ @@ -221,7 +231,7 @@ static const dcd_controller_t _dcd_controller[] = { // INTERNAL OBJECT & FUNCTION DECLARATION //--------------------------------------------------------------------+ -TU_ATTR_ALWAYS_INLINE static inline uint16_t get_buf_offset(void const * buffer) { +TU_ATTR_ALWAYS_INLINE static inline uint16_t get_buf_offset(void const volatile * buffer) { uint32_t addr = (uint32_t) buffer; TU_ASSERT( (addr & 0x3f) == 0, 0 ); return ( (addr >> 6) & 0xFFFFUL ) ; @@ -247,6 +257,16 @@ TU_ATTR_ALWAYS_INLINE static inline bool rhport_is_highspeed(uint8_t rhport) { return _dcd_controller[rhport].is_highspeed; } + +// DEVCMDSTAT mixes RW fields with write-1-to-clear latches (SETUP + the 3 change bits): a blind +// RMW writes a pending latch back as 1 and silently clears it (a SETUP eaten this way strands +// EP0). Mask the latches on every update; pass one in set_mask only to clear it. +TU_ATTR_ALWAYS_INLINE static inline void devcmdstat_update(dcd_registers_t* dcd_reg, + uint32_t clear_mask, uint32_t set_mask) { + const uint32_t v = dcd_reg->DEVCMDSTAT & ~(DEVCMDSTAT_W1C_MASK | clear_mask); + dcd_reg->DEVCMDSTAT = v | set_mask; +} + //--------------------------------------------------------------------+ // CONTROLLER API //--------------------------------------------------------------------+ @@ -284,8 +304,10 @@ bool dcd_init(uint8_t rhport, const tusb_rhport_init_t* rh_init) { dcd_reg->DATABUFSTART = tu_align((uint32_t) &_dcd, TU_BIT(22)); // 22-bit alignment dcd_reg->INTSTAT = dcd_reg->INTSTAT; // clear all pending interrupt dcd_reg->INTEN = INT_DEVICE_STATUS_MASK; - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_DEVICE_ENABLE_MASK | DEVCMDSTAT_DEVICE_CONNECT_MASK | - DEVCMDSTAT_RESET_CHANGE_MASK | DEVCMDSTAT_CONNECT_CHANGE_MASK | DEVCMDSTAT_SUSPEND_CHANGE_MASK; + // deliberately clear every latch (incl. a SETUP left by a bootloader/warm start) for a + // deterministic init state + devcmdstat_update(dcd_reg, 0, DEVCMDSTAT_DEVICE_ENABLE_MASK | DEVCMDSTAT_DEVICE_CONNECT_MASK | + DEVCMDSTAT_W1C_MASK); NVIC_ClearPendingIRQ(_dcd_controller[rhport].irqnum); @@ -309,8 +331,7 @@ void dcd_set_address(uint8_t rhport, uint8_t dev_addr) // Response with status first before changing device address dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0, false); - dcd_reg->DEVCMDSTAT &= ~DEVCMDSTAT_DEVICE_ADDR_MASK; - dcd_reg->DEVCMDSTAT |= dev_addr; + devcmdstat_update(dcd_reg, DEVCMDSTAT_DEVICE_ADDR_MASK, dev_addr); } void dcd_remote_wakeup(uint8_t rhport) @@ -321,13 +342,13 @@ void dcd_remote_wakeup(uint8_t rhport) void dcd_connect(uint8_t rhport) { dcd_registers_t* dcd_reg = _dcd_controller[rhport].regs; - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_DEVICE_CONNECT_MASK; + devcmdstat_update(dcd_reg, 0, DEVCMDSTAT_DEVICE_CONNECT_MASK); } void dcd_disconnect(uint8_t rhport) { dcd_registers_t* dcd_reg = _dcd_controller[rhport].regs; - dcd_reg->DEVCMDSTAT &= ~DEVCMDSTAT_DEVICE_CONNECT_MASK; + devcmdstat_update(dcd_reg, DEVCMDSTAT_DEVICE_CONNECT_MASK, 0); } void dcd_sof_enable(uint8_t rhport, bool en) @@ -380,9 +401,17 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) uint8_t const ep_id = ep_addr2id(ep_addr); + // Preserve rf_tv: for non-control endpoints it is a TYPE bit, not the toggle value (UM11126: + // T=1 + RF 1/0 = interrupt/iso). Zeroing it here turned HS periodic interrupt endpoints into + // isochronous - no handshake on OUT, dead IN (usbtest cases 25/26 on lpc55 HS port). + // TODO implement the Errata LPC546xx USB.13 work-around (same semantics in UM11126): with RF/TV preserved at 1, TR + // loads the toggle from TV, so an HS interrupt endpoint restarts on DATA1 after clear-halt and + // the host discards one packet as a retransmission. The documented workaround needs an + // interrupt-on-NAK state machine (park as generic TR=1/TV=0, wait for a NAKed token to latch + // toggle 0 via EPTOGGLE, restore the type) - deferred; one lost packet beats the fully broken + // endpoint the old rf_tv clear caused. _dcd.ep[ep_id][0].cmd_sts.stall = 0; _dcd.ep[ep_id][0].cmd_sts.toggle_reset = 1; - _dcd.ep[ep_id][0].cmd_sts.rf_tv = 0; } bool dcd_edpt_open(uint8_t rhport, tusb_desc_endpoint_t const * p_endpoint_desc) @@ -432,7 +461,7 @@ void dcd_edpt_close_all (uint8_t rhport) { for (uint8_t ep_id = 0; ep_id < 2*_dcd_controller[rhport].ep_pairs; ++ep_id) { - _dcd.ep[ep_id][0].cmd_sts.active = _dcd.ep[ep_id][0].cmd_sts.active = 0; // TODO proper way is to EPSKIP then wait ep[][].active then write ep[][].disable (see table 778 in LPC55S69 Use Manual) + _dcd.ep[ep_id][0].cmd_sts.active = _dcd.ep[ep_id][1].cmd_sts.active = 0; // TODO proper way is to EPSKIP then wait ep[][].active then write ep[][].disable (see table 778 in LPC55S69 Use Manual) _dcd.ep[ep_id][0].cmd_sts.disable = _dcd.ep[ep_id][1].cmd_sts.disable = 1; } } @@ -538,7 +567,7 @@ static void bus_reset(uint8_t rhport) dcd_reg->EPSKIP = 0xFFFFFFFF; dcd_reg->INTSTAT = dcd_reg->INTSTAT; // clear all pending interrupt - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_SETUP_RECEIVED_MASK; // clear setup received interrupt + devcmdstat_update(dcd_reg, 0, DEVCMDSTAT_SETUP_RECEIVED_MASK); // clear setup received interrupt dcd_reg->INTEN = INT_DEVICE_STATUS_MASK | TU_BIT(0) | TU_BIT(1); // enable device status & control endpoints } @@ -597,18 +626,25 @@ void dcd_int_handler(uint8_t rhport) { dcd_registers_t* dcd_reg = _dcd_controller[rhport].regs; - uint32_t const cmd_stat = dcd_reg->DEVCMDSTAT; - uint32_t int_status = dcd_reg->INTSTAT; - int_status &= dcd_reg->INTEN; + int_status &= dcd_reg->INTEN; dcd_reg->INTSTAT = int_status; // Acknowledge handled interrupt if (int_status == 0) return; + // Snapshot after the INTSTAT ack: latch bits persist (RWC) so nothing is lost, while the reverse + // order could consume INTSTAT bit0 for a SETUP not yet visible in the snapshot - stranding the + // SETUP (INTSTAT is edge-latched) and feeding bit0 to process_xfer_isr as a bogus completion. + uint32_t const cmd_stat = dcd_reg->DEVCMDSTAT; + //------------- Device Status -------------// if ( int_status & INT_DEVICE_STATUS_MASK ) { - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_RESET_CHANGE_MASK | DEVCMDSTAT_CONNECT_CHANGE_MASK | DEVCMDSTAT_SUSPEND_CHANGE_MASK; + // clear only the change latches observed in the snapshot: one latched by hardware between the + // snapshot and this write would be acknowledged unseen (its DEV_INT re-latches and dispatches + // next pass instead) + devcmdstat_update(dcd_reg, 0, cmd_stat & + (DEVCMDSTAT_RESET_CHANGE_MASK | DEVCMDSTAT_CONNECT_CHANGE_MASK | DEVCMDSTAT_SUSPEND_CHANGE_MASK)); if ( cmd_stat & DEVCMDSTAT_RESET_CHANGE_MASK) // bus reset { @@ -653,15 +689,43 @@ void dcd_int_handler(uint8_t rhport) _dcd.ep[0][0].cmd_sts.active = _dcd.ep[1][0].cmd_sts.active = 0; _dcd.ep[0][0].cmd_sts.stall = _dcd.ep[1][0].cmd_sts.stall = 0; - dcd_reg->DEVCMDSTAT |= DEVCMDSTAT_SETUP_RECEIVED_MASK; + // UM flow: ack the latch FIRST, then read the payload. This IP has no setup lockout, so a + // back-to-back SETUP can overwrite _dcd.setup_packet at any time - but with the latch already + // released, any such overwrite re-latches SETUP_RECEIVED and is redelivered (worst case a + // superseded duplicate, absorbed by usbd's queued-setup counter). The reverse order can + // consume the newer SETUP's latch unseen and lose it. + devcmdstat_update(dcd_reg, 0, DEVCMDSTAT_SETUP_RECEIVED_MASK); + + // UM11126 Fig 163 (control EP0 flowchart) requires clearing the EP0IN interrupt here: a + // control IN completion latched before this SETUP must not reach usbd after it, where it + // would be applied to the new request and arm its status stage early. EP0OUT goes with it - + // bit0 is set by SETUP reception too, and left set it would replay next pass as a phantom + // completion. Neither can discard live work: the SETUP latch NAKs all EP0 traffic until the + // update above, and both EP0 Active bits were cleared a few lines up. + dcd_reg->INTSTAT = TU_BIT(0) | TU_BIT(1); - dcd_event_setup_received(rhport, _dcd.setup_packet, true); + // Copied a byte at a time rather than with memcpy: C orders volatile accesses only against + // each other, so a non-volatile copy of this buffer may be sunk below the guard read that + // follows - gcc does exactly that at -O2 and -O3, leaving only -Os correct. + uint8_t setup_copy[8]; + for (uint8_t i = 0; i < sizeof(setup_copy); i++) { + setup_copy[i] = _dcd.setup_packet[i]; + } + + // a SETUP that raced in after the acks (its bit0 consumed above) makes this copy suspect: + // its latch is visible again, so re-raise the endpoint interrupt and let the next pass + // deliver the newer payload rather than passing up bytes that may be torn between the two + if (dcd_reg->DEVCMDSTAT & DEVCMDSTAT_SETUP_RECEIVED_MASK) { + dcd_reg->INTSETSTAT = TU_BIT(0); + } else { + dcd_event_setup_received(rhport, setup_copy, true); + } // keep waiting for next setup prepare_setup_packet(rhport); - // clear bit0 - int_status = tu_bit_clear(int_status, 0); + // drop both EP0 bits: acked above, and neither belongs to the request this SETUP starts + int_status &= ~(TU_BIT(0) | TU_BIT(1)); } // Endpoint transfer complete interrupt |
