6 comments:
File src/mainboard/google/hatch/variants/baseboard/memory.c:
/* 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.c since it will be the same for all hatch variants.
File src/soc/intel/cannonlake/cnl_memcfg_init.c:
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 whatever is set in spd_smbus_address.
Patch Set #3, Line 121: printk
Probably add a die here? since this should not occur in production, but should be caught in development.
File src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h:
Patch Set #3, Line 43: mem_info_read_type
It would be helpful to add comments indicating what each type means.
Patch Set #3, Line 52: spd_data_by
A comment to explain how the data relates to the type would be helpful.
Patch Set #3, Line 62: struct spd_info spd;
how do you support configurations where you have one SPD in CBFS and one stored on DDR4 EEPROM?
There should be one mem_info_read_type for each slot.
Currently, FSP does not support that. IIUC, if SMBUS address is provided, it does not use the spd data pointers: https://review.coreboot.org/cgit/coreboot.git/tree/src/vendorcode/intel/fsp/fsp2_0/cannonlake/FspmUpd.h?id=refs/heads/master#n64
To view, visit change 32513. To unsubscribe, or for help writing mail filters, visit settings.