Attention is currently required from: 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/+/85126?usp=email )
Change subject: soc/mediatek/mt8196: Add PMIF and PMIC driver support ......................................................................
Patch Set 14:
(7 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85126/comment/350d5198_7ba36e7b?usp... : PS12, Line 11: fix low : probability boot failure issue
I suggest reverting the patch to the patchset 8 and submitting an individual path to address the boo […]
OK
File src/soc/mediatek/mt8196/include/soc/pmif.h:
https://review.coreboot.org/c/coreboot/+/85126/comment/263e3fcc_bf258568?usp... : PS12, Line 142:
Just one tab.
no need in this patch, will push another patch
https://review.coreboot.org/c/coreboot/+/85126/comment/0481c005_a4c1aea8?usp... : PS12, Line 142: +
Space around `+`
no need in this patch, will push another patch
https://review.coreboot.org/c/coreboot/+/85126/comment/609307af_5cef0b29?usp... : PS12, Line 142: #define SPMI_SCL_M_PD_ADDR (IOCFG_LM2_BASE+0x80) : #define SPMI_SDA_M_PD_ADDR (IOCFG_LM2_BASE+0x80) : #define SPMI_SCL_P_PD_ADDR (IOCFG_LM2_BASE+0x80) : #define SPMI_SDA_P_PD_ADDR (IOCFG_LM2_BASE+0x80)
Why do we need 4 macros, if their values are all the same?
no need in this patch, will push another patch
https://review.coreboot.org/c/coreboot/+/85126/comment/a39499e7_0cc66a6b?usp... : PS12, Line 147: )
remove
no need in this patch, will push another patch
File src/soc/mediatek/mt8196/pmif_spmi.c:
https://review.coreboot.org/c/coreboot/+/85126/comment/ab4a29f2_9d0fe000?usp... : PS12, Line 164: IOCFG_LM2_BASE + 0x4
Define macros for `IOCFG_LM2_BASE + 0x4` and `IOCFG_LM2_BASE + 0x8`, respecitvely, just like `SPMI_S […]
Done
https://review.coreboot.org/c/coreboot/+/85126/comment/4a95f9e5_1ed5d7fb?usp... : PS12, Line 221: (
remove
Done
-
Jarried Lin (Code Review)