Skip to content

dcd/musb: defer EP0 SETUP during DATA_IN/STATUS race#3643

Open
HiFiPhile wants to merge 3 commits into
masterfrom
musb_ep0_race
Open

dcd/musb: defer EP0 SETUP during DATA_IN/STATUS race#3643
HiFiPhile wants to merge 3 commits into
masterfrom
musb_ep0_race

Conversation

@HiFiPhile

@HiFiPhile HiFiPhile commented May 14, 2026

Copy link
Copy Markdown
Collaborator

Handle cases where a new SETUP arrives before the previous control transfer fully completes by buffering the SETUP and replaying it after status completion. Split EP0 DATA state into DATA_IN/DATA_OUT and finalize pending status-out completion before processing deferred SETUP.

Take care these 2 cases if new SETUP packet arrived while old control transfer is not finished yet. This could happen in following scenarios when CPU load is high:

  • Status IN/OUT finished, IRQ and new setup packet IRQ arrive at the same time.
  • Data IN finished and status OUT is received, both IRQs and new setup packet IRQ arrive at the same time.

Supercede #3626

Tested intensively with DFU example with Interrupt turned on/off every 20ms simulating high CP load:

diff --git a/examples/device/dfu/src/main.c b/examples/device/dfu/src/main.c
index fb3c22630..d4e75a44b 100644
--- a/examples/device/dfu/src/main.c
+++ b/examples/device/dfu/src/main.c
@@ -45,6 +45,7 @@
 #include "bsp/board_api.h"
 #include "tusb.h"
 
+#include "device/dcd.h"
 //--------------------------------------------------------------------+
 // MACRO CONSTANT TYPEDEF PROTYPES
 //--------------------------------------------------------------------+
