Skip to content

[DM/FDT] Fix garble when booting #10166

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

Merged
merged 1 commit into from
Apr 14, 2025

Conversation

1078249029
Copy link
Contributor

@1078249029 1078249029 commented Apr 2, 2025

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

[

为什么提交这份PR (why to submit this PR)

Fix garble caused by empty implementation of earlycon series function in driver.If driver doesn't offer earlycon_id or rt_f dt_earlycon_id.setup function but open option, the checking of best_earlycon_id will failed.

Fixed #10154

你的解决方案是什么 (what is your solution)

请提供验证的bsp和config (provide the config and bsp)

  • BSP:
  • .config:
  • action:

]

当前拉取/合并请求的状态 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
  • 如果是新增bsp, 已经添加ci检查到.github/workflows/bsp_buildings.yml 详细请参考链接BSP自查

@1078249029
Copy link
Contributor Author

@unicornx 老师我这里修好了,应该是cvitek使用了设备树并分配了earlycon资源但驱动并未实现earlycon相关函数导致的问题
@GuEe-GUI bug我已经修了,不知道其他有设备树的开发板能正常跑么?我这里缺硬件,您能帮测一下么?谢谢啦

@unicornx unicornx self-requested a review April 2, 2025 23:28
@unicornx
Copy link
Contributor

unicornx commented Apr 8, 2025

Fix garble caused by empty implementation of earlycon series function in driver.If driver isn't offer earlycon_id or rt_f dt_earlycon_id.setup function but open option, the checking of best_earlycon_id will failed.

@1078249029
没看明白这段英文,而且这段文字好像有明显的语法错误 ”driver isn't offer ..." -> "driver doesn't offer ..." ?
建议如果写英文可以先用中文描述好后机翻一下。或者还是在 github 里直接写中文吧~~~。

另外你虽然列出了关联的 issue,但是语法不对,导致 github 并没有将 pr 和 issue 关联起来,建议再仔细阅读一下 https://github.com/plctlab/plct-rt-thread/blob/notes/0.notes/20241212-github-tips.md 并修改。

具体的代码修改,还是要请设备树的专家 @GuEe-GUI 看一下。

@GuEe-GUI
Copy link
Contributor

GuEe-GUI commented Apr 8, 2025

看了一下,题主的解决思路主要还是没匹配到早期打印接口就直接不执行打印。

但其实并不需要这么做,因为如果没匹配到接口,rt_kputs 理论上不会打印任何东西:

rt_kputs
-->rt_hw_console_output
----> rt_fdt_earlycon_output
--------> fdt_earlycon.console_putc

所以还是建议查清楚之前打印乱码到底是谁打印出来的比较好,不一定是直接打印的,可能是之前缓存在 fdt early msg 中的数据(那又是谁写进去的缓存?),因为其他平台测试过都是没有这个问题的。

@1078249029
Copy link
Contributor Author

1078249029 commented Apr 10, 2025

看了一下,题主的解决思路主要还是没匹配到早期打印接口就直接不执行打印。

但其实并不需要这么做,因为如果没匹配到接口,rt_kputs 理论上不会打印任何东西:

rt_kputs
-->rt_hw_console_output
----> rt_fdt_earlycon_output
--------> fdt_earlycon.console_putc

所以还是建议查清楚之前打印乱码到底是谁打印出来的比较好,不一定是直接打印的,可能是之前缓存在 fdt early msg 中的数据(那又是谁写进去的缓存?),因为其他平台测试过都是没有这个问题的。

我这里又测了一遍,将fdt_earlycon放入bss,data段也不会乱码,所以我认为内存初始化问题无疑了。我实在看不懂为什么要把fdt_earlycon专门放入.bss.noclean.earlycon,理论上应该默认清空啊,而且为什么不放在数据段呢?是要给系统启动后节省空间么?

@1078249029 1078249029 force-pushed the cvitek_arm64_garble branch 2 times, most recently from 3ac7685 to a2be033 Compare April 10, 2025 10:39
@1078249029
Copy link
Contributor Author

我详细说一下bug是怎么产生的吧。首先cvitek开启了fdt并在设备树中描述了fdt_earlycon的资源,这样kernel会在.bss.noclean.earlycon段开辟一段空间,但是由于段的性质该处并未清0。同时,在驱动中cvitek也没有给出fdt_earlycon相关实现,因此,best_earlycon_id是个空指针。但是在真正执行rt_kputs时,嵌套调用rt_fdt_earlycon_outputfdt_earlycon.console_putc并不为空(已经在bss段指定符号地址了),所以串口还是会输出,并且输出的是未被清0的段的内容。
具体的修改您看怎么合适?是修改fdt_earlycon段的位置,还是不信任驱动开发者从而引入空指针判定?还是什么都不做,毕竟开启设备树但不实现驱动也不适合使得做法?
@GuEe-GUI

@GuEe-GUI
Copy link
Contributor

GuEe-GUI commented Apr 11, 2025

那就采用清空 .bss 就可以了,原则上任何打印都不能错过。

其次如果只是使用普通的 .bss 段,不需要手动进行修饰 rt_section(".bss");,变量不显式初始化都会放进 .bss 中。

感谢题主排查出问题!

Fix garble caused by empty implementation of earlycon series
function in driver.If driver doesn't offer earlycon_id or rt_f
dt_earlycon_id.setup function but open option, the checking
of best_earlycon_id will failed.

Signed-off-by: 1078249029 <[email protected]>
@1078249029 1078249029 force-pushed the cvitek_arm64_garble branch from a2be033 to 24482f4 Compare April 11, 2025 07:01
@1078249029
Copy link
Contributor Author

@unicornx @GuEe-GUI 麻烦两位重新review
我为什么不能邀请reviewer呢?是权限不够么?
图片

@GuEe-GUI
Copy link
Contributor

@unicornx review 完成就可以了。

Copy link
Contributor

@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.

测试通过。

@unicornx
Copy link
Contributor

@unicornx review 完成就可以了。

@GuEe-GUI 只是追问一下,为啥当初要把 fdt_earlycon 放到 ".bss.noclean.earlycon" 这个 section 去呢?现在改成 .bss 有没有副作用?

@GuEe-GUI
Copy link
Contributor

GuEe-GUI commented Apr 14, 2025

因为这段大小可以调整很大,如果放在 bss会导致清理时间很长,但是经过后期实际验证,大部分平台使用默认值(128 字节)已经足够,即便翻倍也不会明显导致初始化时间较长,毕竟使用设备树的平台性能不会差到哪去。

其次就是后期确实有更早的打印需求(一般都是调试用,发布版本不会启用),那之前假设 earlycon 初始化之前不打印的情况就不符合这个需求了。

@unicornx unicornx requested a review from Rbb666 April 14, 2025 05:34
@unicornx
Copy link
Contributor

@Rbb666 请 review,谢谢

@Rbb666 Rbb666 merged commit 9d1fb86 into RT-Thread:master Apr 14, 2025
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] bsp/cvitek AARCH:启动内核时控制台打印乱码
4 participants