Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44704 )
Change subject: soc/mediatek/mt8192: Do dramc software impedance calibration ......................................................................
Patch Set 2:
(6 comments)
https://review.coreboot.org/c/coreboot/+/44704/2/src/soc/mediatek/mt8192/dra... File src/soc/mediatek/mt8192/dramc_pi_calibration_api.c:
https://review.coreboot.org/c/coreboot/+/44704/2/src/soc/mediatek/mt8192/dra... PS2, Line 9: Extra indent
https://review.coreboot.org/c/coreboot/+/44704/2/src/soc/mediatek/mt8192/dra... PS2, Line 16: u8 vref_tmp = 0; : vref_tmp = imp_vref_sel[odt][drv_type]; Can be combined into one line.
https://review.coreboot.org/c/coreboot/+/44704/2/src/soc/mediatek/mt8192/dra... PS2, Line 44: switch (drv_type) { This can be simplified by
static const char *const drv_type_str[IMP_DRV_MAX] = { [DRVP] = "DRVP", ... [ODTN] = "ODTN", }; if (drv_type >= IMP_DRV_MAX) die(...); drv_str = drv_type_str[drv_type];
https://review.coreboot.org/c/coreboot/+/44704/2/src/soc/mediatek/mt8192/dra... PS2, Line 89: dramc_sw_impedance_cal Please add this function to dramc_pi_api.h in this patch.
https://review.coreboot.org/c/coreboot/+/44704/2/src/soc/mediatek/mt8192/dra... PS2, Line 91: chn Add 'const' for it.
https://review.coreboot.org/c/coreboot/+/44704/2/src/soc/mediatek/mt8192/dra... PS2, Line 92: cal_res No need to initialize it.