[coreboot-gerrit] Change in ...coreboot[master]: cr50: Add probe command to poll Cr50 until DID VID is valid

[Edward Hill (Code Review)]

Hello Keith Short,

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

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

to review the following change.


Change subject: cr50: Add probe command to poll Cr50 until DID VID is valid
......................................................................

cr50: Add probe command to poll Cr50 until DID VID is valid

Added new routine cr50_i2c_probe() which ensures that communication
with the Cr50 over I2C is good prior to attempting other initialization
of the Cr50 and TPM state.  This avoids a race condition when the Cr50
is first booting that it may reset it's I2C slave interface during the
first few I2C transactions initiated from coreboot.

BUG=b:120009037
BRANCH=none
TEST=Run the Cr50 factory update against Careena board.  Confirm that
I2C reads are retried until the DID VID is valid.  Tested against debug
Cr50 firmware that forced failure of cr50_i2c_probe() and verfied that
coreboot shows recovery screen.

Change-Id: I47c59a32378ad00336277e111e81ba8d2d63e69a
Signed-off-by: Keith Short <keithshort at chromium.org>
---
M src/drivers/i2c/tpm/cr50.c
1 file changed, 38 insertions(+), 0 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/30295/1

diff --git a/src/drivers/i2c/tpm/cr50.c b/src/drivers/i2c/tpm/cr50.c
index b0d3b29..c0d8afd 100644
--- a/src/drivers/i2c/tpm/cr50.c
+++ b/src/drivers/i2c/tpm/cr50.c
@@ -456,6 +456,41 @@
 	return 0;
 }
 
+static int cr50_i2c_probe(struct tpm_chip *chip)
+{
+	int retries;
+	uint32_t vendor = 0;
+
+	/*
+	 * 150 ms should be enough to synchronize with the TPM even under the
+	 * worst nested reset request conditions. In vast majority of cases
+	 * there would be no wait at all.
+	 */
+	printk(BIOS_INFO, "Probing TPM I2C: ");
+
+	for (retries = 15; retries > 0; retries--) {
+		int rc;
+
+		rc = cr50_i2c_read(chip, TPM_DID_VID(0), (uint8_t *)&vendor, 4);
+
+		/* Exit once DID and VID verified */
+		if (!rc && (vendor == CR50_DID_VID)) {
+			printk(BIOS_INFO, " done! DID_VID %08x\n", vendor);
+			return 0;
+		}
+
+		/* TPM might be resetting, let's retry in a bit. */
+		mdelay(10);
+		printk(BIOS_INFO, ".");
+	}
+
+	/*
+	 * I2C reads were successful, but the DID and VID didn't match
+	 */
+	printk(BIOS_ERR, "Vendor ID 0x%08x not recognized\n", vendor);
+	return -1;
+}
+
 int tpm_vendor_init(struct tpm_chip *chip, unsigned int bus, uint32_t dev_addr)
 {
 	struct tpm_inf_dev *tpm_dev = car_get_var_ptr(&g_tpm_dev);
@@ -471,6 +506,9 @@
 
 	cr50_vendor_init(chip);
 
+	if (cr50_i2c_probe(chip))
+		return -1;
+
 	if (ENV_VERSTAGE || ENV_BOOTBLOCK)
 		if (process_reset(chip))
 			return -1;

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/30295
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I47c59a32378ad00336277e111e81ba8d2d63e69a
Gerrit-Change-Number: 30295
Gerrit-PatchSet: 1
Gerrit-Owner: Edward Hill <ecgh at chromium.org>
Gerrit-Reviewer: Keith Short <keithshort at chromium.org>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20181218/47d6eb88/attachment.html>