Hung-Te Lin 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:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34332/12/src/soc/mediatek/mt8183/dr... File src/soc/mediatek/mt8183/dramc_init_setting.c:
https://review.coreboot.org/c/coreboot/+/34332/12/src/soc/mediatek/mt8183/dr... PS12, Line 309: ((freq == LP4X_DDR1600) || (freq == LP4X_DDR2400)) ? FSP_0 : FSP_1;
Out of curiosity, is this coreboot's convention or just for readability?
I'd say readability.
If you want to return as one line I'm probably fine, but ((a == b) || (c == d)) should be avoided. since the operator precedence is clear we can simply do (a == b || c == d).
And next - the code below wasdoing
if (operate_fsp = FSP1) ... else ...
So I'd say for consistency, if-else may be better in this case.
I'd more prefer to use ?: in very simple, inline params.
https://review.coreboot.org/c/coreboot/+/34332/15/src/soc/mediatek/mt8183/dr... File src/soc/mediatek/mt8183/dramc_init_setting.c:
https://review.coreboot.org/c/coreboot/+/34332/15/src/soc/mediatek/mt8183/dr... PS15, Line 639: TIMEOUT_CNT TIMEOUT_US = 100
https://review.coreboot.org/c/coreboot/+/34332/15/src/soc/mediatek/mt8183/dr... PS15, Line 658: while (!wait_us(1, read32(&ch[chn].nao.spcmdresp) & (0x1 << 4)) && (time_cnt > 0)) : time_cnt--; : : if (time_cnt == 0) { : dramc_dbg("ZQCAL Start fail (time out)\n"); : return 1; : } if (!wait_us(TIMEOUT_US, read32(&ch[chn].nao.spcmdresp) & 0x1 << 4) { dramc_dbg("ZQCAL Start fail (time out)\n"); return 1; }
https://review.coreboot.org/c/coreboot/+/34332/15/src/soc/mediatek/mt8183/dr... PS15, Line 671: : while (!wait_us(1, read32(&ch[chn].nao.spcmdresp) & (0x1 << 6)) && (time_cnt > 0)) : time_cnt--; : : if (time_cnt == 0) { : dramc_dbg("ZQCAL Latch fail (time out)\n"); : return 1; : } if (!wait_us(TIMEOUT_US, read32(&ch[chn].nao.spcmdresp) & 0x1 << 6)) { dramc_dbg("ZACAL latch fail (timeout)\n"); return 1; }