fix(dcd_ch32_usbfs): fix dropped bulk OUT packets by properly re-arming EP_RX_CTRL state#3622
fix(dcd_ch32_usbfs): fix dropped bulk OUT packets by properly re-arming EP_RX_CTRL state#3622mickabrig7 wants to merge 1 commit into
Conversation
…ng EP_RX_CTRL state
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| ch32v103r_r1_1v0/dfu_runtime | 8,596 → 8,676 (+80) | — | — | — | 8,964 → 9,044 (+80) | +0.9% |
| ch32v103r_r1_1v0/hid_generic_inout | 9,644 → 9,720 (+76) | — | — | — | 10,012 → 10,088 (+76) | +0.8% |
| ch32v103r_r1_1v0/hid_boot_interface | 10,496 → 10,576 (+80) | — | — | — | 10,864 → 10,944 (+80) | +0.7% |
| ch32v103r_r1_1v0/midi_test | 10,836 → 10,916 (+80) | — | — | — | 11,204 → 11,284 (+80) | +0.7% |
| ch32v103r_r1_1v0/hid_multiple_interface | 10,480 → 10,556 (+76) | — | — | — | 10,848 → 10,924 (+76) | +0.7% |
| ch32v103r_r1_1v0/hid_composite | 10,720 → 10,796 (+76) | — | — | — | 11,088 → 11,164 (+76) | +0.7% |
| ch32v103r_r1_1v0/cdc_dual_ports | 11,848 → 11,928 (+80) | — | — | — | 12,216 → 12,296 (+80) | +0.7% |
| ch32v103r_r1_1v0/audio_test | 11,972 → 12,052 (+80) | — | — | — | 12,356 → 12,436 (+80) | +0.6% |
| ch32v103r_r1_1v0/printer_to_cdc | 12,016 → 12,096 (+80) | — | — | — | 12,376 → 12,456 (+80) | +0.6% |
| ch32v103r_r1_1v0/audio_test_multi_rate | 12,292 → 12,372 (+80) | — | — | — | 12,680 → 12,760 (+80) | +0.6% |
There was a problem hiding this comment.
Pull request overview
This PR targets reliability of bulk OUT transfers on WCH CH32 USBFS device controller by ensuring OUT endpoints are properly re-armed via EP_RX_CTRL, preventing missed receive callbacks and dropped packets during sustained high-rate transfers.
Changes:
- Update
update_out()to programEP_RX_CTRL(ACK/NAK) for non-EP0 OUT endpoints based on whether an OUT transfer is still active. - Update
dcd_edpt_xfer()to explicitly re-enable OUT endpoint RX ACK when a new OUT transfer is queued.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (ep == 0) { | ||
| EP_RX_CTRL(0) = USBFS_EP_R_RES_ACK; | ||
| } else { | ||
| EP_RX_CTRL(ep) = (EP_RX_CTRL(ep) & ~USBFS_EP_R_RES_MASK) | | ||
| (xfer->valid ? USBFS_EP_R_RES_ACK : USBFS_EP_R_RES_NAK); | ||
| } |
| if (dir == TUSB_DIR_IN) { | ||
| update_in(rhport, ep, true); | ||
| } else { | ||
| EP_RX_CTRL(ep) = (EP_RX_CTRL(ep) & ~USBFS_EP_R_RES_MASK) | USBFS_EP_R_RES_ACK; |
|
Thank you, I think we should also remove ACK enablement in I don't have CH30V20x at hand but surprised by their USB IP implementation, if CPU load is high setting NAK after packet received can still missing data. |
|
I bought a nanoch32v203 (and v305), I do observe OUT EP send ACK blindly without checking if data is actually read 😱 😱 Captured with CDC example: I think to fix it correctly we need to disable |
When stress-testing a custom Vendor interface with lots of fast bulk transfers on CH32V30x (which use the USBFS controller), I noticed that a lot of data was dropped because
tud_vendor_rx_cb()would not always fire.After investigating, it seemed like
dcd_ch32_usbfswasn't updating theEP_RX_CTRLregister after a transfer completed or when a new one was queued.update_out()now properly setsEP_RX_CTRLto ACK or NAK for all endpoints based on transfer state, anddcd_edpt_xfer()explicitly re-enables ACK when re-queuing an OUT transfer.Bulk transfers now complete reliably without data loss or missed callbacks.