Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31043
Change subject: mb/google/sarien: Fix recovery mode detection ......................................................................
mb/google/sarien: Fix recovery mode detection
In order to support the physical recovery GPIO on sarien it needs to enable the option VBOOT_PHYSICAL_REC_SWITCH and set the GPIO number in the coreboot table appropriately so that depthcharge can correctly determine the GPIO number. The same is done for the write protect GPIO in this table.
Additionally since we are reading a recovery request from H1 it needs to cache the result since H1 will only return true on the first request. All subsequent queries to H1 will not indicate recovery. Add a CAR global here to keep track of the state and only read it from H1 the first time.
BUG=b:121380403 TEST=test_that DUT firmware_DevMode
Change-Id: Ia816a2e285d3c2c3769b25fc5d20147abbc71421 Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/mainboard/google/sarien/Kconfig M src/mainboard/google/sarien/chromeos.c 2 files changed, 32 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/31043/1
diff --git a/src/mainboard/google/sarien/Kconfig b/src/mainboard/google/sarien/Kconfig index ff2f678..f93910f 100644 --- a/src/mainboard/google/sarien/Kconfig +++ b/src/mainboard/google/sarien/Kconfig @@ -100,5 +100,6 @@ select HAS_RECOVERY_MRC_CACHE select MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN select VBOOT_LID_SWITCH + select VBOOT_PHYSICAL_REC_SWITCH
endif # BOARD_GOOGLE_BASEBOARD_SARIEN diff --git a/src/mainboard/google/sarien/chromeos.c b/src/mainboard/google/sarien/chromeos.c index 8b2090e..0ea237a 100644 --- a/src/mainboard/google/sarien/chromeos.c +++ b/src/mainboard/google/sarien/chromeos.c @@ -14,6 +14,7 @@ */
#include <arch/acpi.h> +#include <arch/early_variables.h> #include <boot/coreboot_tables.h> #include <gpio.h> #include <soc/gpio.h> @@ -21,12 +22,20 @@ #include <vendorcode/google/chromeos/chromeos.h> #include <security/tpm/tss.h>
+enum rec_mode_state { + REC_MODE_UNINITIALIZED, + REC_MODE_NOT_REQUESTED, + REC_MODE_REQUESTED, +}; +static enum rec_mode_state saved_rec_mode CAR_GLOBAL;
void fill_lb_gpios(struct lb_gpios *gpios) { struct lb_gpio chromeos_gpios[] = { - {-1, ACTIVE_HIGH, get_write_protect_state(), "write protect"}, - {-1, ACTIVE_HIGH, get_recovery_mode_switch(), "recovery"}, + {GPIO_PCH_WP, ACTIVE_HIGH, get_write_protect_state(), + "write protect"}, + {GPIO_REC_MODE, ACTIVE_LOW, get_recovery_mode_switch(), + "recovery"}, {-1, ACTIVE_HIGH, get_lid_switch(), "lid"}, {-1, ACTIVE_HIGH, 0, "power"}, {-1, ACTIVE_HIGH, gfx_get_init_done(), "oprom"}, @@ -72,16 +81,30 @@
int get_recovery_mode_switch(void) { - uint8_t recovery_button_state; - int recovery_mode_switch = 0; + enum rec_mode_state state = car_get_var(saved_rec_mode); + uint8_t recovery_button_state = 0;
+ /* Check the global variable first. */ + if (state == REC_MODE_NOT_REQUESTED) + return 0; + else if (state == REC_MODE_REQUESTED) + return 1; + + state = REC_MODE_NOT_REQUESTED; + + /* Read state from the GPIO controlled by servo. */ if (cros_get_gpio_value(CROS_GPIO_REC)) - recovery_mode_switch = 1; + state = REC_MODE_REQUESTED; + /* Read one-time recovery request from cr50. */ else if (tlcl_cr50_get_recovery_button(&recovery_button_state) - == TPM_SUCCESS) - recovery_mode_switch = recovery_button_state; + == TPM_SUCCESS) + state = recovery_button_state ? + REC_MODE_REQUESTED : REC_MODE_NOT_REQUESTED;
- return recovery_mode_switch; + /* Store the state in case this is called again in verstage. */ + car_set_var(saved_rec_mode, state); + + return state == REC_MODE_REQUESTED; }
int get_lid_switch(void)
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31043 )
Change subject: mb/google/sarien: Fix recovery mode detection ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31043 )
Change subject: mb/google/sarien: Fix recovery mode detection ......................................................................
mb/google/sarien: Fix recovery mode detection
In order to support the physical recovery GPIO on sarien it needs to enable the option VBOOT_PHYSICAL_REC_SWITCH and set the GPIO number in the coreboot table appropriately so that depthcharge can correctly determine the GPIO number. The same is done for the write protect GPIO in this table.
Additionally since we are reading a recovery request from H1 it needs to cache the result since H1 will only return true on the first request. All subsequent queries to H1 will not indicate recovery. Add a CAR global here to keep track of the state and only read it from H1 the first time.
BUG=b:121380403 TEST=test_that DUT firmware_DevMode
Change-Id: Ia816a2e285d3c2c3769b25fc5d20147abbc71421 Signed-off-by: Duncan Laurie dlaurie@google.com Reviewed-on: https://review.coreboot.org/c/31043 Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/sarien/Kconfig M src/mainboard/google/sarien/chromeos.c 2 files changed, 32 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved
diff --git a/src/mainboard/google/sarien/Kconfig b/src/mainboard/google/sarien/Kconfig index ff2f678..f93910f 100644 --- a/src/mainboard/google/sarien/Kconfig +++ b/src/mainboard/google/sarien/Kconfig @@ -100,5 +100,6 @@ select HAS_RECOVERY_MRC_CACHE select MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN select VBOOT_LID_SWITCH + select VBOOT_PHYSICAL_REC_SWITCH
endif # BOARD_GOOGLE_BASEBOARD_SARIEN diff --git a/src/mainboard/google/sarien/chromeos.c b/src/mainboard/google/sarien/chromeos.c index 8b2090e..0ea237a 100644 --- a/src/mainboard/google/sarien/chromeos.c +++ b/src/mainboard/google/sarien/chromeos.c @@ -14,6 +14,7 @@ */
#include <arch/acpi.h> +#include <arch/early_variables.h> #include <boot/coreboot_tables.h> #include <gpio.h> #include <soc/gpio.h> @@ -21,12 +22,20 @@ #include <vendorcode/google/chromeos/chromeos.h> #include <security/tpm/tss.h>
+enum rec_mode_state { + REC_MODE_UNINITIALIZED, + REC_MODE_NOT_REQUESTED, + REC_MODE_REQUESTED, +}; +static enum rec_mode_state saved_rec_mode CAR_GLOBAL;
void fill_lb_gpios(struct lb_gpios *gpios) { struct lb_gpio chromeos_gpios[] = { - {-1, ACTIVE_HIGH, get_write_protect_state(), "write protect"}, - {-1, ACTIVE_HIGH, get_recovery_mode_switch(), "recovery"}, + {GPIO_PCH_WP, ACTIVE_HIGH, get_write_protect_state(), + "write protect"}, + {GPIO_REC_MODE, ACTIVE_LOW, get_recovery_mode_switch(), + "recovery"}, {-1, ACTIVE_HIGH, get_lid_switch(), "lid"}, {-1, ACTIVE_HIGH, 0, "power"}, {-1, ACTIVE_HIGH, gfx_get_init_done(), "oprom"}, @@ -72,16 +81,30 @@
int get_recovery_mode_switch(void) { - uint8_t recovery_button_state; - int recovery_mode_switch = 0; + enum rec_mode_state state = car_get_var(saved_rec_mode); + uint8_t recovery_button_state = 0;
+ /* Check the global variable first. */ + if (state == REC_MODE_NOT_REQUESTED) + return 0; + else if (state == REC_MODE_REQUESTED) + return 1; + + state = REC_MODE_NOT_REQUESTED; + + /* Read state from the GPIO controlled by servo. */ if (cros_get_gpio_value(CROS_GPIO_REC)) - recovery_mode_switch = 1; + state = REC_MODE_REQUESTED; + /* Read one-time recovery request from cr50. */ else if (tlcl_cr50_get_recovery_button(&recovery_button_state) - == TPM_SUCCESS) - recovery_mode_switch = recovery_button_state; + == TPM_SUCCESS) + state = recovery_button_state ? + REC_MODE_REQUESTED : REC_MODE_NOT_REQUESTED;
- return recovery_mode_switch; + /* Store the state in case this is called again in verstage. */ + car_set_var(saved_rec_mode, state); + + return state == REC_MODE_REQUESTED; }
int get_lid_switch(void)
-
Duncan Laurie (Code Review) -
Patrick Georgi (Code Review)
-
Stefan Reinauer (Code Review)