[{"id":1763339,"web_url":"http://patchwork.ozlabs.org/comment/1763339/","msgid":"<4a89dc35-bb1d-8908-3bc8-6de746101aa7@canonical.com>","list_archive_url":null,"date":"2017-09-05T13:02:22","subject":"Re: [PATCH] acpi: method: sbbr: set some methods as optional","submitter":{"id":2900,"url":"http://patchwork.ozlabs.org/api/people/2900/","name":"Colin Ian King","email":"colin.king@canonical.com"},"content":"On 05/09/17 13:57, Sakar Arora wrote:\n> As per SBBR spec, methods _WAK and _PTS are made optional.\n> \n> Method _SST is mandatory. But its absence is now reported\n> as warning. Systems that do not support user visible\n> status such as LED (e.g. ARM FVP), can treat this as just\n> warning. Others must treat _SST method's absence as failure.\n> \n> Signed-off-by: Sakar Arora \n> ---\n> src/acpi/method/method.c | 48 +++++++++---------------------------------------\n> 1 file changed, 9 insertions(+), 39 deletions(-)\n> \n> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c\n> index d082fc1..4000deb 100644\n> --- a/src/acpi/method/method.c\n> +++ b/src/acpi/method/method.c\n> @@ -4449,12 +4449,13 @@ static int method_test_SST(fwts_framework *fw)\n> \targ[0].Type = ACPI_TYPE_INTEGER;\n> \tfor (i = 0; i <= 4; i++) {\n> \t\targ[0].Integer.Value = i;\n> -\t\tif (fw->flags & FWTS_FLAG_TEST_SBBR)\n> -\t\t\tret = method_evaluate_method(fw, METHOD_MANDATORY,\n> -\t\t\t\t\"_SST\", arg, 1, method_test_NULL_return, NULL);\n> -\t\telse\n> -\t\t\tret = method_evaluate_method(fw, METHOD_OPTIONAL,\n> -\t\t\t\t\"_SST\", arg, 1, method_test_NULL_return, NULL);\n> +\t\tret = method_evaluate_method(fw, METHOD_OPTIONAL,\n> +\t\t\t\"_SST\", arg, 1, method_test_NULL_return, NULL);\n> +\n> +\t\tif(ret == FWTS_NOT_EXIST && (fw->flags & FWTS_FLAG_TEST_SBBR)) {\n\nminor style nitpick, can we have a space between if and the following (\n\n> +\t\t\tfwts_warning(fw, \"_SST method not found. This should be treated as error \"\n> +\t\t\t\t\t\"if the platform provides user visible status such as LED.\");\n> +\t\t}\n> \n> \t\tif (ret != FWTS_OK)\n> \t\t\tbreak;\n> @@ -6483,23 +6484,7 @@ static int method_test_PTS(fwts_framework *fw)\n> \t\targ[0].Integer.Value = i;\n> \n> \t\tfwts_log_info(fw, \"Test _PTS(%d).\", i);\n> -\t\tif (!(fw->flags & FWTS_FLAG_TEST_SBBR))\n> -\t\t\tmethod_evaluate_method(fw, METHOD_OPTIONAL, \"_PTS\", arg, 1, method_test_NULL_return, NULL);\n> -\t\telse\n> -\t\t\tif (method_evaluate_method(fw, METHOD_MANDATORY, \"_PTS\", arg, 1,\n> -\t\t\t\tmethod_test_NULL_return, NULL) == FWTS_NOT_EXIST) {\n> -\t\t\t\tfwts_advice(fw,\n> -\t\t\t\t\t\"Could not find _PTS. This method provides a \"\n> -\t\t\t\t\t\"mechanism to do housekeeping functions, such \"\n> -\t\t\t\t\t\"as write sleep state to the embedded \"\n> -\t\t\t\t\t\"controller before entering a sleep state. If \"\n> -\t\t\t\t\t\"the machine cannot suspend (S3), \"\n> -\t\t\t\t\t\"hibernate (S4) or shutdown (S5) then it \"\n> -\t\t\t\t\t\"could be because _PTS is missing. Note that \"\n> -\t\t\t\t\t\"ACPI 1.0 wants _PTS to be executed before \"\n> -\t\t\t\t\t\"suspending devices.\");\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> +\t\tmethod_evaluate_method(fw, METHOD_OPTIONAL, \"_PTS\", arg, 1, method_test_NULL_return, NULL);\n> \t\tfwts_log_nl(fw);\n> \t}\n> \treturn FWTS_OK;\n> @@ -6577,22 +6562,7 @@ static int method_test_WAK(fwts_framework *fw)\n> \t\targ[0].Type = ACPI_TYPE_INTEGER;\n> \t\targ[0].Integer.Value = i;\n> \t\tfwts_log_info(fw, \"Test _WAK(%d) System Wake, State S%d.\", i, i);\n> -\t\tif (!(fw->flags & FWTS_FLAG_TEST_SBBR))\n> -\t\t\tmethod_evaluate_method(fw, METHOD_OPTIONAL, \"_WAK\", arg, 1, method_test_WAK_return, &i);\n> -\t\telse\n> -\t\t\tif (method_evaluate_method(fw, METHOD_MANDATORY, \"_WAK\", arg, 1,\n> -\t\t\t\tmethod_test_WAK_return, &i) == FWTS_NOT_EXIST) {\n> -\t\t\t\tfwts_advice(fw,\n> -\t\t\t\t\t\"Section 7.3.7 states that a system that wakes \"\n> -\t\t\t\t\t\"from a sleeping state will invoke the _WAK \"\n> -\t\t\t\t\t\"control to issue device, thermal and other \"\n> -\t\t\t\t\t\"notifications to ensure that the operating system \"\n> -\t\t\t\t\t\"checks the states of various devices, thermal \"\n> -\t\t\t\t\t\"zones, etc. The Linux kernel will report an \"\n> -\t\t\t\t\t\"ACPI exception if _WAK is does not exist when \"\n> -\t\t\t\t\t\"it returns from a sleep state.\");\n> -\t\t\t\tbreak;\n> -\t\t\t}\n> +\t\tmethod_evaluate_method(fw, METHOD_OPTIONAL, \"_WAK\", arg, 1, method_test_WAK_return, &i);\n> \t\tfwts_log_nl(fw);\n> \t}\n> \treturn FWTS_OK;\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.ubuntu.com\n\t(client-ip=91.189.94.19; helo=huckleberry.canonical.com;\n\tenvelope-from=fwts-devel-bounces@lists.ubuntu.com;\n\treceiver=)","Received":["from huckleberry.canonical.com (huckleberry.canonical.com\n\t[91.189.94.19])\n\tby ozlabs.org (Postfix) with ESMTP id 3xmn1b2b65z9t16;\n\tTue, 5 Sep 2017 23:02:27 +1000 (AEST)","from localhost ([127.0.0.1] helo=huckleberry.canonical.com)\n\tby huckleberry.canonical.com with esmtp (Exim 4.86_2)\n\t(envelope-from )\n\tid 1dpDUo-0007o4-5B; Tue, 05 Sep 2017 13:02:26 +0000","from youngberry.canonical.com ([91.189.89.112])\n\tby huckleberry.canonical.com with esmtps\n\t(TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128)\n\t(Exim 4.86_2) (envelope-from )\n\tid 1dpDUm-0007nk-5K\n\tfor fwts-devel@lists.ubuntu.com; Tue, 05 Sep 2017 13:02:24 +0000","from 1.general.cking.uk.vpn ([10.172.193.212])\n\tby youngberry.canonical.com with esmtpsa\n\t(TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.76) (envelope-from )\n\tid 1dpDUl-0007tV-Nv; Tue, 05 Sep 2017 13:02:23 +0000"],"Subject":"Re: [PATCH] acpi: method: sbbr: set some methods as optional","To":"Sakar Arora , fwts-devel@lists.ubuntu.com","References":"<1504616268-28745-1-git-send-email-Sakar.Arora@arm.com>","From":"Colin Ian King ","Message-ID":"<4a89dc35-bb1d-8908-3bc8-6de746101aa7@canonical.com>","Date":"Tue, 5 Sep 2017 14:02:22 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.8.0","MIME-Version":"1.0","In-Reply-To":"<1504616268-28745-1-git-send-email-Sakar.Arora@arm.com>","X-BeenThere":"fwts-devel@lists.ubuntu.com","X-Mailman-Version":"2.1.20","Precedence":"list","List-Id":"Firmware Test Suite Development ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Prasanth.Pulla@arm.com, Charles.Garcia-Tobin@arm.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"fwts-devel-bounces@lists.ubuntu.com","Sender":"\"fwts-devel\" "}}]