coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3270 discussions

[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: Eran Mitrani, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Jérémy Compostella, Matt DeVillier, Nick Vaccaro, Pranava Y N, 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 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/4c3446e9_375f708e?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. > > agreed > > > But according to the `TEST` flag in the commit message, it seems that this CL is helping. How is it possible ? > > good question - I've not yet tested it myself acknowledged, looks like i have build for a platform without using zero value to case this scenario (therefore no assertion). ``` /* Check if all values are different AND not all zero */ 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; ``` -- 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: 2 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: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(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: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 18:40:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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

[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: 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/5dca8013_124c1136?us… : PS2, Line 1: Parent: 9dee482a (mb/google/nissa/var/telith: Configure Acoustic noise mitigation) > can we get an explanation why / what the effect of doing so is? No idea, I simply wrote the patch and submitted it. Maybe it's just a leftover after a local rebase? -- 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-Comment-Date: Mon, 27 Jan 2025 18:37:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>

1 0

[S] Change in coreboot[main]: payloads/external/iPXE: introduce support for named configurations
by Filip Lewiński (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Filip Lewiński, Krystian Hebel, Martin L Roth, Michał Kopeć. Hello Krystian Hebel, Martin L Roth, Michał Kopeć, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82041?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: payloads/external/iPXE: introduce support for named configurations ...................................................................... payloads/external/iPXE: introduce support for named configurations This commit adds support for iPXE's named configuration mechanism, allowing for optional customization of configuration headers without modifying iPXE source files directly. Named configurations align with iPXE's documented practices and provide a structured way to apply customizations, while keeping the source tree clean. - A new `payloads/external/iPXE/named-configs` directory was introduced to hold named configuration files (e.g., `general.h`, `console.h`, `serial.h`) for specific use cases. - The `Makefile` was updated to: - Copy named configuration files into `src/config/local/<name>` within the iPXE build directory during the build process, if a named configuration is explicitly enabled via `CONFIG_IPXE_NAMED_CONFIG`. - Retain the existing `sed`-based modifications to configuration files for legacy and default workflows. - A new Kconfig option (`CONFIG_IPXE_NAMED_CONFIG`) was added to allow users to specify a named configuration. If unset, the named configuration logic is skipped entirely, ensuring no impact on existing builds. The previous workflow relied entirely on `sed` commands to modify iPXE header files during the build. While this approach works, it directly modifies the source tree, which: - Can complicate branch switching or repository updates. - Does not utilize iPXE's native named configuration mechanism, which provides a cleaner and more maintainable way to handle customizations. Named configurations offer an alternative that avoids directly altering iPXE's source files. This is particularly useful for maintaining reproducible and isolated builds. While the existing `sed` logic is preserved, named configurations could potentially replace it in the future. - No changes were made to the default workflow. Builds that do not specify `CONFIG_IPXE_NAMED_CONFIG` continue to rely on the existing `sed` logic. - The named configuration mechanism is entirely optional and will only apply if explicitly enabled by setting `CONFIG_IPXE_NAMED_CONFIG` in the platform's defconfig. To use a named configuration: 1. Create a directory under `payloads/external/iPXE/named-configs/<name>` with custom header files (e.g., `general.h`, `console.h`). 2. Set `CONFIG_IPXE_NAMED_CONFIG="<name>"` in the platform's defconfig. 3. Build as usual. If no named configuration is specified, the build process remains unchanged. Change-Id: Ibf7c2d9e4407035bfdf0115f4628f23b38272656 Signed-off-by: Filip Lewiński <filip.lewinski(a)3mdeb.com> --- M payloads/external/iPXE/Kconfig M payloads/external/iPXE/Makefile A payloads/external/iPXE/named-configs/.gitkeep 3 files changed, 35 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/82041/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/82041?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: Ibf7c2d9e4407035bfdf0115f4628f23b38272656 Gerrit-Change-Number: 82041 Gerrit-PatchSet: 6 Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>

1 0

[S] Change in coreboot[main]: payloads/external/iPXE: introduce support for named configurations
by Filip Lewiński (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Filip Lewiński, Krystian Hebel, Martin L Roth, Michał Kopeć. Hello Krystian Hebel, Martin L Roth, Michał Kopeć, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82041?usp=email to look at the new patch set (#5). Change subject: payloads/external/iPXE: introduce support for named configurations ...................................................................... payloads/external/iPXE: introduce support for named configurations This commit adds support for iPXE's named configuration mechanism, allowing for optional customization of configuration headers without modifying iPXE source files directly. Named configurations align with iPXE's documented practices and provide a structured way to apply customizations, while keeping the source tree clean. - A new `payloads/external/iPXE/named-configs` directory was introduced to hold named configuration files (e.g., `general.h`, `console.h`, `serial.h`) for specific use cases. - The `Makefile` was updated to: - Copy named configuration files into `src/config/local/<name>` within the iPXE build directory during the build process, if a named configuration is explicitly enabled via `CONFIG_IPXE_NAMED_CONFIG`. - Retain the existing `sed`-based modifications to configuration files for legacy and default workflows. - A new Kconfig option (`CONFIG_IPXE_NAMED_CONFIG`) was added to allow users to specify a named configuration. If unset, the named configuration logic is skipped entirely, ensuring no impact on existing builds. The previous workflow relied entirely on `sed` commands to modify iPXE header files during the build. While this approach works, it directly modifies the source tree, which: - Can complicate branch switching or repository updates. - Does not utilize iPXE's native named configuration mechanism, which provides a cleaner and more maintainable way to handle customizations. Named configurations offer an alternative that avoids directly altering iPXE's source files. This is particularly useful for maintaining reproducible and isolated builds. While the existing `sed` logic is preserved, named configurations could potentially replace it in the future. - No changes were made to the default workflow. Builds that do not specify `CONFIG_IPXE_NAMED_CONFIG` continue to rely on the existing `sed` logic. - The named configuration mechanism is entirely optional and will only apply if explicitly enabled by setting `CONFIG_IPXE_NAMED_CONFIG` in the platform's defconfig. To use a named configuration: 1. Create a directory under `payloads/external/iPXE/named-configs/<name>` with custom header files (e.g., `general.h`, `console.h`). 2. Set `CONFIG_IPXE_NAMED_CONFIG="<name>"` in the platform's defconfig. 3. Build as usual. If no named configuration is specified, the build process remains unchanged. Change-Id: Ibf7c2d9e4407035bfdf0115f4628f23b38272656 Signed-off-by: Filip Lewiński <filip.lewinski(a)3mdeb.com> --- M payloads/external/iPXE/Kconfig M payloads/external/iPXE/Makefile A payloads/external/iPXE/named-configs/.gitkeep 3 files changed, 35 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/82041/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/82041?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: Ibf7c2d9e4407035bfdf0115f4628f23b38272656 Gerrit-Change-Number: 82041 Gerrit-PatchSet: 5 Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>

1 0

[S] Change in coreboot[main]: payloads/external/iPXE: introduce support for named configurations
by Filip Lewiński (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Filip Lewiński, Krystian Hebel, Martin L Roth, Michał Kopeć. Hello Krystian Hebel, Martin L Roth, Michał Kopeć, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82041?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: payloads/external/iPXE: introduce support for named configurations ...................................................................... payloads/external/iPXE: introduce support for named configurations This commit adds support for iPXE's named configuration mechanism, allowing for optional customization of configuration headers without modifying iPXE source files directly. Named configurations align with iPXE's documented practices and provide a structured way to apply customizations, while keeping the source tree clean. - A new `payloads/external/iPXE/named-configs` directory was introduced to hold named configuration files (e.g., `general.h`, `console.h`, `serial.h`) for specific use cases. - The `Makefile` was updated to: - Copy named configuration files into `src/config/local/<name>` within the iPXE build directory during the build process, if a named configuration is explicitly enabled via `CONFIG_IPXE_NAMED_CONFIG`. - Retain the existing `sed`-based modifications to configuration files for legacy and default workflows. - A new Kconfig option (`CONFIG_IPXE_NAMED_CONFIG`) was added to allow users to specify a named configuration. If unset, the named configuration logic is skipped entirely, ensuring no impact on existing builds. The previous workflow relied entirely on `sed` commands to modify iPXE header files during the build. While this approach works, it directly modifies the source tree, which: - Can complicate branch switching or repository updates. - Does not utilize iPXE's native named configuration mechanism, which provides a cleaner and more maintainable way to handle customizations. Named configurations offer an alternative that avoids directly altering iPXE's source files. This is particularly useful for maintaining reproducible and isolated builds. While the existing `sed` logic is preserved, named configurations could potentially replace it in the future. - No changes were made to the default workflow. Builds that do not specify `CONFIG_IPXE_NAMED_CONFIG` continue to rely on the existing `sed` logic. - The named configuration mechanism is entirely optional and will only apply if explicitly enabled by setting `CONFIG_IPXE_NAMED_CONFIG` in the platform's defconfig. To use a named configuration: 1. Create a directory under `payloads/external/iPXE/named-configs/<name>` with custom header files (e.g., `general.h`, `console.h`). 2. Set `CONFIG_IPXE_NAMED_CONFIG="<name>"` in the platform's defconfig. 3. Build as usual. If no named configuration is specified, the build process remains unchanged. Change-Id: Ibf7c2d9e4407035bfdf0115f4628f23b38272656 --- M payloads/external/iPXE/Kconfig M payloads/external/iPXE/Makefile A payloads/external/iPXE/named-configs/.gitkeep 3 files changed, 35 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/82041/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/82041?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: Ibf7c2d9e4407035bfdf0115f4628f23b38272656 Gerrit-Change-Number: 82041 Gerrit-PatchSet: 4 Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>

1 0

[S] Change in coreboot[main]: payloads/external/iPXE: introduce support for named configurations
by Filip Lewiński (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Filip Lewiński, Krystian Hebel, Martin L Roth, Michał Kopeć. Hello Krystian Hebel, Martin L Roth, Michał Kopeć, Michał Żygowski, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82041?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: payloads/external/iPXE: introduce support for named configurations ...................................................................... payloads/external/iPXE: introduce support for named configurations This commit adds support for iPXE's named configuration mechanism, allowing for optional customization of configuration headers without modifying iPXE source files directly. Named configurations align with iPXE's documented practices and provide a structured way to apply customizations while keeping the source tree clean. - A new `payloads/external/iPXE/named-configs` directory was introduced to hold named configuration files (e.g., `general.h`, `console.h`, `serial.h`) for specific use cases. - The `Makefile` was updated to: - Copy named configuration files into `src/config/local/<name>` within the iPXE build directory during the build process, if a named configuration is explicitly enabled via `CONFIG_IPXE_NAMED_CONFIG`. - Retain the existing `sed`-based modifications to configuration files for legacy and default workflows. - A new Kconfig option (`CONFIG_IPXE_NAMED_CONFIG`) was added to allow users to specify a named configuration. If unset, the named configuration logic is skipped entirely, ensuring no impact on existing builds. The previous workflow relied entirely on `sed` commands to modify iPXE header files during the build. While this approach works, it directly modifies the source tree, which: - Can complicate branch switching or repository updates. - Does not utilize iPXE's native named configuration mechanism, which provides a cleaner and more maintainable way to handle customizations. Named configurations offer an alternative that avoids directly altering iPXE's source files. This is particularly useful for maintaining reproducible and isolated builds. While the existing `sed` logic is preserved, named configurations could potentially replace it in the future. - No changes were made to the default workflow. Builds that do not specify `CONFIG_IPXE_NAMED_CONFIG` continue to rely on the existing `sed` logic. - The named configuration mechanism is entirely optional and will only apply if explicitly enabled by setting `CONFIG_IPXE_NAMED_CONFIG` in the platform's defconfig. To use a named configuration: 1. Create a directory under `payloads/external/iPXE/named-configs/<name>` with custom header files (e.g., `general.h`, `console.h`). 2. Set `CONFIG_IPXE_NAMED_CONFIG="<name>"` in the platform's defconfig. 3. Build as usual. If no named configuration is specified, the build process remains unchanged. Change-Id: Ibf7c2d9e4407035bfdf0115f4628f23b38272656 --- M payloads/external/iPXE/Kconfig M payloads/external/iPXE/Makefile A payloads/external/iPXE/named-configs/.gitkeep 3 files changed, 35 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/82041/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/82041?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: Ibf7c2d9e4407035bfdf0115f4628f23b38272656 Gerrit-Change-Number: 82041 Gerrit-PatchSet: 3 Gerrit-Owner: Filip Lewiński <filip.lewinski(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Filip Lewiński <filip.lewinski(a)3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.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: Eran Mitrani, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Nick Vaccaro, 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/+/86164?usp=email ) Change subject: soc/intel: Add check to pmc_/gpe0_different_values ...................................................................... Patch Set 2: Code-Review-1 -- 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: 2 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: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 17:56:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

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: Eran Mitrani, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Nick Vaccaro, Pranava Y N, 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 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/1b587f12_28094f41?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. agreed > But according to the `TEST` flag in the commit message, it seems that this CL is helping. How is it possible ? good question - I've not yet tested it myself -- 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: 2 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: 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-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 17:54:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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: Eran Mitrani, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Nick Vaccaro, 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/+/86164?usp=email ) Change subject: soc/intel: Add check to pmc_/gpe0_different_values ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/3c458ce8_d1298365?us… : PS2, Line 14: This prevents potential issues arising from a zeroed configuration > logic error, should be `result = different || all_zero` Both the commit message and the code are wrong to me then. But according to the `TEST` flag in the commit message, it seems that this CL is helping. How is it possible ? -- 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: 2 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: 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-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 17:47:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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 Matt DeVillier (Code Review) 27 Jan '25

27 Jan '25

Attention is currently required from: Eran Mitrani, Intel coreboot Reviewers, Jakub Czapiga, Jayvik Desai, Nick Vaccaro, Pranava Y N, 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 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/86164/comment/7311c407_b7635be5?us… : PS2, Line 14: This prevents potential issues arising from a zeroed configuration > I am trying here to wake up my brain. […] logic error, should be `result = 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: 2 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: 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-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 27 Jan 2025 17:41:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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