Change in coreboot[master]: vc/amd/pi: Fix typo

[Patrick Georgi (Code Review)]

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37676 )

Change subject: vc/amd/pi: Fix typo
......................................................................

vc/amd/pi: Fix typo

Change-Id: Ic3d1b9f90c6ed3d85ff209f433de9ab939d760a6
Signed-off-by: Patrick Georgi 
---
M src/vendorcode/amd/pi/00670F00/Makefile.inc
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/37676/1

diff --git a/src/vendorcode/amd/pi/00670F00/Makefile.inc b/src/vendorcode/amd/pi/00670F00/Makefile.inc
index a019f05..c78e15a 100644
--- a/src/vendorcode/amd/pi/00670F00/Makefile.inc
+++ b/src/vendorcode/amd/pi/00670F00/Makefile.inc
@@ -129,7 +129,7 @@
 AGESA_POST_MEM_INPUT_ELF = $(call strip_quotes,$(CONFIG_AGESA_POST_MEMORY_BINARY_PI_FILE))

# If no post-mem file then also skip pre-mem file
-ifeq ($(AGESA_POST_MEM_INPUT_ELF,))
+ifeq ($(AGESA_POST_MEM_INPUT_ELF),)
 files_added:: warn_no_agesa
 else
 AGESA_POST_MEM_ELF = $(objcbfs)/$(patsubst %.elf,%.debug,$(notdir $(AGESA_POST_MEM_INPUT_ELF)))

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37676 )

Change subject: vc/amd/pi: Fix typo
......................................................................

Patch Set 1: Code-Review+2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37676 )

Change subject: vc/amd/pi: Fix typo
......................................................................

Patch Set 1:

Somewhat related, if google/kahlee ships with AGESA_SPLIT_MEMORY_FILES=y, who would need AGESA_SPLIT_MEMORY_FILES=n? Is it that a different set of blobs are under 3rdparty/ or what's going on here?

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37676 )

Change subject: vc/amd/pi: Fix typo
......................................................................

Patch Set 1: Code-Review+2
> Somewhat related, if google/kahlee ships with AGESA_SPLIT_MEMORY_FILES=y, who would need AGESA_SPLIT_MEMORY_FILES=n? Is it that a different set of blobs are under 3rdparty/ or what's going on here?

I doubt anyone would _need_ a non-split image.  There's only one blob that's under 3rdparty currently, and that's a non-split one.  It seems like amd/padmelon w/Prarie Falcon has landed, or is close, and will use the single image.  (BTW, I'm not sure who's driving that but it's not something I'm working on besides a few reviews.  So I don't know much more than you do.)

Splitting into two blobs was work that Google did.  I've wanted to push images to coreboot.org but I need to double-check the legality of it first.  For example, maybe that's now considered a derivative work and AMD doesn't have rights to redistribute it -- I'm just not certain without digging...

[Patrick Georgi (Code Review)]

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

Change subject: vc/amd/pi: Fix typo
......................................................................

Patch Set 1:

Quite obvious issue with as-obvious fix, original author of the line +2'd, fixes a build bug: not waiting 24 hours.

[Patrick Georgi (Code Review)]

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

Change subject: vc/amd/pi: Fix typo
......................................................................

vc/amd/pi: Fix typo

Change-Id: Ic3d1b9f90c6ed3d85ff209f433de9ab939d760a6
Signed-off-by: Patrick Georgi 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37676
Reviewed-by: Kyösti Mälkki 
Reviewed-by: Marshall Dawson 
Tested-by: build bot (Jenkins) 
---
M src/vendorcode/amd/pi/00670F00/Makefile.inc
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, approved
  Marshall Dawson: Looks good to me, approved

diff --git a/src/vendorcode/amd/pi/00670F00/Makefile.inc b/src/vendorcode/amd/pi/00670F00/Makefile.inc
index a019f05..c78e15a 100644
--- a/src/vendorcode/amd/pi/00670F00/Makefile.inc
+++ b/src/vendorcode/amd/pi/00670F00/Makefile.inc
@@ -129,7 +129,7 @@
 AGESA_POST_MEM_INPUT_ELF = $(call strip_quotes,$(CONFIG_AGESA_POST_MEMORY_BINARY_PI_FILE))

# If no post-mem file then also skip pre-mem file
-ifeq ($(AGESA_POST_MEM_INPUT_ELF,))
+ifeq ($(AGESA_POST_MEM_INPUT_ELF),)
 files_added:: warn_no_agesa
 else
 AGESA_POST_MEM_ELF = $(objcbfs)/$(patsubst %.elf,%.debug,$(notdir $(AGESA_POST_MEM_INPUT_ELF)))

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37676 )

Change subject: vc/amd/pi: Fix typo
......................................................................

Patch Set 1:
> Patch Set 1: Code-Review+2
> > Somewhat related, if google/kahlee ships with AGESA_SPLIT_MEMORY_FILES=y, who would need AGESA_SPLIT_MEMORY_FILES=n? Is it that a different set of blobs are under 3rdparty/ or what's going on here?
>
> I doubt anyone would _need_ a non-split image.  There's only one blob that's under 3rdparty currently, and that's a non-split one.  It seems like amd/padmelon w/Prarie Falcon has landed, or is close, and will use the single image.  (BTW, I'm not sure who's driving that but it's not something I'm working on besides a few reviews.  So I don't know much more than you do.)

Well at the moment I consider You Marshall to be the 'coreboot man inside AMD'. If you are not driving to get things improved about these blob formats and licenses and repositories then who is.

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37676 )

Change subject: vc/amd/pi: Fix typo
......................................................................

Patch Set 2:
> Patch Set 1:
> > Patch Set 1: Code-Review+2
> > > Somewhat related, if google/kahlee ships with AGESA_SPLIT_MEMORY_FILES=y, who would need AGESA_SPLIT_MEMORY_FILES=n? Is it that a different set of blobs are under 3rdparty/ or what's going on here?
> >
> > I doubt anyone would _need_ a non-split image.  There's only one blob that's under 3rdparty currently, and that's a non-split one.  It seems like amd/padmelon w/Prarie Falcon has landed, or is close, and will use the single image.  (BTW, I'm not sure who's driving that but it's not something I'm working on besides a few reviews.  So I don't know much more than you do.)
>
> Well at the moment I consider You Marshall to be the 'coreboot man inside AMD'. If you are not driving to get things improved about these blob formats and licenses and repositories then who is.

Specifically, I meant I don't know who is pushing Embedded's Padmelon board and Prairie Falcon.  SilverBack had been interested in working on it, and I've asked Richard what's going on, yet I can't say whether the Embedded group was requesting it or not.  As you might imagine, AMD has multiple organizations and business units, and Embedded is pretty far removed from my group.

I appreciate that I'm perceived as the coreboot man inside AMD, but I don't plan to be able to change the whole company overnight; that will require time, staffing, and a little persuasion.  In the meantime, I'm your best option for improvements.  Regarding refreshing the Stoney Ridge binaryPI(s), I'm open to suggestions for where you would prioritize that vs. the other things in progress.  AFAIK the only boards using Stoney Ridge are Chromebooks, with Google-built custom blobs, and amd/padmelon with a particular APU.  Also, I think I've mentioned it elsewhere that I'm trying to get AMD to enable more companies to support/deploy binaryPI.