Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44708 )
Change subject: soc/mediatek/mt8192: Update initial settings of dramc ......................................................................
Patch Set 43:
(10 comments)
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... PS43, Line 3089: u8 dq_hyst_sel = 0x1, ca_hyst_sel = 0x1; : u8 dq_cap_sel = 0x18, ca_cap_sel = 0x18;
No need to initialize these.
Ack
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... PS43, Line 3537: UI
lowercase
Ack
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... PS43, Line 3569: u32
const u32
Ack
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... PS43, Line 3583: u8 ca_pi = 0, ca_ui = 1;
const
Ack
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... PS43, Line 3593: C
"c" for consistency
Ack
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... PS43, Line 3632: 0
Define a variable for this. […]
Ack
https://review.coreboot.org/c/coreboot/+/44708/39/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/44708/39/src/soc/mediatek/mt8192/dr... PS39, Line 182: memcpy(result, &cali->impedance.result, sizeof(result));
should we make sure impedance.result has exactly same size as result? (if yes please add assert) […]
use a pointer to cali->impedance.result.
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... PS43, Line 177: = {0}
No need to initialize it.
change to a pointer.
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... PS43, Line 185: &ch[0].phy_ao
Please define a local variable for this.
Ack
https://review.coreboot.org/c/coreboot/+/44708/43/src/soc/mediatek/mt8192/dr... PS43, Line 229:
Error from built bot: […]
Done