[M] Change in coreboot[main]: drivers/intel/fsp2_0: Support FSP 2.4 64-bits

[Subrata Banik (Code Review)]

Attention is currently required from: Andrey Petrov, Arthur Heymans, Bora Guvendik, Christian Walter, Felix Held, Fred Reitberger, Jason Glenesk, Johnny Lin, Jérémy Compostella, Lean Sheng Tan, Matt DeVillier, Patrick Rudolph, Ronak Kanabar, Shuo Liu, Tim Chu, Wonkyu Kim.

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80277?usp=email )

Change subject: drivers/intel/fsp2_0: Support FSP 2.4 64-bits
......................................................................

Patch Set 2:

(15 comments)

File src/drivers/intel/fsp2_0/Kconfig:

https://review.coreboot.org/c/coreboot/+/80277/comment/b7cac58d_5d46c73c :
PS2, Line 62: Starting with FSP specification 2.4 they can be 64-bits.
i thought u said elsewhere that FSP 2.4 also supports 32-bit?

base don line#58, I'm unable to follow how can one select 32-bit flow of FSP2.4?

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

https://review.coreboot.org/c/coreboot/+/80277/comment/a99d878f_bd6645fc :
PS2, Line 94: efi_return_status_t
need to include `#include <efi/efi_datatype.h>`

https://review.coreboot.org/c/coreboot/+/80277/comment/94fc20ac_b4afbb7a :
PS2, Line 97: status
why not use `size_t status` directly ?

File src/drivers/intel/fsp2_0/include/fsp/api.h:

https://review.coreboot.org/c/coreboot/+/80277/comment/d533c158_ac7a74d7 :
PS2, Line 11: #define __efiapi	EFIAPI
why not inside `efi_datatype.h` file ?

File src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h:

https://review.coreboot.org/c/coreboot/+/80277/comment/5af5574e_b4bbae99 :
PS2, Line 13: #include <efi/efi_datatype.h>
order ?

File src/drivers/intel/fsp2_0/include/fsp/info_header.h:

https://review.coreboot.org/c/coreboot/+/80277/comment/aa66f7a4_94cfe2a2 :
PS2, Line 39: #if CONFIG(PLATFORM_USES_FSP2_4)
look at line #36, i don;t think u need a guard here as well. we are not using sizeof(fsp_header) anywhere IMO

File src/drivers/intel/fsp2_0/include/fsp/soc_binding.h:

https://review.coreboot.org/c/coreboot/+/80277/comment/8cd4782c_ddb89ade :
PS2, Line 8: #if CONFIG(VENDOR_INTEL)
why Intel? this is UEFI calling convention and nothing SoC vendor specific

also use edi_datatype.h for keeping UEFI specific declaration

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

https://review.coreboot.org/c/coreboot/+/80277/comment/1472693a_20af67e1 :
PS2, Line 282: efi_return_status_t
size_t?

File src/include/efi/efi_datatype.h:

https://review.coreboot.org/c/coreboot/+/80277/comment/026ab103_0a14713b :
PS2, Line 81: statuses
???

https://review.coreboot.org/c/coreboot/+/80277/comment/da9a9bfc_123a4adc :
PS2, Line 88: _Static_assert(sizeof(size_t) == sizeof(efi_return_status_t),
            : 	       "Unexpected EFI_STATUS size");
if u use `size_t` over `efi_return_status_t` then I don't believe u need this logic

File src/soc/intel/xeon_sp/bootblock.c:

https://review.coreboot.org/c/coreboot/+/80277/comment/7648b325_394a4246 :
PS2, Line 5: #include <fsp/util.h>
please submit this cl separately

https://review.coreboot.org/c/coreboot/+/80277/comment/aba490be_58eccd89 :
PS2, Line 9: #include <soc/iomap.h>
same

https://review.coreboot.org/c/coreboot/+/80277/comment/6fd5b5ba_9a5d1e1f :
PS2, Line 10: #include <console/console.h>
            : #include <cpu/x86/mtrr.h>
this is also out of order

https://review.coreboot.org/c/coreboot/+/80277/comment/cdb7e30c_9249f016 :
PS2, Line 12: #include <intelblocks/lpc_lib.h>
same

https://review.coreboot.org/c/coreboot/+/80277/comment/2fdf612b_9c1fc96b :
PS2, Line 15: #include <soc/bootblock.h>
same