dcd/dwc2: fix EP0 multi-packet transfer logic #3295

HiFiPhile · 2025-10-10T11:53:26Z

Describe the PR
There was an issue in edpt_schedule_packets, as ep0_pending = xfer->max_size is set in dcd_edpt_xfer, ep0_pending will become 0.

if (epnum == 0) {
    total_bytes = tu_min16(_dcd_data.ep0_pending[dir], xfer->max_size);
    _dcd_data.ep0_pending[dir] -= total_bytes;
    num_packets = 1;
  }

The issue didn't broken since we schedule only one control transfer each time.

I've add an option CFG_TUD_EP0_BUFSIZE to allow setting EP0 buffer size individually, tested with DFU examples withCFG_TUD_EP0_BUFSIZE=512

…m_packets = 1 Signed-off-by: HiFiPhile <[email protected]>

Signed-off-by: HiFiPhile <[email protected]>

Copilot

Pull Request Overview

This PR fixes EP0 multi-packet transfer logic in the DWC2 driver and adds configurable EP0 buffer size support. The issue was in the edpt_schedule_packets function where ep0_pending was incorrectly being set to xfer->max_size instead of the proper endpoint size, causing transfer size calculation errors.

Key changes:

Added CFG_TUD_EP0_BUFSIZE configuration option for EP0 buffer size
Fixed EP0 packet scheduling logic to use correct endpoint size
Enhanced packet transmission handling for single vs multi-packet transfers
Updated DFU examples with BOS descriptors and Microsoft OS 2.0 compatibility

Reviewed Changes

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

Show a summary per file

File	Description
src/tusb_option.h	Added CFG_TUD_EP0_BUFSIZE configuration option
src/portable/synopsys/dwc2/dcd_dwc2.c	Fixed EP0 transfer logic and improved packet handling
src/device/usbd_control.c	Updated to use new CFG_TUD_EP0_BUFSIZE configuration
examples/device/dfu_runtime/src/usb_descriptors.c	Added BOS descriptor and Microsoft OS 2.0 support
examples/device/dfu/src/usb_descriptors.c	Added BOS descriptor and Microsoft OS 2.0 support

examples/device/dfu_runtime/src/usb_descriptors.c

examples/device/dfu/src/usb_descriptors.c

Signed-off-by: HiFiPhile <[email protected]>

examples/device/dfu/src/usb_descriptors.c

examples/device/dfu_runtime/src/usb_descriptors.c

Signed-off-by: HiFiPhile <[email protected]>

…to/from use EP0 buffer. Signed-off-by: HiFiPhile <[email protected]>

# Conflicts: # examples/device/dfu/src/usb_descriptors.c # examples/device/dfu_runtime/src/usb_descriptors.c # src/portable/synopsys/dwc2/dcd_dwc2.c

