Attention is currently required from: Cliff Huang, David Ruth, Lance Zhao, Nico Huber, Paul Menzel, Subrata Banik, Tim Wawrzynczak.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80170?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: Add MTCL function to ACPI SSDT tables
......................................................................
Patch Set 14:
(2 comments)
File src/drivers/wifi/generic/mtcl.c:
https://review.coreboot.org/c/coreboot/+/80170/comment/e06bb933_efe9128d :
PS14, Line 9: WIFI_MTCL_CBFS_DEFAULT_FILENAME "wifi_mtcl.hex"
nit: can we use `WIFI_MTCL_CBFS_FILEPATH` config directly here ? (instead of defining a new macro)?
just update the default value directly to the WIFI_MTCL_CBFS_FILEPATH config
https://review.coreboot.org/c/coreboot/+/80170/comment/0a8e4ff1_7215ac42 :
PS14, Line 10: #define MAX_VERSION 2
: #define MAX_SUPPORT_STATE 2
: #define COUNTRY_LIST_SIZE 6
: #define NAME_SIZE 4
: #define MTCL_NAME "MTCL"
nit: wondering if moving these macros into mtcl.h makes sense.?
--
To view, visit https://review.coreboot.org/c/coreboot/+/80170?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: I9b5e7312a44e114270e664b983626faa6cfee350
Gerrit-Change-Number: 80170
Gerrit-PatchSet: 14
Gerrit-Owner: David Ruth <druth(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com>
Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: David Ruth <druth(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Comment-Date: Thu, 01 Feb 2024 05:17:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov, Arthur Heymans, Bora Guvendik, Christian Walter, Felix Held, Fred Reitberger, Jason Glenesk, Johnny Lin, Jérémy Compostella, Lean Sheng Tan, Matt DeVillier, Patrick Rudolph, Ronak Kanabar, Shuo Liu, Tim Chu, Wonkyu Kim.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80277?usp=email )
Change subject: drivers/intel/fsp2_0: Support FSP 2.4 64-bits
......................................................................
Patch Set 2:
(15 comments)
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/80277/comment/b7cac58d_5d46c73c :
PS2, Line 62: Starting with FSP specification 2.4 they can be 64-bits.
i thought u said elsewhere that FSP 2.4 also supports 32-bit?
base don line#58, I'm unable to follow how can one select 32-bit flow of FSP2.4?
File src/drivers/intel/fsp2_0/debug.c:
https://review.coreboot.org/c/coreboot/+/80277/comment/a99d878f_bd6645fc :
PS2, Line 94: efi_return_status_t
need to include `#include <efi/efi_datatype.h>`
https://review.coreboot.org/c/coreboot/+/80277/comment/94fc20ac_b4afbb7a :
PS2, Line 97: status
why not use `size_t status` directly ?
File src/drivers/intel/fsp2_0/include/fsp/api.h:
https://review.coreboot.org/c/coreboot/+/80277/comment/d533c158_ac7a74d7 :
PS2, Line 11: #define __efiapi EFIAPI
why not inside `efi_datatype.h` file ?
File src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h:
https://review.coreboot.org/c/coreboot/+/80277/comment/5af5574e_b4bbae99 :
PS2, Line 13: #include <efi/efi_datatype.h>
order ?
File src/drivers/intel/fsp2_0/include/fsp/info_header.h:
https://review.coreboot.org/c/coreboot/+/80277/comment/aa66f7a4_94cfe2a2 :
PS2, Line 39: #if CONFIG(PLATFORM_USES_FSP2_4)
look at line #36, i don;t think u need a guard here as well. we are not using sizeof(fsp_header) anywhere IMO
File src/drivers/intel/fsp2_0/include/fsp/soc_binding.h:
https://review.coreboot.org/c/coreboot/+/80277/comment/8cd4782c_ddb89ade :
PS2, Line 8: #if CONFIG(VENDOR_INTEL)
why Intel? this is UEFI calling convention and nothing SoC vendor specific
also use edi_datatype.h for keeping UEFI specific declaration
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/80277/comment/1472693a_20af67e1 :
PS2, Line 282: efi_return_status_t
size_t?
File src/include/efi/efi_datatype.h:
https://review.coreboot.org/c/coreboot/+/80277/comment/026ab103_0a14713b :
PS2, Line 81: statuses
???
https://review.coreboot.org/c/coreboot/+/80277/comment/da9a9bfc_123a4adc :
PS2, Line 88: _Static_assert(sizeof(size_t) == sizeof(efi_return_status_t),
: "Unexpected EFI_STATUS size");
if u use `size_t` over `efi_return_status_t` then I don't believe u need this logic
File src/soc/intel/xeon_sp/bootblock.c:
https://review.coreboot.org/c/coreboot/+/80277/comment/7648b325_394a4246 :
PS2, Line 5: #include <fsp/util.h>
please submit this cl separately
https://review.coreboot.org/c/coreboot/+/80277/comment/aba490be_58eccd89 :
PS2, Line 9: #include <soc/iomap.h>
same
https://review.coreboot.org/c/coreboot/+/80277/comment/6fd5b5ba_9a5d1e1f :
PS2, Line 10: #include <console/console.h>
: #include <cpu/x86/mtrr.h>
this is also out of order
https://review.coreboot.org/c/coreboot/+/80277/comment/cdb7e30c_9249f016 :
PS2, Line 12: #include <intelblocks/lpc_lib.h>
same
https://review.coreboot.org/c/coreboot/+/80277/comment/2fdf612b_9c1fc96b :
PS2, Line 15: #include <soc/bootblock.h>
same
--
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: 2
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: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
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: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
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-Comment-Date: Thu, 01 Feb 2024 04:42:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anil Kumar K, Arthur Heymans, Bora Guvendik, Chen, Gang C, Cliff Huang, Gaggery, Jamie Ryu, Jérémy Compostella, Kane Chen, Karthik Ramasubramanian, Krishna P Bhat D, Lance Zhao, Pratikkumar V Prajapati, Ravishankar Sarawadi, Ronak Kanabar, Subrata Banik, Tim Wawrzynczak, V Sowmya, Wonkyu Kim.
Shuo Liu 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:
(4 comments)
Patchset:
PS14:
Hello, we are from server coreboot team and would like to adopt this change, with some comments. Could you please check?
File src/soc/intel/common/block/acpi/acpi_bdat.c:
https://review.coreboot.org/c/coreboot/+/77255/comment/acebef54_ff25913d :
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 copy could be a pattern to apply.
File src/soc/intel/common/block/include/intelblocks/acpi_bdat.h:
https://review.coreboot.org/c/coreboot/+/77255/comment/dee03acd_63b636f2 :
PS14, Line 66: EFI_GUID schema_hob_guids[MAX_SCHEMA_LIST_LENGTH];
Can we use base/size instead? a.k.a. some customers will create schema by themselves instead of reading HOBs from FSP (e.g. to create something beyonds FSP created HOBs, by reading registers in coreboot codes, which might be private codes, or public codes), thus giving customers some flexibility in their implementation instead of raising requests to FSP.
https://review.coreboot.org/c/coreboot/+/77255/comment/d29a4a4e_3fbcad4d :
PS14, Line 67: } bdat_schema_list_hob;
Can we rename it as bdat_schema_list? a.k.a. an intel platform might use arbitrary way to collect the schema list but may not from a hob.
--
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: Gaggery <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: 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 04:39:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Jason Glenesk, Martin L Roth.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73158?usp=email )
Change subject: Docs: Replace Recommonmark with MyST Parser
......................................................................
Patch Set 6: -Code-Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/73158?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: I0837c1722fa56d25c9441ea218e943d8f3d9b804
Gerrit-Change-Number: 73158
Gerrit-PatchSet: 6
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Maslowski <info(a)orangecms.org>
Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 01 Feb 2024 04:31:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Qinghong Zeng, Subrata Banik.
Weimin Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80278?usp=email )
Change subject: mb/google/byra/var/anraggar: Set WLAN device type back to pci
......................................................................
Patch Set 8: Code-Review+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/80278?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: I7a33362e28c3b9027e9cc5a2e11dc0c316acbd69
Gerrit-Change-Number: 80278
Gerrit-PatchSet: 8
Gerrit-Owner: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
Gerrit-Comment-Date: Thu, 01 Feb 2024 03:36:49 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, Weimin Wu.
Hello Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, Weimin Wu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80278?usp=email
to look at the new patch set (#8).
Change subject: mb/google/byra/var/anraggar: Set WLAN device type back to pci
......................................................................
mb/google/byra/var/anraggar: Set WLAN device type back to pci
Refers to commit 4b957b9665
("mb/google/byra/var/*: Set WLAN device type back to pci"). Set the WLAN device type to pci uniformly.
BUG=none
TEST=The verification function is normal, and it can pass the FAFT test.
Change-Id: I7a33362e28c3b9027e9cc5a2e11dc0c316acbd69
Signed-off-by: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
---
M src/mainboard/google/brya/variants/anraggar/overridetree.cb
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/80278/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/80278?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: I7a33362e28c3b9027e9cc5a2e11dc0c316acbd69
Gerrit-Change-Number: 80278
Gerrit-PatchSet: 8
Gerrit-Owner: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, Weimin Wu.
Hello Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, Weimin Wu, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80278?usp=email
to look at the new patch set (#7).
Change subject: mb/google/byra/var/anraggar: Set WLAN device type back to pci
......................................................................
mb/google/byra/var/anraggar: Set WLAN device type back to pci
This part refers to commit 4b957b9665
("mb/google/byra/var/*: Set WLAN device type back to pci"). Set the WLAN device type to pci uniformly.
BUG=null
TEST= The verification function is normal, and it can pass the FAFT test.
Change-Id: I7a33362e28c3b9027e9cc5a2e11dc0c316acbd69
Signed-off-by: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
---
M src/mainboard/google/brya/variants/anraggar/overridetree.cb
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/80278/7
--
To view, visit https://review.coreboot.org/c/coreboot/+/80278?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: I7a33362e28c3b9027e9cc5a2e11dc0c316acbd69
Gerrit-Change-Number: 80278
Gerrit-PatchSet: 7
Gerrit-Owner: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Andrey Petrov, Bora Guvendik, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Ronak Kanabar, Tarun, Wonkyu Kim.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80275?usp=email )
Change subject: drivers/intel/fsp2_0: Add limited to 32-bits FSP 2.4 support
......................................................................
Patch Set 6:
(5 comments)
Patchset:
PS3:
> > quick feedback
> > 1. This CL doesn't mentioned the test vehicle used to verify
> Test vehicle is not public. Considering the 2.4 specification is public, we can provide the implementation.
it doesn't matter but a TEST statement is basic req for any CL to land. Like I'm able to test this CL on some XYZ platform.
>
> > 2. specify the FSP 2.4 spec link
>
> I gave the document number but I did not add the link as links can change.
There are two doc numbers in the commit msg hence, I'm unable to understand which one is the spec.
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/80275/comment/76a035f2_9bc0a445 :
PS3, Line 58: default n if PLATFORM_USES_FSP2_4
> 2.4 also supports 32-bits.
if FSP 2.4 supports both 32/64bit execution hence, I'm unable to follow how one can still select `PLATFORM_USES_FSP2_X86_32` when PLATFORM_USES_FSP2_4 is enabled ?
I had an impression that FSP2.4 enforces only 64-bit execution
I can see patchset 6 fixes this problem.
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/80275/comment/c7bbd33e_3bfe2a68 :
PS3, Line 282: error_handler
> Considering we are in memory_init.c and this is static function I don't see the benefit of surcharging the function name with unnecessary prefix.
the `error_hanlder` function name is very ambiguous as it doesn't clarify the intention of calling this function. adding `fsp_` prefix at minimum is req to make this complete
https://review.coreboot.org/c/coreboot/+/80275/comment/8809c7c1_a73eac5d :
PS3, Line 297: multi_phase_init
> Considering we are in memory_init.c and this is static function I don't see the benefit of surcharging the function name with unnecessary prefix.
this to improve the code readability also,
https://review.coreboot.org/c/coreboot/+/80275/comment/01d3d332_2dba7f2e/https://review.coreboot.org/c/coreboot/+/80275/comment/608cd31b_70929ec3 :
PS3, Line 437: if (hdr->fsp_multi_phase_mem_init_entry_offset)
> Wouldn't it be dangerous to look outside after the limit of the data structure ? These fields are added with 2.4.
in that case, check the FSP version index and call into the `fsp_multi_phase_init`. preprocessor macro is not required here IMO and we don't use such unless absolute necessary.
--
To view, visit https://review.coreboot.org/c/coreboot/+/80275?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: I1c24d26e105c3dcbd9cca0e7197ab1362344aa97
Gerrit-Change-Number: 80275
Gerrit-PatchSet: 6
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(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: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.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-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Thu, 01 Feb 2024 03:31:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik.
Hello Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/80278?usp=email
to look at the new patch set (#6).
Change subject: mb/google/byra/var/anraggar: Set WLAN device type back to pci
......................................................................
mb/google/byra/var/anraggar: Set WLAN device type back to pci
This part refers to commit 4b957b9665
("mb/google/byra/var/*: Set WLAN device type back to pci"). Set the WLAN device type to pci uniformly.
BUG=NA
TEST= Can be compiled normally, the verification function is normal, and it can pass the FAFT test.
Change-Id: I7a33362e28c3b9027e9cc5a2e11dc0c316acbd69
Signed-off-by: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
---
M src/mainboard/google/brya/variants/anraggar/overridetree.cb
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/80278/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/80278?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: I7a33362e28c3b9027e9cc5a2e11dc0c316acbd69
Gerrit-Change-Number: 80278
Gerrit-PatchSet: 6
Gerrit-Owner: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-MessageType: newpatchset