Change in coreboot[master]: src/drivers/ipmi: Add function to get BMC revision

[Tim Chu (Code Review)]

Tim Chu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46070 )

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

src/drivers/ipmi: Add function to get BMC revision

Provide a way to get BMC revision.

Tested=On OCP Delta Lake, function can get BMC revision well.

Signed-off-by: Tim Chu 
Change-Id: Iaaa4e8bf181a38452b53c83a762c7b648e95e643
---
M src/drivers/ipmi/ipmi_kcs.h
M src/drivers/ipmi/ipmi_kcs_ops.c
2 files changed, 14 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/46070/1

diff --git a/src/drivers/ipmi/ipmi_kcs.h b/src/drivers/ipmi/ipmi_kcs.h
index 501e5dd..33ddd5f 100644
--- a/src/drivers/ipmi/ipmi_kcs.h
+++ b/src/drivers/ipmi/ipmi_kcs.h
@@ -33,6 +33,8 @@
  * returns CB_SUCCESS on success and CB_ERR if an error occurred. */
 enum cb_err ipmi_kcs_premem_init(const u16 port, const u16 device);

+void ipmi_bmc_version(uint8_t *ipmi_bmc_major_revision, uint8_t *ipmi_bmc_minor_revision);
+
 struct ipmi_rsp {
    uint8_t lun;
    uint8_t cmd;
diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c
index 7fa28e3..3dec65d 100644
--- a/src/drivers/ipmi/ipmi_kcs_ops.c
+++ b/src/drivers/ipmi/ipmi_kcs_ops.c
@@ -28,6 +28,9 @@
 static u8 ipmi_revision_major = 0x1;
 static u8 ipmi_revision_minor = 0x0;

+static u8 bmc_revision_major = 0x0;
+static u8 bmc_revision_minor = 0x0;
+
 static int ipmi_get_device_id(struct device *dev, struct ipmi_devid_rsp *rsp)
 {
    int ret;
@@ -144,6 +147,9 @@
    	ipmi_revision_minor = IPMI_IPMI_VERSION_MINOR(rsp.ipmi_version);
    	ipmi_revision_major = IPMI_IPMI_VERSION_MAJOR(rsp.ipmi_version);

+		bmc_revision_major = rsp.fw_rev1;
+		bmc_revision_minor = rsp.fw_rev2;
+		
    	memcpy(&man_id, rsp.manufacturer_id,
    	       sizeof(rsp.manufacturer_id));

@@ -272,6 +278,12 @@
 }
 #endif

+void ipmi_bmc_version(uint8_t *ipmi_bmc_major_revision, uint8_t *ipmi_bmc_minor_revision)
+{
+	*ipmi_bmc_major_revision = bmc_revision_major;
+	*ipmi_bmc_minor_revision = bmc_revision_minor;
+}
+
 #if CONFIG(GENERATE_SMBIOS_TABLES)
 static int ipmi_smbios_data(struct device *dev, int *handle,
    		    unsigned long *current)

[Tim Chu (Code Review)]

Tim Chu has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/46070 )

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

src/drivers/ipmi: Add function to get BMC revision

Provide a way to get BMC revision.

Tested=On OCP Delta Lake, function can get BMC revision well.

Signed-off-by: Tim Chu 
Change-Id: Iaaa4e8bf181a38452b53c83a762c7b648e95e643
---
M src/drivers/ipmi/ipmi_kcs.h
M src/drivers/ipmi/ipmi_kcs_ops.c
2 files changed, 14 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/46070/2

[Jonathan Zhang (Code Review)]

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

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

Patch Set 2: Code-Review+1

[Patrick Rudolph (Code Review)]

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

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46070/2/src/drivers/ipmi/ipmi_kcs_o... 
File src/drivers/ipmi/ipmi_kcs_ops.c:

https://review.coreboot.org/c/coreboot/+/46070/2/src/drivers/ipmi/ipmi_kcs_o... 
PS2, Line 283: bmc_revision_major
How do you make sure bmc_revision_major contains valid data?

[Tim Chu (Code Review)]

Hello Philipp Deppenwiese, build bot (Jenkins), Ryback Hung, Jonathan Zhang, Christian Walter, Angel Pons, Bryant Ou, Patrick Rudolph,

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

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

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

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

src/drivers/ipmi: Add function to get BMC revision

Provide a way to get BMC revision.

Tested=On OCP Delta Lake, function can get BMC revision well.

