Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35164 )
Change subject: mediatek/mt8183: Use cached calibration result for faster bootup ......................................................................
Patch Set 29:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35164/29/src/soc/mediatek/mt8183/dr... File src/soc/mediatek/mt8183/dramc_init_setting.c:
https://review.coreboot.org/c/coreboot/+/35164/29/src/soc/mediatek/mt8183/dr... PS29, Line 615: if (have_calibration_params(params)) { : dramc_dbg("bypass duty calibration\n"); : : for (u8 chn = 0; chn < CHANNEL_MAX; chn++) { : dramc_duty_set_clk_delay(chn, params->duty_clk_delay[chn]); : dramc_duty_set_dqs_delay(chn, params->duty_dqs_delay[chn]); : } what about just 'return' after done so we don't need the 'else' block to change following lines?
https://review.coreboot.org/c/coreboot/+/35164/29/src/soc/mediatek/mt8183/em... File src/soc/mediatek/mt8183/emi.c:
https://review.coreboot.org/c/coreboot/+/35164/29/src/soc/mediatek/mt8183/em... PS29, Line 365: const struct sdram_params *freq_params since dramc_params is already merged in previous change, will it be easier if we pass dramc_params here?
https://review.coreboot.org/c/coreboot/+/35164/29/src/soc/mediatek/mt8183/in... File src/soc/mediatek/mt8183/include/soc/emi.h:
https://review.coreboot.org/c/coreboot/+/35164/29/src/soc/mediatek/mt8183/in... PS29, Line 88: have_calibration_params do you want to move this into dramc_params? we can check status, flags, ... everything there