Change in coreboot[master]: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

[S] Change in coreboot[master]:...

Michał Żygowski (Code Review)

7 Oct 2019 7 Oct '19

1:47 p.m.

Michał Żygowski has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35853 )

Change subject: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS ......................................................................

src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS

The amdfwtool expects firmware location to be passed in hexadecimal format. When CONFIG_AMDFW_OUTSIDE_CBFS the firmware location is calculated by int-add call which produces decimal output.

Use shell printf to generate a hexadecimal value of HUDSON_FWM_POSITION.

TEST=prepare simple FMD file with AMD_FW region at 0x20000, select CONFIG_AMDFW_OUTSIDE_CBFS and build PC Engines apu2 board

Change-Id: Ib4b03f971b88acbc3392b66e084babe738659ea6 Signed-off-by: Michał Żygowski michal.zygowski@3mdeb.com --- M src/southbridge/amd/pi/hudson/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/35853/1

diff --git a/src/southbridge/amd/pi/hudson/Makefile.inc b/src/southbridge/amd/pi/hudson/Makefile.inc index 4b4b138..9b543ec 100644 --- a/src/southbridge/amd/pi/hudson/Makefile.inc +++ b/src/southbridge/amd/pi/hudson/Makefile.inc @@ -194,7 +194,7 @@ $(OPT_2SMUFIRMWARE2_FN_FILE) \ $(OPT_2SMUSCS_FILE) \ --flashsize $(CONFIG_ROM_SIZE) \ - --location $(HUDSON_FWM_POSITION) \ + --location $(shell printf "0x%x" $(HUDSON_FWM_POSITION)) \ --output $@

ifeq ($(CONFIG_AMDFW_OUTSIDE_CBFS),y)

Attachments:

attachment.htm (text/html — 2.9 KB)

Show replies by date

Felix Held (Code Review)

16 Mar 16 Mar

9:57 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35853 )

Change subject: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS ......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/35853/1/src/southbridge/amd/pi/huds... File src/southbridge/amd/pi/hudson/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/35853/1/src/southbridge/amd/pi/huds... PS1, Line 76: HUDSON_FWM_POSITION=$(call int-add, $(call int-subtract, 0xffffffff $(CONFIG_ROM_SIZE)) 0x20000 1) is it intended to not add the conversion here, but below? i would have added this here

-- To view, visit https://review.coreboot.org/c/coreboot/+/35853 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4b03f971b88acbc3392b66e084babe738659ea6 Gerrit-Change-Number: 35853 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Mon, 16 Mar 2020 21:57:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Michał Żygowski (Code Review)

16 Apr 16 Apr

9:03 a.m.

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35853 )

Change subject: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35853/1/src/southbridge/amd/pi/huds... File src/southbridge/amd/pi/hudson/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/35853/1/src/southbridge/amd/pi/huds... PS1, Line 76: HUDSON_FWM_POSITION=$(call int-add, $(call int-subtract, 0xffffffff $(CONFIG_ROM_SIZE)) 0x20000 1)

...

is it intended to not add the conversion here, but below? i would have added this here

I used the same approach as stoneyridge

-- To view, visit https://review.coreboot.org/c/coreboot/+/35853 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4b03f971b88acbc3392b66e084babe738659ea6 Gerrit-Change-Number: 35853 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 16 Apr 2020 09:03:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held felix-coreboot@felixheld.de Gerrit-MessageType: comment

Felix Held (Code Review)

8:06 p.m.

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35853 )

Change subject: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35853/1/src/southbridge/amd/pi/huds... File src/southbridge/amd/pi/hudson/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/35853/1/src/southbridge/amd/pi/huds... PS1, Line 76: HUDSON_FWM_POSITION=$(call int-add, $(call int-subtract, 0xffffffff $(CONFIG_ROM_SIZE)) 0x20000 1)

...

I used the same approach as stoneyridge

I'd prefer if HUDSON_FWM_POSITION already contains the prefix; this would also make this case consistent with the other one where the 0x prefix is present. Having this inconsistent here and doing the fixup 100 lines later is also not too obvious

-- To view, visit https://review.coreboot.org/c/coreboot/+/35853 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4b03f971b88acbc3392b66e084babe738659ea6 Gerrit-Change-Number: 35853 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Thu, 16 Apr 2020 20:06:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski michal.zygowski@3mdeb.com Comment-In-Reply-To: Felix Held felix-coreboot@felixheld.de Gerrit-MessageType: comment

Angel Pons (Code Review)

24 May 24 May

10:32 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35853 )

Change subject: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS ......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/35853/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/35853/1//COMMIT_MSG@10 PS1, Line 10: calculated nit: I'd put this on the next line

-- To view, visit https://review.coreboot.org/c/coreboot/+/35853 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4b03f971b88acbc3392b66e084babe738659ea6 Gerrit-Change-Number: 35853 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Sun, 24 May 2020 10:32:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Paul Menzel (Code Review)

9 Dec 9 Dec

10:24 a.m.

Attention is currently required from: Michał Żygowski. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35853 )

Change subject: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS ......................................................................

Patch Set 1:

(1 comment)

Commit Message:

https://review.coreboot.org/c/coreboot/+/35853/comment/9d4da503_f1cf609e PS1, Line 7: src/ Remove.

-- To view, visit https://review.coreboot.org/c/coreboot/+/35853 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4b03f971b88acbc3392b66e084babe738659ea6 Gerrit-Change-Number: 35853 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Martin Roth martinroth@google.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-Attention: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Comment-Date: Thu, 09 Dec 2021 10:24:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Martin L Roth (Code Review)

7 Aug 7 Aug

8:40 p.m.

New subject: [XS] Change in coreboot[master]: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35853?usp=email )

Change subject: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS ......................................................................

Abandoned

This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.

-- To view, visit https://review.coreboot.org/c/coreboot/+/35853?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib4b03f971b88acbc3392b66e084babe738659ea6 Gerrit-Change-Number: 35853 Gerrit-PatchSet: 1 Gerrit-Owner: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Felix Held felix-coreboot@felixheld.de Gerrit-Reviewer: Martin L Roth gaumless@gmail.com Gerrit-Reviewer: Michał Żygowski michal.zygowski@3mdeb.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@mailbox.org Gerrit-MessageType: abandon

507

days inactive

1907

days old

coreboot-gerrit@coreboot.org

6 comments

5 participants

tags (0)

participants (5)

Angel Pons (Code Review)
Felix Held (Code Review)
Martin L Roth (Code Review)
Michał Żygowski (Code Review)
Paul Menzel (Code Review)