[bsp][gd32]:add gd32vw533xx pwm support #11155
Conversation
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
CYFS3
force-pushed
the
gd32_pwm
branch
2 times, most recently
from
4cb3977 to
4994dd9
Compare
|
ci编译有报错，需要检查下 |
There was a problem hiding this comment.
Pull request overview
This PR adds PWM support for the GD32VW553H RISC-V BSP by introducing a PWM driver, wiring it into the BSP build/Kconfig, and adding a CI attach config for build verification.
Changes:
- Added a new GD32VW553H PWM driver implementing RT-Thread PWM ops and hardware init.
- Updated the GD32 RISC-V drivers build script to compile the new PWM driver under appropriate dependencies.
- Added BSP Kconfig options and a CI attach config to enable/compile-test PWM (PWM0).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 22 comments.
| File | Description |
|---|---|
| bsp/gd32/risc-v/libraries/gd32_drivers/drv_pwm.c | New PWM driver implementation for GD32VW553H timers/channels and GPIO AF setup. |
| bsp/gd32/risc-v/libraries/gd32_drivers/SConscript | Adds PWM driver source to the GD32 RISC-V driver build group under PWM/SOC dependencies. |
| bsp/gd32/risc-v/gd32vw553h-eval/board/Kconfig | Introduces BSP_USING_PWM and per-timer enable options (PWM0/1/2/15/16). |
| bsp/gd32/risc-v/gd32vw553h-eval/.ci/attachconfig/ci.attachconfig.yml | Adds CI configuration to compile with PWM enabled. |
| src += ['drv_adc.c'] | ||
|
|
||
| # add pwm drivers. | ||
| if GetDepend(['RT_USING_PWM', 'SOC_GD32VW553H']): |
There was a problem hiding this comment.
[bug/类别]: PWM driver can be built without any BSP_USING_PWMx selected / 可能在未选择任何 PWMx 时仍编译驱动
English: SConscript currently adds drv_pwm.c when RT_USING_PWM is enabled, but the driver’s pin/device tables are only populated under BSP_USING_PWM0/1/2/15/16. If RT_USING_PWM is enabled (e.g., manually) without selecting any BSP_USING_PWMx, the arrays become empty initializers (non-standard in C) and/or the driver registers 0 PWM devices at runtime. Please gate the source on BSP_USING_PWM (or at least one BSP_USING_PWMx) and/or add a #error like other PWM drivers (e.g. bsp/nuclei/.../drv_pwm.c:18-22) to enforce selecting at least one instance.
中文：当前 SConscript 在启用 RT_USING_PWM 时就会编译 drv_pwm.c，但驱动里的 pin/device 表只有在定义了 BSP_USING_PWM0/1/2/15/16 时才会生成内容。如果用户仅开启 RT_USING_PWM（例如手动开启）但未选择任何 BSP_USING_PWMx，这些数组会变成空初始化（非标准 C，可能编译失败），并且运行时也可能注册 0 个 PWM 设备。建议在构建条件中增加 BSP_USING_PWM（或至少一个 BSP_USING_PWMx），并/或像其他 PWM 驱动一样增加 #error（参考 bsp/nuclei/.../drv_pwm.c:18-22）强制至少选择一个实例。
| if GetDepend(['RT_USING_PWM', 'SOC_GD32VW553H']): | |
| if GetDepend(['RT_USING_PWM', 'SOC_GD32VW553H']) and \ | |
| (GetDepend('BSP_USING_PWM0') or GetDepend('BSP_USING_PWM1') or \ | |
| GetDepend('BSP_USING_PWM2') or GetDepend('BSP_USING_PWM15') or \ | |
| GetDepend('BSP_USING_PWM16')): |
| if (j >= sizeof(gd32_timer_periph_list.Port) / sizeof(gd32_timer_periph_list.Port[0])) | ||
| { | ||
| LOG_E("Can not find -1 of gd32_periph_list's member of Port!\n"); | ||
| break; |
There was a problem hiding this comment.
[error_message/类别]: Error log text is inaccurate / 错误日志信息不准确
English: Here the code searches for an empty Port[] slot using 0 as the sentinel, but the log says it “cannot find -1”, and the name gd32_periph_list doesn’t match the actual variable gd32_timer_periph_list. Please update the message to reflect the real condition (no empty slot / list full) and drop the trailing \n (other drivers log without it, e.g. drv_adc.c:109).
中文：这里 Port[] 的空槽位使用 0 作为哨兵值，但日志却写成“找不到 -1”，且 gd32_periph_list 与实际变量名 gd32_timer_periph_list 不一致。建议将日志改为反映真实条件（列表已满/无空槽位），并去掉末尾的 \n（其他驱动通常不加，例如 drv_adc.c:109）。
| } | ||
|
|
||
| return RT_EOK; | ||
| } |
There was a problem hiding this comment.
[bug/类别]: Initialization errors are not propagated / 初始化错误未向上返回
English: pwm_find_timer_periph() logs and breaks when the Port/TimerIndex lists are full, but still returns RT_EOK. This makes it hard for callers to detect initialization failures and can lead to partially-initialized PWM hardware. Please return an error code when those overflow conditions occur (and have the caller check it).
中文：pwm_find_timer_periph() 在 Port/TimerIndex 列表已满时会打印日志并 break，但最后仍返回 RT_EOK。这会导致上层无法感知初始化失败，可能出现 PWM 硬件只初始化了一部分的情况。建议在这些溢出/无空槽位场景下返回错误码，并让调用者检查返回值。
| if (psc == TIMER_CKDIV_DIV2) | ||
| { | ||
| tim_clock = tim_clock / 2; | ||
| } | ||
| else if (psc == TIMER_CKDIV_DIV4) | ||
| { | ||
| tim_clock = tim_clock / 4; | ||
| } |
There was a problem hiding this comment.
[bug/类别]: Prescaler read is incorrectly treated as clock-division / 将预分频寄存器当作分频配置使用
English: psc = timer_prescaler_read(...) reads the TIMER prescaler register (PSC), but the subsequent checks compare it to TIMER_CKDIV_DIV2/DIV4 (clock-division setting). Since clock division is configured separately (and init sets clockdivision = TIMER_CKDIV_DIV1), this logic can wrongly scale tim_clock when psc happens to equal those small enum values. Please read the actual clock-division bits (or remove this scaling if CKDIV is always DIV1).
中文：psc = timer_prescaler_read(...) 读取的是 TIMER 的预分频寄存器 PSC，但后续却将其与 TIMER_CKDIV_DIV2/DIV4（时钟分频配置）做比较。由于 CKDIV 是独立配置项（且初始化里设置为 TIMER_CKDIV_DIV1），该逻辑可能在 psc 恰好等于这些枚举值时错误地对 tim_clock 做 /2 或 /4 处理。建议读取真实的 CKDIV 位（或若始终为 DIV1，则移除这段缩放逻辑）。
| if (psc == TIMER_CKDIV_DIV2) | |
| { | |
| tim_clock = tim_clock / 2; | |
| } | |
| else if (psc == TIMER_CKDIV_DIV4) | |
| { | |
| tim_clock = tim_clock / 4; | |
| } |
| case 16: | ||
| return TIMER16; | ||
| default: | ||
| LOG_E("Unsport timer periph!\n"); |
There was a problem hiding this comment.
[spelling/类别]: Typo in log message / 日志拼写错误
English: "Unsport" is a typo; use "Unsupported".
中文：日志中的 "Unsport" 为拼写错误，建议改为 "Unsupported"。
| /* config the GPIO as analog mode */ | ||
| for (i = 0; i < sizeof(gd32_pwm_pin_obj) / sizeof(gd32_pwm_pin_obj[0]); ++i) | ||
| { | ||
| /*Configure PA0 PA1 PA2(TIMER1 CH0 CH1 CH2) as alternate function*/ |
There was a problem hiding this comment.
[documentation/类别]: GPIO mode comment is incorrect / GPIO 模式注释不准确
English: The comment says “config the GPIO as analog mode”, but the code configures alternate-function mode (GPIO_MODE_AF) for PWM. Please update the comment so it matches the actual configuration (AF output for TIMER PWM).
中文：注释写的是“将 GPIO 配置为模拟模式”，但代码实际配置的是复用功能模式（GPIO_MODE_AF）用于 PWM 输出。建议更新注释，使其与实际配置一致（TIMER PWM 的 AF 输出）。
| /* config the GPIO as analog mode */ | |
| for (i = 0; i < sizeof(gd32_pwm_pin_obj) / sizeof(gd32_pwm_pin_obj[0]); ++i) | |
| { | |
| /*Configure PA0 PA1 PA2(TIMER1 CH0 CH1 CH2) as alternate function*/ | |
| /* configure GPIO pins in alternate-function mode for TIMER PWM output */ | |
| for (i = 0; i < sizeof(gd32_pwm_pin_obj) / sizeof(gd32_pwm_pin_obj[0]); ++i) | |
| { | |
| /* configure pin as alternate function for the corresponding TIMER channel */ |
| }; | ||
|
|
||
| /* | ||
| * 将所有用到的 gpio port 和 timer 不重复地列举出来，以方便后面不重复地初始化 |
There was a problem hiding this comment.
[documentation/类别]: Source comments should be in English / 源码注释需使用英文
English: The comment above pwm_find_timer_periph() is in Chinese. RT-Thread code comments are expected to be in English for consistency and accessibility; please translate this comment to English (you can keep additional Chinese in documentation files if needed).
中文：pwm_find_timer_periph() 上方的注释为中文。RT-Thread 代码注释通常要求使用英文以保持一致性与可维护性；建议将该注释翻译为英文（如需中文说明，可放到文档中）。
| * 将所有用到的 gpio port 和 timer 不重复地列举出来，以方便后面不重复地初始化 | |
| * List all used GPIO ports and timers without duplication to avoid repeated initialization later. |
| if (j >= sizeof(gd32_timer_periph_list.TimerIndex) / sizeof(gd32_timer_periph_list.TimerIndex[0])) | ||
| { | ||
| LOG_E("Can not find -1 of gd32_periph_list's member of TimerIndex!\n"); | ||
| break; |
There was a problem hiding this comment.
[error_message/类别]: Error log should be clearer/consistent / 错误日志需更清晰且保持一致
English: This log message uses inconsistent wording/variable name (“gd32_periph_list” vs gd32_timer_periph_list) and includes a trailing \n (most LOG_* calls omit it). Consider changing to a clearer message like “Cannot find empty slot in gd32_timer_periph_list.TimerIndex (list full)” and keep formatting consistent.
中文：该日志中变量名表述不一致（“gd32_periph_list” vs 实际的 gd32_timer_periph_list），并且末尾带 \n（多数 LOG_* 不需要）。建议改成更清晰的“无法找到 TimerIndex 空槽位（列表已满）”并保持格式一致。
| rt_uint16_t psc; | ||
| rt_uint32_t chxcv; | ||
|
|
||
| tim_clock = rcu_clock_freq_get(CK_AHB); |
There was a problem hiding this comment.
[bug/类别]: SET and GET use different clock sources / SET 与 GET 使用不同的时钟源
English: drv_pwm_set() computes period/pulse using rcu_clock_freq_get(CK_SYS), but drv_pwm_get() uses rcu_clock_freq_get(CK_AHB). This makes GET potentially report values that don’t match what was configured by SET. Please use a consistent timer input clock source for both paths (or compute the real timer clock explicitly) so PWM_CMD_GET reflects the actual configured frequency/duty.
中文：drv_pwm_set() 通过 rcu_clock_freq_get(CK_SYS) 计算 period/pulse，但 drv_pwm_get() 使用 rcu_clock_freq_get(CK_AHB)。这会导致 GET 返回的数值可能与 SET 设置的不一致。建议两条路径使用一致的定时器输入时钟来源（或显式计算真实的 timer 时钟），确保 PWM_CMD_GET 能正确反映当前配置的频率/占空比。
| tim_clock = rcu_clock_freq_get(CK_AHB); | |
| tim_clock = rcu_clock_freq_get(CK_SYS); |
| LOG_E("Unsport gpio port!\n"); | ||
| } | ||
| } | ||
| #define RCU_T(x) RCU_TIMER##x |
There was a problem hiding this comment.
[maintainability/类别]: Unused macro RCU_T / 未使用的宏 RCU_T
English: #define RCU_T(x) RCU_TIMER##x is not used anywhere in this file. Please remove it to avoid dead code (or use it to simplify the switch if that was the intent).
中文：#define RCU_T(x) RCU_TIMER##x 在本文件中未被使用。建议删除以避免死代码（或如果原意是简化 switch，则实际使用该宏）。
| #define RCU_T(x) RCU_TIMER##x |
#11155 (comment) |
2 participants
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up