coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3270 discussions

[XS] Change in coreboot[main]: mb/erying/tgl: Drop specifying which timers to use
by Angel Pons (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Alicja Michalska, Matt DeVillier. Angel Pons has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/86093?usp=email ) Change subject: mb/erying/tgl: Drop specifying which timers to use ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86093/comment/fca2da78_4504d469?us… : PS2, Line 1: Parent: 9dee482a (mb/google/nissa/var/telith: Configure Acoustic noise mitigation) > Yes, I did test it with X2APIC before and everything was fine (while default was giving me random is […] I'm confused, the Kconfigs in question aren't about X2APIC -- To view, visit https://review.coreboot.org/c/coreboot/+/86093?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: If6e0ac1d289447c292a49111251d321c951078e2 Gerrit-Change-Number: 86093 Gerrit-PatchSet: 2 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 22:14:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com> Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[XS] Change in coreboot[main]: soc/intel/alderlake/romstage: Disable UFS controller on every boot
by Karthik Ramasubramanian (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Bob Moragues, Dinesh Gehlot, Intel coreboot Reviewers, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Subrata Banik. Karthik Ramasubramanian has posted comments on this change by Karthik Ramasubramanian. ( https://review.coreboot.org/c/coreboot/+/86111?usp=email ) Change subject: soc/intel/alderlake/romstage: Disable UFS controller on every boot ...................................................................... Patch Set 5: (1 comment) File src/soc/intel/alderlake/romstage/romstage.c: https://review.coreboot.org/c/coreboot/+/86111/comment/5c4511ee_f74d65b5?us… : PS4, Line 202: ps->prev_sleep_state == ACPI_S5 > When the isssue happens, the previous sleep state is read as S0. […] Done. Here in https://review.coreboot.org/c/coreboot/+/86170/1 -- To view, visit https://review.coreboot.org/c/coreboot/+/86111?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: I1d7089a90bc42a2b36c26b05b81c1a2f12861fdc Gerrit-Change-Number: 86111 Gerrit-PatchSet: 5 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.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: 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-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Wentao Qin <qinwentao(a)huaqin.corp-partner.google.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Bob Moragues <moragues(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Mon, 27 Jan 2025 21:58:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>

1 0

[XS] Change in coreboot[main]: soc/intel/alderlake/romstage: Update UFS disable sequence
by Karthik Ramasubramanian (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Dinesh Gehlot, Intel coreboot Reviewers, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Subrata Banik. Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86170?usp=email ) Change subject: soc/intel/alderlake/romstage: Update UFS disable sequence ...................................................................... soc/intel/alderlake/romstage: Update UFS disable sequence Currently after UFS is disabled, if the device is coming out of S5 sleep state then a warm reset is triggered such that PMC samples the UFS function disable bit and disables the UFS controller accordingly. Sometimes during the boot flow, an additional kind of reset gets triggered - Power cycle Reset through CMoff. Hence initiate a warm reset when the host comes out of S5 sleep state or Power cycle Reset through CMoff. BUG=b:391449110 TEST=Build Brox BIOS image and boot to OS. Ensure that when the device switches from normal mode to developer mode an extra warm reset is triggered such that the UFS controller is disabled. Change-Id: I85cad1a1eb00a2a7f520a57cda789ad6737fcb97 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/soc/intel/alderlake/romstage/romstage.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/86170/1 diff --git a/src/soc/intel/alderlake/romstage/romstage.c b/src/soc/intel/alderlake/romstage/romstage.c index 2f07236..f84a8a1 100644 --- a/src/soc/intel/alderlake/romstage/romstage.c +++ b/src/soc/intel/alderlake/romstage/romstage.c @@ -199,7 +199,9 @@ (CONFIG(USE_UNIFIED_AP_FIRMWARE_FOR_UFS_AND_NON_UFS))) { printk(BIOS_INFO, "Disabling UFS controllers\n"); disable_ufs(); - if (ps->prev_sleep_state == ACPI_S5 && !mainboard_expects_another_reset()) { + if ((ps->prev_sleep_state == ACPI_S5 || + cse_match_current_pm_event(PWR_CYCLE_RESET_CMOFF)) && + !mainboard_expects_another_reset()) { printk(BIOS_INFO, "Warm Reset after disabling UFS controllers\n"); system_reset(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/86170?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I85cad1a1eb00a2a7f520a57cda789ad6737fcb97 Gerrit-Change-Number: 86170 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jayvik Desai <jayvik(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-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>

1 0

[S] Change in coreboot[main]: soc/intel/common/block/cse: Add API to match current PM event
by Karthik Ramasubramanian (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Intel coreboot Reviewers. Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86169?usp=email ) Change subject: soc/intel/common/block/cse: Add API to match current PM event ...................................................................... soc/intel/common/block/cse: Add API to match current PM event Introduce an API to read the CSME host firmware status register for current Power Management event and match it against an input event. BUG=b:391449110 TEST=Build Brox BIOS image and boot to OS. Change-Id: Ie9a49382ee2c1a8f59da6233e510cf2e38ac32ad Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/include/intelblocks/cse.h 2 files changed, 25 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/86169/1 diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index 6389044..61913b7 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -298,6 +298,21 @@ return !hfs1.fields.mfg_mode; } +#define ME_HFSTS2_CUR_PM_EVENT_SHIFT 24 +#define ME_HFSTS2_CUR_PM_EVENT_MASK (0xF << ME_HFSTS2_CUR_PM_EVENT_SHIFT) +static uint8_t cse_get_hfs2_current_pm_event(void) +{ + union me_hfsts2 hfs2; + hfs2.data = me_read_config32(PCI_ME_HFSTS2); + return (uint8_t)((hfs2.data & ME_HFSTS2_CUR_PM_EVENT_MASK) >> + ME_HFSTS2_CUR_PM_EVENT_SHIFT); +} + +bool cse_match_current_pm_event(uint8_t event) +{ + return cse_get_hfs2_current_pm_event() == event; +} + bool cse_is_hfs3_fw_sku_lite(void) { union me_hfsts3 hfs3; diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h index c97e4ec..cc7a458 100644 --- a/src/soc/intel/common/block/include/intelblocks/cse.h +++ b/src/soc/intel/common/block/include/intelblocks/cse.h @@ -383,6 +383,10 @@ CSE_RESET_ONLY = 3, }; +enum cse_fw_sts_current_pm_event { + PWR_CYCLE_RESET_CMOFF = 0xB, +}; + /* * Sends GLOBAL_RESET_REQ cmd to CSE with reset type GLOBAL_RESET. * Returns 0 on failure and 1 on success. @@ -618,4 +622,10 @@ */ bool is_cse_boot_to_rw(void); +/* + * Check if the CSE FW Status Current Power Management Event matches with input. + * Returns true if the current pm event matches, false otherwise. + */ +bool cse_match_current_pm_event(uint8_t event); + #endif // SOC_INTEL_COMMON_CSE_H -- To view, visit https://review.coreboot.org/c/coreboot/+/86169?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie9a49382ee2c1a8f59da6233e510cf2e38ac32ad Gerrit-Change-Number: 86169 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>

1 0

[XS] Change in coreboot[main]: soc/intel/alderlake/romstage: Disable UFS controller on every boot
by Karthik Ramasubramanian (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Bob Moragues, Dinesh Gehlot, Intel coreboot Reviewers, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Subrata Banik. Hello Bob Moragues, Dinesh Gehlot, Intel coreboot Reviewers, Jayvik Desai, 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/+/86111?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Code-Review+1 by Bob Moragues, Verified+1 by build bot (Jenkins) Change subject: soc/intel/alderlake/romstage: Disable UFS controller on every boot ...................................................................... soc/intel/alderlake/romstage: Disable UFS controller on every boot On boards with non-UFS storage, currently UFS controller is disabled only when the system is coming out of S5 sleep state. On certain kind of resets during boot flow, this leaves some UFS resources turned on and causing the suspend failure. To fix this issue, disable UFS controller on every boot cycle. BUG=b:391449110 TEST=Build Brox BIOS image and boot to OS. Ensure that when the device switches from normal mode to developer mode and vice-versa, the device is able to suspend/resume successfully. Change-Id: I1d7089a90bc42a2b36c26b05b81c1a2f12861fdc Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/soc/intel/alderlake/romstage/romstage.c 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/86111/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/86111?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: I1d7089a90bc42a2b36c26b05b81c1a2f12861fdc Gerrit-Change-Number: 86111 Gerrit-PatchSet: 5 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.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: 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-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Wentao Qin <qinwentao(a)huaqin.corp-partner.google.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Bob Moragues <moragues(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>

1 0

[M] Change in coreboot[main]: soc/intel: Allow zero values for PMC GPE0 DW registers
by Matt DeVillier (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Eric Lai, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Pranava Y N, Sean Rhodes, Subrata Banik, Tarun, Werner Zeh. Matt DeVillier has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86164?usp=email ) Change subject: soc/intel: Allow zero values for PMC GPE0 DW registers ...................................................................... Patch Set 6: Code-Review+1 (1 comment) Patchset: PS6: patchset 6 does resolve the assert error, which I should note only halts booting if CONFIG_FATAL_ASSERTS is set, which it isn't by default -- To view, visit https://review.coreboot.org/c/coreboot/+/86164?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: Ie66d6dbcf49d5400b3fc3e4da113a569fe52dd51 Gerrit-Change-Number: 86164 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: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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-Attention: Eric Lai <ericllai(a)google.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: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 20:45:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: soc/intel: Allow zero values for PMC GPE0 DW registers
by Matt DeVillier (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Eric Lai, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Nick Vaccaro, Pranava Y N, Sean Rhodes, Tarun, Werner Zeh. Matt DeVillier has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86164?usp=email ) Change subject: soc/intel: Allow zero values for PMC GPE0 DW registers ...................................................................... Patch Set 5: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/a08e6039_fd2e7cd0?us… : PS5, Line 11: This prevented platforms from : disabling GPE routing via PMC by setting all DW values to zero. no, it prevented platforms from using the default GPE routing by not programming the routes in devicetree File src/soc/intel/alderlake/pmutil.c: https://review.coreboot.org/c/coreboot/+/86164/comment/8f0615de_04c1b85e?us… : PS3, Line 164: override > > do we really need this here? there's already another printk in this case (PMC: Using default GPE route.) > > `PMC: Using default GPE route` this often mislead as default value from baseboard but doesn't convey if we are using all zero value which is ideally what you wish to avoid here. All zero values for all dwords is not valid. right, but the zeros are not being programmed - see `pmc_gpe_init` in `src/soc/intel/common/block/pmc/pmclib.c`. If all zeros are passed in, then dw0 == dw1 == dw2, and the POR values are read from `GPIO_GPE_CFG` and programmed into `MISCCFG`. -- To view, visit https://review.coreboot.org/c/coreboot/+/86164?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: Ie66d6dbcf49d5400b3fc3e4da113a569fe52dd51 Gerrit-Change-Number: 86164 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: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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-Attention: Eric Lai <ericllai(a)google.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: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 20:28:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>

1 0

[M] Change in coreboot[main]: soc/intel: Allow zero values for PMC GPE0 DW registers
by Subrata Banik (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Eric Lai, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Pranava Y N, Sean Rhodes, Tarun, Werner Zeh. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86164?usp=email ) Change subject: soc/intel: Allow zero values for PMC GPE0 DW registers ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/035a43c9_ff374030?us… : PS5, Line 21: - **All DWs zero:** A warning message is printed, and the system boots > It is unclear how incorporating markdown syntax enhances the quality of a commit message. […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/86164?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: Ie66d6dbcf49d5400b3fc3e4da113a569fe52dd51 Gerrit-Change-Number: 86164 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: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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-Attention: Eric Lai <ericllai(a)google.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: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 20:25:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[M] Change in coreboot[main]: soc/intel: Allow zero values for PMC GPE0 DW registers
by Subrata Banik (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Eric Lai, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Pranava Y N, Sean Rhodes, Subrata Banik, Tarun, Werner Zeh. Hello Dinesh Gehlot, Eran Mitrani, Eric Lai, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Pranava Y N, Sean Rhodes, Tarun, Werner Zeh, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86164?usp=email to look at the new patch set (#6). Change subject: soc/intel: Allow zero values for PMC GPE0 DW registers ...................................................................... soc/intel: Allow zero values for PMC GPE0 DW registers The `pmc_gpe0_different_values` function previously asserted if any two of the GPE0 DW registers (DW0, DW1, DW2) had the same value, as introduced in commit 640a41f3ee938b794b14 ("soc/intel: Assert if `pmc_/gpe0_dwX` values are not unique"). This prevented platforms from disabling GPE routing via PMC by setting all DW values to zero. This commit modifies the check to allow all DW registers to be zero. This enables platforms that don't require PMC-controlled GPE routing to boot without triggering the assertion. A warning message is printed if all DW values are zero, indicating that dwX value is set to 0. The change was verified by testing the following scenarios: - All DWs zero: A warning message is printed, and the system boots using the default GPE route. No assertion occurs. - Duplicate DWs (e.g., DW0=1, DW1=2, DW2=2): The existing assertion is triggered as expected. - Unique DWs (e.g., DW0=1, DW1=2, DW2=3): No errors occur. TEST=Built and booted normally. No assertion failure observed. Change-Id: Ie66d6dbcf49d5400b3fc3e4da113a569fe52dd51 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/soc/intel/alderlake/pmutil.c M src/soc/intel/apollolake/pmutil.c M src/soc/intel/cannonlake/pmutil.c M src/soc/intel/elkhartlake/pmutil.c M src/soc/intel/jasperlake/pmutil.c M src/soc/intel/meteorlake/pmutil.c M src/soc/intel/pantherlake/pmutil.c M src/soc/intel/skylake/pmutil.c M src/soc/intel/tigerlake/pmutil.c 9 files changed, 90 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/86164/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/86164?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: Ie66d6dbcf49d5400b3fc3e4da113a569fe52dd51 Gerrit-Change-Number: 86164 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: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>

1 0

[M] Change in coreboot[main]: soc/intel: Allow zero values for PMC GPE0 DW registers
by Jérémy Compostella (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Eric Lai, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Matt DeVillier, Nick Vaccaro, Pranava Y N, Sean Rhodes, Subrata Banik, Tarun, Werner Zeh. Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86164?usp=email ) Change subject: soc/intel: Allow zero values for PMC GPE0 DW registers ...................................................................... Patch Set 5: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/ba64427e_22a34cea?us… : PS2, Line 14: This prevents potential issues arising from a zeroed configuration > > The commit message still needs some update in my opinion: […] Thank you fixing the logic. And for the explaining that it avoids an a assertion when platform decides to not configure GPE and use all DWs set to zero value in the commit message. Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/e27c1eed_3f91921c?us… : PS5, Line 21: - **All DWs zero:** A warning message is printed, and the system boots It is unclear how incorporating markdown syntax enhances the quality of a commit message. From my perspective, this addition may compromise readability, considering commit messages are typically consumed as plain ASCII text. Furthermore, the inclusion of such syntax could potentially complicate search operations within commit messages. -- To view, visit https://review.coreboot.org/c/coreboot/+/86164?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: Ie66d6dbcf49d5400b3fc3e4da113a569fe52dd51 Gerrit-Change-Number: 86164 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: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.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-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 20:18:54 +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> Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025