diff mbox series

acpi: method: sbbr: set some methods as optional

Message ID 1504616268-28745-1-git-send-email-Sakar.Arora@arm.com
State Superseded
Headers show
Series acpi: method: sbbr: set some methods as optional | expand

Commit Message

Sakar Arora Sept. 5, 2017, 12:57 p.m. UTC
As per SBBR spec, methods _WAK and _PTS are made optional.

Method _SST is mandatory. But its absence is now reported
as warning. Systems that do not support user visible
status such as LED (e.g. ARM FVP), can treat this as just
warning. Others must treat _SST method's absence as failure.

Signed-off-by: Sakar Arora <Sakar.Arora@arm.com>
---
 src/acpi/method/method.c | 48 +++++++++---------------------------------------
 1 file changed, 9 insertions(+), 39 deletions(-)

Comments

Colin Ian King Sept. 5, 2017, 1:02 p.m. UTC | #1
On 05/09/17 13:57, Sakar Arora wrote:
> As per SBBR spec, methods _WAK and _PTS are made optional.
> 
> Method _SST is mandatory. But its absence is now reported
> as warning. Systems that do not support user visible
> status such as LED (e.g. ARM FVP), can treat this as just
> warning. Others must treat _SST method's absence as failure.
> 
> Signed-off-by: Sakar Arora <Sakar.Arora@arm.com>
> ---
>  src/acpi/method/method.c | 48 +++++++++---------------------------------------
>  1 file changed, 9 insertions(+), 39 deletions(-)
> 
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index d082fc1..4000deb 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -4449,12 +4449,13 @@ static int method_test_SST(fwts_framework *fw)
>  	arg[0].Type = ACPI_TYPE_INTEGER;
>  	for (i = 0; i <= 4; i++) {
>  		arg[0].Integer.Value = i;
> -		if (fw->flags & FWTS_FLAG_TEST_SBBR)
> -			ret = method_evaluate_method(fw, METHOD_MANDATORY,
> -				"_SST", arg, 1, method_test_NULL_return, NULL);
> -		else
> -			ret = method_evaluate_method(fw, METHOD_OPTIONAL,
> -				"_SST", arg, 1, method_test_NULL_return, NULL);
> +		ret = method_evaluate_method(fw, METHOD_OPTIONAL,
> +			"_SST", arg, 1, method_test_NULL_return, NULL);
> +
> +		if(ret == FWTS_NOT_EXIST && (fw->flags & FWTS_FLAG_TEST_SBBR)) {

minor style nitpick, can we have a space between if and the following (

> +			fwts_warning(fw, "_SST method not found. This should be treated as error "
> +					"if the platform provides user visible status such as LED.");
> +		}
>  
>  		if (ret != FWTS_OK)
>  			break;
> @@ -6483,23 +6484,7 @@ static int method_test_PTS(fwts_framework *fw)
>  		arg[0].Integer.Value = i;
>  
>  		fwts_log_info(fw, "Test _PTS(%d).", i);
> -		if (!(fw->flags & FWTS_FLAG_TEST_SBBR))
> -			method_evaluate_method(fw, METHOD_OPTIONAL, "_PTS", arg, 1, method_test_NULL_return, NULL);
> -		else
> -			if (method_evaluate_method(fw, METHOD_MANDATORY, "_PTS", arg, 1,
> -				method_test_NULL_return, NULL) == FWTS_NOT_EXIST) {
> -				fwts_advice(fw,
> -					"Could not find _PTS. This method provides a "
> -					"mechanism to do housekeeping functions, such "
> -					"as write sleep state to the embedded "
> -					"controller before entering a sleep state. If "
> -					"the machine cannot suspend (S3), "
> -					"hibernate (S4) or shutdown (S5) then it "
> -					"could be because _PTS is missing.  Note that "
> -					"ACPI 1.0 wants _PTS to be executed before "
> -					"suspending devices.");
> -				break;
> -			}
> +		method_evaluate_method(fw, METHOD_OPTIONAL, "_PTS", arg, 1, method_test_NULL_return, NULL);
>  		fwts_log_nl(fw);
>  	}
>  	return FWTS_OK;
> @@ -6577,22 +6562,7 @@ static int method_test_WAK(fwts_framework *fw)
>  		arg[0].Type = ACPI_TYPE_INTEGER;
>  		arg[0].Integer.Value = i;
>  		fwts_log_info(fw, "Test _WAK(%d) System Wake, State S%d.", i, i);
> -		if (!(fw->flags & FWTS_FLAG_TEST_SBBR))
> -			method_evaluate_method(fw, METHOD_OPTIONAL, "_WAK", arg, 1, method_test_WAK_return, &i);
> -		else
> -			if (method_evaluate_method(fw, METHOD_MANDATORY, "_WAK", arg, 1,
> -				method_test_WAK_return, &i) == FWTS_NOT_EXIST) {
> -				fwts_advice(fw,
> -					"Section 7.3.7 states that a system that wakes "
> -					"from a sleeping state will invoke the _WAK "
> -					"control to issue device, thermal and other "
> -					"notifications to ensure that the operating system "
> -					"checks the states of various devices, thermal "
> -					"zones, etc.  The Linux kernel will report an "
> -					"ACPI exception if _WAK is does not exist when "
> -					"it returns from a sleep state.");
> -				break;
> -			}
> +		method_evaluate_method(fw, METHOD_OPTIONAL, "_WAK", arg, 1, method_test_WAK_return, &i);
>  		fwts_log_nl(fw);
>  	}
>  	return FWTS_OK;
>
diff mbox series

