Cr50: I2CS TPM: Ensure HW read fifo is empty for register reads

This change is a safegaurd to ensure that TPM register data is always
placed in the correct location in the HW read I2CS fifo. It is only
checked for 1 or 4 byte regsiter reads. Because of the way in which a
TPM command is sent and the response is read, there are multiple reads
of the STS register prior to reading the TPM fifo register. Therefore
ensuring the fifo has zero depth when 1 or 4 byte regsiter reads,
improves the robustness of the design.

Added a counter to track the number of times the fifo is adjusted and
a new console command 'i2cs disp|rst' to display the count and reset
it to 0 if desired.

Removed section in code for TPM fifo register reads intended to handle
the case where there was a mismatch between how many bytes were copied
into the fifo and the number read by the host. Since the burstcount
field in the status register always contains a valid amount of data
that can be read by the host, there should not be cases where the
host reads less data than was copied from the TPM fifo register. In
the unexpected cases where the host may not drain all of the I2CS read
fifo data during a TPM register read, the I2CS fifo depth will be
corrected the next time that it reads either the access or STS
register which happens prior to the start of any TPM transaction.

BRANCH=none
BUG=chrome-os-partner:57338,chrome-os-partner:59191
TEST=manual
Booted Reef and verfied that TPM functionality is working.

Change-Id: I065a55e64bbcc0cb3357a2bd83447a05400b8899
Signed-off-by: Scott <scollyer@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/382689
Commit-Ready: Scott Collyer <scollyer@chromium.org>
Tested-by: Scott Collyer <scollyer@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

3 files changed, 48 insertions, 21 deletions

diff --git a/chip/g/i2cs.c b/chip/g/i2cs.c
index 5c54af9d9d..38668ac932 100644
--- a/chip/g/i2cs.c
+++ b/chip/g/i2cs.c
@@ -290,7 +290,7 @@ int i2cs_register_write_complete_handler(wr_complete_handler_f wc_handler)
 	return 0;
 }
 
-size_t i2cs_get_read_fifo_buffer_depth(void)
+size_t i2cs_zero_read_fifo_buffer_depth(void)
 {
 	uint32_t hw_read_pointer;
 	size_t depth;
@@ -303,6 +303,15 @@ size_t i2cs_get_read_fifo_buffer_depth(void)
 	hw_read_pointer = GREAD(I2CS, READ_PTR) >> 3;
 	/* Determine the number of bytes buffered in the HW fifo */
 	depth = (last_read_pointer - hw_read_pointer) & REGISTER_FILE_MASK;
-
+	/*
+	 * If queue depth is not zero, force it to 0 by adjusting
+	 * last_read_pointer to where the hw read pointer is.
+	 */
+	if (depth)
+		last_read_pointer = (uint16_t)hw_read_pointer;
+	/*
+	 * Return number of bytes queued when this funciton is called so it can
+	 * be tracked or logged by caller if desired.
+	 */
 	return depth;
 }
diff --git a/chip/g/i2cs.h b/chip/g/i2cs.h
index 76e8117be0..809e4127a0 100644
--- a/chip/g/i2cs.h
+++ b/chip/g/i2cs.h
@@ -31,11 +31,14 @@ void i2cs_post_read_data(uint8_t byte_to_read);
 void i2cs_set_pinmux(void);
 
 /*
- * Determine the number of bytes currently buffered in the I2CS READ fifo. This
+ * Ensure no bytes are currently buffered in the I2CS READ fifo. This
  * value is calculated by finding the difference between read pointer that's
- * used by FW to add bytes to the HW fifo and the HW's read pointer.
+ * used by FW to add bytes to the HW fifo and the current value of the
+ * I2CS_READ_PTR register.
+ *
+ * @returns: the number of bytes buffered when the function is called
  */
-size_t i2cs_get_read_fifo_buffer_depth(void);
+size_t i2cs_zero_read_fifo_buffer_depth(void);
 
 /*
  * Write buffer of data into the I2CS HW read fifo. The function will operate a
diff --git a/common/i2cs_tpm.c b/common/i2cs_tpm.c
index 9b41f3c9ec..274e6b6ba0 100644
--- a/common/i2cs_tpm.c
+++ b/common/i2cs_tpm.c
@@ -79,6 +79,9 @@ static const struct i2c_tpm_reg_map i2c_to_tpm[] = {
 	{0xf, 0, 0xf90}, /* TPM_FW_VER */
 };
 
+/* Used to track number of times i2cs hw read fifo was adjusted */
+static uint32_t i2cs_fifo_adjust_count;
+
 static void process_read_access(uint16_t reg_size,
 				uint16_t tpm_reg, uint8_t *data)
 {
@@ -92,6 +95,15 @@ static void process_read_access(uint16_t reg_size,
 	if (reg_size == 1 || reg_size == 4) {
 		/* Always read regsize number of bytes */
 		tpm_register_get(tpm_reg, reg_value, reg_size);
+		/*
+		 * For 1 or 4 byte register reads there should not be any data
+		 * buffered in the i2cs hw read fifo. This function will check
+		 * the current fifo queue depth and if non-zero, will adjust the
+		 * fw pointer to force it to 0.
+		 */
+		if (i2cs_zero_read_fifo_buffer_depth())
+			/* Count each instance that fifo was adjusted */
+			i2cs_fifo_adjust_count++;
 		for (i = 0; i < reg_size; i++)
 			i2cs_post_read_data(reg_value[i]);
 		return;
@@ -105,22 +117,6 @@ static void process_read_access(uint16_t reg_size,
 	reg_size = tpm_get_burst_size();
 
 	/*
-	 * For TPM fifo reads, if there is already data pending in the I2CS hw
-	 * fifo, then don't read any more TPM fifo data until the I2CS hw fifo
-	 * has been fully drained.
-	 *
-	 * The Host will only read only enough data to extract the full TPM
-	 * message length. However, Cr50 will fill the I2CS hw fifo with
-	 * 'burstsize' amount of bytes. The 2nd fifo access for a given TPM
-	 * repsonse by the Host will extract the queued up data. Following
-	 * this, the Host will then read 'burstcount' amount of data for
-	 * subsequent fifo accesses until the response has been fully read.
-	 */
-	if (i2cs_get_read_fifo_buffer_depth())
-		/* Data is already in the queue, just return */
-		return;
-
-	/*
 	 * Now, this is a hack, but we are short on SRAM, so let's reuse the
 	 * receive buffer for the FIFO data sotrage. We know that the ISR has
 	 * a 64 byte buffer were it moves received data.
@@ -226,5 +222,24 @@ static void i2cs_if_register(void)
 		return;
 
 	tpm_register_interface(i2cs_tpm_enable);
+	i2cs_fifo_adjust_count = 0;
 }
 DECLARE_HOOK(HOOK_INIT, i2cs_if_register, HOOK_PRIO_LAST);
+
+static int command_i2cs(int argc, char **argv)
+{
+	ccprintf("fifo adjust count = %d\n", i2cs_fifo_adjust_count);
+	if (argc < 2)
+		return EC_SUCCESS;
+
+	if (!strcasecmp(argv[1], "rst")) {
+		i2cs_fifo_adjust_count = 0;
+		ccprintf("fifo adjust count reset to 0\n");
+	} else
+		return EC_ERROR_PARAM1;
+
+	return EC_SUCCESS;
+}
+DECLARE_SAFE_CONSOLE_COMMAND(i2cs, command_i2cs,
+			     "rst",
+			     "Display fifo adjust count");