Attention is currently required from: Eric Lai, Felix Held, Fred Reitberger, Jason Glenesk, Karthik Ramasubramanian.
Hello Eric Lai, Felix Held, Fred Reitberger, Jason Glenesk, Karthik Ramasubramanian, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/78823?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Eric Lai, Verified+1 by build bot (Jenkins)
Change subject: soc/amd/*: Ensure PSP soft fuse bitmask set properly
......................................................................
soc/amd/*: Ensure PSP soft fuse bitmask set properly
Commit e728766f4596 ("soc/amd/mendocino: Do not load MP2 Firmware when
in RO") added logic to ensure that the MP2 disable soft fuse bit was set
for the RO section, but failed to check if the bit was already set
otherwise (as it is for non-ChromeOS builds). This caused the bit to
appear twice in the PSP_RO_SOFTFUSE_BITS string, and when the string
was converted to a series of numeric values and added together, bit
(n+1) ended up being set instead of bit n.
To mitigate this, use the newly-added filter_dupes macro to ensure the
PSP_[RO_]SOFTFUSE_BITS string does not contain and duplicates before
the bistmask is calculated. Apply this to all AMD SoC makefiles where
the softfuse bits are added.
Another approach would be to create a macro to bitwise-or the fuse bit
values rather than adding them, but a workable implementation was
unable to be found, and this was the next best option.
TEST=build/boot google/skyrim (frostflow). Use a verbose build (V=1)
to verify that the correct soft fuse value is passed to amdfwtool for
RO and RW_A/B for both ChromeOS and non-ChromeOS builds.
Change-Id: I2e207e20132d44016fbcb986bdfd8e935d8fead5
Signed-off-by: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
---
M src/soc/amd/cezanne/Makefile.inc
M src/soc/amd/genoa/Makefile.inc
M src/soc/amd/glinda/Makefile.inc
M src/soc/amd/mendocino/Makefile.inc
M src/soc/amd/phoenix/Makefile.inc
M src/soc/amd/picasso/Makefile.inc
6 files changed, 7 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/78823/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/78823?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: I2e207e20132d44016fbcb986bdfd8e935d8fead5
Gerrit-Change-Number: 78823
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/78830?usp=email )
Change subject: .editorconfig: Add indent style & size of 2 spaces for shell
......................................................................
.editorconfig: Add indent style & size of 2 spaces for shell
This adds a default style for shell scripts. fmtsh now looks for this
when reformatting shell scripts.
Signed-off-by: Martin Roth <gaumless(a)gmail.com>
Change-Id: I348f23badf302a48c851231a08c1ce4be94738a4
---
M .editorconfig
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/78830/1
diff --git a/.editorconfig b/.editorconfig
index eb44fcd..f270d4c 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -9,3 +9,7 @@
insert_final_newline = true
end_of_line = lf
trim_trailing_whitespace = true
+
+[*.sh]
+indent_style = space
+indent_size = 2
--
To view, visit https://review.coreboot.org/c/coreboot/+/78830?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: I348f23badf302a48c851231a08c1ce4be94738a4
Gerrit-Change-Number: 78830
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-MessageType: newchange
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78442?usp=email )
Change subject: util/kconfig: fix 0009-util-kconfig... patch
......................................................................
util/kconfig: fix 0009-util-kconfig... patch
This was reverted via commit 9ab3a1fe4a1d and causes unapply to fail
so we adjust the patch to preserve the original return value.
Change-Id: I5ad2180854e0263d2d097b059cb16ec478b859c5
Signed-off-by: Richard Marko <srk(a)48.io>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78442
Reviewed-by: Martin L Roth <gaumless(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M util/kconfig/patches/0009-util-kconfig-Allow-emitting-false-booleans-into-kconfig-output.patch
1 file changed, 0 insertions(+), 13 deletions(-)
Approvals:
build bot (Jenkins): Verified
Martin L Roth: Looks good to me, approved
diff --git a/util/kconfig/patches/0009-util-kconfig-Allow-emitting-false-booleans-into-kconfig-output.patch b/util/kconfig/patches/0009-util-kconfig-Allow-emitting-false-booleans-into-kconfig-output.patch
index 70e9d3f..c74ef67 100644
--- a/util/kconfig/patches/0009-util-kconfig-Allow-emitting-false-booleans-into-kconfig-output.patch
+++ b/util/kconfig/patches/0009-util-kconfig-Allow-emitting-false-booleans-into-kconfig-output.patch
@@ -10,19 +10,6 @@
Change-Id: I9e62b05e45709f1539e455e2eed37308609be15e
Signed-off-by: Patrick Georgi <pgeorgi(a)google.com>
-Index: kconfig/symbol.c
-===================================================================
---- kconfig.orig/symbol.c
-+++ kconfig/symbol.c
-@@ -757,7 +757,7 @@ const char *sym_get_string_default(struc
- }
- case S_INT:
- case S_HEX:
-- return str;
-+ return "0";
- case S_STRING:
- return str;
- case S_UNKNOWN:
Index: kconfig/confdata.c
===================================================================
--- kconfig.orig/confdata.c
--
To view, visit https://review.coreboot.org/c/coreboot/+/78442?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: I5ad2180854e0263d2d097b059cb16ec478b859c5
Gerrit-Change-Number: 78442
Gerrit-PatchSet: 4
Gerrit-Owner: Richard Marko <srk(a)48.io>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78441?usp=email )
Change subject: util/kconfig: Fix README.md formatting
......................................................................
util/kconfig: Fix README.md formatting
Change-Id: I0c47a603cc6e6174cd4895ff9f44b5bc242c653e
Signed-off-by: Richard Marko <srk(a)48.io>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78441
Reviewed-by: Martin L Roth <gaumless(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M util/kconfig/README.md
1 file changed, 8 insertions(+), 3 deletions(-)
Approvals:
Martin L Roth: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/util/kconfig/README.md b/util/kconfig/README.md
index 4d508a5..a02d903 100644
--- a/util/kconfig/README.md
+++ b/util/kconfig/README.md
@@ -13,7 +13,9 @@
in an already-configured tree (`quilt pop -a` should cleanly unapply them all)
or manually if quilt doesn't have its tracking metadata around yet:
- $ for i in `ls patches/*.patch | tac`; do patch -p1 -R -i "$i"; done
+```sh
+for i in $( ls patches/*.patch | tac ); do patch -p1 -R -i "$i"; done
+```
The result should be a subtree that, apart from a few coreboot specific
files on our side (e.g. documentation, integration in our build system)
@@ -23,9 +25,11 @@
`git log util/kconfig` output in our tree.
Assuming that you want to uprev from Linux 5.13 to 5.14, with a Linux git tree
-available in ~/linux,
+available in `~/linux`
- $ cd util/kconfig && (cd ~/linux/ && git diff v5.13..v5.14 scripts/kconfig) | patch -p2`
+```sh
+cd util/kconfig && (cd ~/linux/ && git diff v5.13..v5.14 scripts/kconfig) | patch -p2`
+```
applies the changes to your local tree.
@@ -39,6 +43,7 @@
the tree has been upreved to.
## Adding a new patch
+
The format of the patches to kconfig is a mix of the headers produced by `git
format-patch` and the patch format of quilt. However neither git nor quilt
seems to have any functionality to directly produce a file in such a format
--
To view, visit https://review.coreboot.org/c/coreboot/+/78441?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: I0c47a603cc6e6174cd4895ff9f44b5bc242c653e
Gerrit-Change-Number: 78441
Gerrit-PatchSet: 4
Gerrit-Owner: Richard Marko <srk(a)48.io>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78440?usp=email )
Change subject: util/kconfig: add savedefconfig patch to quilt
......................................................................
util/kconfig: add savedefconfig patch to quilt
Adds commit 48ad5c23680c util/kconfig: chmod +w before savedefconfig
to quilt patch series.
Change-Id: I381dce2fee995227efc60169fd90ab505c99b74b
Signed-off-by: Richard Marko <srk(a)48.io>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78440
Reviewed-by: Martin L Roth <gaumless(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
A util/kconfig/patches/0015-util-kconfig-chmod-w-before-savedefconfig.patch
M util/kconfig/patches/series
2 files changed, 31 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Martin L Roth: Looks good to me, approved
diff --git a/util/kconfig/patches/0015-util-kconfig-chmod-w-before-savedefconfig.patch b/util/kconfig/patches/0015-util-kconfig-chmod-w-before-savedefconfig.patch
new file mode 100644
index 0000000..2b96723
--- /dev/null
+++ b/util/kconfig/patches/0015-util-kconfig-chmod-w-before-savedefconfig.patch
@@ -0,0 +1,30 @@
+From 48ad5c23680c81614663e09c6586ebeb26bf8c18 Mon Sep 17 00:00:00 2001
+From: Richard Marko <srk(a)48.io>
+Date: Mon, 16 Oct 2023 15:26:33 +0200
+Subject: [PATCH] util/kconfig: chmod +w before savedefconfig
+
+This prevents a headscratcher when .config in root doesn't have a write
+permission bit set which causes a build failure of savedefconfig
+not able to write to copied file, for example
+
+*** Error while saving defconfig to:
+ build/mainboard/emulation/qemu-i440fx/cbfs-file.eU5E0t.out.tmp2
+
+Change-Id: I2e7d35c9f6e8add3e7438d163850bc5fda5a99b2
+Signed-off-by: Richard Marko <srk(a)48.io>
+---
+ util/kconfig/Makefile.inc | 1 +
+ 1 file changed, 1 insertion(+)
+
+Index: kconfig/Makefile.inc
+===================================================================
+--- kconfig.orig/Makefile.inc
++++ kconfig/Makefile.inc
+@@ -34,6 +34,7 @@ oldconfig: KCONFIG_STRICT=
+
+ savedefconfig: $(objk)/conf
+ cp $(DOTCONFIG) $(DEFCONFIG)
++ chmod +w $(DEFCONFIG)
+ $< --savedefconfig=$(DEFCONFIG) $(KBUILD_KCONFIG)
+
+ FORCE:
diff --git a/util/kconfig/patches/series b/util/kconfig/patches/series
index 655c493..252eb64 100644
--- a/util/kconfig/patches/series
+++ b/util/kconfig/patches/series
@@ -10,3 +10,4 @@
0010-reenable-source-in-choice.patch
0013-util-kconfig-detect-ncurses-on-FreeBSD.patch
0014-util-kconfig-Move-Kconfig-deps-back-into-build-confi.patch
+0015-util-kconfig-chmod-w-before-savedefconfig.patch
--
To view, visit https://review.coreboot.org/c/coreboot/+/78440?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: I381dce2fee995227efc60169fd90ab505c99b74b
Gerrit-Change-Number: 78440
Gerrit-PatchSet: 4
Gerrit-Owner: Richard Marko <srk(a)48.io>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78415?usp=email )
Change subject: util/kconfig: chmod +w before savedefconfig
......................................................................
util/kconfig: chmod +w before savedefconfig
This prevents a headscratcher when .config in root doesn't have a write
permission bit set which causes a build failure of savedefconfig
not able to write to copied file, for example
*** Error while saving defconfig to:
build/mainboard/emulation/qemu-i440fx/cbfs-file.eU5E0t.out.tmp2
Change-Id: I2e7d35c9f6e8add3e7438d163850bc5fda5a99b2
Signed-off-by: Richard Marko <srk(a)48.io>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/78415
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Martin L Roth <gaumless(a)gmail.com>
Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org>
---
M util/kconfig/Makefile.inc
1 file changed, 1 insertion(+), 0 deletions(-)
Approvals:
Paul Menzel: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
Martin L Roth: Looks good to me, approved
diff --git a/util/kconfig/Makefile.inc b/util/kconfig/Makefile.inc
index 36678da..5f75ba3 100644
--- a/util/kconfig/Makefile.inc
+++ b/util/kconfig/Makefile.inc
@@ -34,6 +34,7 @@
savedefconfig: $(objk)/conf
cp $(DOTCONFIG) $(DEFCONFIG)
+ chmod +w $(DEFCONFIG)
$< --savedefconfig=$(DEFCONFIG) $(KBUILD_KCONFIG)
FORCE:
--
To view, visit https://review.coreboot.org/c/coreboot/+/78415?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: I2e7d35c9f6e8add3e7438d163850bc5fda5a99b2
Gerrit-Change-Number: 78415
Gerrit-PatchSet: 4
Gerrit-Owner: Richard Marko <srk(a)48.io>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Arthur Heymans, Eran Mitrani, Jakub Czapiga, Julius Werner, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Nico Huber, Tarun.
Subrata Banik 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 32:
(1 comment)
File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/77886/comment/b0b2c5f9_82d6c3a6 :
PS32, Line 48: data = NULL;
: size = 0;
> The `data = NULL;` would have to stay because it's static and used
> to decide what to return in later calls.
ah, i overlook the static part
--
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: 32
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: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Tue, 31 Oct 2023 17:04:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78818?usp=email )
Change subject: soc/amd/common/psp: Remove unnecessary prompts from Kconfig
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/78818?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: Idcd2ba84591d31a9a25bcc6cae3ec163939d7836
Gerrit-Change-Number: 78818
Gerrit-PatchSet: 1
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Tue, 31 Oct 2023 16:58:25 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Nien, Karthik Ramasubramanian, Martin L Roth, Raul Rangel.
Mark Hasemeyer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78137?usp=email )
Change subject: mb/google/guybrush: Set PS2K_IRQ to level/low
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> Please do not merge yet. […]
Questions about how this affects the EC are resolved.
--
To view, visit https://review.coreboot.org/c/coreboot/+/78137?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: I7d093d94a666263684645ef724e945069c68c806
Gerrit-Change-Number: 78137
Gerrit-PatchSet: 3
Gerrit-Owner: Mark Hasemeyer <markhas(a)google.com>
Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Tue, 31 Oct 2023 16:58:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mark Hasemeyer <markhas(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Eran Mitrani, Jakub Czapiga, Julius Werner, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Subrata Banik, Tarun.
Nico Huber 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 32:
(2 comments)
File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/77886/comment/3b5d45a5_b4bb8ad8 :
PS4, Line 27: cbfs_map
> > I did at line 36 in case of error. […]
IIRC, locate_vbt() is called rather early in ramstage. Doesn't this mean
we would lose a huge part of the cache if we don't unmap? IMO, it should
be part of this patch. Or is it a common practice to leak cache space?
File src/drivers/intel/gma/opregion.c:
https://review.coreboot.org/c/coreboot/+/77886/comment/116f4056_d17a5ad8 :
PS32, Line 48: data = NULL;
: size = 0;
> can't we just `return NULL` from here ?
The `data = NULL;` would have to stay because it's static and used
to decide what to return in later calls.
--
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: 32
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: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(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: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Tue, 31 Oct 2023 16:57:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment