coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[M] Change in coreboot[main]: superio/ite: Add special fan vectors and further options
by Matt DeVillier (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Joel Linn, Nico Huber. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81426?usp=email ) Change subject: superio/ite: Add special fan vectors and further options ...................................................................... Patch Set 4: (1 comment) File src/superio/ite/common/env_ctrl.c: https://review.coreboot.org/c/coreboot/+/81426/comment/720462c0_2b08c912 : PS1, Line 369: #endif > Hmmm, that's annoying. Another, clean alternative would be to put all the […] just set the vector count to 2 regardless of the Kconfig enablement, unless it causes an issue elsewhere I'm not seeing -- To view, visit https://review.coreboot.org/c/coreboot/+/81426?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: I93df2b5652fc3fde775b6161fa5bebc4a34d5e94 Gerrit-Change-Number: 81426 Gerrit-PatchSet: 4 Gerrit-Owner: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-Comment-Date: Sun, 24 Mar 2024 20:47:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: [Squash]: Add ACPI BDAT support
by Cliff Huang (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Anil Kumar K, Arthur Heymans, Bora Guvendik, Chen, Gang C, Gaggery Tsai, 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. Cliff Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77255?usp=email ) Change subject: [Squash]: Add ACPI BDAT support ...................................................................... Patch Set 16: (1 comment) Patchset: PS16: NOTE: -DBDAT_SUPPORT=1 is already be added by gSiPkgTokenSpaceGuid.PcdBdatEnable, which is enabled in ClientOneSiliconPkg/Product/MeteorLake/SiPkgPcdInit.dsc. -- 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: 16 Gerrit-Owner: Gaggery Tsai <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-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: Gaggery Tsai <gaggery.tsai(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: Sun, 24 Mar 2024 20:25:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: superio/ite: Add special fan vectors and further options
by Nico Huber (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Joel Linn, Matt DeVillier. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81426?usp=email ) Change subject: superio/ite: Add special fan vectors and further options ...................................................................... Patch Set 4: (1 comment) File src/superio/ite/common/env_ctrl.c: https://review.coreboot.org/c/coreboot/+/81426/comment/05e9cdd0_8db071b1 : PS1, Line 369: #endif > Unfortunately GCC will cry about the loop condition here […] Hmmm, that's annoying. Another, clean alternative would be to put all the related code into a separate .c file. Only compile it if fan vectors are selected. -- To view, visit https://review.coreboot.org/c/coreboot/+/81426?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: I93df2b5652fc3fde775b6161fa5bebc4a34d5e94 Gerrit-Change-Number: 81426 Gerrit-PatchSet: 4 Gerrit-Owner: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-Comment-Date: Sun, 24 Mar 2024 20:17:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: superio/ite: Add special fan vectors and further options
by Joel Linn (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Matt DeVillier, Nico Huber. Joel Linn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81426?usp=email ) Change subject: superio/ite: Add special fan vectors and further options ...................................................................... Patch Set 4: (1 comment) File src/superio/ite/common/env_ctrl.c: https://review.coreboot.org/c/coreboot/+/81426/comment/b6937413_e122828a : PS1, Line 369: #endif > Done Unfortunately GCC will cry about the loop condition here error: comparison of unsigned expression in '< 0' is always false [-Werror=type-limits] I added pragmas to disable the warning as I couldn't find an obvious leaner local solution. One could keep the #if here but then you have an unused symbol which would either require a pragma or C23 [[maybe_unused]]. GCC/Clang have __maybe_unused, would that be practical? -- To view, visit https://review.coreboot.org/c/coreboot/+/81426?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: I93df2b5652fc3fde775b6161fa5bebc4a34d5e94 Gerrit-Change-Number: 81426 Gerrit-PatchSet: 4 Gerrit-Owner: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Sun, 24 Mar 2024 20:07:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: doc/releases: Fix embedded rST syntax for MyST Parser
by Nicholas Chin (Code Review) 24 Mar '24

24 Mar '24

Nicholas Chin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81500?usp=email ) Change subject: doc/releases: Fix embedded rST syntax for MyST Parser ...................................................................... doc/releases: Fix embedded rST syntax for MyST Parser After commit 35599f9a66 (Docs: Replace Recommonmark with MyST Parser), embedded rST should uses `{eval-rst}` instead of `eval_rst`. This was missed during manual rebasing of that patch before it was merged. Change-Id: I648a95488df25d70e1b581872a19272c51f33b7b Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M Documentation/releases/coreboot-24.02-relnotes.md 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/81500/1 diff --git a/Documentation/releases/coreboot-24.02-relnotes.md b/Documentation/releases/coreboot-24.02-relnotes.md index e4e7892..f82a6a8 100644 --- a/Documentation/releases/coreboot-24.02-relnotes.md +++ b/Documentation/releases/coreboot-24.02-relnotes.md @@ -202,7 +202,7 @@ ### coreboot-wide or architecture-wide issues -```eval_rst +```{eval-rst} +-----+-----------------------------------------------------------------+ | # | Subject | +=====+=================================================================+ @@ -217,7 +217,7 @@ ### Payload-specific issues -```eval_rst +```{eval-rst} +-----+-----------------------------------------------------------------+ | # | Subject | +=====+=================================================================+ @@ -234,7 +234,7 @@ ### Platform-specific issues -```eval_rst +```{eval-rst} +-----+-----------------------------------------------------------------+ | # | Subject | +=====+=================================================================+ -- To view, visit https://review.coreboot.org/c/coreboot/+/81500?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: I648a95488df25d70e1b581872a19272c51f33b7b Gerrit-Change-Number: 81500 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: mb/google/brox: Enable Intel RMT+
by Gaggery Tsai (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Paul Menzel. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80568?usp=email ) Change subject: mb/google/brox: Enable Intel RMT+ ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/80568/comment/e1ef0003_600c19d1 : PS1, Line 7: src/ > Please remove. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/80568?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: Ib9939cd93f6ff020c52966553e7fb8e4aa67ed1a Gerrit-Change-Number: 80568 Gerrit-PatchSet: 2 Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Sun, 24 Mar 2024 19:20:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/rex: Enable Intel RMT+
by Gaggery Tsai (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Paul Menzel, Subrata Banik, Tarun. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77684?usp=email ) Change subject: mb/google/rex: Enable Intel RMT+ ...................................................................... Patch Set 10: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/77684/comment/b7afbeea_74090e93 : PS9, Line 7: rc/ > Please remove. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/77684?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: I4e56c32cee1ce8c10b9da9d0123cbd13eaf9d8d3 Gerrit-Change-Number: 77684 Gerrit-PatchSet: 10 Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Sun, 24 Mar 2024 19:20:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: src/soc/intel/common: Add Intel RMT+ support
by Gaggery Tsai (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Anil Kumar K, Arthur Heymans, Bora Guvendik, Cliff Huang, Jamie Ryu, Jérémy Compostella, Kane Chen, Karthik Ramasubramanian, Krishna P Bhat D, Pratikkumar V Prajapati, Ravishankar Sarawadi, Ronak Kanabar, Subrata Banik, V Sowmya, Wonkyu Kim. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77904?usp=email ) Change subject: src/soc/intel/common: Add Intel RMT+ support ...................................................................... Patch Set 8: (3 comments) File src/soc/intel/common/block/memory/Kconfig: https://review.coreboot.org/c/coreboot/+/77904/comment/ed30f586_26745ce0 : PS7, Line 33: depends on VBOOT > > > Needs it to query boot mode. […] Done File src/soc/intel/common/block/memory/meminit.c: https://review.coreboot.org/c/coreboot/+/77904/comment/cee45507_8ed95680 : PS7, Line 221: __weak void enable_rmt_plus_platform(FSP_M_CONFIG *mem_cfg) : { : printk(BIOS_DEBUG, "RMT+ is not supported.\n"); : } > I will revise it. Done https://review.coreboot.org/c/coreboot/+/77904/comment/2154ba86_75187dc4 : PS7, Line 228: vboot_developer_mode_enabled() > > Only enables this RMT+ feature with DEV mode since it will increase about 8% of boot time. […] Added another flag for DEV mode. -- To view, visit https://review.coreboot.org/c/coreboot/+/77904?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: I03df4d0a927caa101da3db078b268dcdb4e78550 Gerrit-Change-Number: 77904 Gerrit-PatchSet: 8 Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(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: 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: Subrata Banik <subratabanik(a)google.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: Arthur Heymans <arthur(a)aheymans.xyz> 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.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: Sun, 24 Mar 2024 18:55:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Gaggery Tsai <gaggery.tsai(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

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

