[coreboot-gerrit] [M] Change in coreboot[master]: src/drivers/ipmi: retry ipmi_get_device_id in ipmi_kcs_init

[Johnny Lin (Code Review)]

Attention is currently required from: TangYiwei.

Hello TangYiwei,

I'd like you to do a code review.
Please visit

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

to review the following change.

Change subject: src/drivers/ipmi: retry ipmi_get_device_id in ipmi_kcs_init
......................................................................

src/drivers/ipmi: retry ipmi_get_device_id in ipmi_kcs_init

Without this retry there's a chance that ipmi_get_device_id
failed then ipmi device won't be enabled.

Change-Id: I2b972c905fb0f8223570212432a4a10bd715f3f7
Signed-off-by: Yiwei Tang 
Signed-off-by: Johnny Lin 
---
M src/drivers/ipmi/ipmi_kcs_ops.c
1 file changed, 43 insertions(+), 22 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/69310/1

diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c
index 28ced49..8fc8fa0 100644
--- a/src/drivers/ipmi/ipmi_kcs_ops.c
+++ b/src/drivers/ipmi/ipmi_kcs_ops.c
@@ -63,6 +63,7 @@
    uint32_t man_id = 0, prod_id = 0;
    struct drivers_ipmi_config *conf = dev->chip_info;
    const struct gpio_operations *gpio_ops;
+	int retries = 10;

if (!conf) {
    	printk(BIOS_WARNING, "IPMI: chip_info is missing! Skip init.\n");
@@ -116,31 +117,37 @@
    	/* Don't write tables if communication failed */
    	dev->enabled = 0;

-	if (!ipmi_get_device_id(dev, &rsp)) {
-		/* Queried the IPMI revision from BMC */
-		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));
-
-		memcpy(&prod_id, rsp.product_id, sizeof(rsp.product_id));
-
-		printk(BIOS_INFO, "IPMI: Found man_id 0x%06x, prod_id 0x%04x\n",
-		       man_id, prod_id);
-
-		printk(BIOS_INFO, "IPMI: Version %01x.%01x\n",
-		       ipmi_revision_major, ipmi_revision_minor);
-	} else {
-		/* Don't write tables if communication failed */
-		dev->enabled = 0;
+	while (ipmi_get_device_id(dev, &rsp)) {
+		retries--;
+		printk(BIOS_DEBUG, "IPMI: BMC does not respond to get device id, %d retries left\n",
+			retries);
+		if (retries == 0) {
+			dev->enabled = 0;
+			break;
+		}
+		mdelay(1000);
    }

-	if (!dev->enabled)
+	if (!dev->enabled) {
+		printk(BIOS_ERR, "IPMI: BMC does not respond to get device id even after retries.\n");
    	return;
+	}
+
+	/* Queried the IPMI revision from BMC */
+	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));
+
+	memcpy(&prod_id, rsp.product_id, sizeof(rsp.product_id));
+
+	printk(BIOS_INFO, "IPMI: Found man_id 0x%06x, prod_id 0x%04x\n", man_id, prod_id);
+
+	printk(BIOS_INFO, "IPMI: Version %01x.%01x\n", ipmi_revision_major,
+	       ipmi_revision_minor);

if (CONFIG(DRIVERS_IPMI_SUPERMICRO_OEM))
    	supermicro_ipmi_oem(dev->path.pnp.port);