Change in coreboot[master]: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis...

[Johnny Lin (Code Review)]

Johnny Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info

Tested on OCP Tioga Pass
Signed-off-by: Johnny Lin

Change-Id: Ib05fdb34b2b324b6eb766de15727668ce91d2844
---
M src/drivers/ipmi/ipmi_fru.c
M src/drivers/ipmi/ipmi_ops.h
2 files changed, 174 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/40522/1

diff --git a/src/drivers/ipmi/ipmi_fru.c b/src/drivers/ipmi/ipmi_fru.c
index c16b8f6..cecaa0b 100644
--- a/src/drivers/ipmi/ipmi_fru.c
+++ b/src/drivers/ipmi/ipmi_fru.c
@@ -211,40 +211,44 @@
    return ret;
 }

-static void read_fru_board_info_area(const int port, const uint8_t id,
+static enum cb_err read_fru_board_info_area(const int port, const uint8_t id,
    			uint8_t offset, struct fru_board_info *info)
 {
    uint8_t length;
    struct ipmi_read_fru_data_req req;
-	uint8_t *data_ptr;
+	uint8_t *data_ptr, *end, *custom_data_ptr;
+	int ret = CB_SUCCESS;
+
+	if (!offset)
+		return CB_ERR;

offset = offset * OFFSET_LENGTH_MULTIPLIER;
-	if (!offset)
-		return;
    req.fru_device_id = id;
    /* Read Board Info Area length first. */
    req.fru_offset = offset + 1;
    req.count = sizeof(length);
    if (ipmi_read_fru(port, &req, &length) != CB_SUCCESS || !length) {
    	printk(BIOS_ERR, "%s failed, length: %d\n", __func__, length);
-		return;
+		return CB_ERR;
    }
    length = length * OFFSET_LENGTH_MULTIPLIER;
    data_ptr = (uint8_t *)malloc(length);
    if (!data_ptr) {
    	printk(BIOS_ERR, "malloc %d bytes for board info failed\n", length);
-		return;
+		return CB_ERR;
    }
-
+	end = data_ptr + length;
    /* Read Board Info Area data. */
    req.fru_offset = offset;
    req.count = length;
    if (ipmi_read_fru(port, &req, data_ptr) != CB_SUCCESS) {
    	printk(BIOS_ERR, "%s failed to read fru\n", __func__);
+		ret = CB_ERR;
    	goto out;
    }
    if (checksum(data_ptr, length)) {
    	printk(BIOS_ERR, "Bad FRU board info checksum.\n");
+		ret = CB_ERR;
    	goto out;
    }
    printk(BIOS_DEBUG, "Read board manufacturer string\n");
@@ -263,45 +267,86 @@
    data_ptr += length + 1;
    length = read_data_string(data_ptr, &info->part_number);

+	printk(BIOS_DEBUG, "Read board FRU file ID string.\n");
+	data_ptr += length + 1;
+	length = read_data_string(data_ptr, &info->fru_file_id);
+
+	/* Check how many valid custom fields first. */
+	info->custom_count = 0;
+	custom_data_ptr = data_ptr;
+	while ((data_ptr < end) && ((data_ptr[0] != FRU_END_OF_FIELDS))) {
+		length = NUM_DATA_BYTES(data_ptr[0]);
+		if (length > 0)
+			info->custom_count++;
+		data_ptr += length + 1;
+	}
+	if (!info->custom_count)
+		goto out;
+
+	info->board_custom = malloc(info->custom_count * sizeof(char *));
+	if (!info->board_custom) {
+		printk(BIOS_ERR, "%s failed to malloc %ld bytes for "
+			"board custom data array.\n", __func__,
+			info->custom_count * sizeof(char *));
+		ret = CB_ERR;
+		goto out;
+	}
+
+	/* Start reading custom board data. */
+	data_ptr = custom_data_ptr;
+	int count = 0;
+	while ((data_ptr < end) && ((data_ptr[0] != FRU_END_OF_FIELDS))) {
+		length = NUM_DATA_BYTES(data_ptr[0]);
+		if (length > 0) {
+			length = read_data_string(data_ptr, info->board_custom + count);
+			count++;
+		}
+		data_ptr += length + 1;
+	}
+
 out:
    free(data_ptr);
+	return ret;
 }

-static void read_fru_product_info_area(const int port, const uint8_t id,
+static enum cb_err read_fru_product_info_area(const int port, const uint8_t id,
    			uint8_t offset, struct fru_product_info *info)
 {
    uint8_t length;
    struct ipmi_read_fru_data_req req;
-	uint8_t *data_ptr;
+	uint8_t *data_ptr, *end, *custom_data_ptr;
+	int ret = CB_SUCCESS;
+
+	if (!offset)
+		return CB_ERR;

offset = offset * OFFSET_LENGTH_MULTIPLIER;
-	if (!offset)
-		return;
-
    req.fru_device_id = id;
    /* Read Product Info Area length first. */
    req.fru_offset = offset + 1;
    req.count = sizeof(length);
    if (ipmi_read_fru(port, &req, &length) != CB_SUCCESS || !length) {
    	printk(BIOS_ERR, "%s failed, length: %d\n", __func__, length);
-		return;
+		return CB_ERR;
    }
    length = length * OFFSET_LENGTH_MULTIPLIER;
    data_ptr = (uint8_t *)malloc(length);
    if (!data_ptr) {
    	printk(BIOS_ERR, "malloc %d bytes for product info failed\n", length);
-		return;
+		return CB_ERR;
    }
-
+	end = data_ptr + length;
    /* Read Product Info Area data. */
    req.fru_offset = offset;
    req.count = length;
    if (ipmi_read_fru(port, &req, data_ptr) != CB_SUCCESS) {
    	printk(BIOS_ERR, "%s failed to read fru\n", __func__);
+		ret = CB_ERR;
    	goto out;
    }
    if (checksum(data_ptr, length)) {
    	printk(BIOS_ERR, "Bad FRU product info checksum.\n");
+		ret = CB_ERR;
    	goto out;
    }
    printk(BIOS_DEBUG, "Read product manufacturer string.\n");
@@ -328,8 +373,46 @@
    printk(BIOS_DEBUG, "Read asset tag string.\n");
    length = read_data_string(data_ptr, &info->asset_tag);

+	printk(BIOS_DEBUG, "Read product FRU file ID string.\n");
+	data_ptr += length + 1;
+	length = read_data_string(data_ptr, &info->fru_file_id);
+
+	/* Check how many valid custom fields first. */
+	info->custom_count = 0;
+	custom_data_ptr = data_ptr;
+	while ((data_ptr < end) && ((data_ptr[0] != FRU_END_OF_FIELDS))) {
+		length = NUM_DATA_BYTES(data_ptr[0]);
+		if (length > 0)
+			info->custom_count++;
+		data_ptr += length + 1;
+	}
+	if (!info->custom_count)
+		goto out;
+
+	info->product_custom = malloc(info->custom_count * sizeof(char *));
+	if (!info->product_custom) {
+		printk(BIOS_ERR, "%s failed to malloc %ld bytes for "
+			"product custom data array.\n", __func__,
+			info->custom_count * sizeof(char *));
+		ret = CB_ERR;
+		goto out;
+	}
+
+	/* Start reading custom board data. */
+	data_ptr = custom_data_ptr;
+	int count = 0;
+	while ((data_ptr < end) && ((data_ptr[0] != FRU_END_OF_FIELDS))) {
+		length = NUM_DATA_BYTES(data_ptr[0]);
+		if (length > 0) {
+			length = read_data_string(data_ptr, info->product_custom + count);
+			count++;
+		}
+		data_ptr += length + 1;
+	}
+
 out:
    free(data_ptr);
+	return ret;
 }

void read_fru_areas(const int port, const uint8_t id, uint16_t offset,
@@ -420,3 +503,69 @@
    	break;
    }
 }
+
+void print_fru_areas(struct fru_info_str *fru_info_str)
+{
+	int count = 0;
+	if (fru_info_str == NULL) {
+		printk(BIOS_ERR, "FRU data is null pointer\n");
+		return;
+	}
+	struct fru_product_info prod_info = fru_info_str->prod_info;
+	struct fru_board_info board_info = fru_info_str->board_info;
+	struct fru_chassis_info chassis_info = fru_info_str->chassis_info;
+
+	printk(BIOS_DEBUG, "Printing Product Info Area...\n");
+	if (prod_info.manufacturer != NULL)
+		printk(BIOS_DEBUG, "manufacturer: %s\n", prod_info.manufacturer);
+	if (prod_info.product_name != NULL)
+		printk(BIOS_DEBUG, "product name: %s\n", prod_info.product_name);
+	if (prod_info.product_partnumber != NULL)
+		printk(BIOS_DEBUG, "product part numer: %s\n", prod_info.product_partnumber);
+	if (prod_info.product_version != NULL)
+		printk(BIOS_DEBUG, "product version: %s\n", prod_info.product_version);
+	if (prod_info.serial_number != NULL)
+		printk(BIOS_DEBUG, "serial number: %s\n", prod_info.serial_number);
+	if (prod_info.asset_tag != NULL)
+		printk(BIOS_DEBUG, "asset tag: %s\n", prod_info.asset_tag);
+	if (prod_info.fru_file_id != NULL)
+		printk(BIOS_DEBUG, "FRU file ID: %s\n", prod_info.fru_file_id);
+
+	for (count = 0; count < prod_info.custom_count; count++) {
+		if (*(prod_info.product_custom + count) != NULL)
+			printk(BIOS_DEBUG, "product custom data %i: %s\n", count,
+				*(prod_info.product_custom + count));
+	}
+
+	printk(BIOS_DEBUG, "Printing Board Info Area...\n");
+	if (board_info.manufacturer != NULL)
+		printk(BIOS_DEBUG, "manufacturer: %s\n", board_info.manufacturer);
+	if (board_info.product_name != NULL)
+		printk(BIOS_DEBUG, "product name: %s\n", board_info.product_name);
+	if (board_info.serial_number != NULL)
+		printk(BIOS_DEBUG, "serial number: %s\n", board_info.serial_number);
+	if (board_info.part_number != NULL)
+		printk(BIOS_DEBUG, "part number: %s\n", board_info.part_number);
+	if (board_info.fru_file_id != NULL)
+		printk(BIOS_DEBUG, "FRU file ID: %s\n", board_info.fru_file_id);
+
+	for (count = 0; count < board_info.custom_count; count++) {
+		if (*(board_info.board_custom + count) != NULL)
+			printk(BIOS_DEBUG, "board custom data %i: %s\n", count,
+				*(board_info.board_custom + count));
+	}
+
+	printk(BIOS_DEBUG, "Printing Chassis Info Area...\n");
+	printk(BIOS_DEBUG, "chassis type: %d\n", chassis_info.chassis_type);
+	if (chassis_info.chassis_partnumber != NULL)
+		printk(BIOS_DEBUG, "part number: %s\n", chassis_info.chassis_partnumber);
+	if (chassis_info.serial_number != NULL)
+		printk(BIOS_DEBUG, "serial number: %s\n", chassis_info.serial_number);
+
+	for (count = 0; count < chassis_info.custom_count; count++) {
+		if (*(chassis_info.chassis_custom + count) != NULL)
+			printk(BIOS_DEBUG, "chassis custom data %i: %s\n", count,
+				*(chassis_info.chassis_custom + count));
+	}
+
+}
diff --git a/src/drivers/ipmi/ipmi_ops.h b/src/drivers/ipmi/ipmi_ops.h
index a6d9e59..dbc0c79 100644
--- a/src/drivers/ipmi/ipmi_ops.h
+++ b/src/drivers/ipmi/ipmi_ops.h
@@ -110,6 +110,9 @@
    char *product_version;
    char *serial_number;
    char *asset_tag;
+	char *fru_file_id;
+	char **product_custom;
+	int custom_count; /* Number of custom fields */
 };

