dcd/dwc2: fix enumeration when EP0 size=8 #3279
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes USB enumeration issues with the DWC2 USB controller when EP0 max packet size is 8 bytes by ensuring the RX FIFO size is at least 64 words. Additionally, it updates the CMake preset generation to handle Espressif boards with Ninja single config generator.
- Fixed DWC2 RX FIFO allocation to use minimum 64 words regardless of EP0 size
- Corrected EP0 max packet size handling during bus reset to use configured size
- Updated preset generation to separate Espressif boards with single-config Ninja generator
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/portable/synopsys/dwc2/dcd_dwc2.c | Fixed RX FIFO sizing and EP0 packet size handling for proper enumeration |
| tools/gen_presets.py | Added logic to generate separate presets for Espressif boards with Ninja generator |
| hw/bsp/BoardPresets.json | Reorganized board presets to use single config for Espressif boards |
96ab8d2 to
71239f0
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting |
|
|
||
| TU_ATTR_ALWAYS_INLINE static inline uint16_t calc_device_grxfsiz(uint16_t largest_ep_size, uint8_t ep_count) { | ||
| return 13 + 1 + 2 * ((largest_ep_size / 4) + 1) + 2 * ep_count; | ||
| return 13 + 1 + 2 * ((largest_ep_size / 4) + 1) + 1 * (2 * ep_count) + 2 * ep_count; |
There was a problem hiding this comment.
is it correct that we need 1*2*ep_count, grx is for EPOUT only ? Does EP in disabled status also take a byte from grxfifo as well ?
There was a problem hiding this comment.
I think the manual means every endpoint:
❑ An additional location for EPDisable status for each endpoint is also required.
And we also need to add another field:
❑ Along with last packet of each endpoint, transfer complete status information is also pushed in to the FIFO.
There was a problem hiding this comment.
programming guide 2.1.3 method 2 to calculate shared rx
There was a problem hiding this comment.
Hum the manual is unclear, it says in Dedicated TxFIFO mode RxFIFO is the same as Shared TxFIFO mode. Then transfer complete status and EPDisable status mentioned in 2.1.1.1 are not in 2.1.3.1 formula...
I'm going to do some tests of minimal buffer size required.
There was a problem hiding this comment.
When I retest I found it is still broken with a hub.
With that hub 1st Get Descriptor Device length is 18 bytes, and enumeration is broken by #1102
USBD Setup Received 80 06 00 01 00 00 12 00
Get Descriptor Device
Queue EP 80 with 8 bytes ...
USBD Xfer Complete on EP 80 with 8 bytes
Queue EP 80 with 8 bytes ...
In my test USB 3 CV set wLength to 8 for 1st request, may we change #1102 to use 8 bytes length ? @kasjer
Current formula is correct in fact, the real issue is MPS of EP0 needs to be set accordingly, without setting MPS to 8 more FIFO size will be needed.
There was a problem hiding this comment.
@HiFiPhile let me retest with USB 3 CV tool, it was failing with Dialog (now Renesas) device DA1469x.
There was a problem hiding this comment.
Sorry I must mixed up the capture data, in device selection phase it uses wLength=8, but with actual test wLength is indeed 18.
There was a problem hiding this comment.
Good news, there is no need to change usbd.c, my hub will try to use wLength=64 after initial wlength=18 attempt. Previous failure is fact caused by something else in dwc2 driver.
USBD Bus Reset : Full Speed
USBD Suspend Skipped
USBD Setup Received 80 06 00 01 00 00 12 00
Get Descriptor Device
Queue EP 80 with 8 bytes ...
USBD Xfer Complete on EP 80 with 8 bytes
Queue EP 80 with 8 bytes ...
USBD Setup Received 80 06 00 01 00 00 12 00
Get Descriptor Device
Queue EP 80 with 8 bytes ...
USBD Xfer Complete on EP 80 with 8 bytes
Queue EP 80 with 8 bytes ...
USBD Xfer Complete on EP 80 with 8 bytes
Queue EP 80 with 2 bytes ...
USBD Xfer Complete on EP 80 with 2 bytes
Queue EP 00 with 0 bytes ...
USBD Xfer Complete on EP 00 with 0 bytes
USBD Bus Reset : Full Speed
USBD Suspend Skipped
USBD Setup Received 80 06 00 01 00 00 40 00
Get Descriptor Device
Queue EP 80 with 8 bytes ...
USBD Xfer Complete on EP 80 with 8 bytes
Queue EP 00 with 0 bytes ...
USBD Xfer Complete on EP 00 with 0 bytes
USBD Bus Reset : Full Speed
USBD Setup Received 00 05 36 00 00 00 00 00
Set Address
USBD Xfer Complete on EP 80 with 0 bytes
USBD Setup Received 80 06 00 01 00 00 12 00
Get Descriptor Device
Queue EP 80 with 8 bytes ...
USBD Xfer Complete on EP 80 with 8 bytes
Queue EP 80 with 8 bytes ...
USBD Xfer Complete on EP 80 with 8 bytes
Queue EP 80 with 2 bytes ...
USBD Xfer Complete on EP 80 with 2 bytes
Queue EP 00 with 0 bytes ...
USBD Xfer Complete on EP 00 with 0 bytes
Signed-off-by: Mengsk <admin@hifiphile.com>
Signed-off-by: Mengsk <admin@hifiphile.com>
…is received Signed-off-by: Mengsk <admin@hifiphile.com>
Describe the PR
FIFO allocation needs additional space for EP disable status, which is 1 location for each EP.MPSiz needs to be set correctly for EP0 in order to reserve Rx FIFO correctly.
Also fix preset with espressif, who needs Ninja single config.