coreboot-gerrit February 2024

coreboot-gerrit@coreboot.org

1 participants
3066 discussions

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Support FSP 2.4 64-bits
by Jérémy Compostella (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Andrey Petrov, Arthur Heymans, Bora Guvendik, Christian Walter, Felix Held, Fred Reitberger, Jason Glenesk, Johnny Lin, Lean Sheng Tan, Matt DeVillier, Patrick Rudolph, Paul Menzel, Ronak Kanabar, Shuo Liu, Tim Chu, Wonkyu Kim. Hello Andrey Petrov, Arthur Heymans, Bora Guvendik, Christian Walter, Felix Held, Fred Reitberger, Jason Glenesk, Johnny Lin, Lean Sheng Tan, Matt DeVillier, Patrick Rudolph, Ronak Kanabar, Shuo Liu, Tim Chu, Wonkyu Kim, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80277?usp=email to look at the new patch set (#4). Change subject: drivers/intel/fsp2_0: Support FSP 2.4 64-bits ...................................................................... drivers/intel/fsp2_0: Support FSP 2.4 64-bits FSP 2.4 brings FSP 64-bits support which requires some adjustments in coreboot: - Stack alignment: 1. FSP functions must be called with the stack 16-bytes aligned. This is already setup properly with the default value of the `mpreferred-stack-boundary' compiler option (4). 2. The FSP stack buffer supplied by coreboot through the `StackBase' UPD must be 16-bytes aligned. - The EDK2 EFIAPI macro definition relies on compiler flags such as __GNUC__ which is not working well when included by coreboot. While it has no side-effect on i386 because the C calling convention used by coreboot and FSP are the same, it breaks on x86_64 because FSP/UEFI uses the Microsoft x64 calling convention while coreboot uses the System V AMD64 ABI. Fortunately, EDK2 header allows to override the EFIAPI definition. The __ms_abi__ attribute works for both i386 and x86_64. This attribute has to be set to all functions calling or called by the FSP. - Add fsp print helper macros to print `efi_return_status_t' with the appropriate format Change-Id: If0397f5cc8d0f4f1872bd37a001fe42e0c37ec99 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/drivers/intel/fsp2_0/debug.c M src/drivers/intel/fsp2_0/fsp_debug_event.c M src/drivers/intel/fsp2_0/include/fsp/debug.h M src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/include/fsp/soc_binding.h M src/drivers/intel/fsp2_0/include/fsp/util.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/ppi/mp_service1.c M src/drivers/intel/fsp2_0/ppi/mp_service2.c M src/drivers/intel/fsp2_0/silicon_init.c M src/drivers/intel/fsp2_0/util.c M src/include/efi/efi_datatype.h M src/soc/amd/common/fsp/fsp_reset.c M src/soc/intel/common/fsp_reset.c 15 files changed, 90 insertions(+), 61 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/80277/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/80277?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: If0397f5cc8d0f4f1872bd37a001fe42e0c37ec99 Gerrit-Change-Number: 80277 Gerrit-PatchSet: 4 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.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: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-CC: Appukuttan V K <appukuttan.vk(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: mb/google/brox: Initialize TCHSCR_RST_L to 0
by Shelley Chen (Code Review) 01 Feb '24

01 Feb '24

Shelley Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80300?usp=email ) Change subject: mb/google/brox: Initialize TCHSCR_RST_L to 0 ...................................................................... mb/google/brox: Initialize TCHSCR_RST_L to 0 TCHSCR_RST_L signal was originally being configured to 1 in gpio.c but this was causing some leakage. Configuring it to 0 initially should fix this. BUG=b:322249892 BRANCH=None TEST=Make brox boots and touchscreen is still working Change-Id: I5bf1901a3a40a38237b950abcb758f96aebcc1cf Signed-off-by: Shelley Chen <shchen(a)google.com> --- M src/mainboard/google/brox/variants/baseboard/brox/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/80300/1 diff --git a/src/mainboard/google/brox/variants/baseboard/brox/gpio.c b/src/mainboard/google/brox/variants/baseboard/brox/gpio.c index 5432bd1..2f31ae8 100644 --- a/src/mainboard/google/brox/variants/baseboard/brox/gpio.c +++ b/src/mainboard/google/brox/variants/baseboard/brox/gpio.c @@ -262,7 +262,7 @@ /* GPP_F16 : [NF1: GSXCLK NF3: THC1_SPI2_CS# NF4: GSPI1_CS0# NF6: USB_C_GPP_F16] ==> PCH_TCHSCR_REPORT_EN */ PAD_CFG_GPO(GPP_F16, 0, PLTRST), /* GPP_F17 : [NF3: THC1_SPI2_RST# NF6: USB_C_GPP_F17] ==> TCHSCR_RST_L */ - PAD_CFG_GPO(GPP_F17, 1, DEEP), + PAD_CFG_GPO(GPP_F17, 0, DEEP), /* GPP_F18 : [NF3: THC1_SPI2_INT# NF6: USB_C_GPP_F18] ==> TCHSCR_INT_ODL */ PAD_CFG_GPI_APIC(GPP_F18, NONE, PLTRST, LEVEL, INVERT), /* GPP_F19 : SRCCLKREQ6 ==> NC */ -- To view, visit https://review.coreboot.org/c/coreboot/+/80300?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: I5bf1901a3a40a38237b950abcb758f96aebcc1cf Gerrit-Change-Number: 80300 Gerrit-PatchSet: 1 Gerrit-Owner: Shelley Chen <shchen(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: commonlib: Add assembly optimization for ipchksum() on arm64
by Julius Werner (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Paul Menzel. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80254?usp=email ) Change subject: commonlib: Add assembly optimization for ipchksum() on arm64 ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/80254/comment/541599d8_0ad72bee : PS2, Line 10: to take advantage of larger load : sizes and the add-with-carry instruction > Excuse my ignorance, but why can’t the compilers do that themselves? It's very hard to write C code for this in the first place because the C language has no means to describe the carry that gets shifted out of the end of a data type. The best I can come up with is to check it afterwards like this ``` for (size_t i = 0; i < size; i++) { uint64_t new_sum = result + *data++; if (new_sum < result) new_sum++; result = new_sum; } ``` and interestingly the compiler does make something clever out of that on both x86 ``` 10: 48 03 04 d7 add (%rdi,%rdx,8),%rax 14: 48 83 d0 00 adc $0x0,%rax 18: 48 83 c2 01 add $0x1,%rdx 1c: 48 39 d6 cmp %rdx,%rsi 1f: 75 ef jne 10 <sum+0x10> (File Offset: 0x50) ``` and Arm ``` 8: f8637804 ldr x4, [x0, x3, lsl #3] c: 91000463 add x3, x3, #0x1 10: ab040042 adds x2, x2, x4 14: 9a823442 cinc x2, x2, cs // cs = hs, nlast 18: eb03003f cmp x1, x3 1c: 54ffff61 b.ne 8 <sum+0x8> (File Offset: 0x48) // b.any ``` but it's not really as clever as my code. In general I found that for very tight loops when every instruction can make a significant performance difference, GCC still doesn't really get as good as handcrafted assembly. (Also, when writing code that so explicitly tries to bait a specific instruction, one could argue that just writing the assembly directly is actually more readable.) https://review.coreboot.org/c/coreboot/+/80254/comment/f6d7ec59_1c24940a : PS2, Line 12: by more than 20x > Thank you for documenting this. […] They're not really worth mentioning (a couple of microseconds) because the coreboot tables that get checksummed by this are pretty small (at least on the platform I tested). I did this mostly for fun tbh, not for serious gain. I think some other platforms are also using that checksum for other things so it might help them a bit more (we used to use it for MRC cache where the impact was actually significant, but now that was replaced with xxhash already). -- To view, visit https://review.coreboot.org/c/coreboot/+/80254?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: I9c7bbc9d7a1cd083ced62fe9222592243a796077 Gerrit-Change-Number: 80254 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> 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, 01 Feb 2024 21:57:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: lib: Move IP checksum to commonlib
by Julius Werner (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Angel Pons, Arthur Heymans, Hung-Te Lin, Jakub Czapiga, Jérémy Compostella, Paul Menzel, Yidi Lin, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80251?usp=email ) Change subject: lib: Move IP checksum to commonlib ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/80251/comment/81dee1ce_a7f996e8 : PS2, Line 16: calculatted > calculated? Done Patchset: PS2: > ok for the arm/mtk related files. […] I don't see a conflict? Rebased again, just in case. -- To view, visit https://review.coreboot.org/c/coreboot/+/80251?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: Ic04c714c00439a17fc04a8a6e730cc2aa19b8e68 Gerrit-Change-Number: 80251 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 01 Feb 2024 21:55:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: lib: Move IP checksum to commonlib
by Julius Werner (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Angel Pons, Arthur Heymans, Hung-Te Lin, Jakub Czapiga, Julius Werner, Jérémy Compostella, Yidi Lin, Yu-Ping Wu. Hello Angel Pons, Arthur Heymans, Hung-Te Lin, Jakub Czapiga, Jérémy Compostella, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80251?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by Hung-Te Lin, Code-Review+2 by Arthur Heymans, Code-Review+2 by Yidi Lin, Verified+1 by build bot (Jenkins) Change subject: lib: Move IP checksum to commonlib ...................................................................... lib: Move IP checksum to commonlib This patch moves the IP checksum algorithm into commonlib to prepare for it being shared with libpayload. The current implementation is ancient and pretty hard to read (and does some unnecessary questionable things like the type-punning stuff which leads to suboptimal code generation), so this reimplements it from scratch (that also helps with the licensing). This algorithm is prepared to take in a pre-calculated "wide" checksum in a machine-register-sized data type which is then narrowed down to 16 bits (see RFC 1071 for why that's valid). This isn't used yet (and the code will get optimized out), but will be used later in this patch series for architecture-specific optimization. Change-Id: Ic04c714c00439a17fc04a8a6e730cc2aa19b8e68 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/arch/x86/boot.c M src/commonlib/Makefile.mk A src/commonlib/bsd/include/commonlib/bsd/ipchksum.h A src/commonlib/bsd/ipchksum.c M src/drivers/elog/boot_count.c M src/drivers/intel/fsp1_1/hob.c M src/drivers/net/ne2k.c D src/include/ip_checksum.h M src/lib/Makefile.mk D src/lib/compute_ip_checksum.c M src/lib/coreboot_table.c M src/northbridge/intel/haswell/broadwell_mrc/raminit.c M src/northbridge/intel/haswell/haswell_mrc/raminit.c M src/northbridge/intel/ironlake/raminit.c M src/northbridge/intel/sandybridge/raminit_mrc.c M src/soc/intel/common/basecode/ramtop/ramtop.c M src/soc/intel/common/block/cse/cse_lite_cmos.c M src/soc/mediatek/common/memory.c M src/soc/mediatek/mt8183/memory.c M src/southbridge/intel/bd82x6x/early_pch.c M tests/commonlib/bsd/Makefile.mk A tests/commonlib/bsd/ipchksum-test.c M tests/lib/Makefile.mk D tests/lib/compute_ip_checksum-test.c 24 files changed, 193 insertions(+), 202 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/80251/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/80251?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: Ic04c714c00439a17fc04a8a6e730cc2aa19b8e68 Gerrit-Change-Number: 80251 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: soc/amd/phoenix/chip.h: guard FSP-specific data structures
by Felix Held (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier. Hello Fred Reitberger, Jason Glenesk, Matt DeVillier, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80298?usp=email to look at the new patch set (#2). Change subject: soc/amd/phoenix/chip.h: guard FSP-specific data structures ...................................................................... soc/amd/phoenix/chip.h: guard FSP-specific data structures Since the USB configuration data structure is FSP-specific, add guards on this part of the soc_amd_phoenix_config struct and the corresponding include. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I6c324421fbc3dc7b9a7bf6f5868785e9718147a5 --- M src/soc/amd/phoenix/chip.h 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/80298/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80298?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: I6c324421fbc3dc7b9a7bf6f5868785e9718147a5 Gerrit-Change-Number: 80298 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[main]: [Squash]: Add ACPI BDAT support
by Gaggery (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Anil Kumar K, Arthur Heymans, Bora Guvendik, Chen, Gang C, Cliff Huang, Jamie Ryu, Jérémy Compostella, Kane Chen, Karthik Ramasubramanian, Krishna P Bhat D, Lance Zhao, Pratikkumar V Prajapati, Ravishankar Sarawadi, Ronak Kanabar, Shuo Liu, Subrata Banik, Tim Wawrzynczak, V Sowmya, Wonkyu Kim. Gaggery has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77255?usp=email ) Change subject: [Squash]: Add ACPI BDAT support ...................................................................... Patch Set 14: (3 comments) File src/soc/intel/common/block/acpi/acpi_bdat.c: https://review.coreboot.org/c/coreboot/+/77255/comment/b8dca13e_6f3963e0 : PS14, Line 175: memcpy(next_block, schema_data, data_size); > Can we make sure in ACPI BDAT spec, all schema data are not pointers? if no pointers, the directly c […] Regarding the spec of BDAT, the schema_data itself does not contain pointers. Please refer to memory schema 2/2B/4/4B and RMT schema 4/5. The memcpy is the efficient way to do so. Please let me know if you still have concerns. File src/soc/intel/common/block/include/intelblocks/acpi_bdat.h: https://review.coreboot.org/c/coreboot/+/77255/comment/c9f0add9_c093c165 : PS14, Line 66: EFI_GUID schema_hob_guids[MAX_SCHEMA_LIST_LENGTH]; > Can we use base/size instead? a.k.a. […] I'm not sure how to accommodate the code of handling ways other than hobs as I don't have a platform to test that scenario. The best way I can think of is to search for the guids in hobs, if it couldn't it form the hobs, then looking for the guids from a weak function that should be implemented by the platform code. Does it sound good? And could you please elaborate more about base/size? https://review.coreboot.org/c/coreboot/+/77255/comment/5e93c9f0_907d6548 : PS14, Line 67: } bdat_schema_list_hob; > Can we rename it as bdat_schema_list? a.k.a. […] I am not sure if the structure from non-hobs should leverage the same structure. -- To view, visit https://review.coreboot.org/c/coreboot/+/77255?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: I5eb57f65ef5f24458f09587b7c7694156f2ed1ce Gerrit-Change-Number: 77255 Gerrit-PatchSet: 14 Gerrit-Owner: Gaggery <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 01 Feb 2024 21:32:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/brox: Fix the I2C configuration
by Shelley Chen (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Karthik Ramasubramanian, Nick Vaccaro. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80303?usp=email ) Change subject: mb/google/brox: Fix the I2C configuration ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80303?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: I9f24beb9ef587163362cc6ded88efb05be1329b9 Gerrit-Change-Number: 80303 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 01 Feb 2024 21:02:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/brox: Fix the I2C configuration
by Karthik Ramasubramanian (Code Review) 01 Feb '24

01 Feb '24

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80303?usp=email ) Change subject: mb/google/brox: Fix the I2C configuration ...................................................................... mb/google/brox: Fix the I2C configuration Update the I2C configuration to match the usage such that only required I2C controllers are enabled. BUG=b:319390850 TEST=Build Brox BIOS image and boot to OS. Ensure that only the required I2C controllers are enabled. Change-Id: I9f24beb9ef587163362cc6ded88efb05be1329b9 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/80303/1 diff --git a/src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb b/src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb index e1c8134..3e0e7fb 100644 --- a/src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb +++ b/src/mainboard/google/brox/variants/baseboard/brox/devicetree.cb @@ -48,9 +48,9 @@ [PchSerialIoIndexI2C0] = PchSerialIoPci, [PchSerialIoIndexI2C1] = PchSerialIoPci, [PchSerialIoIndexI2C2] = PchSerialIoPci, - [PchSerialIoIndexI2C3] = PchSerialIoPci, - [PchSerialIoIndexI2C4] = PchSerialIoDisabled, - [PchSerialIoIndexI2C5] = PchSerialIoPci, + [PchSerialIoIndexI2C3] = PchSerialIoDisabled, + [PchSerialIoIndexI2C4] = PchSerialIoPci, + [PchSerialIoIndexI2C5] = PchSerialIoDisabled, }" register "serial_io_gspi_mode" = "{ -- To view, visit https://review.coreboot.org/c/coreboot/+/80303?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: I9f24beb9ef587163362cc6ded88efb05be1329b9 Gerrit-Change-Number: 80303 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: mb/amd/birman: add Phoenix with openSIL mainboard option
by Felix Held (Code Review) 01 Feb '24

01 Feb '24

Attention is currently required from: Fred Reitberger, Jason Glenesk. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80299?usp=email ) Change subject: mb/amd/birman: add Phoenix with openSIL mainboard option ...................................................................... mb/amd/birman: add Phoenix with openSIL mainboard option Introduce BOARD_AMD_BIRMAN_PHOENIX_OPENSIL which selects the openSIL based Phoenix SoC code. Since the Phoenix chip.c is different due to some FSP-specific data structures in there that are guarded in the openSIL case, a separate devicetree for the openSIL case is added. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I248102e92818b2d395d561a4bf2627f80906b2f7 --- M src/mainboard/amd/birman/Kconfig M src/mainboard/amd/birman/Kconfig.name R src/mainboard/amd/birman/devicetree_phoenix_fsp.cb A src/mainboard/amd/birman/devicetree_phoenix_opensil.cb 4 files changed, 155 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/80299/1 diff --git a/src/mainboard/amd/birman/Kconfig b/src/mainboard/amd/birman/Kconfig index cd7e938..9fafae1 100644 --- a/src/mainboard/amd/birman/Kconfig +++ b/src/mainboard/amd/birman/Kconfig @@ -19,6 +19,10 @@ select BOARD_AMD_BIRMAN_COMMON select SOC_AMD_PHOENIX_FSP +config BOARD_AMD_BIRMAN_PHOENIX_OPENSIL + select BOARD_AMD_BIRMAN_COMMON + select SOC_AMD_PHOENIX_OPENSIL + config BOARD_AMD_BIRMAN_GLINDA select BOARD_AMD_BIRMAN_COMMON select SOC_AMD_GLINDA @@ -27,7 +31,7 @@ config FMDFILE default "src/mainboard/amd/birman/chromeos_glinda.fmd" if CHROMEOS && BOARD_AMD_BIRMAN_GLINDA - default "src/mainboard/amd/birman/chromeos_phoenix.fmd" if CHROMEOS && BOARD_AMD_BIRMAN_PHOENIX_FSP + default "src/mainboard/amd/birman/chromeos_phoenix.fmd" if CHROMEOS default "src/mainboard/amd/birman/board_glinda.fmd" if BOARD_AMD_BIRMAN_GLINDA default "src/mainboard/amd/birman/board_phoenix.fmd" @@ -37,10 +41,12 @@ config MAINBOARD_PART_NUMBER default "Birman_Glinda" if BOARD_AMD_BIRMAN_GLINDA default "Birman_Phoenix_FSP" if BOARD_AMD_BIRMAN_PHOENIX_FSP + default "Birman_Phoenix_openSIL" if BOARD_AMD_BIRMAN_PHOENIX_OPENSIL config DEVICETREE default "devicetree_glinda.cb" if BOARD_AMD_BIRMAN_GLINDA - default "devicetree_phoenix.cb" + default "devicetree_phoenix_fsp.cb" if BOARD_AMD_BIRMAN_PHOENIX_FSP + default "devicetree_phoenix_opensil.cb" if BOARD_AMD_BIRMAN_PHOENIX_OPENSIL config BIRMAN_HAVE_MCHP_FW bool "Have Microchip EC firmware?" diff --git a/src/mainboard/amd/birman/Kconfig.name b/src/mainboard/amd/birman/Kconfig.name index 9eee171..6daea88 100644 --- a/src/mainboard/amd/birman/Kconfig.name +++ b/src/mainboard/amd/birman/Kconfig.name @@ -3,5 +3,8 @@ config BOARD_AMD_BIRMAN_PHOENIX_FSP bool "-> Birman for Phoenix SoC using FSP" +config BOARD_AMD_BIRMAN_PHOENIX_OPENSIL + bool "-> Birman for Phoenix SoC using openSIL" + config BOARD_AMD_BIRMAN_GLINDA bool "-> Birman for Glinda SoC" diff --git a/src/mainboard/amd/birman/devicetree_phoenix.cb b/src/mainboard/amd/birman/devicetree_phoenix_fsp.cb similarity index 100% rename from src/mainboard/amd/birman/devicetree_phoenix.cb rename to src/mainboard/amd/birman/devicetree_phoenix_fsp.cb diff --git a/src/mainboard/amd/birman/devicetree_phoenix_opensil.cb b/src/mainboard/amd/birman/devicetree_phoenix_opensil.cb new file mode 100644 index 0000000..58cead5 --- /dev/null +++ b/src/mainboard/amd/birman/devicetree_phoenix_opensil.cb @@ -0,0 +1,144 @@ +# SPDX-License-Identifier: GPL-2.0-only + +# TODO: Update for birman + +chip soc/amd/phoenix + register "common_config.espi_config" = "{ + .std_io_decode_bitmap = ESPI_DECODE_IO_0x80_EN | ESPI_DECODE_IO_0X2E_0X2F_EN | ESPI_DECODE_IO_0X60_0X64_EN, + .generic_io_range[0] = { + .base = 0x3f8, + .size = 8, + }, + .generic_io_range[1] = { + .base = 0x600, + .size = 256, + }, + .io_mode = ESPI_IO_MODE_QUAD, + .op_freq_mhz = ESPI_OP_FREQ_16_MHZ, + .crc_check_enable = 1, + .alert_pin = ESPI_ALERT_PIN_PUSH_PULL, + .periph_ch_en = 1, + .vw_ch_en = 1, + .oob_ch_en = 1, + .flash_ch_en = 0, + }" + + register "i2c_scl_reset" = "GPIO_I2C0_SCL | GPIO_I2C1_SCL | + GPIO_I2C2_SCL | GPIO_I2C3_SCL" + + register "i2c[0].early_init" = "1" + register "i2c[1].early_init" = "1" + register "i2c[2].early_init" = "1" + register "i2c[3].early_init" = "1" + + # I2C Pad Control RX Select Configuration + register "i2c_pad[0].rx_level" = "I2C_PAD_RX_1_8V" + register "i2c_pad[1].rx_level" = "I2C_PAD_RX_1_8V" + register "i2c_pad[2].rx_level" = "I2C_PAD_RX_1_8V" + register "i2c_pad[3].rx_level" = "I2C_PAD_RX_1_8V" + + register "s0ix_enable" = "true" + + register "pspp_policy" = "DXIO_PSPP_DISABLED" # TODO: reenable when PSPP works< + + device domain 0 on + device ref iommu on end + device ref gpp_bridge_1_1 on end # MXM + device ref gpp_bridge_1_2 on + # Required so the NVMe gets placed into D3 when entering S0i3. + chip drivers/pcie/rtd3/device + register "name" = ""NVME"" + device pci 00.0 on end + end + end # NVMe SSD1 + device ref gpp_bridge_1_3 on end # GBE + device ref gpp_bridge_2_1 on end # SD + device ref gpp_bridge_2_2 on end # WWAN + device ref gpp_bridge_2_3 on end # WIFI + device ref gpp_bridge_2_4 on + # Required so the NVMe gets placed into D3 when entering S0i3. + chip drivers/pcie/rtd3/device + register "name" = ""NVME"" + device pci 00.0 on end + end + end # NVMe SSD0 + device ref gpp_bridge_a on # Internal GPP Bridge 0 to Bus A + device ref gfx on end # Internal GPU (GFX) + device ref gfx_hda on end # Display HD Audio Controller (GFXAZ) + device ref crypto on end # Crypto Coprocessor + device ref xhci_0 on # USB 3.1 (USB0) + chip drivers/usb/acpi + device ref xhci_0_root_hub on + chip drivers/usb/acpi + device ref usb3_port2 on end + end + chip drivers/usb/acpi + device ref usb3_port3 on end + end + chip drivers/usb/acpi + device ref usb2_port2 on end + end + chip drivers/usb/acpi + device ref usb2_port3 on end + end + chip drivers/usb/acpi + device ref usb2_port4 on end + end + chip drivers/usb/acpi + device ref usb2_port5 on end + end + chip drivers/usb/acpi + device ref usb2_port6 on end + end + end + end + end + device ref xhci_1 on # USB 3.1 (USB1) + chip drivers/usb/acpi + device ref xhci_1_root_hub on + chip drivers/usb/acpi + device ref usb3_port7 on end + end + chip drivers/usb/acpi + device ref usb2_port7 on end + end + end + end + end + device ref acp on end # Audio Processor (ACP) + end + device ref gpp_bridge_c on # Internal GPP Bridge 2 to Bus C + device ref usb4_xhci_0 on + chip drivers/usb/acpi + device ref usb4_xhci_0_root_hub on + chip drivers/usb/acpi + device ref usb3_port0 on end + end + chip drivers/usb/acpi + device ref usb2_port0 on end + end + end + end + end + device ref usb4_xhci_1 on + chip drivers/usb/acpi + device ref usb4_xhci_1_root_hub on + chip drivers/usb/acpi + device ref usb3_port1 on end + end + chip drivers/usb/acpi + device ref usb2_port1 on end + end + end + end + end + end + end + + device ref i2c_0 on end + device ref i2c_1 on end + device ref i2c_2 on end + device ref i2c_3 on end + device ref uart_0 on end # UART0 + +end -- To view, visit https://review.coreboot.org/c/coreboot/+/80299?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: I248102e92818b2d395d561a4bf2627f80906b2f7 Gerrit-Change-Number: 80299 Gerrit-PatchSet: 1 Gerrit-Owner: 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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2024