Attention is currently required from: Jakub Czapiga, Paul Menzel, Jan Dabros.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57708 )
Change subject: libpayload: Add mock architecture
......................................................................
Patch Set 9: Code-Review+2
(2 comments)
File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/57708/comment/10454e1d_f03866bc
PS8, Line 125: MOCK
> Done
Sorry, I think you misunderstood me. I meant you should change the prompt string (on this line) to something nicer, not the help text. The prompt string is what is directly displayed in menuconfig as the name of this option (and it doesn't have to be in all-caps or anything, it can be a full string with spaces etc.). The help text is just what you see when you then select that option and press the help button.
File payloads/libpayload/arch/mock/head.c:
https://review.coreboot.org/c/coreboot/+/57708/comment/48f388ef_5175ca4c
PS8, Line 3: /* This file is empty on purpose. It should not be used. */
> It has to stay, because of the target: […]
Hmm... okay.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57708
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie3a6e6f6cad2f8a2e48a8e546d3b79c577653080
Gerrit-Change-Number: 57708
Gerrit-PatchSet: 9
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: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Thu, 07 Oct 2021 23:26:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Raul Rangel.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57628 )
Change subject: lib/thread: Switch to using CPU_INFO_V2
......................................................................
Patch Set 7:
(1 comment)
File src/lib/thread.c:
https://review.coreboot.org/c/coreboot/+/57628/comment/0ecf05b2_1f24a4dc
PS7, Line 49: return cpu_info()->thread;
(Sorry for being late, patch itself looks fine to me, just have some furthergoing theoretical questions.)
Is there actually any point in having this current thread info stored in the cpu_info()? I thought (and we only do SMP on x86 which I'm not super familiar with, so please correct me if I'm wrong) that coreboot isn't a "full" SMP system where all cores are equal -- instead most work is done on the boot CPU and the other cores only run some special CPU init code and otherwise try to stay out of the way, right? In particular, this whole cooperative multitasking framework is only supposed to run all threads on the boot CPU, and it would be illegal for any of the other CPUs to call thread_run()? (Because all that mutex stuff and everything else isn't actually written to deal with real concurrency between different cores.)
So then I think we can go further and just put current_thread in a simple global. I think before this patch, this code was only dealing with cpu_info because cpu_info was part of the stack, not because it really needs to be involved in the thread framework. The only thing that I think the non-boot CPUs do here is for when udelay() calls thread_yield_microseconds(), they have to fail on the thread_can_yield() (because current_thread() would be NULL). We can have that easier by just making an is_boot_cpu() API or something like that. It would be nice if cpu_info() could just become a completely arch/x86-specific thing and the other architectures can just hardcode something like is_boot_cpu() to 1.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57628
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5e04d254a00db43714ec60ebed7c4aa90e23190a
Gerrit-Change-Number: 57628
Gerrit-PatchSet: 7
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Eric Peers <epeers(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Comment-Date: Thu, 07 Oct 2021 23:12:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Ravi kumar.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49392 )
Change subject: sc7280: Add SHRM firmware support
......................................................................
Patch Set 79:
(1 comment)
File src/soc/qualcomm/sc7280/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/49392/comment/e90b3316_a4dd0c58
PS79, Line 110: none
> Somehow when either lz4 or lzma is used, a runtime exception is thrown as ravi mentioned in https:// […]
Oh, sorry, yeah... I only pay occasional attention to these so I missed Ravi's response. I think maybe directly decompression into the SHRM memory is the problem because it is mapped with the device memory type, and I think that only allows 4-byte aligned accesses. Maybe mapping it as cached memory (temporarily, and then flushing it before starting the SHRM) would help.
--
To view, visit https://review.coreboot.org/c/coreboot/+/49392
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I44484573a829eaefbd34907c6fe78d427506a762
Gerrit-Change-Number: 49392
Gerrit-PatchSet: 79
Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Comment-Date: Thu, 07 Oct 2021 23:03:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Shelley Chen <shchen(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: John Zhao, Paul Menzel, Nick Vaccaro.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57625 )
Change subject: soc/intel/tigerlake: Add ACPI addition for USB4/TBT latency optimization
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
+Nick to evaluate if this should go in the FW branch
--
To view, visit https://review.coreboot.org/c/coreboot/+/57625
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5a19118b75ed0a78b7436f2f90295c03928300d7
Gerrit-Change-Number: 57625
Gerrit-PatchSet: 3
Gerrit-Owner: John Zhao <john.zhao(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
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: John Zhao <john.zhao(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Comment-Date: Thu, 07 Oct 2021 22:43:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Ravi kumar.
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49392 )
Change subject: sc7280: Add SHRM firmware support
......................................................................
Patch Set 79:
(1 comment)
File src/soc/qualcomm/sc7280/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/49392/comment/232304dc_09de6b14
PS79, Line 110: none
> Why not $(CBFS_PRERAM_COMPRESS_FLAG)?
Somehow when either lz4 or lzma is used, a runtime exception is thrown as ravi mentioned in https://review.coreboot.org/c/coreboot/+/49392/comment/27e6e23f_32ba473d/
--
To view, visit https://review.coreboot.org/c/coreboot/+/49392
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I44484573a829eaefbd34907c6fe78d427506a762
Gerrit-Change-Number: 49392
Gerrit-PatchSet: 79
Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Comment-Date: Thu, 07 Oct 2021 22:40:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel.
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57625 )
Change subject: soc/intel/tigerlake: Add ACPI addition for USB4/TBT latency optimization
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/tigerlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/57625/comment/e41a6020_7ccb4ca8
PS3, Line 114: FUN9 = 1
> Technically shouldn't bit 0 be set as well?
Yes, technically agreed. But it seems not necessary as kernel already invokes function 0 (as corresponding to bit 0 set) to query other functions availability. Literally there should be no function impact with/without setting bit 0 here.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57625
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5a19118b75ed0a78b7436f2f90295c03928300d7
Gerrit-Change-Number: 57625
Gerrit-PatchSet: 3
Gerrit-Owner: John Zhao <john.zhao(a)intel.com>
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-Comment-Date: Thu, 07 Oct 2021 22:39:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Ravi kumar.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49392 )
Change subject: sc7280: Add SHRM firmware support
......................................................................
Patch Set 79:
(3 comments)
File src/soc/qualcomm/sc7280/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/49392/comment/62a12f4b_6b32da47
PS79, Line 110: none
Why not $(CBFS_PRERAM_COMPRESS_FLAG)?
File src/soc/qualcomm/sc7280/shrm_load_reset.c:
https://review.coreboot.org/c/coreboot/+/49392/comment/894210e7_31c50bb4
PS79, Line 16: if (!shrm_fw_entry)
nit: unnecessarily complicated way to write
if (!selfload(&shrm_fw_prog))
https://review.coreboot.org/c/coreboot/+/49392/comment/c12a36c6_2ac44ad5
PS79, Line 21: \n
nit: please refrain from randomly inserting extra newlines in the console unless there's actually a big "break" in the firmware flow that deserves to visually stand out (e.g. QcLib entry and exit, but not this).
--
To view, visit https://review.coreboot.org/c/coreboot/+/49392
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I44484573a829eaefbd34907c6fe78d427506a762
Gerrit-Change-Number: 49392
Gerrit-PatchSet: 79
Gerrit-Owner: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: mturney mturney <mturney(a)codeaurora.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Ravi Kumar Bokka <c_rbokka(a)qualcomm.corp-partner.google.com>
Gerrit-Attention: Ravi kumar <rbokka(a)codeaurora.org>
Gerrit-Comment-Date: Thu, 07 Oct 2021 22:27:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Hsuan-ting Chen, Tim Wawrzynczak, Nick Vaccaro, EricR Lai.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58183 )
Change subject: mb/google/brya: Add GPIO_IN_RW to all variants' early GPIO tables
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58183
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6b1af50f257dc7b627c4c00d7480ba7732c3d1a0
Gerrit-Change-Number: 58183
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 07 Oct 2021 22:26:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Tim Wawrzynczak has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/58183 )
Change subject: mb/google/brya: Add GPIO_IN_RW to all variants' early GPIO tables
......................................................................
mb/google/brya: Add GPIO_IN_RW to all variants' early GPIO tables
Before attempting another commit 6260bf71, ensure that brya's variants
all program EC_IN_RW as an input GPIO in bootblock so that it can be
read from in verstage.
Change-Id: I6b1af50f257dc7b627c4c00d7480ba7732c3d1a0
Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
---
M src/mainboard/google/brya/variants/anahera/gpio.c
M src/mainboard/google/brya/variants/brask/gpio.c
M src/mainboard/google/brya/variants/brya0/gpio.c
M src/mainboard/google/brya/variants/felwinter/gpio.c
M src/mainboard/google/brya/variants/gimble/gpio.c
M src/mainboard/google/brya/variants/kano/gpio.c
M src/mainboard/google/brya/variants/primus/gpio.c
M src/mainboard/google/brya/variants/redrix/gpio.c
M src/mainboard/google/brya/variants/taeko/gpio.c
9 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/58183/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58183
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6b1af50f257dc7b627c4c00d7480ba7732c3d1a0
Gerrit-Change-Number: 58183
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: newpatchset