Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32513 )
Change subject: soc/intel/cannonlake: Support different SPD read type for each slot ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/#/c/32513/6/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/memory.c:
https://review.coreboot.org/#/c/32513/6/src/mainboard/google/hatch/variants/... PS6, Line 60: for (int i = 0; i < ARRAY_SIZE(bcfg->spd); i++) { If bcfg->spd has more than 4 elements, the calculation to index into bcfg->channel_empty[] will read past the end of that array. Please use an assert() or a die() to protect against this failure case.
https://review.coreboot.org/#/c/32513/6/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/kohaku/memory.c:
https://review.coreboot.org/#/c/32513/6/src/mainboard/google/hatch/variants/... PS6, Line 90: for (int i = 0; i < ARRAY_SIZE(bcfg->spd); i++) { Same comment as for baseboard/memory.c line 60
https://review.coreboot.org/#/c/32513/6/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h:
https://review.coreboot.org/#/c/32513/6/src/soc/intel/cannonlake/include/soc... PS6, Line 67: struct spd_info spd[4]; The #define that Patrick Rudolph requested in cnl_memcfg_init.c line 60, please use that in the array definition here.