coreboot-gerrit April 2024

coreboot-gerrit@coreboot.org

1 participants
3292 discussions

[S] Change in coreboot[main]: riscv/opensbi: make the opensbi payload a flat binary
by ron minnich (Code Review) 05 Apr '24

05 Apr '24

Attention is currently required from: Arthur Heymans, Arthur Heymans, Jonathan Neuschäfer, Maximilian Brune, Philipp Hug. Hello Arthur Heymans, Jonathan Neuschäfer, Maximilian Brune, Philipp Hug, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81632?usp=email to look at the new patch set (#5). Change subject: riscv/opensbi: make the opensbi payload a flat binary ...................................................................... riscv/opensbi: make the opensbi payload a flat binary opensbi is now self-relocating and, further, has strict requirements on the alignment of the data segment -- it has to be power of 2. As opposed to trying to play too many games with ELF files and ldscripts, taking them beyond what's possible, it is simplest just to unpack the opensbi elf into a flat binary and specify its location in cbfs. Works on the sifive unmatched. Still tracking down a failure on qemu. Change-Id: I31525125e87ff5925ca82f4e48fe1191ba6326fd Signed-off-by: Ronald G Minnich <rminnich(a)gmail.com> --- M src/arch/riscv/Makefile.mk M src/mainboard/emulation/qemu-riscv/memlayout.ld 2 files changed, 7 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/81632/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/81632?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: I31525125e87ff5925ca82f4e48fe1191ba6326fd Gerrit-Change-Number: 81632 Gerrit-PatchSet: 5 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: riscv: remove test for OPENSBI_TEXT_START == linker address
by Maximilian Brune (Code Review) 05 Apr '24

05 Apr '24

Attention is currently required from: Arthur Heymans, Philipp Hug, ron minnich. Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81596?usp=email ) Change subject: riscv: remove test for OPENSBI_TEXT_START == linker address ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: I understand that OpenSBI is now capable of relocating itself, but I don't think that is a desirable feature for us. I don't see any upsides in using it. I see a few downsides on the other hand: - It is a huge pain for debugging to deal with relocatable binaries/symbols. - It is error prone - we need to make it a flat binary (although not much trouble) The first issue is really my main concern. Especially when using debuggers like GDB your debugging time easily takes x10 longer. The less relocating components in the bootflow the better. -- To view, visit https://review.coreboot.org/c/coreboot/+/81596?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: Ib16ae611ebaa9e532c86bc8a88e2352665e5b2fd Gerrit-Change-Number: 81596 Gerrit-PatchSet: 2 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Fri, 05 Apr 2024 00:06:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: Update opensbi submodule to upstream master branch
by Maximilian Brune (Code Review) 04 Apr '24

04 Apr '24

Attention is currently required from: Philipp Hug, ron minnich. Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81543?usp=email ) Change subject: Update opensbi submodule to upstream master branch ...................................................................... Patch Set 6: (1 comment) File src/mainboard/emulation/qemu-riscv/memlayout.ld: https://review.coreboot.org/c/coreboot/+/81543/comment/399b3fa4_a0a30b5f : PS6, Line 7: #define BOOTBLOCKSIZE 128K : #define OPENSBISIZE 512K : #define ROMSTAGESIZE 256K : #define RAMSTAGESIZE 2M : #define CONSOLESIZE 8K : #define FMAPSIZE 2K : #define MCACHESIZE 10K : #define STACKSIZE 4M : IMO that looks less readable than before. It also doesn't match the style we have for all the other linker scripts on RISCV and aarch64. -- To view, visit https://review.coreboot.org/c/coreboot/+/81543?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: I50ce0ba3b6409247a94b49565cc04454903e88d0 Gerrit-Change-Number: 81543 Gerrit-PatchSet: 6 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Thu, 04 Apr 2024 23:57:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Enhance portability with uintptr_t/size_t
by Julius Werner (Code Review) 04 Apr '24

04 Apr '24

Attention is currently required from: Andrey Petrov, Arthur Heymans, Dinesh Gehlot, Elyes Haouas, Felix Held, Fred Reitberger, Jason Glenesk, Kapil Porwal, Paul Menzel, Subrata Banik. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81615?usp=email ) Change subject: drivers/intel/fsp2_0: Enhance portability with uintptr_t/size_t ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81615?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: Iab5c612e0640441a2a10e77949416de2afdb8985 Gerrit-Change-Number: 81615 Gerrit-PatchSet: 8 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 04 Apr 2024 23:46:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: lib: Refactor bmp_load_logo() implementation
by Julius Werner (Code Review) 04 Apr '24

