Change in coreboot[master]: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake
Mimoja has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake
This commits adds a dependency check for the FSP_USE_REPO config option which so far was not able to deal with IceLake systems.
Change-Id: I29faa8d3acff5680b611951fc193d33f514dc0d3 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/37561/1
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 77382d3..c51b268 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -55,7 +55,7 @@ depends on ADD_FSP_BINARIES depends on SOC_INTEL_APOLLOLAKE || SOC_INTEL_SKYLAKE || \ SOC_INTEL_KABYLAKE || SOC_INTEL_COFFEELAKE || \ - SOC_INTEL_WHISKEYLAKE + SOC_INTEL_ICELAKE || SOC_INTEL_WHISKEYLAKE help When selecting this option, the SoC must set FSP_HEADER_PATH and FSP_FD_PATH correctly so FSP splitting works.
Christoph Pomaska has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 1: Code-Review+1
Hello Patrick Rudolph, Christoph Pomaska, Jonathan Neuschäfer, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37561
to look at the new patch set (#2).
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake
This commits adds a dependency check for the FSP_USE_REPO config option which so far was not able to deal with IceLake systems.
It also disables it by default to not break with the existing IceLake systems.
Change-Id: I29faa8d3acff5680b611951fc193d33f514dc0d3 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/37561/2
Hello Patrick Rudolph, Christoph Pomaska, Jonathan Neuschäfer, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37561
to look at the new patch set (#3).
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake
This commits adds a dependency check for the FSP_USE_REPO config option which so far was not able to deal with IceLake systems.
For this to work we also need to update 3rdparty/fsp to a commit that contains the FSP binaries required for this option to work.
Change-Id: I29faa8d3acff5680b611951fc193d33f514dc0d3 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M 3rdparty/fsp M src/drivers/intel/fsp2_0/Kconfig 2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/37561/3
Hello Patrick Rudolph, Christoph Pomaska, Jonathan Neuschäfer, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37561
to look at the new patch set (#4).
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake
This commits adds a dependency check for the FSP_USE_REPO config option which so far was not able to deal with IceLake systems.
Change-Id: I29faa8d3acff5680b611951fc193d33f514dc0d3 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/37561/4
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37561/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37561/4//COMMIT_MSG@7 PS4, Line 7: to add adding
Hello Patrick Rudolph, Christoph Pomaska, Jonathan Neuschäfer, Philipp Deppenwiese, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37561
to look at the new patch set (#5).
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake
This commit is adding a dependency check for the FSP_USE_REPO config option which so far was not able to deal with IceLake systems.
Change-Id: I29faa8d3acff5680b611951fc193d33f514dc0d3 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/37561/5
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 6:
(1 comment)
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37561/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37561/4//COMMIT_MSG@7 PS4, Line 7: to add
adding
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 6:
Should we drop `vendorcode/intel/fsp/fsp2_0/icelake/`?
Hello Aaron Durbin, Patrick Rudolph, Christoph Pomaska, Subrata Banik, Rizwan Qureshi, Jonathan Neuschäfer, Philipp Deppenwiese, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37561
to look at the new patch set (#7).
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake
This commit is adding a dependency check for the FSP_USE_REPO config option which so far was not able to deal with IceLake systems.
Change-Id: I29faa8d3acff5680b611951fc193d33f514dc0d3 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/intel/fsp2_0/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/37561/7
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 7:
Patch Set 6:
Should we drop `vendorcode/intel/fsp/fsp2_0/icelake/`?
Maybe I misunderstand things, but adding the option to read it for the 3rdparty repo is there to be an addition. It still should work without the in-tree repo checked out, right?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 7:
Should we drop `vendorcode/intel/fsp/fsp2_0/icelake/`?
Maybe I misunderstand things, but adding the option to read it for the 3rdparty repo is there to be an addition. It still should work without the in-tree repo checked out, right?
Not really. Well, depends on what should work. It wouldn't boot anyway without the binaries. And for build tests, we assume the 3rdparty/ repos to be checked out.
It's not a requirement to drop the stale copies now. Still, I'd like to take the opportunity to discuss it. Otherwise we might drag them along for ever without anybody making use of them.
Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 7:
Patch Set 7:
Should we drop `vendorcode/intel/fsp/fsp2_0/icelake/`?
Maybe I misunderstand things, but adding the option to read it for the 3rdparty repo is there to be an addition. It still should work without the in-tree repo checked out, right?
Not really. Well, depends on what should work. It wouldn't boot anyway without the binaries. And for build tests, we assume the 3rdparty/ repos to be checked out.
It's not a requirement to drop the stale copies now. Still, I'd like to take the opportunity to discuss it. Otherwise we might drag them along for ever without anybody making use of them.
Shall we merge it like this and I open an additional CR to cleanup vendorcode/intel/fsp2_0 ?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 7:
Should we drop `vendorcode/intel/fsp/fsp2_0/icelake/`?
Maybe I misunderstand things, but adding the option to read it for the 3rdparty repo is there to be an addition. It still should work without the in-tree repo checked out, right?
Not really. Well, depends on what should work. It wouldn't boot anyway without the binaries. And for build tests, we assume the 3rdparty/ repos to be checked out.
It's not a requirement to drop the stale copies now. Still, I'd like to take the opportunity to discuss it. Otherwise we might drag them along for ever without anybody making use of them.
Shall we merge it like this and I open an additional CR to cleanup vendorcode/intel/fsp2_0 ?
If you are in a hurry, we can do that. I would just have waited for people to comment here.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 7: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/37561/7/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/37561/7/src/drivers/intel/fsp2_0/Kc... PS7, Line 58: SOC_INTEL_ICELAKE || SOC_INTEL_WHISKEYLAKE At some point we should turn this into a opt-out before the list becomes too long, I guess.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37561/7/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/37561/7/src/drivers/intel/fsp2_0/Kc... PS7, Line 58: SOC_INTEL_ICELAKE || SOC_INTEL_WHISKEYLAKE
At some point we should turn this into a opt-out before the […]
I am thinking of an additional option HAVE_INTEL_FSP_REPO which would be selected by the SoC and a dependency of FSP_USE_REPO.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37561 )
Change subject: drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake ......................................................................
drivers/intel/fsp2_0: Allow to add FSP binaries from repo for IceLake
This commit is adding a dependency check for the FSP_USE_REPO config option which so far was not able to deal with IceLake systems.
Change-Id: I29faa8d3acff5680b611951fc193d33f514dc0d3 Signed-off-by: Johanna Schander coreboot@mimoja.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/37561 Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/fsp2_0/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index d9d7fb2..824fd0b 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -55,7 +55,7 @@ depends on ADD_FSP_BINARIES depends on SOC_INTEL_APOLLOLAKE || SOC_INTEL_SKYLAKE || \ SOC_INTEL_KABYLAKE || SOC_INTEL_COFFEELAKE || \ - SOC_INTEL_WHISKEYLAKE + SOC_INTEL_ICELAKE || SOC_INTEL_WHISKEYLAKE help When selecting this option, the SoC must set FSP_HEADER_PATH and FSP_FD_PATH correctly so FSP splitting works.
-
Angel Pons (Code Review) -
Christoph Pomaska (Code Review)
-
Felix Singer (Code Review)
-
Mimoja (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)