Change in ...coreboot[master]: Makefile.inc: Update fsp submodule if CONFIG_USE_BLOBS

[Matt DeVillier (Code Review)]

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32526

Change subject: Makefile.inc: Update fsp submodule if CONFIG_USE_BLOBS
......................................................................

Makefile.inc: Update fsp submodule if CONFIG_USE_BLOBS

Rather than selectively update the fsp submodule based on
platform selection, update it if CONFIG_USE_BLOBS is selected
so all platforms using fsp repo have latest version available.

Change-Id: If07d55828a1863623e04a4ecdd1514c3cb6d9c11
Signed-off-by: Matt DeVillier 
---
M Makefile.inc
1 file changed, 1 insertion(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/32526/1

diff --git a/Makefile.inc b/Makefile.inc
index 36c05db..ebc8ee5 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -193,18 +193,11 @@
 # try to fetch non-optional submodules if the source is under git
 forgetthis:=$(if $(GIT),$(shell git submodule update --init))
 ifeq ($(CONFIG_USE_BLOBS),y)
-# this is necessary because 3rdparty/blobs is update=none, and so is ignored
+# this is necessary because 3rdparty/blobs,fsp are update=none, and so are ignored
 # unless explicitly requested and enabled through --checkout
 forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/blobs))
-ifeq ($(CONFIG_PLATFORM_USES_FSP2_0),y)
 forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/fsp))
 endif
-ifeq ($(CONFIG_PLATFORM_USES_FSP1_0),y)
-ifeq ($(CONFIG_SOC_INTEL_FSP_BROADWELL_DE),y)
-forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/fsp))
-endif
-endif
-endif
 UPDATED_SUBMODULES:=1
 COREBOOT_EXPORTS += UPDATED_SUBMODULES
 endif

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32526 )

Change subject: Makefile.inc: Update fsp submodule if CONFIG_USE_BLOBS
......................................................................

Patch Set 1: Code-Review+2

[Nico Huber (Code Review)]

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

Change subject: Makefile.inc: Update fsp submodule if CONFIG_USE_BLOBS
......................................................................

Patch Set 1:

Patrick, any feelings about this? IIRC, you added the
first `ifeq (...FSP...)` here. If preferred we can also
only check it out when any FSP board is selected. Then
we could add a Kconfig for that (probably better than
yet another `ifeq`).

[Nico Huber (Code Review)]

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

Change subject: Makefile.inc: Update fsp submodule if CONFIG_USE_BLOBS
......................................................................

Patch Set 1:

(1 comment)

I've just checked the FSP repo is already 75MiB of burden,
and unpredictable. So maybe the original approach was
better. But instead of adding individual platforms, we
could only check for any FSP platform?

https://review.coreboot.org/#/c/32526/1/Makefile.inc 
File Makefile.inc:

https://review.coreboot.org/#/c/32526/1/Makefile.inc@199 
PS1, Line 199: forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/fsp))
Alternatively, how about condensing it to this:
    ifeq ($(CONFIG_PLATFORM_USES_FSP1_0)$(CONFIG_PLATFORM_USES_FSP1_1)$(CONFIG_PLATFORM_USES_FSP2_0),y)
    forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/fsp))
    endif

Change in ...coreboot[master]: Makefile.inc: Update fsp submodule for all FSP platforms

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins), Nico Huber, Martin Roth, Patrick Georgi,

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

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

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

Change subject: Makefile.inc: Update fsp submodule for all FSP platforms
......................................................................

Makefile.inc: Update fsp submodule for all FSP platforms

Rather than selectively update the fsp submodule based on
FSP version or platform selection, update it when building
for any FSP-enabled platform, so all have latest version available.

Change-Id: If07d55828a1863623e04a4ecdd1514c3cb6d9c11
Signed-off-by: Matt DeVillier 
---
M Makefile.inc
1 file changed, 3 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/32526/2

Change in ...coreboot[master]: Makefile.inc: Update fsp submodule for all FSP platforms

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32526 )

Change subject: Makefile.inc: Update fsp submodule for all FSP platforms
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32526/1/Makefile.inc 
File Makefile.inc:

https://review.coreboot.org/#/c/32526/1/Makefile.inc@199 
PS1, Line 199: forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/fsp))
> Alternatively, how about condensing it to this: […]

Done

Change in ...coreboot[master]: Makefile.inc: Update fsp submodule for all FSP platforms

[Nico Huber (Code Review)]

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

Change subject: Makefile.inc: Update fsp submodule for all FSP platforms
......................................................................

Patch Set 2: Code-Review+2

Change in ...coreboot[master]: Makefile.inc: Update fsp submodule for all FSP platforms

[Nico Huber (Code Review)]

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

Change subject: Makefile.inc: Update fsp submodule for all FSP platforms
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32526/1/Makefile.inc 
File Makefile.inc:

https://review.coreboot.org/#/c/32526/1/Makefile.inc@199 
PS1, Line 199: forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/fsp))
> Done

Ack

Change in ...coreboot[master]: Makefile.inc: Update fsp submodule for all FSP platforms

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32526 )

Change subject: Makefile.inc: Update fsp submodule for all FSP platforms
......................................................................

Makefile.inc: Update fsp submodule for all FSP platforms

Rather than selectively update the fsp submodule based on
FSP version or platform selection, update it when building
for any FSP-enabled platform, so all have latest version available.

Change-Id: If07d55828a1863623e04a4ecdd1514c3cb6d9c11
Signed-off-by: Matt DeVillier 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32526
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
M Makefile.inc
1 file changed, 3 insertions(+), 6 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/Makefile.inc b/Makefile.inc
index fc04a16..a9aaaed 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -196,14 +196,11 @@
 # this is necessary because 3rdparty/blobs is update=none, and so is ignored
 # unless explicitly requested and enabled through --checkout
 forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/blobs))
-ifeq ($(CONFIG_PLATFORM_USES_FSP2_0),y)
+ifeq ($(CONFIG_PLATFORM_USES_FSP1_0)$(CONFIG_PLATFORM_USES_FSP1_1)$(CONFIG_PLATFORM_USES_FSP2_0),y)
+# this is necessary because 3rdparty/fsp is update=none, and so is ignored
+# unless explicitly requested and enabled through --checkout
 forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/fsp))
 endif
-ifeq ($(CONFIG_PLATFORM_USES_FSP1_0),y)
-ifeq ($(CONFIG_SOC_INTEL_FSP_BROADWELL_DE),y)
-forgetthis:=$(if $(GIT),$(shell git submodule update --init --checkout 3rdparty/fsp))
-endif
-endif
 endif
 UPDATED_SUBMODULES:=1
 COREBOOT_EXPORTS += UPDATED_SUBMODULES