Yu-Ping Wu 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 49:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44709/49//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44709/49//COMMIT_MSG@9 PS49, Line 9: 8 phase calibration do MCK 0/180/45 : training, select the best PI settings Does the following make sense?
perform 8 phase calibration to do MCK 0/180/45 training and select the best PI settings.
https://review.coreboot.org/c/coreboot/+/44709/49/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/44709/49/src/soc/mediatek/mt8192/dr... PS49, Line 308: if (r180 > r0) { : r_tmp = r0 + ((r180 - r0) >> 2); : dramc_dbg("R = %u, R180 = %u\n", r_tmp, r180); : break; : } How about we imitate the DQS_8PH_DEGREE_45 case:
if (r180 <= r0) { dqs_level = DQS_LEVEL_UNKNOWN; continue; }
r_tmp = r0 + ((r180 - r0) >> 2); dramc_dbg("R = %u, R180 = %u\n", r_tmp, r180);
and then we can move the 3 'break's outside the if-else block of phase_sm.
https://review.coreboot.org/c/coreboot/+/44709/49/src/soc/mediatek/mt8192/dr... PS49, Line 341: } else {
else is not generally useful after a break or return
Please see the above comment.