From patchwork Thu Jan 10 13:07:50 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Colin Ian King X-Patchwork-Id: 211000 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 472892C0311 for ; Fri, 11 Jan 2013 00:07:56 +1100 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1TtHrO-0003R7-5m; Thu, 10 Jan 2013 13:07:54 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1TtHrL-0003QT-I7 for fwts-devel@lists.ubuntu.com; Thu, 10 Jan 2013 13:07:51 +0000 Received: from cpc3-craw6-2-0-cust180.croy.cable.virginmedia.com ([77.100.248.181] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1TtHrL-0008UO-B2 for fwts-devel@lists.ubuntu.com; Thu, 10 Jan 2013 13:07:51 +0000 From: Colin King To: fwts-devel@lists.ubuntu.com Subject: [PATCH] acpi: method: add helper to check for package contents Date: Thu, 10 Jan 2013 13:07:50 +0000 Message-Id: <1357823270-24887-1-git-send-email-colin.king@canonical.com> X-Mailer: git-send-email 1.8.0 X-BeenThere: fwts-devel@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Firmware Test Suite Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: fwts-devel-bounces@lists.ubuntu.com Errors-To: fwts-devel-bounces@lists.ubuntu.com From: Colin Ian King Add method_package_elements_all_type() that checks to see if the package contains elements that are all of a specified type. This allows us to remove a lot of duplicated code. Signed-off-by: Colin Ian King Acked-by: Ivan Hu --- src/acpi/method/method.c | 237 ++++++++++++++++++++--------------------------- 1 file changed, 103 insertions(+), 134 deletions(-) diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c index 21c89c0..5b1cfdf 100644 --- a/src/acpi/method/method.c +++ b/src/acpi/method/method.c @@ -746,6 +746,64 @@ static void method_test_polling_return( } } +/* + * Common types that can be returned. This is not a complete + * list but it does cover the types we expect to return from + * an ACPI evaluation. + */ +static const char *method_type_name(const ACPI_OBJECT_TYPE type) +{ + switch (type) { + case ACPI_TYPE_INTEGER: + return "integer"; + case ACPI_TYPE_STRING: + return "string"; + case ACPI_TYPE_BUFFER: + return "buffer"; + case ACPI_TYPE_PACKAGE: + return "package"; + case ACPI_TYPE_BUFFER_FIELD: + return "buffer_field"; + case ACPI_TYPE_LOCAL_REFERENCE: + return "reference"; + default: + return "unknown"; + } +} + +/* + * method_package_elements_all_type() + * sanity check fields in a package that all have + * the same type + */ +static int method_package_elements_all_type( + fwts_framework *fw, + const char *name, + const char *objname, + const ACPI_OBJECT *obj, + const ACPI_OBJECT_TYPE type) +{ + uint32_t i; + bool failed = false; + char tmp[128]; + + for (i = 0; i < obj->Package.Count; i++) { + if (obj->Package.Elements[i].Type != type) { + snprintf(tmp, sizeof(tmp), "Method%sElementType", objname); + fwts_failed(fw, LOG_LEVEL_MEDIUM, tmp, + "%s package element %" PRIu32 " was not the expected " + "type '%s', was instead type '%s'.", + name, i, + method_type_name(type), + method_type_name(obj->Package.Elements[i].Type)); + fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); + failed = true; + } + } + + return failed ? FWTS_ERROR: FWTS_OK; +} + /****************************************************************************/ /* @@ -1855,28 +1913,16 @@ static void method_test_EDL_return( ACPI_OBJECT *obj, void *private) { - uint32_t i; - bool failed = false; - FWTS_UNUSED(private); if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK) return; - /* All elements in the package must be references */ - for (i = 0; i < obj->Package.Count; i++) { - if (obj->Package.Elements[i].Type != ACPI_TYPE_LOCAL_REFERENCE) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_EDLElementType", - "%s package element %" PRIu32 " was not a reference.", - name, i); - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); - failed = true; - } - } + if (method_package_elements_all_type(fw, name, "_EDL", + obj, ACPI_TYPE_LOCAL_REFERENCE) != FWTS_OK) + return; - if (!failed) - method_passed_sane(fw, name, "package"); + method_passed_sane(fw, name, "package"); } static int method_test_EDL(fwts_framework *fw) @@ -2103,28 +2149,18 @@ static void method_test_power_resources_return( ACPI_OBJECT *obj, void *private) { - uint32_t i; - bool failed = false; + char *objname = (char *)private; FWTS_UNUSED(private); if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK) return; - /* All elements in the package must be references */ - for (i = 0; i < obj->Package.Count; i++) { - if (obj->Package.Elements[i].Type != ACPI_TYPE_LOCAL_REFERENCE) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_PowerResourceElementType", - "%s package element %" PRIu32 " was not a reference.", - name, i); - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); - failed = true; - } - } + if (method_package_elements_all_type(fw, name, objname, + obj, ACPI_TYPE_LOCAL_REFERENCE) != FWTS_OK) + return; - if (!failed) - method_passed_sane(fw, name, "package"); + method_passed_sane(fw, name, "package"); } #define method_test_POWER(name) \ @@ -2422,12 +2458,8 @@ static void method_test_CSD_return( uint32_t j; bool elements_ok = true; - if (obj->Package.Elements[i].Type != ACPI_TYPE_PACKAGE) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_CSDElementType", - "%s package element %" PRIu32 " was not a package.", - name, i); - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); + if (method_package_elements_all_type(fw, name, "_CSD", + obj, ACPI_TYPE_PACKAGE) != FWTS_OK) { failed = true; continue; /* Skip processing sub-package */ } @@ -2694,9 +2726,6 @@ static void method_test_PCT_return( ACPI_OBJECT *obj, void *private) { - uint32_t i; - bool failed = false; - FWTS_UNUSED(private); if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK) @@ -2706,23 +2735,11 @@ static void method_test_PCT_return( if (method_package_count_min(fw, name, "_PCT", obj, 2) != FWTS_OK) return; - for (i = 0; i < obj->Package.Count; i++) { - /* - * Fairly shallow checks here, should probably check - * for a register description in the buffer - */ - if (obj->Package.Elements[i].Type != ACPI_TYPE_BUFFER) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_PCTElementType", - "%s package element %" PRIu32 - " was not a buffer.", name, i); - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); - failed = true; - continue; /* Skip processing sub-package */ - } - } - if (!failed) - method_passed_sane(fw, name, "package"); + if (method_package_elements_all_type(fw, name, "_PCT", + obj, ACPI_TYPE_BUFFER) != FWTS_OK) + return; + + method_passed_sane(fw, name, "package"); } static int method_test_PCT(fwts_framework *fw) @@ -3762,8 +3779,7 @@ static void method_test_BST_return( ACPI_OBJECT *obj, void *private) { - uint32_t i; - int failed = 0; + bool failed = false; FWTS_UNUSED(private); @@ -3773,17 +3789,8 @@ static void method_test_BST_return( if (method_package_count_equal(fw, name, "_BST", obj, 4) != FWTS_OK) return; - for (i = 0; (i < 4) && (i < obj->Package.Count); i++) { - if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_BSTBadType", - "%s package element %" PRIu32 - " is not of type DWORD Integer.", - name, i); - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); - failed++; - } - } + if (method_package_elements_all_type(fw, name, "_BST", obj, ACPI_TYPE_INTEGER) != FWTS_OK) + return; /* Sanity check each field */ /* Battery State */ @@ -3794,7 +3801,7 @@ static void method_test_BST_return( "be 0..7, got 0x%8.8" PRIx64 ".", name, obj->Package.Elements[0].Integer.Value); fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); - failed++; + failed = true; } /* Ensure bits 0 (discharging) and 1 (charging) are not both set, see 10.2.2.6 */ if (((obj->Package.Elements[0].Integer.Value) & 3) == 3) { @@ -3805,7 +3812,7 @@ static void method_test_BST_return( "which is not allowed. Got value 0x%8.8" PRIx64 ".", name, obj->Package.Elements[0].Integer.Value); fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); - failed++; + failed = true; } /* Battery Present Rate - cannot check, pulled from EC */ /* Battery Remaining Capacity - cannot check, pulled from EC */ @@ -3889,9 +3896,6 @@ static void method_test_BMD_return( ACPI_OBJECT *obj, void *private) { - uint32_t i; - int failed = 0; - FWTS_UNUSED(private); if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK) @@ -3900,22 +3904,12 @@ static void method_test_BMD_return( if (method_package_count_equal(fw, name, "_BMD", obj, 5) != FWTS_OK) return; + if (method_package_elements_all_type(fw, name, "_BMD", obj, ACPI_TYPE_INTEGER) != FWTS_OK) + return; + fwts_acpi_object_dump(fw, obj); - for (i= 0; (i < 4) && (i < obj->Package.Count); i++) { - if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_BMDBadType", - "%s package element %" PRIu32 - " is not of type DWORD Integer.", - name, i); - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); - failed++; - } - } - /* TODO: check return values */ - if (!failed) - method_passed_sane(fw, name, "package"); + method_passed_sane(fw, name, "package"); } static int method_test_BMD(fwts_framework *fw) @@ -4031,17 +4025,8 @@ static void method_test_FIF_return( if (method_package_count_equal(fw, name, "_FIF", obj, 4) != FWTS_OK) return; - fwts_acpi_object_dump(fw, obj); - - if ((obj->Package.Elements[0].Type != ACPI_TYPE_INTEGER) || - (obj->Package.Elements[1].Type != ACPI_TYPE_INTEGER) || - (obj->Package.Elements[2].Type != ACPI_TYPE_INTEGER) || - (obj->Package.Elements[3].Type != ACPI_TYPE_INTEGER)) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_FIFBadType", - "%s should return package of 4 " - "integers.", name); - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); + if (method_package_elements_all_type(fw, name, "_FIF", + obj, ACPI_TYPE_INTEGER) != FWTS_OK) { fwts_advice(fw, "%s is not returning the correct " "fan information. It may be worth " @@ -4049,8 +4034,12 @@ static void method_test_FIF_return( "interactive 'fan' test to see if " "this affects the control and " "operation of the fan.", name); - } else - method_passed_sane(fw, name, "package"); + return; + } + + fwts_acpi_object_dump(fw, obj); + + method_passed_sane(fw, name, "package"); } static int method_test_FIF(fwts_framework *fw) @@ -4084,15 +4073,8 @@ static void method_test_FST_return( if (method_package_count_equal(fw, name, "_FST", obj, 3) != FWTS_OK) return; - fwts_acpi_object_dump(fw, obj); - if ((obj->Package.Elements[0].Type != ACPI_TYPE_INTEGER) || - (obj->Package.Elements[1].Type != ACPI_TYPE_INTEGER) || - (obj->Package.Elements[2].Type != ACPI_TYPE_INTEGER)) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_FSTBadType", - "%s should return package of 3 " - "integers.", name); - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); + if (method_package_elements_all_type(fw, name, "_FST", + obj, ACPI_TYPE_INTEGER) != FWTS_OK) { fwts_advice(fw, "%s is not returning the correct " "fan status information. It may be " @@ -4100,8 +4082,12 @@ static void method_test_FST_return( "suite interactive 'fan' test to see " "if this affects the control and " "operation of the fan.", name); - } else - method_passed_sane(fw, name, "package"); + return; + } + + fwts_acpi_object_dump(fw, obj); + + method_passed_sane(fw, name, "package"); } static int method_test_FST(fwts_framework *fw) @@ -4416,16 +4402,8 @@ static void method_test_WAK_return( if (method_package_count_equal(fw, name, "_WAK", obj, 2) != FWTS_OK) return; - if ((obj->Package.Elements[0].Type != ACPI_TYPE_INTEGER) || - (obj->Package.Elements[1].Type != ACPI_TYPE_INTEGER)) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_WAKBadType", - "%s should return package of 2 " - "integers, got %" PRIu32 " instead.", - name, obj->Package.Count); - fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN); + if (method_package_elements_all_type(fw, name, "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK) return; - } method_passed_sane(fw, name, "package"); } @@ -4618,19 +4596,10 @@ static void method_test_BCL_return( if (method_package_count_min(fw, name, "_BCL", obj, 3) != FWTS_OK) return; - fwts_acpi_object_dump(fw, obj); - - for (i = 0; i < obj->Package.Count; i++) { - if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER) - failed++; - } - if (failed) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "Method_BCLNoPackage", - "%s did not return a package of %" PRIu32 - " integers.", name, obj->Package.Count); + if (method_package_elements_all_type(fw, name, "_FIF", obj, ACPI_TYPE_INTEGER) != FWTS_OK) return; - } + + fwts_acpi_object_dump(fw, obj); if (obj->Package.Elements[0].Integer.Value < obj->Package.Elements[1].Integer.Value) {