coreboot-gerrit February 2025

coreboot-gerrit@coreboot.org

1 participants
2886 discussions

[M] Change in coreboot[main]: [This Must Fail]Upgrade GCC to 15-20250202 Snapshot
by Felix Singer (Code Review) 06 Feb '25

06 Feb '25

Attention is currently required from: Elyes Haouas, Julius Werner, Martin L Roth, Maximilian Brune. Felix Singer has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/86100?usp=email ) Change subject: [This Must Fail]Upgrade GCC to 15-20250202 Snapshot ...................................................................... Patch Set 5: (1 comment) Patchset: PS1: > Jenkins has a serious problem ! Thank you for noticing the issue and having a precise eye for it, but please calm down. As you can see in the coreboot-toolchain builder log the toolchain build fails. Jenkins continues building with the included toolchain though, because it's not told to stop and to create a proper failure report for the toolchain build. It worked and it was the case earlier, but at some point people did changes to the coreboot-toolchain builder and now it's broken. The issue is known. I already brought it up months ago, but well.. I don't comment on that. So the issue still persists. However, when testing new toolchains I just look into the build log and verify that it's actually being built and succeeded, which **should** be done in any case as the toolchain is a critical part of coreboot and one should never fully rely on the CI. -- To view, visit https://review.coreboot.org/c/coreboot/+/86100?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: I5e6c3745eebeb6ded494c153010283b761498184 Gerrit-Change-Number: 86100 Gerrit-PatchSet: 5 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Fri, 07 Feb 2025 05:15:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[S] Change in coreboot[main]: mb/google/fatcat: Set frequency and gears for SaGv work points
by Subrata Banik (Code Review) 06 Feb '25

06 Feb '25

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86274?usp=email ) Change subject: mb/google/fatcat: Set frequency and gears for SaGv work points ...................................................................... mb/google/fatcat: Set frequency and gears for SaGv work points Update sagv gears and frequency values as per recommendation from power and performance team. BUG=none TEST=Boot to OS. Signed-off-by: Bora Guvendik <bora.guvendik(a)intel.com> Change-Id: I315fcac387680df9312880120b7e6d33bded38e0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/86274 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M src/mainboard/google/fatcat/variants/baseboard/fatcat/devicetree.cb 1 file changed, 12 insertions(+), 0 deletions(-) Approvals: Subrata Banik: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/mainboard/google/fatcat/variants/baseboard/fatcat/devicetree.cb b/src/mainboard/google/fatcat/variants/baseboard/fatcat/devicetree.cb index 342c534..c217a15 100644 --- a/src/mainboard/google/fatcat/variants/baseboard/fatcat/devicetree.cb +++ b/src/mainboard/google/fatcat/variants/baseboard/fatcat/devicetree.cb @@ -33,6 +33,18 @@ # Enable SAGv register "sagv" = "SAGV_ENABLED" + register "sagv_freq_mhz[0]" = "2400" + register "sagv_gear[0]" = "GEAR_4" + + register "sagv_freq_mhz[1]" = "3200" + register "sagv_gear[1]" = "GEAR_4" + + register "sagv_freq_mhz[2]" = "6000" + register "sagv_gear[2]" = "GEAR_4" + + register "sagv_freq_mhz[3]" = "6400" + register "sagv_gear[3]" = "GEAR_4" + # Enable s0ix register "s0ix_enable" = "true" -- To view, visit https://review.coreboot.org/c/coreboot/+/86274?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I315fcac387680df9312880120b7e6d33bded38e0 Gerrit-Change-Number: 86274 Gerrit-PatchSet: 6 Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.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: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in coreboot[main]: soc/intel/pantherlake: Add ability to set SaGv work points
by Subrata Banik (Code Review) 06 Feb '25

