【dd2.0】Support Open Firmware API #7788

ErikChanHub · 2023-07-06T02:31:26Z

拉取/合并请求描述：(PR description)

[
AArch64: support hardware atomic
Utilities/libadt: support adt API for DM
Drivers: Support Open Firmware API and model of PIC
]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality：

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

已经仔细查看过代码改动的对比 Already check the difference between PR and old code
代码风格正确，包括缩进空格，命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
没有垃圾代码，代码尽量精简，不包含#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up
所有变更均有原因及合理的，并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
代码是高质量的 Code in this PR is of high quality
已经使用formatting等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification

We add the device name and id set api in DM, now driver could set name like sprintf without know how many devices it is. The misc.h and byteorder.h add some macros to developers that they always use in drivers. Signed-off-by: GuEe-GUI <[email protected]>

Support aarch64 rt_hw_atomic_* api. Add atomic implemente by rt_atomic api: rt_atomic_dec_and_test rt_atomic_fetch_add_unless rt_atomic_add_unless rt_atomic_inc_not_zero Signed-off-by: GuEe-GUI <[email protected]>

Add reference with rt_atomic in resources' put/get management. Add bitmap operator base on rt_ubase_t. Add hashmap for key->data map management. Signed-off-by: GuEe-GUI <[email protected]>

We support OFW API to replace fdt old API, and add IRQ, IO, Platform-Bus, CPUs ... OFW node contorl. To support work with Device Tree or ACPI in drivers that use IRQ, we make a programmable interrupt controller driver's model. Signed-off-by: GuEe-GUI <[email protected]>

mysterywolf · 2023-07-06T02:52:15Z

请使用 https://github.com/mysterywolf/formatting 格式化一下格式

zhkag · 2023-07-06T05:10:33Z

#7789 这个合并了之后 CI就能过了，到时候重新提交一下

mysterywolf · 2023-07-06T05:35:16Z

#7789 这个合并了之后 CI就能过了，到时候重新提交一下

已合并

bsp/nuvoton/nk-n9h30/.config

components/drivers/ofw/libfdt/fdt_addresses.c

geniusgogo · 2023-07-06T07:55:53Z

components/drivers/core/dm.c

+ * Date           Author       Notes
+ * 2023-04-20     ErikChan     the first version
+ */
+


关于dm的缩写，要能找到地方知道是什么意思。

geniusgogo · 2023-07-06T07:58:12Z

components/drivers/core/dm.c

+    len = rt_strlen(name) - 1;
+    name += len;
+
+    while (len --> 0)


这里的代码格式要注意，--是需要跟len变量组合的，所以得靠拢len而不是靠拢>。另外尽量避免写出多个运算符连续出现在一起的代码，一定要这么写加括号吧

是的, 这两天会把这些建议优化一次

geniusgogo · 2023-07-06T08:06:50Z

components/drivers/core/dm.c

+
+    while (len --> 0)
+    {
+        if (*name < '0' || *name > '9')


这个if和里面的while逻辑没看懂，这里判断是非数字就进if，while里又把他当数字来用。
这种代码要写注释呢，感觉你是默认了name是一种特殊的格式：
name指向的字符串最后是数字？既然默认了特殊格式就没必要判断了吧？既然要判断就说明他格式可能出错，既然会出错那现在的检查手段完全不够，while里调过第一个非数字后就一定是数字了？

这个函数就是为了获取 dm 的注册的设备名称，名称只能是 rtc 或者 rtc0，rtc123 这种格式，当我们从字符串末尾向前开始扫描到非数字的字符的时候，再继续向后 +1 后开始读取数字，如果是 rtc 这种格式,自然不会扫描到数字，返回的就是 0 了。

geniusgogo · 2023-07-06T08:10:01Z

components/drivers/include/drivers/byteorder.h

+#define rt_cpu_to_le32(x) __builtin_bswap32(x)
+#define rt_cpu_to_le64(x) __builtin_bswap64(x)
+#else
+#define rt_cpu_to_be16(x) __builtin_bswap16(x)


所有编译器都有这个符号吗？

这个符号目前只有 GCC 和 Clang 支持，但是 DM 平台目前支持的平台芯片基本只使用 GCC 和 Clang。

geniusgogo · 2023-07-06T08:11:48Z

components/drivers/include/drivers/core/rtdm.h

+#include <drivers/misc.h>
+#include <drivers/byteorder.h>
+
+#ifndef RT_CPUS_NR


这个宏定义让menuconfig自动生成吧，当SMP没配置的时候默认生成1？

是的，因为有些驱动对资源的配置是根据CPU数量来的

geniusgogo · 2023-07-06T08:17:28Z

components/drivers/include/drivers/misc.h

+#endif
+#define RT_BITS_PER_LONG_LONG 64
+
+#define RT_DIV_ROUND_UP(n, d)   (((n) + (d) - 1) / (d))


这里的1需要限定符号和长度，例如1u、1ul

可以考虑额外提供RT_DIV_ROUND_UP_ULL

geniusgogo · 2023-07-06T08:58:20Z

components/drivers/ofw/base.c

+    return count;
+}
+
+static rt_int32_t ofw_strcmp(const char *cs, const char *ct)


inline是不是更好？

