[PATCH][RFC] acpi: method: refine _EVT test

[Colin Ian King]

On 31/07/15 14:44, Alex Hung wrote:
> According to ACPI spec, _EVT's events should be in adjacent _AEI
> control method.  This patch also reduces lengthy testing time.
> 
> This was modified from a patch from Fan Wu <wufan at codeaurora.org>
> Most changes are styles, comments and some small code refactoring.
> 
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>  src/acpi/method/method.c | 86 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 76 insertions(+), 10 deletions(-)
> 
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 638e937..732facc 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -27,6 +27,7 @@
>  #include <inttypes.h>
>  #include "fwts_acpi_object_eval.h"
>  
> +
>  /*
>   * ACPI methods + objects used in Linux ACPI driver:
>   *
> @@ -958,21 +959,86 @@ static int method_test_AEI(fwts_framework *fw)
>  		"_AEI", NULL, 0, method_test_AEI_return, NULL);
>  }
>  
> -static int method_test_EVT(fwts_framework *fw)
> +static void check_evt_event (
> +	fwts_framework *fw,
> +	ACPI_RESOURCE_GPIO *gpio)
>  {
>  	ACPI_OBJECT arg[1];
> -	int ret, i;
> +	ACPI_HANDLE evtMethodHandle;
> +	ACPI_STATUS Status;
> +	char Path[256];
> +	uint16_t i;

Style wise, if we can replace Status with status and Path with path as
that is the fwts codeing style.

> +
> +	/* Skip the leading spaces in ResourceSource. */
> +	for (i = 0; i < gpio->ResourceSource.StringLength; i++) {
> +		if (gpio->ResourceSource.StringPtr[i] != ' ')
> +			break;
> +	}
>  
> -	arg[0].Type = ACPI_TYPE_INTEGER;
> -	for (i = 0; i < 65535; i++) {
> -		arg[0].Integer.Value = i;
> -		ret = method_evaluate_method(fw, METHOD_OPTIONAL,
> -			"_EVT", arg, 1, method_test_NULL_return, NULL);
> +	if (i == gpio->ResourceSource.StringLength ) {

                                                  ^
just a minor style issue, can you remove the extraneous white spaces, e.g.
			

> +		fwts_log_warning(fw, "Invalid ResourceSource");
> +		return;
> +	}
>  
> -		if (ret != FWTS_OK)
> -			break;
> +	/* Get the handle of return;the _EVT method. */
> +	sprintf (Path, "%s._EVT", &gpio->ResourceSource.StringPtr[i]);
> +
> +	Status = AcpiGetHandle (NULL, Path, &evtMethodHandle);
> +	if (ACPI_FAILURE(Status)) {
> +		fwts_log_warning(fw, "Failed to find valid handle for EVT method (0x%x), %s",	Status, Path);
..and spaces between ,   Status
> +		return;
>  	}
> -	return ret;
> +
> +	/* Call the _EVT method with all the pins defined for the GpioInt */
> +	for (i = 0; i < gpio->PinTableLength; i++) {
> +		ACPI_OBJECT_LIST  arg_list;
> +
> +		arg[0].Type = ACPI_TYPE_INTEGER;
> +		arg[0].Integer.Value = gpio->PinTable[i];
> +		arg_list.Count   = 1;
> +		arg_list.Pointer = arg;
> +
> +		method_evaluate_found_method(fw, Path, method_test_NULL_return, NULL, &arg_list);
> +	}
> +}
> +
> +static void method_test_EVT_return (
> +	fwts_framework *fw,
> +	char *name,
> +	ACPI_BUFFER *buf,
> +	ACPI_OBJECT *obj,
> +	void *private)
> +{
> +	ACPI_RESOURCE *resource;
> +	ACPI_STATUS   Status;
> +
> +	FWTS_UNUSED(buf);
> +	FWTS_UNUSED(private);
> +
> +	if (method_check_type(fw, name, buf, ACPI_TYPE_BUFFER) != FWTS_OK)
> +		return;
> +
> +	Status = AcpiBufferToResource( obj->Buffer.Pointer, obj->Buffer.Length, &resource );

clean up white spaces ( obj-> ... resource );


> +	if (ACPI_FAILURE(Status))
> +		return;
> +
> +	do {
> +		if (!resource->Length) {
> +			fwts_log_warning(fw, "Invalid zero length descriptor in resource list\n");
> +			break;
> +		}
> +
> +		if (resource->Type == ACPI_RESOURCE_TYPE_GPIO && resource->Data.Gpio.ConnectionType == ACPI_RESOURCE_GPIO_TYPE_INT)

..this is a really long line..

> +				check_evt_event(fw, &resource->Data.Gpio);
> +
> +		resource = ACPI_NEXT_RESOURCE(resource);
> +	} while (resource->Type != ACPI_RESOURCE_TYPE_END_TAG);
> +}
> +static int method_test_EVT(fwts_framework *fw)
> +{
> +	/* Only test the _EVT method with pins defined in AEI. */
> +	return method_evaluate_method(fw, METHOD_OPTIONAL,
> +		"_AEI", NULL, 0, method_test_EVT_return, NULL);
>  }
>  
>  /*
> 

And can you fix up the regression test now we've got a change in
functionality, I'm seeing:

make check
...
FAIL: fwts-test/method-0001/test-0001.sh
...

Apart from the minor style issues, I'm OK with the changes. I still need
to run it through coverity scan, but I'm over quota on this for fwts
this week. I'll get back and report on this on Monday, so if you can
wait until then I'll let you know if that finds any other issues.

Colin