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 5:
(3 comments)
https://review.coreboot.org/#/c/32513/3/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/memory.c:
https://review.coreboot.org/#/c/32513/3/src/mainboard/google/hatch/variants/... PS3, Line 23: /* Access memory info using the spd file specified by spd_index */ : .spd = {.read_type = READ_SPD_CBFS},
I think its better to set this and spd_index in romstage. […]
I moved it to variant_memory_params() below because the per-slot spd info has dependence on whether single channel or not.
https://review.coreboot.org/#/c/32513/3/src/soc/intel/cannonlake/cnl_memcfg_... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/#/c/32513/3/src/soc/intel/cannonlake/cnl_memcfg_... PS3, Line 106: if (!cnl_cfg->spd.spd_spec.spd_smbus_address[i]) : continue;
Do we need this check? If the user is explicitly setting READ_SMBUS now, we should just copy whateve […]
Removed
https://review.coreboot.org/#/c/32513/3/src/soc/intel/cannonlake/cnl_memcfg_... PS3, Line 121: printk
Probably add a die here? since this should not occur in production, but should be caught in developm […]
Done