Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39111 )
Change subject: soc/intel/tigerlake: Add Jasper lake GPIO support
......................................................................
Patch Set 12:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/common/acpi…
File src/soc/intel/common/acpi/gpio_op.asl:
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/common/acpi…
PS12, Line 4: Copyright (C) 2020 Intel Corporation.
Please use "Copyright 2020 The coreboot project Authors." here and in all the other files in this CL.
Also if possible using SPDX license header.
Refer to the document here - https://doc.coreboot.org/releases/coreboot-4.11-relnotes.html#shorter-file-…https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/tigerlake/a…
File src/soc/intel/tigerlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/tigerlake/a…
PS12, Line 122: rg0 >= GPP_B0 && Arg0 <= GPP_A24)
> This was intentionally written in ACPI ASL2.0. […]
Actually all the ASL files in this CL seems to be written using legacy ASL. Is there a reason to not use ASL2.0?
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/tigerlake/a…
PS12, Line 90: /* GPIO Community 4 */
Here GPIO community 2 is removed for TGL. Can you please put it back?
--
To view, visit https://review.coreboot.org/c/coreboot/+/39111
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iae4e694ecb30658e43c5ed99e5436579fd7d2ed2
Gerrit-Change-Number: 39111
Gerrit-PatchSet: 12
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Justin TerAvest <teravest(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Sooraj Govindan <sooraj.govindan(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Usha P <usha.p(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 03 Mar 2020 00:25:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Joel Kitching has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38780 )
Change subject: vboot: remove VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT option
......................................................................
vboot: remove VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT option
With CL:1940398, this option is no longer needed. Recovery
requests are not cleared until kernel verification stage is
reached. If the FSP triggers any reboots, recovery requests
will be preserved. In particular:
- Manual requests will be preserved via recovery switch state,
whose behaviour is modified in CB:XXXXXX.
- Other recovery requests will remain in nvdata across reboot.
BUG=b:124141368, b:35576380
TEST=make clean && make test-abuild
BRANCH=none
Change-Id: I52d17a3c6730be5c04c3c0ae020368d11db6ca3c
Signed-off-by: Joel Kitching <kitching(a)google.com>
---
M src/security/vboot/Kconfig
M src/security/vboot/bootmode.c
M src/security/vboot/misc.h
M src/security/vboot/vbnv.c
M src/security/vboot/vbnv.h
M src/security/vboot/vboot_logic.c
M src/soc/amd/stoneyridge/Kconfig
M src/soc/intel/apollolake/Kconfig
M src/soc/intel/cannonlake/Kconfig
M src/soc/intel/icelake/Kconfig
M src/soc/intel/skylake/Kconfig
M src/soc/intel/tigerlake/Kconfig
12 files changed, 2 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/38780/1
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig
index ea70e65..54e88dd 100644
--- a/src/security/vboot/Kconfig
+++ b/src/security/vboot/Kconfig
@@ -156,14 +156,6 @@
reused by the succeeding stage. This is useful if a RAM space is too
small to fit both the verstage and the succeeding stage.
-config VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
- bool
- default n
- help
- This option ensures that the recovery request is not lost because of
- reboots caused after vboot verification is run. e.g. reboots caused by
- FSP components on Intel platforms.
-
config VBOOT_MUST_REQUEST_DISPLAY
bool
default y if VGA_ROM_RUN
diff --git a/src/security/vboot/bootmode.c b/src/security/vboot/bootmode.c
index 61b3d76..5f15790 100644
--- a/src/security/vboot/bootmode.c
+++ b/src/security/vboot/bootmode.c
@@ -50,62 +50,21 @@
return sd->recovery_reason;
}
-void vboot_save_recovery_reason_vbnv(void)
-{
- if (!CONFIG(VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT))
- return;
-
- int reason = vboot_get_recovery_reason_shared_data();
- if (!reason)
- return;
-
- set_recovery_mode_into_vbnv(reason);
-}
-
-static void vboot_clear_recovery_reason_vbnv(void *unused)
-{
- if (!CONFIG(VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT))
- return;
-
- set_recovery_mode_into_vbnv(0);
-}
-
-/*
- * Recovery reason stored in VBNV needs to be cleared before the state of VBNV
- * is backed-up anywhere or jumping to the payload (whichever occurs
- * first). Currently, vbnv_cmos.c backs up VBNV on POST_DEVICE. Thus, we need to
- * make sure that the stored recovery reason is cleared off before that
- * happens.
- * IMPORTANT: Any reboot occurring after BS_DEV_INIT state will cause loss of
- * recovery reason on reboot. Until now, we have seen reboots occurring on x86
- * only in FSP stages which run before BS_DEV_INIT.
- */
-BOOT_STATE_INIT_ENTRY(BS_DEV_INIT, BS_ON_EXIT,
- vboot_clear_recovery_reason_vbnv, NULL);
-
/*
* vb2_check_recovery_request looks up different components to identify if there
* is a recovery request and returns appropriate reason code:
* 1. Checks if recovery mode is initiated by EC. If yes, returns
* VB2_RECOVERY_RO_MANUAL.
- * 2. Checks if recovery request is present in VBNV and returns the code read
- * from it.
- * 3. Checks if vboot verification is done. If yes, return the reason code from
+ * 2. Checks if vboot verification is done. If yes, return the reason code from
* shared data.
- * 4. If nothing applies, return 0 indicating no recovery request.
+ * 3. If nothing applies, return 0 indicating no recovery request.
*/
int vboot_check_recovery_request(void)
{
- int reason = 0;
-
/* EC-initiated recovery. */
if (get_recovery_mode_switch())
return VB2_RECOVERY_RO_MANUAL;
- /* Recovery request in VBNV. */
- if ((reason = get_recovery_mode_from_vbnv()) != 0)
- return reason;
-
/* Identify if vboot verification is already complete. */
if (vboot_logic_executed())
return vboot_get_recovery_reason_shared_data();
diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h
index 2d5b084..c629ca5 100644
--- a/src/security/vboot/misc.h
+++ b/src/security/vboot/misc.h
@@ -52,11 +52,6 @@
int vboot_locate_firmware(struct vb2_context *ctx, struct region_device *fw);
/*
- * Source: security/vboot/bootmode.c
- */
-void vboot_save_recovery_reason_vbnv(void);
-
-/*
* The stage loading code is compiled and entered from multiple stages. The
* helper functions below attempt to provide more clarity on when certain
* code should be called. They are implemented inline for better compile-time
diff --git a/src/security/vboot/vbnv.c b/src/security/vboot/vbnv.c
index be598ac..a5a7806 100644
--- a/src/security/vboot/vbnv.c
+++ b/src/security/vboot/vbnv.c
@@ -101,26 +101,6 @@
vbnv_initialized = 0;
}
-/* Save a recovery reason into VBNV. */
-void set_recovery_mode_into_vbnv(int recovery_reason)
-{
- uint8_t vbnv_copy[VBOOT_VBNV_BLOCK_SIZE];
-
- read_vbnv(vbnv_copy);
-
- vbnv_copy[RECOVERY_OFFSET] = recovery_reason;
- vbnv_copy[CRC_OFFSET] = crc8_vbnv(vbnv_copy, CRC_OFFSET);
-
- save_vbnv(vbnv_copy);
-}
-
-/* Read the recovery reason from VBNV. */
-int get_recovery_mode_from_vbnv(void)
-{
- vbnv_setup();
- return vbnv[RECOVERY_OFFSET];
-}
-
/* Read the USB Device Controller(UDC) enable flag from VBNV. */
int vbnv_udc_enable_flag(void)
{
diff --git a/src/security/vboot/vbnv.h b/src/security/vboot/vbnv.h
index a2f0b4c..7d288d5 100644
--- a/src/security/vboot/vbnv.h
+++ b/src/security/vboot/vbnv.h
@@ -23,8 +23,6 @@
void save_vbnv(const uint8_t *vbnv_copy);
int verify_vbnv(uint8_t *vbnv_copy);
void regen_vbnv_crc(uint8_t *vbnv_copy);
-int get_recovery_mode_from_vbnv(void);
-void set_recovery_mode_into_vbnv(int recovery_reason);
/* Read the USB Device Controller(UDC) enable flag from VBNV. */
int vbnv_udc_enable_flag(void);
diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c
index eabf8f7..465c85b 100644
--- a/src/security/vboot/vboot_logic.c
+++ b/src/security/vboot/vboot_logic.c
@@ -429,8 +429,5 @@
vboot_is_firmware_slot_a(ctx) ? 'A' : 'B');
verstage_main_exit:
- /* Save recovery reason in case of unexpected reboots on x86. */
- vboot_save_recovery_reason_vbnv();
-
timestamp_add_now(TS_END_VBOOT);
}
diff --git a/src/soc/amd/stoneyridge/Kconfig b/src/soc/amd/stoneyridge/Kconfig
index c3fcad9..7d69a92 100644
--- a/src/soc/amd/stoneyridge/Kconfig
+++ b/src/soc/amd/stoneyridge/Kconfig
@@ -93,7 +93,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_STARTS_IN_BOOTBLOCK
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig
index 0d69da2..6c90294 100644
--- a/src/soc/intel/apollolake/Kconfig
+++ b/src/soc/intel/apollolake/Kconfig
@@ -113,7 +113,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig
index d098785..b68e93d 100644
--- a/src/soc/intel/cannonlake/Kconfig
+++ b/src/soc/intel/cannonlake/Kconfig
@@ -260,7 +260,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig
index 15e895f..210fd0d 100644
--- a/src/soc/intel/icelake/Kconfig
+++ b/src/soc/intel/icelake/Kconfig
@@ -165,7 +165,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig
index 6277cea..7382df0 100644
--- a/src/soc/intel/skylake/Kconfig
+++ b/src/soc/intel/skylake/Kconfig
@@ -93,7 +93,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig
index cef1fd0..56a0d74 100644
--- a/src/soc/intel/tigerlake/Kconfig
+++ b/src/soc/intel/tigerlake/Kconfig
@@ -189,7 +189,6 @@
config VBOOT
select VBOOT_SEPARATE_VERSTAGE
select VBOOT_MUST_REQUEST_DISPLAY
- select VBOOT_SAVE_RECOVERY_REASON_ON_REBOOT
select VBOOT_STARTS_IN_BOOTBLOCK
select VBOOT_VBNV_CMOS
select VBOOT_VBNV_CMOS_BACKUP_TO_FLASH
--
To view, visit https://review.coreboot.org/c/coreboot/+/38780
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I52d17a3c6730be5c04c3c0ae020368d11db6ca3c
Gerrit-Change-Number: 38780
Gerrit-PatchSet: 1
Gerrit-Owner: Joel Kitching <kitching(a)google.com>
Gerrit-MessageType: newchange
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38813 )
Change subject: soc/intel/gpio_defs: add a new macro for pad config
......................................................................
Patch Set 15: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/38813
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7aa4d4dee34bd46a064079c576ed64525fd489e6
Gerrit-Change-Number: 38813
Gerrit-PatchSet: 15
Gerrit-Owner: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 02 Mar 2020 23:28:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment