dwc2: preserve EP0 status completion before SETUP#3634
Conversation
On STM32 DWC2, SETUP phase done and EP0 OUT transfer complete can be reported together. Processing SETUP first can overwrite control state before the previous zero-length OUT status stage is acknowledged, which causes DFU DNLOAD/GETSTATUS traffic to lose the status ACK and stall. Queue the EP0 OUT zero-length transfer completion before queuing the SETUP event when the endpoint has no pending OUT data and total_len is zero. This keeps TinyUSB control-transfer ordering intact for the combined interrupt case.
There was a problem hiding this comment.
Pull request overview
This PR adjusts the DWC2 (STM32) device controller driver to preserve TinyUSB control-transfer event ordering when DWC2 reports EP0 OUT transfer completion and SETUP phase done in the same interrupt, avoiding loss of the EP0 OUT status-stage ACK (notably impacting DFU DNLOAD/GETSTATUS sequences).
Changes:
- Detect the combined
setup_phase_done + xfer_completeinterrupt case on EP0 OUT with zero total length. - Enqueue an EP0 OUT zero-length
xfer_completeevent before enqueuing the SETUP event (in both slave and DMA paths).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static void handle_epout_slave(uint8_t rhport, uint8_t epnum, dwc2_doepint_t doepint_bm) { | ||
| xfer_ctl_t* xfer = XFER_CTL_BASE(epnum, TUSB_DIR_OUT); | ||
| const bool ep0_status_complete_before_setup = (epnum == 0) && doepint_bm.setup_phase_done && | ||
| doepint_bm.xfer_complete && |
| dwc2_regs_t* dwc2 = DWC2_REG(rhport); | ||
| xfer_ctl_t* xfer = XFER_CTL_BASE(epnum, TUSB_DIR_OUT); | ||
| const bool ep0_status_complete_before_setup = (epnum == 0) && doepint_bm.setup_phase_done && | ||
| doepint_bm.xfer_complete && |
Track when an EP0 OUT zero-length transfer is actually armed and require that state before synthesizing a status-stage completion ahead of a co-reported SETUP event. This preserves the validated status-before-SETUP ordering fix while avoiding stale zero-length state from producing spurious EP0 OUT completions.
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| raspberrypi_cm4/cdc_msc | 57,116 → 57,500 (+384) | 744 → 4,456 (+3,712) | — | — | 57,860 → 61,956 (+4,096) | +7.1% |
| raspberrypi_zero/audio_4_channel_mic | 57,040 → 57,420 (+380) | 1,767 → 5,483 (+3,716) | — | — | 58,807 → 62,903 (+4,096) | +7.0% |
| raspberrypi_zero/dynamic_configuration | 61,212 → 61,592 (+380) | 1,147 → 4,863 (+3,716) | — | — | 62,359 → 66,455 (+4,096) | +6.6% |
| xmc4500_relax/dfu_runtime | 12,048 → 12,176 (+128) | — | — | — | 24,148 → 24,404 (+256) | +1.1% |
| xmc4500_relax/hid_multiple_interface | 13,712 → 13,840 (+128) | — | — | — | 27,476 → 27,732 (+256) | +0.9% |
| xmc4500_relax/hid_boot_interface | 13,732 → 13,860 (+128) | — | — | — | 27,516 → 27,772 (+256) | +0.9% |
| sipeed_longan_nano/audio_test | 16,412 → 16,604 (+192) | — | — | 1,612 → 1,616 (+4) | 21,294 → 21,490 (+196) | +0.9% |
| xmc4500_relax/cdc_dual_ports | 14,908 → 15,036 (+128) | — | — | — | 29,868 → 30,124 (+256) | +0.9% |
| xmc4500_relax/printer_to_cdc | 14,968 → 15,096 (+128) | — | — | — | 29,984 → 30,240 (+256) | +0.9% |
| xmc4500_relax/audio_test | 15,060 → 15,188 (+128) | — | — | — | 30,192 → 30,448 (+256) | +0.8% |
|
Thank you, with the test method in #3643 DFU has no more error. Are you still working on it ? |
|
Nice to hear! I have it currently paused, need to finish a MVP product first, then I can test in more detail and look further into it, might take a week or two. But I can briefly try with the PR branch you linked on Monday if our functionality on STM32U5 with DFU still works and report back. |
|
@HiFiPhile I now had time to take a look: Using the branch from #3643, DFU works. But, the fix is not in this PR, I think the actual fix is in previous #3640 with DWC2 changes, using commit |
|
Well, I now realised my dfu-util I tried with different USB hubs and cables, but currently even the old commit Since I don't have a USB sniffer to actually look into the protocol details, I can only test functionality. I'll see if I can find out what needs to change to trigger the DFU failure again. Maybe with delaying the USB interrupt handling within my application I could force the error again. |
Observed Problem
xfer_completeandsetup_phase_donetogether.DNLOADSETUP arrived while DFU was still inDFU_DNLOAD_SYNC, causingLIBUSB_ERROR_PIPE.Implemented Fix
dcd_event_xfer_complete()beforedcd_event_setup_received()when both are co-reported.ep0_out_zlp_armedlatch so the synthetic completion only fires if EP0 OUT ZLP was actually armed.Remaining Upstream-Polish Question
dcd_edpt_xfer_fifo()does not updateep0_out_zlp_armed.dcd_edpt_xfer().Current status
Hardware validated on STM32U5G9J custom PCB with USB CDC, USB DFU device and HS USB host for flash drives. Should be in accordance to ST
RM0456for STM32U5, section73 USB on-the-go high-speed (OTG_HS).No further tests/validation currently done.
More tests with STM32U5G9J-DK2 evalboard and reproducable examples are planned, but I don't have time for it now.