Skip to content

Support duo256m arm core#6

Merged
unicornx merged 1 commit intoplctlab:mainfrom
jojoandgyc:Support_duo256m_ARM_Core
Feb 27, 2025
Merged

Support duo256m arm core#6
unicornx merged 1 commit intoplctlab:mainfrom
jojoandgyc:Support_duo256m_ARM_Core

Conversation

@jojoandgyc
Copy link
Contributor

1.修改 prebuilt 文件夹目录结构
2.添加 ARM Cortex A53核的 prebuilt 文件
3. mkpkg.sh 中只修改了几处文件路径
4. Readme.md 中添加了使用描述说明

  1. Modify prebuilt directory
  2. Add arm core prebuilt file
  3. Only a few file paths have been modified in mkpkg.sh
  4. Add usage description in Readme file

@jojoandgyc
Copy link
Contributor Author

jojoandgyc commented Feb 19, 2025

Copy link
Collaborator

@unicornx unicornx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1:commit 信息里为啥中文和英文各写了一遍?只要写英文的就好了。

2:commit 信息的最后缺少签名,你再看一下 https://github.com/plctlab/plct-rt-thread/blob/notes/0.notes/20241212-github-tips.md#2-how-to-write-git-commit-message

3:我建议你针对 mkpkg 直接增加一个 DPT_ARCH 的环境变量算了,就是你代码中的 CORE_ARCH,只不过统一用 "DPT_" 的前缀。
处理逻辑和 DPT_BOARD_TYPE 类似,命令中可选,如果不写,默认是 riscv,这样对旧的使用习惯没有大的影响。

4:对于 arm 的模式,我看你会强制设置 DPT_BOARD_TYPE 为 duo256m,这种做法对用户不友好。正确的做法是对于用户在 arm 下指定除了 duo256m 之外的应该直接提示错误并返回。我建议你统一修改 script/board_types.sh 中的逻辑,现在要做的是对于新增加的 arm,我们只支持 duo256m,对 riscv 支持 3个。

5:关于 README 的修改,除了按照上述 review 意见做相应修改外,需要补充的一点是,在 “更新 prebuild 文件” 中说明,对于 arm 的 prebuild 我们其实没有从 sdk 中从源码固件,而是默认使用了一套固定的版本,这个版本从 RTT 仓库中直接拿过来的,每次更新 prebuild 只涉及 riscv。这个在文档中可以标记为 FIXME,因为目前我们的做法的确是一个偷懒的做法,原因是我们并没有长期维护 ARM 的计划 :)

@unicornx
Copy link
Collaborator

提交依据 RT-Thread/rt-thread#9968 (comment) 我的仓库地址 https://github.com/jojoandgyc/rttpkgtool/tree/Support_duo256m_ARM_Core

提交 pr 即可,不用提供你的仓库地址,我们直接看 PR 上的信息就好

@jojoandgyc
Copy link
Contributor Author

Accept

@jojoandgyc jojoandgyc force-pushed the Support_duo256m_ARM_Core branch 4 times, most recently from 04aff3a to 9b1895b Compare February 24, 2025 12:16
@jojoandgyc jojoandgyc requested a review from unicornx February 24, 2025 12:23
Copy link
Collaborator

@unicornx unicornx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

除了代码中的问题外,script/prebuild.sh 这个脚本也要改一下,因为 prebuild 目录下的结构变了。

@jojoandgyc
Copy link
Contributor Author

accept

@jojoandgyc jojoandgyc force-pushed the Support_duo256m_ARM_Core branch from 9b1895b to 06263ae Compare February 26, 2025 06:27
1. Modify prebuilt directory
2. Add arm core prebuilt file
3. Only a few file paths have been modified in mkpkg.sh
4. Add usage description in Readme file

Signed-off-by: YunZhan Huang  <1583267844@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants