coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[XS] Change in coreboot[main]: mb/google/brox: Configure I2C timing for I2C devices
by Shelley Chen (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Ivy Jian. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81348?usp=email ) Change subject: mb/google/brox: Configure I2C timing for I2C devices ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81348?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: I79af4fde4eb08d4eb896794756a633701bebb755 Gerrit-Change-Number: 81348 Gerrit-PatchSet: 1 Gerrit-Owner: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Wed, 20 Mar 2024 16:39:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/brox: Configure I2C timing for I2C devices
by Karthik Ramasubramanian (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Ivy Jian, Shelley Chen. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81348?usp=email ) Change subject: mb/google/brox: Configure I2C timing for I2C devices ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81348?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: I79af4fde4eb08d4eb896794756a633701bebb755 Gerrit-Change-Number: 81348 Gerrit-PatchSet: 1 Gerrit-Owner: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Ivy Jian <ivy.jian(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Wed, 20 Mar 2024 16:11:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/intel/common/block/fast_spi: probe for 2nd flash component
by Nico Huber (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Maciej Pijanowski, Paul Menzel. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80608?usp=email ) Change subject: soc/intel/common/block/fast_spi: probe for 2nd flash component ...................................................................... Patch Set 4: Code-Review+1 (3 comments) Patchset: PS4: Code change looks ok. But I don't know why the code uses SFDP in general... File src/soc/intel/common/block/fast_spi/fast_spi_def.h: https://review.coreboot.org/c/coreboot/+/80608/comment/87a71010_ce0f62ac : PS4, Line 156: #define SPIBAR_PTINX_IDX_MASK 0xffc Why drop it? File src/soc/intel/common/block/fast_spi/fast_spi_flash.c: https://review.coreboot.org/c/coreboot/+/80608/comment/c6ad75c8_3d49ae1e : PS4, Line 308: * SFDP table. FLCOMP.C0DEN is no longer used by the Flash Controller. This seems a little odd, the code was added in 2017. SPI guides up to today still document C0DEN/C1DEN. Even state that they are used to decide if VSCC0 or VSCC1 parameters should be used. I guess it doesn't matter if the descriptor and SFDP report the same. But if they don't, we'd have to know for sure which values are used in hardware. Flashprog seems happy to read the descriptor via FDO btw. It looks like this was added in 2017 by CB:18557 which I can't access. I couldn't find any document of that time that would match the reference above. -- To view, visit https://review.coreboot.org/c/coreboot/+/80608?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: I8d7449f9e1470dc234fe5ba5217d3ce4c142b49c Gerrit-Change-Number: 80608 Gerrit-PatchSet: 4 Gerrit-Owner: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 20 Mar 2024 15:47:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: mainboard/qemu-riscv: Add PCI support
by Alper Nebi Yasak (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Arthur Heymans, Felix Held, Philipp Hug, ron minnich. Alper Nebi Yasak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80378?usp=email ) Change subject: mainboard/qemu-riscv: Add PCI support ...................................................................... Patch Set 2: (6 comments) File src/mainboard/emulation/qemu-riscv/include/mainboard/addressmap.h: https://review.coreboot.org/c/coreboot/+/80378/comment/c3830659_ac7cf9a5 : PS2, Line 11: #define QEMU_VIRT_PCIE_LOW_MMIO_SIZE 0x40000000 This looks odd compared to others in the `(1ULL * GiB)` form, but it's like that in the QEMU code as well (and not a `#define` there). File src/mainboard/emulation/qemu-riscv/include/mainboard/addressmap.h: https://review.coreboot.org/c/coreboot/+/80378/comment/aac41625_8fa09da1 : PS1, Line 12: #define QEMU_VIRT_PCIE_ECAM_BASE 0x30000000 : #define QEMU_VIRT_PCIE_ECAM_SIZE 0x3fffffff > probably not needed, since there are CONFIG_ECAM_MMCONF_BASE_ADDRESS and CONFIG_ECAM_MMCONF_LENGTH. […] Yeah, these `QEMU_VIRT_PCIE_ECAM_BASE/SIZE` defines are here because I've mimicked how qemu-aarch64 does it (CB:75925). The second one should've been a size of `(256 * MiB)` instead of that address. I removed them and switched to `mmconf_resource()`. https://review.coreboot.org/c/coreboot/+/80378/comment/e6eec1de_1c5a99d1 : PS1, Line 15: #define QEMU_VIRT_PCIE_HIGH_MMIO_BASE 0x300000000ULL > this is correct for rv32, but not for rv64, the base depends on the selected amount of memory. […] Thanks, looks like I got confused a lot here. The QEMU code also aligns it to `VIRT64_HIGH_PCIE_MMIO_SIZE` just after the calculation you quoted, so I think the (unaligned) value shouldn't work at all for RV64. I updated the patch to calculate the MMIO base in `read_resources()` based on `cbmem_top()`. Hopefully correctly, but having bochs-display work with the wrong values is making me less confident in relying on my testing. File src/mainboard/emulation/qemu-riscv/mainboard.c: https://review.coreboot.org/c/coreboot/+/80378/comment/4150fb70_4e611187 : PS1, Line 47: mmio_range(dev, index++, QEMU_VIRT_PCIE_ECAM_BASE, QEMU_VIRT_PCIE_ECAM_SIZE); > can mmconf_resource be used instead? That appears to work fine (also on ARM64), at least enough to get bochs-display working with my other patches. File src/mainboard/emulation/qemu-riscv/mainboard.c: https://review.coreboot.org/c/coreboot/+/80378/comment/f9ec9490_6b78e6de : PS2, Line 28: res->limit = 0xffff; I'm having some trouble with this on RV32: ``` [INFO ] === Resource allocator: DOMAIN: 00000000 - Pass 1 (relative placement) === [INFO ] === Resource allocator: DOMAIN: 00000000 - Pass 2 (allocating resources) === [DEBUG] DOMAIN: 00000000 io: base: 0 size: 0 align: 0 gran: 65535 limit: 800bbdb800000000<NULL> [INFO ] DOMAIN: 00000000: Resource ranges: [INFO ] * Base: 3b000000000, Size: 10000000000, Tag: 802a5e74 [INFO ] * Base: fc2000000000, Size: 10000000000, Tag: 40000000 [DEBUG] PCI: 00:00:03.0 10 * [0xffff00000000 - 0xffff00000000] limit: 800bd29c00000000 [DEBUG] DOMAIN: 00000000 io: base: 0 size: 0 align: 0 gran: 65535 limit: 800bd9e000000000<NULL> [DEBUG] DOMAIN: 00000000 mem: base: 0 size: 0 align: 0 gran: 2147483647 limit: 800bbdb800000000<NULL> [DEBUG] DOMAIN: 00000000 mem: base: 3 size: 0 align: 0 gran: 4294967295 limit: 800bbdb800000003=== Resource allocator: %s - Pass 2 (allocating resources) === ``` If I set `res->size = 0xffff` as well, I see it changes `base` in the message above instead. I tried putting a few `printk(BIOS_DEBUG, "limit: %llx\n", res->limit)` in `print_domain_res()` but the value is different every time. I can't figure out how, some confusion about the width of a type or argument order, causing a shift? Would be nice if someone could take a deeper look. But somehow bochs-display works regardless of all that, so I didn't notice this in the first patchset. https://review.coreboot.org/c/coreboot/+/80378/comment/2f274922_9dbcdccd : PS2, Line 42: res->base = ALIGN_UP((uintptr_t)_dram + (uintptr_t)cbmem_top(), `cbmem_top()` via probing RAM is limited to detecting up to 16 GiB after CB:36486 even if there's more, where this could result in the wrong value. We could take the amount of RAM from device-tree, but there's PCI details there as well which might be preferable if we could decipher them. -- To view, visit https://review.coreboot.org/c/coreboot/+/80378?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: Ie57b35620af82c681d1f0d78fa8e514e4df0d2ac Gerrit-Change-Number: 80378 Gerrit-PatchSet: 2 Gerrit-Owner: Alper Nebi Yasak <alpernebiyasak(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 20 Mar 2024 15:36:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Philipp Hug <philipp(a)hug.cx> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: amdfwtool: Move linking BHD2 to PSP2 from main to link funcion
by Bao Zheng (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Felix Held, Zheng Bao. Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81334?usp=email ) Change subject: amdfwtool: Move linking BHD2 to PSP2 from main to link funcion ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81334/comment/a1c9c862_02d4d3f5 : PS1, Line 11: > TEST=Identical test on all AMD SOC platform Done -- To view, visit https://review.coreboot.org/c/coreboot/+/81334?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: Ia549a0d08c2a60b8858440543ac8d8b5259017dd Gerrit-Change-Number: 81334 Gerrit-PatchSet: 2 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Zheng Bao Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 20 Mar 2024 15:17:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: amdfwtool: Move linking BHD2 to PSP2 from main to link funcion
by Bao Zheng (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Felix Held, Zheng Bao. Hello Felix Held, Zheng Bao, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81334?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+2 by Felix Held, Verified+1 by build bot (Jenkins) Change subject: amdfwtool: Move linking BHD2 to PSP2 from main to link funcion ...................................................................... amdfwtool: Move linking BHD2 to PSP2 from main to link funcion Move the complexity from main to function, so the main flow is easy to understand. TEST=Identical test on all AMD SOC platform Change-Id: Ia549a0d08c2a60b8858440543ac8d8b5259017dd Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> --- M util/amdfwtool/amdfwtool.c 1 file changed, 16 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/81334/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81334?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: Ia549a0d08c2a60b8858440543ac8d8b5259017dd Gerrit-Change-Number: 81334 Gerrit-PatchSet: 2 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Zheng Bao Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[NoOp] Change in coreboot[main]: mb/google/zork: Update APCB to increase UMA size to 128MB
by Jason Glenesk (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Jason Nien, Martin Roth, Matt DeVillier. Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81364?usp=email ) Change subject: mb/google/zork: Update APCB to increase UMA size to 128MB ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81364?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: I8c6487a4cb8155f826d20fd3ceca87859829199c Gerrit-Change-Number: 81364 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Wed, 20 Mar 2024 15:12:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: arch/arm64: Add Clang as supported target
by Arthur Heymans (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Jon Murphy, Julius Werner. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74501?usp=email ) Change subject: arch/arm64: Add Clang as supported target ...................................................................... Patch Set 26: (1 comment) File src/soc/qualcomm/sc7180/Kconfig: https://review.coreboot.org/c/coreboot/+/74501/comment/4be09dbe_dfeb44fc : PS23, Line 27: default n if SOC_QUALCOMM_SC7180 && CBFS_VERIFICATION && TPM_MEASURED_BOOT > ditto Done -- To view, visit https://review.coreboot.org/c/coreboot/+/74501?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: I940a1ccf5cc4ec7bed5b6c8be92fc47922e1e747 Gerrit-Change-Number: 74501 Gerrit-PatchSet: 26 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jon Murphy <jpmurphy(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Wed, 20 Mar 2024 15:03:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: amdfwtool: Use macro to get the table relative address
by Felix Held (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Bao Zheng, Fred Reitberger, Zheng Bao. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78281?usp=email ) Change subject: amdfwtool: Use macro to get the table relative address ...................................................................... Patch Set 7: (1 comment) Patchset: PS7: does this result in identical binaries for all socs? looks to me that the BUFF_TO_RUN_MODE macro does different things depending on which address mode is used, but haven't looked into the details if this might change any behavior -- To view, visit https://review.coreboot.org/c/coreboot/+/78281?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: Iece4ba65e0476543a8d472168d93801714330dde Gerrit-Change-Number: 78281 Gerrit-PatchSet: 7 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Wed, 20 Mar 2024 15:01:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: arch/ppc64/Makefile.inc: Add -target to CPP .ld files with clang
by Arthur Heymans (Code Review) 20 Mar '24

20 Mar '24

Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/78447?usp=email ) Change subject: arch/ppc64/Makefile.inc: Add -target to CPP .ld files with clang ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/78447?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: I17db3071415b298be88a613051296bf6a1081820 Gerrit-Change-Number: 78447 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-MessageType: abandon

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024