coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3270 discussions

[S] Change in coreboot[main]: vc/google/chromeos: Add low battery indicator screen
by Jérémy Compostella (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Karthik Ramasubramanian, Subrata Banik. Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86225?usp=email ) Change subject: vc/google/chromeos: Add low battery indicator screen ...................................................................... Patch Set 2: (1 comment) File src/vendorcode/google/chromeos/Makefile.mk: https://review.coreboot.org/c/coreboot/+/86225/comment/… [View More]faf80f63_dd0cd4c9?us… : PS1, Line 43: cbfs-files-$(CONFIG_CHROMEOS_LOW_BATTERY_INDICATOR_SCREEN) += low_battery_logo.bmp > > This is the second time this code is duplicated. I would suggest to introduce a function. […] In the provided code snippet, a repetitive pattern can be observed across three blocks of code, with only minor differences between them. ```makefile cbfs-files-$(CONFIG_CHROMEOS_FW_SPLASH_SCREEN) += cb_logo.bmp cb_logo.bmp-file := $(call strip_quotes,$(CONFIG_CHROMEOS_LOGO_PATH)) cb_logo.bmp-type := raw cb_logo.bmp-compression := $(BMP_LOGO_COMPRESS_FLAG) cbfs-files-$(CONFIG_CHROMEOS_FW_SPLASH_SCREEN) += cb_plus_logo.bmp cb_plus_logo.bmp-file := $(call strip_quotes,$(CONFIG_CHROMEBOOK_PLUS_LOGO_PATH)) cb_plus_logo.bmp-type := raw cb_plus_logo.bmp-compression := $(BMP_LOGO_COMPRESS_FLAG) cbfs-files-$(CONFIG_CHROMEOS_LOW_BATTERY_INDICATOR_SCREEN) += low_battery_logo.bmp low_battery_logo.bmp-file := $(call strip_quotes,$(CONFIG_CHROMEOS_LOW_BATTERY_INDICATOR_LOGO_PATH)) low_battery_logo.bmp-type := raw low_battery_logo.bmp-compression := $(BMP_LOGO_COMPRESS_FLAG) ``` To optimize and simplify the code, I recommend defining a macro function (untested) that encapsulates the common elements of these blocks. This macro can then be used to replace the repetitive code, enhancing maintainability and readability. ```makefile define define_logo_target cbfs-files-$(1) += $(2) $(2)-file := $$(call strip_quotes,$$($(3))) $(2)-type := raw $(2)-compression := $$(BMP_LOGO_COMPRESS_FLAG) endef ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/86225?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: I711c53455639b449fe85903139bbc06cdab08d09 Gerrit-Change-Number: 86225 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: 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: Thu, 30 Jan 2025 17:34:30 +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> [View Less]

1 0

[S] Change in coreboot[main]: soc/intel/adl: Display low battery message on screen
by Subrata Banik (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Dinesh Gehlot, Intel coreboot Reviewers, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Nick Vaccaro. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86224?usp=email ) Change subject: soc/intel/adl: Display low battery message on screen ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/… [View More]

1 0

[S] Change in coreboot[main]: vc/google/chromeos: Add low battery indicator screen
by Subrata Banik (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Jérémy Compostella, Karthik Ramasubramanian. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86225?usp=email ) Change subject: vc/google/chromeos: Add low battery indicator screen ...................................................................... Patch Set 2: (1 comment) File src/vendorcode/google/chromeos/Kconfig: https://review.coreboot.org/c/coreboot/+/86225/comment/… [View More]

1 0

