Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34988 )
Change subject: mediatek/mt8183: Implement the dramc init setting ......................................................................
Patch Set 21:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34988/17/src/soc/mediatek/mt8183/dr... File src/soc/mediatek/mt8183/dramc_init_setting.c:
https://review.coreboot.org/c/coreboot/+/34988/17/src/soc/mediatek/mt8183/dr... PS17, Line 94: if (ca_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); : }
do you have the name of the fields? […]
Done
https://review.coreboot.org/c/coreboot/+/34988/17/src/soc/mediatek/mt8183/dr... PS17, Line 208: if (freq_group > LP4X_DDR1600) { : clrsetbits_le32(&ch[chn].phy.shu[0].b[0].dq[6], : (0x1 << 26) | (0x1 << 27), (0x1 << 26) | (0x0 << 27)); : clrsetbits_le32(&ch[chn].phy.shu[0].b[1].dq[6], : (0x1 << 26) | (0x1 << 27), (0x1 << 26) | (0x0 << 27)); : clrsetbits_le32(&ch[chn].phy.shu[0].ca_cmd[6], : (0x1 << 26) | (0x1 << 27), (0x1 << 26) | (0x0 << 27)); : } else { : clrsetbits_le32(&ch[chn].phy.shu[0].b[0].dq[6], : (0x1 << 26) | (0x1 << 27), (0x0 << 26) | (0x1 << 27)); : clrsetbits_le32(&ch[chn].phy.shu[0].b[1].dq[6], : (0x1 << 26) | (0x1 << 27), (0x0 << 26) | (0x1 << 27)); : clrsetbits_le32(&ch[chn].phy.shu[0].ca_cmd[6], : (0x1 << 26) | (0x1 << 27), (0x0 << 26) | (0x1 << 27)); : }
I'd recommend same style, having the variables inside if-block, and then set by vars. […]
Done