[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP 2.4 support

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

Attention is currently required from: Andrey Petrov, Bora Guvendik, Ronak Kanabar, Subrata Banik, Wonkyu Kim.

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

Change subject: drivers/intel/fsp2_0: Add FSP 2.4 support
......................................................................

Patch Set 3:

(6 comments)

Patchset:

PS3:
> quick feedback
> This CL doesn't mentioned the test vehicle used to verify

Test vehicle is not public. Considering the 2.4 specification is public, we can provide the implementation.
> specify the FSP 2.4 spec link

I gave the document number but I did not add the link as links can change.

File src/drivers/intel/fsp2_0/Kconfig:

https://review.coreboot.org/c/coreboot/+/80275/comment/c9b78efb_ee6bc4c9 :
PS3, Line 58: default n if PLATFORM_USES_FSP2_4
> ideally this config can be only select if `depends on !PLATFORM_USES_FSP2_4`

2.4 also supports 32-bits.

File src/drivers/intel/fsp2_0/memory_init.c:

https://review.coreboot.org/c/coreboot/+/80275/comment/a30afcba_e976d340 :
PS3, Line 32: void __weak platform_fsp_multi_phase_init_cb(uint32_t phase_index)
> nit: […]

The idea was to reuse the same hook. The risk is that if we have a converged romstage and ramstage image then this would break... I'll separate those.

https://review.coreboot.org/c/coreboot/+/80275/comment/bbbc2147_ff2092c6 :
PS3, Line 282: error_handler
> nit: […]

Considering we are in memory_init.c and this is static function I don't see the benefit of surcharging the function name with unnecessary prefix.

https://review.coreboot.org/c/coreboot/+/80275/comment/e5ed38f3_310e5125 :
PS3, Line 297: multi_phase_init
> nit: multi_phase_mem_init

Considering we are in memory_init.c and this is static function I don't see the benefit of surcharging the function name with unnecessary prefix.

https://review.coreboot.org/c/coreboot/+/80275/comment/6794e6f0_8d9fe00f :
PS3, Line 437: 	if (hdr->fsp_multi_phase_mem_init_entry_offset)
> why not like this ? […]

Wouldn't it be dangerous to look outside after the limit of the data structure ? These fields are added with 2.4.