Message ID | 20200425033159.210995-1-alex.hung@canonical.com |
---|---|
State | Superseded |
Headers | show |
Series | acpi/method: check _PS3 exists when _PS0 is present | expand |
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@molgen.mpg.de> > Signed-off-by: Alex Hung <alex.hung@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
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. + */ + 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)
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@molgen.mpg.de> Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/acpi/method/method.c | 51 +++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 21 deletions(-)