Attention is currently required from: Andrey Petrov, Angel Pons, Damien Zammit, Felix Held, Huang Jin, Julius Werner, Lee Leahy, Matt DeVillier, Ronak Kanabar.
Hello Andrey Petrov, Angel Pons, Damien Zammit, Felix Held, Huang Jin, Lee Leahy, Matt DeVillier, Patrick Rudolph, Ronak Kanabar, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52933?usp=email
to look at the new patch set (#5).
Change subject: lib/romstage_handoff.c: Initialize using a cbmem hook
......................................................................
lib/romstage_handoff.c: Initialize using a cbmem hook
This reduces platform boilerplate code.
Change-Id: I93abf33a6eb055eeaba1fed5042387822de1aa20
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/drivers/amd/agesa/romstage.c
M src/drivers/intel/fsp1_1/romstage.c
M src/drivers/intel/fsp2_0/memory_init.c
M src/include/romstage_handoff.h
M src/lib/romstage_handoff.c
M src/northbridge/intel/gm45/romstage.c
M src/northbridge/intel/haswell/romstage.c
M src/northbridge/intel/i945/early_init.c
M src/northbridge/intel/ironlake/romstage.c
M src/northbridge/intel/pineview/romstage.c
M src/northbridge/intel/sandybridge/romstage.c
M src/northbridge/intel/x4x/romstage.c
M src/soc/amd/stoneyridge/romstage.c
M src/soc/intel/baytrail/romstage/romstage.c
M src/soc/intel/broadwell/romstage.c
15 files changed, 11 insertions(+), 62 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/52933/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/52933?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: I93abf33a6eb055eeaba1fed5042387822de1aa20
Gerrit-Change-Number: 52933
Gerrit-PatchSet: 5
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Huang Jin
Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Patrick Rudolph <rudolphpatrick05(a)gmail.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.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-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Damien Zammit
Gerrit-Attention: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Huang Jin
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Andrey Petrov, Arthur Heymans, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar.
Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email )
Change subject: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
......................................................................
Patch Set 3:
(1 comment)
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/80328/comment/d84861cc_877ecb42 :
PS1, Line 156: goto temp_ram_assigned;
> goto is considered bad taste. Just add an if clause below to skip the UPD setup. […]
For this change, we are not inclined to name it as a workaround. Since the static allocation is reflected into UPD's default value, and it is an existing practice for coreboot to use UPD's default values, e.g. https://github.com/coreboot/coreboot/blob/main/src/drivers/intel/fsp2_0/mem….
--
To view, visit https://review.coreboot.org/c/coreboot/+/80328?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: I17a73e4f627c91808ed17c712b72937662ae9293
Gerrit-Change-Number: 80328
Gerrit-PatchSet: 3
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Fri, 02 Feb 2024 10:43:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Felix Held.
Varshit Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80287?usp=email )
Change subject: vc/amd/opensil/genoa_poc: remove xSIM-api dependency from opensil.h
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/80287?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: I969bc0862560b7254c48f04e9a03387417f328bc
Gerrit-Change-Number: 80287
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 02 Feb 2024 10:29:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Varshit Pandya.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80287?usp=email )
Change subject: vc/amd/opensil/genoa_poc: remove xSIM-api dependency from opensil.h
......................................................................
Patch Set 1:
(1 comment)
File src/vendorcode/amd/opensil/genoa_poc/ramstage.c:
https://review.coreboot.org/c/coreboot/+/80287/comment/5c92bb1f_d27c440d :
PS1, Line 178: /* TODO: also call timepoints 2 and 3 from coreboot. Are they NOOP? */
> Can this be removed now ?
no, this shouldn't be removed yet, since coreboot isn't calling time points 2 and 3. this patch just adds one function to call for each timepoint
--
To view, visit https://review.coreboot.org/c/coreboot/+/80287?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: I969bc0862560b7254c48f04e9a03387417f328bc
Gerrit-Change-Number: 80287
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Comment-Date: Fri, 02 Feb 2024 10:25:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov, Arthur Heymans, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar.
Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email )
Change subject: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/80328/comment/79cffe79_6a49a958 :
PS3, Line 8:
> > Need to justify your change. It does neither describe a problem nor why this change is necessary. […]
Yeah, it is for the 2S issue. https://review.coreboot.org/c/coreboot/+/61164 cannot pass boot in our internal system and hence we need to have a further solution on this.
The real reason behind is that SPR FSP will do static allocation for temp ram and bootloader should align the mapping accordingly, otherwise data will be corrupted mutually. CR61164 is not reaching to fully alignment and hence still cause issue in some machines.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80328?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: I17a73e4f627c91808ed17c712b72937662ae9293
Gerrit-Change-Number: 80328
Gerrit-PatchSet: 3
Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Fri, 02 Feb 2024 10:23:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Stefan Reinauer, c.koehne(a)beckhoff.com.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/em100/+/80248?usp=email )
Change subject: em100: free device list when calling em100 --list
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/em100/+/80248?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: em100
Gerrit-Branch: main
Gerrit-Change-Id: Id1849bb69d534928016256dd01e9550ef414d330
Gerrit-Change-Number: 80248
Gerrit-PatchSet: 2
Gerrit-Owner: c.koehne(a)beckhoff.com
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: c.koehne(a)beckhoff.com
Gerrit-Comment-Date: Fri, 02 Feb 2024 10:22:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
c.koehne(a)beckhoff.com has posted comments on this change. ( https://review.coreboot.org/c/em100/+/80250?usp=email )
Change subject: em100: return an error if a command fails
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/em100/+/80250/comment/01095c16_cf16e2df :
PS1, Line 9: We don't properly return an error in every code path when a command fails. This
: makes it hard to use this tool in a shell script because you can't be sure that
: your command was successful.
:
> Please match 72 characters line limit
Done
--
To view, visit https://review.coreboot.org/c/em100/+/80250?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: em100
Gerrit-Branch: main
Gerrit-Change-Id: I2554252d007a92e939088389d67f9d4cede837ee
Gerrit-Change-Number: 80250
Gerrit-PatchSet: 2
Gerrit-Owner: c.koehne(a)beckhoff.com
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 02 Feb 2024 10:21:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <czapiga(a)google.com>
Gerrit-MessageType: comment
c.koehne(a)beckhoff.com has posted comments on this change. ( https://review.coreboot.org/c/em100/+/80249?usp=email )
Change subject: em100: exit when using an invalid device name
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/em100/+/80249/comment/e6ceb713_dc25bdf6 :
PS1, Line 9: If you use a stupid device name like `em100 --device bla`, we won't exit with
: an error right now. It just ignores the devices and uses the first device it
: can find. This could lead to unexpected issues when connecting more than one
: EM100 device to your machine. It's much better to exit if we're unable to parse
: the device name.
> Please match 72 characters line limit
Done
--
To view, visit https://review.coreboot.org/c/em100/+/80249?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: em100
Gerrit-Branch: main
Gerrit-Change-Id: If0f7b8b576224a9db6ab319c2212bc245110c5c1
Gerrit-Change-Number: 80249
Gerrit-PatchSet: 2
Gerrit-Owner: c.koehne(a)beckhoff.com
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 02 Feb 2024 10:21:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <czapiga(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Jakub Czapiga, Stefan Reinauer.
c.koehne(a)beckhoff.com has posted comments on this change. ( https://review.coreboot.org/c/em100/+/80248?usp=email )
Change subject: em100: free device list when calling em100 --list
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/em100/+/80248/comment/2082739a_ae5a95b3 :
PS1, Line 9: After querying the device list by calling libusb_get_device_list, you have to
: free the list with lib_usb_free_device_list to properly clean up the resources
: allocated by libusb. This solves some warnings issued by libusb when calling
: `em100 --list`.
> Please wrap these lines to 72 characters to conform to git's commit message standard
Done
--
To view, visit https://review.coreboot.org/c/em100/+/80248?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: em100
Gerrit-Branch: main
Gerrit-Change-Id: Id1849bb69d534928016256dd01e9550ef414d330
Gerrit-Change-Number: 80248
Gerrit-PatchSet: 2
Gerrit-Owner: c.koehne(a)beckhoff.com
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Fri, 02 Feb 2024 10:20:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <czapiga(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov, Angel Pons, Damien Zammit, Felix Held, Huang Jin, Julius Werner, Lee Leahy, Matt DeVillier, Ronak Kanabar.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52933?usp=email )
Change subject: lib/romstage_handoff.c: Initialize using a cbmem hook
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52933/comment/11711ba8_c1efe003 :
PS2, Line 9: boilplate
> boil*er*plate
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/52933?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: I93abf33a6eb055eeaba1fed5042387822de1aa20
Gerrit-Change-Number: 52933
Gerrit-PatchSet: 4
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Huang Jin
Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Patrick Rudolph <rudolphpatrick05(a)gmail.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.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-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Damien Zammit
Gerrit-Attention: Lee Leahy <leroy.p.leahy(a)intel.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Huang Jin
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 02 Feb 2024 10:20:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment