-
Notifications
You must be signed in to change notification settings - Fork 5.3k
bsp: k230: add hwtimer support #10346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📌 Code Review Assignment🏷️ Tag: bsp_k230Path: Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-05-30 14:29 UTC)
📝 Review Instructions
|
| # CONFIG_BSP_SDIO0_1V8 is not set | ||
| # CONFIG_BSP_USING_SDIO1 is not set | ||
| CONFIG_BSP_SD_MNT_DEVNAME="sd0p1" | ||
| # CONFIG_BSP_USING_TIMERS is not set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.config和rtconfig.h 没有特殊情况可以不用提交。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这是一个新增的配置项,所以更新一下应该更好吧。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这是注释
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这是注释
这次 pr 在 Kconfig 中增加一项菜单 BSP_USING_TIMERS(默认为 n),所以然后运行 scons --menuconfig 后 kconfiglib 会自动更新出来这个。所以这个修改是需要的。
| # CONFIG_BSP_SDIO0_1V8 is not set | ||
| # CONFIG_BSP_USING_SDIO1 is not set | ||
| CONFIG_BSP_SD_MNT_DEVNAME="sd0p1" | ||
| # CONFIG_BSP_USING_TIMERS is not set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这是注释
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for K230 hardware timers by implementing a driver for 6 general-purpose timers along with corresponding test cases and build configurations.
- Added test case for timer driver under bsp/k230/drivers/utest.
- Implemented driver and ISR routines in bsp/k230/drivers/interdrv/hwtimer.
- Updated SConscript and board configuration (Kconfig and .config) to enable timer support.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| bsp/k230/drivers/utest/test_timer.c | Introduces test cases for hardware timers with minor spelling errors in messages. |
| bsp/k230/drivers/utest/SConscript | Adds dependency check for BSP_USING_TIMERS and includes the new test file. |
| bsp/k230/drivers/interdrv/hwtimer/drv_timer.h | Defines register structures and driver interface for timers. |
| bsp/k230/drivers/interdrv/hwtimer/drv_timer.c | Implements driver functions, timer registration and ISR with potential misassignment issues and missing return. |
| bsp/k230/drivers/interdrv/hwtimer/SConscript | Provides build script for the hardware timer component. |
| bsp/k230/board/Kconfig | Updates board configuration with options for enabling timers. |
| bsp/k230/.config | Reflects changes in timer configuration. |
| struct rt_hwtimer_device *rt_timer = rt_container_of(dev, struct rt_hwtimer_device, parent); | ||
| struct k230_timer *kd_timer = rt_container_of(rt_timer, struct k230_timer, device); | ||
|
|
||
| LOG_I("---> [%s] timeout callback fucntion!\n", kd_timer->name); |
Copilot
AI
May 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the typo in the log message: change "fucntion" to "function".
| LOG_I("---> [%s] timeout callback fucntion!\n", kd_timer->name); | |
| LOG_I("---> [%s] timeout callback function!\n", kd_timer->name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
bsp/k230/drivers/utest/test_timer.c
Outdated
| rt_hwtimerval_t timerval; | ||
| rt_hwtimer_mode_t mode; | ||
| rt_size_t tsize; | ||
| rt_uint32_t freq = 25000000; /* Frequency optionas: 12.5M 25M 50M 100M */ |
Copilot
AI
May 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the misspelling "optionas" in the comment to "options".
| rt_uint32_t freq = 25000000; /* Frequency optionas: 12.5M 25M 50M 100M */ | |
| rt_uint32_t freq = 25000000; /* Frequency options: 12.5M 25M 50M 100M */ |
| .device.parent.fops = &k230_timer_fops, | ||
|
|
||
| .name = "hwtimer4", | ||
| .id = 0, |
Copilot
AI
May 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timer4 is assigned id 0, which conflicts with timer0. It is likely that timer4 should have a unique id (e.g., 4).
| .id = 0, | |
| .id = 4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| .device.parent.fops = &k230_timer_fops, | ||
|
|
||
| .name = "hwtimer5", | ||
| .id = 0, |
Copilot
AI
May 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timer5 is assigned id 0, causing a conflict. It is likely that timer5 should have a unique id (e.g., 5).
| .id = 0, | |
| .id = 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
| timer_devices[i].name); | ||
| rt_hw_interrupt_umask(timer_devices[i].irq_num); | ||
| } | ||
| } |
Copilot
AI
May 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function rt_hw_timer_init does not return a value on success. Consider adding 'return RT_EOK;' at the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
K230 support 6 general hardware timers and 3 stc timers. This patch only implement drivers for general hw timers. Signed-off-by: Chen Wang <unicorn_wang@outlook.com>
K230 支持 6 个通用硬件定时器和 3 个 stc 定时器。本补丁仅实现了通用硬件定时器的驱动程序。
拉取/合并请求描述:(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