| Message ID | 1438350264-27251-1-git-send-email-alex.hung@canonical.com |
|---|---|
| State | Rejected |
| Headers | show |
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@codeaurora.org> > Most changes are styles, comments and some small code refactoring. > > Signed-off-by: Alex Hung <alex.hung@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
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? 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@codeaurora.org> > Most changes are styles, comments and some small code refactoring. > > Signed-off-by: Alex Hung <alex.hung@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); > } > > /* >
Loop in two developers for discussion :). On Fri, Jul 31, 2015 at 10:25 PM, Alex Hung <alex.hung@canonical.com> 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? > > > 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@codeaurora.org> >> Most changes are styles, comments and some small code refactoring. >> >> Signed-off-by: Alex Hung <alex.hung@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); >> } >> >> /* >> > > > -- > Cheers, > Alex Hung
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@codeaurora.org> >> Most changes are styles, comments and some small code refactoring. >> >> Signed-off-by: Alex Hung <alex.hung@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); >> } >> >> /* >> > >
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@codeaurora.org> > Most changes are styles, comments and some small code refactoring. > > Signed-off-by: Alex Hung <alex.hung@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]); snprintf() should always used instead of sprintf() to ensure we don't get buffer overruns. > + > + 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); > } > > /* >
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); } /*
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@codeaurora.org> Most changes are styles, comments and some small code refactoring. Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/acpi/method/method.c | 86 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 76 insertions(+), 10 deletions(-)