arch/x86: Refactor the SMBIOS type 17 write function

List of changes:
1. Create Module Type macros as per Memory Type
(i.e. DDR2/DDR3/DDR4/DDR5/LPDDR4/LPDDR5) and fix compilation
issue due to renaming of existing macros due to scoping the Memory
Type.
2. Use dedicated Memory Type and Module type for `Form Factor`
and `TypeDetail` conversion using `get_spd_info()` function.
3. Create a new API (convert_form_factor_to_module_type()) for
`Form Factor` to 'Module type' conversion as per `Memory Type`.
4. Add new argument as `Memory Type` to
smbios_form_factor_to_spd_mod_type() so that it can internally
call convert_form_factor_to_module_type() for `Module Type`
conversion.
5. Update `test_smbios_form_factor_to_spd_mod_type()` to
accommodate different memory types.
6. Skip fixed module type to form factor conversion using DDR2 SPD4
specification (inside dimm_info_fill()).

Refer to datasheet SPD4.1.2.M-1 for LPDDRx and SPD4.1.2.L-3 for DDRx.

BUG=b:194659789
TEST=Refer to dmidecode -t 17 output as below:
Without this code change:

Handle 0x0012, DMI type 17, 40 bytes
Memory Device
        Array Handle: 0x000A
        Error Information Handle: Not Provided
        Total Width: 16 bits
        Data Width: 16 bits
        Size: 2048 MB
        Form Factor: Unknown
        ....

With this code change:

Handle 0x0012, DMI type 17, 40 bytes
Memory Device
        Array Handle: 0x000A
        Error Information Handle: Not Provided
        Total Width: 16 bits
        Data Width: 16 bits
        Size: 2048 MB
        Form Factor: Row Of Chips
        ....

Change-Id: Ia337ac8f50b61ae78d86a07c7a86aa9c248bad50
Signed-off-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56628
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jakub Czapiga <jacz@semihalf.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>

15 files changed, 404 insertions, 107 deletions

diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index c05f905892..c48ce86c47 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -224,6 +224,9 @@ static int create_smbios_type17_for_dimm(struct dimm_info *dimm,
 					 unsigned long *current, int *handle,
 					 int type16_handle)
 {
+	struct spd_info info;
+	get_spd_info(dimm->ddr_type, dimm->mod_type, &info);
+
 	struct smbios_type17 *t = smbios_carve_table(*current, SMBIOS_MEMORY_DEVICE,
 						     sizeof(*t), *handle);
 
@@ -244,24 +247,7 @@ static int create_smbios_type17_for_dimm(struct dimm_info *dimm,
 	}
 	t->data_width = 8 * (1 << (dimm->bus_width & 0x7));
 	t->total_width = t->data_width + 8 * ((dimm->bus_width & 0x18) >> 3);
-
-	switch (dimm->mod_type) {
-	case SPD_RDIMM:
-	case SPD_MINI_RDIMM:
-		t->form_factor = MEMORY_FORMFACTOR_RIMM;
-		break;
-	case SPD_UDIMM:
-	case SPD_MICRO_DIMM:
-	case SPD_MINI_UDIMM:
-		t->form_factor = MEMORY_FORMFACTOR_DIMM;
-		break;
-	case SPD_SODIMM:
-		t->form_factor = MEMORY_FORMFACTOR_SODIMM;
-		break;
-	default:
-		t->form_factor = MEMORY_FORMFACTOR_UNKNOWN;
-		break;
-	}
+	t->form_factor = info.form_factor;
 
 	smbios_fill_dimm_manufacturer_from_id(dimm->mod_id, t);
 	smbios_fill_dimm_serial_number(dimm, t);
@@ -278,19 +264,8 @@ static int create_smbios_type17_for_dimm(struct dimm_info *dimm,
 	t->maximum_voltage = dimm->vdd_voltage;
 
 	/* Fill in type detail */
-	switch (dimm->mod_type) {
-	case SPD_RDIMM:
-	case SPD_MINI_RDIMM:
-		t->type_detail = MEMORY_TYPE_DETAIL_REGISTERED;
-		break;
-	case SPD_UDIMM:
-	case SPD_MINI_UDIMM:
-		t->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED;
-		break;
-	default:
-		t->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
-		break;
-	}
+	t->type_detail = info.type_detail;
+
 	/* Synchronous = 1 */
 	t->type_detail |= MEMORY_TYPE_DETAIL_SYNCHRONOUS;
 	/* no handle for error information */
diff --git a/src/device/dram/ddr3.c b/src/device/dram/ddr3.c
index 0a32d02de9..b99730d45b 100644
--- a/src/device/dram/ddr3.c
+++ b/src/device/dram/ddr3.c
@@ -545,19 +545,19 @@ enum cb_err spd_add_smbios17(const u8 channel, const u8 slot,
 
 		switch (info->dimm_type) {
 		case SPD_DDR3_DIMM_TYPE_SO_DIMM:
-			dimm->mod_type = SPD_SODIMM;
+			dimm->mod_type = DDR3_SPD_SODIMM;
 			break;
 		case SPD_DDR3_DIMM_TYPE_72B_SO_CDIMM:
-			dimm->mod_type = SPD_72B_SO_CDIMM;
+			dimm->mod_type = DDR3_SPD_72B_SO_CDIMM;
 			break;
 		case SPD_DDR3_DIMM_TYPE_72B_SO_RDIMM:
-			dimm->mod_type = SPD_72B_SO_RDIMM;
+			dimm->mod_type = DDR3_SPD_72B_SO_RDIMM;
 			break;
 		case SPD_DDR3_DIMM_TYPE_UDIMM:
-			dimm->mod_type = SPD_UDIMM;
+			dimm->mod_type = DDR3_SPD_UDIMM;
 			break;
 		case SPD_DDR3_DIMM_TYPE_RDIMM:
-			dimm->mod_type = SPD_RDIMM;
+			dimm->mod_type = DDR3_SPD_RDIMM;
 			break;
 		case SPD_DDR3_DIMM_TYPE_UNDEFINED:
 		default:
diff --git a/src/device/dram/ddr4.c b/src/device/dram/ddr4.c
index c5a8d13f53..eea5f0af56 100644
--- a/src/device/dram/ddr4.c
+++ b/src/device/dram/ddr4.c
@@ -299,16 +299,16 @@ enum cb_err spd_add_smbios17_ddr4(const u8 channel, const u8 slot, const u16 sel
 
 		switch (info->dimm_type) {
 		case SPD_DDR4_DIMM_TYPE_SO_DIMM:
-			dimm->mod_type = SPD_SODIMM;
+			dimm->mod_type = DDR4_SPD_SODIMM;
 			break;
 		case SPD_DDR4_DIMM_TYPE_72B_SO_RDIMM:
-			dimm->mod_type = SPD_72B_SO_RDIMM;
+			dimm->mod_type = DDR4_SPD_72B_SO_RDIMM;
 			break;
 		case SPD_DDR4_DIMM_TYPE_UDIMM:
-			dimm->mod_type = SPD_UDIMM;
+			dimm->mod_type = DDR4_SPD_UDIMM;
 			break;
 		case SPD_DDR4_DIMM_TYPE_RDIMM:
-			dimm->mod_type = SPD_RDIMM;
+			dimm->mod_type = DDR4_SPD_RDIMM;
 			break;
 		default:
 			dimm->mod_type = SPD_UNDEFINED;
diff --git a/src/device/dram/spd.c b/src/device/dram/spd.c
index 0b2dd49eee..11808e2cf3 100644
--- a/src/device/dram/spd.c
+++ b/src/device/dram/spd.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
 #include <device/dram/spd.h>
+#include <spd.h>
 
 const char *spd_manufacturer_name(const uint16_t mod_id)
 {
@@ -38,3 +39,219 @@ const char *spd_manufacturer_name(const uint16_t mod_id)
 		return NULL;
 	}
 }
+
+static void convert_default_module_type_to_spd_info(struct spd_info *info)
+{
+	info->form_factor = MEMORY_FORMFACTOR_UNKNOWN;
+	info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
+}
+
+static void convert_ddr2_module_type_to_spd_info(enum ddr2_module_type module_type,
+		struct spd_info *info)
+{
+	switch (module_type) {
+	case DDR2_SPD_RDIMM:
+	case DDR2_SPD_MINI_RDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_RIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_REGISTERED;
+		break;
+	case DDR2_SPD_UDIMM:
+	case DDR2_SPD_MINI_UDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_DIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED;
+		break;
+	case DDR2_SPD_MICRO_DIMM:
+		info->form_factor = MEMORY_FORMFACTOR_DIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
+		break;
+	case DDR2_SPD_SODIMM:
+		info->form_factor = MEMORY_FORMFACTOR_SODIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
+		break;
+	default:
+		convert_default_module_type_to_spd_info(info);
+		break;
+	}
+}
+
+static void convert_ddr3_module_type_to_spd_info(enum ddr3_module_type module_type,
+		struct spd_info *info)
+{
+	switch (module_type) {
+	case DDR3_SPD_RDIMM:
+	case DDR3_SPD_MINI_RDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_RIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_REGISTERED;
+		break;
+	case DDR3_SPD_UDIMM:
+	case DDR3_SPD_MINI_UDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_DIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED;
+		break;
+	case DDR3_SPD_MICRO_DIMM:
+		info->form_factor = MEMORY_FORMFACTOR_DIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
+		break;
+	case DDR3_SPD_SODIMM:
+	case DDR3_SPD_72B_SO_UDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_SODIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
+		break;
+	default:
+		convert_default_module_type_to_spd_info(info);
+		break;
+	}
+}
+
+static void convert_ddr4_module_type_to_spd_info(enum ddr4_module_type module_type,
+		struct spd_info *info)
+{
+	switch (module_type) {
+	case DDR4_SPD_RDIMM:
+	case DDR4_SPD_MINI_RDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_RIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_REGISTERED;
+		break;
+	case DDR4_SPD_UDIMM:
+	case DDR4_SPD_MINI_UDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_DIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED;
+		break;
+	case DDR4_SPD_SODIMM:
+	case DDR4_SPD_72B_SO_UDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_SODIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
+		break;
+	default:
+		convert_default_module_type_to_spd_info(info);
+		break;
+	}
+}
+
+static void convert_ddr5_module_type_to_spd_info(enum ddr5_module_type module_type,
+		struct spd_info *info)
+{
+	switch (module_type) {
+	case DDR5_SPD_RDIMM:
+	case DDR5_SPD_MINI_RDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_RIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_REGISTERED;
+		break;
+	case DDR5_SPD_UDIMM:
+	case DDR5_SPD_MINI_UDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_DIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNBUFFERED;
+		break;
+	case DDR5_SPD_SODIMM:
+	case DDR5_SPD_72B_SO_UDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_SODIMM;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
+		break;
+	case DDR5_SPD_2DPC:
+		info->form_factor = MEMORY_FORMFACTOR_PROPRIETARY_CARD;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
+		break;
+	default:
+		convert_default_module_type_to_spd_info(info);
+		break;
+	}
+}
+
+static void convert_lpx_module_type_to_spd_info(enum lpx_module_type module_type,
+		struct spd_info *info)
+{
+	switch (module_type) {
+	case LPX_SPD_NONDIMM:
+		info->form_factor = MEMORY_FORMFACTOR_ROC;
+		info->type_detail = MEMORY_TYPE_DETAIL_UNKNOWN;
+		break;
+	default:
+		convert_default_module_type_to_spd_info(info);
+		break;
+	}
+}
+
+void get_spd_info(smbios_memory_type memory_type, uint8_t module_type, struct spd_info *info)
+{
+	switch (memory_type) {
+	case MEMORY_TYPE_DDR2:
+		convert_ddr2_module_type_to_spd_info(module_type, info);
+		break;
+	case MEMORY_TYPE_DDR3:
+		convert_ddr3_module_type_to_spd_info(module_type, info);
+		break;
+	case MEMORY_TYPE_DDR4:
+		convert_ddr4_module_type_to_spd_info(module_type, info);
+		break;
+	case MEMORY_TYPE_DDR5:
+		convert_ddr5_module_type_to_spd_info(module_type, info);
+		break;
+	case MEMORY_TYPE_LPDDR3:
+	case MEMORY_TYPE_LPDDR4:
+	case MEMORY_TYPE_LPDDR5:
+		convert_lpx_module_type_to_spd_info(module_type, info);
+		break;
+	default:
+		convert_default_module_type_to_spd_info(info);
+		break;
+	}
+}
+
+static uint8_t convert_default_form_factor_to_module_type(void)
+{
+	return SPD_UNDEFINED;
+}
+
+static uint8_t convert_ddrx_form_factor_to_module_type(smbios_memory_type memory_type,
+		smbios_memory_form_factor form_factor)
+{
+	uint8_t module_type;
+
+	switch (form_factor) {
+	case MEMORY_FORMFACTOR_DIMM:
+		return DDR2_SPD_UDIMM;
+	case MEMORY_FORMFACTOR_RIMM:
+		return DDR2_SPD_RDIMM;
+	case MEMORY_FORMFACTOR_SODIMM:
+		module_type = (memory_type == MEMORY_TYPE_DDR2) ? DDR2_SPD_SODIMM
+				: DDR3_SPD_SODIMM;
+		return module_type;
+	default:
+		return convert_default_form_factor_to_module_type();
+	}
+}
+
+static uint8_t convert_lpx_form_factor_to_module_type(smbios_memory_form_factor form_factor)
+{
+	switch (form_factor) {
+	case MEMORY_FORMFACTOR_ROC:
+		return LPX_SPD_NONDIMM;
+	default:
+		return convert_default_form_factor_to_module_type();
+	}
+}
+
+uint8_t convert_form_factor_to_module_type(smbios_memory_type memory_type,
+		smbios_memory_form_factor form_factor)
+{
+	uint8_t module_type;
+
+	switch (memory_type) {
+	case MEMORY_TYPE_DDR2:
+	case MEMORY_TYPE_DDR3:
+	case MEMORY_TYPE_DDR4:
+	case MEMORY_TYPE_DDR5:
+		module_type = convert_ddrx_form_factor_to_module_type(memory_type, form_factor);
+		break;
+	case MEMORY_TYPE_LPDDR3:
+	case MEMORY_TYPE_LPDDR4:
+	case MEMORY_TYPE_LPDDR5:
+		module_type = convert_lpx_form_factor_to_module_type(form_factor);
+		break;
+	default:
+		module_type = convert_default_form_factor_to_module_type();
+		break;
+	}
+
+	return module_type;
+}
diff --git a/src/include/device/dram/spd.h b/src/include/device/dram/spd.h
index c677f4ce9b..1a86ea374f 100644
--- a/src/include/device/dram/spd.h
+++ b/src/include/device/dram/spd.h
@@ -3,8 +3,18 @@
 #ifndef DEVICE_DRAM_SPD_H
 #define DEVICE_DRAM_SPD_H
 
+#include <smbios.h>
 #include <types.h>
 
 const char *spd_manufacturer_name(const uint16_t mod_id);
 
+struct spd_info {
+	uint16_t type_detail;
+	uint8_t form_factor;
+};
+
+void get_spd_info(smbios_memory_type memory_type, uint8_t module_type, struct spd_info *info);
+uint8_t convert_form_factor_to_module_type(smbios_memory_type memory_type,
+		smbios_memory_form_factor form_factor);
+
 #endif /* DEVICE_DRAM_SPD_H */
diff --git a/src/include/dimm_info_util.h b/src/include/dimm_info_util.h
index 04e9336179..e7285d5c93 100644
--- a/src/include/dimm_info_util.h
+++ b/src/include/dimm_info_util.h
@@ -28,7 +28,7 @@ uint32_t smbios_memory_size_to_mib(uint16_t memory_size,
  *
  * Use this when setting dimm_info.mod_type.
  */
-uint8_t
-smbios_form_factor_to_spd_mod_type(smbios_memory_form_factor form_factor);
+uint8_t smbios_form_factor_to_spd_mod_type(smbios_memory_type memory_type,
+		smbios_memory_form_factor form_factor);
 
 #endif
diff --git a/src/include/spd.h b/src/include/spd.h
index af3072e8ec..8493d40b6e 100644
--- a/src/include/spd.h
+++ b/src/include/spd.h
@@ -197,18 +197,70 @@ enum spd_memory_type {
 #define MODULE_BUFFERED                  1
 #define MODULE_REGISTERED                2
 
-/* Byte 3: Module type information */
 #define SPD_UNDEFINED 0x00
-#define SPD_RDIMM 0x01
-#define SPD_UDIMM 0x02
-#define SPD_SODIMM 0x04
-#define SPD_72B_SO_CDIMM 0x06
-#define SPD_72B_SO_RDIMM 0x07
-#define SPD_MICRO_DIMM 0x08
-#define SPD_MINI_RDIMM 0x10
-#define SPD_MINI_UDIMM 0x20
-
 #define SPD_ECC_8BIT (1<<3)
 #define SPD_ECC_8BIT_LP5_DDR5 (1<<4)
 
+/* Byte 3: Module type information */
+enum ddr2_module_type {
+	DDR2_SPD_RDIMM = 0x01,
+	DDR2_SPD_UDIMM = 0x02,
+	DDR2_SPD_SODIMM = 0x04,
+	DDR2_SPD_72B_SO_CDIMM = 0x06,
+	DDR2_SPD_72B_SO_RDIMM = 0x07,
+	DDR2_SPD_MICRO_DIMM = 0x08,
+	DDR2_SPD_MINI_RDIMM = 0x10,
+	DDR2_SPD_MINI_UDIMM = 0x20,
+};
+
+enum ddr3_module_type {
+	DDR3_SPD_RDIMM = 0x01,
+	DDR3_SPD_UDIMM = 0x02,
+	DDR3_SPD_SODIMM = 0x03,
+	DDR3_SPD_MICRO_DIMM = 0x04,
+	DDR3_SPD_MINI_RDIMM = 0x05,
+	DDR3_SPD_MINI_UDIMM = 0x06,
+	DDR3_SPD_MINI_CDIMM = 0x07,
+	DDR3_SPD_72B_SO_UDIMM = 0x08,
+	DDR3_SPD_72B_SO_RDIMM = 0x09,
+	DDR3_SPD_72B_SO_CDIMM = 0x0a,
+	DDR3_SPD_LRDIMM = 0x0b,
+	DDR3_SPD_16B_SO_DIMM = 0x0c,
+	DDR3_SPD_32B_SO_RDIMM = 0x0d,
+};
+
+enum ddr4_module_type {
+	DDR4_SPD_RDIMM = 0x01,
+	DDR4_SPD_UDIMM = 0x02,
+	DDR4_SPD_SODIMM = 0x03,
+	DDR4_SPD_LRDIMM = 0x04,
+	DDR4_SPD_MINI_RDIMM = 0x05,
+	DDR4_SPD_MINI_UDIMM = 0x06,
+	DDR4_SPD_72B_SO_UDIMM = 0x08,
+	DDR4_SPD_72B_SO_RDIMM = 0x09,
+	DDR4_SPD_16B_SO_DIMM = 0x0c,
+	DDR4_SPD_32B_SO_RDIMM = 0x0d,
+};
+
+enum ddr5_module_type {
+	DDR5_SPD_RDIMM = 0x01,
+	DDR5_SPD_UDIMM = 0x02,
+	DDR5_SPD_SODIMM = 0x03,
+	DDR5_SPD_LRDIMM = 0x04,
+	DDR5_SPD_MINI_RDIMM = 0x05,
+	DDR5_SPD_MINI_UDIMM = 0x06,
+	DDR5_SPD_72B_SO_UDIMM = 0x08,
+	DDR5_SPD_72B_SO_RDIMM = 0x09,
+	DDR5_SPD_SOLDERED_DOWN = 0x0b,
+	DDR5_SPD_16B_SO_DIMM = 0x0c,
+	DDR5_SPD_32B_SO_RDIMM = 0x0d,
+	DDR5_SPD_1DPC = 0x0e,
+	DDR5_SPD_2DPC = 0x0f,
+};
+
+enum lpx_module_type {
+	LPX_SPD_LPDIMM = 0x07,
+	LPX_SPD_NONDIMM = 0x0e,
+};
+
 #endif
diff --git a/src/lib/dimm_info_util.c b/src/lib/dimm_info_util.c
index 0e7cc12f18..3507366a3d 100644
--- a/src/lib/dimm_info_util.c
+++ b/src/lib/dimm_info_util.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 
+#include <device/dram/spd.h>
 #include <dimm_info_util.h>
 #include <smbios.h>
 #include <spd.h>
@@ -72,18 +73,8 @@ uint32_t smbios_memory_size_to_mib(uint16_t memory_size, uint32_t extended_size)
 		return memory_size;
 }
 
-uint8_t
-smbios_form_factor_to_spd_mod_type(smbios_memory_form_factor form_factor)
+uint8_t smbios_form_factor_to_spd_mod_type(smbios_memory_type memory_type,
+		smbios_memory_form_factor form_factor)
 {
-	/* This switch reverses the switch in smbios.c */
-	switch (form_factor) {
-	case MEMORY_FORMFACTOR_DIMM:
-		return SPD_UDIMM;
-	case MEMORY_FORMFACTOR_RIMM:
-		return SPD_RDIMM;
-	case MEMORY_FORMFACTOR_SODIMM:
-		return SPD_SODIMM;
-	default:
-		return SPD_UNDEFINED;
-	}
+	return convert_form_factor_to_module_type(memory_type, form_factor);
 }
diff --git a/src/mainboard/scaleway/tagada/ramstage.c b/src/mainboard/scaleway/tagada/ramstage.c
index 6d5d3ff4fd..bb0a385f7e 100644
--- a/src/mainboard/scaleway/tagada/ramstage.c
+++ b/src/mainboard/scaleway/tagada/ramstage.c
@@ -83,5 +83,5 @@ void mainboard_add_dimm_info(
 	int channel, int dimm, int index)
 {
 	/* Mainboard only has DDR4 DIMM slots */
-	mem_info->dimm[index].mod_type = SPD_UDIMM;
+	mem_info->dimm[index].mod_type = DDR4_SPD_UDIMM;
 }
diff --git a/src/northbridge/intel/haswell/haswell_mrc/raminit.c b/src/northbridge/intel/haswell/haswell_mrc/raminit.c
index f71d1173e3..e46eb57826 100644
--- a/src/northbridge/intel/haswell/haswell_mrc/raminit.c
+++ b/src/northbridge/intel/haswell/haswell_mrc/raminit.c
@@ -260,7 +260,7 @@ static void setup_sdram_meminfo(struct pei_data *pei_data)
 				dimm->mod_id =
 					(pei_data->spd_data[index][SPD_DIMM_MOD_ID2] << 8) |
 					(pei_data->spd_data[index][SPD_DIMM_MOD_ID1] & 0xff);
-				dimm->mod_type = SPD_SODIMM;
+				dimm->mod_type = DDR3_SPD_SODIMM;
 				dimm->bus_width = MEMORY_BUS_WIDTH_64;
 				dimm_cnt++;
 			}
diff --git a/src/soc/amd/common/fsp/dmi.c b/src/soc/amd/common/fsp/dmi.c
index 223cce1577..b96b047627 100644
--- a/src/soc/amd/common/fsp/dmi.c
+++ b/src/soc/amd/common/fsp/dmi.c
@@ -52,7 +52,8 @@ static void transfer_memory_info(const TYPE17_DMI_INFO *dmi17,
 
 	dimm->rank_per_dimm = dmi17->Attributes;
 
-	dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->FormFactor);
+	dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->MemoryType,
+						dmi17->FormFactor);
 
 	dimm->bus_width = smbios_bus_width_to_spd_width(dmi17->MemoryType, dmi17->TotalWidth,
 						dmi17->DataWidth);
diff --git a/src/soc/amd/common/pi/amd_late_init.c b/src/soc/amd/common/pi/amd_late_init.c
index 3714404590..4d2250b5ce 100644
--- a/src/soc/amd/common/pi/amd_late_init.c
+++ b/src/soc/amd/common/pi/amd_late_init.c
@@ -36,7 +36,8 @@ static void transfer_memory_info(TYPE17_DMI_INFO *dmi17,
 
 	dimm->rank_per_dimm = dmi17->Attributes;
 
-	dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->FormFactor);
+	dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->MemoryType,
+						dmi17->FormFactor);
 
 	dimm->bus_width = smbios_bus_width_to_spd_width(dmi17->MemoryType, dmi17->TotalWidth,
 						dmi17->DataWidth);
diff --git a/src/soc/intel/common/smbios.c b/src/soc/intel/common/smbios.c
index 213be6578b..4aca022d6f 100644
--- a/src/soc/intel/common/smbios.c
+++ b/src/soc/intel/common/smbios.c
@@ -18,28 +18,7 @@ void dimm_info_fill(struct dimm_info *dimm, u32 dimm_capacity, u8 ddr_type,
 		bool ecc_support, u16 mod_id, u8 mod_type)
 {
 	dimm->mod_id = mod_id;
-	/* Translate to DDR2 module type field that SMBIOS code expects. */
-	switch (mod_type) {
-	case SPD_DDR3_DIMM_TYPE_SO_DIMM:
-		dimm->mod_type = SPD_SODIMM;
-		break;
-	case SPD_DDR3_DIMM_TYPE_72B_SO_CDIMM:
-		dimm->mod_type = SPD_72B_SO_CDIMM;
-		break;
-	case SPD_DDR3_DIMM_TYPE_72B_SO_RDIMM:
-		dimm->mod_type = SPD_72B_SO_RDIMM;
-		break;
-	case SPD_DDR3_DIMM_TYPE_UDIMM:
-		dimm->mod_type = SPD_UDIMM;
-		break;
-	case SPD_DDR3_DIMM_TYPE_RDIMM:
-		dimm->mod_type = SPD_RDIMM;
-		break;
-	case SPD_DDR3_DIMM_TYPE_UNDEFINED:
-	default:
-		dimm->mod_type = SPD_UNDEFINED;
-		break;
-	}
+	dimm->mod_type = mod_type;
 	dimm->dimm_size = dimm_capacity;
 	dimm->ddr_type = ddr_type;
 	dimm->ddr_frequency = frequency;
diff --git a/tests/lib/Makefile.inc b/tests/lib/Makefile.inc
index 0aba0f5513..0aea0ce62a 100644
--- a/tests/lib/Makefile.inc
+++ b/tests/lib/Makefile.inc
@@ -138,6 +138,7 @@ bootmem-test-srcs += src/lib/bootmem.c
 bootmem-test-srcs += src/lib/memrange.c
 
 dimm_info_util-test-srcs += tests/lib/dimm_info_util-test.c
+dimm_info_util-test-srcs += src/device/dram/spd.c
 dimm_info_util-test-srcs += src/lib/dimm_info_util.c
 dimm_info_util-test-srcs += tests/stubs/console.c
 
diff --git a/tests/lib/dimm_info_util-test.c b/tests/lib/dimm_info_util-test.c
index 25800d988f..cfe7176006 100644
--- a/tests/lib/dimm_info_util-test.c
+++ b/tests/lib/dimm_info_util-test.c
@@ -4,6 +4,8 @@
 #include <spd.h>
 #include <tests/test.h>
 
+#define MAX_ALLOWED_MODULE_TYPE 3
+
 static void test_smbios_bus_width_to_spd_width_parametrized(smbios_memory_type ddr_type)
 {
 	/* Non-ECC variants */
@@ -86,22 +88,8 @@ static void test_smbios_memory_size_to_mib(void **state)
 	assert_int_equal(memory_size, smbios_memory_size_to_mib(memory_size, 694735));
 }
 
-static void test_smbios_form_factor_to_spd_mod_type(void **state)
+static void test_smbios_form_factor_to_spd_mod_type_ddr(smbios_memory_type memory_type)
 {
-	/* Form factors defined in coreboot */
-	const LargestIntegralType udimm_allowed[] = {
-		SPD_UDIMM, SPD_MICRO_DIMM, SPD_MINI_UDIMM,
-	};
-	assert_in_set(smbios_form_factor_to_spd_mod_type(MEMORY_FORMFACTOR_DIMM),
-			udimm_allowed, ARRAY_SIZE(udimm_allowed));
-
-	const LargestIntegralType rdimm_allowed[] = { SPD_RDIMM, SPD_MINI_RDIMM };
-	assert_in_set(smbios_form_factor_to_spd_mod_type(MEMORY_FORMFACTOR_RIMM),
-			rdimm_allowed, ARRAY_SIZE(rdimm_allowed));
-
-	assert_int_equal(SPD_SODIMM,
-			smbios_form_factor_to_spd_mod_type(MEMORY_FORMFACTOR_SODIMM));
-
 	const smbios_memory_form_factor undefined_factors[] = {
 		MEMORY_FORMFACTOR_OTHER,
 		MEMORY_FORMFACTOR_UNKNOWN,
@@ -119,10 +107,92 @@ static void test_smbios_form_factor_to_spd_mod_type(void **state)
 	};
 	for (int i = 0; i < ARRAY_SIZE(undefined_factors); ++i) {
 		assert_int_equal(SPD_UNDEFINED,
-				smbios_form_factor_to_spd_mod_type(undefined_factors[i]));
+				smbios_form_factor_to_spd_mod_type(memory_type,
+						undefined_factors[i]));
 	}
 }
 
+static void test_smbios_form_factor_to_spd_mod_type_ddrx_parametrized(
+		smbios_memory_type memory_type,
+		const LargestIntegralType udimm_allowed[],
+		const LargestIntegralType rdimm_allowed[],
+		LargestIntegralType expected_module_type)
+{
+	print_message("%s(%d)\n", __func__, memory_type);
+
+	assert_in_set(smbios_form_factor_to_spd_mod_type(memory_type, MEMORY_FORMFACTOR_DIMM),
+			udimm_allowed, MAX_ALLOWED_MODULE_TYPE);
+
+	assert_in_set(smbios_form_factor_to_spd_mod_type(memory_type, MEMORY_FORMFACTOR_RIMM),
+			rdimm_allowed, MAX_ALLOWED_MODULE_TYPE);
+
+	assert_int_equal(expected_module_type, smbios_form_factor_to_spd_mod_type(memory_type,
+					MEMORY_FORMFACTOR_SODIMM));
+
+	test_smbios_form_factor_to_spd_mod_type_ddr(memory_type);
+}
+
+static void test_smbios_form_factor_to_spd_mod_type_lpddrx(smbios_memory_type memory_type)
+{
+	print_message("%s(%d)\n", __func__, memory_type);
+	/* Form factors defined in coreboot */
+	assert_int_equal(LPX_SPD_NONDIMM, smbios_form_factor_to_spd_mod_type(memory_type,
+					MEMORY_FORMFACTOR_ROC));
+}
+
+static void test_smbios_form_factor_to_spd_mod_type(void **state)
+{
+	const struct smbios_form_factor_test_info_ddrx {
+		smbios_memory_type memory_type;
+		const LargestIntegralType udimm_allowed[MAX_ALLOWED_MODULE_TYPE];
+		const LargestIntegralType rdimm_allowed[MAX_ALLOWED_MODULE_TYPE];
+		LargestIntegralType expected_module_type;
+	} ddrx_info[] = {
+		{
+			.memory_type = MEMORY_TYPE_DDR2,
+			.udimm_allowed = { DDR2_SPD_UDIMM, DDR2_SPD_MICRO_DIMM,
+						DDR2_SPD_MINI_UDIMM },
+			.rdimm_allowed = { DDR2_SPD_RDIMM, DDR2_SPD_MINI_RDIMM },
+			.expected_module_type = DDR2_SPD_SODIMM,
+		},
+		{
+			.memory_type = MEMORY_TYPE_DDR3,
+			.udimm_allowed = { DDR3_SPD_UDIMM, DDR3_SPD_MICRO_DIMM,
+						DDR3_SPD_MINI_UDIMM },
+			.rdimm_allowed = { DDR3_SPD_RDIMM, DDR3_SPD_MINI_RDIMM },
+			.expected_module_type =	DDR3_SPD_SODIMM,
+		},
+		{
+			.memory_type = MEMORY_TYPE_DDR4,
+			.udimm_allowed = { DDR4_SPD_UDIMM, DDR4_SPD_MINI_UDIMM },
+			.rdimm_allowed = { DDR4_SPD_RDIMM, DDR4_SPD_MINI_RDIMM },
+			.expected_module_type = DDR4_SPD_SODIMM,
+		},
+		{
+			.memory_type = MEMORY_TYPE_DDR5,
+			.udimm_allowed = { DDR5_SPD_UDIMM, DDR5_SPD_MINI_UDIMM },
+			.rdimm_allowed = { DDR5_SPD_RDIMM, DDR5_SPD_MINI_RDIMM },
+			.expected_module_type = DDR5_SPD_SODIMM
+		},
+	};
+
+	/* Test for DDRx DIMM Modules */
+	for (int i = 0; i < ARRAY_SIZE(ddrx_info); i++)
+		test_smbios_form_factor_to_spd_mod_type_ddrx_parametrized(
+				ddrx_info[i].memory_type, ddrx_info[i].udimm_allowed,
+				ddrx_info[i].rdimm_allowed, ddrx_info[i].expected_module_type);
+
+	smbios_memory_type lpddrx_memory_type[] = {
+		MEMORY_TYPE_LPDDR3,
+		MEMORY_TYPE_LPDDR4,
+		MEMORY_TYPE_LPDDR5,
+	};
+
+	/* Test for Lpddrx DIMM Modules */
+	for (int i = 0; i < ARRAY_SIZE(lpddrx_memory_type); i++)
+		test_smbios_form_factor_to_spd_mod_type_lpddrx(lpddrx_memory_type[i]);
+}
+
 int main(void)
 {
 	const struct CMUnitTest tests[] = {