Power upstream增加 A210 的PINCTRL及电源管理驱动 - #375
Conversation
|
开始测试 log: https://github.com/RVCK-Project/rvck/actions/runs/32934165539 参数解析结果
测试完成 详细结果:
Kunit Test Result[05:32:09] Testing complete. Ran 482 tests: passed: 465, skipped: 17
Kernel Build Result
Check Patch Result
LAVA Check (qemu)
result: Lava check done!
|
|
开始测试 log: https://github.com/RVCK-Project/rvck/actions/runs/32953738779 参数解析结果
测试完成 详细结果:
Kunit Test Result[09:39:13] Testing complete. Ran 482 tests: passed: 465, skipped: 17
Kernel Build Result
Check Patch Result
LAVA Check (qemu)
result: Lava check done!
|
unicornx
left a comment
There was a problem hiding this comment.
1st review
33be89f dt-bindings: pinctrl: zhihe: add A210 pinctrl
-
bindings 中增加 description,介绍一下 a210 的 pin controller
-
缺乏对引脚配置的规格描述,目前只描述了 pin controller 的。具体可以参考 rvck 下的
thead,th1520-pinctrl.yaml或者主线中较新的产品的 pintrl binding 文件,譬如 sophgo 和 spacemit 的。
991a8bb pinctrl: zhihe: add A210 pinctrl driver
-
drivers/pinctrl/Kconfig:default ARCH_ZHIHE这个应该删除,作为一个支持 tristate 的模块来说一般默认为空,如果的确需要作为 m 或者 y 请修改 rvck_defconfig,而且尽量 m,y 会增加 Image 体积。 -
drivers/pinctrl/Makefile: 请按照 CONFIG 的字典顺序排列 -
checkpatch 报告
drivers/pinctrl/pinctrl-a210.c中出现多处超长代码行,看看还能优化一下?实在不行就算了。
425075a dt-bindings: firmware: zhihe: add A210 AON subsystem
-
每个模块的 bindig 文件应该分开单独 patch 提交,所以这个补丁应该分成三个补丁 firmware/mailbox/regulator
-
Documentation/devicetree/bindings/mailbox/zhihe,a210-mailbox.yaml: 提到一个 "Local ICU CPU index" 的概念,建议在 description 中介绍一下 ICU 是什么意思,以及它和 mailbox 的关系。
2c9c502 mailbox: zhihe: add A210 mailbox controller driver
-
drivers/mailbox/Kconfig:default ARCH_ZHIHE这个应该删除,对于这些模块,参考上游的 Kconfig 大部分都不会设置默认值的,如果需要设置可以修改rvck_defconfig。 -
drivers/mailbox/Makefile: 中间用 TAB 对齐。 -
drivers/mailbox/zhihe-mailbox.c:- 文件名问题,这个 mailbox 驱动文件是只对 a210 还是考虑支持以后所有 zhihe 产品?
- include 的字典顺序
- 和文件名问题类似,代码中的宏命名、变量名、函数名的前缀是 zhihe 还是应该用 a210?
- 和文件名问题类似,代码中的驱动名字 "zhihe_mbox" 是否需要修改为 "a210_mbox" 或者 "zhihe_a210_mbox"
cddb99d firmware: zhihe: add A210 AON subsystem driver
-
drivers/firmware/zhihe/Kconfig:default ARCH_ZHIHE这个应该删除,道理同上,如果需要设置可以修改rvck_defconfig。 -
zhihe_proc_debug.h这个文件为何放在include/linux,建议放在drivers/firmware/zhihe下因为只有drivers/firmware/zhihe/zhihe_aon.c用到了。 -
drivers/firmware/zhihe/zhihe_aon.c:- include 的字典顺序
- 类似前面提到的文件名问题,这个 aon 驱动文件是只对 a210 还是考虑支持以后所有 zhihe 产品?
- 类似前面提到的代码中的宏命名、变量名、函数名的前缀问题
- 类似前面提到的驱动名字问题 "zhihe-aon"
-
include/linux/firmware/zhihe/a210-ipc.h:zhihe_aon_misc_set_control和zhihe_aon_misc_get_control没有定义,请删掉。- 为何 include
thead,th1520-aon.h,建议剥离。
9b4c017 regulator: zhihe: add A210 AON regulator driver
-
drivers/regulator/Kconfig:- 请确认一下名字,注释中是否要加上 A210
default ZHIHE_AON的问题,同上
-
drivers/regulator/zhihe-aon-regulator.c:- 类似前面提到的文件名问题, 宏命名、变量名、函数名的前缀问题,以及驱动名字问题 "zhihe-aon-reg"
- 这个驱动的实现为何依赖于 th1520?复用了大量的 th1520 的结构体和宏,建议清理剥离掉。
63ebcdb dt-bindings: power: zhihe: add A210 power domains
-
每个模块的 bindig 文件应该分开单独 patch 提交,所以这个补丁应该分成 2 个补丁, clock/pm
-
“Documentation/devicetree/bindings/clock/zhihe,a210-ccu.yaml” 中的 description 部分,最后一句话 ”the system functions correctly without CCU nodes present“ 是想表达这个 ccu 节点是可选的吗?对于 A210 来说应该是有 CCU ip 的,设备树的用途就是描述硬件,所以如果 A210 中一定有 ccu 模块的话,我理解不存在可选的问题,所以这句话是否可以删去?或者你是想表达 CCU 的功能可以关闭?如果是这样我觉得可以换个说法,而且如何关闭呢,从 binding 上我没有看出来。
9c8ba95 pmdomain: zhihe: add A210 power domain support
drivers/pmdomain/zhihe/Kconfig:default ARCH_ZHIHE的问题,同上。
e2911db (HEAD -> pr-375-review) riscv: dts: zhihe: add A210 power domains
请在 commit message 中解释一下为何针对 dev board 要 /delete-property/ 和 /delete-node/
|
rvck-6.6 升级了,且涉及 rvck_defconfig, 请下次修改时即使升级。 |
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Document the ZhiHe A210 pad controller used for pin muxing and pin configuration. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Add pin control support for the ZhiHe A210 SoC pad controllers. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Document the AON firmware interface of the A210. The Always-On (AON) subsystem runs a dedicated firmware and is reached from the application processor over a mailbox. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Document the mailbox controller of the A210 used for inter-processor communication between the application processor and cores such as the AON subsystem. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Document the virtual regulators of the A210 that are controlled by the AON firmware over the AON mailbox. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Add the mailbox controller driver used for ZhiHe A210 AON IPC messages. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Add the ZhiHe A210 AON firmware driver and its debug support interface. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Add a regulator provider backed by the ZhiHe A210 AON firmware interface. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Document the Clock Control Unit (CCU) embedded in each power domain of the A210, which autonomously sequences clock gating or scaling when its parent power domain transitions between power states. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Document the power domain controller of the A210 and the individual hardware power domains it manages. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Add the ZhiHe A210 power domain provider for on-chip subsystem power control. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Describe ZhiHe A210 pinctrl nodes and board pin mux settings. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Describe the ZhiHe A210 AON firmware node in the SoC device tree. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Describe the ZhiHe A210 power domain controller and domain links. The power domain controller, the AON regulators and the power-domains references of their consumers are described in a210.dtsi. The EVB does not use software-managed power domains, so its board dts removes the power domain controller node, the AON regulator node and the power-domains properties of the affected devices (gmac0, gmac1, watchdog0), leaving the corresponding hardware always powered. The power domain support stays enabled on the dev board. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
|
开始测试 log: https://github.com/RVCK-Project/rvck/actions/runs/33364407819 参数解析结果
测试完成 详细结果:
Kunit Test Result[06:33:19] Testing complete. Ran 482 tests: passed: 465, skipped: 17
Kernel Build Result
Check Patch Result
|
dist inclusion category: feature bugzilla: RVCK-Project#373 -------------------------------- Enable the ZhiHe A210 pinctrl, mailbox, AON firmware, AON regulator and power domain drivers as loadable modules. Signed-off-by: Zhiguo Zhu <zhiguo.zhu@linux.alibaba.com>
|
开始测试 log: https://github.com/RVCK-Project/rvck/actions/runs/33366240004 参数解析结果
测试完成 详细结果:
Kunit Test Result[07:01:08] Testing complete. Ran 482 tests: passed: 465, skipped: 17
Kernel Build Result
Check Patch Result
测试完成 详细结果:
Kunit Test Result[07:01:08] Testing complete. Ran 482 tests: passed: 465, skipped: 17
Kernel Build Result
Check Patch Result
|
|
开始测试 log: https://github.com/RVCK-Project/rvck/actions/runs/33366240004 参数解析结果
测试完成 详细结果:
Kunit Test Result[03:14:22] Testing complete. Ran 482 tests: passed: 465, skipped: 17
Kernel Build Result
Check Patch Result
LAVA Check (qemu)
result: Lava check done!
|
unicornx
left a comment
There was a problem hiding this comment.
2nd review
p.s. 因为今天恰逢月底要锁版本做 release,如果来不及更新,建议等新的 rvck-6.6 发出来后再做 rebase,这个 pr 主要是 bsp 改动,应该 rebase 很方便的。
a322f72 dt-bindings: mailbox: zhihe: add A210 mailbox controller
zhihe,a210-mailbox.yaml 中的 description 部分
Mailbox controller for the Zhihe A210 SoC providing inter-processor
communication channels between the application processor and other
cores such as the AON (Always-On) subsystem.
中 providing 应该是 provides。另外我不确定这里把 AON 称为 cores 是不是容易引起误解,cores 应该指的是处理器核心吧,是否可以改成:other subsystem such as the AON
the mailbox generates for that processor. The zhihe,icu-cpu-id
property tells the driver which ICU, i.e. which local processor,
这里的 The zhihe,icu-cpu-id property 建议改成 The property "zhihe,icu-cpu-id"
932f083 dt-bindings: regulator: zhihe: add A210 AON regulator
zhihe,a210-aon-regulator.yaml 中的 description 部分
Each supported rail is described by a child node. The
zhihe,regulator-id property identifies the corresponding AON firmware
这里的 The zhihe,regulator-id property 建议改成 The property "zhihe,regulator-id property"。
767110d mailbox: zhihe: add A210 mailbox controller driver
- checkpatch 会报 CHECK: Alignment should match open parenthesis, 这些告警应该可以去掉。
288e121 firmware: zhihe: add A210 AON subsystem driver
-
我看你
config ZHIHE_PROC_DEBUG中depends on ZHIHE_A210_AON=y。ZHIHE_A210_AON 是 tristate,如果它被配置为 m(模块),则 ZHIHE_A210_AON=y 为假,当前选项会完全隐藏。我理解 ZHIHE_PROC_DEBUG 只是一个调试开关,和 AON 是 y 或者 m 并不关联,如果你这么写,用户在设置 AON 为模块时无法开启调试功能,可能不符合预期,你确定要做成这样? -
zhihe_proc_debug.c 中 include 的字典顺序
-
zhihe_remove_panic_log_proc这个函数没有使用者,确定要保留以及需要导出? -
checkpatch 会报 CHECK: Alignment should match open parenthesis, 这些告警应该可以去掉。
e2630b0 regulator: zhihe: add A210 AON regulator driver
- checkpatch 会报 CHECK: Alignment should match open parenthesis, 这些告警应该可以去掉。
本 PR 为 ZhiHe A210 增加 pinctrl、AON、mailbox、AON regulator 以及 power domain 相关支持,为后续 A210 平台上的外设电源管理和板级设备树使能提供基础能力。
主要内容包括:
Fixed #373