Philip Chen 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 11:
(3 comments)
https://review.coreboot.org/#/c/32513/11//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32513/11//COMMIT_MSG@19 PS11, Line 19: none
Can you please test to make sure that hatch_whl, hatch and kohaku boot fine?
Sure
https://review.coreboot.org/#/c/32513/11/src/mainboard/google/hatch/romstage... File src/mainboard/google/hatch/romstage.c:
https://review.coreboot.org/#/c/32513/11/src/mainboard/google/hatch/romstage... PS11, Line 37: memory_sku
So, one of the reasons why this was in baseboard defined as a __weak function was to allow any varia […]
OK
https://review.coreboot.org/#/c/32513/11/src/soc/intel/cannonlake/cnl_memcfg... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/#/c/32513/11/src/soc/intel/cannonlake/cnl_memcfg... PS11, Line 95: assert(mem_slot < NUM_DIMM_SLOT);
Is this really required?
It helps in case that someone adds code in this file later and calls meminit_cbfs_spd_index() with an out-of-bound mem_slot. Anyway it's no harm to keep the assert here.