Change in coreboot[master]: drivers/intel: Make FSP_USE_REPO an SOC optin instead of list dependency
Mimoja has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: drivers/intel: Make FSP_USE_REPO an SOC optin instead of list dependency ......................................................................
drivers/intel: Make FSP_USE_REPO an SOC optin instead of list dependency
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in an list beeing not only hard to maintain but also prune to errors. To change that behaviour this commit is introducing the HAVE_INTEL_FSP_REPO config for SOCs that are supported from withing 3rdparty/fsp. All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 11 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/1
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 8c8ac59..9d1aa88 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -53,10 +53,7 @@ config FSP_USE_REPO bool "Use the IntelFSP based binaries" depends on ADD_FSP_BINARIES - depends on SOC_INTEL_APOLLOLAKE || SOC_INTEL_SKYLAKE || \ - SOC_INTEL_KABYLAKE || SOC_INTEL_COFFEELAKE || \ - SOC_INTEL_ICELAKE || SOC_INTEL_WHISKEYLAKE || \ - SOC_INTEL_DENVERTON_NS + depends on HAVE_INTEL_FSP_REPO help When selecting this option, the SoC must set FSP_HEADER_PATH and FSP_FD_PATH correctly so FSP splitting works. diff --git a/src/soc/intel/Kconfig b/src/soc/intel/Kconfig index 1eebeb6..16f43df 100644 --- a/src/soc/intel/Kconfig +++ b/src/soc/intel/Kconfig @@ -46,3 +46,8 @@ than the one in non-topswap bootblock. This string will be passed onto ifittool (-A -n option). ifittool will not parse the region for MCU entries, and only locate the region and insert its address into FIT. + +config HAVE_INTEL_FSP_REPO + bool "Use FSP Blobs from fsp submodule" + help + This SOC has FSP binaries living in 3rdparty/fsp. diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index 0b3b30a..8dc7628 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -44,6 +44,7 @@ select GENERIC_GPIO_LIB select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER + select HAVE_INTEL_FSP_REPO select MRC_SETTINGS_PROTECT select MRC_SETTINGS_VARIABLE_DATA select NO_FIXED_XIP_ROM_SIZE diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 40c2fd7..8b55ac7 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -64,6 +64,7 @@ select CPU_INTEL_FIRMWARE_INTERFACE_TABLE select FSP_M_XIP select GENERIC_GPIO_LIB + select HAVE_INTEL_FSP_REPO select HAVE_FSP_GOP select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER diff --git a/src/soc/intel/denverton_ns/Kconfig b/src/soc/intel/denverton_ns/Kconfig index 9445977..0289cf4 100644 --- a/src/soc/intel/denverton_ns/Kconfig +++ b/src/soc/intel/denverton_ns/Kconfig @@ -33,6 +33,7 @@ select SOC_INTEL_COMMON_RESET select PLATFORM_USES_FSP2_0 select IOAPIC + select HAVE_INTEL_FSP_REPO select HAVE_SMI_HANDLER select CACHE_MRC_SETTINGS select PARALLEL_MP diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 4077619..1dd8bf1 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -20,6 +20,7 @@ select FSP_M_XIP select GENERIC_GPIO_LIB select HAVE_FSP_GOP + select HAVE_INTEL_FSP_REPO select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select IDT_IN_EVERY_STAGE diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 31f809a..cde9909 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -38,6 +38,7 @@ select INTEL_CAR_NEM_ENHANCED select INTEL_GMA_ACPI select INTEL_GMA_ADD_VBT if RUN_FSP_GOP + select HAVE_INTEL_FSP_REPO select IOAPIC select MRC_SETTINGS_PROTECT select NO_FIXED_XIP_ROM_SIZE
Hello Patrick Rudolph, Vanessa Eusebio, David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37582
to look at the new patch set (#2).
Change subject: drivers/intel: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
drivers/intel: Make FSP_USE_REPO an SOC opt-in instead of list dependency
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in an list beeing not only hard to maintain but also prune to errors. To change that behaviour this commit is introducing the HAVE_INTEL_FSP_REPO config for SOCs that are supported from withing 3rdparty/fsp. All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 11 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/2
Hello Patrick Rudolph, Vanessa Eusebio, build bot (Jenkins), David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37582
to look at the new patch set (#3).
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in an list beeing not only hard to maintain but also prune to errors. To change that behaviour this commit is introducing the HAVE_INTEL_FSP_REPO config for SOCs that are supported from withing 3rdparty/fsp. All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 11 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/3
Hello Patrick Rudolph, Vanessa Eusebio, build bot (Jenkins), David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37582
to look at the new patch set (#4).
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in an list beeing not only hard to maintain but also prune to errors. To change that behaviour this commit is introducing the HAVE_INTEL_FSP_REPO config for SOCs that are supported from withing 3rdparty/fsp. All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 12 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/4
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
Patch Set 5: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/5//COMMIT_MSG@10 PS5, Line 10: an list beeing nit: a list being
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/5//COMMIT_MSG@15 PS5, Line 15: All other interactions with FSP_USE_REPO stay the same. Please add exactly one blank line between paragraphs.
Hello build bot (Jenkins), Patrick Georgi, David Guckian, Vanessa Eusebio, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37582
to look at the new patch set (#6).
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in an list beeing not only hard to maintain but also prune to errors.
To change that behaviour this commit is introducing the HAVE_INTEL_FSP_REPO config for SOCs that are supported from withing 3rdparty/fsp.
All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig M src/soc/intel/xeon_sp/Kconfig 8 files changed, 13 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/6
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
Patch Set 6:
Please rebase on https://review.coreboot.org/c/coreboot/+/39372. This fixes some of the errors.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... PS6, Line 68: select HAVE_INTEL_FSP_REPO Select this option by each SOC at the beginning of the file, except SOC_INTEL_CANNONLAKE
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@7 PS6, Line 7: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency Summary has 50 characters limit
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... PS6, Line 68: select HAVE_INTEL_FSP_REPO
Select this option by each SOC at the beginning of the file, except SOC_INTEL_CANNONLAKE
You also could just use `if !SOC_INTEL_CANNONLAKE`
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
Patch Set 6: Code-Review+1
(10 comments)
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@7 PS6, Line 7: SOC nit: I usually see SoC
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@7 PS6, Line 7: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency
Summary has 50 characters limit
Can remove "instead of list dependency"
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@10 PS6, Line 10: ee Only one `e`
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@11 PS6, Line 11: prune typo: pr*o*ne
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@13 PS6, Line 13: is introducing Use the same tense as the commit summary: introduces
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@14 PS6, Line 14: g trailing `g`
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/Kconfig File src/soc/intel/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/Kconfig@53 PS6, Line 53: "Use FSP Blobs from fsp submodule" This shouldn't have a prompt
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/Kconfig@55 PS6, Line 55: This SOC has FSP binaries living in 3rdparty/fsp. How about:
Select this if the platform has FSP binaries are publicly available in 3rdparty/fsp.
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... PS6, Line 68: select HAVE_INTEL_FSP_REPO
You also could just use `if !SOC_INTEL_CANNONLAKE`
Not sure if CannonLake just uses the CoffeeLake FSP package, or if it is not supported at all
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/xeon_sp/Kconf... File src/soc/intel/xeon_sp/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/xeon_sp/Kconf... PS6, Line 41: select HAVE_INTEL_FSP_REPO Um, I don't think so.
Felix Singer has uploaded a new patch set (#8) to the change originally created by Mimoja. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in an list beeing not only hard to maintain but also prune to errors.
To change that behaviour this commit is introducing the HAVE_INTEL_FSP_REPO config for SOCs that are supported from withing 3rdparty/fsp.
All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de Signed-off-by: Felix Singer felixsinger@posteo.net --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 12 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/8
Felix Singer has uploaded a new patch set (#9) to the change originally created by Mimoja. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency ......................................................................
drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in an list beeing not only hard to maintain but also prune to errors.
To change that behaviour this commit is introducing the HAVE_INTEL_FSP_REPO config for SOCs that are supported from withing 3rdparty/fsp.
All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de Signed-off-by: Felix Singer felixsinger@posteo.net --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 13 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/9
Felix Singer has uploaded a new patch set (#10) to the change originally created by Mimoja. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in an list beeing not only hard to maintain but also prune to errors.
To change that behaviour this commit is introducing the HAVE_INTEL_FSP_REPO config for SOCs that are supported from withing 3rdparty/fsp.
All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de Signed-off-by: Felix Singer felixsinger@posteo.net --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 13 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/10
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 10:
(5 comments)
https://review.coreboot.org/c/coreboot/+/37582/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/5//COMMIT_MSG@15 PS5, Line 15: All other interactions with FSP_USE_REPO stay the same.
Please add exactly one blank line between paragraphs.
Done
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/Kconfig File src/soc/intel/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/Kconfig@53 PS6, Line 53: "Use FSP Blobs from fsp submodule"
This shouldn't have a prompt
Done
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/Kconfig@55 PS6, Line 55: This SOC has FSP binaries living in 3rdparty/fsp.
How about: […]
Done
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... PS6, Line 68: select HAVE_INTEL_FSP_REPO
Not sure if CannonLake just uses the CoffeeLake FSP package, or if it is not supported at all
Can't tell, but I would go with `if !SOC_INTEL_CANNONLAKE` since `FSP_FD_PATH` has no default value for it.
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/xeon_sp/Kconf... File src/soc/intel/xeon_sp/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/xeon_sp/Kconf... PS6, Line 41: select HAVE_INTEL_FSP_REPO
Um, I don't think so.
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... PS6, Line 68: select HAVE_INTEL_FSP_REPO
Can't tell, but I would go with `if !SOC_INTEL_CANNONLAKE` since `FSP_FD_PATH` has no default value […]
I doubt Cannon Lake works with the CFL FSP, its headers are incompatible, IIRC, and looking at a certain Git repo, it seems one has to compile for one or the other. (CNL seems closer to ICL than to anything in cannonlake/)
I would prefer to move the `select` to the respective platforms above.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... PS6, Line 68: select HAVE_INTEL_FSP_REPO
I doubt Cannon Lake works with the CFL FSP, its headers are incompatible, […]
Oh right, now I recall.
Felix Singer has uploaded a new patch set (#11) to the change originally created by Mimoja. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in an list beeing not only hard to maintain but also prune to errors.
To change that behaviour this commit is introducing the HAVE_INTEL_FSP_REPO config for SOCs that are supported from withing 3rdparty/fsp.
All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de Signed-off-by: Felix Singer felixsinger@posteo.net --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 13 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/11
Felix Singer has uploaded a new patch set (#12) to the change originally created by Mimoja. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SOCs. This results in a list being not only hard to maintain but also prone to errors.
To change that behaviour this commit introduces the HAVE_INTEL_FSP_REPO config for SOCs that are supported from within 3rdparty/fsp.
All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de Signed-off-by: Felix Singer felixsinger@posteo.net --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 15 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/12
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 12:
(8 comments)
https://review.coreboot.org/c/coreboot/+/37582/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/5//COMMIT_MSG@10 PS5, Line 10: an list beeing
nit: a list being
Done
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@7 PS6, Line 7: SOC
nit: I usually see SoC
Done
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@7 PS6, Line 7: drivers/intel/fsp2_0: Make FSP_USE_REPO an SOC opt-in instead of list dependency
Can remove "instead of list dependency"
Done
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@10 PS6, Line 10: ee
Only one `e`
Done
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@11 PS6, Line 11: prune
typo: pr*o*ne
Done
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@13 PS6, Line 13: is introducing
Use the same tense as the commit summary: introduces
Done
https://review.coreboot.org/c/coreboot/+/37582/6//COMMIT_MSG@14 PS6, Line 14: g
trailing `g`
Done
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/6/src/soc/intel/cannonlake/Kc... PS6, Line 68: select HAVE_INTEL_FSP_REPO
Oh right, now I recall.
Done
Felix Singer has uploaded a new patch set (#13) to the change originally created by Mimoja. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SoCs. This results in a list being not only hard to maintain but also prone to errors.
To change that behaviour this commit introduces the HAVE_INTEL_FSP_REPO config for SoCs that are supported from within 3rdparty/fsp.
All other interactions with FSP_USE_REPO stay the same.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de Signed-off-by: Felix Singer felixsinger@posteo.net --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 15 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/13
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 15: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/15/src/soc/intel/Kconfig File src/soc/intel/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/15/src/soc/intel/Kconfig@42 PS15, Line 42: HAVE_INTEL_FSP_REPO What do you think about having a negative config "FSP_REPO_NOT_PUBLIC"? Then, only the platforms that do not have their FSP repo public need to set this. And once it transitions to having a public repo, that config selection can be removed from the SoC along with its header files.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/15/src/soc/intel/Kconfig File src/soc/intel/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/15/src/soc/intel/Kconfig@42 PS15, Line 42: HAVE_INTEL_FSP_REPO
What do you think about having a negative config "FSP_REPO_NOT_PUBLIC"? Then, only the platforms tha […]
I don't really like negative Kconfig symbols (they don't play nice with Kconfig's additive nature). However, it would not be a problem here, so I don't have a strong opinion on the matter.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 15: Code-Review+2
(1 comment)
I wouldn't mind a negative option as long as the build would break when one forgets to set it...
https://review.coreboot.org/c/coreboot/+/37582/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/15//COMMIT_MSG@17 PS15, Line 17: All other interactions with FSP_USE_REPO stay the same. Not 100% true as you set it to `default y`.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/15/src/soc/intel/Kconfig File src/soc/intel/Kconfig:
https://review.coreboot.org/c/coreboot/+/37582/15/src/soc/intel/Kconfig@42 PS15, Line 42: HAVE_INTEL_FSP_REPO
I don't really like negative Kconfig symbols (they don't play nice with Kconfig's additive nature). […]
I don't like negative options either. Also, in this case, I think, we would reduce the barrier for using the proprietary FSP blobs.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/15//COMMIT_MSG@17 PS15, Line 17: All other interactions with FSP_USE_REPO stay the same.
Not 100% true as you set it to `default y`.
Does anybody want to clear this up?
Felix Singer has uploaded a new patch set (#16) to the change originally created by Mimoja. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SoCs. This results in a list being not only hard to maintain but also prone to errors.
To change that behaviour this commit introduces the HAVE_INTEL_FSP_REPO config option for SoCs that are supported from within 3rdparty/fsp.
If a SoC selects HAVE_INTEL_FSP_REPO the config option FSP_USE_REPO is selected by default, but can be still deselected by the user in menuconfig.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de Signed-off-by: Felix Singer felixsinger@posteo.net --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 15 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/37582/16
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37582/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37582/15//COMMIT_MSG@17 PS15, Line 17: All other interactions with FSP_USE_REPO stay the same.
Does anybody want to clear this up?
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 16:
Let's ignore the overlong lines in the commit message....
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in
For quite a bit now we are extending the FSP_USE_REPO option to be available for all Intel SoCs. This results in a list being not only hard to maintain but also prone to errors.
To change that behaviour this commit introduces the HAVE_INTEL_FSP_REPO config option for SoCs that are supported from within 3rdparty/fsp.
If a SoC selects HAVE_INTEL_FSP_REPO the config option FSP_USE_REPO is selected by default, but can be still deselected by the user in menuconfig.
Change-Id: I68ae373ce591f06073064aa75aac32ceca8fa1cc Signed-off-by: Johanna Schander coreboot@mimoja.de Signed-off-by: Felix Singer felixsinger@posteo.net Reviewed-on: https://review.coreboot.org/c/coreboot/+/37582 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/icelake/Kconfig M src/soc/intel/skylake/Kconfig 7 files changed, 15 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index cf79201..1e1cc19 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -53,10 +53,8 @@ config FSP_USE_REPO bool "Use the IntelFSP based binaries" depends on ADD_FSP_BINARIES - depends on SOC_INTEL_APOLLOLAKE || SOC_INTEL_SKYLAKE || \ - SOC_INTEL_KABYLAKE || SOC_INTEL_COFFEELAKE || \ - SOC_INTEL_ICELAKE || SOC_INTEL_WHISKEYLAKE || \ - SOC_INTEL_DENVERTON_NS || SOC_INTEL_COMETLAKE + depends on HAVE_INTEL_FSP_REPO + default y help When selecting this option, the SoC must set FSP_HEADER_PATH and FSP_FD_PATH correctly so FSP splitting works. diff --git a/src/soc/intel/Kconfig b/src/soc/intel/Kconfig index d519068..75f2f70 100644 --- a/src/soc/intel/Kconfig +++ b/src/soc/intel/Kconfig @@ -38,3 +38,9 @@ than the one in non-topswap bootblock. This string will be passed onto ifittool (-A -n option). ifittool will not parse the region for MCU entries, and only locate the region and insert its address into FIT. + +config HAVE_INTEL_FSP_REPO + bool + help + Select this, if the FSP binaries for the platform are public available + in 3rdparty/fsp. diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index 2bc49c8..ed35eaa 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -44,6 +44,7 @@ select GENERIC_GPIO_LIB select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER + select HAVE_INTEL_FSP_REPO select MRC_SETTINGS_PROTECT select MRC_SETTINGS_VARIABLE_DATA select NO_XIP_EARLY_STAGES diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 1495e2e..baf8756 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -24,6 +24,7 @@ bool select SOC_INTEL_CANNONLAKE_BASE select FSP_USES_CB_STACK + select HAVE_INTEL_FSP_REPO help Intel Coffeelake support
@@ -31,6 +32,7 @@ bool select SOC_INTEL_CANNONLAKE_BASE select FSP_USES_CB_STACK + select HAVE_INTEL_FSP_REPO help Intel Whiskeylake support
@@ -39,6 +41,7 @@ select SOC_INTEL_CANNONLAKE_BASE select MICROCODE_BLOB_UNDISCLOSED select FSP_USES_CB_STACK + select HAVE_INTEL_FSP_REPO help Intel Cometlake support
diff --git a/src/soc/intel/denverton_ns/Kconfig b/src/soc/intel/denverton_ns/Kconfig index 6ca7f3e..c628dbd 100644 --- a/src/soc/intel/denverton_ns/Kconfig +++ b/src/soc/intel/denverton_ns/Kconfig @@ -32,6 +32,7 @@ select SOC_INTEL_COMMON_RESET select PLATFORM_USES_FSP2_0 select IOAPIC + select HAVE_INTEL_FSP_REPO select HAVE_SMI_HANDLER select CACHE_MRC_SETTINGS select PARALLEL_MP diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 9e97d2c..559ba6c 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -20,6 +20,7 @@ select FSP_M_XIP select GENERIC_GPIO_LIB select HAVE_FSP_GOP + select HAVE_INTEL_FSP_REPO select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER select IDT_IN_EVERY_STAGE diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 4493f9b..2beda43 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -39,6 +39,7 @@ select INTEL_CAR_NEM_ENHANCED select INTEL_GMA_ACPI select INTEL_GMA_ADD_VBT if RUN_FSP_GOP + select HAVE_INTEL_FSP_REPO select IOAPIC select MRC_SETTINGS_PROTECT select PARALLEL_MP
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 17:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/1922 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1921 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/1920
Please note: This test is under development and might not be accurate at all!
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37582 )
Change subject: intel/fsp2_0: Make FSP_USE_REPO a SoC opt-in ......................................................................
Patch Set 17:
Thank you Felix for taking over!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Felix Singer (Code Review)
-
Furquan Shaikh (Code Review)
-
Mimoja (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)