Attention is currently required from: David Wu, Eric Lai, Karthik Ramasubramanian, Nick Vaccaro, Reka Norman, Subrata Banik, Tyler Wang.
Ren Kuo has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78791?usp=email )
Change subject: mb/google/nissa: Add AUDIO_CONFIG in fw_config
......................................................................
Patch Set 2: Code-Review+1
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/78791/comment/62518154_e03fab74 :
PS1, Line 10: .S
> Add a space
fixed.
https://review.coreboot.org/c/coreboot/+/78791/comment/2a2f900c_a5a3fc6a :
PS1, Line 10: deptcharge
> depthcharge
fixed.
File src/mainboard/google/brya/variants/baseboard/nissa/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/78791/comment/97d219f4_5594e2d4 :
PS1, Line 4: AMP_I2S
> Please rename this to AMP_RT5650. […]
updated.
--
To view, visit https://review.coreboot.org/c/coreboot/+/78791?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7446fce57557204d91151f1a31755381c1813c6f
Gerrit-Change-Number: 78791
Gerrit-PatchSet: 2
Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Wed, 01 Nov 2023 01:01:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Reka Norman <rekanorman(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: David Wu, Eric Lai, Karthik Ramasubramanian, Nick Vaccaro, Ren Kuo, Subrata Banik, Tyler Wang.
Hello David Wu, Eric Lai, Karthik Ramasubramanian, Nick Vaccaro, Reka Norman, Subrata Banik, Tyler Wang, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/78791?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Ren Kuo, Verified+1 by build bot (Jenkins)
Change subject: mb/google/nissa: Add AUDIO_CONFIG in fw_config
......................................................................
mb/google/nissa: Add AUDIO_CONFIG in fw_config
The codec alc5650 has different setting from other amp codec in
depthcharge. Since nissa has a single shared depthcharge target,
add the fw_config field to allow different audio_configs.
(refer to chromium:4983866)
BUG=b:307410704
TEST=With depthcharge change, set fw_config and gbb flags on craaskana
and check beep sound on firmware screen is workable.
Change-Id: I7446fce57557204d91151f1a31755381c1813c6f
Signed-off-by: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
---
M src/mainboard/google/brya/variants/baseboard/nissa/devicetree.cb
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/78791/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/78791?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7446fce57557204d91151f1a31755381c1813c6f
Gerrit-Change-Number: 78791
Gerrit-PatchSet: 2
Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Eran Mitrani, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Nico Huber, Tarun.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77886?usp=email )
Change subject: drivers/intel/gma/opregion: Use CBFS cache to load VBT
......................................................................
Patch Set 33:
(1 comment)
File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/77886/comment/5f9b5829_f49247a7 :
PS4, Line 27: cbfs_map
> If it's like the generic heap, why have it separate?
Historical reasons, and the fact that it needs to be available in pre-RAM stages. If you're asking whether we should combine it with the ramstage heap into a single malloc() API for all stages, then... maybe. There are a few gotchas (e.g. it needs special alignment requirements on a few platforms) but it could certainly be done.
There might also be some concern that exposing a general malloc() API in pre-RAM stages could lead to it being abused too widely and cause out-of-memory edge cases. As long as it's only for files, there are less edge case combinations to test. There could also be the concern that if more code uses this, the chance that a file allocation can still clean up after itself gets lower (because the allocator is only able to free the last two allocations), so that might be a good reason to keep the heaps separate.
Anyway, probably not a question to decide on this CL.
> We eventually copy the VBT to CBMEM anyway. So there shouldn't be a need to map twice.
I'm not really familiar with this code but it looks like this is an exported function that has a bunch of callers outside this file (e.g. 10 just via `vbt_get()`). It's possible that something could be cleaned up and reuse the CBMEM allocation there, but probably not a small task.
--
To view, visit https://review.coreboot.org/c/coreboot/+/77886?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I1e37e718a71bd85b0d7dee1efc4c0391798f16f7
Gerrit-Change-Number: 77886
Gerrit-PatchSet: 33
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Wed, 01 Nov 2023 00:59:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Yu-Ping Wu.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78845?usp=email )
Change subject: Revert "vboot: Add catchall recovery reason for unspecified phase 4 errors"
......................................................................
Patch Set 2: Verified+1
(1 comment)
Patchset:
PS2:
> Well you should've pushed this revert before the other one, right now the tree is broken.
When you have two broken commits in the tree, it doesn't matter which goes in first, the tree is still going to be broken.
--
To view, visit https://review.coreboot.org/c/coreboot/+/78845?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I8ef853d81ee9b1f18d36dfd82cdf687381ece2c6
Gerrit-Change-Number: 78845
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Wed, 01 Nov 2023 00:57:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Martin L Roth, Yu-Ping Wu.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78845?usp=email )
Change subject: Revert "vboot: Add catchall recovery reason for unspecified phase 4 errors"
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
Patchset:
PS2:
Well you should've pushed this revert before the other one, right now the tree is broken.
--
To view, visit https://review.coreboot.org/c/coreboot/+/78845?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I8ef853d81ee9b1f18d36dfd82cdf687381ece2c6
Gerrit-Change-Number: 78845
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Wed, 01 Nov 2023 00:40:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Julius Werner, Martin L Roth, Yu-Ping Wu.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78845?usp=email )
Change subject: Revert "vboot: Add catchall recovery reason for unspecified phase 4 errors"
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/78845?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I8ef853d81ee9b1f18d36dfd82cdf687381ece2c6
Gerrit-Change-Number: 78845
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Wed, 01 Nov 2023 00:40:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78844?usp=email )
Change subject: Revert "Update vboot submodule to upstream main"
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Sorry, I'm a bit confused what happened here. The pre-submit Jenkins check on that CL seemed to work fine. On https://qa.coreboot.org/job/coreboot/34291/ I can see that the "Console Output" shows some lines about GOOGLE_SKYRIM.NO_VIDEO not building correctly, but not enough to figure out what exactly failed, and under "Test Results" that configuration doesn't appear. Where do I get enough information about the failure to figure out how to fix it?
--
To view, visit https://review.coreboot.org/c/coreboot/+/78844?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I726e2e1ce7dc3350a281dc30256b116580fd63c0
Gerrit-Change-Number: 78844
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 01 Nov 2023 00:35:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78844?usp=email )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: Revert "Update vboot submodule to upstream main"
......................................................................
Revert "Update vboot submodule to upstream main"
This reverts commit 6e03007bfa948d679f5d4d6998c12c581b390d1a.
Reason for revert: Build fails - Jenkins test escappe.
Signed-off-by: Martin Roth <gaumless(a)gmail.com>
Change-Id: I726e2e1ce7dc3350a281dc30256b116580fd63c0
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78844
Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Reviewed-by: David Hendricks <david.hendricks(a)gmail.com>
---
M 3rdparty/vboot
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
David Hendricks: Looks good to me, approved
Martin L Roth: Verified
Matt DeVillier: Looks good to me, approved
Felix Singer: Looks good to me, approved
diff --git a/3rdparty/vboot b/3rdparty/vboot
index f2b01bf..24cb127 160000
--- a/3rdparty/vboot
+++ b/3rdparty/vboot
@@ -1 +1 @@
-Subproject commit f2b01bf08a813fb3035102c7c948b78fa191e544
+Subproject commit 24cb127a5e83a713131ab75cc39b11336019443c
--
To view, visit https://review.coreboot.org/c/coreboot/+/78844?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I726e2e1ce7dc3350a281dc30256b116580fd63c0
Gerrit-Change-Number: 78844
Gerrit-PatchSet: 3
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Eric Lai, Julius Werner, Yu-Ping Wu.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78844?usp=email )
Change subject: Revert "Update vboot submodule to upstream main"
......................................................................
Patch Set 2: Verified+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/78844?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I726e2e1ce7dc3350a281dc30256b116580fd63c0
Gerrit-Change-Number: 78844
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Wed, 01 Nov 2023 00:35:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Eric Lai, Julius Werner, Martin L Roth, Yu-Ping Wu.
Martin L Roth has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/78844?usp=email )
Change subject: Revert "Update vboot submodule to upstream main"
......................................................................
Removed Verified-1 by build bot (Jenkins) <no-reply(a)coreboot.org>
--
To view, visit https://review.coreboot.org/c/coreboot/+/78844?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I726e2e1ce7dc3350a281dc30256b116580fd63c0
Gerrit-Change-Number: 78844
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-MessageType: deleteVote