Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44712 )
Change subject: soc/mediatek/mt8192: Get DDR base information after calibration ......................................................................
Patch Set 41:
(8 comments)
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_basic_api.c:
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 3844: while Can we use wait_us?
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 3849: = One space after "="
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 3856: = 0 No need for initialization.
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... File src/soc/mediatek/mt8192/dramc_pi_main.c:
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 17: static void get_dram_info_after_cal(struct ddr_cali *cali) If cali->density is the only thing modified in this function, why not return max_density?
static u8 get_dram_density(const struct ddr_cali *cali);
or
static u8 get_dram_density(u32 support_ranks);
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 20: G g
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 52: default If this indicates an error, please print an error message.
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 60: d u
https://review.coreboot.org/c/coreboot/+/44712/41/src/soc/mediatek/mt8192/dr... PS41, Line 134: only need do once for Only need to do once to