Patch set 8:Code-Review -1
5 comments:
File src/mainboard/google/hatch/variants/kohaku/memory.c:
Patch Set #8, Line 66: __weak
Why is weak added here?
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;
}
This is not correct. Kohaku and Hatch need to just fill in spd[0] and spd[2]. spd[1] and spd[3] are always NOT_EXISTING. You can just do something like this:
bcfg->spd[0].read_type = READ_SPD_CBFS;
bcfg->spd[0].spd_spec.spd_index = mem_sku;
if (!is_single_ch_mem) {
bcfg->spd[2].read_type = READ_SPD_CBFS;
bcfg->spd[2].spd_spec.spd_index = mem_sku;
}
In fact, everything except memcpy can be done in mainboard_memory_init_params in romstage.c since it is the same for kohaku and hatch.
File src/soc/intel/cannonlake/cnl_memcfg_init.c:
Patch Set #8, Line 114: struct spd_info spdi;
Just use a pointer? struct spd_info *spdi;
Patch Set #8, Line 131: cnl_cfg
Why not just pass in spd_index here?
File src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h:
Patch Set #8, Line 62: MEMMPTR
nit: MEMPTR
To view, visit change 32513. To unsubscribe, or for help writing mail filters, visit settings.