coreboot-gerrit September 2024

coreboot-gerrit@coreboot.org

1 participants
3342 discussions

[S] Change in coreboot[main]: soc/intel: Correct return type of fsp_get_pch_reset_status()
by Jérémy Compostella (Code Review) 27 Sep '24

27 Sep '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Karthik Ramasubramanian, Pranava Y N, Subrata Banik, Tarun, Werner Zeh. Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84571?usp=email ) Change subject: soc/intel: Correct return type of fsp_get_pch_reset_status() ...................................................................... Patch Set 6: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/84571?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0cdee541506bf424f50fd00833d5ee200a3a8a48 Gerrit-Change-Number: 84571 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.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: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(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: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 27 Sep 2024 21:42:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: arch/x86: Define macros for hard-coded HPET registers
by Jérémy Compostella (Code Review) 27 Sep '24

27 Sep '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu, yuchi.chen(a)intel.com. Jérémy Compostella has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/84252?usp=email ) Change subject: arch/x86: Define macros for hard-coded HPET registers ...................................................................... Patch Set 5: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84252/comment/10daae61_127e4e3f?us… : PS5, Line 11: HEPT HPET ? https://review.coreboot.org/c/coreboot/+/84252/comment/bd488bf0_bd8bad87?us… : PS5, Line 14: date the `date` word is unnecessary File src/arch/x86/include/arch/hpet.h: https://review.coreboot.org/c/coreboot/+/84252/comment/8f4d420c_6ea4d115?us… : PS5, Line 10: Date Same here, I don't think the work `Date` brings any value. https://review.coreboot.org/c/coreboot/+/84252/comment/0803cbd5_65f779e0?us… : PS5, Line 17: #define HPET_TIMER_FSB_EN_CNF_MASK (1 << 15) BIT(15) ? -- To view, visit https://review.coreboot.org/c/coreboot/+/84252?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I31413afcbfc42307e3ad3f99d75f33f87092d7aa Gerrit-Change-Number: 84252 Gerrit-PatchSet: 5 Gerrit-Owner: yuchi.chen(a)intel.com Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Fri, 27 Sep 2024 21:31:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: soc/intel/common/block/imc: Add Integrated Memory Controller driver
by Jérémy Compostella (Code Review) 27 Sep '24

27 Sep '24

