[S] Change in coreboot[main]: soc/intel/cmn/pmc: Add support for early power off

[Subrata Banik (Code Review)]

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/ef02298c_90304077?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`.

I'm unable to follow what is *inappropriate* here. the idea of a common code library is to allow inter-IP communication where PMC already communicating with CSE, SPI, GPIO etc. Calling the EC API is not making things very different.

The concern should be if we are implementing an API which is EC specific inside PMClib.c which is not the case. I don't see a point to implement `platform_do_early_poweroff` as override inside EC code which is only meant to implement an EC specific API. If `platform_do_early_poweroff` doesn't fit here to call the EC API "google_chromeec_do_early_poweroff", then probably, we should move this into `reset.c` as it meant to implement reset code.
> 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:
> 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.

Not sure what you meant by intended here ?

while below message gives an impression that device doesn't have right implementation to allow early power-off and halting. The display might be still active and one can sense that the system is at hang stage.
```
    	printk(BIOS_EMERG, "This platform doesn't know how to power off before ramstage,"
    		" hanging!\n");
    	halt();
```

The proposed msg gives an impression that system will use some magic implemention using PMC which can mitigate the first statement "Early power off is not supported on this platform. Proceeding with the standard PMC shutdown procedure."

There is no value to proceed with PMC based shutdown as it will cause the system to stuck into S0 and might give a false impression to the user that system is power-off.

```
__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();
}
```
> Including a disclaimer to indicate that the generic PMC solution may not be effective.