04 Apr '24

Attention is currently required from: Andrey Petrov, Arthur Heymans, Dinesh Gehlot, Felix Held, Fred Reitberger, Jason Glenesk, Kapil Porwal, Ronak Kanabar, Sean Rhodes, Subrata Banik. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81618?usp=email ) Change subject: lib: Refactor bmp_load_logo() implementation ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81618?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: I14bc54670a67980ec93bc366b274832d1f959e50 Gerrit-Change-Number: 81618 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Thu, 04 Apr 2024 23:44:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: riscv/opensbi: make the opensbi payload a flat binary
by ron minnich (Code Review) 04 Apr '24

04 Apr '24

Attention is currently required from: Arthur Heymans, Arthur Heymans, Jonathan Neuschäfer, Maximilian Brune, Philipp Hug. ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81632?usp=email ) Change subject: riscv/opensbi: make the opensbi payload a flat binary ...................................................................... Patch Set 4: (2 comments) File src/arch/riscv/Makefile.mk: https://review.coreboot.org/c/coreboot/+/81632/comment/d947e1d8_ebb157e8 : PS4, Line 178: $(OPENSBI_CBFS)-compression := non > is it executed in place? or why not compress? it is executed in place, it seems. So I went with not compressing. File src/mainboard/emulation/qemu-riscv/memlayout.ld: https://review.coreboot.org/c/coreboot/+/81632/comment/6e675b48_9dcd07e6 : PS4, Line 9: #define ROMSTAGESIZE 0 > Why this change? the bootblock and romstage are combined in one file. I see no reason to ever go back. -- To view, visit https://review.coreboot.org/c/coreboot/+/81632?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: I31525125e87ff5925ca82f4e48fe1191ba6326fd Gerrit-Change-Number: 81632 Gerrit-PatchSet: 4 Gerrit-Owner: ron minnich <rminnich(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Philipp Hug <philipp(a)hug.cx> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Philipp Hug <philipp(a)hug.cx> Gerrit-Attention: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 04 Apr 2024 23:13:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> 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) 04 Apr '24

