coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: lib/thread: Make thread_run not block the current state
by Raul Rangel (Code Review) 16 Jul '21

16 Jul '21

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56351 ) Change subject: lib/thread: Make thread_run not block the current state ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: > Yeah, I've been wondering about this too. It's probably a good idea. […] Sounds like a good idea. File src/lib/thread.c: https://review.coreboot.org/c/coreboot/+/56351/comment/9bb2324d_30a993fc PS1, Line 325: printk(BIOS_ERR, "thread_run() No more threads!\n"); > This is a bit off-topic for this patch, but I'm wondering if the better answer here would be to wait […] I agree that waiting is a better approach. I'll shoot out a CL. -- To view, visit https://review.coreboot.org/c/coreboot/+/56351 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e5b0aed70385ddcd23ffcf7b063f8ccb547fc05 Gerrit-Change-Number: 56351 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Thu, 15 Jul 2021 23:08:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [NOTFORMERGE] soc/amd/picasso: test case for mpinit-problem on mandolin
by Felix Held (Code Review) 16 Jul '21

16 Jul '21

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56355 ) Change subject: [NOTFORMERGE] soc/amd/picasso: test case for mpinit-problem on mandolin ...................................................................... [NOTFORMERGE] soc/amd/picasso: test case for mpinit-problem on mandolin When printing a lot of stuff from the CPU init on BSP and all APs to the serial console, the BSP will end up loading and running SeaBIOS before the mpinit is done and before post_mp_init gets called which results in SeaBIOS getting stuck when it tries to run the VGA option rom. Tested this on mandolin, but this will likely be a problem on all Picasso and Cezanne boards. When removing the unconditional mca_print_error() call added in this test case, the added debug print will show up on the console. When the unconditional mca_print_error call is in there, the debug print in post_mp_init won't show up on the serial console and the last message on the serial console is: Running option rom at c000:0003 BUG=b:193809448 Change-Id: I48a3d820fcf9c26649e2385ea0caaef7d23ca572 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/common/block/cpu/mca/mca_common.c M src/soc/amd/picasso/cpu.c 2 files changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/56355/1 diff --git a/src/soc/amd/common/block/cpu/mca/mca_common.c b/src/soc/amd/common/block/cpu/mca/mca_common.c index b357691..cdad945 100644 --- a/src/soc/amd/common/block/cpu/mca/mca_common.c +++ b/src/soc/amd/common/block/cpu/mca/mca_common.c @@ -18,6 +18,7 @@ return; for (unsigned int i = 0 ; i < num_banks ; i++) { + mca_print_error(i); if (!mca_is_valid_bank(i)) continue; diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c index b04d004..fee502e 100644 --- a/src/soc/amd/picasso/cpu.c +++ b/src/soc/amd/picasso/cpu.c @@ -40,6 +40,7 @@ static void post_mp_init(void) { + printk(BIOS_INFO, "post mp init smi enable\n\n\n"); global_smi_enable(); apm_control(APM_CNT_SMMINFO); } -- To view, visit https://review.coreboot.org/c/coreboot/+/56355 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I48a3d820fcf9c26649e2385ea0caaef7d23ca572 Gerrit-Change-Number: 56355 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/common/apob: Add support for asynchronously reading APOB_NV
by Julius Werner (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56232 ) Change subject: soc/amd/common/apob: Add support for asynchronously reading APOB_NV ...................................................................... Patch Set 7: Code-Review+1 (1 comment) File src/soc/amd/common/block/apob/apob_cache.c: https://review.coreboot.org/c/coreboot/+/56232/comment/6aca2840_89ca3c46 PS6, Line 71: if (fmap_locate_area(DEFAULT_MRC_CACHE, r) < 0) { > I think the reason we do this is because we start with the ro rdev, and if we need to update the SPI […] I would say just use the RW rdev right away in that case, or just look up the FMAP section again. Either option sounds better than this. -- To view, visit https://review.coreboot.org/c/coreboot/+/56232 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I930d58b76eb4558bc4f48ed928c4d6538fefb1e5 Gerrit-Change-Number: 56232 Gerrit-PatchSet: 7 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Eric Peers <epeers(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 15 Jul 2021 22:57:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/herobrine: Retrieve SKU ID from EC
by Philip Chen (Code Review) 16 Jul '21

