Attention is currently required from: Dehui Sun, Hung-Te Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu.
Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85362?usp=email )
Change subject: soc/mediatek/mt8196: Add booker driver ......................................................................
Patch Set 7:
(7 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85362/comment/ba7eef29_7f8e3d51?usp... : PS1, Line 9: booker == CI-700 : But MTK booker does not fully comply with the CI-700 spec. The booker : only uses CHI protocol, while MTK booker does not support coherence. The : booker also uses other protocols such as AXI, which is to translate CHI : transactions into EMI's AXI transactions. : : Booker = Routing + Coherence + MTE + Debug : Currently, mt8196 booker only uses the functions of Routing&MTE&Debug. : : If downstream SLC needs CMO commands propagation from the DSU, it can : set the disable_cmo_prop = 0 in the por_sbsx_cfg_ctl register. When the : bit is programmed, the CI-700 must be idle. It's better to program it : before transactions go to CI-700.
Yes, I agree with your opinion. […]
Done
https://review.coreboot.org/c/coreboot/+/85362/comment/e593bd0b_629c78e1?usp... : PS1, Line 20: It's better to program it : before transactions go to CI-700.
Okay, I'll make the changes later. […]
Done
Commit Message:
https://review.coreboot.org/c/coreboot/+/85362/comment/c64c1ff5_d7d4d0c6?usp... : PS6, Line 23: TEST=build pass
Please paste the new log messages, and please also document a way, how to test if the driver is oper […]
Done
File src/soc/mediatek/mt8196/booker.c:
https://review.coreboot.org/c/coreboot/+/85362/comment/aafe39ac_d5d81a8c?usp... : PS1, Line 16: NDE
This can be modified in the next release. […]
Done
https://review.coreboot.org/c/coreboot/+/85362/comment/53b16872_e0fc8beb?usp... : PS1, Line 23: "ldr x0, =0x0a450a00\n" : "ldr x1, [x0]\n" : "ldr x2, =0x8\n" : "bic x1, x1, x2\n" : "str x1, [x0]\n" : : "ldr x0, =0x0ac50a00\n" : "ldr x1, [x0]\n" : "ldr x2, =0x8\n" : "bic x1, x1, x2\n" : "str x1, [x0]\n" : : "ldr x0, =0x0b450a00\n" : "ldr x1, [x0]\n" : "ldr x2, =0x8\n" : "bic x1, x1, x2\n" : "str x1, [x0]\n" : : "ldr x0, =0x0bc50a00\n" : "ldr x1, [x0]\n" : "ldr x2, =0x8\n" : "bic x1, x1, x2\n" : "str x1, [x0]\n"
Okay, it's better to use clrbits32().
Done
File src/soc/mediatek/mt8196/booker.c:
https://review.coreboot.org/c/coreboot/+/85362/comment/b8e6f700_27462615?usp... : PS5, Line 56: __func__
As a general comment, for each open comment left by reviewers, please either […]
OK, we will only respond to comments that disagree with in the future. Thanks.
File src/soc/mediatek/mt8196/include/soc/booker.h:
https://review.coreboot.org/c/coreboot/+/85362/comment/aac8aa58_ff9333bf?usp... : PS5, Line 6: extern
Okay, This can be modified in the next release.
Done