Attention is currently required from: Intel coreboot Reviewers, Julius Werner, Jérémy Compostella, Karthik Ramasubramanian.
Subrata Banik 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/e64b0f72_6b4a4c24?usp... :
PS3, Line 635: __weak void platform_do_early_poweroff(void)
I propose two enhancements as follows:
- The implementation of the subordinate function should be placed within the `src/ec/google/chromeec/ec.c` file, encapsulated by an appropriate Kconfig conditional compilation block.
`ec.c` is a generic code and should only deal with EC related APIs. Doesn't make any sense to me to implement platform specific implementation there and guard with additional Kconfig which might be intel specific.
I don;t see any problem with current schema of things. if you see, please share with me.
- The default function can be defined using the following code snippet:
__weak void platform_do_early_poweroff(void)
{
printk(BIOS_EMERG, "Early power off is not supported on this platform. "
"Proceeding with the standard PMC shutdown procedure.\n");
pmc_control_poweroff();
}
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.
The `platform_do_early_poweroff` function serves as a generic fallback that issues an emergency print statement and invokes the `pmc_control_poweroff` function to initiate a power-off sequence.
--
To view, visit
https://review.coreboot.org/c/coreboot/+/86336?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: I39f516640b3f75ab4c6a09826922289c0533f79b
Gerrit-Change-Number: 86336
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik
subratabanik@google.com
Gerrit-Reviewer: Angel Pons
th3fanbus@gmail.com
Gerrit-Reviewer: Intel coreboot Reviewers
intel_coreboot_reviewers@intel.com
Gerrit-Reviewer: Julius Werner
jwerner@chromium.org
Gerrit-Reviewer: Karthik Ramasubramanian
kramasub@google.com
Gerrit-Reviewer: build bot (Jenkins)
no-reply@coreboot.org
Gerrit-CC: Jérémy Compostella
jeremy.compostella@intel.com
Gerrit-Attention: Intel coreboot Reviewers
intel_coreboot_reviewers@intel.com
Gerrit-Attention: Jérémy Compostella
jeremy.compostella@intel.com
Gerrit-Attention: Julius Werner
jwerner@chromium.org
Gerrit-Attention: Karthik Ramasubramanian
kramasub@google.com
Gerrit-Comment-Date: Mon, 10 Feb 2025 17:39:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jérémy Compostella
jeremy.compostella@intel.com