24 Mar '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 Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77255?usp=email ) Change subject: [Squash]: Add ACPI BDAT support ...................................................................... Patch Set 16: (22 comments) File src/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/77255/comment/27f588f9_a1e5a9e3 : PS15, Line 1106: unsigned long long > IMO, `uint64_t` would keep the line shorter while also making the understanding of this line slightl […] Done https://review.coreboot.org/c/coreboot/+/77255/comment/935615f9_7f923593 : PS15, Line 1108: /* Calculate checksum */ : header->checksum = acpi_checksum((void *)bdat, header->length); > > why? […] Acknowledged File src/soc/intel/common/block/acpi/acpi_bdat.c: https://review.coreboot.org/c/coreboot/+/77255/comment/d90aa1fe_cf4f15ea : PS15, Line 12: #define INTEL_FSP_BDAT_SCHEMA_LIST_HOB_GUID \ > What about ? […] Done https://review.coreboot.org/c/coreboot/+/77255/comment/993c8668_b8725d78 : PS15, Line 26: if (guid != NULL) { > that test seems unnecessary for a static function called with a non-NULL parameter according the cod […] Replaced it with fsp_print_guid per other reviewer's comments. https://review.coreboot.org/c/coreboot/+/77255/comment/f8f1988d_3d6fd2f3 : PS15, Line 23: static void print_guid(EFI_GUID *guid) : { : int i; : if (guid != NULL) { : printk(BIOS_DEBUG, "GUID = "); : printk(BIOS_DEBUG, "0x%08x-", guid->Data1); : printk(BIOS_DEBUG, "0x%04x-", guid->Data2); : printk(BIOS_DEBUG, "0x%04x-", guid->Data3); : for (i = 0; i < GUID_TAIL_BYTES; i++) { : printk(BIOS_DEBUG, "0x%02x", guid->Data4[i]); : if (i != (GUID_TAIL_BYTES-1)) : printk(BIOS_DEBUG, "-"); : } : printk(BIOS_DEBUG, "\n"); : } : } > this is reimplemented a lot. Can it be added to common code somewhere or reuse existing code. Done https://review.coreboot.org/c/coreboot/+/77255/comment/0d90f660_68378e06 : PS15, Line 40: get_crc16 > Use CRC marcro? IMHO, it might be too complex converting to a macro. Let me know if you're fine with it. https://review.coreboot.org/c/coreboot/+/77255/comment/d90a0f37_aa4fd3e8 : PS15, Line 62: EFI_GUID *guid; > Please use the <efi/efi_datatypes.h> version of the UEFI types. Done https://review.coreboot.org/c/coreboot/+/77255/comment/b3baa188_0de25b2e : PS15, Line 63: const uint32_t *schema[MAX_SCHEMA_LIST_LENGTH] = {NULL}; : uint32_t schema_size[MAX_SCHEMA_LIST_LENGTH]; > This intermediate data structure could be simplified as a structure: […] Done https://review.coreboot.org/c/coreboot/+/77255/comment/3883b0c3_b10ae00c : PS15, Line 67: const uint8_t bdat_header_sign[] = {'B', 'D', 'A', 'T', 'H', 'E', 'A', 'D'}; > I would make is a static variable of the C module instead of a stack variable. […] Done https://review.coreboot.org/c/coreboot/+/77255/comment/7c06f692_f7e23090 : PS15, Line 74: printk(BIOS_ERR, "No BDAT data exists.\n"); > `No BDAT data found. […] Done https://review.coreboot.org/c/coreboot/+/77255/comment/e43a3763_5fdb192e : PS15, Line 77: ld > zu Done https://review.coreboot.org/c/coreboot/+/77255/comment/0f54d42f_1055175d : PS15, Line 78: (void *) > Is that case really necessary ? Done https://review.coreboot.org/c/coreboot/+/77255/comment/0c28405e_b1bd5ffd : PS15, Line 86: if (guid != NULL) > reduce indentation: […] Done https://review.coreboot.org/c/coreboot/+/77255/comment/961f431f_29c89c3f : PS15, Line 86: if (guid != NULL) { > How can `guid` be `NULL` ? Done https://review.coreboot.org/c/coreboot/+/77255/comment/3d30374a_436c5bb2 : PS15, Line 88: uint8_t *p = (uint8_t *)guid; > This intermediate variable seems unnecessary. Done https://review.coreboot.org/c/coreboot/+/77255/comment/30be2b6e_efc49f86 : PS15, Line 91: ld > zu Done https://review.coreboot.org/c/coreboot/+/77255/comment/4c18a0e7_1836ee67 : PS15, Line 92: (void *) schema[schema_count], hob_size); > weird indentation. Done https://review.coreboot.org/c/coreboot/+/77255/comment/c8374445_d60e754d : PS15, Line 146: ld > zu Done https://review.coreboot.org/c/coreboot/+/77255/comment/eca3d54a_a01b2a04 : PS15, Line 155: ) b > Coding style: Usually there is no space between the closing parenthesis of the cast and the name of […] Done https://review.coreboot.org/c/coreboot/+/77255/comment/7616221d_a5164d1c : PS15, Line 162: &bdat_header->bios_data_sig[0] > `bdat_header->bios_data_sig` ? Done https://review.coreboot.org/c/coreboot/+/77255/comment/b4effdb8_f078c793 : PS15, Line 174: schema_data > unnecessary intermediate variable. Done File src/soc/intel/common/block/include/intelblocks/acpi_bdat.h: https://review.coreboot.org/c/coreboot/+/77255/comment/9faa9d36_889f2023 : PS15, Line 69: > that's quite a log of tabs. A single space would do it. Done -- 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: 16 Gerrit-Owner: Gaggery Tsai <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-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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> 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: Sun, 24 Mar 2024 18:54:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Gaggery Tsai <gaggery.tsai(a)intel.com> Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/rex: Enable Intel RMT+
by Gaggery Tsai (Code Review) 24 Mar '24

24 Mar '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Gaggery Tsai, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun. Hello Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Subrata Banik, Tarun, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/77684?usp=email to look at the new patch set (#10). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: mb/google/rex: Enable Intel RMT+ ...................................................................... mb/google/rex: Enable Intel RMT+ This patch enables Intel RMT+ support for Rex platform. BUG=b:293441360 TEST=1. Build FSP with -DBDAT_SUPPORT=1. 2. Build an image with this patch 3. Deploy the image to a Rex device 4. Ensure /sys/firmware/acpi/tables/BDAT is exported from kernel. Change-Id: I4e56c32cee1ce8c10b9da9d0123cbd13eaf9d8d3 Signed-off-by: Gaggery Tsai <gaggery.tsai(a)intel.com> --- M src/mainboard/google/rex/Kconfig 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/77684/10 -- To view, visit https://review.coreboot.org/c/coreboot/+/77684?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: I4e56c32cee1ce8c10b9da9d0123cbd13eaf9d8d3 Gerrit-Change-Number: 77684 Gerrit-PatchSet: 10 Gerrit-Owner: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Gaggery Tsai <gaggery.tsai(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024