Attention is currently required from: Felix Singer, Paul Menzel, Shuo Liu, yuchi.chen(a)intel.com. Jérémy Compostella has posted comments on this change by yuchi.chen(a)intel.com. ( https://review.coreboot.org/c/coreboot/+/83320?usp=email ) Change subject: soc/intel/common/block/imc: Add Integrated Memory Controller driver ...................................................................... Patch Set 23: (7 comments) File src/soc/intel/common/block/imc/imc.c: https://review.coreboot.org/c/coreboot/+/83320/comment/15671360_6239805e?us… : PS23, Line 64: B1E:D0B:F0 should you print values from `dev` ? https://review.coreboot.org/c/coreboot/+/83320/comment/88843f51_9ced3d95?us… : PS23, Line 91: { unnecessary brackets. https://review.coreboot.org/c/coreboot/+/83320/comment/a59f5dff_10545bc3?us… : PS23, Line 119: poll_ready(dev, &status); shouldn't an error message be printed if this fails ? https://review.coreboot.org/c/coreboot/+/83320/comment/6f51c941_2c3824cc?us… : PS23, Line 173: ret = -1; `ret` is necessarily still `-1`. BTW, there is a mix of `-1` and `CB_ERR`, could you use `CB_ERR` everywhere ? https://review.coreboot.org/c/coreboot/+/83320/comment/b05c9b10_34d7bd37?us… : PS23, Line 192: ret = 0; If you use `CB_ERR` you should also use `CB_SUCCESS` File src/soc/intel/common/block/imc/imclib.h: https://review.coreboot.org/c/coreboot/+/83320/comment/a0371689_f4154869?us… : PS23, Line 13: = 0, unnecessary first value. I know you are just copying over so it may sound unfair to do clean that up. File src/soc/intel/common/block/include/intelblocks/imc.h: https://review.coreboot.org/c/coreboot/+/83320/comment/b28f3142_6fdb248c?us… : PS23, Line 4: #include <stdint.h> this one is not needed anymore. -- To view, visit https://review.coreboot.org/c/coreboot/+/83320?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3f47ddeda94d3882852d64c0052f8fb42b6b7ad2 Gerrit-Change-Number: 83320 Gerrit-PatchSet: 23 Gerrit-Owner: yuchi.chen(a)intel.com Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Vasiliy Khoruzhick <vasilykh(a)arista.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Comment-Date: Fri, 27 Sep 2024 21:29:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: soc/intel: Deprecate SoC-specific global reset status configs
by Subrata Banik (Code Review) 27 Sep '24

27 Sep '24

1 0

[M] Change in coreboot[main]: soc/intel: Deprecate SoC-specific global reset status configs
by Subrata Banik (Code Review) 27 Sep '24

27 Sep '24

Attention is currently required from: Andrey Petrov, Christian Walter, David Hendricks, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Karthik Ramasubramanian, Lean Sheng Tan, Nick Vaccaro, Nico Huber, Pranava Y N, Ronak Kanabar, Sean Rhodes, Tarun, Werner Zeh. Hello Andrey Petrov, Christian Walter, David Hendricks, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Karthik Ramasubramanian, Lean Sheng Tan, Nick Vaccaro, Nico Huber, Pranava Y N, Ronak Kanabar, Sean Rhodes, Tarun, Werner Zeh, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84572?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/intel: Deprecate SoC-specific global reset status configs ...................................................................... soc/intel: Deprecate SoC-specific global reset status configs This change removes the SoC-specific `FSP_STATUS_GLOBAL_RESET_REQUIRED_X` Kconfigs, as they are no longer necessary for handling FSP global reset requests. Previously, these Kconfigs were used to select a specific 32-bit reset status code. However, with the introduction of FSP 2.4 and 64-bit interfaces, the global reset status code can now vary between architectures. To address this, the FSP driver now sets the `FSP_STATUS_GLOBAL_RESET` config to a common default value (depending upon most commonly used global reset status code) based on the interface: - 0x40000003 for 32-bit FSP interfaces - 0x4000000000000003 for 64-bit FSP interfaces This default can be overridden if an FSP implementation uses a different status code (for example: Apollo Lake selects different FSP reset status code as 0x40000005). By removing the SoC-specific configurations, this change simplifies global reset handling and ensures compatibility across different FSP versions and platforms. Below table shows the relationship between Platform, FSP and FSP Global Reset Status: +-----------------+--------------+-------------------------+ | Platform | FSP | Global Reset Status | +-----------------+--------------+-------------------------+ | Alder Lake | 32-bit | 0x40000003 | +-----------------+--------------+-------------------------+ | Apollo Lake | 32-bit | 0x40000005 | +-----------------+--------------+-------------------------+ | Cannon Lake | 32-bit | 0x40000003 | +-----------------+--------------+-------------------------+ | Elkhart Lake | 32-bit | 0x40000003 | +-----------------+--------------+-------------------------+ | Jasper Lake | 32-bit | 0x40000003 | +-----------------+--------------+-------------------------+ | Meteor Lake | 32-bit | 0x40000003 | +-----------------+--------------+-------------------------+ | Sky Lake | 32-bit | 0x40000003 | +-----------------+--------------+-------------------------+ | Tiger Lake | 32-bit | 0x40000003 | +-----------------+--------------+-------------------------+ | Panther Lake | 64-bit | 0x4000000000000003 | +-----------------+--------------+-------------------------+ BUG=b:347669091 TEST=Verified FSP requested global reset functionality on google/rex0 (32-bit) and google/rex64 (64-bit) platforms. w/ 32-bit FSP: ``` (Wdt) AllowKnownReset [FspResetSystem2] FSP Reset Initiated FSP returning control to Bootloader with reset required return status 40000003 FSPS, status=0x40000003 FSP: handling reset type, status=0x40000003 GLOBAL RESET! global_reset() called! HECI: Global Reset(Type:1) Command ``` w/ 64-bit FSP: ``` (Wdt) AllowKnownReset [FspResetSystem2] FSP Reset Initiated FSP returning control to Bootloader with reset required return status 3 FSPS, status=0x4000000000000003 FSP: handling reset type, status=0x4000000000000003 GLOBAL RESET! global_reset() called! HECI: Global Reset(Type:1) Command ``` Change-Id: I32bdbf7ea6afa7d5e5f91ea96d887719d26a593f Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/alderlake/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/jasperlake/Kconfig M src/soc/intel/meteorlake/Kconfig M src/soc/intel/pantherlake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/intel/tigerlake/Kconfig 10 files changed, 23 insertions(+), 48 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/84572/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/84572?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I32bdbf7ea6afa7d5e5f91ea96d887719d26a593f Gerrit-Change-Number: 84572 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.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: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>

1 0

[M] Change in coreboot[main]: soc/intel: Deprecate SoC-specific global reset status configs
by Subrata Banik (Code Review) 27 Sep '24

27 Sep '24

1 0

[S] Change in coreboot[main]: soc/intel: Correct return type of fsp_get_pch_reset_status()
by Subrata Banik (Code Review) 27 Sep '24

27 Sep '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jérémy Compostella, Kapil Porwal, Karthik Ramasubramanian, Pranava Y N, Tarun, Werner Zeh. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84571?usp=email ) Change subject: soc/intel: Correct return type of fsp_get_pch_reset_status() ...................................................................... Patch Set 5: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84571/comment/b885976e_92b8cc11?us… : PS5, Line 9: an > a ? Acknowledged File src/soc/intel/common/reset.h: https://review.coreboot.org/c/coreboot/+/84571/comment/87bf8040_ae7793e1?us… : PS5, Line 7: #include <fsp/api.h> > Why do you also include <fsp/api.h> ? this is a mistake that I have initially added `fsp/api.h` but later felt `efi/efi_datatype.h` is more applicable here. -- To view, visit https://review.coreboot.org/c/coreboot/+/84571?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0cdee541506bf424f50fd00833d5ee200a3a8a48 Gerrit-Change-Number: 84571 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.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: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.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: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 27 Sep 2024 21:15:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[S] Change in coreboot[main]: soc/intel: Correct return type of fsp_get_pch_reset_status()
by Subrata Banik (Code Review) 27 Sep '24

27 Sep '24

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Karthik Ramasubramanian, Pranava Y N, Subrata Banik, Tarun, Werner Zeh. Hello Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Kapil Porwal, Karthik Ramasubramanian, Pranava Y N, Tarun, Werner Zeh, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84571?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/intel: Correct return type of fsp_get_pch_reset_status() ...................................................................... soc/intel: Correct return type of fsp_get_pch_reset_status() The `fsp_get_pch_reset_status()` function returns a FSP reset status code. This change corrects its return type from `uint32_t` to `efi_return_status_t` to ensure consistency with the FSP API and prevent potential issues caused by type mismatch. This correction is necessary for compatibility with both 32-bit and 64-bit FSP interfaces. The change also updates all callers of this function in the Meteor Lake and Panther Lake SoCs to use the correct return type. Includes `fsp/api.h` to provide the `efi_return_status_t` definition. BUG=b:347669091 TEST=Verified global reset functionality on google/rex0 (32-bit) and google/rex64 (64-bit) platforms. Change-Id: I0cdee541506bf424f50fd00833d5ee200a3a8a48 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/soc/intel/common/fsp_reset.c M src/soc/intel/common/reset.h M src/soc/intel/meteorlake/chip.c M src/soc/intel/pantherlake/chip.c 4 files changed, 11 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/84571/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/84571?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0cdee541506bf424f50fd00833d5ee200a3a8a48 Gerrit-Change-Number: 84571 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.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: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(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: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com>

1 0

[S] Change in coreboot[main]: mb/google/fatcat/var/fatcat: add support for wifi sar table
by Subrata Banik (Code Review) 27 Sep '24

27 Sep '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84556?usp=email ) Change subject: mb/google/fatcat/var/fatcat: add support for wifi sar table ...................................................................... mb/google/fatcat/var/fatcat: add support for wifi sar table Add wifi sar table support for fatcat. Bit 4-5 in CBI/FW_CONFIG is used to select different sar table (index 0 to 3). BUG=b:348678529 TEST=emerge-fatcat coreboot chromeos-bootimage Change-Id: I2d82f76d7c11378ee5c221a6b9621b4cba83720d Signed-off-by: YH Lin <yueherngl(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/84556 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> Reviewed-by: Pranava Y N <pranavayn(a)google.com> --- M src/mainboard/google/fatcat/Kconfig M src/mainboard/google/fatcat/variants/fatcat/Makefile.mk A src/mainboard/google/fatcat/variants/fatcat/variant.c 3 files changed, 23 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Pranava Y N: Looks good to me, approved Subrata Banik: Looks good to me, approved diff --git a/src/mainboard/google/fatcat/Kconfig b/src/mainboard/google/fatcat/Kconfig index 42c4063..cf4233a 100644 --- a/src/mainboard/google/fatcat/Kconfig +++ b/src/mainboard/google/fatcat/Kconfig @@ -8,6 +8,8 @@ select EC_GOOGLE_CHROMEEC_BOARDID select EC_GOOGLE_CHROMEEC_ESPI select EC_GOOGLE_CHROMEEC_SKUID + select FW_CONFIG + select FW_CONFIG_SOURCE_CHROMEEC_CBI select GOOGLE_SMBIOS_MAINBOARD_VERSION select HAVE_ACPI_TABLES select I2C_TPM @@ -25,6 +27,7 @@ config BOARD_GOOGLE_BASEBOARD_FATCAT def_bool n select BOARD_GOOGLE_FATCAT_COMMON + select CHROMEOS_WIFI_SAR if CHROMEOS select HAVE_SLP_S0_GATE select MAINBOARD_HAS_CHROMEOS select MEMORY_SOLDERDOWN @@ -55,6 +58,14 @@ select GBB_FLAG_FORCE_MANUAL_RECOVERY select HAS_RECOVERY_MRC_CACHE +config CHROMEOS_WIFI_SAR + bool "Enable SAR options for ChromeOS build" + depends on CHROMEOS + select DSAR_ENABLE + select GEO_SAR_ENABLE + select SAR_ENABLE + select USE_SAR + config DEVICETREE default "variants/baseboard/\$(CONFIG_BASEBOARD_DIR)/devicetree.cb" diff --git a/src/mainboard/google/fatcat/variants/fatcat/Makefile.mk b/src/mainboard/google/fatcat/variants/fatcat/Makefile.mk index 4c33dad..88986e4 100644 --- a/src/mainboard/google/fatcat/variants/fatcat/Makefile.mk +++ b/src/mainboard/google/fatcat/variants/fatcat/Makefile.mk @@ -4,3 +4,4 @@ romstage-y += gpio.c romstage-y += memory.c ramstage-y += gpio.c +ramstage-y += variant.c diff --git a/src/mainboard/google/fatcat/variants/fatcat/variant.c b/src/mainboard/google/fatcat/variants/fatcat/variant.c new file mode 100644 index 0000000..8893753 --- /dev/null +++ b/src/mainboard/google/fatcat/variants/fatcat/variant.c @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <acpi/acpigen.h> +#include <baseboard/variants.h> +#include <fw_config.h> +#include <sar.h> + +const char *get_wifi_sar_cbfs_filename(void) +{ + return get_wifi_sar_fw_config_filename(FW_CONFIG_FIELD(WIFI)); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/84556?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2d82f76d7c11378ee5c221a6b9621b4cba83720d Gerrit-Change-Number: 84556 Gerrit-PatchSet: 4 Gerrit-Owner: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in coreboot[main]: mb/google/fatcat/var/fatcat: Add initial FW_CONFIG
by Subrata Banik (Code Review) 27 Sep '24

27 Sep '24

Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/84543?usp=email ) Change subject: mb/google/fatcat/var/fatcat: Add initial FW_CONFIG ...................................................................... Patch Set 6: (5 comments) File src/mainboard/google/fatcat/variants/fatcat/overridetree.cb: https://review.coreboot.org/c/coreboot/+/84543/comment/49bf2844_f76a26b5?us… : PS6, Line 5: option ALC722_SNDW 2 > ALC722 is built-in. should we remove AUDIO_NONE and make ALC722_SNDW as default with value '0'? no, AUDIO_NONE is for our lab device without audio may be. I don't see any change required marking it resolved https://review.coreboot.org/c/coreboot/+/84543/comment/bc27fdd4_ba175bf2?us… : PS6, Line 20: field TOUCHSCREEN 8 10 > Can we drop prefix [field]_ from most of fw_config options? that is, > using LPSS_I2C instead of TOUCHSCREEN_LPSS_I2C, for instance. > > with current option naming, we will have: > if (fw_config_probe(FW_CONFIG(TOUCHSCREEN, TOUCHSCREEN_LPSS_I2C))) { ... > > This will reach 96 characters easily with indentation. Also, it would be expanded in marco and resulting the following in static.c: > .field_name = FW_CONFIG_FIELD_TOUCHSCREEN_NAME, > .option_name = FW_CONFIG_FIELD_TOUCHSCREEN_OPTION_TOUCHSCREEN_LPSS_I2C_NAME, > .mask = FW_CONFIG_FIELD_TOUCHSCREEN_MASK, > .value = FW_CONFIG_FIELD_TOUCHSCREEN_OPTION_TOUCHCREEN_LPSS_I2C_VALUE, > > The option name will not be used as marco directly so we are save and won't run into naming conflict. > > same thing for the options in other filed with [field]_ as prefix. we can always line break, that is not a motivation for changing the prefix. we use prefix as standard practice In FW config. marking it resolved https://review.coreboot.org/c/coreboot/+/84543/comment/a2837660_452dc758?us… : PS6, Line 25: option TOUCHSCREEN_THC0_I2C 4 > either THC0_SPI in line 24 or THC_I2C for consistency. my bad, I will fix it. keeping it open. idea was not to say the bus number https://review.coreboot.org/c/coreboot/+/84543/comment/c53d3364_c2cd1201?us… : PS6, Line 37: field STORAGE 15 16 > This is okay, but we are not limited to have single storage. For instance, two NVMe at the same time. what is the motivation of having two SSD working at the same time ? https://review.coreboot.org/c/coreboot/+/84543/comment/0292a0fa_2bf56111?us… : PS6, Line 55: end > do we need WWAN for fatcat? read line #15, `CELLULAR` -- To view, visit https://review.coreboot.org/c/coreboot/+/84543?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5c90aac4873dcc57e65e641656dca3a96f84d6b8 Gerrit-Change-Number: 84543 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Comment-Date: Fri, 27 Sep 2024 21:09:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2024