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 40:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34332/35/src/soc/mediatek/mt8183/dr... File src/soc/mediatek/mt8183/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/34332/35/src/soc/mediatek/mt8183/dr... PS35, Line 108: ((cke_on ? 1 : 0) << 6) | ((cke_off ? 1 : 0) << 7));
I'm ok either way. […]
I'm not going to change it then.
https://review.coreboot.org/c/coreboot/+/34332/35/src/soc/mediatek/mt8183/dr... PS35, Line 875: if (freq_group == LP4X_DDR1600) : coarse_start = 18; : else if (freq_group == LP4X_DDR2400) : coarse_start = 25; : else if (freq_group == LP4X_DDR3200) : coarse_start = 25; : else if (freq_group == LP4X_DDR3600) : coarse_start = 21; : else : coarse_start = 26;
Change to switch statement.
Done
https://review.coreboot.org/c/coreboot/+/34332/35/src/soc/mediatek/mt8183/dr... PS35, Line 1545: clock_rate = 800;
I think we should really die on things we can't be sure if that will work or not.
Done
https://review.coreboot.org/c/coreboot/+/34332/38/src/soc/mediatek/mt8183/dr... File src/soc/mediatek/mt8183/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/34332/38/src/soc/mediatek/mt8183/dr... PS38, Line 1258: else : *begin = -64;
this implies not a well-supported case that maybe we should probably die or also warn something.
Done