bsp: cvitek: fix cvitek/c906_little build warning #9865
Conversation
Signed-off-by: flyingcys flyingcys@163.com
github-actions
bot
added
BSP
BSP: Cvitek
|
|
||
| #include <encoding.h> | ||
|
|
||
| extern int rt_hw_tick_isr(void); |
There was a problem hiding this comment.
这个可以直接引用头文件嘛？extern声明方式比较偷懒，之前遇到过因为函数参数发生变化，导致每个extern位置都需要改一遍的情况，不利于维护
There was a problem hiding this comment.
- 首先这个不是偷懒不偷懒的问题！！！！
- 我的理解：libcpu比bsp更底层，底层调用上层实现的.h是不是更不合理。
- 如果大家觉得需要调用.h来实现，我没有意见，可以来做
|
可以在 pr 的描述中（譬如第一行）加入如下文字： Fixed #8977这样就可以将这个PR 和 issue 关联起来，pr 合并时会自动 close 关联的 issue。 |
| #include <encoding.h> | ||
|
|
||
| extern int rt_hw_tick_isr(void); | ||
| extern void rt_hw_irq_isr(void); |
There was a problem hiding this comment.
针对 rt_hw_tick_isr & rt_hw_irq_isr， 我认为比较好的做法是参考 rt_hw_soft_irq_isr 的做法，即在 libcpu/risc-v/rv64/trap.c 中为它们定义 weak 函数。
定义 weak 函数的目的实际上就是允许不同的 bsp 可以根据自己的需要在自己的 bsp 中重载其实现，如果没有重载则使用 weak 函数。
而使用 extern 方式则会强制要求 bsp 必须实现重载，不够灵活。
There was a problem hiding this comment.
这2个函数，是强制需要bsp实现的。
因为大部分bsp是不需要 rt_hw_soft_irq_isr 这个实现的，SMP下就需要实现，但是rt_hw_tick_isr和rt_hw_irq_isr是必须要的。
There was a problem hiding this comment.
但是rt_hw_tick_isr和rt_hw_irq_isr是必须要的。
那么就需要在rthw.h头文件中进行声明，纳入到整体进行管理。可以有多个层级，
There was a problem hiding this comment.
首先我搜索发现使用 'rv64'(libcpu/risc-v/rv64) 的 bsp 总共有三个：
bsp/cvitek/c906_little/rtconfig.pybsp/juicevm/rtconfig.pybsp/k210/rtconfig.py
libcpu/risc-v/rv64 中 handle_trap 采用 weak 方式定义，bsp 可以直接用这个函数，也可以自己定义重载它。
如果是直接用这个函数，需要注意这个函数中又会调用三个函数：
rt_hw_soft_irq_isrrt_hw_tick_isrrt_hw_irq_isr
其中在 libcpu/risc-v/rv64 中，rt_hw_soft_irq_isr 有 weak 定义，而其他两个没有，这意味着如果 bsp 没有重载 handle_trap，则自己必须定义 rt_hw_tick_isr 和 rt_hw_irq_isr，这也是 bsp/cvitek/c906_little 的做法，类似的 bsp 还有 bsp/k210，而形如 bsp/juicevm 则是自己实现的 handle_trap。
如此看来，rt_hw_tick_isr 和 rt_hw_irq_isr 并不是所有使用 libcpu/risc-v/rv64 的 bsp 都要必须实现的，bsp 要实现它们的前提是自己没有重载实现 handle_trap。那么我们在 libcpu/risc-v/rv64 中用 extern 修饰 rt_hw_tick_isr 和 rt_hw_irq_isr 也是不对的了。
另外 rt_hw_tick_isr/rt_hw_irq_isr callback 函数看上去也就是 libcpu/risc-v/rv64 在用，不值得将声明放到 ./include/rthw.h 中。
综上所述，我觉得最好的解决方案，还是将 rt_hw_tick_isr/rt_hw_irq_isr 参考 rt_hw_soft_irq_isr 的方式增加 weak 定义。
btw, 我目前觉得 rv64 这个 libcpu 应该标记为 NOT RECOMMEND 了，当然这是后话。
| #define __BOARD_H__ | ||
|
|
||
| #include <rtconfig.h> | ||
| #include "tick.h" |
There was a problem hiding this comment.
我觉得 bsp/cvitek/c906_little/board/tick.h 实际上是不需要的。
tick_isr 和 rt_hw_tick_init 是 定义在 libcpu/risc-v/common64/tick.c 而且是不可重载的。
同时这两个函数已经通过 libcpu/risc-v/common64/tick.h 导出，所以 bsp 应该直接 include libcpu/risc-v/common64/tick.h 这个文件，而不是自己再定义一份 bsp/cvitek/c906_little/board/tick.h 。
There was a problem hiding this comment.
小核现在用的并不是common64中的代码，用的是common+rv64中的代码。这个代码使用与RV64,里面没有对tick做适配，都是每个bsp自己实现
https://github.com/RT-Thread/rt-thread/blob/master/libcpu/risc-v/SConscript
# add common code files
if rtconfig.CPU in common64_arch :
group += SConscript(os.path.join('common64', 'SConscript'))
else :
group += SConscript(os.path.join('common', 'SConscript'))There was a problem hiding this comment.
小核现在用的并不是common64中的代码
嗯，是我看错了。
There was a problem hiding this comment.
请将 bsp/cvitek/c906_little/board/tick.h 中的 tick_isr 删掉，这个函数对于 rv64 根本不存在的。其实 bsp/k210 也有类似的问题。
flyingcys
changed the title
|
本 PR 不再继续，已经新提了 #9953 来继续这个 PR 的工作。 所以先关闭这个 PR。 |
4 participants
Fixed #8977
修复bsp/cvitek/c906_little 编译警告
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
请提供验证的bsp和config (provide the config and bsp)
cvitek/c906_little
]
当前拉取/合并请求的状态 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