[M] Change in coreboot[main]: [RFC] soc/intel/fsp_reset: Drop global-reset sanity check

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

Attention is currently required from: Andrey Petrov, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Nico Huber, Pranava Y N, Ronak Kanabar, Sean Rhodes, Subrata Banik, Tarun, Werner Zeh.

Jérémy Compostella has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/84558?usp=email )

Change subject: [RFC] soc/intel/fsp_reset: Drop global-reset sanity check
......................................................................

Patch Set 1:

(1 comment)

File src/soc/intel/common/fsp_reset.c:

https://review.coreboot.org/c/coreboot/+/84558/comment/4c725227_69021bb0?usp... :
PS1, Line 78: 		/* Use RESET_REQUIRED_3 status for global resets. Needs
            : 		   to be kept in sync with chipset_handle_reset() above. */
            : 		return FSP_STATUS_RESET_REQUIRED_3;
> > I believe that the global-reset check inside chipset_handle_reset() doesn't add much value, since  […]

I like the idea of getting rid of the check as a simplification of the code as this is the only type of reset remaining. If it turns out we need to have a check later, there is always room to re-introduce one.

The issue in my opinion is that the FSP status for global reset is not 100% stable across generations and `EFI_RESET_TYPE` does not offer a well identified global reset option.

So to get rid of the check, I would suggest to re-architecture a bit further:

1.  Have our own reset type `enum reset_type { warm, cold, global, none }` or `enum soc_reset_type` ?
2.  Return this type in `fsp_get_pch_reset_status()`.
3.  Have a `fsp_status_to_reset_type()` function converting a reset status to a `enum reset_type`
4.  Simplify `fsp_handle_reset()`:
    1.  Use `fsp_status_to_reset_type()`
    2.  print the actual type of reset (warm, cold or global) instead of the status (which is my opinion is an improvement)
    3.  Call `global_reset()` directly ? Why do we call `chipset_handle_reset()` in the first place ?

Just my two cents.