Hello Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32607
to review the following change.
Change subject: ec/lenovo/h8: Add VBOOT board support ......................................................................
ec/lenovo/h8: Add VBOOT board support
Use Fn-Key as recovery mode switch.
Tested using Icb7b263ed86551cc53e1db7babccaca6b3ae2fe6.
Change-Id: I2c682431b3f09839db265259205104bd9ef4abfc Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/Makefile.inc A src/ec/lenovo/h8/vboot.c 3 files changed, 66 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/32607/1
diff --git a/src/ec/lenovo/h8/Kconfig b/src/ec/lenovo/h8/Kconfig index b109831..d874975 100644 --- a/src/ec/lenovo/h8/Kconfig +++ b/src/ec/lenovo/h8/Kconfig @@ -32,6 +32,14 @@ bool default n
+config H8_FN_KEY_AS_VBOOT_RECOVERY_SW + bool "Enable Fn-Key as VBOOT recovery switch" + depends on VBOOT + default n + help + If VBOOT is enabled, press Fn-Key at power on to force a recovery mode + boot instead of regular FW_MAIN_x boot. + endif
config H8_DOCK_EARLY_INIT diff --git a/src/ec/lenovo/h8/Makefile.inc b/src/ec/lenovo/h8/Makefile.inc index bccdd91..fbe2171 100644 --- a/src/ec/lenovo/h8/Makefile.inc +++ b/src/ec/lenovo/h8/Makefile.inc @@ -7,6 +7,12 @@ postcar-y += sense.c smm-y += sense.c
+ramstage-$(CONFIG_H8_FN_KEY_AS_VBOOT_RECOVERY_SW) += vboot.c +verstage-$(CONFIG_H8_FN_KEY_AS_VBOOT_RECOVERY_SW) += vboot.c +romstage-$(CONFIG_H8_FN_KEY_AS_VBOOT_RECOVERY_SW) += vboot.c +bootblock-$(CONFIG_H8_FN_KEY_AS_VBOOT_RECOVERY_SW) += vboot.c +postcar-$(CONFIG_H8_FN_KEY_AS_VBOOT_RECOVERY_SW) += vboot.c + ifneq ($(filter y,$(CONFIG_H8_BEEP_ON_DEATH) $(CONFIG_H8_FLASH_LEDS_ON_DEATH)),) romstage-y += panic.c ramstage-y += panic.c diff --git a/src/ec/lenovo/h8/vboot.c b/src/ec/lenovo/h8/vboot.c new file mode 100644 index 0000000..d7723e9 --- /dev/null +++ b/src/ec/lenovo/h8/vboot.c @@ -0,0 +1,52 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Patrick Rudolph siro@das-labor.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <ec/acpi/ec.h> +#include <bootmode.h> +#include <timer.h> +#include <delay.h> + +#include "h8.h" + +/** + * HACK: Use Fn-Key as recovery mode switch. + * Wait for sense register ready and read Fn-Key state. + */ +int get_recovery_mode_switch(void) +{ + struct stopwatch sw; + + /* Tested on Lenovo T500: + * - 700msec from AC power on + * - less than 150msec on Lenovo T520 from AC power on + */ + stopwatch_init_msecs_expire(&sw, 1000); + while (!stopwatch_expired(&sw) && !h8_get_sense_ready()) + mdelay(1); + + if (!h8_get_sense_ready()) + return 0; + + return h8_get_fn_key(); +} + +/** + * Only used if CONFIG_CHROMEOS is set. + * Always zero as there's no HW write protection. + */ +int get_write_protect_state(void) +{ + return 0; +}
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32607 )
Change subject: ec/lenovo/h8: Add VBOOT board support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32607/1/src/ec/lenovo/h8/vboot.c File src/ec/lenovo/h8/vboot.c:
https://review.coreboot.org/#/c/32607/1/src/ec/lenovo/h8/vboot.c@49 PS1, Line 49: int get_write_protect_state(void) This should something specific to the flash protection interface. It is already stubbed if you enable vboot no board support. So please use the kconfig option and remove this function
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32607 )
Change subject: ec/lenovo/h8: Add VBOOT board support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32607/1/src/ec/lenovo/h8/vboot.c File src/ec/lenovo/h8/vboot.c:
https://review.coreboot.org/#/c/32607/1/src/ec/lenovo/h8/vboot.c@49 PS1, Line 49: int get_write_protect_state(void)
This should something specific to the flash protection interface. […]
It is. It reflects the current hardware write protection state as used on chromebooks. As there's none it's always false.
Hello Alexander Couzens, Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32607
to look at the new patch set (#2).
Change subject: ec/lenovo/h8: Add VBOOT board support ......................................................................
ec/lenovo/h8: Add VBOOT board support
Use Fn-Key as recovery mode switch.
Tested using Icb7b263ed86551cc53e1db7babccaca6b3ae2fe6.
Change-Id: I2c682431b3f09839db265259205104bd9ef4abfc Signed-off-by: Patrick Rudolph siro@das-labor.org --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/Makefile.inc A src/ec/lenovo/h8/vboot.c 3 files changed, 69 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/32607/2
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32607 )
Change subject: ec/lenovo/h8: Add VBOOT board support ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/32607/1/src/ec/lenovo/h8/vboot.c File src/ec/lenovo/h8/vboot.c:
https://review.coreboot.org/#/c/32607/1/src/ec/lenovo/h8/vboot.c@49 PS1, Line 49: int get_write_protect_state(void)
It is. It reflects the current hardware write protection state as used on chromebooks. […]
Got it
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32607 )
Change subject: ec/lenovo/h8: Add VBOOT board support ......................................................................
ec/lenovo/h8: Add VBOOT board support
Use Fn-Key as recovery mode switch.
Tested using Icb7b263ed86551cc53e1db7babccaca6b3ae2fe6.
Change-Id: I2c682431b3f09839db265259205104bd9ef4abfc Signed-off-by: Patrick Rudolph siro@das-labor.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32607 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com --- M src/ec/lenovo/h8/Kconfig M src/ec/lenovo/h8/Makefile.inc A src/ec/lenovo/h8/vboot.c 3 files changed, 69 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved
diff --git a/src/ec/lenovo/h8/Kconfig b/src/ec/lenovo/h8/Kconfig index b109831..d874975 100644 --- a/src/ec/lenovo/h8/Kconfig +++ b/src/ec/lenovo/h8/Kconfig @@ -32,6 +32,14 @@ bool default n
+config H8_FN_KEY_AS_VBOOT_RECOVERY_SW + bool "Enable Fn-Key as VBOOT recovery switch" + depends on VBOOT + default n + help + If VBOOT is enabled, press Fn-Key at power on to force a recovery mode + boot instead of regular FW_MAIN_x boot. + endif
config H8_DOCK_EARLY_INIT diff --git a/src/ec/lenovo/h8/Makefile.inc b/src/ec/lenovo/h8/Makefile.inc index bccdd91..51c11be 100644 --- a/src/ec/lenovo/h8/Makefile.inc +++ b/src/ec/lenovo/h8/Makefile.inc @@ -7,6 +7,12 @@ postcar-y += sense.c smm-y += sense.c
+ramstage-$(CONFIG_VBOOT) += vboot.c +verstage-$(CONFIG_VBOOT) += vboot.c +romstage-$(CONFIG_VBOOT) += vboot.c +bootblock-$(CONFIG_VBOOT) += vboot.c +postcar-$(CONFIG_VBOOT) += vboot.c + ifneq ($(filter y,$(CONFIG_H8_BEEP_ON_DEATH) $(CONFIG_H8_FLASH_LEDS_ON_DEATH)),) romstage-y += panic.c ramstage-y += panic.c diff --git a/src/ec/lenovo/h8/vboot.c b/src/ec/lenovo/h8/vboot.c new file mode 100644 index 0000000..3b9f74a --- /dev/null +++ b/src/ec/lenovo/h8/vboot.c @@ -0,0 +1,55 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2019 Patrick Rudolph siro@das-labor.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <ec/acpi/ec.h> +#include <bootmode.h> +#include <timer.h> +#include <delay.h> + +#include "h8.h" + +/** + * HACK: Use Fn-Key as recovery mode switch. + * Wait for sense register ready and read Fn-Key state. + */ +int get_recovery_mode_switch(void) +{ + struct stopwatch sw; + + if (!CONFIG(H8_FN_KEY_AS_VBOOT_RECOVERY_SW)) + return 0; + + /* Tests showed that it takes: + * - 700msec on Lenovo T500 from AC power on + * - less than 150msec on Lenovo T520 from AC power on + */ + stopwatch_init_msecs_expire(&sw, 1000); + while (!stopwatch_expired(&sw) && !h8_get_sense_ready()) + mdelay(1); + + if (!h8_get_sense_ready()) + return 0; + + return h8_get_fn_key(); +} + +/** + * Only used if CONFIG_CHROMEOS is set. + * Always zero as the #WP pin of the flash is tied high. + */ +int get_write_protect_state(void) +{ + return 0; +}
-
Patrick Georgi (Code Review) -
Patrick Rudolph (Code Review)
-
Philipp Deppenwiese (Code Review)