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 10:
(5 comments)
https://review.coreboot.org/#/c/32513/8/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/kohaku/memory.c:
https://review.coreboot.org/#/c/32513/8/src/mainboard/google/hatch/variants/... PS8, Line 66: __weak
Why is weak added here?
My fault over c&p. Just removed it.
https://review.coreboot.org/#/c/32513/8/src/mainboard/google/hatch/variants/... PS8, Line 80: for (int i = 0; i < NUM_DIMM_SLOT; i++) { : if (is_single_ch_mem && i >= (NUM_DIMM_SLOT / 2)) : bcfg->spd[i].read_type = NOT_EXISTING; : else { : bcfg->spd[i].read_type = READ_SPD_CBFS; : bcfg->spd[i].spd_spec.spd_index = mem_sku; : }
What Furquan said. […]
Fixed and removed the common code to romstage.c. Thanks for pointing out this error.
https://review.coreboot.org/#/c/32513/8/src/soc/intel/cannonlake/cnl_memcfg_... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/#/c/32513/8/src/soc/intel/cannonlake/cnl_memcfg_... PS8, Line 114: struct spd_info spdi;
Just use a pointer? struct spd_info *spdi;
Done
https://review.coreboot.org/#/c/32513/8/src/soc/intel/cannonlake/cnl_memcfg_... PS8, Line 122: spdi = cnl_cfg->spd[i];
As Furquan suggested, use a pointer. […]
Done
https://review.coreboot.org/#/c/32513/8/src/soc/intel/cannonlake/cnl_memcfg_... PS8, Line 131: cnl_cfg
Why not just pass in spd_index here?
Good idea. Fixed.