Signed-off-by: Tim Chu 
Change-Id: Iaaa4e8bf181a38452b53c83a762c7b648e95e643
---
M src/drivers/ipmi/ipmi_kcs.h
M src/drivers/ipmi/ipmi_kcs_ops.c
2 files changed, 20 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/46070/3

[Tim Chu (Code Review)]

Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46070 )

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46070/2/src/drivers/ipmi/ipmi_kcs_o... 
File src/drivers/ipmi/ipmi_kcs_ops.c:

https://review.coreboot.org/c/coreboot/+/46070/2/src/drivers/ipmi/ipmi_kcs_o... 
PS2, Line 283: bmc_revision_major
> How do you make sure bmc_revision_major contains valid data?

I've revised it in the last patch. It will check bmc_revision_major and bmc_revision_minor before parsing value.

[Jonathan Zhang (Code Review)]

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

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

Patch Set 3: Code-Review+1

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46070 )

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

Patch Set 3: Code-Review+1

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46070 )

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

Patch Set 3: Code-Review+2

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46070 )

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

Patch Set 3: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: src/drivers/ipmi: Add function to get BMC revision
......................................................................

src/drivers/ipmi: Add function to get BMC revision

Provide a way to get BMC revision.

Tested=On OCP Delta Lake, function can get BMC revision well.

Signed-off-by: Tim Chu 
Change-Id: Iaaa4e8bf181a38452b53c83a762c7b648e95e643
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46070
Tested-by: build bot (Jenkins) 
Reviewed-by: Jonathan Zhang 
Reviewed-by: Angel Pons 
Reviewed-by: Arthur Heymans 
---
M src/drivers/ipmi/ipmi_kcs.h
M src/drivers/ipmi/ipmi_kcs_ops.c
2 files changed, 20 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Arthur Heymans: Looks good to me, approved
  Angel Pons: Looks good to me, approved
  Jonathan Zhang: Looks good to me, but someone else must approve

diff --git a/src/drivers/ipmi/ipmi_kcs.h b/src/drivers/ipmi/ipmi_kcs.h
index 501e5dd..33ddd5f 100644
--- a/src/drivers/ipmi/ipmi_kcs.h
+++ b/src/drivers/ipmi/ipmi_kcs.h
@@ -33,6 +33,8 @@
  * returns CB_SUCCESS on success and CB_ERR if an error occurred. */
 enum cb_err ipmi_kcs_premem_init(const u16 port, const u16 device);

+void ipmi_bmc_version(uint8_t *ipmi_bmc_major_revision, uint8_t *ipmi_bmc_minor_revision);
+
 struct ipmi_rsp {
    uint8_t lun;
    uint8_t cmd;
diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c
index dd8f34c..362f17a 100644
--- a/src/drivers/ipmi/ipmi_kcs_ops.c
+++ b/src/drivers/ipmi/ipmi_kcs_ops.c
@@ -29,6 +29,9 @@
 static u8 ipmi_revision_major = 0x1;
 static u8 ipmi_revision_minor = 0x0;

+static u8 bmc_revision_major = 0x0;
+static u8 bmc_revision_minor = 0x0;
+
 static int ipmi_get_device_id(struct device *dev, struct ipmi_devid_rsp *rsp)
 {
    int ret;
@@ -145,6 +148,9 @@
    	ipmi_revision_minor = IPMI_IPMI_VERSION_MINOR(rsp.ipmi_version);
    	ipmi_revision_major = IPMI_IPMI_VERSION_MAJOR(rsp.ipmi_version);

+		bmc_revision_major = rsp.fw_rev1;
+		bmc_revision_minor = rsp.fw_rev2;
+
    	memcpy(&man_id, rsp.manufacturer_id,
    	       sizeof(rsp.manufacturer_id));

@@ -273,6 +279,18 @@
 }
 #endif

+void ipmi_bmc_version(uint8_t *ipmi_bmc_major_revision, uint8_t *ipmi_bmc_minor_revision)
+{
+	if (!bmc_revision_major || !bmc_revision_minor) {
+		printk(BIOS_ERR, "IPMI: BMC revision missing\n");
+		*ipmi_bmc_major_revision = 0;
+		*ipmi_bmc_minor_revision = 0;
+	} else {
+		*ipmi_bmc_major_revision = bmc_revision_major;
+		*ipmi_bmc_minor_revision = bmc_revision_minor;
+	}
+}
+
 #if CONFIG(GENERATE_SMBIOS_TABLES)
 static int ipmi_smbios_data(struct device *dev, int *handle,
    		    unsigned long *current)