| Message ID | 1518589170-4427-1-git-send-email-alex.hung@canonical.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | acpi: method: allow _WAK to return an integer | expand |
On 02/14/2018 02:19 PM, Alex Hung wrote: > Linux kernel allows _WAK to return an integer, as in acpica/acpredef.h. > The reason seems to be that so many system BIOS returns an integer > instead of a package defined in ACPI spec, and it becomes a strange > standard already. Since operating system allows the integer, it should > be appropriate for fwts to relax it as well. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/method/method.c | 25 +++++++++++++++++-------- > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c > index 6839c3f..7c73e14 100644 > --- a/src/acpi/method/method.c > +++ b/src/acpi/method/method.c > @@ -5552,17 +5552,26 @@ static void method_test_WAK_return( > void *private) > { > FWTS_UNUSED(private); > + FWTS_UNUSED(buf); > > - if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK) > - return; > - > - if (fwts_method_package_count_equal(fw, name, "_WAK", obj, 2) != FWTS_OK) > - return; > + switch (obj->Type) { > + case ACPI_TYPE_PACKAGE: > + if (fwts_method_package_count_equal(fw, name, "_WAK", obj, 2) != FWTS_OK) > + return; > > - if (fwts_method_package_elements_all_type(fw, name, "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK) > - return; > + if (fwts_method_package_elements_all_type(fw, name, "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK) > + return; > > - fwts_method_passed_sane(fw, name, "package"); > + fwts_method_passed_sane(fw, name, "package"); > + break; > + case ACPI_TYPE_INTEGER: > + fwts_method_passed_sane(fw, name, "integer"); > + break; > + default: > + fwts_failed(fw, LOG_LEVEL_CRITICAL, "MethodReturnBadType", > + "%s did not return a package or an integer.", name); > + break; > + } > } > > static int method_test_WAK(fwts_framework *fw) > Should we give some warnings for this? Ivan
On Tue, Feb 20, 2018 at 11:20 PM, ivanhu <ivan.hu@canonical.com> wrote: > > > On 02/14/2018 02:19 PM, Alex Hung wrote: >> >> Linux kernel allows _WAK to return an integer, as in acpica/acpredef.h. >> The reason seems to be that so many system BIOS returns an integer >> instead of a package defined in ACPI spec, and it becomes a strange >> standard already. Since operating system allows the integer, it should >> be appropriate for fwts to relax it as well. >> >> Signed-off-by: Alex Hung <alex.hung@canonical.com> >> --- >> src/acpi/method/method.c | 25 +++++++++++++++++-------- >> 1 file changed, 17 insertions(+), 8 deletions(-) >> >> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c >> index 6839c3f..7c73e14 100644 >> --- a/src/acpi/method/method.c >> +++ b/src/acpi/method/method.c >> @@ -5552,17 +5552,26 @@ static void method_test_WAK_return( >> void *private) >> { >> FWTS_UNUSED(private); >> + FWTS_UNUSED(buf); >> - if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != >> FWTS_OK) >> - return; >> - >> - if (fwts_method_package_count_equal(fw, name, "_WAK", obj, 2) != >> FWTS_OK) >> - return; >> + switch (obj->Type) { >> + case ACPI_TYPE_PACKAGE: >> + if (fwts_method_package_count_equal(fw, name, "_WAK", obj, >> 2) != FWTS_OK) >> + return; >> - if (fwts_method_package_elements_all_type(fw, name, "_WAK", obj, >> ACPI_TYPE_INTEGER) != FWTS_OK) >> - return; >> + if (fwts_method_package_elements_all_type(fw, name, >> "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK) >> + return; >> - fwts_method_passed_sane(fw, name, "package"); >> + fwts_method_passed_sane(fw, name, "package"); >> + break; >> + case ACPI_TYPE_INTEGER: >> + fwts_method_passed_sane(fw, name, "integer"); >> + break; >> + default: >> + fwts_failed(fw, LOG_LEVEL_CRITICAL, "MethodReturnBadType", >> + "%s did not return a package or an integer.", >> name); >> + break; >> + } >> } >> static int method_test_WAK(fwts_framework *fw) >> > > > Should we give some warnings for this? As far as I know, both Linux and Windows have been allowing it for many years, since nobody gets it right since the beginning. It is almost like a standard now... I would say we don't need to worry about it at all. > > Ivan > > -- > fwts-devel mailing list > fwts-devel@lists.ubuntu.com > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/fwts-devel
Hi Alex, I think it's no harm to give a warning for the firmware which not followed the ACPI spec. FWTS tests the firmware, not test for OS(Windows and Linux), even OSes work with it, but it still not followed the ACPI spec. Besides, we are not sure all OSes work with it. Or we can send an ECR to UEFI forum to modify the ACPI _WAK method for returning integer. Anyone has comments? Cheers, Ivan On 02/21/2018 03:27 PM, Alex Hung wrote: > On Tue, Feb 20, 2018 at 11:20 PM, ivanhu <ivan.hu@canonical.com> wrote: >> >> On 02/14/2018 02:19 PM, Alex Hung wrote: >>> Linux kernel allows _WAK to return an integer, as in acpica/acpredef.h. >>> The reason seems to be that so many system BIOS returns an integer >>> instead of a package defined in ACPI spec, and it becomes a strange >>> standard already. Since operating system allows the integer, it should >>> be appropriate for fwts to relax it as well. >>> >>> Signed-off-by: Alex Hung <alex.hung@canonical.com> >>> --- >>> src/acpi/method/method.c | 25 +++++++++++++++++-------- >>> 1 file changed, 17 insertions(+), 8 deletions(-) >>> >>> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c >>> index 6839c3f..7c73e14 100644 >>> --- a/src/acpi/method/method.c >>> +++ b/src/acpi/method/method.c >>> @@ -5552,17 +5552,26 @@ static void method_test_WAK_return( >>> void *private) >>> { >>> FWTS_UNUSED(private); >>> + FWTS_UNUSED(buf); >>> - if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != >>> FWTS_OK) >>> - return; >>> - >>> - if (fwts_method_package_count_equal(fw, name, "_WAK", obj, 2) != >>> FWTS_OK) >>> - return; >>> + switch (obj->Type) { >>> + case ACPI_TYPE_PACKAGE: >>> + if (fwts_method_package_count_equal(fw, name, "_WAK", obj, >>> 2) != FWTS_OK) >>> + return; >>> - if (fwts_method_package_elements_all_type(fw, name, "_WAK", obj, >>> ACPI_TYPE_INTEGER) != FWTS_OK) >>> - return; >>> + if (fwts_method_package_elements_all_type(fw, name, >>> "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK) >>> + return; >>> - fwts_method_passed_sane(fw, name, "package"); >>> + fwts_method_passed_sane(fw, name, "package"); >>> + break; >>> + case ACPI_TYPE_INTEGER: >>> + fwts_method_passed_sane(fw, name, "integer"); >>> + break; >>> + default: >>> + fwts_failed(fw, LOG_LEVEL_CRITICAL, "MethodReturnBadType", >>> + "%s did not return a package or an integer.", >>> name); >>> + break; >>> + } >>> } >>> static int method_test_WAK(fwts_framework *fw) >>> >> >> Should we give some warnings for this? > As far as I know, both Linux and Windows have been allowing it for > many years, since nobody gets it right since the beginning. It is > almost like a standard now... > > I would say we don't need to worry about it at all. > >> Ivan >> >> -- >> fwts-devel mailing list >> fwts-devel@lists.ubuntu.com >> Modify settings or unsubscribe at: >> https://lists.ubuntu.com/mailman/listinfo/fwts-devel > >
On 22/02/18 09:59, ivanhu wrote: > Hi Alex, > > > I think it's no harm to give a warning for the firmware which not > followed the ACPI spec. +1 > > FWTS tests the firmware, not test for OS(Windows and Linux), even OSes > work with it, but it still not followed the ACPI spec. +1 > > Besides, we are not sure all OSes work with it. +1 > > Or we can send an ECR to UEFI forum to modify the ACPI _WAK method for > returning integer. Clarification with the UEFI forum would be useful too. Colin > > > Anyone has comments? > > > Cheers, > > Ivan > > > On 02/21/2018 03:27 PM, Alex Hung wrote: >> On Tue, Feb 20, 2018 at 11:20 PM, ivanhu <ivan.hu@canonical.com> wrote: >>> >>> On 02/14/2018 02:19 PM, Alex Hung wrote: >>>> Linux kernel allows _WAK to return an integer, as in acpica/acpredef.h. >>>> The reason seems to be that so many system BIOS returns an integer >>>> instead of a package defined in ACPI spec, and it becomes a strange >>>> standard already. Since operating system allows the integer, it should >>>> be appropriate for fwts to relax it as well. >>>> >>>> Signed-off-by: Alex Hung <alex.hung@canonical.com> >>>> --- >>>> src/acpi/method/method.c | 25 +++++++++++++++++-------- >>>> 1 file changed, 17 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c >>>> index 6839c3f..7c73e14 100644 >>>> --- a/src/acpi/method/method.c >>>> +++ b/src/acpi/method/method.c >>>> @@ -5552,17 +5552,26 @@ static void method_test_WAK_return( >>>> void *private) >>>> { >>>> FWTS_UNUSED(private); >>>> + FWTS_UNUSED(buf); >>>> - if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != >>>> FWTS_OK) >>>> - return; >>>> - >>>> - if (fwts_method_package_count_equal(fw, name, "_WAK", obj, 2) != >>>> FWTS_OK) >>>> - return; >>>> + switch (obj->Type) { >>>> + case ACPI_TYPE_PACKAGE: >>>> + if (fwts_method_package_count_equal(fw, name, "_WAK", obj, >>>> 2) != FWTS_OK) >>>> + return; >>>> - if (fwts_method_package_elements_all_type(fw, name, "_WAK", obj, >>>> ACPI_TYPE_INTEGER) != FWTS_OK) >>>> - return; >>>> + if (fwts_method_package_elements_all_type(fw, name, >>>> "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK) >>>> + return; >>>> - fwts_method_passed_sane(fw, name, "package"); >>>> + fwts_method_passed_sane(fw, name, "package"); >>>> + break; >>>> + case ACPI_TYPE_INTEGER: >>>> + fwts_method_passed_sane(fw, name, "integer"); >>>> + break; >>>> + default: >>>> + fwts_failed(fw, LOG_LEVEL_CRITICAL, "MethodReturnBadType", >>>> + "%s did not return a package or an integer.", >>>> name); >>>> + break; >>>> + } >>>> } >>>> static int method_test_WAK(fwts_framework *fw) >>>> >>> >>> Should we give some warnings for this? >> As far as I know, both Linux and Windows have been allowing it for >> many years, since nobody gets it right since the beginning. It is >> almost like a standard now... >> >> I would say we don't need to worry about it at all. >> >>> Ivan >>> >>> -- >>> fwts-devel mailing list >>> fwts-devel@lists.ubuntu.com >>> Modify settings or unsubscribe at: >>> https://lists.ubuntu.com/mailman/listinfo/fwts-devel >> >> > > > >
On Thu, Feb 22, 2018 at 2:05 AM, Colin Ian King <colin.king@canonical.com> wrote: > On 22/02/18 09:59, ivanhu wrote: >> Hi Alex, >> >> >> I think it's no harm to give a warning for the firmware which not >> followed the ACPI spec. > > +1 > >> >> FWTS tests the firmware, not test for OS(Windows and Linux), even OSes >> work with it, but it still not followed the ACPI spec. > > +1 > >> >> Besides, we are not sure all OSes work with it. > > +1 > >> >> Or we can send an ECR to UEFI forum to modify the ACPI _WAK method for >> returning integer. > > Clarification with the UEFI forum would be useful too. Certainly, I will send an email to awsg to ask about the history and for comments. > > Colin >> >> >> Anyone has comments? >> >> >> Cheers, >> >> Ivan >> >> >> On 02/21/2018 03:27 PM, Alex Hung wrote: >>> On Tue, Feb 20, 2018 at 11:20 PM, ivanhu <ivan.hu@canonical.com> wrote: >>>> >>>> On 02/14/2018 02:19 PM, Alex Hung wrote: >>>>> Linux kernel allows _WAK to return an integer, as in acpica/acpredef.h. >>>>> The reason seems to be that so many system BIOS returns an integer >>>>> instead of a package defined in ACPI spec, and it becomes a strange >>>>> standard already. Since operating system allows the integer, it should >>>>> be appropriate for fwts to relax it as well. >>>>> >>>>> Signed-off-by: Alex Hung <alex.hung@canonical.com> >>>>> --- >>>>> src/acpi/method/method.c | 25 +++++++++++++++++-------- >>>>> 1 file changed, 17 insertions(+), 8 deletions(-) >>>>> >>>>> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c >>>>> index 6839c3f..7c73e14 100644 >>>>> --- a/src/acpi/method/method.c >>>>> +++ b/src/acpi/method/method.c >>>>> @@ -5552,17 +5552,26 @@ static void method_test_WAK_return( >>>>> void *private) >>>>> { >>>>> FWTS_UNUSED(private); >>>>> + FWTS_UNUSED(buf); >>>>> - if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != >>>>> FWTS_OK) >>>>> - return; >>>>> - >>>>> - if (fwts_method_package_count_equal(fw, name, "_WAK", obj, 2) != >>>>> FWTS_OK) >>>>> - return; >>>>> + switch (obj->Type) { >>>>> + case ACPI_TYPE_PACKAGE: >>>>> + if (fwts_method_package_count_equal(fw, name, "_WAK", obj, >>>>> 2) != FWTS_OK) >>>>> + return; >>>>> - if (fwts_method_package_elements_all_type(fw, name, "_WAK", obj, >>>>> ACPI_TYPE_INTEGER) != FWTS_OK) >>>>> - return; >>>>> + if (fwts_method_package_elements_all_type(fw, name, >>>>> "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK) >>>>> + return; >>>>> - fwts_method_passed_sane(fw, name, "package"); >>>>> + fwts_method_passed_sane(fw, name, "package"); >>>>> + break; >>>>> + case ACPI_TYPE_INTEGER: >>>>> + fwts_method_passed_sane(fw, name, "integer"); >>>>> + break; >>>>> + default: >>>>> + fwts_failed(fw, LOG_LEVEL_CRITICAL, "MethodReturnBadType", >>>>> + "%s did not return a package or an integer.", >>>>> name); >>>>> + break; >>>>> + } >>>>> } >>>>> static int method_test_WAK(fwts_framework *fw) >>>>> >>>> >>>> Should we give some warnings for this? >>> As far as I know, both Linux and Windows have been allowing it for >>> many years, since nobody gets it right since the beginning. It is >>> almost like a standard now... >>> >>> I would say we don't need to worry about it at all. >>> >>>> Ivan >>>> >>>> -- >>>> fwts-devel mailing list >>>> fwts-devel@lists.ubuntu.com >>>> Modify settings or unsubscribe at: >>>> https://lists.ubuntu.com/mailman/listinfo/fwts-devel >>> >>> >> >> >> >> > >
diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c index 6839c3f..7c73e14 100644 --- a/src/acpi/method/method.c +++ b/src/acpi/method/method.c @@ -5552,17 +5552,26 @@ static void method_test_WAK_return( void *private) { FWTS_UNUSED(private); + FWTS_UNUSED(buf); - if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK) - return; - - if (fwts_method_package_count_equal(fw, name, "_WAK", obj, 2) != FWTS_OK) - return; + switch (obj->Type) { + case ACPI_TYPE_PACKAGE: + if (fwts_method_package_count_equal(fw, name, "_WAK", obj, 2) != FWTS_OK) + return; - if (fwts_method_package_elements_all_type(fw, name, "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK) - return; + if (fwts_method_package_elements_all_type(fw, name, "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK) + return; - fwts_method_passed_sane(fw, name, "package"); + fwts_method_passed_sane(fw, name, "package"); + break; + case ACPI_TYPE_INTEGER: + fwts_method_passed_sane(fw, name, "integer"); + break; + default: + fwts_failed(fw, LOG_LEVEL_CRITICAL, "MethodReturnBadType", + "%s did not return a package or an integer.", name); + break; + } } static int method_test_WAK(fwts_framework *fw)
Linux kernel allows _WAK to return an integer, as in acpica/acpredef.h. The reason seems to be that so many system BIOS returns an integer instead of a package defined in ACPI spec, and it becomes a strange standard already. Since operating system allows the integer, it should be appropriate for fwts to relax it as well. Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/acpi/method/method.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)