[M] Change in coreboot[master]: mb/prodrive/atlas: Rework EEPROM layout structures

[Angel Pons (Code Review)]

Attention is currently required from: Christian Walter, Lean Sheng Tan.

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/73983 )

Change subject: mb/prodrive/atlas: Rework EEPROM layout structures
......................................................................

mb/prodrive/atlas: Rework EEPROM layout structures

To avoid having to calculate the length of a struct separately, rework
the code to give the struct a tag name, so that `sizeof()` can be used
instead. This involves refactoring the `get_emi_eeprom_vpd()` function
to return a struct instead of a union, so callers can no longer access
the EEPROM data as an array of bytes without additional code, but this
array view is only used inside `get_emi_eeprom_vpd()` when reading the
data from EMI.

Change-Id: Id1bc40939631baa131b5f60eadbfe42838294ebe
Signed-off-by: Angel Pons 
---
M src/mainboard/prodrive/atlas/vpd.c
M src/mainboard/prodrive/atlas/vpd.h
2 files changed, 44 insertions(+), 32 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/73983/1

diff --git a/src/mainboard/prodrive/atlas/vpd.c b/src/mainboard/prodrive/atlas/vpd.c
index 12371f7..75d1caf 100644
--- a/src/mainboard/prodrive/atlas/vpd.c
+++ b/src/mainboard/prodrive/atlas/vpd.c
@@ -8,22 +8,26 @@
 #include "mainboard.h"
 #include "vpd.h"

-const union emi_eeprom_vpd *get_emi_eeprom_vpd(void)
+const struct emi_eeprom_vpd *get_emi_eeprom_vpd(void)
 {
-	static union emi_eeprom_vpd vpd = {0};
+	static union {
+		struct emi_eeprom_vpd layout;
+		uint8_t raw[sizeof(struct emi_eeprom_vpd)];
+	} vpd = {0};

/* Check if cached VPD is valid */
-	if (vpd.header.revision == VPD_LATEST_REVISION)
-		return &vpd;
+	if (vpd.layout.header.revision == VPD_LATEST_REVISION)
+		return &vpd.layout;

ec_emi_read(vpd.raw, EMI_0_IO_BASE_ADDR, 0, 0, sizeof(vpd.raw));

/* If the magic value doesn't match, consider EEPROM VPD unreliable */
-	if (vpd.header.magic != VPD_MAGIC) {
+	if (vpd.layout.header.magic != VPD_MAGIC) {
    	printk(BIOS_WARNING, "Atlas VPD: Bad magic value, using fallback defaults\n");
-		vpd.header.revision = 0;
+		vpd.layout.header.revision = 0;
    } else {
-		printk(BIOS_DEBUG, "Atlas VPD: Got revision %u from EC\n", vpd.header.revision);
+		printk(BIOS_DEBUG, "Atlas VPD: Got revision %u from EC\n",
+				vpd.layout.header.revision);
    }

/*
@@ -40,21 +44,21 @@
     * of the VPD structure with a reasonable fallback value. Note the
     * intentional falling through.
     */
-	switch (vpd.header.revision) {
+	switch (vpd.layout.header.revision) {
    case 0:
    	memset(vpd.raw, 0, sizeof(vpd.raw));
-		vpd.header.magic = VPD_MAGIC;
-		vpd.serial_number[0] = '\0';
-		vpd.part_number[0] = '\0';
-		vpd.profile = ATLAS_PROF_UNPROGRAMMED;
+		vpd.layout.header.magic = VPD_MAGIC;
+		vpd.layout.serial_number[0] = '\0';
+		vpd.layout.part_number[0] = '\0';
+		vpd.layout.profile = ATLAS_PROF_UNPROGRAMMED;
    	__fallthrough;
    default:
    	/* Ensure serial numbers are NULL-terminated, update revision last */
-		vpd.serial_number[ATLAS_SN_PN_LENGTH - 1] = '\0';
-		vpd.part_number[ATLAS_SN_PN_LENGTH - 1] = '\0';
-		vpd.header.revision = VPD_LATEST_REVISION;
+		vpd.layout.serial_number[ATLAS_SN_PN_LENGTH - 1] = '\0';
+		vpd.layout.part_number[ATLAS_SN_PN_LENGTH - 1] = '\0';
+		vpd.layout.header.revision = VPD_LATEST_REVISION;
    	break;
    }

-	return &vpd;
+	return &vpd.layout;
 }
diff --git a/src/mainboard/prodrive/atlas/vpd.h b/src/mainboard/prodrive/atlas/vpd.h
index 3955c11..086dc8c 100644
--- a/src/mainboard/prodrive/atlas/vpd.h
+++ b/src/mainboard/prodrive/atlas/vpd.h
@@ -36,24 +36,14 @@

#define ATLAS_SN_PN_LENGTH	20

-#define EMI_EEPROM_LAYOUT_LENGTH (			\
-		sizeof(struct emi_eeprom_vpd_header) +	\
-		ATLAS_SN_PN_LENGTH +			\
-		ATLAS_SN_PN_LENGTH +			\
-		sizeof(uint16_t)			\
-	)
-
-union emi_eeprom_vpd {
-	struct __packed {
-		struct emi_eeprom_vpd_header header;
-		char serial_number[ATLAS_SN_PN_LENGTH];	/* xx-xx-xxx-xxx */
-		char part_number[ATLAS_SN_PN_LENGTH];	/* xxx-xxxx-xxxx.Rxx */
-		uint16_t profile;
-	};
-	uint8_t raw[EMI_EEPROM_LAYOUT_LENGTH];
+struct __packed emi_eeprom_vpd {
+	struct emi_eeprom_vpd_header header;
+	char serial_number[ATLAS_SN_PN_LENGTH];	/* xx-xx-xxx-xxx */
+	char part_number[ATLAS_SN_PN_LENGTH];	/* xxx-xxxx-xxxx.Rxx */
+	uint16_t profile;
 };

/* Always returns a non-NULL pointer to valid data */
-const union emi_eeprom_vpd *get_emi_eeprom_vpd(void);
+const struct emi_eeprom_vpd *get_emi_eeprom_vpd(void);

#endif /* ATLAS_VPD_H */