Attention is currently required from: Mario Scheithauer.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63862 )
Change subject: mb/siemens/mc_ehl2: Adjust PSE TSN settings in devicetree
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63862/comment/9e1ddebe_d994c0c9
PS3, Line 9: Two
: of them reside in PCH including Intel Programmable Services Engine (PSE)
: and are controlled by these.
Maybe:
Two of them are initialized by the Programmable Service Engine (PSE).
--
To view, visit https://review.coreboot.org/c/coreboot/+/63862
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I74e660548b2c44d5dbdb6023d5a36cfdd7e96f43
Gerrit-Change-Number: 63862
Gerrit-PatchSet: 3
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Comment-Date: Tue, 03 May 2022 08:46:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Martin L Roth, Maximilian Brune, Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63639 )
Change subject: Add SBOM (Software Bill of Materials) Generation
......................................................................
Patch Set 14:
(4 comments)
File util/goswid/cmd/main.go:
https://review.coreboot.org/c/coreboot/+/63639/comment/db51514d_dae4b73c
PS12, Line 54: //if (output_format != uswid) && (len(input_file_paths) > 1) {
: // ErrorOut("multiple input files are only supported in conjunction with the .uswid output file extension\n")
: //}
> since this corresponds to a different project/repo, I will make the changes there and pull them at a […]
Ack
https://review.coreboot.org/c/coreboot/+/63639/comment/84f5ab8b_58e6ca5e
PS12, Line 68: if if_len >= 5 && input_file_path[if_len-5:if_len] == ".json" {
: err = input_tag.FromJSON(input_file)
: } else if if_len >= 4 && input_file_path[if_len-4:if_len] == ".xml" {
: err = input_tag.FromXML(input_file)
: } else if if_len >= 5 && input_file_path[if_len-5:if_len] == ".cbor" {
: err = input_tag.FromCBOR(input_file)
: } else if if_len >= 6 && input_file_path[if_len-6:if_len] == ".uswid" {
: _, err = uswid_input_tag.FromUSWID(input_file)
: isUSWID = true
: } else {
: fmt.Printf("input file extension not recognized, assuming USWID: %s\n", input_file_path)
: _, err = uswid_input_tag.FromUSWID(input_file)
: isUSWID = true
: }
> I guess yours is a bit more explicit. […]
Ack. My approach avoids having the same information in three different places on each line:
if if_len >= 5 && input_file_path[if_len-5:if_len] == ".json" {
Here, the length of the string to compare appears twice in the form of `5` and is also implicit in the string literal.
File util/goswid/pkg/uswid/uswid.go:
https://review.coreboot.org/c/coreboot/+/63639/comment/c740532f_8d4845f3
PS12, Line 14: // can't be const...
> to be honest I was kind of confused with this one. […]
Huh, weird.
https://review.coreboot.org/c/coreboot/+/63639/comment/1774aa64_a03343d6
PS12, Line 32: 16
> in my opinion this is the most explicit and readable version of doing protocol/encoding stuff. An abstraction would only make it worse (at least in my opinion)
Hmmm, my main complaint is that having magic offsets makes it hard to know what's being accessed (one would need to check some spec/description of the binary data), that's why I'd try to define constants for the numbers (e.g. something like `offset_header_version := 16`).
By abstraction I was thinking of something that can convert the binary data into a structure or an object and viceversa. Does something like this still exist? https://www.jonathan-petitcolas.com/2014/09/25/parsing-binary-files-in-go.h…
In any case, this is not too critical, as the tool is a separate project.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63639
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icb7481d4903f95d200eddbfed7728fbec51819d0
Gerrit-Change-Number: 63639
Gerrit-PatchSet: 14
Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Martin L Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Christian Walter <christian.walter(a)9elements.com>
Gerrit-CC: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin L Roth <martinroth(a)google.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Tue, 03 May 2022 08:44:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Mario Scheithauer.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63861 )
Change subject: soc/intel/elkhartlake: Implement TSN GbE driver
......................................................................
Patch Set 3: Code-Review+1
(2 comments)
File src/soc/intel/elkhartlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/63861/comment/a733a230_f78585ea
PS2, Line 240: TSN_GBE_DRIVER
> Renamed to EHL_TSN_DRIVER - you are right, this is EHL specific
Ack
File src/soc/intel/elkhartlake/tsn_gbe.c:
https://review.coreboot.org/c/coreboot/+/63861/comment/c4a9b289_8116f1ff
PS2, Line 15: if (!io_mem_base) {
: printk(BIOS_ERR, "TSN GbE: Error can't find I/O MEM resource\n");
: return;
: }
> With the removal of the check I must also remove the first two lines for the time being. […]
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/63861
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7522914c56b74486bb088280d2686acf7027d1d3
Gerrit-Change-Number: 63861
Gerrit-PatchSet: 3
Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Gerrit-Comment-Date: Tue, 03 May 2022 08:43:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer(a)siemens.com>
Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64008 )
Change subject: mb/google/brya/acpi: Add support for NBCI _DSM subfunction
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/google/brya/acpi/nbci.asl:
https://review.coreboot.org/c/coreboot/+/64008/comment/3fa238e8_1cba3921
PS1, Line 25: (0 << 10) | /* No 3D Hotkeys */
> is it like `10` is to represent bit10 and `0` to represent `no`? […]
Yes
--
To view, visit https://review.coreboot.org/c/coreboot/+/64008
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I19eb9417923d297a084d6f5329682e91cd506a9e
Gerrit-Change-Number: 64008
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Tue, 03 May 2022 08:35:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Paul Menzel, Ivy Jian, Michael Niewöhner.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62931 )
Change subject: mb/google/brya/: Add PEG and initial Nvidia dGPU ASL support
......................................................................
Patch Set 5: Code-Review+1
(3 comments)
Patchset:
PS2:
> Yeah we'll see how the driver ends up looking, and extend it if needed.
Sounds good
File src/mainboard/google/brya/acpi/gpu_top.asl:
https://review.coreboot.org/c/coreboot/+/62931/comment/53fb1426_e5acf0d4
PS5, Line 32: NVOP
nit: be consistent with space before parentheses (applies to all files)
File src/mainboard/google/brya/acpi/peg.asl:
https://review.coreboot.org/c/coreboot/+/62931/comment/18a6d852_5433d6c9
PS5, Line 7: 0xff
Huh, why this size?
--
To view, visit https://review.coreboot.org/c/coreboot/+/62931
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifce1610210e9636e87dda4b55c8287334adfcc42
Gerrit-Change-Number: 62931
Gerrit-PatchSet: 5
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Tue, 03 May 2022 08:34:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin, Rex-BC Chen, Julius Werner.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63926 )
Change subject: mb/google/corsola: Enable CBFS_VERIFICATION
......................................................................
Patch Set 5:
(1 comment)
File src/mainboard/google/corsola/Kconfig:
https://review.coreboot.org/c/coreboot/+/63926/comment/afcea379_8711fe72
PS5, Line 26: CBFS_VERIFICATION
> Sorry, this shouldn't be here at all. It's a user decision, not describing the hardware. […]
Done in CL:3622648.
--
To view, visit https://review.coreboot.org/c/coreboot/+/63926
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6b85ead37e1e88730fecf9d6b0259d7cad7229fa
Gerrit-Change-Number: 63926
Gerrit-PatchSet: 5
Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Tue, 03 May 2022 08:32:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment