Change in coreboot[master]: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Wim Vervoorn (Code Review)

1 Nov 2019 1 Nov '19

11:44 a.m.

Wim Vervoorn has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames

The quotes were not stripped for the cbfs filenames of the FSP components. This is causing problems when the regions_for_file macro is executed.

BUG=N/A TEST=build

Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/drivers/intel/fsp2_0/Makefile.inc 1 file changed, 11 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/36548/1

diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc index 8e69377..44f43df 100644 --- a/src/drivers/intel/fsp2_0/Makefile.inc +++ b/src/drivers/intel/fsp2_0/Makefile.inc @@ -44,23 +44,23 @@

# Add FSP blobs into cbfs. SoC code may supply additional options with # -options, e.g --xip or -b -cbfs-files-$(CONFIG_FSP_CAR) += $(CONFIG_FSP_T_CBFS) -$(CONFIG_FSP_T_CBFS)-file := $(call strip_quotes,$(CONFIG_FSP_T_FILE)) -$(CONFIG_FSP_T_CBFS)-type := fsp +cbfs-files-$(CONFIG_FSP_CAR) += $(call strip_quotes,$(CONFIG_FSP_T_CBFS)) +$(call strip_quotes,$(CONFIG_FSP_T_CBFS))-file := $(call strip_quotes,$(CONFIG_FSP_T_FILE)) +$(call strip_quotes,$(CONFIG_FSP_T_CBFS))-type := fsp ifeq ($(CONFIG_FSP_T_XIP),y) -$(CONFIG_FSP_T_CBFS)-options := --xip $(TXTIBB) +$(call strip_quotes,$(CONFIG_FSP_T_CBFS))-options := --xip $(TXTIBB) endif

-cbfs-files-$(CONFIG_ADD_FSP_BINARIES) += $(CONFIG_FSP_M_CBFS) -$(CONFIG_FSP_M_CBFS)-file := $(call strip_quotes,$(CONFIG_FSP_M_FILE)) -$(CONFIG_FSP_M_CBFS)-type := fsp +cbfs-files-$(CONFIG_ADD_FSP_BINARIES) += $(call strip_quotes,$(CONFIG_FSP_M_CBFS)) +$(call strip_quotes,$(CONFIG_FSP_M_CBFS))-file := $(call strip_quotes,$(CONFIG_FSP_M_FILE)) +$(call strip_quotes,$(CONFIG_FSP_M_CBFS))-type := fsp ifeq ($(CONFIG_FSP_M_XIP),y) -$(CONFIG_FSP_M_CBFS)-options := --xip $(TXTIBB) +$(call strip_quotes,$(CONFIG_FSP_M_CBFS))-options := --xip $(TXTIBB) endif

-cbfs-files-$(CONFIG_ADD_FSP_BINARIES) += $(CONFIG_FSP_S_CBFS) -$(CONFIG_FSP_S_CBFS)-file := $(call strip_quotes,$(CONFIG_FSP_S_FILE)) -$(CONFIG_FSP_S_CBFS)-type := fsp +cbfs-files-$(CONFIG_ADD_FSP_BINARIES) += $(call strip_quotes,$(CONFIG_FSP_S_CBFS)) +$(call strip_quotes,$(CONFIG_FSP_S_CBFS))-file := $(call strip_quotes,$(CONFIG_FSP_S_FILE)) +$(call strip_quotes,$(CONFIG_FSP_S_CBFS))-type := fsp

ifeq ($(CONFIG_FSP_USE_REPO),y) $(obj)/Fsp_M.fd: $(call strip_quotes,$(CONFIG_FSP_FD_PATH))

Attachments:

attachment.htm (text/html — 5.0 KB)

Show replies by date

Patrick Georgi (Code Review)

1 Nov 1 Nov

12:06 p.m.

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36548/1/src/drivers/intel/fsp2_0/Ma... File src/drivers/intel/fsp2_0/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/36548/1/src/drivers/intel/fsp2_0/Ma... PS1, Line 47: cbfs-files-$(CONFIG_FSP_CAR) += $(call strip_quotes,$(CONFIG_FSP_T_CBFS)) A pattern we sometimes use to reduce the number of instances of strip_quotes calls is

BOOTSPLASH_SUFFIX=$(suffix $(call strip_quotes,$(CONFIG_BOOTSPLASH_FILE)))