04 Apr '24

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80608?usp=email ) Change subject: soc/intel/common/block/fast_spi: probe for 2nd flash component ...................................................................... soc/intel/common/block/fast_spi: probe for 2nd flash component Fast SPI code assumes only one SPI flash is present. The SPI flash driver for older southbridges is able to detect multichip. See the spi_is_multichip() in src/southbridge/intel/common/spi.c. Some boards (e.g. Lenovo ThinkCentre M920 Tiny) still come with two chips populated instead of one. With this change, both chips are probed, and the correct total size is calculated. Otherwise, only the first one was probed, which resulted in an error such as: SF size 0x1000000 does not correspond to CONFIG_ROM_SIZE 0x1800000!! Change-Id: I8d7449f9e1470dc234fe5ba5217d3ce4c142b49c Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Signed-off-by: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80608 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M src/soc/intel/common/block/fast_spi/fast_spi_def.h M src/soc/intel/common/block/fast_spi/fast_spi_flash.c 2 files changed, 31 insertions(+), 12 deletions(-) Approvals: Nico Huber: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/soc/intel/common/block/fast_spi/fast_spi_def.h b/src/soc/intel/common/block/fast_spi/fast_spi_def.h index cc1cdaa..72c359e 100644 --- a/src/soc/intel/common/block/fast_spi/fast_spi_def.h +++ b/src/soc/intel/common/block/fast_spi/fast_spi_def.h @@ -3,6 +3,11 @@ #ifndef SOC_INTEL_COMMON_BLOCK_FAST_SPI_DEF_H #define SOC_INTEL_COMMON_BLOCK_FAST_SPI_DEF_H +/* From JEDEC SFDP JESD216F.02 */ +#define SFDP_HDR_SIG 0x00 /* 1st DWORD of SFDP header */ +#define SFDP_PARAM_DENSITY 0x04 /* 2nd DWORD of SFDP params */ +#define SFDP_SIGNATURE 0x50444653 /* Valid sig in 1st DWORD of SFDP header */ + /* PCI configuration registers */ #define SPI_BIOS_DECODE_EN 0xd8 #define SPI_BIOS_DECODE_LOCK BIT(31) @@ -153,7 +158,6 @@ #define SPIBAR_PTINX_HORD_SFDP (0 << 12) #define SPIBAR_PTINX_HORD_PARAM (1 << 12) #define SPIBAR_PTINX_HORD_JEDEC (2 << 12) -#define SPIBAR_PTINX_IDX_MASK 0xffc /* Register Offsets of BIOS Flash Program Registers */ #define SPIBAR_RESET_LOCK 0xf0 diff --git a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c index f896cd9..e9cfc23 100644 --- a/src/soc/intel/common/block/fast_spi/fast_spi_flash.c +++ b/src/soc/intel/common/block/fast_spi/fast_spi_flash.c @@ -54,22 +54,22 @@ } /* - * The hardware datasheet is not clear on what HORD values actually do. It - * seems that HORD_SFDP provides access to the first 8 bytes of the SFDP, which - * is the signature and revision fields. HORD_JEDEC provides access to the - * actual flash parameters, and is most likely what you want to use when - * probing the flash from software. + * Via component field (bits 15-14) we can select either 1st or 2nd flash + * (in dual flash setups). + * Via HORD - Header or Data (bits 13-12) - we can select either: + * - SFDP Header + * - Param Table Header + * - Data (JEDEC params, including density) + * * It's okay to rely on SFDP, since the SPI flash controller requires an SFDP * 1.5 or newer compliant FAST_SPI flash chip. * NOTE: Due to the register layout of the hardware, all accesses will be * aligned to a 4 byte boundary. */ -static uint32_t fast_spi_flash_read_sfdp_param(struct fast_spi_flash_ctx *ctx, - uint16_t sfdp_reg) +static uint32_t fast_spi_flash_read_sfdp(struct fast_spi_flash_ctx *ctx, + uint32_t ptinx_reg) { - uint32_t ptinx_index = sfdp_reg & SPIBAR_PTINX_IDX_MASK; - fast_spi_flash_ctrlr_reg_write(ctx, SPIBAR_PTINX, - ptinx_index | SPIBAR_PTINX_HORD_JEDEC); + fast_spi_flash_ctrlr_reg_write(ctx, SPIBAR_PTINX, ptinx_reg); return fast_spi_flash_ctrlr_reg_read(ctx, SPIBAR_PTDATA); } @@ -312,15 +312,30 @@ { BOILERPLATE_CREATE_CTX(ctx); uint32_t flash_bits; + uint32_t ptinx_reg; /* * bytes = (bits + 1) / 8; * But we need to do the addition in a way which doesn't overflow for * 4 Gbit devices (flash_bits == 0xffffffff). */ - flash_bits = fast_spi_flash_read_sfdp_param(ctx, 0x04); + ptinx_reg = SPIBAR_PTINX_COMP_0 | SPIBAR_PTINX_HORD_JEDEC | SFDP_PARAM_DENSITY; + flash_bits = fast_spi_flash_read_sfdp(ctx, ptinx_reg); flash->size = (flash_bits >> 3) + 1; + /* + * Now check if we have a second flash component. + * Check SFDP header for the SFDP signature. If valid, then 2nd component is present. + * Increase the flash size if 2nd component is found, analogically like the 1st + * component. + */ + ptinx_reg = SPIBAR_PTINX_COMP_1 | SPIBAR_PTINX_HORD_SFDP | SFDP_HDR_SIG; + if (fast_spi_flash_read_sfdp(ctx, ptinx_reg) == SFDP_SIGNATURE) { + ptinx_reg = SPIBAR_PTINX_COMP_1 | SPIBAR_PTINX_HORD_JEDEC | SFDP_PARAM_DENSITY; + flash_bits = fast_spi_flash_read_sfdp(ctx, ptinx_reg); + flash->size += ((flash_bits >> 3) + 1); + } + memcpy(&flash->spi, dev, sizeof(*dev)); /* Can erase both 4 KiB and 64 KiB chunks. Declare the smaller size. */ -- 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: 7 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-MessageType: merged

1 0

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