06 Feb '25

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86277?usp=email ) Change subject: soc/intel/pantherlake: Add ability to set SaGv work points ...................................................................... soc/intel/pantherlake: Add ability to set SaGv work points Hook up SaGv work point UPDs. BUG=none TEST=Boot to OS. Signed-off-by: Bora Guvendik <bora.guvendik(a)intel.com> Change-Id: Ie38d007edc293727066f2bc9f67037e6fbe77aa5 Reviewed-on: https://review.coreboot.org/c/coreboot/+/86277 Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M src/soc/intel/pantherlake/chip.h M src/soc/intel/pantherlake/romstage/fsp_params.c 2 files changed, 13 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Jérémy Compostella: Looks good to me, approved diff --git a/src/soc/intel/pantherlake/chip.h b/src/soc/intel/pantherlake/chip.h index e879fad..cd91e31 100644 --- a/src/soc/intel/pantherlake/chip.h +++ b/src/soc/intel/pantherlake/chip.h @@ -42,6 +42,14 @@ uint16_t region_mask[MAX_IBECC_REGIONS]; }; +/* SaGv gears */ +enum soc_intel_pantherlake_sagv_gears { + GEAR_AUTO = 0, + GEAR_1 = 1, + GEAR_2 = 2, + GEAR_4 = 4, +}; + enum soc_intel_pantherlake_power_limits { PTL_U_1_CORE, PTL_H_1_CORE, diff --git a/src/soc/intel/pantherlake/romstage/fsp_params.c b/src/soc/intel/pantherlake/romstage/fsp_params.c index 5b96891..0c17d02 100644 --- a/src/soc/intel/pantherlake/romstage/fsp_params.c +++ b/src/soc/intel/pantherlake/romstage/fsp_params.c @@ -73,6 +73,11 @@ m_cfg->SaGvWpMask = config->sagv_wp_bitmap; else m_cfg->SaGvWpMask = SAGV_POINTS_0_1_2_3; + + for (size_t i = 0; i < HOB_MAX_SAGV_POINTS; i++) { + m_cfg->SaGvFreq[i] = config->sagv_freq_mhz[i]; + m_cfg->SaGvGear[i] = config->sagv_gear[i]; + } } if (config->max_dram_speed_mts) -- To view, visit https://review.coreboot.org/c/coreboot/+/86277?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie38d007edc293727066f2bc9f67037e6fbe77aa5 Gerrit-Change-Number: 86277 Gerrit-PatchSet: 4 Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.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: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in coreboot[main]: [This Must Fail]Upgrade GCC to 15-20250202 Snapshot
by Elyes Haouas (Code Review) 06 Feb '25

06 Feb '25

1 0

[M] Change in coreboot[main]: [This Must Fail]Upgrade GCC to 15-20250202 Snapshot
by Elyes Haouas (Code Review) 06 Feb '25

06 Feb '25

Attention is currently required from: Felix Singer, Julius Werner, Martin L Roth, Maximilian Brune. Elyes Haouas has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/86100?usp=email ) Change subject: [This Must Fail]Upgrade GCC to 15-20250202 Snapshot ...................................................................... Patch Set 5: (1 comment) Patchset: PS4: > The fact that you think the new toolchain should fail using jenkins means for me that you already tr […] I don't use gnat, you libgnat is probably old. and for sure, Jenkins has a serious problem!! here what I have for ga-945gcm-s2l build using GCC15: `SUCCESS: Wrote 182 bytes to file 'build/fmap.fmap' (and generated header) The sections containing CBFSes are: COREBOOT CC romstage/mainboard/gigabyte/ga-945gcm-s2l/static.o In file included from src/include/console/console.h:11, from src/include/device/device.h:7, from build/mainboard/gigabyte/ga-945gcm-s2l/static.c:2: src/commonlib/include/commonlib/loglevel.h:170:26: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization] 170 | [BIOS_EMERG] = "EMERG", | ^~~~~~~ src/commonlib/include/commonlib/loglevel.h:171:26: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization] 171 | [BIOS_ALERT] = "ALERT", | ^~~~~~~ src/commonlib/include/commonlib/loglevel.h:172:26: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization] 172 | [BIOS_CRIT] = "CRIT ", | ^~~~~~~ src/commonlib/include/commonlib/loglevel.h:173:26: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization] 173 | [BIOS_ERR] = "ERROR", | ^~~~~~~ src/commonlib/include/commonlib/loglevel.h:174:26: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization] 174 | [BIOS_WARNING] = "WARN ", | ^~~~~~~ src/commonlib/include/commonlib/loglevel.h:175:26: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization] 175 | [BIOS_NOTICE] = "NOTE ", | ^~~~~~~ src/commonlib/include/commonlib/loglevel.h:176:26: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization] 176 | [BIOS_INFO] = "INFO ", | ^~~~~~~ src/commonlib/include/commonlib/loglevel.h:177:26: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization] 177 | [BIOS_DEBUG] = "DEBUG", | ^~~~~~~ src/commonlib/include/commonlib/loglevel.h:178:26: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-initialization] 178 | [BIOS_SPEW] = "SPEW ", | ^~~~~~~ cc1: all warnings being treated as errors ` Again, here the problem is not Werror=unterminated-string-initialization error, but Jenkins is completely and definitely wrong!!! -- To view, visit https://review.coreboot.org/c/coreboot/+/86100?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: I5e6c3745eebeb6ded494c153010283b761498184 Gerrit-Change-Number: 86100 Gerrit-PatchSet: 5 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Fri, 07 Feb 2025 03:35:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[M] Change in coreboot[main]: lib: Refactor ux_locales_get_text API
by Jérémy Compostella (Code Review) 06 Feb '25

06 Feb '25

Attention is currently required from: Dinesh Gehlot, Eran Mitrani, Intel coreboot Reviewers, Jakub "Kuba" Czapiga, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Subrata Banik, Tarun. Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86283?usp=email ) Change subject: lib: Refactor ux_locales_get_text API ...................................................................... Patch Set 5: (1 comment) File src/lib/ux_locales.c: https://review.coreboot.org/c/coreboot/+/86283/comment/ecf0a4f3_656e55a9?us… : PS5, Line 28: "Please do not turn off your device." > nit: I don't think there's a point in `#define`ing these up here, rather than just putting them dire […] +1 -- To view, visit https://review.coreboot.org/c/coreboot/+/86283?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: I4952802396265b9ee8d164d6e43a7f2b3599d6c0 Gerrit-Change-Number: 86283 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: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jakub "Kuba" Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Tarun <tstuli(a)gmail.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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub "Kuba" Czapiga <czapiga(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> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Fri, 07 Feb 2025 03:14:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>

