Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37303 )
Change subject: soc/intel/broadwell_de: Re-read SPD on CRC error ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37303/1/src/soc/intel/fsp_broadwell... File src/soc/intel/fsp_broadwell_de/romstage/memory.c:
https://review.coreboot.org/c/coreboot/+/37303/1/src/soc/intel/fsp_broadwell... PS1, Line 65: "SPD CRC error on channel %d slot %d, retrying..\n",
nit: ellipses usually have 3 dots
Done
https://review.coreboot.org/c/coreboot/+/37303/1/src/soc/intel/fsp_broadwell... PS1, Line 63: while (res == SPD_STATUS_CRC_ERROR && tries++ < MAX_SPD_READ_TRIES) { : printk(BIOS_ERR, : "SPD CRC error on channel %d slot %d, retrying..\n", : channel, slot); : get_spd_smbus(&blk); : res = spd_decode_ddr4(&dimm, spd_data); : } : if (res == SPD_STATUS_OK) {
How about using a do{}while loop here? […]
thanks, I like the less DRY code idea
https://review.coreboot.org/c/coreboot/+/37303/1/src/soc/intel/fsp_broadwell... PS1, Line 70: if (res == SPD_STATUS_OK) {
braces {} are not necessary for single statement blocks
Done