Attention is currently required from: Jakub Czapiga, Paul Fagerburg, Jan Dabros.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56305 )
Change subject: tests: Add lib/lzmadecode-test test case
......................................................................
Patch Set 1:
(1 comment)
File tests/include/tests/lib/lzmadecode_data.h:
https://review.coreboot.org/c/coreboot/+/56305/comment/326d0282_a0669b41
PS1, Line 24: unsigned char test_lzmadecode_data_binary[] = {
> I have checked the `xz` manual again. Specifically, the `Streamed vs. non-streamed .lzma files` section. It says, that XZ Utils always use end-of-payload marker, and put unknown (zero) uncompressed file size in the LZMA header. Moreover, manual suggests using LZMA Utils instead, if one needs uncompressed file size field to be filled.
Sorry, actually, I confused myself there... Chrome OS is using XZ for kernel compression, but coreboot isn't involved in decompressing that (although libpayload is which should(?) pretty much use the same algorithm). For payload compression we just use what's built into cbfstool. So if you're saying that XZ util output doesn't work in coreboot I guess we can skip testing that. The most important thing to test is the one built into cbfstool (you should be able to use cbfs-compression-tool to generate files with that more easily).
--
To view, visit https://review.coreboot.org/c/coreboot/+/56305
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3733adc32ee2f5583f6d2b56df379f1bd170c233
Gerrit-Change-Number: 56305
Gerrit-PatchSet: 1
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Wed, 28 Jul 2021 21:22:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com>
Comment-In-Reply-To: Paul Fagerburg <pfagerburg(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Jakub Czapiga, Paul Fagerburg, Jan Dabros.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56305 )
Change subject: tests: Add lib/lzmadecode-test test case
......................................................................
Patch Set 1:
(1 comment)
File tests/include/tests/lib/lzmadecode_data.h:
https://review.coreboot.org/c/coreboot/+/56305/comment/720d28df_d103a988
PS1, Line 24: unsigned char test_lzmadecode_data_binary[] = {
> Would libcmocka.so be good? It is a object file, its size is roughly similar to the coreboot payloads size.
Yeah, sounds just as good as any other.
> It seems to be simpler than generating data, converting it to C array, sed-ing to match test variable names etc.
Yeah, sure... I mean you're welcome to switch other tests to this format too if you want to. If we write a few common helpers for it it would hopefully become pretty easy.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56305
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3733adc32ee2f5583f6d2b56df379f1bd170c233
Gerrit-Change-Number: 56305
Gerrit-PatchSet: 1
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Paul Fagerburg <pfagerburg(a)chromium.org>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Wed, 28 Jul 2021 21:11:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com>
Comment-In-Reply-To: Paul Fagerburg <pfagerburg(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Subrata Banik, Angel Pons, Patrick Rudolph.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56650 )
Change subject: soc/intel/common: Don't suppress SPD Module Type value
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/smbios.c:
https://review.coreboot.org/c/coreboot/+/56650/comment/4af2bca0_c5ce5167
PS3, Line 18: /* Translate to DDR2 module type field that SMBIOS code expects. */
> i believe, we can combine DDRx and LPx to reduce the if/else as most of the types are same in those […]
I see conflicting types for DDR2 v/s DDR3 in the SPD spec. Not sure about other memory types. If the module type is exactly the same, I think it makes sense to use a common function. But if that is not true, we will need separate functions.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia3f38c24efa6a8685639bb607926e2fd9c702ff6
Gerrit-Change-Number: 56650
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 28 Jul 2021 21:09:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Paul Menzel, Angel Pons, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56650 )
Change subject: soc/intel/common: Don't suppress SPD Module Type value
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/smbios.c:
https://review.coreboot.org/c/coreboot/+/56650/comment/f4b9055e_b350bc92
PS3, Line 18: /* Translate to DDR2 module type field that SMBIOS code expects. */
> > Yes i don't know that answer either and IMHO we should let platform choose if they want to go thro […]
i believe, we can combine DDRx and LPx to reduce the if/else as most of the types are same in those spec. isn't it ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/56650
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia3f38c24efa6a8685639bb607926e2fd9c702ff6
Gerrit-Change-Number: 56650
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Wed, 28 Jul 2021 21:02:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: comment
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56431 )
Change subject: Update chromeec submodule to upstream main
......................................................................
Update chromeec submodule to upstream main
Updating from commit id 1e800ac83:
2021-03-01 22:59:54 +0000 - (docs: point md files in master to main/HEAD)
to commit id 4c21b57eb:
2021-07-19 11:36:07 +0000 - (pd: Fix missing polarity_rm_dts in some conditions)
This brings in 3145 new commits.
Change-Id: Iff2e9f766e750070d71644c2f9895ad10e8b1c9a
Signed-off-by: Patrick Georgi <pgeorgi(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56431
Reviewed-by: Julius Werner <jwerner(a)chromium.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M 3rdparty/chromeec
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Julius Werner: Looks good to me, approved
diff --git a/3rdparty/chromeec b/3rdparty/chromeec
index 1e800ac..4c21b57 160000
--- a/3rdparty/chromeec
+++ b/3rdparty/chromeec
@@ -1 +1 @@
-Subproject commit 1e800ac838504c0d2950c7aa90cdfe7bde251545
+Subproject commit 4c21b57eb9619cc3dc86d11226917d25f62f1bc8
--
To view, visit https://review.coreboot.org/c/coreboot/+/56431
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iff2e9f766e750070d71644c2f9895ad10e8b1c9a
Gerrit-Change-Number: 56431
Gerrit-PatchSet: 6
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56416 )
Change subject: Update arm-trusted-firmware submodule to upstream integration
......................................................................
Update arm-trusted-firmware submodule to upstream integration
Updating from commit id 96404aa27:
2021-05-13 18:27:27 +0200 - (Merge "build(hooks): update Commitizen to ^4.2.4" into integration)
to commit id 586aafa3a:
2021-07-19 05:36:18 +0200 - (Merge "errata: workaround for Neoverse V1 errata 1791573" into integration)
This brings in 207 new commits.
Change-Id: Iaf8af5ffaf377070ee1430ed7cfdc51001a1ba6b
Signed-off-by: Patrick Georgi <pgeorgi(a)google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56416
Reviewed-by: Julius Werner <jwerner(a)chromium.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M 3rdparty/arm-trusted-firmware
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Julius Werner: Looks good to me, approved
diff --git a/3rdparty/arm-trusted-firmware b/3rdparty/arm-trusted-firmware
index 96404aa..586aafa 160000
--- a/3rdparty/arm-trusted-firmware
+++ b/3rdparty/arm-trusted-firmware
@@ -1 +1 @@
-Subproject commit 96404aa27efbf1c9051d515075a60c3cf4fa47be
+Subproject commit 586aafa3a4b13971339f78e430075592c3fe74b5
--
To view, visit https://review.coreboot.org/c/coreboot/+/56416
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaf8af5ffaf377070ee1430ed7cfdc51001a1ba6b
Gerrit-Change-Number: 56416
Gerrit-PatchSet: 5
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Furquan Shaikh, Subrata Banik, Nick Vaccaro, EricR Lai.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51374 )
Change subject: soc/intel/common: Calculate and configure both SF Mask 1 and 2
......................................................................
Patch Set 16:
(5 comments)
Patchset:
PS16:
This patch is getting a little hairy... could it be split into updating SF mask 1 and SF mask 2?
File src/soc/intel/common/block/cpu/Kconfig:
https://review.coreboot.org/c/coreboot/+/51374/comment/36527aef_5ad99392
PS16, Line 70: User to select this
Not really for the user is it? It should be selected by the SoCs that require it.
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/51374/comment/1c012858_b981afa7
PS16, Line 512: dataway
data_ways
https://review.coreboot.org/c/coreboot/+/51374/comment/6d142ca0_6d1027b3
PS16, Line 523: dataway
data_ways
https://review.coreboot.org/c/coreboot/+/51374/comment/cbfa5716_36b1a021
PS16, Line 531: mov %ebx, %eax /* restore dataway in eax */
: movl $0x02, %ecx
: div %ecx
WDYT about `shr` instead of `div`:
```
movl $0x01, %ecx
shr %ebx, %cl
```
the value of `ebx` isn't required after this point (it's overwritten on line #541)
--
To view, visit https://review.coreboot.org/c/coreboot/+/51374
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifd0b7e1a90cad4a4837adf6067fe8301dcd0a941
Gerrit-Change-Number: 51374
Gerrit-PatchSet: 16
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 28 Jul 2021 20:52:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56431 )
Change subject: Update chromeec submodule to upstream main
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56431
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iff2e9f766e750070d71644c2f9895ad10e8b1c9a
Gerrit-Change-Number: 56431
Gerrit-PatchSet: 5
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Wed, 28 Jul 2021 20:52:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56416 )
Change subject: Update arm-trusted-firmware submodule to upstream integration
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56416
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaf8af5ffaf377070ee1430ed7cfdc51001a1ba6b
Gerrit-Change-Number: 56416
Gerrit-PatchSet: 4
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Comment-Date: Wed, 28 Jul 2021 20:52:00 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Yu-Ping Wu.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56543 )
Change subject: helpers: Add GENMASK and GENMASK_ULL macros
......................................................................
Patch Set 5:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56543/comment/77446275_bb79f549
PS5, Line 12: helpers.h, for unsigned long and unsigned long long types, respectively.
Do we really need two helpers? What happens if you just make it ULL to begin with, does it throw warnings or something?
Patchset:
PS5:
Just to be clear I only meant to suggest standardizing on the general name (GENMASK), as long as we're generally compatible we don't have to copy the Linux code 1-to-1 (e.g. I don't think we need a separate _ULL version unless it's really necessary).
File src/commonlib/bsd/include/commonlib/bsd/helpers.h:
https://review.coreboot.org/c/coreboot/+/56543/comment/c8d7d7e3_42617fc6
PS5, Line 65: #define GENMASK_ULL(h, l) (((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
This file is BSD-licensed, so please don't copy Linux code wholesale (including the comment) in here (or did you also find this code under BSD somewhere else?). I mean there are only so many ways to write the same simple operation, but let's at least try to follow the rules as much as possible.
File src/soc/mediatek/common/pll.c:
https://review.coreboot.org/c/coreboot/+/56543/comment/5b6494f6_c273c414
PS5, Line 3: #include <commonlib/bsd/helpers.h>
Prefer to just #include <types.h> instead of including this directly.
File tests/commonlib/bsd/helpers-test.c:
https://review.coreboot.org/c/coreboot/+/56543/comment/76104ba0_3cf6270b
PS4, Line 13: 4, 5
> Adding an assertion seems to make the macro unnecessarily complicated. […]
Yeah I don't think an assertion is needed, just wanted to bring up all options.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56543
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If2e7c4827d8a7d27688534593b556a72f16f0c2b
Gerrit-Change-Number: 56543
Gerrit-PatchSet: 5
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Wed, 28 Jul 2021 20:42:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-MessageType: comment