coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3270 discussions

[S] Change in coreboot[main]: vc/google/chromeos: Refactor Makefile to use a macro for CBFS logo
by Jérémy Compostella (Code Review) 30 Jan '25

30 Jan '25

Jérémy Compostella has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86237?usp=email ) Change subject: vc/google/chromeos: Refactor Makefile to use a macro for CBFS logo ...................................................................... vc/google/chromeos: Refactor Makefile to use a macro for CBFS logo This commit introduces a new macro, cbfs_add_bmp_file, to the ChromeOS vendor code Makefile. The macro simplifies the process of adding BMP files to the CBFS (Coreboot Filesystem) by encapsulating the repetitive tasks of specifying file attributes such as file path, type, and compression flag. TEST:Both 'cb_logo.bmp' and 'cb_plus_logo.bmp' files are included with the same properties, within the coreboot firmware image. Change-Id: I827451da79931c09768965c3ad071ecdd918d367 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/vendorcode/google/chromeos/Makefile.mk 1 file changed, 10 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/86237/1 diff --git a/src/vendorcode/google/chromeos/Makefile.mk b/src/vendorcode/google/chromeos/Makefile.mk index 44d4d2b..10b1f5b 100644 --- a/src/vendorcode/google/chromeos/Makefile.mk +++ b/src/vendorcode/google/chromeos/Makefile.mk @@ -30,12 +30,14 @@ BMP_LOGO_COMPRESS_FLAG := LZ4 endif -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) +define cbfs_add_bmp_file +cbfs-files-$$($(1)) += $(2) +$(2)-file := $$(call strip_quotes,$$($(3))) +$(2)-type := raw +$(2)-compression := $$(BMP_LOGO_COMPRESS_FLAG) +endef -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) +$(eval $(call cbfs_add_bmp_file,CONFIG_CHROMEOS_FW_SPLASH_SCREEN, \ + cb_logo.bmp,CONFIG_CHROMEOS_LOGO_PATH)) +$(eval $(call cbfs_add_bmp_file,CONFIG_CHROMEOS_FW_SPLASH_SCREEN, \ + cb_plus_logo.bmp,CONFIG_CHROMEBOOK_PLUS_LOGO_PATH)) -- To view, visit https://review.coreboot.org/c/coreboot/+/86237?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I827451da79931c09768965c3ad071ecdd918d367 Gerrit-Change-Number: 86237 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[S] Change in coreboot[main]: soc/intel/adl: Handle critical low battery early in romstage
by Karthik Ramasubramanian (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. Karthik Ramasubramanian 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 3: (1 comment) File src/soc/intel/alderlake/romstage/fsp_params.c: https://review.coreboot.org/c/coreboot/+/86227/comment/ba9fe665_411e380d?us… : PS3, Line 421: if (CONFIG(EC_GOOGLE_CHROMEEC) && google_chromeec_is_below_critical_threshold()) { I would recommend to decouple ChromeEC and SoC code as much as possible. Probably introduce a weak platform_early_shutdown callback and then override it in vendorcode/google/chromeos? -- 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: 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-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: Subrata Banik <subratabanik(a)google.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-Comment-Date: Thu, 30 Jan 2025 18:21:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add platform callback for critical shutdown
by Subrata Banik (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Karthik Ramasubramanian, Ronak Kanabar. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86226?usp=email ) Change subject: drivers/intel/fsp2_0: Add platform callback for critical shutdown ...................................................................... Patch Set 2: (1 comment) File src/drivers/intel/fsp2_0/silicon_init.c: https://review.coreboot.org/c/coreboot/+/86226/comment/26b1f7ef_0d314624?us… : PS2, Line 262: if (CONFIG(CHROMEOS_LOW_BATTERY_INDICATOR_SCREEN)) > Is this required especially with the weak override? we can implement a static inline as well to avoid weak function. -- To view, visit https://review.coreboot.org/c/coreboot/+/86226?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: I2d6677d70dea3d24f5a19d70608fd21229a271a0 Gerrit-Change-Number: 86226 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 30 Jan 2025 18:15:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>

1 0

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

30 Jan '25

Attention is currently required from: Alexander Couzens, 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. Angel Pons has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/86234?usp=email ) Change subject: tree: Use memcpy_s instead of memcpy ...................................................................... Patch Set 4: Code-Review-1 (52 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/86234/comment/2aa79ca6_6a74a6d4?us… : PS4, Line 7: tree: Use memcpy_s instead of memcpy Why? `memcpy_s` is not inherently more secure. Most of the changes here would actually break stuff, btw. File src/drivers/i2c/at24rf08c/lenovo_serials.c: https://review.coreboot.org/c/coreboot/+/86234/comment/57107c79_91599cde?us… : PS4, Line 49: sizeof(result) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/cfaac87b_eb23c585?us… : PS4, Line 64: sizeof(result) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/8cb3bfbc_267f16d4?us… : PS4, Line 169: sizeof(result) Instead of that, you could `_Static_Assert(sizeof(result) >= sizeof(ERROR_STRING))` File src/drivers/ipmi/ipmi_kcs_ops.c: https://review.coreboot.org/c/coreboot/+/86234/comment/96496ac0_1b03790a?us… : PS4, Line 137: memcpy_s(&man_id, sizeof(&man_id), rsp.manufacturer_id, sizeof(rsp.manufacturer_id)); : : memcpy_s(&prod_id, sizeof(&prod_id), rsp.product_id, sizeof(rsp.product_id)); I have no idea why these use memcpy. Is `rsp` packed, so the fields are misaligned? File src/mainboard/51nb/x210/romstage.c: https://review.coreboot.org/c/coreboot/+/86234/comment/ba05bcdf_1b3a4e8b?us… : PS4, Line 12: sizeof(rcomp_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/29fd7df7_81d7ffaa?us… : PS4, Line 19: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/mainboard/clevo/kbl-u/variants/n13xwu/romstage.c: https://review.coreboot.org/c/coreboot/+/86234/comment/39b2cc25_308d96d5?us… : PS4, Line 11: sizeof(rcomp_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/0029eb8a_56425af0?us… : PS4, Line 17: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/mainboard/facebook/monolith/spd/spd_util.c: https://review.coreboot.org/c/coreboot/+/86234/comment/c2c4cb03_be0af6f5?us… : PS4, Line 16: sizeof(dq_map_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/3386fb6e_60cb7485?us… : PS4, Line 25: sizeof(dqs_map_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/044881b1_a8781088?us… : PS4, Line 41: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/mainboard/getac/p470/acpi_tables.c: https://review.coreboot.org/c/coreboot/+/86234/comment/1c2de7df_bfc2b75a?us… : PS4, Line 48: sizeof(ecdt->ec_id) `ec_id` is a zero-length array, `sizeof()` will not return a meaningful value. Note that the string size is already accounted for in the code. File src/mainboard/google/eve/romstage.c: https://review.coreboot.org/c/coreboot/+/86234/comment/4782e007_5d12e02c?us… : PS4, Line 32: memcpy_s(&mem_cfg->DqByteMapCh0, sizeof(&mem_cfg->DqByteMapCh0), dq_map[0], sizeof(dq_map[0])); : memcpy_s(&mem_cfg->DqByteMapCh1, sizeof(&mem_cfg->DqByteMapCh1), dq_map[1], sizeof(dq_map[1])); : memcpy_s(&mem_cfg->DqsMapCpu2DramCh0, sizeof(&mem_cfg->DqsMapCpu2DramCh0), dqs_map[0], sizeof(dqs_map[0])); : memcpy_s(&mem_cfg->DqsMapCpu2DramCh1, sizeof(&mem_cfg->DqsMapCpu2DramCh1), dqs_map[1], sizeof(dqs_map[1])); : memcpy_s(&mem_cfg->RcompResistor, sizeof(&mem_cfg->RcompResistor), rcomp_resistor, sizeof(rcomp_resistor)); : memcpy_s(&mem_cfg->RcompTarget, sizeof(&mem_cfg->RcompTarget), rcomp_target, sizeof(rcomp_target)); It would be better to adapt the API so that boards don't have to `memcpy` these arrays. Then one can just have one copy of a `_Static_Assert()` in SoC code in case FSP UPDs change (which is unlikely). Moreover, Rcomp settings aren't always mainboard-specific. SoC code could provide sane defaults, and only override the settings from mainboards that need special values. File src/mainboard/intel/kblrvp/spd/spd_util.c: https://review.coreboot.org/c/coreboot/+/86234/comment/99cccd59_72d769dd?us… : PS4, Line 17: sizeof(dq_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/ceba72b4_e5763295?us… : PS4, Line 18: sizeof(dq_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/4a5956ad_57301d99?us… : PS4, Line 27: sizeof(dqs_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/ac857987_66056ba1?us… : PS4, Line 28: sizeof(dqs_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/5ebc287b_961f0155?us… : PS4, Line 44: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/mainboard/intel/kunimitsu/spd/spd_util.c: https://review.coreboot.org/c/coreboot/+/86234/comment/da02e2b1_a71f8332?us… : PS4, Line 20: sizeof(dq_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/fee9bc13_be4a19ff?us… : PS4, Line 21: sizeof(dq_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/13223e1f_d895ec7e?us… : PS4, Line 30: sizeof(dqs_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/28ec8740_2c594500?us… : PS4, Line 31: sizeof(dqs_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/f311bdf5_a4e52cab?us… : PS4, Line 59: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/mainboard/intel/saddlebrook/spd/spd_util.c: https://review.coreboot.org/c/coreboot/+/86234/comment/47b856c2_4ce807ea?us… : PS4, Line 15: sizeof(dq_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/a0fa9e73_87069a97?us… : PS4, Line 16: sizeof(dq_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/bd9f62c1_809041e3?us… : PS4, Line 25: sizeof(dqs_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/cd0b86cd_733b5faf?us… : PS4, Line 26: sizeof(dqs_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/e597791e_c915fc38?us… : PS4, Line 33: sizeof(rcomp_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/c1b16584_c742cfe3?us… : PS4, Line 40: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/mainboard/lenovo/thinkcentre_m710s/romstage.c: https://review.coreboot.org/c/coreboot/+/86234/comment/83c7ab5a_83f04ce9?us… : PS4, Line 31: assert(sizeof(mem_cfg->RcompResistor) == sizeof(rcomp_resistors)); There's this assertion already. https://review.coreboot.org/c/coreboot/+/86234/comment/917e05db_054c16d5?us… : PS4, Line 36: assert(sizeof(mem_cfg->RcompTarget) == sizeof(rcomp_targets)); There's this assertion already. File src/mainboard/libretrend/lt1000/romstage.c: https://review.coreboot.org/c/coreboot/+/86234/comment/2aa9ed2a_0b486d5e?us… : PS4, Line 14: sizeof(dq_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/6d0ec0d6_006cebd5?us… : PS4, Line 15: sizeof(dq_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/cff16a02_a200c7a4?us… : PS4, Line 23: sizeof(dqs_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/72f3afdf_f91fbdd3?us… : PS4, Line 24: sizeof(dqs_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/2c886e70_9fa5f5a0?us… : PS4, Line 30: sizeof(rcomp_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/18d405ba_c398b876?us… : PS4, Line 36: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/mainboard/ocp/tiogapass/romstage.c: https://review.coreboot.org/c/coreboot/+/86234/comment/14f236bd_5170094e?us… : PS4, Line 39: sizeof(iio_table_buf) Look at how `tp_iio_bifur_table` is defined. It is guaranteed to be of the same size, so no need for a runtime check. File src/mainboard/protectli/vault_kbl/romstage.c: https://review.coreboot.org/c/coreboot/+/86234/comment/403f54d0_82e3b01a?us… : PS4, Line 15: sizeof(dq_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/a71ea974_23269087?us… : PS4, Line 16: sizeof(dq_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/75d774db_933f9241?us… : PS4, Line 24: sizeof(dqs_map_ch0) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/dd6b1d42_7d6ccf57?us… : PS4, Line 25: sizeof(dqs_map_ch1) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/c633caea_3e74b1ce?us… : PS4, Line 31: sizeof(rcomp_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/2a3c9ed5_982c5404?us… : PS4, Line 37: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/mainboard/razer/blade_stealth_kbl/spd/spd_util.c: https://review.coreboot.org/c/coreboot/+/86234/comment/c23e8e0b_8e2d41e7?us… : PS4, Line 14: sizeof(dq_map_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/64aaf902_14426b95?us… : PS4, Line 21: sizeof(dqs_map_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/f09cbf65_ebd428b3?us… : PS4, Line 28: sizeof(rcomp_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/75446cf4_8534349d?us… : PS4, Line 36: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/mainboard/system76/kbl-u/romstage.c: https://review.coreboot.org/c/coreboot/+/86234/comment/9dee6c81_c5679ac3?us… : PS4, Line 10: sizeof(rcomp_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. https://review.coreboot.org/c/coreboot/+/86234/comment/9206bef9_a54d7574?us… : PS4, Line 16: sizeof(rcomp_strength_ptr) `sizeof()` returns pointer size, which is not destination size. This cannot work. File src/northbridge/intel/x4x/dq_dqs.c: https://review.coreboot.org/c/coreboot/+/86234/comment/09403b7c_e797e1c7?us… : PS4, Line 280: sizeof(dq_upper) Both arrays are of the same size. -- 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: comment 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: 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> Gerrit-Comment-Date: Thu, 30 Jan 2025 18:14:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add platform callback for critical shutdown
by Karthik Ramasubramanian (Code Review) 30 Jan '25

30 Jan '25

Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Ronak Kanabar, Subrata Banik. Karthik Ramasubramanian has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86226?usp=email ) Change subject: drivers/intel/fsp2_0: Add platform callback for critical shutdown ...................................................................... Patch Set 2: (1 comment) File src/drivers/intel/fsp2_0/silicon_init.c: https://review.coreboot.org/c/coreboot/+/86226/comment/b552eb0b_364c155e?us… : PS2, Line 262: if (CONFIG(CHROMEOS_LOW_BATTERY_INDICATOR_SCREEN)) Is this required especially with the weak override? -- To view, visit https://review.coreboot.org/c/coreboot/+/86226?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: I2d6677d70dea3d24f5a19d70608fd21229a271a0 Gerrit-Change-Number: 86226 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Thu, 30 Jan 2025 18:07:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

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/+/86224/comment/8d7e0cc2_3a13103a?us… : PS1, Line 9: This commit adds a function ux_inform_user_of_poweroff_operation > This is 75. Cf. https://doc.coreboot.org/contributing/gerrit_guidelines.html#commit-message… thanks, will reflow the commit msg -- 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: 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-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: 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-Comment-Date: Thu, 30 Jan 2025 18:05:53 +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>

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/173f2859_279b82a9?us… : PS1, Line 43: cbfs-files-$(CONFIG_CHROMEOS_LOW_BATTERY_INDICATOR_SCREEN) += low_battery_logo.bmp > Please refer here for more details: https://www.gnu.org/software/make/manual/html_node/Eval-Function.html @jeremy.compostella@intel.com: as this is ur idea, will you be able to push the required changes and then I rebase my CL on top of yours ? -- 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: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 30 Jan 2025 17:46:25 +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: Karthik Ramasubramanian <kramasub(a)google.com>

