Attention is currently required from: Intel coreboot Reviewers, Julius Werner, Karthik Ramasubramanian, Subrata Banik.
Jérémy Compostella has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86336?usp=email )
Change subject: soc/intel/cmn/pmc: Add support for early power off ......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/86336/comment/88873401_4439f76d?usp... : PS3, Line 635: __weak void platform_do_early_poweroff(void) The `pmc.c` file is intended to be a generic module, primarily interacting with PMC-related APIs. Including any calls to Chrome EC-specific functions appears inappropriate within this context. The objective is to assign the early power-off sequence management to the Embedded Controller (EC), and it is suggested that the weak function mechanism be utilized directly for this purpose, rather than introducing an indirect call within `pmclib.c`.
Additionally, scalability should be considered. For instance, if a different vendor desires to implement their own early shutdown process, the current design would necessitate further modifications to the `pmclib.c` file.
Calling pmc_control_poweroff in the early stage doesn't add any value. system might eventually hang in S0 with display being off and system burn all fuels w/o any meaningful purpose.
In my opinion, the goal is twofold:
1. Maintaining the default behavior when no specific early shutdown routine is provided. This approach allows platforms that do not require specialized early shutdown management to operate as intended. 2. Including a disclaimer to indicate that the generic PMC solution may not be effective.