Attention is currently required from: Paul Menzel, Subrata Banik, Angel Pons, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56650 )
Change subject: soc/intel/common: Don't suppress SPD Module Type value ......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/smbios.c:
https://review.coreboot.org/c/coreboot/+/56650/comment/6872d51b_4c50768f PS3, Line 18: /* Translate to DDR2 module type field that SMBIOS code expects. */
Yes i don't know that answer either and IMHO we should let platform choose if they want to go through this conversion or pass the module type directly.
But that is not really a platform decision. We have module type that is provided by SPD which the platform gets either from CBFS SPD or EEPROM SPD. That module type is specific to the ddr type. SMBIOS has its own definitions for memory form factor. So, what we need is conversion of module type for a specific ddr type into smbios form factor. This can be handled by the platform, but I don't think that is required. It will help keep things simpler so that platforms don't have to worry about the conversion.
Its possible to add the type conversion how DDR3 had done so far for other memory type as well like DDR4/LP4/5 but does that make sense is my question?
No, we shouldn't be converting other memory types to DDR2 type. We should be converting all memory types into smbios type directly. i.e. in `create_smbios_type17_for_dimm()` we should do something like this:
``` if (dimm->ddr_type == MEMORY_TYPE_DDR2) t->form_factor = convert_ddr2_module_type(dimm->mod_type); else if (dimm->ddr_type == MEMORY_TYPE_DDR3) t->form_factor = convert_ddr3_module_type(dimm->mod_type); else if (dimm->ddr_type == MEMORY_TYPE_DDR4) t->form_factor = convert_ddr4_module_type(dimm->mod_type); else if (dimm->ddr_type == MEMORY_TYPE_LPDDR3) t->form_factor = convert_lp3_module_type(dimm->mod_type); else ... ```