From fd63ad6c2b102d3ea981047e75b43c8db723534e Mon Sep 17 00:00:00 2001 From: hathach Date: Thu, 9 Jul 2026 23:38:11 +0700 Subject: usbd: forward vendor EP0 requests; clear ep state on iso activate Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HeF2gZ1M7GWkz6Av4BpKPg --- src/device/usbd.c | 84 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 29 deletions(-) (limited to 'src/device') diff --git a/src/device/usbd.c b/src/device/usbd.c index 7a6e13f8d..5471e132d 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -256,7 +256,7 @@ static const usbd_class_driver_t _usbd_driver[] = { .deinit = vendord_deinit, .reset = vendord_reset, .open = vendord_open, - .control_xfer_cb = tud_vendor_control_xfer_cb, + .control_xfer_cb = vendord_control_xfer_cb, .xfer_cb = vendord_xfer_cb, .xfer_isr = NULL, .sof = NULL @@ -418,6 +418,7 @@ TU_ATTR_ALWAYS_INLINE static inline bool queue_event(dcd_event_t const * event, //--------------------------------------------------------------------+ static bool usbd_control_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes); static bool process_setup_received(uint8_t rhport, tusb_control_request_t const * p_request); +static bool process_get_status(uint8_t rhport, tusb_control_request_t const * request, uint16_t status); static bool process_set_config(uint8_t rhport, uint8_t cfg_num); static bool process_get_descriptor(uint8_t rhport, tusb_control_request_t const * p_request); @@ -1041,9 +1042,7 @@ static bool process_std_device_request(uint8_t rhport, tusb_control_request_t co // Device status bit mask // - Bit 0: Self Powered TODO must invoke callback to get actual status // - Bit 1: Remote Wakeup enabled - uint16_t status = (uint16_t) _usbd_dev.dev_state_bm; - tud_control_xfer(rhport, p_request, &status, 2); - return true; + return process_get_status(rhport, p_request, (uint16_t) _usbd_dev.dev_state_bm); } default: @@ -1053,6 +1052,14 @@ static bool process_std_device_request(uint8_t rhport, tusb_control_request_t co } +// Reply to a standard GET_STATUS (device/interface/endpoint) with its 2-byte status word. +// GET_STATUS is Device-to-host only; reject a mis-directed (OUT) request rather than handing +// usbd the address of a stack local to write host data into after this frame has returned. +static bool process_get_status(uint8_t rhport, tusb_control_request_t const * request, uint16_t status) { + TU_VERIFY(request->bmRequestType_bit.direction == TUSB_DIR_IN); + return tud_control_xfer(rhport, request, &status, 2); +} + // This handles the actual request and its response. // Returns false if unable to complete the request, causing caller to stall control endpoints. static bool process_setup_received(uint8_t rhport, tusb_control_request_t const * p_request) { @@ -1150,9 +1157,18 @@ static bool process_setup_received(uint8_t rhport, tusb_control_request_t const } case TUSB_REQ_SET_INTERFACE: + // A class that implements altsettings handles SET_INTERFACE itself and returns true, + // so reaching here means the class does not — where only alt 0 is valid. Any non-zero + // alt (unimplemented, or rejected as invalid by the class) is a Request Error (stall). + TU_VERIFY(tu_u16_low(p_request->wValue) == 0); tud_control_status(rhport, p_request); break; + case TUSB_REQ_GET_STATUS: + // USB 2.0 9.4.5: interface GET_STATUS returns 2 reserved (zero) bytes + TU_VERIFY(process_get_status(rhport, p_request, 0x0000)); + break; + default: return false; } } @@ -1175,35 +1191,34 @@ static bool process_setup_received(uint8_t rhport, tusb_control_request_t const } else { // Handle STD request to endpoint switch (p_request->bRequest) { //-V2520 - case TUSB_REQ_GET_STATUS: { - uint16_t status = usbd_edpt_stalled(rhport, ep_addr) ? 0x0001u : 0x0000u; - tud_control_xfer(rhport, p_request, &status, 2); - } - break; + case TUSB_REQ_GET_STATUS: + // USB 2.0 9.4.5: endpoint GET_STATUS bit 0 = Halt + TU_VERIFY(process_get_status(rhport, p_request, usbd_edpt_stalled(rhport, ep_addr) ? 0x0001u : 0x0000u)); + break; case TUSB_REQ_CLEAR_FEATURE: case TUSB_REQ_SET_FEATURE: { - if ( TUSB_REQ_FEATURE_EDPT_HALT == p_request->wValue ) { - if ( TUSB_REQ_CLEAR_FEATURE == p_request->bRequest ) { - usbd_edpt_clear_stall(rhport, ep_addr); - }else { - usbd_edpt_stall(rhport, ep_addr); - } + // ENDPOINT_HALT is the only endpoint feature; it exists only on a non-control endpoint + // that an interface actually owns. Any other selector, the control endpoint (EP0 has no + // Halt feature, USB 2.0 9.4.9), or an endpoint no driver owns is a Request Error (stall). + TU_VERIFY(TUSB_REQ_FEATURE_EDPT_HALT == p_request->wValue); + TU_VERIFY(ep_num != 0); + TU_VERIFY(driver != NULL); + + if ( TUSB_REQ_CLEAR_FEATURE == p_request->bRequest ) { + usbd_edpt_clear_stall(rhport, ep_addr); + } else { + usbd_edpt_stall(rhport, ep_addr); } - if (driver != NULL) { - // Some classes such as USBTMC needs to clear/re-init its buffer when receiving CLEAR_FEATURE request - // We will also forward std request targeted endpoint to class drivers as well + // Some classes such as USBTMC need to clear/re-init their buffer on CLEAR_FEATURE. + // Clear complete callback if driver set since it can also stall the request. + (void) invoke_class_control(rhport, driver, p_request); + ctrl_xfer->complete_cb = NULL; - // STD request must always be ACKed regardless of driver returned value - // Also clear complete callback if driver set since it can also stall the request. - (void) invoke_class_control(rhport, driver, p_request); - ctrl_xfer->complete_cb = NULL; - - // skip ZLP status if driver already did that - if (!(_usbd_dev.ep_status[0][TUSB_DIR_IN] & TU_EDPT_STATE_BUSY)) { - tud_control_status(rhport, p_request); - } + // STD request must always be ACKed; skip ZLP status if driver already did that. + if (!(_usbd_dev.ep_status[0][TUSB_DIR_IN] & TU_EDPT_STATE_BUSY)) { + tud_control_status(rhport, p_request); } } break; @@ -1648,10 +1663,21 @@ void usbd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) { uint8_t const epnum = tu_edpt_number(ep_addr); uint8_t const dir = tu_edpt_dir(ep_addr); - // only clear if currently stalled TU_LOG_USBD(" Clear Stall EP %02X\r\n", ep_addr); + const bool was_stalled = (_usbd_dev.ep_status[epnum][dir] & TU_EDPT_STATE_STALLED) != 0; dcd_edpt_clear_stall(rhport, ep_addr); - _usbd_dev.ep_status[epnum][dir] &= (uint8_t) ~(TU_EDPT_STATE_STALLED | TU_EDPT_STATE_BUSY); + // Clear STALLED|BUSY unconditionally (long-standing behavior; some classes, e.g. audio's + // set-interface, call this on a non-stalled endpoint solely to drop a leftover BUSY bit). + // Only release the CLAIMED ownership bit when the endpoint was actually stalled: the stall + // aborts the in-flight transfer in the dcd with no completion event to release the claim, so + // clearing it here prevents starvation. On a non-stalled clear (e.g. a data-toggle reset) a + // transfer may still be legitimately claimed by another task, so keep CLAIMED to preserve the + // claim->xfer mutual exclusion. + uint8_t clear_mask = TU_EDPT_STATE_STALLED | TU_EDPT_STATE_BUSY; + if (was_stalled) { + clear_mask |= TU_EDPT_STATE_CLAIMED; + } + _usbd_dev.ep_status[epnum][dir] &= (uint8_t) ~clear_mask; } bool usbd_edpt_stalled(uint8_t rhport, uint8_t ep_addr) { -- cgit v1.3.1 From 7e59f1bf8a7590e13da7fe5e8c602b41c95d2341 Mon Sep 17 00:00:00 2001 From: TenGui Date: Thu, 23 Jul 2026 15:26:18 -0700 Subject: fix narrowing, add cast --- src/device/usbd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/device') diff --git a/src/device/usbd.h b/src/device/usbd.h index 9be12ed0a..296ec417d 100644 --- a/src/device/usbd.h +++ b/src/device/usbd.h @@ -229,7 +229,7 @@ bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_requ // Config number, interface count, string index, total length, attribute, power in mA #define TUD_CONFIG_DESCRIPTOR(config_num, _itfcount, _stridx, _total_len, _attribute, _power_ma) \ - 9, TUSB_DESC_CONFIGURATION, U16_TO_U8S_LE(_total_len), _itfcount, config_num, _stridx, TU_BIT(7) | _attribute, (_power_ma)/2 + 9, TUSB_DESC_CONFIGURATION, U16_TO_U8S_LE(_total_len), _itfcount, config_num, _stridx, TU_BIT(7) | _attribute, (uint8_t)((_power_ma)/2) //--------------------------------------------------------------------+ // CDC Descriptor Templates -- cgit v1.3.1 From 05f19ab438641823d047c811e7426177788d0490 Mon Sep 17 00:00:00 2001 From: TenGui Date: Sun, 26 Jul 2026 12:16:23 -0700 Subject: TU_MIN suggestion --- src/common/tusb_types.h | 2 +- src/device/usbd.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src/device') diff --git a/src/common/tusb_types.h b/src/common/tusb_types.h index d0796ccc8..fa4d67df1 100644 --- a/src/common/tusb_types.h +++ b/src/common/tusb_types.h @@ -244,7 +244,7 @@ enum { TUSB_DESC_CONFIG_ATT_SELF_POWERED = 1 << 6, }; -#define TUSB_DESC_CONFIG_POWER_MA(x) (uint8_t)((x)/2) +#define TUSB_DESC_CONFIG_POWER_MA(x) ((uint8_t)TU_MIN((x)/2, UINT8_MAX)) // USB 2.0 Spec Table 9-7: Test Mode Selectors typedef enum { diff --git a/src/device/usbd.h b/src/device/usbd.h index 296ec417d..2015d1869 100644 --- a/src/device/usbd.h +++ b/src/device/usbd.h @@ -229,7 +229,7 @@ bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_requ // Config number, interface count, string index, total length, attribute, power in mA #define TUD_CONFIG_DESCRIPTOR(config_num, _itfcount, _stridx, _total_len, _attribute, _power_ma) \ - 9, TUSB_DESC_CONFIGURATION, U16_TO_U8S_LE(_total_len), _itfcount, config_num, _stridx, TU_BIT(7) | _attribute, (uint8_t)((_power_ma)/2) + 9, TUSB_DESC_CONFIGURATION, U16_TO_U8S_LE(_total_len), _itfcount, config_num, _stridx, TU_BIT(7) | _attribute, (uint8_t)TU_MIN((_power_ma)/2, UINT8_MAX) //--------------------------------------------------------------------+ // CDC Descriptor Templates -- cgit v1.3.1 From a0249ada9096365697340031a7b4a285beb18a2b Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 12 Aug 2026 22:36:49 +0700 Subject: usbd: don't leak the queued-setup counter when the event queue is full A SETUP arriving while the event queue is full is silently dropped by queue_event(), but _usbd_queued_setup has already been incremented. The leaked count makes the event handler skip every subsequent SETUP ("Skipped since there is other SETUP in queue") forever: EP0 stays deaf until tud_init() while the device otherwise looks alive - enumerated, endpoints armed. Undo the increment when the enqueue fails. Unit test: fill the queue so a SETUP is dropped, then verify the next SETUP still completes a GET_DESCRIPTOR control transfer. --- src/device/usbd.c | 6 +++-- test/unit-test/test/device/usbd/test_usbd.c | 38 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) (limited to 'src/device') diff --git a/src/device/usbd.c b/src/device/usbd.c index 5471e132d..b77b766dd 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1473,8 +1473,10 @@ TU_ATTR_FAST_FUNC void dcd_event_handler(dcd_event_t const* event, bool in_isr) break; } - if (send) { - queue_event(event, in_isr); + if (send && !queue_event(event, in_isr) && event->event_id == DCD_EVENT_SETUP_RECEIVED) { + // dropped by a full queue: undo the increment, else every later SETUP is skipped as + // "other SETUP in queue" and EP0 is deaf until re-init + _usbd_queued_setup--; } } diff --git a/test/unit-test/test/device/usbd/test_usbd.c b/test/unit-test/test/device/usbd/test_usbd.c index 7f3c3f5b2..935a20221 100644 --- a/test/unit-test/test/device/usbd/test_usbd.c +++ b/test/unit-test/test/device/usbd/test_usbd.c @@ -270,6 +270,44 @@ void test_usbd_control_in_zlp(void) tud_task(); } +//--------------------------------------------------------------------+ +// SETUP dropped by full event queue +//--------------------------------------------------------------------+ + +// When the event queue is full, queue_event() drops the SETUP event. The queued-setup +// counter must not keep the dropped SETUP's increment: a leaked count makes the handler +// skip every later SETUP ("other SETUP in queue") forever, leaving EP0 permanently deaf. +void test_usbd_setup_dropped_by_full_queue_recovers(void) +{ + // fillers drain through usbd_reset -> class reset + mscd_reset_Ignore(); + + // fill the queue to the brim, then post one more SETUP: queue_event() drops it + for (unsigned i = 0; i < CFG_TUD_TASK_QUEUE_SZ; i++) { + dcd_event_bus_signal(rhport, DCD_EVENT_UNPLUGGED, false); + } + dcd_event_setup_received(rhport, (uint8_t*) &req_get_desc_device, false); + + // drain all fillers (each tud_task pass handles at most CFG_TUD_TASK_EVENTS_PER_RUN + // events); the dropped SETUP never arrives + for (unsigned i = 0; i < (CFG_TUD_TASK_QUEUE_SZ / CFG_TUD_TASK_EVENTS_PER_RUN) + 1; i++) { + tud_task(); + } + + // the next SETUP must still be answered + desc_device = (uint8_t const*) &data_desc_device; + dcd_event_setup_received(rhport, (uint8_t*) &req_get_desc_device, false); + + dcd_edpt_xfer_ExpectWithArrayAndReturn(rhport, 0x80, (uint8_t*) &data_desc_device, sizeof(tusb_desc_device_t), sizeof(tusb_desc_device_t), false, true); + dcd_event_xfer_complete(rhport, EDPT_CTRL_IN, sizeof(tusb_desc_device_t), 0, false); + + dcd_edpt_xfer_ExpectAndReturn(rhport, EDPT_CTRL_OUT, NULL, 0, false, true); + dcd_event_xfer_complete(rhport, EDPT_CTRL_OUT, 0, 0, false); + dcd_edpt0_status_complete_ExpectWithArray(rhport, &req_get_desc_device, 1); + + tud_task(); +} + //--------------------------------------------------------------------+ // Control OUT data stage host overrun //--------------------------------------------------------------------+ -- cgit v1.3.1 From a52562b2be7ea728a176ae94d8a18d9ae0a4423a Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 12 Aug 2026 22:37:06 +0700 Subject: usbd: clear the queued-setup counter on bus reset A SETUP counted before a bus reset must not be carried across it: the consumer would either skip a post-reset SETUP (count drained by the stale entry) or, if the count leaked high for any other reason, skip them all. usbd_reset() now zeroes the counter; the consumer already guards on zero, and any pre-reset SETUP still in the queue is stale by definition and correctly discarded. --- src/device/usbd.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/device') diff --git a/src/device/usbd.c b/src/device/usbd.c index b77b766dd..79802e70f 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -642,6 +642,8 @@ static void configuration_reset(uint8_t rhport) { static void usbd_reset(uint8_t rhport) { configuration_reset(rhport); + // discard any pre-reset SETUP still counted: a stale count skips post-reset SETUPs + _usbd_queued_setup = 0; } bool tud_task_event_ready(void) { -- cgit v1.3.1 From 91fbbd192ca9539221d3dc096f00ce77836a5d3d Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 12 Aug 2026 23:05:59 +0700 Subject: usbd: clear endpoint busy/claimed when a completion event is dropped An XFER_COMPLETE dropped by a full event queue leaves its endpoint's BUSY|CLAIMED state set forever - the consumer that normally clears it never sees the event, so usbd_edpt_claim()/usbd_edpt_xfer() fail from then on and the class never re-arms the endpoint. Clear both flags when the enqueue fails: the completion is lost either way, but the endpoint stays usable. Unit test: arm a bulk endpoint, drop its completion against a full queue, verify the endpoint can be claimed and re-armed. --- src/device/usbd.c | 16 +++++++--- test/unit-test/test/device/usbd/test_usbd.c | 47 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) (limited to 'src/device') diff --git a/src/device/usbd.c b/src/device/usbd.c index 79802e70f..f5c3046d6 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -1475,10 +1475,18 @@ TU_ATTR_FAST_FUNC void dcd_event_handler(dcd_event_t const* event, bool in_isr) break; } - if (send && !queue_event(event, in_isr) && event->event_id == DCD_EVENT_SETUP_RECEIVED) { - // dropped by a full queue: undo the increment, else every later SETUP is skipped as - // "other SETUP in queue" and EP0 is deaf until re-init - _usbd_queued_setup--; + if (send && !queue_event(event, in_isr)) { + // event dropped by a full queue: undo state that would otherwise wedge permanently + if (event->event_id == DCD_EVENT_SETUP_RECEIVED) { + // undo the increment, else every later SETUP is skipped as "other SETUP in queue" + // and EP0 is deaf until re-init + _usbd_queued_setup--; + } else if (event->event_id == DCD_EVENT_XFER_COMPLETE) { + // clear busy + claimed, else the endpoint can never be claimed or re-armed again + uint8_t const epnum = tu_edpt_number(event->xfer_complete.ep_addr); + uint8_t const ep_dir = tu_edpt_dir(event->xfer_complete.ep_addr); + _usbd_dev.ep_status[epnum][ep_dir] &= (uint8_t) ~(TU_EDPT_STATE_BUSY | TU_EDPT_STATE_CLAIMED); + } } } diff --git a/test/unit-test/test/device/usbd/test_usbd.c b/test/unit-test/test/device/usbd/test_usbd.c index 935a20221..849097326 100644 --- a/test/unit-test/test/device/usbd/test_usbd.c +++ b/test/unit-test/test/device/usbd/test_usbd.c @@ -29,6 +29,7 @@ #include "tusb_fifo.h" #include "tusb.h" #include "usbd.h" +#include "device/usbd_pvt.h" TEST_SOURCE_FILE("usbd.c") // Mock File @@ -308,6 +309,52 @@ void test_usbd_setup_dropped_by_full_queue_recovers(void) tud_task(); } +//--------------------------------------------------------------------+ +// Transfer completion dropped by full event queue +//--------------------------------------------------------------------+ + +// When the event queue is full, queue_event() drops the XFER_COMPLETE event. The endpoint's +// busy/claimed state must not survive the dropped completion: a leaked BUSY makes every later +// usbd_edpt_claim()/usbd_edpt_xfer() on that endpoint fail, so the class never re-arms it. +void test_usbd_xfer_complete_dropped_by_full_queue_recovers(void) +{ + // fillers drain through usbd_reset -> class reset + mscd_reset_Ignore(); + + // open + claim + arm a bulk OUT endpoint the way a class driver would + tusb_desc_endpoint_t desc_ep = { + .bLength = sizeof(tusb_desc_endpoint_t), + .bDescriptorType = TUSB_DESC_ENDPOINT, + .bEndpointAddress = 0x01, + .bmAttributes = { .xfer = TUSB_XFER_BULK }, + .wMaxPacketSize = 64, + .bInterval = 0 + }; + static uint8_t xfer_buf[64]; + + dcd_edpt_open_ExpectAndReturn(rhport, &desc_ep, true); + TEST_ASSERT_TRUE(usbd_edpt_open(rhport, &desc_ep)); + TEST_ASSERT_TRUE(usbd_edpt_claim(rhport, 0x01)); + dcd_edpt_xfer_ExpectAndReturn(rhport, 0x01, xfer_buf, 64, false, true); + TEST_ASSERT_TRUE(usbd_edpt_xfer(rhport, 0x01, xfer_buf, 64, false)); + + // fill the queue to the brim, then complete the transfer: queue_event() drops it + for (unsigned i = 0; i < CFG_TUD_TASK_QUEUE_SZ; i++) { + dcd_event_bus_signal(rhport, DCD_EVENT_UNPLUGGED, false); + } + dcd_event_xfer_complete(rhport, 0x01, 64, XFER_RESULT_SUCCESS, false); + + // the endpoint must be re-armable: the dropped completion must not leak busy/claimed + TEST_ASSERT_TRUE(usbd_edpt_claim(rhport, 0x01)); + dcd_edpt_xfer_ExpectAndReturn(rhport, 0x01, xfer_buf, 64, false, true); + TEST_ASSERT_TRUE(usbd_edpt_xfer(rhport, 0x01, xfer_buf, 64, false)); + + // drain the fillers so later tests start from an empty queue + for (unsigned i = 0; i < (CFG_TUD_TASK_QUEUE_SZ / CFG_TUD_TASK_EVENTS_PER_RUN) + 1; i++) { + tud_task(); + } +} + //--------------------------------------------------------------------+ // Control OUT data stage host overrun //--------------------------------------------------------------------+ -- cgit v1.3.1 From 073942589355676980ba401cb88c0eb9f065e468 Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 17 Aug 2026 01:01:37 +0700 Subject: usbd: split bus reset into start/end edge events A driver that can see reset signalling begin has no way to say so: the only event carries the negotiated speed, which does not exist until the reset ends. On ChipIdea that left the stack believing it was still configured for the whole reset window - 3 ms at minimum, tens of milliseconds in practice - while the controller had already torn its endpoints down, so a class driver writing in that window primed a disabled endpoint over a zeroed queue head. Add DCD_EVENT_BUS_RESET_START for the leading edge and rename the existing event to DCD_EVENT_BUS_RESET_END, keeping DCD_EVENT_BUS_RESET as an alias. START is optional and END stays self-sufficient, so every other driver and the unit tests are untouched. --- src/device/dcd.h | 26 +++++++++++++++++--------- src/device/usbd.c | 12 ++++++++++-- 2 files changed, 27 insertions(+), 11 deletions(-) (limited to 'src/device') 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..7215a8dc5 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; -- cgit v1.3.1 From a85a6afc6d98726f5edfb2d7606527c87c963dba Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 17 Aug 2026 01:02:07 +0700 Subject: usbd: handle a refused transfer without halting, and report it A refused transfer is a recoverable condition - a new setup superseding a control response, for instance - rather than a bug, but every failure path treated it as one. TU_ASSERT carries TU_BREAKPOINT, which is gated on a debugger being attached rather than on CFG_TUSB_DEBUG, so on a rig where a probe is always attached it halted the CPU even in release builds. Use TU_VERIFY on the control transfer paths, including the multi-packet data stage continuation, and drop the breakpoint from the endpoint transfer failure arm, which already marks the endpoint ready again so the next transfer can proceed. The result of usbd_control_xfer_cb() was separately dropped on the floor, leaving EP0 neither armed nor stalled and nothing recorded. It is logged now, and deliberately not stalled: 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 would land after the auto-clear and stall the transfer that superseded this one. The pending setup re-drives EP0 by itself. --- src/device/usbd.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'src/device') diff --git a/src/device/usbd.c b/src/device/usbd.c index 7215a8dc5..e84d72fa4 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -757,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,); @@ -875,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; @@ -929,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); @@ -937,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; @@ -1608,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; } } -- cgit v1.3.1