coreboot-gerrit September 2021

coreboot-gerrit@coreboot.org

1 participants
6530 discussions

Change in coreboot[master]: mb/intel/leafhill,minnow3: remove ADD_FSP_BINARIES config override
by Nico Huber (Code Review) 05 Sep '21

05 Sep '21

Attention is currently required from: Martin Roth, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57346 ) Change subject: mb/intel/leafhill,minnow3: remove ADD_FSP_BINARIES config override ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > that should work perfectly fine with the clutter here. It looks like the Sorry, I meant "without the clutter". -- To view, visit https://review.coreboot.org/c/coreboot/+/57346 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I23439f3134eef9460625addbff7efd64c5f65ae5 Gerrit-Change-Number: 57346 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sat, 04 Sep 2021 22:43:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/intel/leafhill,minnow3: remove ADD_FSP_BINARIES config override
by Nico Huber (Code Review) 05 Sep '21

05 Sep '21

Attention is currently required from: Martin Roth, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57346 ) Change subject: mb/intel/leafhill,minnow3: remove ADD_FSP_BINARIES config override ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: > having that overridable might be useful, but that should be done at the fsp driver Kconfig level and […] Well, if somebody wants to override things from site-local/ or .config, that should work perfectly fine with the clutter here. It looks like the author was paid per added line and so they just copied more things as long as it still built. It actually seems like we could drop everything below UART_FOR_CONSOLE. -- To view, visit https://review.coreboot.org/c/coreboot/+/57346 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I23439f3134eef9460625addbff7efd64c5f65ae5 Gerrit-Change-Number: 57346 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sat, 04 Sep 2021 22:42:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] kconfig_lint: Drop overly restrictive rules about choice configs
by Nico Huber (Code Review) 05 Sep '21

05 Sep '21

Attention is currently required from: Felix Singer, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56410 ) Change subject: [RFC] kconfig_lint: Drop overly restrictive rules about choice configs ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > See Martin's comment. IIRC, one of his points was covered by the current […] Probably worth to check if Kconfig doesn't check it by itself by now. -- To view, visit https://review.coreboot.org/c/coreboot/+/56410 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I48a17f6403470251be6b6d44bb82a8bdcbefe9f6 Gerrit-Change-Number: 56410 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 04 Sep 2021 22:34:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] kconfig_lint: Drop overly restrictive rules about choice configs
by Nico Huber (Code Review) 05 Sep '21

05 Sep '21

Attention is currently required from: Felix Singer, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56410 ) Change subject: [RFC] kconfig_lint: Drop overly restrictive rules about choice configs ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Are there any more todos left? I would like to get CB:56217 into the tree. See Martin's comment. IIRC, one of his points was covered by the current code and shouldn't regress. -- To view, visit https://review.coreboot.org/c/coreboot/+/56410 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I48a17f6403470251be6b6d44bb82a8bdcbefe9f6 Gerrit-Change-Number: 56410 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 04 Sep 2021 22:33:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: configs/config.google_meep_cros: don't select ADD_FSP_BINARIES
by Felix Held (Code Review) 04 Sep '21

04 Sep '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/57351 ) Change subject: configs/config.google_meep_cros: don't select ADD_FSP_BINARIES ...................................................................... configs/config.google_meep_cros: don't select ADD_FSP_BINARIES This config selected ADD_FSP_BINARIES even though HAVE_INTEL_FSP_REPO is only defined for Apollolake and not Geminilake that resides in the same SoC directory and uses the same Kconfig file. This results in the paths to the FSP binaries not being defined, in which case the ADD_FSP_BINARIES option shouldn't be selected. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I95123c4930b44a3b76c87768e130eb7359bbf625 Reviewed-on: https://review.coreboot.org/c/coreboot/+/57351 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Martin Roth <martinroth(a)google.com> --- M configs/config.google_meep_cros 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved diff --git a/configs/config.google_meep_cros b/configs/config.google_meep_cros index 9911614..acb0e4c 100644 --- a/configs/config.google_meep_cros +++ b/configs/config.google_meep_cros @@ -22,7 +22,6 @@ CONFIG_ELOG_BOOT_COUNT_CMOS_OFFSET=144 # Firmware Support Package -CONFIG_ADD_FSP_BINARIES=y # CONFIG_RUN_FSP_GOP is not set # Management Engine -- To view, visit https://review.coreboot.org/c/coreboot/+/57351 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I95123c4930b44a3b76c87768e130eb7359bbf625 Gerrit-Change-Number: 57351 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso: select ADD_FSP_BINARIES if USE_AMD_BLOBS is selected
by Felix Held (Code Review) 04 Sep '21