@@ -65,6 +66,7 @@ enum {
 static uint32_t blink_interval_ms = BLINK_NOT_MOUNTED;
 
 void led_blinking_task(void);
+static void musb_test(void);
 
 /*------------- MAIN -------------*/
 int main(void) {
@@ -79,6 +81,7 @@ int main(void) {
   while (1) {
     tud_task(); // tinyusb device task
     led_blinking_task();
+    musb_test();
   }
 }
 
@@ -86,6 +89,26 @@ int main(void) {
 // Device callbacks
 //--------------------------------------------------------------------+
 
+// on off usb interrupt 20ms for testing
+static void musb_test(void) {
+  static uint32_t start_ms  = 0;
+  static bool irq_enabled = true;
+
+  if (tusb_time_millis_api() - start_ms < 20) {
+    return; // not enough time
+  }
+
+  if (irq_enabled) {
+    dcd_int_disable(0);
+    tusb_time_delay_ms_api(20);
+    dcd_int_enable(0);
+  } else {
+  }
+
+  irq_enabled = !irq_enabled;
+  start_ms += 20;
+}
+
 // Invoked when device is mounted
 void tud_mount_cb(void) {
   blink_interval_ms = BLINK_MOUNTED;
@@ -122,7 +145,7 @@ uint32_t tud_dfu_get_timeout_cb(uint8_t alt, uint8_t state) {
     // For this example
     // - Atl0 Flash is fast : 1   ms
     // - Alt1 EEPROM is slow: 100 ms
-    return (alt == 0) ? 1 : 100;
+    return (alt == 0) ? 0 : 0;
   } else if (state == DFU_MANIFEST) {
     // since we don't buffer entire image and do any flashing in manifest stage
     return 0;
@@ -142,9 +165,9 @@ void tud_dfu_download_cb(uint8_t alt, uint16_t block_num, const uint8_t *data, u
 
   //printf("\r\nReceived Alt %u BlockNum %u of length %u\r\n", alt, wBlockNum, length);
 
-  for (uint16_t i = 0; i < length; i++) {
-    printf("%c", data[i]);
-  }
+  //for (uint16_t i = 0; i < length; i++) {
+  //  printf("%c", data[i]);
+  //}
 
   // flashing op for download complete without error
   tud_dfu_finish_flashing(DFU_STATUS_OK);

Copilot AI review requested due to automatic review settings May 14, 2026 17:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Mentor MUSB device controller (DCD) EP0 control-transfer state machine to better handle a race where a new SETUP arrives before the prior control transfer has fully completed (typically under high CPU load). It also adjusts MUSB port headers to avoid multiple-definition linkage issues around MUSB_BASES.

Changes:

  • Split EP0 DATA state into explicit DATA_IN / DATA_OUT states and add buffering of a “deferred” SETUP request to replay after status completion.
  • Add logic to complete pending STATUS stages before dispatching the deferred SETUP.
  • Make MUSB_BASES static const in MUSB port headers to prevent ODR/multiple-definition problems.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/portable/mentor/musb/musb_ti.h Make MUSB_BASES static const to avoid multiple definitions across translation units.
src/portable/mentor/musb/musb_max32.h Same MUSB_BASES linkage fix for the MAX32 port.
src/portable/mentor/musb/dcd_musb.c Rework EP0 control-transfer state machine and add deferred-SETUP buffering/replay to handle SETUP vs status/data completion races.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/portable/mentor/musb/dcd_musb.c Outdated
Comment thread src/portable/mentor/musb/dcd_musb.c
@github-actions

github-actions Bot commented May 14, 2026

Copy link
Copy Markdown

TinyUSB Average Code Size Metrics

File .text .rodata .data .bss size %
ehci.c 2763 0 0 6274 7783 4.4%
midi2_host.c 1802 0 0 5921 7723 4.3%
cdc_host.c 6381 487 15 985 7579 4.3%
ncm_device.c 1757 28 815 4354 6124 3.4%
usbh.c 4662 57 108 1022 5813 3.3%
hcd_dwc2.c 5014 25 1 513 5553 3.1%
midi_host.c 1341 7 7 3635 4979 2.8%
video_device.c 4443 5 1235 479 4914 2.8%
dcd_dwc2.c 4263 19 0 265 4546 2.6%
audio_device.c 2896 0 1259 1625 4515 2.5%
ohci.c 1925 0 0 2503 4428 2.5%
dcd_ch32_usbfs.c 1582 0 0 2444 4026 2.3%
usbd.c 3519 58 91 355 3936 2.2%
ecm_rndis_device.c 1059 0 1 2759 3818 2.1%
hcd_stm32_fsdev.c 3257 0 1 420 3678 2.1%
dcd_ft9xx.c 3284 0 0 172 3456 1.9%
msc_device.c 2517 108 2281 806 3323 1.9%
dcd_khci.c 1952 0 0 1290 3242 1.8%
dcd_nrf5x.c 2939 0 0 292 3231 1.8%
hcd_musb.c 3071 0 0 157 3228 1.8%
midi2_device.c 2634 52 1175 566 3222 1.8%
dcd_ci_fs.c 1924 0 0 1290 3214 1.8%
dcd_da146xx.c 3067 0 0 144 3211 1.8%
hcd_rusb2.c 2923 0 0 245 3168 1.8%
dcd_rusb2.c 2918 0 0 156 3074 1.7%
hcd_ch32_usbfs.c 2489 0 0 502 2991 1.7%
hcd_khci.c 2443 0 0 454 2897 1.6%
dcd_stm32_fsdev.c 2558 0 0 291 2849 1.6%
dcd_mm32f327x_otg.c 1477 0 0 1290 2767 1.6%
dcd_musb.c 2451 0 0 179 2629 1.5%
usbtmc_device.c 2196 24 68 316 2544 1.4%
hcd_samd.c 2220 0 0 324 2544 1.4%
dcd_ci_hs.c 1756 0 0 1344 2534 1.4%
dcd_eptri.c 2273 0 0 259 2532 1.4%
hid_host.c 1240 0 0 1251 2491 1.4%
dcd_ch32_usbhs.c 1889 0 0 481 2370 1.3%
hcd_rp2040.c 1996 17 4 321 2338 1.3%
mtp_device.c 1706 22 739 588 2302 1.3%
dcd_rp2040.c 840 0 764 653 2257 1.3%
msc_host.c 1638 0 0 394 2033 1.1%
dcd_msp430x5xx.c 1801 0 0 176 1977 1.1%
cdc_device.c 1239 16 1092 735 1972 1.1%
dcd_lpc17_40.c 1481 0 0 648 1805 1.0%
midi_device.c 1151 0 1007 624 1773 1.0%
dcd_nuc505.c 0 0 1533 157 1690 1.0%
dcd_lpc_ip3511.c 1463 0 0 264 1683 0.9%
hub.c 1384 8 8 30 1418 0.8%
printer_device.c 830 0 706 566 1394 0.8%
dcd_samg.c 1322 0 0 72 1394 0.8%
dcd_samd.c 1036 0 0 266 1302 0.7%
dcd_nuc121.c 1170 0 0 101 1270 0.7%
hid_device.c 1125 44 997 119 1244 0.7%
vendor_device.c 641 0 534 565 1204 0.7%
dcd_nuc120.c 1096 0 0 78 1174 0.7%
rp2040_usb.c 386 35 619 11 1051 0.6%
dfu_device.c 777 28 712 138 914 0.5%
tusb_fifo.c 852 0 486 0 846 0.5%
typec_stm32.c 820 8 2 12 842 0.5%
dwc2_common.c 603 22 0 0 615 0.3%
usbc.c 420 2 20 166 608 0.3%
hcd_pio_usb.c 262 0 240 0 502 0.3%
tusb.c 455 0 387 3 457 0.3%
hcd_ci_hs.c 181 0 0 0 181 0.1%
fsdev_common.c 180 0 0 0 180 0.1%
rusb2_common.c 160 0 16 0 176 0.1%
dfu_rt_device.c 157 0 134 0 157 0.1%
TOTAL 124057 1072 17057 52050 177691 100.0%
Input files
  • cmake-build/cmake-build-adafruit_clue/metrics.json
  • cmake-build/cmake-build-apard32690/metrics.json
  • cmake-build/cmake-build-at32f403a_weact_blackpill/metrics.json
  • cmake-build/cmake-build-at_start_f402/metrics.json
  • cmake-build/cmake-build-at_start_f413/metrics.json
  • cmake-build/cmake-build-at_start_f415/metrics.json
  • cmake-build/cmake-build-at_start_f423/metrics.json
  • cmake-build/cmake-build-at_start_f425/metrics.json
  • cmake-build/cmake-build-at_start_f435/metrics.json
  • cmake-build/cmake-build-at_start_f455/metrics.json
  • cmake-build/cmake-build-b_g474e_dpow1/metrics.json
  • cmake-build/cmake-build-b_u585i_iot2a/metrics.json
  • cmake-build/cmake-build-ch32f205r-r0/metrics.json
  • cmake-build/cmake-build-ch32v103r_r1_1v0/metrics.json
  • cmake-build/cmake-build-ch32v203c_r0_1v0/metrics.json
  • cmake-build/cmake-build-ch32v307v_r1_1v0/metrics.json
  • cmake-build/cmake-build-cynthion_d11/metrics.json
  • cmake-build/cmake-build-da14695_dk_usb/metrics.json
  • cmake-build/cmake-build-double_m33_express/metrics.json
  • cmake-build/cmake-build-ea4088_quickstart/metrics.json
  • cmake-build/cmake-build-ea4357/metrics.json
  • cmake-build/cmake-build-ek_tm4c123gxl/metrics.json
  • cmake-build/cmake-build-feather_stm32f405/metrics.json
  • cmake-build/cmake-build-fomu/metrics.json
  • cmake-build/cmake-build-frdm_k32l2a4s/metrics.json
  • cmake-build/cmake-build-frdm_k64f/metrics.json
  • cmake-build/cmake-build-frdm_kl25z/metrics.json
  • cmake-build/cmake-build-frdm_mcxa153/metrics.json
  • cmake-build/cmake-build-frdm_rw612/metrics.json
  • cmake-build/cmake-build-hpm6750evk2/metrics.json
  • cmake-build/cmake-build-lpcxpresso11u37/metrics.json
  • cmake-build/cmake-build-lpcxpresso1347/metrics.json
  • cmake-build/cmake-build-lpcxpresso1549/metrics.json
  • cmake-build/cmake-build-lpcxpresso1769/metrics.json
  • cmake-build/cmake-build-lpcxpresso18s37/metrics.json
  • cmake-build/cmake-build-lpcxpresso51u68/metrics.json
  • cmake-build/cmake-build-lpcxpresso54114/metrics.json
  • cmake-build/cmake-build-metro_m0_express/metrics.json
  • cmake-build/cmake-build-metro_m4_express/metrics.json
  • cmake-build/cmake-build-metro_m7_1011/metrics.json
  • cmake-build/cmake-build-mm32f327x_mb39/metrics.json
  • cmake-build/cmake-build-mm900evxb/metrics.json
  • cmake-build/cmake-build-msp_exp430f5529lp/metrics.json
  • cmake-build/cmake-build-msp_exp432e401y/metrics.json
  • cmake-build/cmake-build-nutiny_nuc126v/metrics.json
  • cmake-build/cmake-build-nutiny_sdk_nuc120/metrics.json
  • cmake-build/cmake-build-nutiny_sdk_nuc121/metrics.json
  • cmake-build/cmake-build-nutiny_sdk_nuc505/metrics.json
  • cmake-build/cmake-build-portenta_c33/metrics.json
  • cmake-build/cmake-build-raspberry_pi_pico/metrics.json
  • cmake-build/cmake-build-raspberrypi_cm4/metrics.json
  • cmake-build/cmake-build-raspberrypi_zero/metrics.json
  • cmake-build/cmake-build-samg55_xplained/metrics.json
  • cmake-build/cmake-build-sipeed_longan_nano/metrics.json
  • cmake-build/cmake-build-stlinkv3mini/metrics.json
  • cmake-build/cmake-build-stm32c071nucleo/metrics.json
  • cmake-build/cmake-build-stm32f070rbnucleo/metrics.json
  • cmake-build/cmake-build-stm32f103_bluepill/metrics.json
  • cmake-build/cmake-build-stm32f207nucleo/metrics.json
  • cmake-build/cmake-build-stm32f303disco/metrics.json
  • cmake-build/cmake-build-stm32g0b1nucleo/metrics.json
  • cmake-build/cmake-build-stm32h503nucleo/metrics.json
  • cmake-build/cmake-build-stm32h743eval/metrics.json
  • cmake-build/cmake-build-stm32h7s3nucleo/metrics.json
  • cmake-build/cmake-build-stm32l052dap52/metrics.json
  • cmake-build/cmake-build-stm32l412nucleo/metrics.json
  • cmake-build/cmake-build-stm32n6570dk/metrics.json
  • cmake-build/cmake-build-stm32u083cdk/metrics.json
  • cmake-build/cmake-build-stm32wb55nucleo/metrics.json
  • cmake-build/cmake-build-stm32wba_nucleo/metrics.json
  • cmake-build/cmake-build-xmc4500_relax/metrics.json

@github-actions

github-actions Bot commented May 14, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

Top 10 targets by memory change (%) (out of 2346 targets) View Project Dashboard →

target .text .rodata .data .bss total % diff
msp_exp432e401y/dfu_runtime 8,592 → 8,816 (+224) 1,048 → 1,056 (+8) 10,372 → 10,604 (+232) +2.2%
msp_exp432e401y/hid_generic_inout 9,644 → 9,868 (+224) 1,252 → 1,260 (+8) 11,628 → 11,860 (+232) +2.0%
msp_exp432e401y/hid_multiple_interface 10,428 → 10,652 (+224) 1,128 → 1,136 (+8) 12,288 → 12,520 (+232) +1.9%
msp_exp432e401y/hid_boot_interface 10,440 → 10,664 (+224) 1,128 → 1,136 (+8) 12,300 → 12,532 (+232) +1.9%
msp_exp432e401y/hid_composite 10,652 → 10,876 (+224) 1,116 → 1,124 (+8) 12,500 → 12,732 (+232) +1.9%
msp_exp432e401y/midi_test 10,748 → 10,972 (+224) 1,248 → 1,256 (+8) 12,728 → 12,960 (+232) +1.8%
ek_tm4c123gxl/dfu_runtime 8,540 → 8,772 (+232) 612 → 620 (+8) 13,316 → 13,556 (+240) +1.8%
msp_exp432e401y/cdc_dual_ports 11,592 → 11,816 (+224) 1,432 → 1,440 (+8) 13,756 → 13,988 (+232) +1.7%
msp_exp432e401y/printer_to_cdc 11,716 → 11,940 (+224) 1,412 → 1,420 (+8) 13,852 → 14,084 (+232) +1.7%
ek_tm4c123gxl/hid_generic_inout 9,536 → 9,768 (+232) 816 → 824 (+8) 14,512 → 14,752 (+240) +1.7%

Handle cases where a new SETUP arrives before the previous control transfer fully completes by buffering the SETUP and replaying it after status completion. Split EP0 DATA state into DATA_IN/DATA_OUT and finalize pending status-out completion before processing deferred SETUP.

Signed-off-by: HiFiPhile <admin@hifiphile.com>
Signed-off-by: HiFiPhile <admin@hifiphile.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants