coreboot-gerrit February 2025

coreboot-gerrit@coreboot.org

1 participants
2886 discussions

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add early low-battery shutdown during memory init
by Jérémy Compostella (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Karthik Ramasubramanian, Ronak Kanabar, Subrata Banik. Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86452?usp=email ) Change subject: drivers/intel/fsp2_0: Add early low-battery shutdown during memory init ...................................................................... Patch Set 6: (1 comment) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/86452/comment/33721a15_e77fe7a6?us… : PS6, Line 352: do_low_battery_poweroff > > While reviewing this CL, I realized something odd. […] No, I am suggesting setting the `__noreturn` attribute on the function that should not return. modified src/include/halt.h @@ -12,6 +12,6 @@ static inline __noreturn void halt(void) } /* Power off the system. */ -void poweroff(void); +void __noreturn poweroff(void); #endif /* __HALT_H__ */ modified src/soc/intel/common/reset.h @@ -30,6 +30,6 @@ efi_return_status_t fsp_get_pch_reset_status(void); * * Call this function to power off the platform if the battery level is critically low. */ -void do_low_battery_poweroff(void); +void __noreturn do_low_battery_poweroff(void); #endif /* _INTEL_COMMON_RESET_H_ */ Your code could look like this: /* User has been notified of low battery; safe to power off. */ if (CONFIG(MAINBOARD_HAS_EARLY_LIBGFXINIT)) { platform_display_early_shutdown_notification(NULL); do_low_battery_poweroff(); /* Do not return */ } /* Defer shutdown until FSP-M (uGOP) display text message for user notification */ platform_display_early_shutdown_notification(fspm_upd); *defer_shutdown = true; -- To view, visit https://review.coreboot.org/c/coreboot/+/86452?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: Ia135b238d1e16722c2ca8d3b461e83b4ce513adf Gerrit-Change-Number: 86452 Gerrit-PatchSet: 6 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> 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: Wed, 19 Feb 2025 18:41:18 +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

[XS] Change in coreboot[main]: soc/intel/common/gpio: Add macro for interrupt GPI with driver mode
by Jérémy Compostella (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Cliff Huang, Intel coreboot Reviewers. Jérémy Compostella has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/86414?usp=email ) Change subject: soc/intel/common/gpio: Add macro for interrupt GPI with driver mode ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/common/block/include/intelblocks/gpio_defs.h: https://review.coreboot.org/c/coreboot/+/86414/comment/86b72cca_e34dbdfe?us… : PS3, Line 440: #define PAD_CFG_GPI_APIC_DRIVER(pad, pull, rst, trig, inv) \ Could you leverage PAD_CFG_GPI_APIC instead of copy pasting the content ? ``` #define PAD_CFG_GPI_APIC_DRIVER(pad, pull, rst, trig, inv) \ PAD_CFG_GPI_APIC(pad, pull, rest, trig, inv) | PAD_CFG_OWN_GPIO(DRIVER) ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/86414?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: Ibc1ed3089c24302bc7eb02318714b8ec464fad01 Gerrit-Change-Number: 86414 Gerrit-PatchSet: 3 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Comment-Date: Wed, 19 Feb 2025 18:33:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: soc/intel/pantherlake: Bind SoC config VR settings to respective UPD
by Subrata Banik (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Intel coreboot Reviewers, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Paul Menzel, Pranava Y N. Subrata Banik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/85131?usp=email ) Change subject: soc/intel/pantherlake: Bind SoC config VR settings to respective UPD ...................................................................... Patch Set 13: (1 comment) Patchset: PS6: > No problem, I will let you handle the process, allowing you to take it at your own pace and ensure that nothing breaks on your side. Please close the comment and submit the CL once you're ready. sure, i will get back by next week, i'm focusing to close eSOL this week. -- To view, visit https://review.coreboot.org/c/coreboot/+/85131?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: Ie72e4725cb97b4af7843a43eeaedd687d28b6752 Gerrit-Change-Number: 85131 Gerrit-PatchSet: 13 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.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: Paul Menzel <paulepanter(a)mailbox.org> 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> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Wed, 19 Feb 2025 18:09:32 +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

