Huayang Duan 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/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 82: if (u1CA_DLL_Mode[chn] == DLL_MASTER) { : clrsetbits_le32(&ch[chn].phy.shu[0].ca_dll[0], : (0x1 << 31) | (0x1 << 30) | (0xf << 20) | (0xf << 16) | : (0xf << 12) | (0x1 << 10) | (0x1 << 9) | (0x1 << 4), : (0x0 << 31) | (0x0 << 30) | (0x6 << 20) | (0x9 << 16) | : (0x8 << 12) | (0x1 << 10) | (0x1 << 9) | (0x1 << 4)); : clrsetbits_le32(&ch[chn].phy.shu[0].ca_dll[1], : (0x1 << 2) | (0x1 << 0), (0x1 << 2) | (0x0 << 0)); : clrsetbits_le32(&ch[chn].phy.shu[0].ca_cmd[6], 0x1 << 7, 0x1 << 7); : } else { : clrsetbits_le32(&ch[chn].phy.shu[0].ca_dll[0], : (0x1 << 31) | (0x1 << 30) | (0xf << 20) | (0xf << 16) | : (0xf << 12) | (0x1 << 10) | (0x1 << 9) | (0x1 << 4), : (0x1 << 31) | (0x1 << 30) | (0x7 << 20) | (0x7 << 16) | : (0x8 << 12) | (0x1 << 10) | (0x1 << 9) | (0x0 << 4)); : clrsetbits_le32(&ch[chn].phy.shu[0].ca_dll[1], : (0x1 << 2) | (0x1 << 0), (0x0 << 2) | (0x1 << 0)); : clrsetbits_le32(&ch[chn].phy.shu[0].ca_cmd[6], 0x1 << 7, 0x0 << 7); : }
it's really not easy to figure out the difference in such long list. […]
I think current style was easy to debug and read.
https://review.coreboot.org/c/coreboot/+/34332/12/src/soc/mediatek/mt8183/dr... File src/soc/mediatek/mt8183/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/34332/12/src/soc/mediatek/mt8183/dr... PS12, Line 177: (0x1 << 29) | (0x0 << 4) | (0x1 << 0));
I think "line count of existing file" is not important; in fact it's more important for "line count […]
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 430: MR13Value &= ~(0x1<<3);
Add space around "<<".
Done
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... PS13, Line 660: pass_byte_cnt |= (1<<dqs);
Add space around "<<".
Done
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... PS13, Line 825: static void dramc_rx_dqs_gating_cal(u8 chn, u8 rank, u8 freq_group, const struct sdram_params *params)
line over 96 characters
Done
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... PS13, Line 1600: dqsdly_byte[byte] = (dqsdly_byte[byte] > 0) ? 0 : -dqsdly_byte[byte];
Extra space before "=".
Done
https://review.coreboot.org/c/coreboot/+/34332/13/src/soc/mediatek/mt8183/dr... PS13, Line 1895: best_coarse_tune2t[rank][dqs] = (dqsg0 >> (dqs * 8)) & 0x7;
Extra space after "=".
Done