[PATCH] acpi: method: relax mandatory requirement for _PTS, _TTS and _WAK (LP: #1296737)

[Alex Hung]

> OK, can you advise me where abouts in the spec? I can then pass that
> back to the party that requested I tweaked this.

I guess it is more indirectly definition: in Section 73. of ACPI spec
5.0, it explicitly specifies optional control methods (ex. _BFS and
_GTS), but it does not state _PTS and _WAK as optional.

>
>> Changing them to optional makes it kernel-dependent and is not exactly
>> testing firmware.
>
> +1
>
>>
>> Will it be better to change the advise or lower the severity so users
>> know the impacts are minimal?
>>
>
> How about I add some contextual advice on this instead explaining what
> the kernel does in these cases?

That sounds good.

On Tue, Mar 25, 2014 at 4:06 PM, Colin Ian King
<colin.king at canonical.com> wrote:
> On 25/03/14 03:13, Alex Hung wrote:
>> On 03/24/2014 09:59 PM, Colin King wrote:
>>> From: Colin Ian King <colin.king at canonical.com>
>>>
>>> The kernel does not throw any errors if these controls don't exist,
>>> so we probably should relax the method test so it does not report
>>> a failure if these controls don't exist.
>>>
>>> Signed-off-by: Colin Ian King <colin.king at canonical.com>
>>> ---
>>>  src/acpi/method/method.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
>>> index 6e6c5c2..377d2ef 100644
>>> --- a/src/acpi/method/method.c
>>> +++ b/src/acpi/method/method.c
>>> @@ -4305,7 +4305,7 @@ static int method_test_PTS(fwts_framework *fw)
>>>
>>>              fwts_log_info(fw, "Test _PTS(%d).", i);
>>>
>>> -            if (method_evaluate_method(fw, METHOD_MANDITORY, "_PTS", arg, 1,
>>> +            if (method_evaluate_method(fw, METHOD_OPTIONAL, "_PTS", arg, 1,
>>>                      method_test_NULL_return, NULL) == FWTS_NOT_EXIST) {
>>>                      fwts_advice(fw,
>>>                              "Could not find _PTS. This method provides a "
>>> @@ -4336,7 +4336,7 @@ static int method_test_TTS(fwts_framework *fw)
>>>                      fwts_log_info(fw,
>>>                              "Test _TTS(%d) Transition To State S%d.", i, i);
>>>
>>> -                    if (method_evaluate_method(fw, METHOD_MANDITORY,
>>> +                    if (method_evaluate_method(fw, METHOD_OPTIONAL,
>>>                              "_TTS", arg, 1, method_test_NULL_return,
>>>                              NULL) == FWTS_NOT_EXIST)
>>>                              break;
>>> @@ -4412,7 +4412,7 @@ static int method_test_WAK(fwts_framework *fw)
>>>              arg[0].Type = ACPI_TYPE_INTEGER;
>>>              arg[0].Integer.Value = i;
>>>              fwts_log_info(fw, "Test _WAK(%d) System Wake, State S%d.", i, i);
>>> -            if (method_evaluate_method(fw, METHOD_MANDITORY, "_WAK", arg, 1,
>>> +            if (method_evaluate_method(fw, METHOD_OPTIONAL, "_WAK", arg, 1,
>>>                      method_test_WAK_return, &i) == FWTS_NOT_EXIST)
>>>                      break;
>>>              fwts_log_nl(fw);
>>>
>>
>>
>> This may not the best idea since they are needed according to ACPI spec.
>
> OK, can you advise me where abouts in the spec? I can then pass that
> back to the party that requested I tweaked this.
>
>> Changing them to optional makes it kernel-dependent and is not exactly
>> testing firmware.
>
> +1
>
>>
>> Will it be better to change the advise or lower the severity so users
>> know the impacts are minimal?
>>
>
> How about I add some contextual advice on this instead explaining what
> the kernel does in these cases?
>
> Colin
>
> --
> fwts-devel mailing list
> fwts-devel at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel



-- 
Cheers,
Alex Hung