Hello Matt DeVillier, Patrick Georgi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/39031
to review the following change.
Change subject: cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS ......................................................................
cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS
Make the variable override for CPU_MICROCODE_CBFS_EXTERNAL_BINS local to the target. Otherwise, `cpu_microcode_bin +=` lines that are evaluated after `src/cpu/Makefile.inc` still append to it.
Change-Id: If81f307afc325ff3c1e987e9483ed5e45fdc403e Signed-off-by: Nico Huber nico.h@gmx.de --- M src/cpu/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/39031/1
diff --git a/src/cpu/Makefile.inc b/src/cpu/Makefile.inc index 0289be0..92e47aa 100644 --- a/src/cpu/Makefile.inc +++ b/src/cpu/Makefile.inc @@ -28,7 +28,7 @@ endif
ifeq ($(CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS),y) -cpu_microcode_bins := $(call strip_quotes,$(CONFIG_CPU_UCODE_BINARIES)) +$(obj)/cpu_microcode_blob.bin: cpu_microcode_bins := $(call strip_quotes,$(CONFIG_CPU_UCODE_BINARIES)) endif # otherwise `cpu_microcode_bins` should be filled by platform makefiles
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39031 )
Change subject: cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS ......................................................................
Patch Set 1: Code-Review+1
retesting on my repo, only Sandy/Ivybridge seems affected, and is fixed by this patch
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39031 )
Change subject: cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS ......................................................................
Patch Set 1: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39031 )
Change subject: cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/39031/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39031/1//COMMIT_MSG@11 PS1, Line 11: append append*ed* ?
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39031 )
Change subject: cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39031/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39031/1//COMMIT_MSG@11 PS1, Line 11: append
append*ed* ?
nope, correct as is
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39031 )
Change subject: cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS ......................................................................
cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS
Make the variable override for CPU_MICROCODE_CBFS_EXTERNAL_BINS local to the target. Otherwise, `cpu_microcode_bin +=` lines that are evaluated after `src/cpu/Makefile.inc` still append to it.
Change-Id: If81f307afc325ff3c1e987e9483ed5e45fdc403e Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/39031 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Matt DeVillier matt.devillier@gmail.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/cpu/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Matt DeVillier: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved
diff --git a/src/cpu/Makefile.inc b/src/cpu/Makefile.inc index 0289be0..92e47aa 100644 --- a/src/cpu/Makefile.inc +++ b/src/cpu/Makefile.inc @@ -28,7 +28,7 @@ endif
ifeq ($(CONFIG_CPU_MICROCODE_CBFS_EXTERNAL_BINS),y) -cpu_microcode_bins := $(call strip_quotes,$(CONFIG_CPU_UCODE_BINARIES)) +$(obj)/cpu_microcode_blob.bin: cpu_microcode_bins := $(call strip_quotes,$(CONFIG_CPU_UCODE_BINARIES)) endif # otherwise `cpu_microcode_bins` should be filled by platform makefiles
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39031 )
Change subject: cpu/microcode: Fix config CPU_MICROCODE_CBFS_EXTERNAL_BINS ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39031/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39031/1//COMMIT_MSG@11 PS1, Line 11: append
nope, correct as is
ah, I see:
Otherwise, <adjective> lines that are evaluated after <noun> still append to it.
I would usually say "get appended", but bleh
-
Angel Pons (Code Review) -
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)