[M] Change in coreboot[main]: tree: Use memcpy_s instead of memcpy
by Elyes Haouas (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Alexander Couzens, Angel Pons, Elyes Haouas, Erik van den Bogaert, Frans Hendriks, Intel coreboot Reviewers, Jeremy Soller, Johnny Lin, Matt DeVillier, Michael Niewöhner, Michał Żygowski, Morgan Jang, Piotr Król, Sean Rhodes, Tim Crawford. Hello Alexander Couzens, Angel Pons, Erik van den Bogaert, Frans Hendriks, Intel coreboot Reviewers, Jeremy Soller, Johnny Lin, Matt DeVillier, Michael Niewöhner, Michał Żygowski, Morgan Jang, Piotr Król, Sean Rhodes, … [View More]Tim Crawford, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86234?usp=email to look at the new patch set (#4). Change subject: tree: Use memcpy_s instead of memcpy ...................................................................... tree: Use memcpy_s instead of memcpy `find . -type f -name "*.c" -print0 | xargs -0 perl -pi -e 's/memcpy$([^,]+), ([^,]+), sizeof\(\2$\);/memcpy_s(\1, sizeof(\1), \2, sizeof(\2));/g'` is used to make this patch. Change-Id: I35e7504e169f93a64579379827cdca5cd8abae47 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> --- M src/drivers/efi/info.c M src/drivers/i2c/at24rf08c/lenovo_serials.c M src/drivers/ipmi/ipmi_kcs_ops.c M src/mainboard/51nb/x210/romstage.c M src/mainboard/clevo/kbl-u/variants/n13xwu/romstage.c M src/mainboard/facebook/monolith/spd/spd_util.c M src/mainboard/getac/p470/acpi_tables.c M src/mainboard/google/eve/romstage.c M src/mainboard/google/fizz/romstage.c M src/mainboard/intel/apollolake_rvp/romstage.c M src/mainboard/intel/kblrvp/spd/spd_util.c M src/mainboard/intel/kunimitsu/spd/spd_util.c M src/mainboard/intel/saddlebrook/spd/spd_util.c M src/mainboard/lenovo/thinkcentre_m710s/romstage.c M src/mainboard/libretrend/lt1000/romstage.c M src/mainboard/ocp/tiogapass/romstage.c M src/mainboard/protectli/vault_kbl/romstage.c M src/mainboard/razer/blade_stealth_kbl/spd/spd_util.c M src/mainboard/starlabs/starbook/variants/kbl/romstage.c M src/mainboard/system76/kbl-u/romstage.c M src/northbridge/intel/x4x/dq_dqs.c 21 files changed, 67 insertions(+), 67 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/86234/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/86234?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: I35e7504e169f93a64579379827cdca5cd8abae47 Gerrit-Change-Number: 86234 Gerrit-PatchSet: 4 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> [View Less]

1 0

[S] Change in coreboot[main]: soc/intel/adl: Handle critical low battery early in romstage
by Subrata Banik (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Dinesh Gehlot, Intel coreboot Reviewers, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Karthik Ramasubramanian, Nick Vaccaro. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86227?usp=email ) Change subject: soc/intel/adl: Handle critical low battery early in romstage ...................................................................... Patch Set 2: (2 comments) File src/soc/intel/… [View More]alderlake/romstage/fsp_params.c: https://review.coreboot.org/c/coreboot/+/86227/comment/b69bd6f2_fa4de1b8?us… : PS1, Line 427: google_chromeec_reboot(EC_REBOOT_COLD_AP_OFF,0); > > `space required after that ',' (ctx:VxV)` […] Acknowledged https://review.coreboot.org/c/coreboot/+/86227/comment/22d53e52_2b3b482a?us… : PS1, Line 446: is_early_critical_shutdown_needed > A function start with `is_` is commonly expected to return a boolean and to not take any action. […] Acknowledged -- To view, visit https://review.coreboot.org/c/coreboot/+/86227?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: Ib4be86ed17818ee05b7bec0337a90f80017183c2 Gerrit-Change-Number: 86227 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: 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: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 30 Jan 2025 17:17:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com> [View Less]

1 0

[S] Change in coreboot[main]: soc/intel/adl: Handle critical low battery early in romstage
by Subrata Banik (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Dinesh Gehlot, Intel coreboot Reviewers, Jayvik Desai, Kapil Porwal, Karthik Ramasubramanian, Nick Vaccaro, Subrata Banik. Hello Dinesh Gehlot, Intel coreboot Reviewers, Jayvik Desai, Kapil Porwal, Karthik Ramasubramanian, Nick Vaccaro, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86227?usp=email to look at the new patch set (#3). Change subject: soc/intel/adl: Handle critical low battery early in … [View More]romstage ...................................................................... soc/intel/adl: Handle critical low battery early in romstage This commit implements early handling of critical low battery conditions in the romstage for Alder Lake platforms. A message is displayed to the user via ux_inform_user_of_poweroff_operation. A short delay is introduced to allow the user to see the message. A low battery event is logged. The system is shut down via the Chrome EC. This early handling prevents the system from proceeding with boot (while performing firmware update) if the battery is critically low and ensures a clean shutdown. This is particularly important for ChromeOS devices. BUG=b:339673254 TEST=Verified low battery boot event logging and controlled shutdown. Change-Id: Ib4be86ed17818ee05b7bec0337a90f80017183c2 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/soc/intel/alderlake/romstage/fsp_params.c 1 file changed, 18 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/86227/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/86227?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: Ib4be86ed17818ee05b7bec0337a90f80017183c2 Gerrit-Change-Number: 86227 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> [View Less]

1 0

[S] Change in coreboot[main]: util/cbfstool/eventlog: Add low battery event type
by Subrata Banik (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Karthik Ramasubramanian. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86223?usp=email ) Change subject: util/cbfstool/eventlog: Add low battery event type ...................................................................... Patch Set 2: (1 comment) File util/cbfstool/eventlog.c: https://review.coreboot.org/c/coreboot/+/86223/comment/638bd375_16cd0280?us… : PS1, Line 696: … [View More]

1 0

[S] Change in coreboot[main]: vc/google/chromeos: Add low battery indicator screen
by Subrata Banik (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Jérémy Compostella, Karthik Ramasubramanian. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86225?usp=email ) Change subject: vc/google/chromeos: Add low battery indicator screen ...................................................................... Patch Set 2: (1 comment) File src/vendorcode/google/chromeos/Makefile.mk: https://review.coreboot.org/c/coreboot/+/86225/comment/… [View More]

1 0

[M] Change in coreboot[main]: include/string.h: Add memcpy_s function
by Elyes Haouas (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Elyes Haouas, Intel coreboot Reviewers, Jeff Daly, Maximilian Brune, Philipp Hug, Vanessa Eusebio, ron minnich. Hello Intel coreboot Reviewers, Jeff Daly, Maximilian Brune, Philipp Hug, Vanessa Eusebio, build bot (Jenkins), ron minnich, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86233?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified-1 by … [View More]

1 0

[S] Change in coreboot[main]: soc/intel/adl: Display low battery message on screen
by Karthik Ramasubramanian (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Dinesh Gehlot, Intel coreboot Reviewers, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Subrata Banik. Karthik Ramasubramanian has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86224?usp=email ) Change subject: soc/intel/adl: Display low battery message on screen ...................................................................... Patch Set 1: (5 comments) Commit Message: https://review.coreboot.org/c/… [View More]coreboot/+/86224/comment/d496525b_3ba63099?us… : PS1, Line 11: Nit: Seems there is an extra space. https://review.coreboot.org/c/coreboot/+/86224/comment/367fe8a5_c182e6af?us… : PS1, Line 16: rely Nit: relies https://review.coreboot.org/c/coreboot/+/86224/comment/75ce56a8_1c7994f8?us… : PS1, Line 16: use Nit: used File src/soc/intel/alderlake/romstage/ux.c: https://review.coreboot.org/c/coreboot/+/86224/comment/cef9979a_78c4af52?us… : PS1, Line 50: UX_MEMORY_TRAINING_DESC Should it be a different locale name here instead of relying on memory training? https://review.coreboot.org/c/coreboot/+/86224/comment/1615f3f8_64145ede?us… : PS1, Line 59: } Looks like both `ux_inform_user_of_update_operation` and `user_inform_user_of_poweroff_operation` have identical implementation for the most part and a common helper function with the below signature can be defined. ``` static bool ux_inform_user_of_operation(const char *name, const char *ux_locale_desc, const char *default_text) { ... } ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/86224?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: I3b24d2c89ade8cc62b7e47c487d52d47b7f3376d Gerrit-Change-Number: 86224 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Thu, 30 Jan 2025 17:11:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No [View Less]

1 0

← Newer
1
...
5
6
7
8
9
10
11
...
327
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025