Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30295 )
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@chromium.org Reviewed-on: https://review.coreboot.org/c/30295 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org Reviewed-by: Martin Roth martinroth@google.com Reviewed-by: Daniel Kurtz djkurtz@google.com --- M src/drivers/i2c/tpm/cr50.c 1 file changed, 39 insertions(+), 11 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved Daniel Kurtz: Looks good to me, approved Raul Rangel: Looks good to me, approved
diff --git a/src/drivers/i2c/tpm/cr50.c b/src/drivers/i2c/tpm/cr50.c index b0d3b29..19c8f6b 100644 --- a/src/drivers/i2c/tpm/cr50.c +++ b/src/drivers/i2c/tpm/cr50.c @@ -456,10 +456,44 @@ return 0; }
+static int cr50_i2c_probe(struct tpm_chip *chip, uint32_t *did_vid) +{ + int retries; + + /* + * 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 *)did_vid, 4); + + /* Exit once DID and VID verified */ + if (!rc && (*did_vid == CR50_DID_VID)) { + printk(BIOS_INFO, "done! DID_VID 0x%08x\n", *did_vid); + return 0; + } + + /* TPM might be resetting, let's retry in a bit. */ + mdelay(10); + printk(BIOS_INFO, "."); + } + + /* + * I2C reads failed, or the DID and VID didn't match + */ + printk(BIOS_ERR, "DID_VID 0x%08x not recognized\n", *did_vid); + 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); - uint32_t vendor; + uint32_t did_vid = 0;
if (dev_addr == 0) { printk(BIOS_ERR, "%s: missing device address\n", __func__); @@ -471,6 +505,9 @@
cr50_vendor_init(chip);
+ if (cr50_i2c_probe(chip, &did_vid)) + return -1; + if (ENV_VERSTAGE || ENV_BOOTBLOCK) if (process_reset(chip)) return -1; @@ -478,17 +515,8 @@ if (claim_locality(chip)) return -1;
- /* Read four bytes from DID_VID register */ - if (cr50_i2c_read(chip, TPM_DID_VID(0), (uint8_t *)&vendor, 4) < 0) - return -1; - - if (vendor != CR50_DID_VID) { - printk(BIOS_DEBUG, "Vendor ID 0x%08x not recognized\n", vendor); - return -1; - } - printk(BIOS_DEBUG, "cr50 TPM 2.0 (i2c %u:0x%02x id 0x%x)\n", - bus, dev_addr, vendor >> 16); + bus, dev_addr, did_vid >> 16);
chip->is_open = 1; return 0;