1 0

[S] Change in coreboot[main]: ec/starlabs/merlin: Add an option to disabled the lid switch
by Matt DeVillier (Code Review) 06 Feb '25

06 Feb '25

Attention is currently required from: Sean Rhodes. Matt DeVillier has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/86305?usp=email ) Change subject: ec/starlabs/merlin: Add an option to disabled the lid switch ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86305?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: I1dd666a44b332ffbbef4420799eeffd746fd1664 Gerrit-Change-Number: 86305 Gerrit-PatchSet: 1 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Comment-Date: Fri, 07 Feb 2025 02:47:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: ec/starlabs/merlin: Add an option to disable the fan
by Matt DeVillier (Code Review) 06 Feb '25

06 Feb '25

Attention is currently required from: Sean Rhodes. Matt DeVillier has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/86304?usp=email ) Change subject: ec/starlabs/merlin: Add an option to disable the fan ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86304?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: I39f6599056fe0116abbd7e2eb4084f77a7c395d3 Gerrit-Change-Number: 86304 Gerrit-PatchSet: 1 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Comment-Date: Fri, 07 Feb 2025 02:47:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: [This Must Fail]Upgrade GCC to 15-20250202 Snapshot
by Maximilian Brune (Code Review) 06 Feb '25

06 Feb '25

Attention is currently required from: Elyes Haouas, Felix Singer, Julius Werner, Martin L Roth. Maximilian Brune has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/86100?usp=email ) Change subject: [This Must Fail]Upgrade GCC to 15-20250202 Snapshot ...................................................................... Patch Set 5: (1 comment) Patchset: PS4: > I think I am not being clear, maybe my English is making the subject I would like to be the focus, w […] The fact that you think the new toolchain should fail using jenkins means for me that you already tried it locally and it failed (I hope). So that is what I was trying to do. Verifying that it should not compile, by doing the same steps that jenkins would do if it actually would use the new toolchain. You didn't mention before that there is a new warning in GCC15. Regarding my compilation. It may be because of my local libgnat.so version. I don't have a problem with GCC14, but maybe GCC15 depends on a more recent version of libgnat. This is the log when I try to compile GCC15: ``` /usr/bin/ld: ada/raise-gcc.o: in function `__gnat_obtain_caught_object': raise-gcc.c:(.text+0x4a7): undefined reference to `__gnat_convert_caught_object' /usr/bin/ld: ada/raise-gcc.o: in function `__gnat_personality_v0': raise-gcc.c:(.text+0xd9a): undefined reference to `__gnat_convert_caught_object' collect2: error: ld returned 1 exit status make[1]: *** [../../gcc-15-20250202/gcc/ada/gcc-interface/Make-lang.in:808: gnatbind] Error 1 make[1]: *** Waiting for unfinished jobs.... rm gfdl.pod gcc.pod gcov-dump.pod gcov-tool.pod fsf-funding.pod gpl.pod cpp.pod gcov.pod lto-dump.pod make[1]: Leaving directory '/home/max/coreboot/util/crossgcc/build-i386-elf-GCC/gcc' make: *** [Makefile:4672: all-gcc] Error 2 ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/86100?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: I5e6c3745eebeb6ded494c153010283b761498184 Gerrit-Change-Number: 86100 Gerrit-PatchSet: 5 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Fri, 07 Feb 2025 01:30:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[M] Change in coreboot[main]: vc/google/chromeos: Implement platform callback for critical shutdown
by Julius Werner (Code Review) 06 Feb '25

06 Feb '25

Attention is currently required from: Karthik Ramasubramanian, Subrata Banik. Julius Werner has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86228?usp=email ) Change subject: vc/google/chromeos: Implement platform callback for critical shutdown ...................................................................... Patch Set 4: (2 comments) File src/vendorcode/google/chromeos/battery.c: https://review.coreboot.org/c/coreboot/+/86228/comment/1ec200c6_a456aed9?us… : PS4, Line 35: checked = true; No, this needs to be outside the inner `if`. Even if the result was false (= no need to shut down), we want to make sure we cache that result and don't query it again later. https://review.coreboot.org/c/coreboot/+/86228/comment/69aeecd3_67f70251?us… : PS4, Line 50: * 3. If in romstage and Chrome EC is supported, sends a shutdown command to the EC. Why the romstage/ramstage difference? You're saying what the code does but you're not explaining _why_ it does things that way, which is really the most important part. -- To view, visit https://review.coreboot.org/c/coreboot/+/86228?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: I119f80a45c045a6095cae98f179c755a2e948e9c Gerrit-Change-Number: 86228 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 07 Feb 2025 00:48:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2025