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 Alicja Michalska (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Angel Pons, Matt DeVillier. Alicja Michalska 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/488b19a1_c3e9e9b4?us… : PS2, Line 1: Parent: 9dee482a (mb/google/nissa/var/telith: Configure Acoustic noise mitigation) > Do you know why the timer Kconfigs were selected in the first place? Does this change have any notic […] Yes, I did test it with X2APIC before and everything was fine (while default was giving me random issues), but few days ago I've noticed that Samsung 990 Pro 2TB didn't like X2APIC, and would randomly drop off the bus in OS -- 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: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 20:13:59 +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]: 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: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86093/comment/ad9d8439_d8e2cea9?us… : PS2, Line 1: Parent: 9dee482a (mb/google/nissa/var/telith: Configure Acoustic noise mitigation) > No idea, I simply wrote the patch and submitted it. […] Do you know why the timer Kconfigs were selected in the first place? Does this change have any noticeable impact on the functioning of the mainboard? -- 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 20:11:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.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. 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 (#5). 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/5 -- 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: 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: 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>

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 3: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/2b985d39_d269acd2?us… : PS2, Line 14: This prevents potential issues arising from a zeroed configuration > The commit message still needs some update in my opinion: > 1. the describe logic is wrong and not aligned with the code fixed > 2. It does not really explain the issue being fixed I don't believe this is any problem in the existing code as in one platform can't configure all GPE DWs to zero value which is also a valid GPIO community. So, technically this is not fixing any problem. It avoids a assertion when platform decides to not configure GPE and use all DWs set to zero value. > 3. It should probably point out to https://review.coreboot.org/c/coreboot/+/85161 sure File src/soc/intel/alderlake/pmutil.c: https://review.coreboot.org/c/coreboot/+/86164/comment/fc0a0de0_94a33963?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. https://review.coreboot.org/c/coreboot/+/86164/comment/d2cf0a58_e1f06449?us… : PS3, Line 167: bool result = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && : (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw2) && : (config->pmc_gpe0_dw1 != config->pmc_gpe0_dw2) || all_zero; : : assert(result); > slightly cleaner? […] 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: 3 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:03:39 +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

[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, 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 (#4). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) 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 hash 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/4 -- 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: 4 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: 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: Add check to pmc_/gpe0_different_values
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, 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: Add check to pmc_/gpe0_different_values ...................................................................... Patch Set 3: (2 comments) File src/soc/intel/alderlake/pmutil.c: https://review.coreboot.org/c/coreboot/+/86164/comment/179cc196_fa25346f?us… : PS3, Line 164: override do we really need this here? there's already another printk in this case (PMC: Using default GPE route.) https://review.coreboot.org/c/coreboot/+/86164/comment/8393ac32_f175b80e?us… : PS3, Line 167: bool result = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && : (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw2) && : (config->pmc_gpe0_dw1 != config->pmc_gpe0_dw2) || all_zero; : : assert(result); slightly cleaner? ```suggestion bool all_different = (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw1) && (config->pmc_gpe0_dw0 != config->pmc_gpe0_dw2) && (config->pmc_gpe0_dw1 != config->pmc_gpe0_dw2); assert(all_different || all_zero); ``` -- 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: 3 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: 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 19:09:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: soc/intel: Add check to pmc_/gpe0_different_values
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: Add check to pmc_/gpe0_different_values ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/bffab20e_506390ec?us… : PS2, Line 14: This prevents potential issues arising from a zeroed configuration > The commit message still needs some update. The commit message still needs some update in my opinion: 1. the describe logic is wrong and not aligned with the code 2. It does not really explain the issue being fixed 3. It should probably point out to https://review.coreboot.org/c/coreboot/+/85161 -- 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: 3 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 18:57:34 +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

[M] Change in coreboot[main]: soc/intel: Add check to pmc_/gpe0_different_values
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: Add check to pmc_/gpe0_different_values ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/8058c26e_34043c3f?us… : PS2, Line 14: This prevents potential issues arising from a zeroed configuration > @matt, can you please test the latest patchset? The commit message still needs some update. -- 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: 3 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 18:55:20 +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

[M] Change in coreboot[main]: soc/intel: Add check to pmc_/gpe0_different_values
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: Add check to pmc_/gpe0_different_values ...................................................................... Patch Set 3: (2 comments) Patchset: PS2: > right, zero is a valid GPE route. […] Acknowledged Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/7eded514_39da2c85?us… : PS2, Line 14: This prevents potential issues arising from a zeroed configuration > > > Both the commit message and the code are wrong to me then. […] @matt, can you please test the latest patchset? -- 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: 3 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 18:51:41 +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> Comment-In-Reply-To: Kapil Porwal <kapilporwal(a)google.com>

1 0

[M] Change in coreboot[main]: soc/intel: Add check to pmc_/gpe0_different_values
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, Matt DeVillier, Nick Vaccaro, Pranava Y N, Sean Rhodes, 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 (#3). The following approvals got outdated and were removed: Code-Review+2 by Dinesh Gehlot, Code-Review+2 by Eric Lai, Code-Review+2 by Sean Rhodes, Code-Review-1 by Jérémy Compostella, Verified+1 by build bot (Jenkins) Change subject: soc/intel: Add check to pmc_/gpe0_different_values ...................................................................... soc/intel: Add check to pmc_/gpe0_different_values The pmc_/gpe0_different_values function previously only checked if the dw0, dw1, and dw2 values were different from each other. This commit adds a check to ensure that the values are not all zero, as this can represent an invalid configuration. This prevents potential issues arising from a zeroed configuration by ensuring the assertion only passes if the values are different and at least one of them is non-zero. 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, 81 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/86164/3 -- 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: 3 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: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025