Attention is currently required from: Xi Chen. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51203 )
Change subject: src/mediatek/common: Add DRAM common files ......................................................................
Patch Set 2:
(4 comments)
File src/soc/mediatek/common/Kconfig:
https://review.coreboot.org/c/coreboot/+/51203/comment/4711fb42_e9873ca9 PS1, Line 1: config MEDIATEK_DRAM_DVFS : bool : default n : help : This option enables DRAM calibration with multiple frequencies (low, : medium and high frequency groups, with total 7 frequencies) for DVFS : feature. All supported data rates are: 800, 1200, 1600, 1866, 2400, : 3200, 4266. : : config MEDIATEK_DRAM_DVFS_LIMIT_FREQ_CNT : bool : default y : select MEDIATEK_DRAM_DVFS : help : This options limit DRAM frequency calibration count from total 7 to 3, : other frequency will directly use the low frequency shu result. : : config MEMORY_TEST : bool : default y : help : This option enables memory basic compare test to verify the DRAM read : or write is as expected. : I think we have to move this to an if-block so only MTK platforms will have these options, otherwise other non-mtk configs will also have them.
Please check amd common/Kconfig*.
File src/soc/mediatek/common/dram_init.c:
https://review.coreboot.org/c/coreboot/+/51203/comment/f1aaed76_e96c7c88 PS1, Line 10: extern void (*do_putc)(unsigned char c); I think we don't need this anymore?
https://review.coreboot.org/c/coreboot/+/51203/comment/f94c135b_d4c052bf PS1, Line 30: const If mt_set_emi can't take const, what about just removing the const for init_dram_by_params? So we don't need to cast below, and make it more clear.
File src/soc/mediatek/common/memory.c:
https://review.coreboot.org/c/coreboot/+/51203/comment/157afba9_b54c6f93 PS1, Line 28: } else { : printk(BIOS_DEBUG, "[MEM] complex R/W mem test passed\n"); : } : remove the else and move the printk to either add printing rank number, or move to outside of the for-loop.