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 46:
(9 comments)
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... PS43, Line 357: u8 dqsien_pi = 0;
No need to initialize it.
Ack
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... PS43, Line 358: ph_dly = 0, ph_start = 0, ph_end = 0
Also no need to initialize these.
Ack
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... PS43, Line 358: = 0xff
Since ph_dly_final is an output argument of dramc_8_phase_cal_find_best_dly(), there's no need for i […]
If ph_dly_final isn't initialized, the compile will tip may-be-uninitialized if not call "dramc_8_phase_cal_find_best_dly", so keep the default value.
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... PS43, Line 359: = 0
Same. Since this is an output argument, no need to initialize it.
Yes, it's an output argument, but dramc_8_phase_cal_find_best_dly(..., &ph_dly_loop_break) may not set it, so, the output argument may be undecided. Move the initialization code in dramc_8_phase_cal_find_best_dly?
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... PS43, Line 379: dramc_dbg("phase_sm err!\n");
Since this indicates a serious bug in this code block, I'd prefer […]
Ack
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... PS43, Line 383: d
u. Same for the others.
Ack
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... PS43, Line 391: dqs_level
Should we declare this *inside* the for loop of ph_dly or *outside*?
move inside the function dramc_8_phase_cal_find_best_dly.
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... PS43, Line 397: ph_dly_loop_break
Since it's used as a boolean, declare it as 'int' or 'bool'. Then, we may write […]
Ack
https://review.coreboot.org/c/coreboot/+/44709/43/src/soc/mediatek/mt8192/dr... PS43, Line 403: d
u
Ack