src/device/usbd_control.c

  // a short packet is sent including zero-length packet.
  if ((_ctrl_xfer.request.wLength == _ctrl_xfer.total_xferred) ||
-      (xferred_bytes < CFG_TUD_ENDPOINT0_SIZE)) {
+      (xferred_bytes < CFG_TUD_EP0_BUFSIZE)) {


examples/device/dfu/src/usb_descriptors.c

examples/device/dfu_runtime/src/usb_descriptors.c

hathach

I forgot most of the dwc2 driver, didn't re-check the specs. These are 2 things pop-up in my mind when we are making control transfer faster like bulk.

PS: also I enable a few code scanning tools (free for OSS), still testing it out, so it can be very verbose atm

hathach · 2025-10-29T20:23:50Z

src/portable/synopsys/dwc2/dcd_dwc2.c

    // Enable tx fifo empty interrupt only if there is data. Note must after depctl enable
    if (dir == TUSB_DIR_IN && total_bytes != 0) {
-      dwc2->diepempmsk |= (1u << epnum);
+      // For num_packets = 1 we write the packet directly


I think we should keep using the TXFE interrupt to write to fifo. Otherwise there could be race condition whereas usbd mis-manage control transfer, and we are attempting to xfer on busy endpoint0.

also I think we need to also increase (double) the dfifo_alloc() for EP0 when the EP0_BUFSIZE is large enough e.g 8x64 bytes since user is clearly want to make a large transfer with EP0

I think we should keep using the TXFE interrupt to write to fifo. Otherwise there could be race condition whereas usbd mis-manage control transfer, and we are attempting to xfer on busy endpoint0.

I think it should be safe, which scenario you are thinking about ?

also I think we need to also increase (double) the dfifo_alloc() for EP0 when the EP0_BUFSIZE is large enough e.g 8x64 bytes since user is clearly want to make a large transfer with EP0

EP0 IN can only queue 3 packets each time, EP0 OUT can only queue 1 packet each time, so increasing the fifo won't have much help.

I think it should be safe, which scenario you are thinking about ?

You are right, I just feel a bit un-easy when both handle_epin_slave() and edpt_schedule_packets() can write to difof. Especially handle_epin_slave() also called edpt_schedule_packets() in case of ep0. But it is probably safe enough, actually we can refactor this and try to write as many packet as we could before enable the TX empty if there is remaining packet ?

dwc2_ep_tsize_t tsiz = {.value = epin->tsiz}; const uint16_t remain_packets = tsiz.packet_count; // Process every single packet (only whole packets can be written to fifo) for (uint16_t i = 0; i < remain_packets; i++) { tsiz.value = epin->tsiz; const uint16_t remain_bytes = (uint16_t) tsiz.xfer_size; const uint16_t xact_bytes = tu_min16(remain_bytes, xfer->max_size); // Check if dtxfsts has enough space available if (xact_bytes > ((epin->dtxfsts & DTXFSTS_INEPTFSAV_Msk) << 2)) { break; } // Push packet to Tx-FIFO if (xfer->ff) { volatile uint32_t* tx_fifo = dwc2->fifo[epnum]; tu_fifo_read_n_const_addr_full_words(xfer->ff, (void*)(uintptr_t)tx_fifo, xact_bytes); } else { dfifo_write_packet(dwc2, epnum, xfer->buffer, xact_bytes); xfer->buffer += xact_bytes; } }

EP0 IN can only queue 3 packets each time, EP0 OUT can only queue 1 packet each time, so increasing the fifo won't have much help.

right, again, I am too lazy to check the specs, the increased ep0 bufsize only help to reduce the overhead when pushing isr --> usbd event stack. Let keep the ep0 1 packet per xfer is good enough.

I've refactored the fifo writing routine and added a tunable CFG_TUD_DWC2_IN_DOUBLE_BUFFER to set GAHBCFG_TX_FIFO_EPMTY_LVL and fifo allocation, in order to allow writing 2 packets to fifo.
By default we allocate fifo size=packet size, the writing loop will likely stop after one packet is written.

hathach · 2025-10-29T20:39:32Z

src/portable/synopsys/dwc2/dcd_dwc2.c

  if (epnum == 0) {
-    total_bytes = tu_min16(_dcd_data.ep0_pending[dir], xfer->max_size);
+    total_bytes = tu_min16(_dcd_data.ep0_pending[dir], CFG_TUD_ENDPOINT0_SIZE);
    _dcd_data.ep0_pending[dir] -= total_bytes;


I think we should not pre-calculate the pending, we should just store the total_bytes and minus xferred_bytes in complete isr instead like other bulk transfers

I think we can't get rid of ep0_pending, otherwise we need to add ep0_xferred_bytes which ends up the same thing, just one counting up another counting down.

I agree, ep0_xferred_bytes is better since it will only increased when xfer is complete, and we can also use that in case of error for xferred so far. Maybe i will do that in follow-up PR later.

For OUT transfer it's easy to get ep0_xferred_bytes, but for IN transfer it can't be read from register, we have to add something like last_xfer_bytes and set it in edpt_schedule_packets. I don't know if it's worth the change.

you are right, sorry, I forgot some of these thing.

Signed-off-by: Mengsk <[email protected]>

hathach · 2025-10-31T04:14:49Z

src/tusb_option.h


+// Allocate double buffer for IN endpoint for better performance (only applies to non-periodic endpoints)
+#ifndef CFG_TUD_DWC2_IN_DOUBLE_BUFFER
+  #define CFG_TUD_DWC2_IN_DOUBLE_BUFFER 0


I think we should not introduce this option. I always think the fine-tune fifo should be dictated by application per endpoint basic. E.g MSC + CDC, we would want a massive endpoint size for msc and smaller for cdc. I am think we should do that via a tud_configure() (which allo to pass an option id + pointer to struct) that fine tune the behavior of dcd or stack. similar to tuh_configure() https://github.com/hathach/tinyusb/blob/master/src/host/usbh.h#L145 . dwc2 option may include grxsize and txfifo size for each endpoint (0 for auto as current).

Yeah a configurable parameter is better, didn't notice there is a tuh_configure().

hathach · 2025-10-31T04:37:23Z

src/portable/synopsys/dwc2/dcd_dwc2.c

  if (epnum == 0) {
-    total_bytes = tu_min16(_dcd_data.ep0_pending[dir], xfer->max_size);
+    total_bytes = tu_min16(_dcd_data.ep0_pending[dir], CFG_TUD_ENDPOINT0_SIZE);
    _dcd_data.ep0_pending[dir] -= total_bytes;


you are right, sorry, I forgot some of these thing.

hathach · 2025-10-31T04:47:20Z

src/portable/synopsys/dwc2/dcd_dwc2.c

-        xfer->buffer += xact_bytes;
-      }
-    }
+    epin_write_tx_fifo(rhport, epnum);


we have a possibility to call epin_write_tx_fifo() 2nd time with ep0. although it would not write any bytes, I think we should skip this for ep0.

You are right.

Signed-off-by: HiFiPhile <[email protected]>

sonarqubecloud · 2025-10-31T10:07:30Z

Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

HiFiPhile added 2 commits October 10, 2025 13:42

dcd/dwc2: fix EP0 mulit-packet logic, write the packet directly if nu…

d4fa891

…m_packets = 1 Signed-off-by: HiFiPhile <[email protected]>

usbd: support set EP0 buffer size

37592da

Signed-off-by: HiFiPhile <[email protected]>

Copilot AI review requested due to automatic review settings October 10, 2025 11:53

github-actions bot added the Prio Higher label Oct 10, 2025

Copilot AI reviewed Oct 10, 2025

View reviewed changes

examples/device/dfu_runtime/src/usb_descriptors.c Outdated Show resolved Hide resolved

examples/device/dfu/src/usb_descriptors.c Outdated Show resolved Hide resolved

HiFiPhile force-pushed the dwc2_ep0 branch from 9d413ec to 42f93de Compare October 10, 2025 11:55

example/dfu: add MSOS 2.0 descriptor for auto driver loading

c566db8

Signed-off-by: HiFiPhile <[email protected]>

HiFiPhile force-pushed the dwc2_ep0 branch from 42f93de to c566db8 Compare October 10, 2025 12:01

github-advanced-security bot found potential problems Oct 10, 2025

View reviewed changes

HiFiPhile added 2 commits October 11, 2025 16:09

Merge branch 'master' into dwc2_ep0

785454a

Signed-off-by: HiFiPhile <[email protected]>

dfu: remove transfer buffer out of USB section, since data will copy …

4a52edd

…to/from use EP0 buffer. Signed-off-by: HiFiPhile <[email protected]>

HiFiPhile force-pushed the dwc2_ep0 branch from 3cb9a72 to 4a52edd Compare October 11, 2025 14:31

Merge branch 'master' into dwc2_ep0

4da7845

# Conflicts: # examples/device/dfu/src/usb_descriptors.c # examples/device/dfu_runtime/src/usb_descriptors.c # src/portable/synopsys/dwc2/dcd_dwc2.c

This comment was marked as outdated.

Sign in to view

github-advanced-security bot found potential problems Oct 29, 2025

View reviewed changes

examples/device/dfu/src/usb_descriptors.c Fixed Show fixed Hide fixed

examples/device/dfu_runtime/src/usb_descriptors.c Fixed Show fixed Hide fixed

hathach reviewed Oct 29, 2025

View reviewed changes

Refactor IN fifo write

7c4cc0f

Signed-off-by: Mengsk <[email protected]>

codescene-delta-analysis bot approved these changes Oct 30, 2025

View reviewed changes

hathach reviewed Oct 31, 2025

View reviewed changes

HiFiPhile added 2 commits October 31, 2025 10:56

Add boundary check

cee7937

Signed-off-by: HiFiPhile <[email protected]>

Skip TXFE for EP0

be4b38c

Signed-off-by: HiFiPhile <[email protected]>

HiFiPhile force-pushed the dwc2_ep0 branch from 5cf081b to be4b38c Compare October 31, 2025 10:05

codescene-delta-analysis bot approved these changes Oct 31, 2025

View reviewed changes

dcd/dwc2: fix EP0 multi-packet transfer logic #3295

Are you sure you want to change the base?

dcd/dwc2: fix EP0 multi-packet transfer logic #3295

Uh oh!

Conversation

HiFiPhile commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Check warning

Uh oh!

Uh oh!

hathach left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HiFiPhile Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 31, 2025

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

HiFiPhile commented Oct 10, 2025 •

edited

Loading

hathach left a comment •

edited

Loading

HiFiPhile Oct 29, 2025 •

edited

Loading