此处是由于调用ofw_prop_index_of_string的第二个参数需要传入比较函数的指针，所以没使用 inline，而 rt_strcmp，rt_strncmp 和 strcmp，strncmp 这些函数由于 rtthread.h 宏干扰不能直接找到定义（有未定义函数警告），所以独立设置了个函数用于间接跳转。

geniusgogo · 2023-07-06T09:01:56Z

components/drivers/ofw/fdt.c

+
+#include "ofw_internal.h"
+
+struct rt_fdt_earlycon fdt_earlycon rt_section(".bss.noclean.earlycon");


看了一下没被外部引用加static

此处后续如果 earlycon 框架确实没用到会加上 static

geniusgogo · 2023-07-06T09:05:15Z

components/drivers/ofw/fdt.c

+
+rt_inline rt_err_t commit_memregion(const char *name, rt_uint64_t base, rt_uint64_t size, rt_bool_t is_reserved)
+{
+    return rt_fdt_commit_memregion_early(&(rt_region_t)


不建议写这种偷懒的代码

这种写法确实少见，如果存在安全隐患会后续可以改成声明局部变量的方式。

geniusgogo · 2023-07-06T09:18:43Z

components/drivers/ofw/ofw_internal.h

+struct fdt_info
+{
+    /* Always "/", because we save "ofw" information in root node. */
+    char name[sizeof("/")];


用sizeof定义数组只在C99标准并且支持可变长度数组规范下才支持吧？
换编译器会翻车？

C98 已经 out 了吧，但是此处其实只要有 2 字节就够了，不会翻车的

GuEe-GUI added 5 commits July 4, 2023 13:55

AArch64: support hardware atomic

3b7e46d

Support aarch64 rt_hw_atomic_* api. Add atomic implemente by rt_atomic api: rt_atomic_dec_and_test rt_atomic_fetch_add_unless rt_atomic_add_unless rt_atomic_inc_not_zero Signed-off-by: GuEe-GUI <[email protected]>

Utilities/libadt: support adt API for DM

950d71e

Add reference with rt_atomic in resources' put/get management. Add bitmap operator base on rt_ubase_t. Add hashmap for key->data map management. Signed-off-by: GuEe-GUI <[email protected]>

Merge branch 'master' of https://github.com/RT-Thread/rt-thread

81c8919

ErikChanHub closed this Jul 6, 2023

ErikChanHub reopened this Jul 6, 2023

ErikChanHub closed this Jul 6, 2023

【libfdt】replace tab with space

cc87819

ErikChanHub reopened this Jul 6, 2023

【ofw】 format encoding of irq.c to utf-8

fa10a99

mysterywolf closed this Jul 6, 2023

mysterywolf reopened this Jul 6, 2023

mysterywolf requested review from BernardXiong and Guozhanxin July 6, 2023 05:44

Guozhanxin reviewed Jul 6, 2023

View reviewed changes

bsp/nuvoton/nk-n9h30/.config Show resolved Hide resolved

Guozhanxin reviewed Jul 6, 2023

View reviewed changes

components/drivers/ofw/libfdt/fdt_addresses.c Show resolved Hide resolved

Guozhanxin approved these changes Jul 6, 2023

View reviewed changes

Guozhanxin added the +1 Agree +1 label Jul 6, 2023

Guozhanxin merged commit 7886791 into RT-Thread:master Jul 6, 2023

geniusgogo requested changes Jul 6, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

【dd2.0】Support Open Firmware API #7788

【dd2.0】Support Open Firmware API #7788

ErikChanHub commented Jul 6, 2023 •

edited

Loading

mysterywolf commented Jul 6, 2023

zhkag commented Jul 6, 2023

mysterywolf commented Jul 6, 2023

geniusgogo Jul 6, 2023

geniusgogo Jul 6, 2023

ErikChanHub Jul 6, 2023

geniusgogo Jul 6, 2023

GuEe-GUI Jul 7, 2023

geniusgogo Jul 6, 2023

GuEe-GUI Jul 7, 2023 •

edited

Loading

geniusgogo Jul 6, 2023

GuEe-GUI Jul 7, 2023 •

edited

Loading

geniusgogo Jul 6, 2023

GuEe-GUI Jul 7, 2023

geniusgogo Jul 6, 2023

GuEe-GUI Jul 7, 2023

geniusgogo Jul 6, 2023

GuEe-GUI Jul 7, 2023

geniusgogo Jul 6, 2023

GuEe-GUI Jul 7, 2023

geniusgogo Jul 6, 2023

GuEe-GUI Jul 7, 2023


		#include "ofw_internal.h"

		struct rt_fdt_earlycon fdt_earlycon rt_section(".bss.noclean.earlycon");

【dd2.0】Support Open Firmware API #7788

【dd2.0】Support Open Firmware API #7788

Conversation

ErikChanHub commented Jul 6, 2023 • edited Loading

拉取/合并请求描述：(PR description)

当前拉取/合并请求的状态 Intent for your PR

代码质量 Code Quality：

mysterywolf commented Jul 6, 2023

zhkag commented Jul 6, 2023

mysterywolf commented Jul 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuEe-GUI Jul 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuEe-GUI Jul 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikChanHub commented Jul 6, 2023 •

edited

Loading

GuEe-GUI Jul 7, 2023 •

edited

Loading

GuEe-GUI Jul 7, 2023 •

edited

Loading