04 Apr '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 6: Code-Review+2 (3 comments) Patchset: PS4: > Thanks for the review, please let me know if you expect some action from me here. It's fine, otherwise I would have marked this as unresolved. File src/soc/intel/common/block/fast_spi/fast_spi_def.h: https://review.coreboot.org/c/coreboot/+/80608/comment/d65f6b17_3b22281d : PS4, Line 156: #define SPIBAR_PTINX_IDX_MASK 0xffc > Because it is not used anymore. The function used to be limited to reading SPDF parameter. […] Ah, I missed that you removed the last usage. I thought it's kind of documenting that the other bits are the index and that it's aligned to 4. But I don't mind dropping it. File src/soc/intel/common/block/fast_spi/fast_spi_flash.c: https://review.coreboot.org/c/coreboot/+/80608/comment/0cfb2bf4_0be07125 : PS4, Line 308: * SFDP table. FLCOMP.C0DEN is no longer used by the Flash Controller. > I cannot possibly comment on this code as a whole I doubt we'll find a reviewer who can. And we've waited long enough I guess. -- 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: 6 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: Thu, 04 Apr 2024 21:03:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: tree: Remove duplicated <stdint.h>
by Felix Held (Code Review) 04 Apr '24

04 Apr '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81633?usp=email ) Change subject: tree: Remove duplicated <stdint.h> ...................................................................... tree: Remove duplicated <stdint.h> <types.h> is supposed to provide <stdint.h>. Change-Id: Ia68a0dc8fba4a48401e213ebb8356e32f0a019ab Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81633 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/cpu/x86/smm/smm_module_loader.c M src/soc/sifive/fu740/include/soc/sdram.h 2 files changed, 0 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index 17ef92e..ea51d63 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -10,7 +10,6 @@ #include <device/device.h> #include <device/mmio.h> #include <rmodule.h> -#include <stdint.h> #include <stdio.h> #include <string.h> #include <types.h> diff --git a/src/soc/sifive/fu740/include/soc/sdram.h b/src/soc/sifive/fu740/include/soc/sdram.h index 8848d1a..83bfe64 100644 --- a/src/soc/sifive/fu740/include/soc/sdram.h +++ b/src/soc/sifive/fu740/include/soc/sdram.h @@ -3,7 +3,6 @@ #ifndef SIFIVE_COMMON_SDRAM_H #define SIFIVE_COMMON_SDRAM_H -#include <stdint.h> #include <types.h> void sdram_init(size_t dram_size); -- To view, visit https://review.coreboot.org/c/coreboot/+/81633?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: Ia68a0dc8fba4a48401e213ebb8356e32f0a019ab Gerrit-Change-Number: 81633 Gerrit-PatchSet: 3 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> 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-MessageType: merged

1 0

[M] Change in coreboot[main]: util/intelp2m: Add support for TigerLake-H SoC
by Felix Held (Code Review) 04 Apr '24