16 Jul '21

Philip Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56354 ) Change subject: mb/google/herobrine: Retrieve SKU ID from EC ...................................................................... mb/google/herobrine: Retrieve SKU ID from EC BUG=b:186264627 BRANCH=none TEST=build herobrine Signed-off-by: Philip Chen <philipchen(a)google.com> Change-Id: Id3faf7af64c0129ec646a01085adc43b561225d2 --- M src/mainboard/google/herobrine/Kconfig M src/mainboard/google/herobrine/boardid.c 2 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/56354/1 diff --git a/src/mainboard/google/herobrine/Kconfig b/src/mainboard/google/herobrine/Kconfig index 7f88b94..38bb328 100644 --- a/src/mainboard/google/herobrine/Kconfig +++ b/src/mainboard/google/herobrine/Kconfig @@ -9,6 +9,7 @@ select COMMON_CBFS_SPI_WRAPPER select EC_GOOGLE_CHROMEEC if !BOARD_GOOGLE_SENOR select EC_GOOGLE_CHROMEEC_RTC if !BOARD_GOOGLE_SENOR + select EC_GOOGLE_CHROMEEC_SKUID if !BOARD_GOOGLE_SENOR select EC_GOOGLE_CHROMEEC_SPI if !BOARD_GOOGLE_SENOR select RTC if !BOARD_GOOGLE_SENOR select MISSING_BOARD_RESET if BOARD_GOOGLE_SENOR diff --git a/src/mainboard/google/herobrine/boardid.c b/src/mainboard/google/herobrine/boardid.c index eecbeab..6368d25 100644 --- a/src/mainboard/google/herobrine/boardid.c +++ b/src/mainboard/google/herobrine/boardid.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <boardid.h> +#include <ec/google/chromeec/ec.h> #include <gpio.h> uint32_t board_id(void) @@ -19,7 +20,5 @@ uint32_t sku_id(void) { - static uint32_t id = UNDEFINED_STRAPPING_ID; - - return id; + return google_chromeec_get_board_sku(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/56354 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id3faf7af64c0129ec646a01085adc43b561225d2 Gerrit-Change-Number: 56354 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Chen <philipchen(a)google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/common/block/lpc/spi_dma: Implement SPI DMA functionality
by Raul Rangel (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Jason Glenesk, Furquan Shaikh, Marshall Dawson, Julius Werner, Eric Peers, Karthik Ramasubramanian, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56228 ) Change subject: soc/amd/common/block/lpc/spi_dma: Implement SPI DMA functionality ...................................................................... Patch Set 6: (1 comment) File src/soc/amd/common/block/lpc/Kconfig: https://review.coreboot.org/c/coreboot/+/56228/comment/9aba2730_d8195605 PS6, Line 15: The SPI DMA controller : is only functional on Cezanne, Renoir or later SoCs. > Rather than putting this in the help text, why not model it programmatically with depends on? I didn't want to have to update it for each new SOC. I guess I could do a `depends on !SOC_AMD_PICASSO and !SOC_AMD_STONEYRIDGE`. -- To view, visit https://review.coreboot.org/c/coreboot/+/56228 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0be555956581fd82bbe1482d8afa8828c61aaa01 Gerrit-Change-Number: 56228 Gerrit-PatchSet: 6 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Peers <epeers(a)google.com> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 15 Jul 2021 22:57:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/thread: Move thread_run and thread_run_until outside of #if guard
by Julius Werner (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Raul Rangel. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56352 ) Change subject: lib/thread: Move thread_run and thread_run_until outside of #if guard ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: Ahh, here you did it already. Excellent! Ignore my comment in the earlier CL. -- To view, visit https://review.coreboot.org/c/coreboot/+/56352 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If9983fca939c8a15fa570481bfe016a388458830 Gerrit-Change-Number: 56352 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Comment-Date: Thu, 15 Jul 2021 22:53:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/thread: Make thread_run not block the current state
by Julius Werner (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Raul Rangel. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56351 ) Change subject: lib/thread: Make thread_run not block the current state ...................................................................... Patch Set 1: Code-Review+2 (2 comments) Patchset: PS1: Yeah, I've been wondering about this too. It's probably a good idea. My only concern is that there's nothing really preventing your thread from running over the end of the stage in that case. It might be a good idea to add a check somewhere in payload_run() to make sure there are no more running threads in the background. File src/lib/thread.c: https://review.coreboot.org/c/coreboot/+/56351/comment/58a6f291_baa1d756 PS1, Line 325: printk(BIOS_ERR, "thread_run() No more threads!\n"); This is a bit off-topic for this patch, but I'm wondering if the better answer here would be to wait and yield until a new thread slot becomes available. With the way your thread_join() and the automatic termination and the storing the result in the thread_handle works now, it would be easy to accidentally spawn more threads over the course of the ramstage than CONFIG_NUM_THREADS, and it might be fine during testing because one of the threads terminates (even though its thread_join() hasn't been reached yet) before the other one starts. But then later patches or other external factors might introduce a tiny timing change, and suddenly that thread does not terminate before the next one tries to run. We would want to make sure that the system doesn't just fail to boot in that case (which is what returning -1 here would probably do most of the time), so I think printing a BIOS_WARNING and waiting until a thread slot frees up would be the best option. -- To view, visit https://review.coreboot.org/c/coreboot/+/56351 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e5b0aed70385ddcd23ffcf7b063f8ccb547fc05 Gerrit-Change-Number: 56351 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Comment-Date: Thu, 15 Jul 2021 22:53:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: payloads: FILO: Hook up autoboot options
by Paul Menzel (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Nico Huber, Martin Roth, Stefan Reinauer. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56213 ) Change subject: payloads: FILO: Hook up autoboot options ...................................................................... Patch Set 4: (2 comments) Patchset: PS3: > Your call. Done File payloads/external/FILO/Kconfig: https://review.coreboot.org/c/coreboot/+/56213/comment/bf8abb18_3985455c PS3, Line 29: hda1 > I find it generally hard to predict what people actually need. Is there a […] I left it, and put the line as an example. -- To view, visit https://review.coreboot.org/c/coreboot/+/56213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id167e9a144bf466da87469108002672b299b702a Gerrit-Change-Number: 56213 Gerrit-PatchSet: 4 Gerrit-Owner: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Thu, 15 Jul 2021 22:42:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/thread: Add thread_handle
by Julius Werner (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Raul Rangel, Eric Peers, Felix Held. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56229 ) Change subject: lib/thread: Add thread_handle ...................................................................... Patch Set 6: Code-Review+2 (1 comment) File src/include/thread.h: https://review.coreboot.org/c/coreboot/+/56229/comment/d9e7c639_e3e07333 PS5, Line 111: return CB_ERR; > I moved it out of the #if guards. This will result in a linker error. […] Sounds good! In fact, would probably be good to do that for thread_run() and variations too. -- To view, visit https://review.coreboot.org/c/coreboot/+/56229 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie6f64d0c5a5acad4431a605f0b0b5100dc5358ff Gerrit-Change-Number: 56229 Gerrit-PatchSet: 6 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Peers <epeers(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 15 Jul 2021 22:37:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/thread: Add mutex
by Julius Werner (Code Review) 16 Jul '21

16 Jul '21

Attention is currently required from: Raul Rangel, Eric Peers, Felix Held. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56230 ) Change subject: lib/thread: Add mutex ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56230 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ife1ac95ec064ebcdd00fcaacec37a06ac52885ff Gerrit-Change-Number: 56230 Gerrit-PatchSet: 6 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Peers <epeers(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 15 Jul 2021 22:32:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021