Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80367?usp=email )
Change subject: mb/qemu/fw_cfg: Use fw_cfg_read() to read SMBIOS data
......................................................................
mb/qemu/fw_cfg: Use fw_cfg_read() to read SMBIOS data
The QEMU firmware configuration driver can help initialize SMBIOS tables
using the table data that QEMU provides over the device. While doing so,
it reads from the device "file" manually using port-based IO.
Use the fw_cfg_read() helper function to read the SMBIOS-related file,
so that the driver is easier to port the driver to other architectures.
Change-Id: I18e60b8e9de34f2b0ff67af4113beec1d7467329
Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80367
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Reviewed-by: Martin L Roth <gaumless(a)gmail.com>
Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
1 file changed, 2 insertions(+), 2 deletions(-)
Approvals:
Martin L Roth: Looks good to me, approved
Matt DeVillier: Looks good to me, approved
build bot (Jenkins): Verified
Nico Huber: Looks good to me, but someone else must approve
diff --git a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
index 0f217cc..a15773c 100644
--- a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
+++ b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
@@ -362,9 +362,9 @@
fw_cfg_get(FW_CFG_SMBIOS_ENTRIES, &count, sizeof(count));
for (i = 0; i < count; i++) {
- insb(FW_CFG_PORT_DATA, &entry, sizeof(entry));
+ fw_cfg_read(&entry, sizeof(entry));
buf = malloc(entry.length - sizeof(entry));
- insb(FW_CFG_PORT_DATA, buf, entry.length - sizeof(entry));
+ fw_cfg_read(buf, entry.length - sizeof(entry));
if (entry.headertype == SMBIOS_FIELD_ENTRY &&
entry.tabletype == 1) {
switch (entry.fieldoffset) {
--
To view, visit https://review.coreboot.org/c/coreboot/+/80367?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: I18e60b8e9de34f2b0ff67af4113beec1d7467329
Gerrit-Change-Number: 80367
Gerrit-PatchSet: 2
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Alper Nebi Yasak.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80367?usp=email )
Change subject: mb/qemu/fw_cfg: Use fw_cfg_read() to read SMBIOS data
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80367?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: I18e60b8e9de34f2b0ff67af4113beec1d7467329
Gerrit-Change-Number: 80367
Gerrit-PatchSet: 1
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Comment-Date: Tue, 27 Feb 2024 20:22:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80366?usp=email )
Change subject: mb/qemu/fw_cfg: Fix build when not generating SMBIOS tables
......................................................................
mb/qemu/fw_cfg: Fix build when not generating SMBIOS tables
Parts of the QEMU firmware configuration device driver refers to SMBIOS
related kconfig values. These depend on GENERATE_SMBIOS_TABLES and are
undefined if it isn't enabled, causing a build error.
Cover the SMBIOS-related region in this driver with an #if directive
checking the necessary config option. This is mostly to help port the
driver to non-x86 architectures where support for generating SMBIOS
tables isn't there yet.
Change-Id: I3ff388d4574eb52686a5dda3dcbc3d64a7ce6f7b
Signed-off-by: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80366
Reviewed-by: Martin L Roth <gaumless(a)gmail.com>
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M src/mainboard/emulation/qemu-i440fx/fw_cfg.c
1 file changed, 2 insertions(+), 0 deletions(-)
Approvals:
Nico Huber: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
Martin L Roth: Looks good to me, approved
diff --git a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
index 5c23988..0f217cc 100644
--- a/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
+++ b/src/mainboard/emulation/qemu-i440fx/fw_cfg.c
@@ -341,6 +341,7 @@
/* ---------------------------------------------------------------------- */
/* pick up smbios information from fw_cfg */
+#if CONFIG(GENERATE_SMBIOS_TABLES)
static const char *type1_manufacturer;
static const char *type1_product_name;
static const char *type1_version;
@@ -504,6 +505,7 @@
fw_cfg_smbios_init();
memcpy(uuid, type1_uuid, 16);
}
+#endif /* CONFIG(GENERATE_SMBIOS_TABLES) */
/*
* Configure DMA setup
--
To view, visit https://review.coreboot.org/c/coreboot/+/80366?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: I3ff388d4574eb52686a5dda3dcbc3d64a7ce6f7b
Gerrit-Change-Number: 80366
Gerrit-PatchSet: 2
Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80751?usp=email )
Change subject: Docs/releases: Finalize 24.02 release notes
......................................................................
Docs/releases: Finalize 24.02 release notes
Change-Id: I5ba6619ee7ed408a33548ab5b6f7d2a2143e88e7
Signed-off-by: Martin Roth <gaumless(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80751
Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M Documentation/releases/coreboot-24.02-relnotes.md
1 file changed, 20 insertions(+), 20 deletions(-)
Approvals:
Felix Singer: Looks good to me, approved
build bot (Jenkins): Verified
Matt DeVillier: Looks good to me, approved
diff --git a/Documentation/releases/coreboot-24.02-relnotes.md b/Documentation/releases/coreboot-24.02-relnotes.md
index d941e2c..eb5349e 100644
--- a/Documentation/releases/coreboot-24.02-relnotes.md
+++ b/Documentation/releases/coreboot-24.02-relnotes.md
@@ -1,10 +1,7 @@
-Upcoming release - coreboot 24.02
+coreboot 24.02 release
========================================================================
-The 24.02 release is scheduled for February 19, 2024. The next release,
-which will be 24.05, is scheduled for mid-May.
-
-The coreboot project is happy to announce our next release for February
+The coreboot project is happy to announce our release for February
2024. Over the past three months, our contributors have focused on
refining the coreboot codebase, generally prioritizing cleanup and
quality enhancements. We extend our gratitude to all the contributors
@@ -12,6 +9,8 @@
invaluable contributions to this vital phase of maintenance and
optimization.
+The next release is scheduled for mid-May.
+
### Release number format update
@@ -127,9 +126,9 @@
### Toolchain updates
* Add buildgcc support for Apple M1/M2 devices
-* crossgcc: Upgrade GCC from 11.4.0 to 13.2.0
-* util/crossgcc: Update CMake from 3.26.4 to 3.27.7
-* util/kconfig: Uprev to Linux 6.7 kconfig
+* Upgrade GCC from 11.4.0 to 13.2.0
+* Update CMake from 3.26.4 to 3.27.7
+* Uprev to Kconfig from Linux 6.7
### Git submodule pointers
@@ -140,9 +139,9 @@
17bef2248d (701 commits)
* /3rdparty/fsp: Update from commit id 481ea7cf0b to 507ef01cce (16 commits)
* /3rdparty/intel-microcode: Update from commit id 6788bb07eb to
- ece0d294a2 (1 commits)
-* /3rdparty/vboot: Update from commit id 24cb127a5e to 3d37d2aafe (121
- commits)
+ ece0d294a2 (1 commit)
+* /3rdparty/vboot: Update from commit id 24cb127a5e to 3d37d2aafe
+ (121 commits)
### External payloads
@@ -168,15 +167,15 @@
Statistics from the 4.22 to the 24.02 release
--------------------------------------------
-* Total Commits: 814
-* Average Commits per day: 8.65
-* Total lines added: 105203
-* Average lines added per commit: 129.24
-* Number of patches adding more than 100 lines: 46
+* Total Commits: 815
+* Average Commits per day: 8.63
+* Total lines added: 105433
+* Average lines added per commit: 129.37
+* Number of patches adding more than 100 lines: 47
* Average lines added per small commit: 41.34
-* Total lines removed: 16505
-* Average lines removed per commit: 20.28
-* Total difference between added and removed: 88698
+* Total lines removed: 16534
+* Average lines removed per commit: 20.29
+* Total difference between added and removed: 88899
* Total authors: 111
* New authors: 19
@@ -185,7 +184,8 @@
Significant Known and Open Issues
---------------------------------
-* AMD chromebooks will not currently work with the signed vboot image.
+* AMD chromebooks will not work with the signed PSP_verstage images and
+ the version of verstage used in coreboot 24.02.
## Issues from the coreboot bugtracker: https://ticket.coreboot.org/
--
To view, visit https://review.coreboot.org/c/coreboot/+/80751?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: I5ba6619ee7ed408a33548ab5b6f7d2a2143e88e7
Gerrit-Change-Number: 80751
Gerrit-PatchSet: 2
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80645?usp=email )
Change subject: device/pnp_device: Demote unassigned resource printk to NOTICE
......................................................................
device/pnp_device: Demote unassigned resource printk to NOTICE
Often times not all available resources are used on a PNP function, so
those resources not being specified is intentional, not an error. Keep
the printk but demote it so it doesn't pollute a normal cbmem log.
TEST=build/boot purism/librem_cnl (Mini v2), verify errors in cbmem
related to RTC IO/IRQ not being assigned are no longer present.
Change-Id: I3d9f22a06088596e14680190aede2d69880001fa
Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80645
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
---
M src/device/pnp_device.c
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Felix Singer: Looks good to me, approved
Nico Huber: Looks good to me, but someone else must approve
build bot (Jenkins): Verified
diff --git a/src/device/pnp_device.c b/src/device/pnp_device.c
index f799530..c7c6228 100644
--- a/src/device/pnp_device.c
+++ b/src/device/pnp_device.c
@@ -130,7 +130,7 @@
resource->index, resource_type(resource),
resource->size);
else
- printk(BIOS_ERR, "%s %02lx %s size: 0x%010llx "
+ printk(BIOS_NOTICE, "%s %02lx %s size: 0x%010llx "
"not assigned in devicetree\n", dev_path(dev), resource->index,
resource_type(resource), resource->size);
return;
--
To view, visit https://review.coreboot.org/c/coreboot/+/80645?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: I3d9f22a06088596e14680190aede2d69880001fa
Gerrit-Change-Number: 80645
Gerrit-PatchSet: 3
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80750?usp=email )
Change subject: soc/intel/common/lpc: Don't open a window for unassigned resources
......................................................................
soc/intel/common/lpc: Don't open a window for unassigned resources
Don't attempt to open a PMIO window for a resource which doesn't have
the IORESOURCE_ASSIGNED flag set, since there is no point in doing so
and there's a high likelihood that the base address is 0, which will
throw an error.
TEST=build/boot purism/librem_cnl (Mini v2), ensure no errors in cbmem
log for attempting to open a PMIO window for unaassigned resources with
base address 0.
Change-Id: Ifba14a8f134ba12d5f5e9fdbac775d4f82b4c4de
Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80750
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
---
M src/soc/intel/common/block/lpc/lpc.c
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Felix Singer: Looks good to me, approved
build bot (Jenkins): Verified
Nico Huber: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/common/block/lpc/lpc.c b/src/soc/intel/common/block/lpc/lpc.c
index b27e09e9..e805035 100644
--- a/src/soc/intel/common/block/lpc/lpc.c
+++ b/src/soc/intel/common/block/lpc/lpc.c
@@ -91,7 +91,7 @@
return;
for (res = dev->resource_list; res; res = res->next) {
- if (res->flags & IORESOURCE_IO)
+ if ((res->flags & IORESOURCE_IO) && (res->flags & IORESOURCE_ASSIGNED))
lpc_open_pmio_window(res->base, res->size);
}
pch_lpc_set_child_resources(dev);
--
To view, visit https://review.coreboot.org/c/coreboot/+/80750?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: Ifba14a8f134ba12d5f5e9fdbac775d4f82b4c4de
Gerrit-Change-Number: 80750
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Jeremy Soller, Tim Crawford.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80756?usp=email )
Change subject: mb/system76/adl,rpl: Add timeouts for PCIe 3.0 RPs
......................................................................
Patch Set 1:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80756/comment/07e1592b_8f55aa5c :
PS1, Line 7: Add timeouts for PCIe 3.0 RPs
:
Maybe:
> Add 50 ms timeouts for PCIe 3.0 RPs
https://review.coreboot.org/c/coreboot/+/80756/comment/895be8d3_31d7097f :
PS1, Line 14: Tested on lemp12 with Samsung 980 PRO and 990 PRO drives.
What drive firmware versions?
https://review.coreboot.org/c/coreboot/+/80756/comment/7b43751d_ea1fb68f :
PS1, Line 15:
Is there a default time-out value? Why 50 ms and not another value?
File src/mainboard/system76/adl/variants/darp8/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/80756/comment/08282b45_21fa1e06 :
PS1, Line 155: .pcie_rp_detect_timeout_ms = 50,
Is there a default timeout?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80756?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: I4f44fc429c52e407b7566d6bb6dd31b2cf85c48d
Gerrit-Change-Number: 80756
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Comment-Date: Tue, 27 Feb 2024 20:17:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Forest Mittelberg, Matt DeVillier.
Hello Caveh Jalali, Forest Mittelberg, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80712?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Forest Mittelberg, Verified+1 by build bot (Jenkins)
Change subject: ec/chromeec: Enable auto fan control on startup
......................................................................
ec/chromeec: Enable auto fan control on startup
Several older ChromeOS boards have issues with fan control on cold boot
and/or on S3 resume, so add functionality to allow those boards to
programmatically enable auto fan control.
TEST=build/boot google/link, verify fan ramps up/down accordingly with
CPU load.
Change-Id: I08a8562531f8af0c71230477d0221d536443f096
Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com>
---
M src/ec/google/chromeec/Kconfig
M src/ec/google/chromeec/ec.c
2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/80712/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80712?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: I08a8562531f8af0c71230477d0221d536443f096
Gerrit-Change-Number: 80712
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Forest Mittelberg <bmbm(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Forest Mittelberg <bmbm(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Martin L Roth, Maximilian Brune.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80731?usp=email )
Change subject: util/crossgcc: Use ninja to build llvm/clang
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> I did 4 measurements (2 each) with 8 jobs on a machine that did nothing besides building the toolcha […]
I also did some test builds using 32 threads and 128 GiB RAM.
Using Ninja
```
real 17m19.320s
user 477m53.363s
sys 23m44.042s
```
Using Make
```
real 17m41.831s
user 470m21.309s
sys 23m52.428s
```
An improvement is not really noticeable here. So I rather would like to find a different reason for this change. Is Ninja the preferred build system? Or maybe the successor of Make?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80731?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: I66d2a4e6e5c265f5580a74e59309e2ad8b1f1f61
Gerrit-Change-Number: 80731
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Tue, 27 Feb 2024 19:56:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Paz Zcharya, Subrata Banik.
Gwendal Grignou has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80738?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: vc/google/chromeos: Implement dynamic ChromeOS boot logo selection
......................................................................
Patch Set 7:
(3 comments)
File src/vendorcode/google/chromeos/tpm_factory_config.c:
https://review.coreboot.org/c/coreboot/+/80738/comment/5f8a6855_968b9cfa :
PS7, Line 49: * - Bits 3-0 (0x1): Must be 0x1 to signify compliance with Chromebook-Plus
> > Not true: this field contains the "feature_level" that indicate the chromebook plus generation. […]
To be future-proof, the test on line 61 should be
`return factory_config & CHROMEBOOK_PLUS_DEVICE;`
https://review.coreboot.org/c/coreboot/+/80738/comment/8b4cb17a_a1e66168 :
PS7, Line 74: * requirement.
> > Not true. […]
I assume the design doc is at `go/cros-tiering-dd`.
If this CL only applies to new chromebook that have identification in their GSC where `chassis_x_branded` and `hw_x_compliance_version` are properly set, then we don't need to read the VPD at all.
Per design [see section `OS Runtime flow`], all the information we need in the GSC.
https://review.coreboot.org/c/coreboot/+/80738/comment/1274f52e_2f4b0617 :
PS7, Line 104: return strncmp(vpd_get_feature_device_info(), "CAI", 3) == 0;
> > This is not acceptable for production code. "CAI" is not defined anywhere in the code base. […]
The previous comment states "all older devices will have legacy config", so we don't need the non-GSC bit parsing logic, do we?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80738?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: I9bb1e868764738333977bd8c990bea4253c9d37b
Gerrit-Change-Number: 80738
Gerrit-PatchSet: 7
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Gwendal Grignou <gwendal(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Paz Zcharya <pazz(a)google.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paz Zcharya <pazz(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 27 Feb 2024 19:33:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Gwendal Grignou <gwendal(a)chromium.org>
Gerrit-MessageType: comment