Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32469
Change subject: Makefile: Check for errors in ACPI decompilation, too ......................................................................
Makefile: Check for errors in ACPI decompilation, too
We only grep'ed for "ACPI Warning" resulting in an actual more sever "ACPI Error" being ignored.
Change-Id: I9cec8a388f5558b1ffc383cc2fc69405252cbb37 Signed-off-by: Nico Huber nico.huber@secunet.com --- M Makefile.inc 1 file changed, 8 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/32469/1
diff --git a/Makefile.inc b/Makefile.inc index 36c05db..fc04a16 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -267,7 +267,14 @@ @printf " IASL $$(subst $(top)/,,$$(@))\n" $(CC_ramstage) -x assembler-with-cpp -E -MMD -MT $$(@) $$(CPPFLAGS_ramstage) -D__ACPI__ -P -include $(src)/include/kconfig.h -I$(obj) -I$(src) -I$(src)/include -I$(src)/arch/$(ARCHDIR-$(ARCH-ramstage-y))/include -I$(src)/mainboard/$(MAINBOARDDIR) $$< -o $(obj)/$(1).asl cd $$(dir $$@); $(IASL) $(IGNORED_IASL_WARNINGS) -we -p $$(notdir $$@) $(1).asl - if [ -z "$$$$($(IASL) -d $$@ 2>&1 | grep 'ACPI Warning')" ]; then echo " IASL $$@ disassembled correctly."; true; else echo "Error: Could not correctly disassemble $$@"; $(IASL) -d $$@; false; fi + if ! $(IASL) -d $$@ 2>&1 | grep -Eq 'ACPI (Warning|Error)'; then \ + echo " IASL $$@ disassembled correctly."; \ + true; \ + else \ + echo "Error: Could not correctly disassemble $$@"; \ + $(IASL) -d $$@; \ + false; \ + fi endef
#######################################################################
Hello Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32469
to look at the new patch set (#2).
Change subject: Makefile: Check for errors in ACPI decompilation, too ......................................................................
Makefile: Check for errors in ACPI decompilation, too
We only grep'ed for "ACPI Warning" resulting in an actual more severe "ACPI Error" being ignored.
Change-Id: I9cec8a388f5558b1ffc383cc2fc69405252cbb37 Signed-off-by: Nico Huber nico.huber@secunet.com --- M Makefile.inc 1 file changed, 8 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/32469/2
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32469 )
Change subject: Makefile: Check for errors in ACPI decompilation, too ......................................................................
Patch Set 2:
This patch works fine (detects the issue on patchset 3 of https://review.coreboot.org/c/coreboot/+/31822)
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32469 )
Change subject: Makefile: Check for errors in ACPI decompilation, too ......................................................................
Patch Set 3: Code-Review+1
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32469 )
Change subject: Makefile: Check for errors in ACPI decompilation, too ......................................................................
Patch Set 3: Code-Review+1
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32469 )
Change subject: Makefile: Check for errors in ACPI decompilation, too ......................................................................
Patch Set 3: Code-Review+1
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32469 )
Change subject: Makefile: Check for errors in ACPI decompilation, too ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32469 )
Change subject: Makefile: Check for errors in ACPI decompilation, too ......................................................................
Makefile: Check for errors in ACPI decompilation, too
We only grep'ed for "ACPI Warning" resulting in an actual more severe "ACPI Error" being ignored.
Change-Id: I9cec8a388f5558b1ffc383cc2fc69405252cbb37 Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32469 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Frans Hendriks fhendriks@eltan.com Reviewed-by: Patrick Georgi pgeorgi@google.com --- M Makefile.inc 1 file changed, 8 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve HAOUAS Elyes: Looks good to me, but someone else must approve Frans Hendriks: Looks good to me, but someone else must approve
diff --git a/Makefile.inc b/Makefile.inc index 36c05db..fc04a16 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -267,7 +267,14 @@ @printf " IASL $$(subst $(top)/,,$$(@))\n" $(CC_ramstage) -x assembler-with-cpp -E -MMD -MT $$(@) $$(CPPFLAGS_ramstage) -D__ACPI__ -P -include $(src)/include/kconfig.h -I$(obj) -I$(src) -I$(src)/include -I$(src)/arch/$(ARCHDIR-$(ARCH-ramstage-y))/include -I$(src)/mainboard/$(MAINBOARDDIR) $$< -o $(obj)/$(1).asl cd $$(dir $$@); $(IASL) $(IGNORED_IASL_WARNINGS) -we -p $$(notdir $$@) $(1).asl - if [ -z "$$$$($(IASL) -d $$@ 2>&1 | grep 'ACPI Warning')" ]; then echo " IASL $$@ disassembled correctly."; true; else echo "Error: Could not correctly disassemble $$@"; $(IASL) -d $$@; false; fi + if ! $(IASL) -d $$@ 2>&1 | grep -Eq 'ACPI (Warning|Error)'; then \ + echo " IASL $$@ disassembled correctly."; \ + true; \ + else \ + echo "Error: Could not correctly disassemble $$@"; \ + $(IASL) -d $$@; \ + false; \ + fi endef
#######################################################################
-
Frans Hendriks (Code Review) -
HAOUAS Elyes (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)