[PATCH] acpi/method: check _PS3 exists when _PS0 is present

[Paul Menzel]

Dear Alex,


Thank you very much for the patch.

Am 25.04.20 um 05:31 schrieb Alex Hung:
> 7.3 Device Power Management Objects in ACPI spec requires methods for
> setting devices to D0 and D3 to be present at least when _PSx or _PRx
> is supported.
> 
> Reported-by: Paul Menzel <pmenzel at molgen.mpg.de>
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>   src/acpi/method/method.c | 51 +++++++++++++++++++++++-----------------
>   1 file changed, 30 insertions(+), 21 deletions(-)
> 
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 48001d7b..cae9cc03 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -1535,34 +1535,43 @@ static int method_test_PRW(fwts_framework *fw)
>   		"_PRW", NULL, 0, method_test_PRW_return, NULL);
>   }
>   
> -static int method_test_PS0(fwts_framework *fw)
> +static void method_test_PS0_return(
> +	fwts_framework *fw,
> +	char *name,
> +	ACPI_BUFFER *buf,
> +	ACPI_OBJECT *obj,
> +	void *private)
>   {
> -	/*
> -	 *  iASL (ACPICA commit 6922796cfdfca041fdb96dc9e3918cbc7f43d830)
> -	 *  checks that one of _PS1, _PS2, _PS3 must exist if _PS0 exists.
> -	 */
> -	if (fwts_acpi_object_exists("_PS0") != NULL) {
> -		bool ok = false;
> -		int i;
> +	ACPI_HANDLE handle;
> +	ACPI_STATUS status;
> +	char *ps3path;
>   
> -		for (i = 1; i < 4; i++) {
> -			char name[6];
> +	/* Check _PS3 according to ACPI spec:
> +	 * If any ACPI Object that controls power (_PSx or _PRx, where x =0, 1, 2,
> +	 * or 3) exists, then methods to set the device into D0 and D3 device
> +	  * states (at least) must be present.

The alignment in the beginning looks off.

> +	 */
> +	ps3path = (char*) malloc(strlen(name));
> +	if (ps3path) {
> +		strcpy(ps3path, name);
> +		ps3path[strlen(name) -1 ] = '1';
>   
> -			snprintf(name, sizeof(name), "_PS%1d", i);
> -			if (fwts_acpi_object_exists(name) != NULL) {
> -				ok = true;
> -				break;
> -			}
> -		}
> -		if (!ok) {
> +		status = AcpiGetHandle(NULL, ps3path, &handle);
> +		if (status == AE_NOT_FOUND) {
>   			fwts_failed(fw, LOG_LEVEL_HIGH, "Method_PS0",
> -				"_PS0 requires that one of the _PS1, _PS2, _PS3 "
> -				"control methods must also exist, however, "
> -				"none were found.");
> +			"%s requires _PS3 control methods but "
> +			"it was not found.", name);
>   		}
> +		free(ps3path);
>   	}
> +
> +	fwts_method_test_NULL_return(fw, name, buf, obj, private);
> +}
> +
> +static int method_test_PS0(fwts_framework *fw)
> +{
>   	return method_evaluate_method(fw, METHOD_OPTIONAL, "_PS0",
> -		 NULL, 0, fwts_method_test_NULL_return, "_PS0");
> +		NULL, 0, method_test_PS0_return, NULL);
>   }
>   
>   static int method_test_PS1(fwts_framework *fw)

The rest looks good to me.


Kind regards,

Paul