[L] Change in coreboot[main]: drivers/intel/touch: Add Intel Touch Controller driver
by Cliff Huang (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Bora Guvendik, Intel coreboot Reviewers, Jérémy Compostella, Kyoung Il Kim, Paul Menzel, Subrata Banik. Hello Bora Guvendik, Intel coreboot Reviewers, Jérémy Compostella, Kyoung Il Kim, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85198?usp=email to look at the new patch set (#16). Change subject: drivers/intel/touch: Add Intel Touch Controller driver ...................................................................... drivers/intel/touch: Add Intel Touch Controller driver THC is a hardware component that interfaces between a touch sensor and the system's SPI or I2C bus. This driver publishes data into the Secondary System Descriptor Table (SSDT). This driver generates the following ACPI objects: - Device Specific Method (_DSM) - Current Resource Settings (_CRS) - Power resource with Status (_STA), _ON, and _OFF methods - Device Specific Data (_DSD) for THC-I2C - Device Reset (_RST) for THC-SPI Template device configuration for the following supported devices: - Wacom: THC-SPI touchscreen only - Elan: both THC-SPI and THC-I2C touchscreen - Hynitron: THC-I2C touchpad only The configuration is spitted into device, SoC, and MB specific. - SoC-specific: Implement soc_get_thc_hidi2c_info and soc_get_thc_hidspi_info functions for SoC-specific configurations. Theses can be placed in the SoC's chip.c. - device-specific: This driver provides the device-specific configuration for the supported devices; otherwise, require information via the device tree for unsupported/generic devices. - MB-specific: The MB-specific, such as LTR value, needs to be provided in the device tree. BUG=none TEST=Select the DRIVERS_INTEL_TOUCH option on a motherboard with the necessary configurations, and verify that the THC ACPI tables are generated in the SSDT. Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> Change-Id: Ibcd2a75a41460dee67aebdc61ee9e85fa98b71bf --- A src/drivers/intel/touch/Kconfig A src/drivers/intel/touch/Makefile.mk A src/drivers/intel/touch/chip.h A src/drivers/intel/touch/elan.h A src/drivers/intel/touch/hynitron.h A src/drivers/intel/touch/touch.c A src/drivers/intel/touch/wacom.h 7 files changed, 930 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/85198/16 -- To view, visit https://review.coreboot.org/c/coreboot/+/85198?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: Ibcd2a75a41460dee67aebdc61ee9e85fa98b71bf Gerrit-Change-Number: 85198 Gerrit-PatchSet: 16 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kyoung Il Kim <kyoung.il.kim(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kyoung Il Kim <kyoung.il.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[S] Change in coreboot[main]: soc/intel/pantherlake: Bind SoC config VR settings to respective UPD
by Jérémy Compostella (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Intel coreboot Reviewers, Jayvik Desai, Kapil Porwal, Paul Menzel, Pranava Y N, Subrata Banik. Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/85131?usp=email ) Change subject: soc/intel/pantherlake: Bind SoC config VR settings to respective UPD ...................................................................... Patch Set 13: (1 comment) Patchset: PS6: > > > It is already in 8027111 which to my understanding has been merged. […] No problem, I will let you handle the process, allowing you to take it at your own pace and ensure that nothing breaks on your side. Please close the comment and submit the CL once you're ready. -- To view, visit https://review.coreboot.org/c/coreboot/+/85131?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: Ie72e4725cb97b4af7843a43eeaedd687d28b6752 Gerrit-Change-Number: 85131 Gerrit-PatchSet: 13 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.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: Paul Menzel <paulepanter(a)mailbox.org> 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> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Wed, 19 Feb 2025 18:07:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[L] Change in coreboot[main]: drivers/intel/touch: Add Intel Touch Controller driver
by Cliff Huang (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Bora Guvendik, Intel coreboot Reviewers, Jérémy Compostella, Kyoung Il Kim, Paul Menzel, Subrata Banik. Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/85198?usp=email ) Change subject: drivers/intel/touch: Add Intel Touch Controller driver ...................................................................... Patch Set 15: (3 comments) File src/drivers/intel/touch/chip.h: https://review.coreboot.org/c/coreboot/+/85198/comment/c7f9f873_b886bc0f?us… : PS12, Line 18: Period > line 55 explains the unit. Since there are several Period related parameter below. […] Acknowledged File src/drivers/intel/touch/elan.h: https://review.coreboot.org/c/coreboot/+/85198/comment/bebad387_1f88ce97?us… : PS14, Line 33: .sensor_dev_name = ELAN_DEV_NAME, > Could you remove the defines and move the values directly in the static constant ? I don't see any v […] Acknowledged File src/drivers/intel/touch/elan.h: https://review.coreboot.org/c/coreboot/+/85198/comment/be050b82_618580c4?us… : PS12, Line 17: #define ELAN_RST_SEQ_DLY 300 > unit commented in line 14 should be sufficient. This has been moved to SoC. -- To view, visit https://review.coreboot.org/c/coreboot/+/85198?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: Ibcd2a75a41460dee67aebdc61ee9e85fa98b71bf Gerrit-Change-Number: 85198 Gerrit-PatchSet: 15 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kyoung Il Kim <kyoung.il.kim(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kyoung Il Kim <kyoung.il.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 19 Feb 2025 18:07:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com> Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[L] Change in coreboot[main]: drivers/intel/touch: Add Intel Touch Controller driver
by Cliff Huang (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Bora Guvendik, Cliff Huang, Intel coreboot Reviewers, Kyoung Il Kim, Paul Menzel, Subrata Banik. Hello Bora Guvendik, Intel coreboot Reviewers, Jérémy Compostella, Kyoung Il Kim, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85198?usp=email to look at the new patch set (#15). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: drivers/intel/touch: Add Intel Touch Controller driver ...................................................................... drivers/intel/touch: Add Intel Touch Controller driver THC is a hardware component that interfaces between a touch sensor and the system's SPI or I2C bus. This driver publishes data into the Secondary System Descriptor Table (SSDT). This driver generates the following ACPI objects: - Device Specific Method (_DSM) - Current Resource Settings (_CRS) - Power resource with Status (_STA), _ON, and _OFF methods - Device Specific Data (_DSD) for THC-I2C - Device Reset (_RST) for THC-SPI Template device configuration for the following supported devices: - Wacom: THC-SPI touchscreen only - Elan: both THC-SPI and THC-I2C touchscreen - Hynitron: THC-I2C touchpad only The configuration is spitted into device, SoC, and MB specific. - SoC-specific: Implement soc_get_thc_hidi2c_info and soc_get_thc_hidspi_info functions for SoC-specific configurations. Theses can be placed in the SoC's chip.c. - device-specific: This driver provides the device-specific configuration for the supported devices; otherwise, require information via the device tree for unsupported/generic devices. - MB-specific: The MB-specific, such as LTR value, needs to be provided in the device tree. BUG=none TEST=Select the DRIVERS_INTEL_TOUCH option on a motherboard with the necessary configurations, and verify that the THC ACPI tables are generated in the SSDT. Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> Change-Id: Ibcd2a75a41460dee67aebdc61ee9e85fa98b71bf --- A src/drivers/intel/touch/Kconfig A src/drivers/intel/touch/Makefile.mk A src/drivers/intel/touch/chip.h A src/drivers/intel/touch/elan.h A src/drivers/intel/touch/hynitron.h A src/drivers/intel/touch/touch.c A src/drivers/intel/touch/wacom.h 7 files changed, 930 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/85198/15 -- To view, visit https://review.coreboot.org/c/coreboot/+/85198?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: Ibcd2a75a41460dee67aebdc61ee9e85fa98b71bf Gerrit-Change-Number: 85198 Gerrit-PatchSet: 15 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kyoung Il Kim <kyoung.il.kim(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kyoung Il Kim <kyoung.il.kim(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[M] Change in coreboot[main]: drivers/usb/acpi: Account for GPIO polarity
by Jérémy Compostella (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Sean Rhodes. Jérémy Compostella has posted comments on this change by Sean Rhodes. ( https://review.coreboot.org/c/coreboot/+/86400?usp=email ) Change subject: drivers/usb/acpi: Account for GPIO polarity ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86400?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: Ib481d49d536b702fef149af882209501c61de6da Gerrit-Change-Number: 86400 Gerrit-PatchSet: 12 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> 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: Wed, 19 Feb 2025 17:46:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add early low-battery shutdown during memory init
by Subrata Banik (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Jérémy Compostella, Karthik Ramasubramanian, Ronak Kanabar. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86452?usp=email ) Change subject: drivers/intel/fsp2_0: Add early low-battery shutdown during memory init ...................................................................... Patch Set 6: (1 comment) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/86452/comment/53f3aee7_23a3c416?us… : PS6, Line 352: do_low_battery_poweroff > While reviewing this CL, I realized something odd. Shouldn't power_off, halt and do_low_battery_poweroff been tagged as noreturn function ? If so the else keyword could be removed here. are you suggesting something like ``` if (CONFIG(MAINBOARD_HAS_EARLY_LIBGFXINIT)) { platform_display_early_shutdown_notification(NULL); do_low_battery_poweroff(); /* Don't return back */ halt(); } if (CONFIG(FSP_UGOP_EARLY_SIGN_OF_LIFE)) { /* Defer shutdown until FSP-M (uGOP) display text message for user notification */ platform_display_early_shutdown_notification(fspm_upd); *defer_shutdown = true; /* Don't return back */ halt(); } ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/86452?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: Ia135b238d1e16722c2ca8d3b461e83b4ce513adf Gerrit-Change-Number: 86452 Gerrit-PatchSet: 6 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.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: Julius Werner <jwerner(a)chromium.org> 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: Wed, 19 Feb 2025 17:37:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add early low-battery shutdown during memory init
by Jérémy Compostella (Code Review) 19 Feb '25

19 Feb '25

Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Karthik Ramasubramanian, Ronak Kanabar, Subrata Banik. Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86452?usp=email ) Change subject: drivers/intel/fsp2_0: Add early low-battery shutdown during memory init ...................................................................... Patch Set 6: (3 comments) Patchset: PS6: Just a few notes. File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/86452/comment/55c2f3cb_e79d09ec?us… : PS6, Line 352: do_low_battery_poweroff While reviewing this CL, I realized something odd. Shouldn't power_off, halt and do_low_battery_poweroff been tagged as noreturn function ? If so the else keyword could be removed here. https://review.coreboot.org/c/coreboot/+/86452/comment/484f12f3_0a28a206?us… : PS6, Line 367: defer_shutdown_in_low_battery_boot what about `poweroff_after_fsp_execution` ? -- To view, visit https://review.coreboot.org/c/coreboot/+/86452?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: Ia135b238d1e16722c2ca8d3b461e83b4ce513adf Gerrit-Change-Number: 86452 Gerrit-PatchSet: 6 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> 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: Wed, 19 Feb 2025 17:31:54 +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