Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30742 )
Change subject: vendorcode/intel/fsp1_0/broadwell_de: Use FSP from 3rdparty/fsp ......................................................................
Patch Set 6:
(3 comments)
How does this work for those that generate their own FSP binary and headers from source?
How did it work before? Actually, with this change (and the addition of `FSP_SRC_PATH`) there are less paths hardcoded than before.
Unless I'm missing something
I think you do. Have you looked into it at all?
https://review.coreboot.org/#/c/30742/5/src/drivers/intel/fsp1_0/Makefile.in... File src/drivers/intel/fsp1_0/Makefile.inc:
https://review.coreboot.org/#/c/30742/5/src/drivers/intel/fsp1_0/Makefile.in... PS5, Line 51: ramstage-y += $(wildcard $(top)/$(call strip_quotes,$(CONFIG_FSP_SRC_PATH))/*.c)
Replaced wildcard with single Kconfig file path
Thanks. That's even better, as Jay's concerns show.
https://review.coreboot.org/#/c/30742/6/src/soc/intel/fsp_broadwell_de/fsp/K... File src/soc/intel/fsp_broadwell_de/fsp/Kconfig:
https://review.coreboot.org/#/c/30742/6/src/soc/intel/fsp_broadwell_de/fsp/K... PS6, Line 14: string "Location of FSP headers" This should be declared where it's used, i.e. in `drivers/intel/fsp1_0`. The default should stay here ofc.
https://review.coreboot.org/#/c/30742/6/src/soc/intel/fsp_broadwell_de/fsp/K... PS6, Line 18: string "Additional FSP source file" Same as above.