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

Colin Ian King colin.king at canonical.com
Fri Jul 31 14:38:48 UTC 2015


On 31/07/15 15:25, Alex Hung wrote:
> There is a side-effect on this patch that I'd like to discuss (therefore
> RFC in topic).
> 
> When I run "sudo make check", it shows an failure as below:
> 
> < method          _EVT.
> ---
>> method          _AEI.
> 
> As it is the result of below:
> 
> method          Test 2 of 180: Test _AEI.
> method          SKIPPED: Test 2, Skipping test for non-existant object
> method          _AEI.
> method
> method          Test 3 of 180: Test _EVT (Event Method).
> method          SKIPPED: Test 3, Skipping test for non-existant object
> method          _AEI.
> 
> From function perspective, the patch works fine but it may not look
> quite the same as other methods.  Does anyone have any comments?

The skipping message is a bit confusing. One way around this is to add
an extra bit to the test_type flag in method_evaluate_method() so we can
decided to print messages or not, e.g.

#define METHOD_SILENT           8

...

and in method_evaluate_method() do:

	if (!(test_type & METHOD_SILENT)) {
                /* Manditory not-found test are a failure */
                if (test_type & METHOD_MANDITORY) {
                        fwts_failed(fw, LOG_LEVEL_MEDIUM, "MethodNotExist",
                                "Object %s did not exist.", name);
                }

                /* Mobile specific tests on non-mobile platform? */
                if ((test_type & METHOD_MOBILE) &&
(!fadt_mobile_platform)) {
                        fwts_skipped(fw,
                                "Machine is not a mobile platform,
skipping "
                                "test for non-existant mobile platform "
                                "related object %s.", name);
                } else {
                        fwts_skipped(fw,
                                "Skipping test for non-existant object %s.",
                                name);
                }
	}
	return FWTS_NOT_EXIST;

and in then:

static int method_test_EVT(fwts_framework *fw)
{
	int ret;

        /* Only test the _EVT method with pins defined in AEI. */
        ret = method_evaluate_method(fw, METHOD_OPTIONAL |
METHOD_SILENT, "_AEI", NULL, 0, method_test_EVT_return, NULL);

	if (ret == FWTS_NOT_EXIST)
		fwts_skipped(fw,
                      "Skipping test for non-existant object %s.",name);

	return ret;
}

Perhaps that's the simplest way forward.

> 
> 
> On 07/31/2015 09:44 PM, 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;
>> +
>> +	/* 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 ) {
>> +		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);
>> +		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 );
>> +	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)
>> +				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);
>>  }
>>  
>>  /*
>>
> 
> 




More information about the fwts-devel mailing list