Xi Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44709 )
Change subject: soc/mediatek/mt8192: Add dramc 8 phase calibration ......................................................................
Patch Set 47:
(9 comments)
https://review.coreboot.org/c/coreboot/+/44709/45/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/44709/45/src/soc/mediatek/mt8192/dr... PS45, Line 328: if (loop_cnt > early_break_cnt)
Too many leading tabs - consider code refactoring
done
https://review.coreboot.org/c/coreboot/+/44709/46/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/44709/46/src/soc/mediatek/mt8192/dr... PS46, Line 257: static
static const
err_code_min may be assigned to a new value, so only use static modifier.
https://review.coreboot.org/c/coreboot/+/44709/46/src/soc/mediatek/mt8192/dr... PS46, Line 261: 0xff
Since it's used multiple times, please define a macro for this. Maybe […]
Ack
https://review.coreboot.org/c/coreboot/+/44709/46/src/soc/mediatek/mt8192/dr... PS46, Line 263: d
u […]
Ack
https://review.coreboot.org/c/coreboot/+/44709/46/src/soc/mediatek/mt8192/dr... PS46, Line 328: else if (err_code >= err_code_min)
Just "else"
Ack
https://review.coreboot.org/c/coreboot/+/44709/46/src/soc/mediatek/mt8192/dr... PS46, Line 330: if (loop_cnt > early_break_cnt)
Too many leading tabs - consider code refactoring
done
https://review.coreboot.org/c/coreboot/+/44709/46/src/soc/mediatek/mt8192/dr... PS46, Line 331: 1
Does this mean success or failure?
Means exit the 8 phase detect loop or not.
https://review.coreboot.org/c/coreboot/+/44709/46/src/soc/mediatek/mt8192/dr... PS46, Line 341: dramc_dbg("phase_sm err!\n");
Since this is theoretically impossible to happen, how about […]
Ack
https://review.coreboot.org/c/coreboot/+/44709/46/src/soc/mediatek/mt8192/dr... PS46, Line 402:
trailing whitespace
Done