04 Sep '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/57348 ) Change subject: soc/amd/picasso: select ADD_FSP_BINARIES if USE_AMD_BLOBS is selected ...................................................................... soc/amd/picasso: select ADD_FSP_BINARIES if USE_AMD_BLOBS is selected Since the FSP binaries for Picasso are present in the amd_blobs repo, select the ADD_FSP_BINARIES option if the Kconfig option to check out that repo is set. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I9a8571730cf271ad5e113e5df87700882b3c5475 Reviewed-on: https://review.coreboot.org/c/coreboot/+/57348 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Martin Roth <martinroth(a)google.com> --- M src/soc/amd/picasso/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index addff5e..473f316 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -18,6 +18,7 @@ select X86_AMD_FIXED_MTRRS select X86_AMD_INIT_SIPI select ACPI_SOC_NVS + select ADD_FSP_BINARIES if USE_AMD_BLOBS select DRIVERS_I2C_DESIGNWARE select DRIVERS_USB_PCI_XHCI select GENERIC_GPIO_LIB -- To view, visit https://review.coreboot.org/c/coreboot/+/57348 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9a8571730cf271ad5e113e5df87700882b3c5475 Gerrit-Change-Number: 57348 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: elogtool: add argc / argv support for commands
by Furquan Shaikh (Code Review) 04 Sep '21

04 Sep '21

1 0

Change in coreboot[master]: elogtool: add next_available_event_offset function
by Furquan Shaikh (Code Review) 04 Sep '21

04 Sep '21

Attention is currently required from: Ricardo Quesada, Jack Rosenthal. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57395 ) Change subject: elogtool: add next_available_event_offset function ...................................................................... Patch Set 1: (2 comments) File util/cbfstool/elogtool.c: https://review.coreboot.org/c/coreboot/+/57395/comment/56f89b8a_54ce9e29 PS1, Line 118: static int next_available_event_offset(const struct buffer *buf) : { : const struct event_header *event; : int offset = sizeof(struct elog_header); : : event = buffer_get(buf) + offset; : while ((const void *)(event) < buffer_end(buf)) { : if (event->type == ELOG_TYPE_EOL || event->length == 0) : break; : : offset += event->length; : assert((size_t)offset <= buffer_size(buf) && "Out of bounds"); : event = elog_get_next_event(event); : } : return offset; : } Couple of things: 1. Instead of taking the input buffer as is, if this function takes a buffer that points to the first event, things can be somewhat simplified for this function and the caller. 2. The math for adjusting offset can be dropped (almost) completely by using buffer operations. I am thinking something like this: ``` static size_t next_available_event_offset(const struct buffer *buf) { » const struct event_header *event; » struct buffer copy, *iter = © » while (buffer_size(iter) >= sizeof(struct event_header)) { » » event = buffer_get(iter); » » if (event->type == ELOG_TYPE_EOL || event->length == 0) » » » break; » » assert(event->length <= buffer_size(iter)); » » buffer_seek(event->length); » } » return buffer_offset(iter) - buffer_offset(buf); } ``` What do you think? https://review.coreboot.org/c/coreboot/+/57395/comment/976672f0_16fafd53 PS1, Line 172: memset(start + sizeof(struct elog_header), ELOG_TYPE_EOL, : buffer_size(b) - sizeof(struct elog_header)); Continuing the comment above, this function can be reorganized as: ``` static int cmd_clear(const struct buffer *b) { » struct buffer buf; » size_t used_data_size; » buffer_clone(&buf, b); » buffer_seek(&buf, sizeof(struct elog_header)); » used_data_size = next_available_event_offset(&buf); » memset(buffer_get(&buf), ELOG_TYPE_EOL, buffer_size(&buf)); » [...] } ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/57395 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0d8ecc893675758d7f90845282a588d367b55567 Gerrit-Change-Number: 57395 Gerrit-PatchSet: 1 Gerrit-Owner: Ricardo Quesada <ricardoq(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ricardo Quesada <ricardoq(a)google.com> Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Comment-Date: Sat, 04 Sep 2021 15:41:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: elog: move MAX_EVENT_SIZE to commonlib/bsd/include
by Furquan Shaikh (Code Review) 04 Sep '21

04 Sep '21

1 0

Change in coreboot[master]: elogtool: add "clear" command
by Furquan Shaikh (Code Review) 04 Sep '21

04 Sep '21

Attention is currently required from: Ricardo Quesada, Jack Rosenthal. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56883 ) Change subject: elogtool: add "clear" command ...................................................................... Patch Set 15: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/56883/comment/17abe1c0_55e41c7d PS14, Line 26: logtool clear && elogtool list : elogtool clear -f invalid.raw : elogtool clear -f valid.raw > for future... […] +1 -- To view, visit https://review.coreboot.org/c/coreboot/+/56883 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia28a6eb34c82103ab078a0841b022e2e5e430585 Gerrit-Change-Number: 56883 Gerrit-PatchSet: 15 Gerrit-Owner: Ricardo Quesada <ricardoq(a)google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: Ricardo Quesada <ricardoq(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ricardo Quesada <ricardoq(a)google.com> Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Comment-Date: Sat, 04 Sep 2021 14:24:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2021