[coreboot-gerrit] Change in coreboot[master]: drivers/spi/tpm: using tpm irq to sync tpm transaction

[Furquan Shaikh (Code Review)]

Hello Julius Werner,

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

    https://review.coreboot.org/19113

to review the following change.


Change subject: drivers/spi/tpm: using tpm irq to sync tpm transaction
......................................................................

drivers/spi/tpm: using tpm irq to sync tpm transaction

BUG=b:35647967
TEST=boot from bob

Change-Id: Ib64107b17fb6e93dbe626ce92f3bc9da8b84784e
Signed-off-by: Jeffy Chen <jeffy.chen at rock-chips.com>
Reviewed-on: https://chromium-review.googlesource.com/452284
Commit-Ready: Caesar Wang <wxt at rock-chips.com>
Tested-by: Caesar Wang <wxt at rock-chips.com>
Reviewed-by: Julius Werner <jwerner at chromium.org>
---
M src/drivers/spi/tpm/tpm.c
M src/include/tpm.h
2 files changed, 49 insertions(+), 5 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/19113/1

diff --git a/src/drivers/spi/tpm/tpm.c b/src/drivers/spi/tpm/tpm.c
index 341419c..20ad8b1 100644
--- a/src/drivers/spi/tpm/tpm.c
+++ b/src/drivers/spi/tpm/tpm.c
@@ -21,6 +21,7 @@
 #include <endian.h>
 #include <string.h>
 #include <timer.h>
+#include <tpm.h>
 
 #include "tpm.h"
 
@@ -102,6 +103,39 @@
 	*info = tpm_info;
 }
 
+__attribute__((weak)) int tis_plat_irq_status(void)
+{
+	static int warning_displayed;
+
+	if (!warning_displayed) {
+		printk(BIOS_WARNING, "WARNING: tis_plat_irq_status() not implemented, wasting 10ms to wait on Cr50!\n");
+		warning_displayed = 1;
+	}
+	mdelay(10);
+
+	return 1;
+}
+
+/*
+ * TPM may trigger a irq after finish processing previous transfer.
+ * Waiting for this irq to sync tpm status.
+ *
+ * Returns 1 on success, 0 on failure (timeout).
+ */
+static int tpm_sync(void)
+{
+	struct stopwatch sw;
+
+	stopwatch_init_usecs_expire(&sw, 10 * 1000);
+	while (!tis_plat_irq_status()) {
+		if (stopwatch_expired(&sw)) {
+			printk(BIOS_ERR, "Timeout wait for tpm irq!\n");
+			return 0;
+		}
+	}
+	return 1;
+}
+
 /*
  * Each TPM2 SPI transaction starts the same: CS is asserted, the 4 byte
  * header is sent to the TPM, the master waits til TPM is ready to continue.
@@ -114,12 +148,13 @@
 	uint8_t byte;
 	int i;
 	struct stopwatch sw;
+	static int tpm_sync_needed;
 
-	/*
-	 * Give it 10 ms. TODO(vbendeb): remove this once cr50 SPS TPM driver
-	 * performance is fixed.
-	 */
-	mdelay(10);
+	/* Wait for tpm to finish previous transaction if needed */
+	if (tpm_sync_needed)
+		tpm_sync();
+	else
+		tpm_sync_needed = 1;
 
 	/* Try to wake cr50 if it is asleep. */
 	tpm_if.cs_assert(tpm_if.slave);
diff --git a/src/include/tpm.h b/src/include/tpm.h
index b15ca6e..bd85b43 100644
--- a/src/include/tpm.h
+++ b/src/include/tpm.h
@@ -64,4 +64,13 @@
 
 void init_tpm(int s3resume);
 
+/*
+ * tis_plat_irq_status()
+ *
+ * Check tpm irq and clear it.
+ *
+ * Returns 1 when irq pending or 0 when not.
+ */
+int tis_plat_irq_status(void);
+
 #endif /* TPM_H_ */

-- 
To view, visit https://review.coreboot.org/19113
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib64107b17fb6e93dbe626ce92f3bc9da8b84784e
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>