[coreboot-gerrit] [M] Change in coreboot[main]: drivers/intel/fsp2_0: Add early low-battery shutdown during memory init

[Jérémy Compostella (Code Review)]

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?usp... :
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;