feat[driver][gd32]: Improve the RTC driver and support alarm interrupts #10745
Conversation
|
👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流。 🛠 操作步骤 | Steps
完成后，提交将自动更新至 如有问题欢迎联系我们，再次感谢您的贡献！💐 |
📌 Code Review Assignment🏷️ Tag: bsp_gd32105c-evalReviewers: CXSforHPU Changed Files (Click to expand)
🏷️ Tag: bsp_gd32470z-lckfbReviewers: godmial Changed Files (Click to expand)
🏷️ Tag: bsp_gd32e503v-evalReviewers: meng-plus Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-09-26 17:40 CST)
📝 Review Instructions
|
|
@Yaochenger 顺哥帮忙一起看看哈 |
There was a problem hiding this comment.
Pull Request Overview
This PR improves the GD32 RTC driver by implementing RT-Thread's standardized RTC operations interface and adding alarm interrupt support. The changes address RTC initialization issues and provide better compatibility across different GD32 series.
Key changes:
- Refactored RTC driver to use rt_rtc_ops interface instead of legacy device control methods
- Added comprehensive alarm interrupt support with configurable ALARM0/ALARM1 selection
- Fixed RTC clock initialization to use LSE as default clock source with proper error handling
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| bsp/gd32/arm/libraries/gd32_drivers/drv_rtc.c | Complete RTC driver rewrite with ops interface, alarm support, and improved initialization |
| bsp/gd32/arm/*/board/Kconfig | Updated RTC configuration options to support alarm selection instead of clock source selection |
| rt_interrupt_enter(); | ||
|
|
||
| /* Check if alarm interrupt occurred */ | ||
| if (rtc_flag_get(RTC_FLAG_ALARM0) != RESET) |
There was a problem hiding this comment.
Bug: The interrupt handler checks both ALARM0 and ALARM1 flags regardless of which alarm is actually configured. This could cause spurious interrupts for the unused alarm. The handler should only check the flag corresponding to BSP_ALARM_FLAG that was configured at compile time.
English: The interrupt handler should only check the alarm flag that matches the configured BSP_ALARM_FLAG macro.
中文：中断处理程序应该只检查与配置的 BSP_ALARM_FLAG 宏匹配的报警标志。
Copilot uses AI. Check for mistakes.
| LOG_D("RTC Alarm0 triggered"); | ||
| } | ||
|
|
||
| if (rtc_flag_get(RTC_FLAG_ALARM1) != RESET) |
There was a problem hiding this comment.
Bug: The interrupt handler checks both ALARM0 and ALARM1 flags regardless of which alarm is actually configured. This could cause spurious interrupts for the unused alarm. The handler should only check the flag corresponding to BSP_ALARM_FLAG that was configured at compile time.
English: The interrupt handler should only check the alarm flag that matches the configured BSP_ALARM_FLAG macro.
中文：中断处理程序应该只检查与配置的 BSP_ALARM_FLAG 宏匹配的报警标志。
Copilot uses AI. Check for mistakes.
| } | ||
|
|
||
| /* Reset backup domain only if RTC is not running */ | ||
| if ((RTC_STAT & RTC_STAT_RSYNF) == 0) |
There was a problem hiding this comment.
Bug: The condition logic is inverted. RSYNF flag indicates when RTC registers are synchronized and ready to read. The code should reset backup domain only when synchronization has failed or RTC is not running, but this condition resets when synchronization is successful.
English: The synchronization check logic is incorrect - it should reset backup domain when RSYNF is not set, indicating synchronization failure.
中文：同步检查逻辑错误 - 应该在 RSYNF 未设置时重置备份域，表示同步失败。
Copilot uses AI. Check for mistakes.
| alarm->tm_sec = bcd_to_bin(rtc_alarm.alarm_second); | ||
|
|
||
| /* Check if alarm is enabled */ | ||
| alarm->enable = (RTC_CTL & (BSP_RTC_ALARM == RTC_ALARM0 ? RTC_CTL_ALRM0EN : RTC_CTL_ALRM1EN)) ? 1 : 0; |
There was a problem hiding this comment.
[nitpick] Maintainability: This complex ternary expression is difficult to read and maintain. The alarm enable check logic should be simplified using the configured BSP_RTC_ALARM and corresponding enable bit macros.
English: Simplify the alarm enable check using the pre-configured macros for better readability.
中文：使用预配置的宏简化报警使能检查，提高代码可读性。
Example/示例:
uint32_t alarm_enable_bit = (BSP_RTC_ALARM == RTC_ALARM0) ? RTC_CTL_ALRM0EN : RTC_CTL_ALRM1EN;
alarm->enable = (RTC_CTL & alarm_enable_bit) ? 1 : 0;| alarm->enable = (RTC_CTL & (BSP_RTC_ALARM == RTC_ALARM0 ? RTC_CTL_ALRM0EN : RTC_CTL_ALRM1EN)) ? 1 : 0; | |
| uint32_t alarm_enable_bit = (BSP_RTC_ALARM == RTC_ALARM0) ? RTC_CTL_ALRM0EN : RTC_CTL_ALRM1EN; | |
| alarm->enable = (RTC_CTL & alarm_enable_bit) ? 1 : 0; |
Copilot uses AI. Check for mistakes.
拉取/合并请求描述：(PR description)
[
1.修复RTC驱动无法使用问题
2.gd32 rtc支持ops
3.支持alarm中断
4.规范rtc驱动并考虑通用性
5.修复rtc时钟时钟初始化问题（考虑到通用性，建议默认外设时钟源使用通用时钟源，如需更高精度可以引导用户修改）
6.修复alarm中断默认就初始化问题
已在gd32h759imt6上进行测试：
为什么提交这份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