Change in coreboot[master]: src/southbridge/amd/pi/hudson/Makefile.inc: fix AMDFW outside CBFS
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) -- 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-MessageType: newchange
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 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 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 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
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 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
participants (5)
-
Angel Pons (Code Review) -
Felix Held (Code Review)
-
Martin L Roth (Code Review)
-
Michał Żygowski (Code Review)
-
Paul Menzel (Code Review)