Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34332 )
Change subject: mediatek/mt8183: Support more DRAM frequency bootup ......................................................................
Patch Set 15:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34332/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34332/1//COMMIT_MSG@9 PS1, Line 9: DDR frequency fix at 3600Mbps
Please elaborate. […]
Done
https://review.coreboot.org/c/coreboot/+/34332/1//COMMIT_MSG@16 PS1, Line 16: Change-Id: Ic1378ca43fb333c445ca77e7dc0844cdf65f2207
Please move this directly above the Signed-off-by line.
Done
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... File src/soc/mediatek/mt8183/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... PS13, Line 147: tmp_0p5t = (((read32(reg_0) >> shift) & DQ_DIV_MASK) & (~(1 << DQ_DIV_SHIFT))); Don't need the outermost parentheses.
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... PS13, Line 551: while (wait_us(100, read32(&ch[chn].nao.testrpt) & status) != status) { What does it mean to compare the return value of wait_us() to status?
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... PS13, Line 870: coarse_start = 22; Consider ``` else coarse_start = 26; ``` for readability.
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... PS13, Line 1053: Extra space
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... PS13, Line 1098: adjust_cneter adjust_center