Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32513 )
Change subject: soc/intel/cannonlake: Clean up cannonlake_memcfg_init ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/32513/3/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h:
https://review.coreboot.org/#/c/32513/3/src/soc/intel/cannonlake/include/soc... PS3, Line 62: struct spd_info spd;
It's supported when you fetch the SPD in coreboot and point to it in heap. […]
Actually, looking back at the comments in the header file above, I think it is possible to do it with FSP i.e. each address entry is treated differently. In that case, we need to have spd_info for each slot rather than just read_type for each slot.
i.e. struct spd_info spd[MAX_SLOTS] which would be 4 for CNL and family.
Then each board needs to set the configuration it has.
e.g.: spd_info[] = { [0] = { .read_type = READ_SMBUS, .spd_smbus_address = 0xXX }, [1] = { .read_type = NOT_EXISTING }, [2] = { .read_type = READ_SPD_CBFS, .spd_index = X }, [3] = { .read_type = READ_SPD_MEMPTR, .spd_data_ptr_info = { ... } }, },
And then cannonlake_memcfg_init will have to address each slot based on the read_type for it.
In fact, cannonlake_memcfg_init also needs to set DisableDimmChannel0 and DisableDimmChannel1 as per the spd_info above i.e. if any read_type is NOT_EXISTING then that Dimm of the channel should be disabled.