Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44731 )
Change subject: soc/mediatek/mt8192: Limit DRAM calibration frequency count to reduce bootup time ......................................................................
Patch Set 2:
(7 comments)
https://review.coreboot.org/c/coreboot/+/44731/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44731/2//COMMIT_MSG@8 PS2, Line 8: Please elaborate and give numbers for the boot time reduction.
Also, it’d be great if you pasted the new log messages.
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/Kco... File src/soc/mediatek/mt8192/Kconfig:
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/Kco... PS2, Line 43: This options limit
This option limits
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/Kco... PS2, Line 44: shu Please elaborate in the text, what shu is.
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/Kco... PS2, Line 43: This options limit DRAM frequency calibration count from total 7 to 3, : other frequency will directly use the low frequency shu result. Please also extend, when this option should be selected (boottime).
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/dra... File src/soc/mediatek/mt8192/dramc_pi_main.c:
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/dra... PS2, Line 382: u8 size_t would use the native size. No reason to limit.
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/dra... PS2, Line 385: dramc_info("This shu no need do calibration, use shu0 result directly\n");
shu %s calibration skipped due to limitted frequency count. Using shu0.
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/dramc_pi_api.h:
https://review.coreboot.org/c/coreboot/+/44731/2/src/soc/mediatek/mt8192/inc... PS2, Line 286: bool is_freq_need_k(dram_cali_seq k_seq); Maybe we can come up with a better function name. Would the meaning of `is_freq_needing_k` be correct?