[coreboot-gerrit] [S] Change in coreboot[main]: vc/intel/fsp: Refactor FSP header inclusion for EDK2 compatibility

[Subrata Banik (Code Review)]

Attention is currently required from: Appukuttan V K, Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Ronak Kanabar.

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

Change subject: vc/intel/fsp: Refactor FSP header inclusion for EDK2 compatibility
......................................................................

Patch Set 5:

(1 comment)

File src/vendorcode/intel/fsp/fsp2_0/IntelFspPkg/Include/FspInfoHeader.h:

https://review.coreboot.org/c/coreboot/+/81623/comment/bc3e4fa9_c30e15c2 :
PS5, Line 7: vendorcode/intel/edk2/UDK2017/
> > > > > > > > > This prefix is used in other places in the tree. Can we use a Kconfig param for it that depends on the UDK kconfig? That would make this file trivial and same with vendorcode/intel/Makefile.mk
> > > > > > > > 
> > > > > > > > can you please help to share a little more details which I can take as an incremental change.
> > > > > > > > 
> > > > > > > > I'm sensing that you are asking to introduce a kconfig for each `UDK version` to mask the `vendorcode/intel/edk2/UDK2017` path ?
> > > > > > > > 
> > > > > > > > Then, this file only includes only one header like
> > > > > > > > 
> > > > > > > > 
> > > > > > > > ```#include <CONFIG(UDK_PATH)/Include/Guid/FspHeaderFile.h>```
> > > > > > > > 
> > > > > > > > UDK_PATH would depend on UDK configs like UDK_2017_VERSION etc.?
> > > > > > > 
> > > > > > > Yes that would be my suggestion. It's used in other places (makefile) too so that's why I think it might be a good idea.
> > > > > >
> > > > > > sure, let me see if I can address it here or i will submit an incremental CL. sounds good to you ?
> > > > >
> > > > > Sure
> > > >
> > > > question: wondering if we define a Kconfig with type string then we might ended up with double quote (which we might need to strip before concatenate for #include)?
> > > > config UDK_2017_PATH
> > > >    string "vendorcode/intel/edk2/UDK2017"
> > > >
> > > >
> > > > #include <CONFIG(UDK_PATH)/Include/Guid/FspHeaderFile.h>
> > > > |
> > > > v
> > > > #include <"vendorcode/intel/edk2/UDK2017"/Include/Guid/FspHeaderFile.h>
> > >
> > > Hmm I suppose makefile could do the quote stripping and passing it via a compiler commandline argument '-D' in CFLAGS to have it without quotes in C.
> >
> > yes possible for makefile. I'm wondering for #include in header file.
>
> I suppose the header is always included in a C file so if the UDK path is passed via commandline define it can be used in the #include statement in this file?

yes, using makefile.mk for sure (for fsp_relocate.c)