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:
(2 comments)
File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/86452/comment/7cf31de7_7450ae5f?usp... : PS6, Line 352: do_low_battery_poweroff
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;
I have accommodated your feedback in CB:86578 and don't feel that we need to modify poweroff w/ `__noreturn` because adding halt() inside do_low_battery_poweroff API is enough to ensure that return type is non-return (to avoid compilation error).
``` static inline __noreturn void halt(void) { abort(); } ```
https://review.coreboot.org/c/coreboot/+/86452/comment/ef1c731b_1a47dcf5?usp... : PS6, Line 367: defer_shutdown_in_low_battery_boot
what about `poweroff_after_fsp_execution` ?
Acknowledged