Attention is currently required from: Bao Zheng, Felix Held, Julius Werner, Marshall Dawson, Zheng Bao, ritul guru.
Maximilian Brune has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/84533?usp=email )
Change subject: amdfwtool: Move ISH before PSP L2
......................................................................
Patch Set 12:
(1 comment)
File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/84533/comment/26add1c1_f04cdad7?us… :
PS12, Line 972: if (!cb_config->combo_new_rab || ctx->combo_index == 0) {
: pspdir = new_psp_dir(ctx, cb_config->multi_level, cookie);
: ctx->pspdir = pspdir;
: if (recovery_ab)
: ctx->pspdir_bak = new_psp_dir(ctx, cb_config->multi_level, cookie);
: }
: /* The ISH tables are with PSP L1. */
: if (cb_config->need_ish && ctx->ish_a_dir == NULL) /* Need ISH */
: ctx->ish_a_dir = new_ish_dir(ctx);
: if (cb_config->need_ish && ctx->ish_b_dir == NULL) /* Need ISH */
: ctx->ish_b_dir = new_ish_dir(ctx);
What happens if `combo_new_rab = 0` and `recovery_ab = 1` ? In that case no PSP L1 directory is created?
That raises a bigger question for me:
What is the difference between `combo_new_rab` and `recovery_ab`? Can you point me to a place in the spec that talks about it?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84533?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Id69268619893d78d9b5330052a4fd5b501263f75
Gerrit-Change-Number: 84533
Gerrit-PatchSet: 12
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com>
Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Zheng Bao
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 11 Feb 2025 15:44:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Karthik Ramasubramanian, Ronak Kanabar.
Hello Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Karthik Ramasubramanian, Ronak Kanabar, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/86226?usp=email
to look at the new patch set (#10).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: drivers/intel/fsp2_0: Add platform callback for critical shutdown
......................................................................
drivers/intel/fsp2_0: Add platform callback for critical shutdown
This commit adds the `platform_is_low_battery_shutdown_needed` callback
to the FSP API. This allows platforms to integrate low-battery handling
logic directly into the FSP silicon initialization process. By checking
for critical conditions (e.g., low battery) within this callback after
FSP silicon initialization, the platform can initiate a controlled
shutdown before proceeding with further boot stages, preventing abrupt
shutdowns later in the boot process.
BUG=b:339673254
TEST=Able to build and boot google/brox.
Change-Id: I2d6677d70dea3d24f5a19d70608fd21229a271a0
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/drivers/intel/fsp2_0/include/fsp/api.h
M src/drivers/intel/fsp2_0/silicon_init.c
M src/lib/bmp_logo.c
3 files changed, 25 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/86226/10
--
To view, visit https://review.coreboot.org/c/coreboot/+/86226?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I2d6677d70dea3d24f5a19d70608fd21229a271a0
Gerrit-Change-Number: 86226
Gerrit-PatchSet: 10
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Jérémy Compostella, Karthik Ramasubramanian, Ronak Kanabar.
Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86225?usp=email )
Change subject: drivers/intel/fsp2_0: Add low battery indicator screen
......................................................................
Patch Set 11:
(1 comment)
File src/vendorcode/google/chromeos/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/86225/comment/591847b3_2fef5657?us… :
PS8, Line 45: low_battery.bmp,CONFIG_PLATFORM_LOW_BATTERY_INDICATOR_LOGO_PATH))
> Oh, okay, so we have it there. I didn't see that. It is kinda weird though that the code that uses it is in `src/lib`, but the code adding that logo is in `src/drivers/intel/fsp2_0` (so if you used the splash screen on a non-Intel board you wouldn't actually get the logo in your CBFS).
That is true because we are eventually using FSP to render the bitmap. Also BMP_LOGO Kconfig added inside FSP driver.
>
> Not really related to this patch but it would probably make sense to move that part into `src/lib/Makefile.mk` at some point.
Done.
--
To view, visit https://review.coreboot.org/c/coreboot/+/86225?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I711c53455639b449fe85903139bbc06cdab08d09
Gerrit-Change-Number: 86225
Gerrit-PatchSet: 11
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 15:22:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Jérémy Compostella, Karthik Ramasubramanian, Ronak Kanabar, Subrata Banik.
Hello Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Karthik Ramasubramanian, Ronak Kanabar, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/86225?usp=email
to look at the new patch set (#11).
The following approvals got outdated and were removed:
Code-Review+2 by Julius Werner, Verified+1 by build bot (Jenkins)
Change subject: drivers/intel/fsp2_0: Add low battery indicator screen
......................................................................
drivers/intel/fsp2_0: Add low battery indicator screen
This commit adds low battery indicator bitmap into CBFS. This screen
is displayed when the system detects a critically low battery condition.
The screen displays a logo and can be configured with a custom path.
An option to display an early low battery indicator in text mode is also
included. This early indicator can defer the firmware update.
This feature is controlled by the PLATFORM_HAS_LOW_BATTERY_INDICATOR
Kconfig option.
BUG=b:339673254
TEST=Able to see low-battery user notification in text mode before
memory init. Verified low-battery boot event listed in the eventlog.
Change-Id: I711c53455639b449fe85903139bbc06cdab08d09
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/drivers/intel/fsp2_0/Kconfig
M src/lib/Makefile.mk
2 files changed, 42 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/86225/11
--
To view, visit https://review.coreboot.org/c/coreboot/+/86225?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I711c53455639b449fe85903139bbc06cdab08d09
Gerrit-Change-Number: 86225
Gerrit-PatchSet: 11
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Attention is currently required from: Martin L Roth.
Yu-Ping Wu has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/86366?usp=email )
Change subject: util/docker/coreboot-sdk: Add xxd
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I suppose this is the environment where Jenkins build coreboot boards. Martin, what's the process to update the docker image that Jenkins use?
--
To view, visit https://review.coreboot.org/c/coreboot/+/86366?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Icc95d25504cdccae5967ecf9276b7c1f904a14a2
Gerrit-Change-Number: 86366
Gerrit-PatchSet: 1
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 14:51:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No