or, even more extreme (also requires more care to see that all other users of the Kconfig variable are fine with that):

CONFIG_DEVICETREE:=$(call strip_quotes, $(CONFIG_DEVICETREE))

Wim Vervoorn (Code Review)

1:38 p.m.

Hello Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36548

to look at the new patch set (#2).

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames

The quotes were not stripped for the cbfs filenames of the FSP components. This is causing problems when the regions_for_file macro is executed.

BUG=N/A TEST=build

Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com --- M src/drivers/intel/fsp2_0/Makefile.inc M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/denverton_ns/Makefile.inc M src/soc/intel/quark/Makefile.inc 4 files changed, 20 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/36548/2

Wim Vervoorn (Code Review)

1:40 p.m.

Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/36548/1/src/drivers/intel/fsp2_0/Ma... File src/drivers/intel/fsp2_0/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/36548/1/src/drivers/intel/fsp2_0/Ma... PS1, Line 47: cbfs-files-$(CONFIG_FSP_CAR) += $(call strip_quotes,$(CONFIG_FSP_T_CBFS))

...

A pattern we sometimes use to reduce the number of instances of strip_quotes calls is […]

I updated the patchset with reduced strip_quotes calls. This looks a lot better. Thanks for the suggestion.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 2 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanny E vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 01 Nov 2019 13:40:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Georgi pgeorgi@google.com Gerrit-MessageType: comment

Nico Huber (Code Review)

3:14 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

Patch Set 2: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/36548/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/36548/2//COMMIT_MSG@10 PS2, Line 10: regions_for_file `regions-for-file`

https://review.coreboot.org/c/coreboot/+/36548/2//COMMIT_MSG@11 PS2, Line 11: executed. I assume, when using vboot and listing these files in CONFIG_RO_REGION_ONLY ?

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 2 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanny E vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 01 Nov 2019 15:14:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Wim Vervoorn (Code Review)

3:28 p.m.

Hello Patrick Rudolph, Vanny E, build bot (Jenkins), Nico Huber, David Guckian, Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36548

to look at the new patch set (#3).

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames

The quotes were not stripped for the cbfs filenames of the FSP components. This is causing problems when the regions-for-file macro is executed.

BUG=N/A TEST=build

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/36548/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 3 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanny E vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Wim Vervoorn (Code Review)

3:30 p.m.

Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/36548/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/36548/2//COMMIT_MSG@10 PS2, Line 10: regions_for_file

...

`regions-for-file`

Done

https://review.coreboot.org/c/coreboot/+/36548/2//COMMIT_MSG@11 PS2, Line 11: executed.

...

I assume, when using vboot and listing these files in CONFIG_RO_REGION_ONLY ?

Indeed

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 3 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanny E vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Fri, 01 Nov 2019 15:30:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Wim Vervoorn (Code Review)

3:35 p.m.

Hello Patrick Rudolph, Vanny E, build bot (Jenkins), Nico Huber, David Guckian, Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36548

to look at the new patch set (#4).

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames

The quotes were not stripped for the cbfs filenames of the FSP components. This is causing problems when the regions-for-file macro is executed (when VBOOT is enabled and the files should be filtered).

BUG=N/A TEST=build

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/36548/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 4 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanny E vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Frans Hendriks (Code Review)

5 Nov 5 Nov

7:03 a.m.

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

Patch Set 4: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 4 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Frans Hendriks fhendriks@eltan.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanny E vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 05 Nov 2019 07:03:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

3:04 p.m.

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames

BUG=N/A TEST=build

Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Signed-off-by: Wim Vervoorn wvervoorn@eltan.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/36548 Reviewed-by: Frans Hendriks fhendriks@eltan.com Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/intel/fsp2_0/Makefile.inc M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/denverton_ns/Makefile.inc M src/soc/intel/quark/Makefile.inc 4 files changed, 20 insertions(+), 16 deletions(-)

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Frans Hendriks: Looks good to me, approved

diff --git a/src/drivers/intel/fsp2_0/Makefile.inc b/src/drivers/intel/fsp2_0/Makefile.inc index 8e69377..806d805 100644 --- a/src/drivers/intel/fsp2_0/Makefile.inc +++ b/src/drivers/intel/fsp2_0/Makefile.inc @@ -42,25 +42,29 @@

CPPFLAGS_common += -I$(src)/drivers/intel/fsp2_0/include

+FSP_T_CBFS = $(call strip_quotes,$(CONFIG_FSP_T_CBFS)) +FSP_M_CBFS = $(call strip_quotes,$(CONFIG_FSP_M_CBFS)) +FSP_S_CBFS = $(call strip_quotes,$(CONFIG_FSP_S_CBFS)) + # Add FSP blobs into cbfs. SoC code may supply additional options with # -options, e.g --xip or -b -cbfs-files-$(CONFIG_FSP_CAR) += $(CONFIG_FSP_T_CBFS) -$(CONFIG_FSP_T_CBFS)-file := $(call strip_quotes,$(CONFIG_FSP_T_FILE)) -$(CONFIG_FSP_T_CBFS)-type := fsp +cbfs-files-$(CONFIG_FSP_CAR) += $(FSP_T_CBFS) +$(FSP_T_CBFS)-file := $(call strip_quotes,$(CONFIG_FSP_T_FILE)) +$(FSP_T_CBFS)-type := fsp ifeq ($(CONFIG_FSP_T_XIP),y) -$(CONFIG_FSP_T_CBFS)-options := --xip $(TXTIBB) +$(FSP_T_CBFS)-options := --xip $(TXTIBB) endif

-cbfs-files-$(CONFIG_ADD_FSP_BINARIES) += $(CONFIG_FSP_M_CBFS) -$(CONFIG_FSP_M_CBFS)-file := $(call strip_quotes,$(CONFIG_FSP_M_FILE)) -$(CONFIG_FSP_M_CBFS)-type := fsp +cbfs-files-$(CONFIG_ADD_FSP_BINARIES) += $(FSP_M_CBFS) +$(FSP_M_CBFS)-file := $(call strip_quotes,$(CONFIG_FSP_M_FILE)) +$(FSP_M_CBFS)-type := fsp ifeq ($(CONFIG_FSP_M_XIP),y) -$(CONFIG_FSP_M_CBFS)-options := --xip $(TXTIBB) +$(FSP_M_CBFS)-options := --xip $(TXTIBB) endif

-cbfs-files-$(CONFIG_ADD_FSP_BINARIES) += $(CONFIG_FSP_S_CBFS) -$(CONFIG_FSP_S_CBFS)-file := $(call strip_quotes,$(CONFIG_FSP_S_FILE)) -$(CONFIG_FSP_S_CBFS)-type := fsp +cbfs-files-$(CONFIG_ADD_FSP_BINARIES) += $(FSP_S_CBFS) +$(FSP_S_CBFS)-file := $(call strip_quotes,$(CONFIG_FSP_S_FILE)) +$(FSP_S_CBFS)-type := fsp

ifeq ($(CONFIG_FSP_USE_REPO),y) $(obj)/Fsp_M.fd: $(call strip_quotes,$(CONFIG_FSP_FD_PATH)) diff --git a/src/soc/intel/apollolake/Makefile.inc b/src/soc/intel/apollolake/Makefile.inc index 7655d5a..42cbab0 100644 --- a/src/soc/intel/apollolake/Makefile.inc +++ b/src/soc/intel/apollolake/Makefile.inc @@ -108,7 +108,7 @@ CPPFLAGS_common += -I$(src)/soc/intel/apollolake/include

# Since FSP-M runs in CAR we need to relocate it to a specific address -$(CONFIG_FSP_M_CBFS)-options := -b $(CONFIG_FSP_M_ADDR) +$(FSP_M_CBFS)-options := -b $(CONFIG_FSP_M_ADDR)

# Handle GLK paging requirements ifeq ($(CONFIG_PAGING_IN_CACHE_AS_RAM),y) diff --git a/src/soc/intel/denverton_ns/Makefile.inc b/src/soc/intel/denverton_ns/Makefile.inc index 10bb665..51ae136 100644 --- a/src/soc/intel/denverton_ns/Makefile.inc +++ b/src/soc/intel/denverton_ns/Makefile.inc @@ -90,8 +90,8 @@

##Set FSP binary blobs memory location

-$(CONFIG_FSP_T_CBFS)-options := -b $(CONFIG_FSP_T_ADDR) --xip -$(CONFIG_FSP_M_CBFS)-options := -b $(CONFIG_FSP_M_ADDR) --xip -$(CONFIG_FSP_S_CBFS)-options := -b $(CONFIG_FSP_S_ADDR) --xip +$(FSP_T_CBFS)-options := -b $(CONFIG_FSP_T_ADDR) --xip +$(FSP_M_CBFS)-options := -b $(CONFIG_FSP_M_ADDR) --xip +$(FSP_S_CBFS)-options := -b $(CONFIG_FSP_S_ADDR) --xip

endif ## CONFIG_SOC_INTEL_DENVERTON_NS diff --git a/src/soc/intel/quark/Makefile.inc b/src/soc/intel/quark/Makefile.inc index 3a58cc9..f2413c8 100644 --- a/src/soc/intel/quark/Makefile.inc +++ b/src/soc/intel/quark/Makefile.inc @@ -71,7 +71,7 @@ CPPFLAGS_common += -I3rdparty/blobs/soc/intel/quark

# Since FSP-M runs in CAR we need to relocate it to a specific address -$(CONFIG_FSP_M_CBFS)-options := -b $(CONFIG_FSP_ESRAM_LOC) +$(FSP_M_CBFS)-options := -b $(CONFIG_FSP_ESRAM_LOC)

# Add the FSP binary to the CBFS image cbfs-files-$(CONFIG_ADD_FSP_RAW_BIN) += fsp.bin

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 5 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Frans Hendriks fhendriks@eltan.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanny E vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

Werner Zeh (Code Review)

6 Nov 6 Nov

8:12 a.m.

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

Patch Set 5:

This patch breaks our Apollo Lake based mainboards as FSP-M never returns once it is called. If I have a closer look into the build log, I can see that the -b option for FSP-M is missing on the cbfs-add line:

CBFS fspm.bin build/util/cbfstool/cbfstool build/coreboot.pre.tmp add -f fsp_m.fd -n fspm.bin -t fsp -r COREBOOT If I revert this patch, everything boots fine again. The cbfs-add line looks as expexted in this case:

CBFS fspm.bin build/util/cbfstool/cbfstool build/coreboot.pre.tmp add -f fsp_m.fd -n "fspm.bin" -t fsp -r COREBOOT -b 0xfef40000

Wim, which platform have you tested this patch on? Do I have to change anything in my .config or how is your build able to survive this?

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 5 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Frans Hendriks fhendriks@eltan.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanessa Eusebio vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Werner Zeh werner.zeh@siemens.com Gerrit-Comment-Date: Wed, 06 Nov 2019 08:12:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Mario Scheithauer (Code Review)

10:25 a.m.

Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

Patch Set 5:

I made a patch for it - https://review.coreboot.org/c/coreboot/+/36645

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 5 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Frans Hendriks fhendriks@eltan.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanessa Eusebio vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Mario Scheithauer mario.scheithauer@siemens.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Werner Zeh werner.zeh@siemens.com Gerrit-Comment-Date: Wed, 06 Nov 2019 10:25:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Mario Scheithauer (Code Review)

10:28 a.m.

Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36548 )

Change subject: drivers/intel/fsp2_0: Use strip_quotes for cbfs filenames ......................................................................

Patch Set 5:

Thanks to Nico for the quick help.

-- To view, visit https://review.coreboot.org/c/coreboot/+/36548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I14267502cfab5308d3874a0c0fd18a71b08bb9f8 Gerrit-Change-Number: 36548 Gerrit-PatchSet: 5 Gerrit-Owner: Wim Vervoorn Gerrit-Reviewer: David Guckian david.guckian@intel.com Gerrit-Reviewer: Frans Hendriks fhendriks@eltan.com Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Vanessa Eusebio vanessa.f.eusebio@intel.com Gerrit-Reviewer: Wim Vervoorn Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Mario Scheithauer mario.scheithauer@siemens.com Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: Werner Zeh werner.zeh@siemens.com Gerrit-Comment-Date: Wed, 06 Nov 2019 10:28:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1790

days inactive

1795

days old

coreboot-gerrit@coreboot.org

12 comments

6 participants

tags (0)

participants (6)

Frans Hendriks (Code Review)
Mario Scheithauer (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)
Werner Zeh (Code Review)
Wim Vervoorn (Code Review)