Patch

diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
index d082fc1..4000deb 100644
--- a/src/acpi/method/method.c
+++ b/src/acpi/method/method.c
@@ -4449,12 +4449,13 @@  static int method_test_SST(fwts_framework *fw)
 	arg[0].Type = ACPI_TYPE_INTEGER;
 	for (i = 0; i <= 4; i++) {
 		arg[0].Integer.Value = i;
-		if (fw->flags & FWTS_FLAG_TEST_SBBR)
-			ret = method_evaluate_method(fw, METHOD_MANDATORY,
-				"_SST", arg, 1, method_test_NULL_return, NULL);
-		else
-			ret = method_evaluate_method(fw, METHOD_OPTIONAL,
-				"_SST", arg, 1, method_test_NULL_return, NULL);
+		ret = method_evaluate_method(fw, METHOD_OPTIONAL,
+			"_SST", arg, 1, method_test_NULL_return, NULL);
+
+		if(ret == FWTS_NOT_EXIST && (fw->flags & FWTS_FLAG_TEST_SBBR)) {
+			fwts_warning(fw, "_SST method not found. This should be treated as error "
+					"if the platform provides user visible status such as LED.");
+		}
 
 		if (ret != FWTS_OK)
 			break;
@@ -6483,23 +6484,7 @@  static int method_test_PTS(fwts_framework *fw)
 		arg[0].Integer.Value = i;
 
 		fwts_log_info(fw, "Test _PTS(%d).", i);
-		if (!(fw->flags & FWTS_FLAG_TEST_SBBR))
-			method_evaluate_method(fw, METHOD_OPTIONAL, "_PTS", arg, 1, method_test_NULL_return, NULL);
-		else
-			if (method_evaluate_method(fw, METHOD_MANDATORY, "_PTS", arg, 1,
-				method_test_NULL_return, NULL) == FWTS_NOT_EXIST) {
-				fwts_advice(fw,
-					"Could not find _PTS. This method provides a "
-					"mechanism to do housekeeping functions, such "
-					"as write sleep state to the embedded "
-					"controller before entering a sleep state. If "
-					"the machine cannot suspend (S3), "
-					"hibernate (S4) or shutdown (S5) then it "
-					"could be because _PTS is missing.  Note that "
-					"ACPI 1.0 wants _PTS to be executed before "
-					"suspending devices.");
-				break;
-			}
+		method_evaluate_method(fw, METHOD_OPTIONAL, "_PTS", arg, 1, method_test_NULL_return, NULL);
 		fwts_log_nl(fw);
 	}
 	return FWTS_OK;
@@ -6577,22 +6562,7 @@  static int method_test_WAK(fwts_framework *fw)
 		arg[0].Type = ACPI_TYPE_INTEGER;
 		arg[0].Integer.Value = i;
 		fwts_log_info(fw, "Test _WAK(%d) System Wake, State S%d.", i, i);
-		if (!(fw->flags & FWTS_FLAG_TEST_SBBR))
-			method_evaluate_method(fw, METHOD_OPTIONAL, "_WAK", arg, 1, method_test_WAK_return, &i);
-		else
-			if (method_evaluate_method(fw, METHOD_MANDATORY, "_WAK", arg, 1,
-				method_test_WAK_return, &i) == FWTS_NOT_EXIST) {
-				fwts_advice(fw,
-					"Section 7.3.7 states that a system that wakes "
-					"from a sleeping state will invoke the _WAK "
-					"control to issue device, thermal and other "
-					"notifications to ensure that the operating system "
-					"checks the states of various devices, thermal "
-					"zones, etc.  The Linux kernel will report an "
-					"ACPI exception if _WAK is does not exist when "
-					"it returns from a sleep state.");
-				break;
-			}
+		method_evaluate_method(fw, METHOD_OPTIONAL, "_WAK", arg, 1, method_test_WAK_return, &i);
 		fwts_log_nl(fw);
 	}
 	return FWTS_OK;