04 Apr '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81587?usp=email ) Change subject: util/intelp2m: Add support for TigerLake-H SoC ...................................................................... util/intelp2m: Add support for TigerLake-H SoC Add support for TigerLake Halo SoC, based on CNL profile. Test: Convert GPIO dump from inteltool into coreboot macros for out-of-tree TGL board. Change-Id: I26eff225c2045edfe5836283be7b4c63f6b405e8 Signed-off-by: Alicja Michalska <ahplka19(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81587 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Maxim <max.senia.poliak(a)gmail.com> --- M util/intelp2m/config/config.go M util/intelp2m/main.go M util/intelp2m/parser/parser.go A util/intelp2m/platforms/tgl/macro.go A util/intelp2m/platforms/tgl/template.go 5 files changed, 149 insertions(+), 2 deletions(-) Approvals: Maxim: Looks good to me, approved build bot (Jenkins): Verified diff --git a/util/intelp2m/config/config.go b/util/intelp2m/config/config.go index 7b491b1..62242b9 100644 --- a/util/intelp2m/config/config.go +++ b/util/intelp2m/config/config.go @@ -28,8 +28,9 @@ LewisburgType uint8 = 1 ApolloType uint8 = 2 CannonType uint8 = 3 - AlderType uint8 = 4 - JasperType uint8 = 5 + TigerType uint8 = 4 + AlderType uint8 = 5 + JasperType uint8 = 6 ) var key uint8 = SunriseType @@ -39,6 +40,7 @@ "lbg": LewisburgType, "apl": ApolloType, "cnl": CannonType, + "tgl": TigerType, "adl": AlderType, "jsl": JasperType, } @@ -67,6 +69,9 @@ func IsPlatformCannonLake() bool { return IsPlatform(CannonType) } +func IsPlatformTigerLake() bool { + return IsPlatform(TigerType) +} func IsPlatformAlderLakeH() bool { return IsPlatform(AlderType) } diff --git a/util/intelp2m/main.go b/util/intelp2m/main.go index b3835ff..64db9fee 100644 --- a/util/intelp2m/main.go +++ b/util/intelp2m/main.go @@ -70,6 +70,7 @@ "\tlbg - Lewisburg PCH with Xeon SP\n"+ "\tapl - Apollo Lake SoC\n"+ "\tcnl - CannonLake-LP or Whiskeylake/Coffeelake/Cometlake-U SoC\n"+ + "\ttgl - TigerLake-H SoC\n"+ "\tadl - AlderLake PCH\n"+ "\tjsl - Jasper Lake SoC\n") diff --git a/util/intelp2m/parser/parser.go b/util/intelp2m/parser/parser.go index 0e2a64b..a6fa315 100644 --- a/util/intelp2m/parser/parser.go +++ b/util/intelp2m/parser/parser.go @@ -11,6 +11,7 @@ "review.coreboot.org/coreboot.git/util/intelp2m/platforms/lbg" "review.coreboot.org/coreboot.git/util/intelp2m/platforms/apl" "review.coreboot.org/coreboot.git/util/intelp2m/platforms/cnl" + "review.coreboot.org/coreboot.git/util/intelp2m/platforms/tgl" "review.coreboot.org/coreboot.git/util/intelp2m/platforms/adl" "review.coreboot.org/coreboot.git/util/intelp2m/platforms/jsl" "review.coreboot.org/coreboot.git/util/intelp2m/config" @@ -150,6 +151,7 @@ config.CannonType : cnl.PlatformSpecific{ InheritanceTemplate : snr.PlatformSpecific{}, }, + config.TigerType : tgl.PlatformSpecific{}, config.AlderType : adl.PlatformSpecific{}, config.JasperType : jsl.PlatformSpecific{}, } diff --git a/util/intelp2m/platforms/tgl/macro.go b/util/intelp2m/platforms/tgl/macro.go new file mode 100644 index 0000000..3b436ee --- /dev/null +++ b/util/intelp2m/platforms/tgl/macro.go @@ -0,0 +1,115 @@ +package tgl + +import ( + "strings" + "fmt" + + "review.coreboot.org/coreboot.git/util/intelp2m/platforms/common" + "review.coreboot.org/coreboot.git/util/intelp2m/platforms/snr" + "review.coreboot.org/coreboot.git/util/intelp2m/platforms/cnl" + "review.coreboot.org/coreboot.git/util/intelp2m/config" + "review.coreboot.org/coreboot.git/util/intelp2m/fields" +) + +const ( + PAD_CFG_DW0_RO_FIELDS = (0x1 << 27) | (0x1 << 24) | (0x3 << 21) | (0xf << 16) | 0xfc + PAD_CFG_DW1_RO_FIELDS = 0xfdffc3ff +) + +const ( + PAD_CFG_DW0 = common.PAD_CFG_DW0 + PAD_CFG_DW1 = common.PAD_CFG_DW1 + MAX_DW_NUM = common.MAX_DW_NUM +) + +type InheritanceMacro interface { + Pull() + GpiMacroAdd() + GpoMacroAdd() + NativeFunctionMacroAdd() + NoConnMacroAdd() +} + +type PlatformSpecific struct { + InheritanceMacro +} + +// RemmapRstSrc - remmap Pad Reset Source Config +func (PlatformSpecific) RemmapRstSrc() { + macro := common.GetMacro() + if config.TemplateGet() != config.TempInteltool { + // Use reset source remapping only if the input file is inteltool.log dump + return + } + if strings.Contains(macro.PadIdGet(), "GPD") { + // See reset map for the TigerLake Community 2: + // https://github.com/coreboot/coreboot/blob/master/src/soc/intel/tigerlake/gp… + // remmap is not required because it is the same as common. + return + } + + dw0 := macro.Register(PAD_CFG_DW0) + var remapping = map[uint8]uint32{ + 0: common.RST_RSMRST << common.PadRstCfgShift, + 1: common.RST_DEEP << common.PadRstCfgShift, + 2: common.RST_PLTRST << common.PadRstCfgShift, + } + resetsrc, valid := remapping[dw0.GetResetConfig()] + if valid { + // dw0.SetResetConfig(resetsrc) + ResetConfigFieldVal := (dw0.ValueGet() & 0x3fffffff) | remapping[dw0.GetResetConfig()] + dw0.ValueSet(ResetConfigFieldVal) + } else { + fmt.Println("Invalid Pad Reset Config [ 0x", resetsrc ," ] for ", macro.PadIdGet()) + } + dw0.CntrMaskFieldsClear(common.PadRstCfgMask) +} + +// Adds The Pad Termination (TERM) parameter from PAD_CFG_DW1 to the macro +// as a new argument +func (platform PlatformSpecific) Pull() { + platform.InheritanceMacro.Pull() +} + +// Adds PAD_CFG_GPI macro with arguments +func (platform PlatformSpecific) GpiMacroAdd() { + platform.InheritanceMacro.GpiMacroAdd() +} + +// Adds PAD_CFG_GPO macro with arguments +func (platform PlatformSpecific) GpoMacroAdd() { + platform.InheritanceMacro.GpoMacroAdd() +} + +// Adds PAD_CFG_NF macro with arguments +func (platform PlatformSpecific) NativeFunctionMacroAdd() { + platform.InheritanceMacro.NativeFunctionMacroAdd() +} + +// Adds PAD_NC macro +func (platform PlatformSpecific) NoConnMacroAdd() { + platform.InheritanceMacro.NoConnMacroAdd() +} + +// GenMacro - generate pad macro +// dw0 : DW0 config register value +// dw1 : DW1 config register value +// return: string of macro +// error +func (PlatformSpecific) GenMacro(id string, dw0 uint32, dw1 uint32, ownership uint8) string { + macro := common.GetInstanceMacro( + PlatformSpecific{ + InheritanceMacro: cnl.PlatformSpecific{ + InheritanceMacro: snr.PlatformSpecific{}, + }, + }, + fields.InterfaceGet(), + ) + macro.Clear() + macro.Register(PAD_CFG_DW0).CntrMaskFieldsClear(common.AllFields) + macro.Register(PAD_CFG_DW1).CntrMaskFieldsClear(common.AllFields) + macro.PadIdSet(id).SetPadOwnership(ownership) + macro.Register(PAD_CFG_DW0).ValueSet(dw0).ReadOnlyFieldsSet(PAD_CFG_DW0_RO_FIELDS) + macro.Register(PAD_CFG_DW1).ValueSet(dw1).ReadOnlyFieldsSet(PAD_CFG_DW1_RO_FIELDS) + return macro.Generate() +} diff --git a/util/intelp2m/platforms/tgl/template.go b/util/intelp2m/platforms/tgl/template.go new file mode 100644 index 0000000..f98272a --- /dev/null +++ b/util/intelp2m/platforms/tgl/template.go @@ -0,0 +1,24 @@ +package tgl + +import "review.coreboot.org/coreboot.git/util/intelp2m/platforms/common" + +// GroupNameExtract - This function extracts the group ID, if it exists in a row +// line : string from the configuration file +// return +// bool : true if the string contains a group identifier +// string : group identifier +func (PlatformSpecific) GroupNameExtract(line string) (bool, string) { + return common.KeywordsCheck(line, + "GPP_A", "GPP_R", "GPP_B", "GPP_D", "GPP_C", "GPP_S", "GPP_G", + "GPD", "GPP_E", "GPP_F", "GPP_H", "GPP_J", "GPP_K", "GPP_I", + "VGPIO_USB", "VGPIO_PCIE") + +} + +// KeywordCheck - This function is used to filter parsed lines of the configuration file and +// returns true if the keyword is contained in the line. +// line : string from the configuration file +func (PlatformSpecific) KeywordCheck(line string) bool { + isIncluded, _ := common.KeywordsCheck(line, "GPP_", "GPD", "VGPIO") + return isIncluded +} -- To view, visit https://review.coreboot.org/c/coreboot/+/81587?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: I26eff225c2045edfe5836283be7b4c63f6b405e8 Gerrit-Change-Number: 81587 Gerrit-PatchSet: 5 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Maxim <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2024