struct fru_board_info {
@@ -117,6 +120,9 @@
    char *product_name;
    char *serial_number;
    char *part_number;
+	char *fru_file_id;
+	char **board_custom;
+	int custom_count;
 };

struct fru_chassis_info {
@@ -124,7 +130,7 @@
    char *chassis_partnumber;
    char *serial_number;
    char **chassis_custom;
-	int custom_count; /* Number of custom fields */
+	int custom_count;
 };

struct fru_info_str {
@@ -173,4 +179,7 @@
 /* Add a SEL record entry, returns CB_SUCCESS on success and CB_ERR
  * if an error occurred */
 enum cb_err ipmi_add_sel(const int port, struct sel_event_record *sel);
+
+/* Print all IPMI read FRU data */
+void print_fru_areas(struct fru_info_str *fru_info_str);
 #endif

[Johnny Lin (Code Review)]

Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, David Hendricks, Jingle Hsu, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40522

to look at the new patch set (#2).

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info

Tested on OCP Tioga Pass
Signed-off-by: Johnny Lin

Change-Id: Ib05fdb34b2b324b6eb766de15727668ce91d2844
---
M src/drivers/ipmi/ipmi_fru.c
M src/drivers/ipmi/ipmi_ops.h
2 files changed, 174 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/40522/2

[Johnny Lin (Code Review)]

Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, David Hendricks, Jingle Hsu, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40522

to look at the new patch set (#3).

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info

Tested on OCP Tioga Pass
Signed-off-by: Johnny Lin

Change-Id: Ib05fdb34b2b324b6eb766de15727668ce91d2844
---
M src/drivers/ipmi/ipmi_fru.c
M src/drivers/ipmi/ipmi_ops.h
2 files changed, 174 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/40522/3

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40522/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40522/3//COMMIT_MSG@11 
PS3, Line 11: 
Please remove the blank line.

[Johnny Lin (Code Review)]

Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, David Hendricks, Jingle Hsu, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40522

to look at the new patch set (#5).

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info

Tested on OCP Tioga Pass

Signed-off-by: Johnny Lin 
Change-Id: Ib05fdb34b2b324b6eb766de15727668ce91d2844
---
M src/drivers/ipmi/ipmi_fru.c
M src/drivers/ipmi/ipmi_ops.h
2 files changed, 174 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/40522/5

[Johnny Lin (Code Review)]

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40522/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40522/3//COMMIT_MSG@11 
PS3, Line 11:
> Please remove the blank line.

Done

[Jonathan Zhang (Code Review)]

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

Patch Set 5:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_fru.c 
File src/drivers/ipmi/ipmi_fru.c:

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_fru.c... 
PS5, Line 516: 	struct fru_chassis_info chassis_info = fru_info_str->chassis_info;
Need to check whether they are NULL pointers or not as well.

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_ops.h 
File src/drivers/ipmi/ipmi_ops.h:

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_ops.h... 
PS5, Line 115: 	int custom_count; /* Number of custom fields */
Is it possible that these two fields are flipped around?

[Andrey Petrov (Code Review)]

Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

Patch Set 5: Code-Review+2

[Johnny Lin (Code Review)]

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

Patch Set 5:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_fru.c 
File src/drivers/ipmi/ipmi_fru.c:

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_fru.c... 
PS5, Line 516: 	struct fru_chassis_info chassis_info = fru_info_str->chassis_info;
> Need to check whether they are NULL pointers or not as well.

prod_info, board_info and chassis_info are structs not pointers.

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_ops.h 
File src/drivers/ipmi/ipmi_ops.h:

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_ops.h... 
PS5, Line 115: 	int custom_count; /* Number of custom fields */
> Is it possible that these two fields are flipped around?

Do you mean the order of the members in the struct? I think the order and the memory space should be preserved.

[Johnny Lin (Code Review)]

Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, David Hendricks, Jingle Hsu, Morgan Jang, Andrey Petrov, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40522

to look at the new patch set (#6).

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info

Tested on OCP Tioga Pass

Signed-off-by: Johnny Lin 
Change-Id: Ib05fdb34b2b324b6eb766de15727668ce91d2844
---
M src/drivers/ipmi/ipmi_fru.c
M src/drivers/ipmi/ipmi_ops.h
2 files changed, 174 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/40522/6

[Johnny Lin (Code Review)]

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read all FRU data fields for Product, Board and Chassis Info
......................................................................

Patch Set 6:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_fru.c 
File src/drivers/ipmi/ipmi_fru.c:

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_fru.c... 
PS5, Line 516: 	struct fru_chassis_info chassis_info = fru_info_str->chassis_info;
> prod_info, board_info and chassis_info are structs not pointers.

Done

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_ops.h 
File src/drivers/ipmi/ipmi_ops.h:

https://review.coreboot.org/c/coreboot/+/40522/5/src/drivers/ipmi/ipmi_ops.h... 
PS5, Line 115: 	int custom_count; /* Number of custom fields */
> Do you mean the order of the members in the struct? I think the order and the memory space should be […]

Done

Change in coreboot[master]: drivers/ipmi: Read more FRU data fields for Product and Board Info

[Johnny Lin (Code Review)]

Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, David Hendricks, Jingle Hsu, Morgan Jang, Andrey Petrov, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40522

to look at the new patch set (#10).

Change subject: drivers/ipmi: Read more FRU data fields for Product and Board Info
......................................................................

drivers/ipmi: Read more FRU data fields for Product and Board Info

Tested on OCP Tioga Pass

Signed-off-by: Johnny Lin 
Change-Id: Ib05fdb34b2b324b6eb766de15727668ce91d2844
---
M src/drivers/ipmi/ipmi_fru.c
M src/drivers/ipmi/ipmi_ops.h
2 files changed, 176 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/40522/10

Change in coreboot[master]: drivers/ipmi: Read more FRU data fields for Product and Board Info

[Jonathan Zhang (Code Review)]

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read more FRU data fields for Product and Board Info
......................................................................

Patch Set 10: Code-Review+1

Change in coreboot[master]: drivers/ipmi: Read more FRU data fields for Product and Board Info

[Johnny Lin (Code Review)]

Hello build bot (Jenkins), Andrey Petrov, Jonathan Zhang, David Hendricks, Jingle Hsu, Morgan Jang, Andrey Petrov, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/40522

to look at the new patch set (#17).

Change subject: drivers/ipmi: Read more FRU data fields for Product and Board Info
......................................................................

drivers/ipmi: Read more FRU data fields for Product and Board Info

Tested on OCP Tioga Pass

Signed-off-by: Johnny Lin 
Change-Id: Ib05fdb34b2b324b6eb766de15727668ce91d2844
---
M src/drivers/ipmi/ipmi_fru.c
M src/drivers/ipmi/ipmi_ops.h
2 files changed, 176 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/40522/17

Change in coreboot[master]: drivers/ipmi: Read more FRU data fields for Product and Board Info

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read more FRU data fields for Product and Board Info
......................................................................

Patch Set 21: Code-Review+2

Change in coreboot[master]: drivers/ipmi: Read more FRU data fields for Product and Board Info

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40522 )

Change subject: drivers/ipmi: Read more FRU data fields for Product and Board Info
......................................................................

drivers/ipmi: Read more FRU data fields for Product and Board Info

Tested on OCP Tioga Pass

Signed-off-by: Johnny Lin 
Change-Id: Ib05fdb34b2b324b6eb766de15727668ce91d2844
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40522
Reviewed-by: Patrick Rudolph 
Reviewed-by: Jonathan Zhang 
Tested-by: build bot (Jenkins) 
---
M src/drivers/ipmi/ipmi_fru.c
M src/drivers/ipmi/ipmi_ops.h
2 files changed, 176 insertions(+), 16 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Rudolph: Looks good to me, approved
  Jonathan Zhang: Looks good to me, but someone else must approve

diff --git a/src/drivers/ipmi/ipmi_fru.c b/src/drivers/ipmi/ipmi_fru.c
index 9cfe893..976df1c 100644
--- a/src/drivers/ipmi/ipmi_fru.c
+++ b/src/drivers/ipmi/ipmi_fru.c
@@ -213,40 +213,44 @@
    return ret;
 }

-static void read_fru_board_info_area(const int port, const uint8_t id,
+static enum cb_err read_fru_board_info_area(const int port, const uint8_t id,
    			uint8_t offset, struct fru_board_info *info)
 {
    uint8_t length;
    struct ipmi_read_fru_data_req req;
-	uint8_t *data_ptr;
+	uint8_t *data_ptr, *end, *custom_data_ptr;
+	int ret = CB_SUCCESS;
+
+	if (!offset)
+		return CB_ERR;

offset = offset * OFFSET_LENGTH_MULTIPLIER;
-	if (!offset)
-		return;
    req.fru_device_id = id;
    /* Read Board Info Area length first. */
    req.fru_offset = offset + 1;
    req.count = sizeof(length);
    if (ipmi_read_fru(port, &req, &length) != CB_SUCCESS || !length) {
    	printk(BIOS_ERR, "%s failed, length: %d\n", __func__, length);
-		return;
+		return CB_ERR;
    }
    length = length * OFFSET_LENGTH_MULTIPLIER;
    data_ptr = (uint8_t *)malloc(length);
    if (!data_ptr) {
    	printk(BIOS_ERR, "malloc %d bytes for board info failed\n", length);
-		return;
+		return CB_ERR;
    }
-
+	end = data_ptr + length;
    /* Read Board Info Area data. */
    req.fru_offset = offset;
    req.count = length;
    if (ipmi_read_fru(port, &req, data_ptr) != CB_SUCCESS) {
    	printk(BIOS_ERR, "%s failed to read fru\n", __func__);
+		ret = CB_ERR;
    	goto out;
    }
    if (checksum(data_ptr, length)) {
    	printk(BIOS_ERR, "Bad FRU board info checksum.\n");
+		ret = CB_ERR;
    	goto out;
    }
    printk(BIOS_DEBUG, "Read board manufacturer string\n");
@@ -265,45 +269,87 @@
    data_ptr += length + 1;
    length = read_data_string(data_ptr, &info->part_number);

+	printk(BIOS_DEBUG, "Read board FRU file ID string.\n");
+	data_ptr += length + 1;
+	length = read_data_string(data_ptr, &info->fru_file_id);
+
+	/* Check how many valid custom fields first. */
+	data_ptr += length + 1;
+	info->custom_count = 0;
+	custom_data_ptr = data_ptr;
+	while ((data_ptr < end) && ((data_ptr[0] != FRU_END_OF_FIELDS))) {
+		length = NUM_DATA_BYTES(data_ptr[0]);
+		if (length > 0)
+			info->custom_count++;
+		data_ptr += length + 1;
+	}
+	if (!info->custom_count)
+		goto out;
+
+	info->board_custom = malloc(info->custom_count * sizeof(char *));
+	if (!info->board_custom) {
+		printk(BIOS_ERR, "%s failed to malloc %ld bytes for "
+			"board custom data array.\n", __func__,
+			info->custom_count * sizeof(char *));
+		ret = CB_ERR;
+		goto out;
+	}
+
+	/* Start reading custom board data. */
+	data_ptr = custom_data_ptr;
+	int count = 0;
+	while ((data_ptr < end) && ((data_ptr[0] != FRU_END_OF_FIELDS))) {
+		length = NUM_DATA_BYTES(data_ptr[0]);
+		if (length > 0) {
+			length = read_data_string(data_ptr, info->board_custom + count);
+			count++;
+		}
+		data_ptr += length + 1;
+	}
+
 out:
    free(data_ptr);
+	return ret;
 }

-static void read_fru_product_info_area(const int port, const uint8_t id,
+static enum cb_err read_fru_product_info_area(const int port, const uint8_t id,
    			uint8_t offset, struct fru_product_info *info)
 {
    uint8_t length;
    struct ipmi_read_fru_data_req req;
-	uint8_t *data_ptr;
+	uint8_t *data_ptr, *end, *custom_data_ptr;
+	int ret = CB_SUCCESS;
+
+	if (!offset)
+		return CB_ERR;

offset = offset * OFFSET_LENGTH_MULTIPLIER;
-	if (!offset)
-		return;
-
    req.fru_device_id = id;
    /* Read Product Info Area length first. */
    req.fru_offset = offset + 1;
    req.count = sizeof(length);
    if (ipmi_read_fru(port, &req, &length) != CB_SUCCESS || !length) {
    	printk(BIOS_ERR, "%s failed, length: %d\n", __func__, length);
-		return;
+		return CB_ERR;
    }
    length = length * OFFSET_LENGTH_MULTIPLIER;
    data_ptr = (uint8_t *)malloc(length);
    if (!data_ptr) {
    	printk(BIOS_ERR, "malloc %d bytes for product info failed\n", length);
-		return;
+		return CB_ERR;
    }
-
+	end = data_ptr + length;
    /* Read Product Info Area data. */
    req.fru_offset = offset;
    req.count = length;
    if (ipmi_read_fru(port, &req, data_ptr) != CB_SUCCESS) {
    	printk(BIOS_ERR, "%s failed to read fru\n", __func__);
+		ret = CB_ERR;
    	goto out;
    }
    if (checksum(data_ptr, length)) {
    	printk(BIOS_ERR, "Bad FRU product info checksum.\n");
+		ret = CB_ERR;
    	goto out;
    }
    printk(BIOS_DEBUG, "Read product manufacturer string.\n");
@@ -330,8 +376,47 @@
    printk(BIOS_DEBUG, "Read asset tag string.\n");
    length = read_data_string(data_ptr, &info->asset_tag);

+	printk(BIOS_DEBUG, "Read product FRU file ID string.\n");
+	data_ptr += length + 1;
+	length = read_data_string(data_ptr, &info->fru_file_id);
+
+	/* Check how many valid custom fields first. */
+	data_ptr += length + 1;
+	info->custom_count = 0;
+	custom_data_ptr = data_ptr;
+	while ((data_ptr < end) && ((data_ptr[0] != FRU_END_OF_FIELDS))) {
+		length = NUM_DATA_BYTES(data_ptr[0]);
+		if (length > 0)
+			info->custom_count++;
+		data_ptr += length + 1;
+	}
+	if (!info->custom_count)
+		goto out;
+
+	info->product_custom = malloc(info->custom_count * sizeof(char *));
+	if (!info->product_custom) {
+		printk(BIOS_ERR, "%s failed to malloc %ld bytes for "
+			"product custom data array.\n", __func__,
+			info->custom_count * sizeof(char *));
+		ret = CB_ERR;
+		goto out;
+	}
+
+	/* Start reading custom product data. */
+	data_ptr = custom_data_ptr;
+	int count = 0;
+	while ((data_ptr < end) && ((data_ptr[0] != FRU_END_OF_FIELDS))) {
+		length = NUM_DATA_BYTES(data_ptr[0]);
+		if (length > 0) {
+			length = read_data_string(data_ptr, info->product_custom + count);
+			count++;
+		}
+		data_ptr += length + 1;
+	}
+
 out:
    free(data_ptr);
+	return ret;
 }

void read_fru_areas(const int port, const uint8_t id, uint16_t offset,
@@ -422,3 +507,69 @@
    	break;
    }
 }
+
+void print_fru_areas(struct fru_info_str *fru_info_str)
+{
+	int count = 0;
+	if (fru_info_str == NULL) {
+		printk(BIOS_ERR, "FRU data is null pointer\n");
+		return;
+	}
+	struct fru_product_info prod_info = fru_info_str->prod_info;
+	struct fru_board_info board_info = fru_info_str->board_info;
+	struct fru_chassis_info chassis_info = fru_info_str->chassis_info;
+
+	printk(BIOS_DEBUG, "Printing Product Info Area...\n");
+	if (prod_info.manufacturer != NULL)
+		printk(BIOS_DEBUG, "manufacturer: %s\n", prod_info.manufacturer);
+	if (prod_info.product_name != NULL)
+		printk(BIOS_DEBUG, "product name: %s\n", prod_info.product_name);
+	if (prod_info.product_partnumber != NULL)
+		printk(BIOS_DEBUG, "product part numer: %s\n", prod_info.product_partnumber);
+	if (prod_info.product_version != NULL)
+		printk(BIOS_DEBUG, "product version: %s\n", prod_info.product_version);
+	if (prod_info.serial_number != NULL)
+		printk(BIOS_DEBUG, "serial number: %s\n", prod_info.serial_number);
+	if (prod_info.asset_tag != NULL)
+		printk(BIOS_DEBUG, "asset tag: %s\n", prod_info.asset_tag);
+	if (prod_info.fru_file_id != NULL)
+		printk(BIOS_DEBUG, "FRU file ID: %s\n", prod_info.fru_file_id);
+
+	for (count = 0; count < prod_info.custom_count; count++) {
+		if (*(prod_info.product_custom + count) != NULL)
+			printk(BIOS_DEBUG, "product custom data %i: %s\n", count,
+				*(prod_info.product_custom + count));
+	}
+
+	printk(BIOS_DEBUG, "Printing Board Info Area...\n");
+	if (board_info.manufacturer != NULL)
+		printk(BIOS_DEBUG, "manufacturer: %s\n", board_info.manufacturer);
+	if (board_info.product_name != NULL)
+		printk(BIOS_DEBUG, "product name: %s\n", board_info.product_name);
+	if (board_info.serial_number != NULL)
+		printk(BIOS_DEBUG, "serial number: %s\n", board_info.serial_number);
+	if (board_info.part_number != NULL)
+		printk(BIOS_DEBUG, "part number: %s\n", board_info.part_number);
+	if (board_info.fru_file_id != NULL)
+		printk(BIOS_DEBUG, "FRU file ID: %s\n", board_info.fru_file_id);
+
+	for (count = 0; count < board_info.custom_count; count++) {
+		if (*(board_info.board_custom + count) != NULL)
+			printk(BIOS_DEBUG, "board custom data %i: %s\n", count,
+				*(board_info.board_custom + count));
+	}
+
+	printk(BIOS_DEBUG, "Printing Chassis Info Area...\n");
+	printk(BIOS_DEBUG, "chassis type: 0x%x\n", chassis_info.chassis_type);
+	if (chassis_info.chassis_partnumber != NULL)
+		printk(BIOS_DEBUG, "part number: %s\n", chassis_info.chassis_partnumber);
+	if (chassis_info.serial_number != NULL)
+		printk(BIOS_DEBUG, "serial number: %s\n", chassis_info.serial_number);
+
+	for (count = 0; count < chassis_info.custom_count; count++) {
+		if (*(chassis_info.chassis_custom + count) != NULL)
+			printk(BIOS_DEBUG, "chassis custom data %i: %s\n", count,
+				*(chassis_info.chassis_custom + count));
+	}
+
+}
diff --git a/src/drivers/ipmi/ipmi_ops.h b/src/drivers/ipmi/ipmi_ops.h
index 82296a9..60983b4 100644
--- a/src/drivers/ipmi/ipmi_ops.h
+++ b/src/drivers/ipmi/ipmi_ops.h
@@ -109,6 +109,9 @@
    char *product_version;
    char *serial_number;
    char *asset_tag;
+	char *fru_file_id;
+	char **product_custom;
+	int custom_count; /* Number of custom fields */
 };

struct fru_board_info {
@@ -116,6 +119,9 @@
    char *product_name;
    char *serial_number;
    char *part_number;
+	char *fru_file_id;
+	char **board_custom;
+	int custom_count;
 };

struct fru_chassis_info {
@@ -123,7 +129,7 @@
    char *chassis_partnumber;
    char *serial_number;
    char **chassis_custom;
-	int custom_count; /* Number of custom fields */
+	int custom_count;
 };

struct fru_info_str {
@@ -172,4 +178,7 @@
 /* Add a SEL record entry, returns CB_SUCCESS on success and CB_ERR
  * if an error occurred */
 enum cb_err ipmi_add_sel(const int port, struct sel_event_record *sel);
+
+/* Print all IPMI read FRU data */
+void print_fru_areas(struct fru_info_str *fru_info_str);
 #endif