Attention is currently required from: Karthik Ramasubramanian, Subrata Banik.
Julius Werner has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86228?usp=email )
Change subject: vc/google/chromeos: Implement platform callback for critical shutdown ......................................................................
Patch Set 7:
(1 comment)
File src/vendorcode/google/chromeos/battery.c:
https://review.coreboot.org/c/coreboot/+/86228/comment/d9e36e45_05dc603b?usp... : PS7, Line 59: google_chromeec_reboot(EC_REBOOT_COLD_AP_OFF, 0);
I'm still not really clear why this needs to be an EC command rather than a normal `poweroff()`? I […]
Well then, like I said, I think the better solution would be to make sure the `poweroff()` function implemented by the platform does the right thing, rather than forcing platform-independent code to deal with this quirk.
For example, you could add something like this to the top of the `poweroff()` implementation in `intel/common/block/pmc/pmclib.c`: ``` if (ENV_ROMSTAGE_OR_BEFORE) { platform_do_intel_early_poweroff(); } else { pmc_enable_... if (!ENV_SMM) halt(); } ``` and then elsewhere in that file ``` __weak void platform_do_intel_early_poweroff(void) { printk(BIOS_EMERG, "This platform doesn't know how to power off before ramstage, hanging!\n"); halt(); } ``` and then somewhere in `vendorcode/google/chromeos` you can override that function (in a file that gets compiled for Intel platforms when CONFIG_EC_GOOGLE_CHROMEEC is set) ``` void platform_do_intel_early_poweroff(void) { google_chromeec_reboot(EC_REBOOT_COLD_AP_OFF, 0); halt(); } ``` And then other parts of coreboot can just call `poweroff()` whenever they need to power off the system without having to know about these platform-specific quirks.