[coreboot-gerrit] [M] Change in coreboot[main]: drivers/intel/fsp2_0: Simplify FSP global reset definition

[Subrata Banik (Code Review)]

Attention is currently required from: Andrey Petrov, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Paul Menzel, Pranava Y N, Rishika Raj, Ronak Kanabar, Sean Rhodes, Tarun.

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

Change subject: drivers/intel/fsp2_0: Simplify FSP global reset definition
......................................................................

Patch Set 6:

(1 comment)

File src/soc/intel/apollolake/Kconfig:

https://review.coreboot.org/c/coreboot/+/84356/comment/7c8b33ea_8612faa6?usp... :
PS6, Line 400: 	default 5
> I disagree. The previous implementation was unmanageable.

you can only blame the FSP spec for that. look at FSP spec < 2.4 (if you think it is unmanageable)
> It was duplicating information, hard-coding values and adding unnecessary complexity. What is wrong in having a simple number as documented in the Kconfig instead of creating many Kconfig ?

The previous implementation was following "FSP EAS v2.0 section 11.2.2 - OEM Status Code".

details: https://chrome-internal.googlesource.com/chromeos/third_party/intel-fsp/edk2...

looks like with FSP2.4 specification, Intel has added support for 64-bit hence, updated the hardcoded macro as https://github.com/tianocore/edk2/blob/master/IntelFsp2Pkg/Include/FspEas/Fs...

The issue is that we need to keep things consistent between the old and new spec FSPs. In the older spec, the values `3` and `5` don't mean anything on their own. They're always the full 32-bit value, like `0x40000003` or `0x40000005`. The latest spec FSP proposes encoding a reset type as `3` or `5` inside a wrapper. This makes sense for an EDK2-based bootloader, but not for us. We need to figure out what the reset type is by looking at the macro, not just the number.