Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34872 )
Change subject: google/kukui: Load and run DRAM init blob ......................................................................
Patch Set 24:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... File src/mainboard/google/kukui/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 36: $(DRAM_CBFS)-file := 3rdparty/blobs/soc/mediatek/mt8183/dram.elf Should the path be configurable in Kconfig?
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 30: static int dram_run_full_calibration(struct sdram_params *freq_params) Please document the return values.
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 36: return -1; Please add debug messages.
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 39: return -2; Please add debug messages.
https://review.coreboot.org/c/coreboot/+/34872/24/src/mainboard/google/kukui... PS24, Line 55: printk(BIOS_ERR, "failed to do full calibration(%d), fall back to load default sdram param\n", err); 1. Please add a space before the opening bracket (. 2. *F*ailed 3. As this is more user visible, please add some more information.
The system will still work, but might not work with the best performance.
(No idea, if that is good.)