[coreboot-gerrit] Change in ...coreboot[master]: mainboard/google/sarien: skip tpm check when !verstage

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32030 )

Change subject: mainboard/google/sarien: skip tpm check when !verstage
......................................................................

mainboard/google/sarien: skip tpm check when !verstage

The TPM driver isn't loaded in other stages but verstage so when we try
to communicate with the TPM it fails. We don't need to communicate with
it anyway since the TPM won't continue to tell us that recovery was
requested, only the first query responds with the recovery request.

BRANCH=none
BUG=b:129150074,b:123360379
TEST=1)boot arcada without recovery and notice that the "tpm transaction
failed" log lines are no longer present. 2) boot into recovery using the
ESC refresh power key combination and verify that the recovery reason
was "recovery button pressed"

Change-Id: I13284483d069ed50b0d16b36d0120d006485f7f4
Signed-off-by: Jett Rink 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32030
Tested-by: build bot (Jenkins) 
Reviewed-by: Keith Short 
Reviewed-by: Duncan Laurie 
---
M src/mainboard/google/sarien/chromeos.c
1 file changed, 19 insertions(+), 14 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Duncan Laurie: Looks good to me, approved
  Keith Short: Looks good to me, but someone else must approve

diff --git a/src/mainboard/google/sarien/chromeos.c b/src/mainboard/google/sarien/chromeos.c
index 1e363fd..2bef829 100644
--- a/src/mainboard/google/sarien/chromeos.c
+++ b/src/mainboard/google/sarien/chromeos.c
@@ -30,7 +30,6 @@
    REC_MODE_NOT_REQUESTED,
    REC_MODE_REQUESTED,
 };
-static enum rec_mode_state saved_rec_mode;

void fill_lb_gpios(struct lb_gpios *gpios)
 {
@@ -84,25 +83,31 @@

int get_recovery_mode_switch(void)
 {
-	enum rec_mode_state state = saved_rec_mode;
-	uint8_t recovery_button_state = 0;
+	static enum rec_mode_state saved_rec_mode = REC_MODE_UNINITIALIZED;
+	enum rec_mode_state state = REC_MODE_NOT_REQUESTED;
+	uint8_t cr50_state = 0;

-	/* Check the global variable first. */
-	if (state == REC_MODE_NOT_REQUESTED)
-		return 0;
-	else if (state == REC_MODE_REQUESTED)
-		return 1;
+	/* Check cached state, since TPM will only tell us the first time */
+	if (saved_rec_mode != REC_MODE_UNINITIALIZED)
+		return saved_rec_mode == REC_MODE_REQUESTED;

-	state = REC_MODE_NOT_REQUESTED;
+	/*
+	 * Read one-time recovery request from cr50 in verstage only since
+	 * the TPM driver won't be set up in time for other stages like romstage
+	 * and the value from the TPM would be wrong anyway since the verstage
+	 * read would have cleared the value on the TPM.
+	 *
+	 * The TPM recovery request is passed between stages through the
+	 * vboot_get_shared_data or cbmem depending on stage.
+	 */
+	if (ENV_VERSTAGE &&
+	    tlcl_cr50_get_recovery_button(&cr50_state) == TPM_SUCCESS &&
+	    cr50_state)
+		state = REC_MODE_REQUESTED;

/* Read state from the GPIO controlled by servo. */
    if (cros_get_gpio_value(CROS_GPIO_REC))
    	state = REC_MODE_REQUESTED;
-	/* Read one-time recovery request from cr50. */
-	else if (tlcl_cr50_get_recovery_button(&recovery_button_state)
-		 == TPM_SUCCESS)
-		state = recovery_button_state ?
-			REC_MODE_REQUESTED : REC_MODE_NOT_REQUESTED;

/* Store the state in case this is called again in verstage. */
    saved_rec_mode = state;