1 0

[S] Change in coreboot[main]: soc/intel/adl: Display low battery message on screen
by Jérémy Compostella (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. Jérémy Compostella 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/+/86224/comment/42720797_c24a869c?us… : PS1, Line 9: This commit adds a function ux_inform_user_of_poweroff_operation > > Is this is setup for a paragraph filling of 75 characters ? […] This is 75. Cf. https://doc.coreboot.org/contributing/gerrit_guidelines.html#commit-message… -- 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: 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-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: 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:44:14 +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>

1 0

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

30 Jan '25

Attention is currently required from: Subrata Banik. Karthik Ramasubramanian 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: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/86225/comment/8c92dc03_db19e7f3?us… : PS2, Line 11: Nit: Looks like a double space. Same for the next line too. https://review.coreboot.org/c/coreboot/+/86225/comment/33b00759_04f5cbe5?us… : PS2, Line 23: TEST=Able to capture the eventlog for low battery boot event. Can you also please mention that you saw the low battery logo on the screen? File src/vendorcode/google/chromeos/Kconfig: https://review.coreboot.org/c/coreboot/+/86225/comment/ad2c9564_f69d8d92?us… : PS2, Line 121: depends on CHROMEOS_LOW_BATTERY_INDICATOR_SCREEN depends on CHROMEOS_ENABLE_ESOL? File src/vendorcode/google/chromeos/Makefile.mk: https://review.coreboot.org/c/coreboot/+/86225/comment/06d6bf4c_affab8c9?us… : PS1, Line 43: cbfs-files-$(CONFIG_CHROMEOS_LOW_BATTERY_INDICATOR_SCREEN) += low_battery_logo.bmp > In the provided code snippet, a repetitive pattern can be observed across three blocks of code, with […] Please refer here for more details: https://www.gnu.org/software/make/manual/html_node/Eval-Function.html -- 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-Comment-Date: Thu, 30 Jan 2025 17:37:28 +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>

1 0

[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/Kconfig: https://review.coreboot.org/c/coreboot/+/86225/comment/dec51425_a175b28f?us… : PS1, Line 128: Don't select if not sure. > > Is this a useful statement ? […] Then maybe something like "Enable only if the platform support it" would be more helpful. Should we introduce a `HAVE_` flag to capture this dependency formally? -- 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:37:07 +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>

1 0

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

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025