Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35110 )
Change subject: google/kukui: Load calibration params and run DRAM calibration flow ......................................................................
Patch Set 41:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35110/39/src/mainboard/google/kukui... File src/mainboard/google/kukui/romstage.c:
https://review.coreboot.org/c/coreboot/+/35110/39/src/mainboard/google/kukui... PS39, Line 64: -1;
should we enum these values?
The return value is not used (only compared with 0). What's the benefit to enum them?
https://review.coreboot.org/c/coreboot/+/35110/39/src/mainboard/google/kukui... PS39, Line 78: |
I think you mean &
Done
https://review.coreboot.org/c/coreboot/+/35110/39/src/mainboard/google/kukui... PS39, Line 79: return
printk(BIOS_ERR, "Full calibration blob executed without saving parameters. " […]
Done
https://review.coreboot.org/c/coreboot/+/35110/39/src/mainboard/google/kukui... PS39, Line 100: if (!read_calibration_data_from_flash(&dparam, sizeof(dparam)) || : !is_valid_dramc_param(&dparam) || : dparam.header.config != config) {
We probably want to give more reason telling user why we choose re-calibrating data. […]
How about using 'BIOS_NOTICE'? I don't think missing flash data is an error.