NAK: [PATCH 1/2] acpi: add tests for _SRT control method
Colin Ian King
colin.king at canonical.com
Fri Nov 10 10:37:52 UTC 2017
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.
>
>
>> +}
>> +
>> 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)
>>
>
>
More information about the fwts-devel
mailing list