Initial support for GD32C103 and GD32E103#413
Initial support for GD32C103 and GD32E103#413minh7a6 wants to merge 7 commits intocherry-embedded:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds a weak extension hook for runtime DWC2 hardware-parameter overrides and a new GD32 ARM glue layer supplying platform-specific user/hw params, low-level init/deinit, FIFO customization, delay/clock helpers, and an endpoint-count override. Changes
Sequence Diagram(s)sequenceDiagram
participant App as USB stack
participant DWC2_Core as DWC2 core
participant GD32_Glue as GD32 glue
participant HW as USB hardware
App->>DWC2_Core: usb_dc_init(busid)
DWC2_Core->>DWC2_Core: dwc2_get_hwparams(reg_base, hw)
DWC2_Core->>DWC2_Core: dwc2_get_user_params(reg_base, user) (if provided)
DWC2_Core->>GD32_Glue: dwc2_override_hw_params(reg_base, hw) [weak hook]
GD32_Glue-->>DWC2_Core: (optional) modify hw params
DWC2_Core->>GD32_Glue: usb_dc_low_level_init(busid)
GD32_Glue->>HW: enable clocks / IRQ
DWC2_Core->>HW: initialize controller with hw/user params
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
minh7a6
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
port/dwc2/usb_glue_gd_arm.c (1)
42-48: Silence the unused-parameter warning onbusid.Both helpers ignore
busid, which trips-Wunused-parameterin strict build configurations. Add an explicit(void)busid;(matching the pattern used in the weak hook itself).🛠️ Proposed fix
void usb_dc_low_level_init(uint8_t busid) { + (void)busid; NVIC_EnableIRQ(USBFS_IRQn); } void usb_dc_low_level_deinit(uint8_t busid) { + (void)busid; NVIC_DisableIRQ(USBFS_IRQn); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@port/dwc2/usb_glue_gd_arm.c` around lines 42 - 48, Both usb_dc_low_level_init(uint8_t busid) and usb_dc_low_level_deinit(uint8_t busid) ignore the busid parameter and trigger -Wunused-parameter; add an explicit (void)busid; at the start of each function (in usb_dc_low_level_init and usb_dc_low_level_deinit) to silence the warning and match the pattern used in the weak hook implementations, keeping the rest of the functions unchanged.port/dwc2/usb_dc_dwc2.c (1)
326-329: Consider relocating the weak hook for readability.The weak
dwc2_override_hw_paramsdefinition is awkwardly inserted betweendwc2_ep_writeanddwc2_ep_read. Functionally fine, but grouping it near the param-related helpers (or at the top of the file with other forward-declared helpers) keeps related code together.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@port/dwc2/usb_dc_dwc2.c` around lines 326 - 329, The weak hook dwc2_override_hw_params is currently placed between dwc2_ep_write and dwc2_ep_read which breaks logical grouping; move the __WEAK void dwc2_override_hw_params(uint32_t reg_base, struct dwc2_hw_params *hw) stub so it lives with other parameter-related helpers (or with the top section of forward-declared helper hooks) to improve readability and maintainability—ensure you remove the stub from between dwc2_ep_write and dwc2_ep_read and reinsert it near the hw-param helper functions (or header/top helpers) preserving the same signature and (void) casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@port/dwc2/usb_glue_gd_arm.c`:
- Around line 6-10: Add a direct include for the C string header and switch the
system stdint include to angle-bracket form: explicitly include <string.h> (so
memcpy is guaranteed available and not relied on transitively via usbd_core.h)
and change "stdint.h" to use <stdint.h>; update the include list near the top of
usb_glue_gd_arm.c (look for the existing "stdint.h" and uses of memcpy) to make
these changes.
- Around line 66-70: The hard-coded assignment in dwc2_override_hw_params
(hw->num_dev_ep = 4) breaks boards that are built with CONFIG_USBDEV_EP_NUM ==
6; change the override to derive the number of device endpoints from the build
config instead of hard-coding 4 (e.g., set hw->num_dev_ep = CONFIG_USBDEV_EP_NUM
or a min/max guard using CONFIG_USBDEV_EP_NUM), and ensure this hw override
remains active even when CONFIG_USB_DWC2_CUSTOM_PARAM is defined (i.e., only
gate dwc2_get_user_params on CONFIG_USB_DWC2_CUSTOM_PARAM and keep
dwc2_override_hw_params unconditional) so usb_dc_init and usbd_ep_open will see
the correct endpoint count.
- Around line 12-36: Make the global param_pa11_pa12 static and either provide a
second params set for the 6‑EP layout or restrict this file to the 4‑EP variant:
mark the constant param_pa11_pa12 as static, then address device_tx_fifo_size
and total_fifo_size so they match the supported CONFIG_USBDEV_EP_NUM (for 6 EPs
allocate non‑zero entries for indices 0..5 and ensure total_fifo_size covers all
FIFOs), or update the `#error` block to declare only the pa11/pa12 (4‑EP) variant
is supported; also ensure usb_dc_init() will see the correct params selection
based on CONFIG_USBDEV_EP_NUM.
- Around line 72-78: The busy-wait in usbd_dwc2_delay_ms assumes one cycle per
iteration and is inaccurate and fragile; change the prototype to accept uint32_t
ms, and replace the loop with one of two options: (A) use a hardware timer
(preferably DWT cycle counter or SysTick) to compute exact delay based on
SystemCoreClock and busy-wait on the cycle count for precise timing, or (B) if
hardware timers are unavailable, calibrate the loop by measuring
cycles/iteration on the target and divide the computed count (e.g., count =
SystemCoreClock / 1000 / N * ms) so the loop matches real cycles; update all
callers (e.g., dwc2_set_mode and usbd_set_remote_wakeup) to pass uint32_t and
verify delays remain ~10 ms.
---
Nitpick comments:
In `@port/dwc2/usb_dc_dwc2.c`:
- Around line 326-329: The weak hook dwc2_override_hw_params is currently placed
between dwc2_ep_write and dwc2_ep_read which breaks logical grouping; move the
__WEAK void dwc2_override_hw_params(uint32_t reg_base, struct dwc2_hw_params
*hw) stub so it lives with other parameter-related helpers (or with the top
section of forward-declared helper hooks) to improve readability and
maintainability—ensure you remove the stub from between dwc2_ep_write and
dwc2_ep_read and reinsert it near the hw-param helper functions (or header/top
helpers) preserving the same signature and (void) casts.
In `@port/dwc2/usb_glue_gd_arm.c`:
- Around line 42-48: Both usb_dc_low_level_init(uint8_t busid) and
usb_dc_low_level_deinit(uint8_t busid) ignore the busid parameter and trigger
-Wunused-parameter; add an explicit (void)busid; at the start of each function
(in usb_dc_low_level_init and usb_dc_low_level_deinit) to silence the warning
and match the pattern used in the weak hook implementations, keeping the rest of
the functions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f79a897-cf26-4988-9ab4-c5fdd301ba6e
📒 Files selected for processing (2)
port/dwc2/usb_dc_dwc2.cport/dwc2/usb_glue_gd_arm.c
| #include "usbd_core.h" | ||
| #include "stdint.h" | ||
| #include "usb_dwc2_reg.h" | ||
| #include "usb_dwc2_param.h" | ||
| #include "board_config.h" |
There was a problem hiding this comment.
Missing <string.h> and prefer angle-bracket form for <stdint.h>.
memcpy is called at line 53 but <string.h> is not included anywhere in this file (it may transitively be available via usbd_core.h, but relying on that is fragile). Also, stdint.h is a system header and should use angle brackets, matching the rest of the project.
🛠️ Proposed fix
`#include` "usbd_core.h"
-#include "stdint.h"
+#include <stdint.h>
+#include <string.h>
`#include` "usb_dwc2_reg.h"
`#include` "usb_dwc2_param.h"
`#include` "board_config.h"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include "usbd_core.h" | |
| #include "stdint.h" | |
| #include "usb_dwc2_reg.h" | |
| #include "usb_dwc2_param.h" | |
| #include "board_config.h" | |
| `#include` "usbd_core.h" | |
| `#include` <stdint.h> | |
| `#include` <string.h> | |
| `#include` "usb_dwc2_reg.h" | |
| `#include` "usb_dwc2_param.h" | |
| `#include` "board_config.h" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@port/dwc2/usb_glue_gd_arm.c` around lines 6 - 10, Add a direct include for
the C string header and switch the system stdint include to angle-bracket form:
explicitly include <string.h> (so memcpy is guaranteed available and not relied
on transitively via usbd_core.h) and change "stdint.h" to use <stdint.h>; update
the include list near the top of usb_glue_gd_arm.c (look for the existing
"stdint.h" and uses of memcpy) to make these changes.
| void usbd_dwc2_delay_ms(uint8_t ms) | ||
| { | ||
| uint32_t count = SystemCoreClock / 1000 * ms; | ||
| while (count--) { | ||
| __asm volatile("nop"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all callers of usbd_dwc2_delay_ms and the duration values they pass,
# to gauge timing sensitivity.
rg -nP '\busbd_dwc2_delay_ms\s*\(' -C1Repository: cherry-embedded/CherryUSB
Length of output: 1818
🏁 Script executed:
#!/bin/bash
# Check implementations across multiple glue files to see if pattern is consistent
echo "=== GD ARM implementation ==="
sed -n '72,78p' port/dwc2/usb_glue_gd_arm.c
echo -e "\n=== GD implementation ==="
sed -n '44,50p' port/dwc2/usb_glue_gd.c
echo -e "\n=== ST implementation ==="
sed -n '717,723p' port/dwc2/usb_glue_st.c
echo -e "\n=== HC implementation ==="
sed -n '297,303p' port/dwc2/usb_glue_hc.cRepository: cherry-embedded/CherryUSB
Length of output: 776
🏁 Script executed:
#!/bin/bash
# Search for broader context of the delay calls and related functions
echo "=== Context around delay calls in usb_dc_dwc2.c ==="
sed -n '125,145p' port/dwc2/usb_dc_dwc2.c
echo -e "\n=== Context around second delay call ==="
sed -n '660,670p' port/dwc2/usb_dc_dwc2.c
echo -e "\n=== Search for dwc2_set_mode and usbd_set_remote_wakeup mentioned in review ==="
rg -n 'dwc2_set_mode|usbd_set_remote_wakeup' port/dwc2/ -A2 -B2 | head -50Repository: cherry-embedded/CherryUSB
Length of output: 2448
🏁 Script executed:
#!/bin/bash
# Check if there's SystemCoreClock definition or any other delay implementations
echo "=== Looking for other delay timing approaches in the codebase ==="
rg -n 'SysTick|DWT|SystemTick' port/dwc2/ | head -20
echo ""
echo "=== Checking if there are calibration constants or timing configs ==="
rg -n 'calibrat|timing|cycle' port/dwc2/ -i | head -20Repository: cherry-embedded/CherryUSB
Length of output: 345
Loop-based delay is subject to significant cycle overhead.
The implementation assumes each loop iteration takes exactly one cycle, but the actual loop body (count--, compare, branch, plus volatile nop) typically consumes 3+ cycles on Cortex-M. This results in an actual delay roughly 3× longer than the calculation intends. For the current 10 ms calls (in dwc2_set_mode and usbd_set_remote_wakeup), this stretches the delay to ~30 ms or more, which is acceptable for USB mode switching but still inaccurate and fragile.
Recommend either:
- Use a SysTick- or DWT-cycle-counter-based delay for accurate timing.
- Calibrate the loop and divide the count, e.g.
count = SystemCoreClock / 1000 / 4 * msafter measurement on the target.
Additionally, ms parameter is uint8_t, limiting the API to 255 ms. Consider widening to uint32_t for consistency with typical delay signatures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@port/dwc2/usb_glue_gd_arm.c` around lines 72 - 78, The busy-wait in
usbd_dwc2_delay_ms assumes one cycle per iteration and is inaccurate and
fragile; change the prototype to accept uint32_t ms, and replace the loop with
one of two options: (A) use a hardware timer (preferably DWT cycle counter or
SysTick) to compute exact delay based on SystemCoreClock and busy-wait on the
cycle count for precise timing, or (B) if hardware timers are unavailable,
calibrate the loop by measuring cycles/iteration on the target and divide the
computed count (e.g., count = SystemCoreClock / 1000 / N * ms) so the loop
matches real cycles; update all callers (e.g., dwc2_set_mode and
usbd_set_remote_wakeup) to pass uint32_t and verify delays remain ~10 ms.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
port/dwc2/usb_glue_gd_arm.c (1)
42-48: Unusedbusidparameter.
busidis unused in bothusb_dc_low_level_initandusb_dc_low_level_deinit. Add(void)busid;to silence-Wunused-parameter, matching other glue layers.♻️ Proposed fix
void usb_dc_low_level_init(uint8_t busid) { + (void)busid; NVIC_EnableIRQ(USBFS_IRQn); } void usb_dc_low_level_deinit(uint8_t busid) { + (void)busid; NVIC_DisableIRQ(USBFS_IRQn); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@port/dwc2/usb_glue_gd_arm.c` around lines 42 - 48, The functions usb_dc_low_level_init and usb_dc_low_level_deinit currently ignore their busid parameter and trigger -Wunused-parameter; add a deliberate unused-parameter suppression by inserting (void)busid; at the start of each function (inside usb_dc_low_level_init and usb_dc_low_level_deinit) to match other glue layers and silence the warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@port/dwc2/usb_glue_gd_arm.c`:
- Around line 67-73: In dwc2_override_hw_params update the hardcoded num_dev_ep
and magic address: set hw->num_dev_ep to CONFIG_USBDEV_EP_NUM - 1 (since
num_dev_ep is the highest zero-based EP index used by the core loops) instead of
4, and replace the literal 0x50000000UL with the platform constant (e.g.,
USBFS_BASE from your GD32 CMSIS headers included via board_config.h); if
USBFS_BASE may be missing on some variants, add a local conditional fallback
define for that platform constant and use it in the reg_base comparison.
---
Nitpick comments:
In `@port/dwc2/usb_glue_gd_arm.c`:
- Around line 42-48: The functions usb_dc_low_level_init and
usb_dc_low_level_deinit currently ignore their busid parameter and trigger
-Wunused-parameter; add a deliberate unused-parameter suppression by inserting
(void)busid; at the start of each function (inside usb_dc_low_level_init and
usb_dc_low_level_deinit) to match other glue layers and silence the warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7cb8dfff-0cde-453f-8f7a-3c44ff25d299
📒 Files selected for processing (1)
port/dwc2/usb_glue_gd_arm.c
| void dwc2_override_hw_params(uint32_t reg_base, struct dwc2_hw_params *hw) { | ||
| /* HWCFG2 reads 0, this is unknown why */ | ||
| if(reg_base == 0x50000000UL) { | ||
| hw->num_dev_ep = 4; | ||
| } | ||
| /* TODO: For other GD32, potentially will need to update like this as well*/ | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=c -C2 '\bnum_dev_ep\b' port/dwc2/usb_dc_dwc2.c
rg -nP --type=c -C1 '\bUSBD_BASE\b|\bUSBFS_BASE\b' port/dwc2/Repository: cherry-embedded/CherryUSB
Length of output: 2750
🏁 Script executed:
#!/bin/bash
# Search for CONFIG_USBDEV_EP_NUM definition and usage
rg -n 'CONFIG_USBDEV_EP_NUM' --type=c --type=h
# Search for all definitions of USBD_BASE and USBFS_BASE
rg -n 'define\s+(USBD_BASE|USBFS_BASE)' --type=c --type=h
# Search for 0x50000000 to see where the magic literal appears
rg -n '0x50000000' --type=c --type=h
# Check GD32-related header includes
rg -n 'include.*gd32' --type=c --type=h -i | head -20Repository: cherry-embedded/CherryUSB
Length of output: 5664
🏁 Script executed:
#!/bin/bash
# Search for GD32-related includes or USB definitions
fd -type f -name "*.h" | xargs grep -l "GD32\|gd32" 2>/dev/null | head -20
# Search for USB base defines in the codebase
rg -n 'define.*BASE.*0x50000000' --type=h --type=c
# Check what's in the GD files - look for the glue file itself and related headers
cat -n port/dwc2/usb_glue_gd_arm.c | head -75
# Look for includes in usb_glue_gd_arm.c
rg -n '#include' port/dwc2/usb_glue_gd_arm.cRepository: cherry-embedded/CherryUSB
Length of output: 3084
🏁 Script executed:
#!/bin/bash
# Find board_config.h references
rg -l 'board_config.h' --type=c --type=h | head -10
# Search for USBFS or USB related defines in GD32 context
rg -n 'USBFS\|USBD' --type=h | grep -i gd32 | head -20
# Look for any GD32 CMakeLists or config files
fd -name 'CMakeLists.txt' -o -name '*.cmake' | xargs grep -l 'gd32\|GD32' 2>/dev/null
# Check examples directory structure for GD32
find . -type d -name '*gd*' 2>/dev/null | head -10
# Search for where USBFS_IRQn is defined (used in usb_glue_gd_arm.c)
rg -n 'USBFS_IRQn' --type=c --type=hRepository: cherry-embedded/CherryUSB
Length of output: 493
🏁 Script executed:
#!/bin/bash
# Search for any GD32-related header files or documentation
find . -type f \( -name "*.h" -o -name "*.md" \) -exec grep -l "GD32\|0x50000000" {} \; | head -20
# Look at includes in the glue file more carefully
grep -E '#include|USBFS' port/dwc2/usb_glue_gd_arm.c
# Search for any constants that might map to 0x50000000
rg '0x50000000' -B2 -A2 --type=c --type=h
# Check the dwc2_override_hw_params function calls
rg -A5 -B5 'dwc2_override_hw_params' --type=c | head -40Repository: cherry-embedded/CherryUSB
Length of output: 3329
Fix the off-by-one error in num_dev_ep and replace the magic literal with a platform constant.
Two critical issues require correction:
-
Off-by-one on
num_dev_ep. The core loops atusb_dc_dwc2.c:530, 602, 1156iterate asi < (num_dev_ep + 1), confirmingnum_dev_epis the highest zero-based EP index, not the count. WithCONFIG_USBDEV_EP_NUM = 4(EPs 0–3),num_dev_epmust be3, not4. Setting it to4causes 5 iterations, accessingdevice_tx_fifo_size[4](which is0), violating FIFO semantics. Usehw->num_dev_ep = CONFIG_USBDEV_EP_NUM - 1;. -
Magic literal
0x50000000UL. Replace with a GD32 platform constant (e.g., from the GD32 CMSIS header included viaboard_config.h). A magic literal drifts silently if the platform header changes and blocks future variant support (see the TODO comment).
🛠️ Proposed fix
void dwc2_override_hw_params(uint32_t reg_base, struct dwc2_hw_params *hw) {
- /* HWCFG2 reads 0, this is unknown why */
- if(reg_base == 0x50000000UL) {
- hw->num_dev_ep = 4;
- }
+ /* HWCFG2 reads 0 on GD32C103/E103; supply known values manually. */
+ if (reg_base == USBFS_BASE) {
+ hw->num_dev_ep = CONFIG_USBDEV_EP_NUM - 1; /* highest EP index */
+ }
/* TODO: For other GD32, potentially will need to update like this as well*/
}Note: Verify that USBFS_BASE is defined in your GD32 CMSIS headers (included via board_config.h). If unavailable, define a local constant for this variant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@port/dwc2/usb_glue_gd_arm.c` around lines 67 - 73, In dwc2_override_hw_params
update the hardcoded num_dev_ep and magic address: set hw->num_dev_ep to
CONFIG_USBDEV_EP_NUM - 1 (since num_dev_ep is the highest zero-based EP index
used by the core loops) instead of 4, and replace the literal 0x50000000UL with
the platform constant (e.g., USBFS_BASE from your GD32 CMSIS headers included
via board_config.h); if USBFS_BASE may be missing on some variants, add a local
conditional fallback define for that platform constant and use it in the
reg_base comparison.
|
Thanks, you do the better solution for gd patches, gd official removed hwcfg registers so we cannot read, this patches will merge in next version. |
|
Thanks! It is odd why they did that. It makes probing much more difficult than necessary. I only override the number of EPs for now. But from the TinyUSB port and my own testing, all hwcfg registers and snpsid are 0. Hopefully some representatives from GD can describe more what the values are for these |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@port/dwc2/usb_glue_gd_arm.c`:
- Around line 42-48: usb_dc_low_level_init currently only enables the IRQ
(NVIC_EnableIRQ(USBFS_IRQn)) but omits USB clock and GPIO setup; either
implement the missing hardware init (enable USBFS peripheral clock via
rcu_periph_clock_enable(RCU_USBFS), configure USB 48MHz prescaler with
rcu_usb_clock_config(...), and configure USB D+ / D- pins (e.g., PA11/PA12 or
PB14/PB15) to the USB AF) within usb_dc_low_level_init (and undo/disable in
usb_dc_low_level_deinit), or add a clear comment at the top of this file stating
that board_config.h is responsible for RCU and GPIO setup so callers know the
contract; reference usb_dc_low_level_init, usb_dc_low_level_deinit,
NVIC_EnableIRQ, USBFS_IRQn, rcu_periph_clock_enable, rcu_usb_clock_config, and
PA11/PA12 (or PB14/PB15) when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 517ae6c7-e5a1-435d-99d1-3cc2f61a190f
📒 Files selected for processing (1)
port/dwc2/usb_glue_gd_arm.c
| void usb_dc_low_level_init(uint8_t busid) { | ||
| NVIC_EnableIRQ(USBFS_IRQn); | ||
| } | ||
|
|
||
| void usb_dc_low_level_deinit(uint8_t busid) { | ||
| NVIC_DisableIRQ(USBFS_IRQn); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether USBFS clock / GPIO setup happens elsewhere in the tree.
rg -nP -C2 'RCU_USBFS|rcu_usb_clock_config|rcu_periph_clock_enable\s*\(\s*RCU_USBFS'
rg -nP -C2 '\b(GPIO|gpio)_(init|mode_set|af_set).*PA1[12]\b'
fd -t f -e c -e h | xargs rg -lP 'usb_dc_low_level_init' 2>/dev/nullRepository: cherry-embedded/CherryUSB
Length of output: 996
🏁 Script executed:
cat -n port/dwc2/usb_glue_gd.cRepository: cherry-embedded/CherryUSB
Length of output: 1489
🏁 Script executed:
cat -n port/dwc2/usb_glue_gd_arm.cRepository: cherry-embedded/CherryUSB
Length of output: 2892
🏁 Script executed:
rg -n 'RCU_USBFS|rcu_usb_clock_config' port/dwc2/ -A 3 -B 3Repository: cherry-embedded/CherryUSB
Length of output: 51
🏁 Script executed:
rg -n 'PA1[12]|PB1[45]' port/dwc2/ -A 2 -B 2Repository: cherry-embedded/CherryUSB
Length of output: 530
🏁 Script executed:
wc -l port/dwc2/usb_glue_gd.cRepository: cherry-embedded/CherryUSB
Length of output: 96
🏁 Script executed:
cat -n port/dwc2/usb_glue_gd.c | tail -100Repository: cherry-embedded/CherryUSB
Length of output: 1489
🏁 Script executed:
find . -name '*gd*' -path '*/port/*' -type f | grep -E '\.(c|h)$'Repository: cherry-embedded/CherryUSB
Length of output: 125
🏁 Script executed:
rg -n 'RCU_USBFS|rcu_periph_clock_enable.*USBFS|rcu_usb_clock_config' --type c --type hRepository: cherry-embedded/CherryUSB
Length of output: 51
🏁 Script executed:
rg -n 'board_config|board.h' port/dwc2/usb_glue_gd_arm.cRepository: cherry-embedded/CherryUSB
Length of output: 98
🏁 Script executed:
find . -name 'board_config.h' -o -name 'board.h' | head -5Repository: cherry-embedded/CherryUSB
Length of output: 51
🏁 Script executed:
rg -n 'rcu_periph_clock_enable|RCU_' --type c --type h | head -20Repository: cherry-embedded/CherryUSB
Length of output: 51
🏁 Script executed:
rg -n 'gpio.*init|GPIO.*AFIO' --type c --type h -i | grep -i 'gd32\|pa1[12]\|pb1[45]' | head -10Repository: cherry-embedded/CherryUSB
Length of output: 51
🏁 Script executed:
rg -n 'usb_dc_low_level_init|usb_dc_init' -A 10 port/ --type c | head -80Repository: cherry-embedded/CherryUSB
Length of output: 4321
🏁 Script executed:
rg -n 'rcu_periph_clock_enable|clock.*enable' port/ --type c -B2 -A2 | grep -E '(usb|USB)' | head -30Repository: cherry-embedded/CherryUSB
Length of output: 1805
🏁 Script executed:
ls -la port/dwc2/ | grep glueRepository: cherry-embedded/CherryUSB
Length of output: 684
🏁 Script executed:
cat -n port/dwc2/usb_glue_st.c | head -100Repository: cherry-embedded/CherryUSB
Length of output: 3461
🏁 Script executed:
cat -n port/dwc2/usb_glue_esp.c | head -100Repository: cherry-embedded/CherryUSB
Length of output: 4179
🏁 Script executed:
rg -n 'void usb_dc_low_level_init' port/dwc2/ -A 15 --type cRepository: cherry-embedded/CherryUSB
Length of output: 7618
usb_dc_low_level_init() only enables NVIC; clock and GPIO setup are missing or undocumented.
The function only calls NVIC_EnableIRQ(USBFS_IRQn). Typical GD32 USBFS initialization also requires:
- Enabling the USBFS bus clock (
rcu_periph_clock_enable(RCU_USBFS)) - Configuring USB clock prescaler for 48 MHz (
rcu_usb_clock_config) - Setting up PA11/PA12 (or PB14/PB15) GPIO alternate-function pins
Other DWC2 port implementations (ST, Nation, Infineon, HC, Kendryte) perform such setup in their usb_dc_low_level_init() functions. If your board code handles this via the included board_config.h, add a comment documenting the contract; otherwise this file is incomplete.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@port/dwc2/usb_glue_gd_arm.c` around lines 42 - 48, usb_dc_low_level_init
currently only enables the IRQ (NVIC_EnableIRQ(USBFS_IRQn)) but omits USB clock
and GPIO setup; either implement the missing hardware init (enable USBFS
peripheral clock via rcu_periph_clock_enable(RCU_USBFS), configure USB 48MHz
prescaler with rcu_usb_clock_config(...), and configure USB D+ / D- pins (e.g.,
PA11/PA12 or PB14/PB15) to the USB AF) within usb_dc_low_level_init (and
undo/disable in usb_dc_low_level_deinit), or add a clear comment at the top of
this file stating that board_config.h is responsible for RCU and GPIO setup so
callers know the contract; reference usb_dc_low_level_init,
usb_dc_low_level_deinit, NVIC_EnableIRQ, USBFS_IRQn, rcu_periph_clock_enable,
rcu_usb_clock_config, and PA11/PA12 (or PB14/PB15) when making the change.
Signed-off-by: sakumisu <1203593632@qq.com>
Signed-off-by: sakumisu <1203593632@qq.com>
sakumisu
force-pushed
the
master
branch
2 times, most recently
from
4eb3bc1 to
c9625ff
Compare
2 participants
Hi all,
I'm creating this PR to add initial support for GD32C103 and GD32E103. I've tested preliminarily for GD32C103 in device mode. Since GD32E103 is pretty much the same variant without CAN Bus, I expected this to work the same. Similarly to GD32VF103 port from TinyUSB, there are unknown reasons why hwcfg2 reads all 0. Hence I'm adding new functions to override this hw params.
Thanks
Summary by CodeRabbit