NAK: [PATCH 1/2] acpi: add tests for _SRT control method
Alex Hung
alex.hung at canonical.com
Fri Nov 10 12:31:16 UTC 2017
On Fri, Nov 10, 2017 at 6:37 PM, Colin Ian King
<colin.king at canonical.com> wrote:
> On 10/11/17 10:17, Alex Hung wrote:
>> On 2017-11-10 03:37 PM, Alex Hung wrote:
>>> Signed-off-by: Alex Hung <alex.hung at canonical.com>
>>> ---
>>> src/acpi/devices/time/time.c | 43
>>> ++++++++++++++++++++++++++++++++-
>>> src/acpi/method/method.c | 41
>>> +++++++++++++++++++++++++++++--
>>> src/lib/include/fwts_acpi_object_eval.h | 15 ++++++++++++
>>> 3 files changed, 96 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/acpi/devices/time/time.c b/src/acpi/devices/time/time.c
>>> index 8721182..9722330 100644
>>> --- a/src/acpi/devices/time/time.c
>>> +++ b/src/acpi/devices/time/time.c
>>> @@ -147,6 +147,47 @@ static int method_test_GRT(fwts_framework *fw)
>>> "_GRT", NULL, 0, method_test_GRT_return, NULL);
>>> }
>>> +static void method_test_SRT_return(
>>> + fwts_framework *fw,
>>> + char *name,
>>> + ACPI_BUFFER *buf,
>>> + ACPI_OBJECT *obj,
>>> + void *private)
>>> +{
>>> + FWTS_UNUSED(private);
>>> +
>>> + if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) !=
>>> FWTS_OK)
>>> + return;
>>> +
>>> + fwts_method_passed_sane_uint64(fw, name, obj->Integer.Value);
>>> +}
>>> +
>>> +static int method_test_SRT(fwts_framework *fw)
>>> +{
>>> + uint32_t time_size = sizeof(fwts_acpi_time_buffer);
>>> + fwts_acpi_time_buffer real_time;
>>> + ACPI_OBJECT arg0;
>>> +
>>> + real_time.year = 2000;
>>> + real_time.month = 1;
>>> + real_time.day = 1;
>>> + real_time.hour = 0;
>>> + real_time.minute = 0;
>>> + real_time.milliseconds = 1;
>>> + real_time.timezone = 0;
>>> +
>>> + arg0.Type = ACPI_TYPE_BUFFER;
>>> + arg0.Buffer.Length = time_size;
>>> + arg0.Buffer.Pointer = (void *)&real_time;
>>> +
>>> + if (capability & 0x04)
>>> + return fwts_evaluate_method(fw, METHOD_MANDATORY, &device,
>>> + "_SRT", &arg0, time_size, method_test_SRT_return, NULL);
>>> + else
>>> + return fwts_evaluate_method(fw, METHOD_OPTIONAL, &device,
>>> + "_SRT", &arg0, time_size, method_test_SRT_return, NULL);
>>> +}
>>> +
>>> static void method_test_GWS_return(
>>> fwts_framework *fw,
>>> char *name,
>>> @@ -270,7 +311,7 @@ static int method_test_TIV(fwts_framework *fw)
>>> static fwts_framework_minor_test acpi_time_alarm_tests[] = {
>>> { method_test_GCP, "Test _GCP (Get Capabilities)." },
>>> { method_test_GRT, "Test _GRT (Get Real Time)." },
>>> - /* { method_test_SRT, "Test _SRT (Set Real Time)." }, */
>>> + { method_test_SRT, "Test _SRT (Set Real Time)." },
>>> { method_test_GWS, "Test _GWS (Get Wake Status)." },
>>> { method_test_CWS, "Test _CWS (Clear Wake Status)." },
>>> { method_test_STP, "Test _STP (Set Expired Timer Wake Policy)." },
>>> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
>>> index 509424f..2089294 100644
>>> --- a/src/acpi/method/method.c
>>> +++ b/src/acpi/method/method.c
>>> @@ -213,7 +213,7 @@
>>> * _SLI N
>>> * _SPD Y
>>> * _SRS n/a
>>> - * _SRT n/a
>>> + * _SRT Y
>>> * _SRV Y
>>> * _SST Y
>>> * _STA Y
>>> @@ -3720,6 +3720,43 @@ static int method_test_GRT(fwts_framework *fw)
>>> "_GRT", NULL, 0, method_test_GRT_return, NULL);
>>> }
>>> +static void method_test_SRT_return(
>>> + fwts_framework *fw,
>>> + char *name,
>>> + ACPI_BUFFER *buf,
>>> + ACPI_OBJECT *obj,
>>> + void *private)
>>> +{
>>> + FWTS_UNUSED(private);
>>> +
>>> + if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) !=
>>> FWTS_OK)
>>> + return;
>>> +
>>> + fwts_method_passed_sane_uint64(fw, name, obj->Integer.Value);
>>> +}
>>> +
>>> +static int method_test_SRT(fwts_framework *fw)
>>> +{
>>> + uint32_t time_size = sizeof(fwts_acpi_time_buffer);
>>> + fwts_acpi_time_buffer real_time;
>>> + ACPI_OBJECT arg0;
>>> +
>>> + real_time.year = 2000;
>>> + real_time.month = 1;
>>> + real_time.day = 1;
>>> + real_time.hour = 0;
>>> + real_time.minute = 0;
>>> + real_time.milliseconds = 1;
>>> + real_time.timezone = 0;
>>> +
>>> + arg0.Type = ACPI_TYPE_BUFFER;
>>> + arg0.Buffer.Length = time_size;
>>> + arg0.Buffer.Pointer = (void *)&real_time;
>>> +
>>> + return method_evaluate_method(fw, METHOD_OPTIONAL,
>>> + "_SRT", &arg0, time_size, method_test_SRT_return, NULL);
>>
>> Nack this because Coverity Scan reports "Taking address with "&arg0"
>> yields a singleton pointer." I will look into it and submit a V2.
>
> I'm not so sure this is a bug per-se, I think this is CoverityScan being
> a bit too pedantic. I suggest double checking this and if it look sane
> to you then I'm OK with it. I've already tagged it in CoverityScan to
> ignore this particular warning.
I think this particular warning can be ignored, but I found another
error that needs fixing so a V2 is still needed.
>
>>
>>
>>> +}
>>> +
>>> static void method_test_GWS_return(
>>> fwts_framework *fw,
>>> char *name,
>>> @@ -6084,7 +6121,7 @@ static fwts_framework_minor_test method_tests[] = {
>>> { method_test_GRT, "Test _GRT (Get Real Time)." },
>>> { method_test_GWS, "Test _GWS (Get Wake Status)." },
>>> { method_test_CWS, "Test _CWS (Clear Wake Status)." },
>>> - /* { method_test_SRT, "Test _SRT (Set Real Time)." }, */
>>> + { method_test_SRT, "Test _SRT (Set Real Time)." },
>>> { method_test_STP, "Test _STP (Set Expired Timer Wake Policy)." },
>>> { method_test_STV, "Test _STV (Set Timer Value)." },
>>> { method_test_TIP, "Test _TIP (Expired Timer Wake Policy)." },
>>> diff --git a/src/lib/include/fwts_acpi_object_eval.h
>>> b/src/lib/include/fwts_acpi_object_eval.h
>>> index 17f9a6d..3fa3606 100644
>>> --- a/src/lib/include/fwts_acpi_object_eval.h
>>> +++ b/src/lib/include/fwts_acpi_object_eval.h
>>> @@ -51,6 +51,21 @@ typedef struct {
>>> const char *name; /* Field name */
>>> } fwts_package_element;
>>> +/* Time format for ACPI000E (Time and Alarm Device) */
>>> +typedef struct {
>>> + uint16_t year;
>>> + uint8_t month;
>>> + uint8_t day;
>>> + uint8_t hour;
>>> + uint8_t minute;
>>> + uint8_t second;
>>> + uint8_t valid; // pad1 for _SRT
>>> + uint16_t milliseconds;
>>> + uint16_t timezone;
>>> + uint8_t daylight;
>>> + uint8_t pad2[3];
>>> +} __attribute__ ((packed)) fwts_acpi_time_buffer;
>>> +
>>> #define fwts_method_check_type(fw, name, buf, type) \
>>> fwts_method_check_type__(fw, name, buf, type, #type)
>>>
>>
>>
>
--
Cheers,
Alex Hung
More information about the fwts-devel
mailing list