[PATCH] acpi: update checks of _BIX return package
Colin Ian King
colin.king at canonical.com
Wed Nov 8 09:31:37 UTC 2017
On 08/11/17 08:40, Alex Hung wrote:
> ACPI 6.0 introduced a new change "Swapping Capability" to _BIX method in
> mantis 1284.
>
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
> src/acpi/devices/battery/battery.c | 82 ++++++++++++++++++++++++++++++++------
> src/acpi/method/method.c | 66 ++++++++++++++++++++++++++++--
> 2 files changed, 132 insertions(+), 16 deletions(-)
>
> diff --git a/src/acpi/devices/battery/battery.c b/src/acpi/devices/battery/battery.c
> index f692232..dcdb762 100644
> --- a/src/acpi/devices/battery/battery.c
> +++ b/src/acpi/devices/battery/battery.c
> @@ -223,7 +223,9 @@ static void method_test_BIX_return(
> void *private)
> {
> bool failed = false;
> + uint64_t revision = 0;
>
> + /* Revision 0 in ACPI 5.x */
> static const fwts_package_element elements[] = {
> { ACPI_TYPE_INTEGER, "Revision" },
> { ACPI_TYPE_INTEGER, "Power Unit" },
> @@ -247,21 +249,65 @@ static void method_test_BIX_return(
> { ACPI_TYPE_STRING, "OEM Information" }
> };
>
> + /* Revision 1 in ACPI 6.x introduces swapping capability */
> + static const fwts_package_element elements_v1[] = {
> + { ACPI_TYPE_INTEGER, "Revision" },
> + { ACPI_TYPE_INTEGER, "Power Unit" },
> + { ACPI_TYPE_INTEGER, "Design Capacity" },
> + { ACPI_TYPE_INTEGER, "Last Full Charge Capacity" },
> + { ACPI_TYPE_INTEGER, "Battery Technology" },
> + { ACPI_TYPE_INTEGER, "Design Voltage" },
> + { ACPI_TYPE_INTEGER, "Design Capacity of Warning" },
> + { ACPI_TYPE_INTEGER, "Design Capactty of Low" },
Capacity typo above ^^
> + { ACPI_TYPE_INTEGER, "Cycle Count" },
> + { ACPI_TYPE_INTEGER, "Measurement Accuracy" },
> + { ACPI_TYPE_INTEGER, "Max Sampling Time" },
> + { ACPI_TYPE_INTEGER, "Min Sampling Time" },
> + { ACPI_TYPE_INTEGER, "Max Averaging Interval" },
> + { ACPI_TYPE_INTEGER, "Min Averaging Interval" },
> + { ACPI_TYPE_INTEGER, "Battery Capacity Granularity 1" },
> + { ACPI_TYPE_INTEGER, "Battery Capacity Granularity 2" },
> + { ACPI_TYPE_STRING, "Model Number" },
> + { ACPI_TYPE_STRING, "Serial Number" },
> + { ACPI_TYPE_STRING, "Battery Type" },
> + { ACPI_TYPE_STRING, "OEM Information" },
> + { ACPI_TYPE_INTEGER, "Battery Swapping Capability" }
> + };
> +
> FWTS_UNUSED(private);
>
> if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)
> return;
>
> - if (fwts_method_package_count_equal(fw, name, "_BIX", obj, 20) != FWTS_OK)
> - return;
> + if (obj->Package.Count > 1 && obj->Package.Elements[0].Type == ACPI_TYPE_INTEGER)
> + revision = obj->Package.Elements[0].Integer.Value;
>
> - if (fwts_method_package_elements_type(fw, name, "_BIX", obj, elements, 20) != FWTS_OK)
> - return;
I'd prefer a switch statement on revision just because there could be
more in the future :-)
> + if (revision == 0) {
> + if (fwts_method_package_count_equal(fw, name, "_BIX", obj, 20) != FWTS_OK)
> + return;
> +
> + if (fwts_method_package_elements_type(fw, name, "_BIX", obj, elements, 20) != FWTS_OK)
> + return;
> + } else if (revision == 1) {
> + if (fwts_method_package_count_equal(fw, name, "_BIX", obj, 21) != FWTS_OK)
> + return;
> +
> + if (fwts_method_package_elements_type(fw, name, "_BIX", obj, elements_v1, 21) != FWTS_OK)
> + return;
> + } else {
> + fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + "Method_BIXRevision",
> + "%s: Expected %s (Element 0) to be "
> + "0 or 1, got 0x%8.8" PRIx64 ".",
> + name, elements_v1[0].name,
> + (uint64_t)obj->Package.Elements[0].Integer.Value);
> + failed = true;
> + }
>
> /* Sanity check each field */
> /* Power Unit */
> if (obj->Package.Elements[1].Integer.Value > 0x00000002) {
> - fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> "Method_BIXPowerUnit",
> "%s: Expected %s (Element 1) to be "
> "0 (mWh) or 1 (mAh), got 0x%8.8" PRIx64 ".",
> @@ -273,10 +319,10 @@ static void method_test_BIX_return(
> /*
> * Since this information may be evaluated by communicating with
> * the EC we skip these checks as we can't do this from userspace
> - */
> + */
> /* Design Capacity */
> if (obj->Package.Elements[2].Integer.Value > 0x7fffffff) {
> - fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + fwts_failed(fw, LOG_LEVEL_LOW,
> "Method_BIXDesignCapacity",
> "%s: %s (Element 2) is "
> "unknown: 0x%8.8" PRIx64 ".",
> @@ -286,7 +332,7 @@ static void method_test_BIX_return(
> }
> /* Last Full Charge Capacity */
> if (obj->Package.Elements[3].Integer.Value > 0x7fffffff) {
> - fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + fwts_failed(fw, LOG_LEVEL_LOW,
> "Method_BIXFullChargeCapacity",
> "%s: %s (Element 3) "
> "is unknown: 0x%8.8" PRIx64 ".",
> @@ -310,10 +356,10 @@ static void method_test_BIX_return(
> /*
> * Since this information may be evaluated by communicating with
> * the EC we skip these checks as we can't do this from userspace
> - */
> + */
> /* Design Voltage */
> if (obj->Package.Elements[5].Integer.Value > 0x7fffffff) {
> - fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + fwts_failed(fw, LOG_LEVEL_LOW,
> "Method_BIXDesignVoltage",
> "%s: %s (Element 5) is unknown: "
> "0x%8.8" PRIx64 ".",
> @@ -323,7 +369,7 @@ static void method_test_BIX_return(
> }
> /* Design capacity warning */
> if (obj->Package.Elements[6].Integer.Value > 0x7fffffff) {
> - fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + fwts_failed(fw, LOG_LEVEL_LOW,
> "Method_BIXDesignCapacityE6",
> "%s: %s (Element 6) "
> "is unknown: 0x%8.8" PRIx64 ".",
> @@ -333,7 +379,7 @@ static void method_test_BIX_return(
> }
> /* Design capacity low */
> if (obj->Package.Elements[7].Integer.Value > 0x7fffffff) {
> - fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + fwts_failed(fw, LOG_LEVEL_LOW,
> "Method_BIXDesignCapacityE7",
> "%s: %s (Element 7) "
> "is unknown: 0x%8.8" PRIx64 ".",
> @@ -355,6 +401,18 @@ static void method_test_BIX_return(
> }
>
> #endif
> +
> + /* Swapping Capability */
> + if (revision == 1 && obj->Package.Elements[20].Integer.Value > 2) {
> + fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + "Method_BIXSwappingCapability",
> + "%s: %s (Element 20) "
> + "is unknown: 0x%8.8" PRIx64 ".",
> + name, elements_v1[20].name,
> + obj->Package.Elements[20].Integer.Value);
> + failed = true;
> + }
> +
> if (failed)
> fwts_advice(fw,
> "Battery %s package contains errors. It is "
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 509424f..d0958f3 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -4047,7 +4047,9 @@ static void method_test_BIX_return(
> void *private)
> {
> bool failed = false;
> + uint64_t revision = 0;
>
> + /* Revision 0 in ACPI 5.x */
> static const fwts_package_element elements[] = {
> { ACPI_TYPE_INTEGER, "Revision" },
> { ACPI_TYPE_INTEGER, "Power Unit" },
> @@ -4071,16 +4073,60 @@ static void method_test_BIX_return(
> { ACPI_TYPE_STRING, "OEM Information" }
> };
>
> + /* Revision 1 in ACPI 6.x introduces swapping capability */
> + static const fwts_package_element elements_v1[] = {
> + { ACPI_TYPE_INTEGER, "Revision" },
> + { ACPI_TYPE_INTEGER, "Power Unit" },
> + { ACPI_TYPE_INTEGER, "Design Capacity" },
> + { ACPI_TYPE_INTEGER, "Last Full Charge Capacity" },
> + { ACPI_TYPE_INTEGER, "Battery Technology" },
> + { ACPI_TYPE_INTEGER, "Design Voltage" },
> + { ACPI_TYPE_INTEGER, "Design Capacity of Warning" },
> + { ACPI_TYPE_INTEGER, "Design Capactty of Low" },
Typo in Capacity ^^
> + { ACPI_TYPE_INTEGER, "Cycle Count" },
> + { ACPI_TYPE_INTEGER, "Measurement Accuracy" },
> + { ACPI_TYPE_INTEGER, "Max Sampling Time" },
> + { ACPI_TYPE_INTEGER, "Min Sampling Time" },
> + { ACPI_TYPE_INTEGER, "Max Averaging Interval" },
> + { ACPI_TYPE_INTEGER, "Min Averaging Interval" },
> + { ACPI_TYPE_INTEGER, "Battery Capacity Granularity 1" },
> + { ACPI_TYPE_INTEGER, "Battery Capacity Granularity 2" },
> + { ACPI_TYPE_STRING, "Model Number" },
> + { ACPI_TYPE_STRING, "Serial Number" },
> + { ACPI_TYPE_STRING, "Battery Type" },
> + { ACPI_TYPE_STRING, "OEM Information" },
> + { ACPI_TYPE_INTEGER, "Battery Swapping Capability" }
> + };
> +
> FWTS_UNUSED(private);
>
> if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)
> return;
>
> - if (fwts_method_package_count_equal(fw, name, "_BIX", obj, 20) != FWTS_OK)
> - return;
> + if (obj->Package.Count > 1 && obj->Package.Elements[0].Type == ACPI_TYPE_INTEGER)
> + revision = obj->Package.Elements[0].Integer.Value;
>
> - if (fwts_method_package_elements_type(fw, name, "_BIX", obj, elements, 20) != FWTS_OK)
> - return;
Again, switch statement is preferred on revision
> + if (revision == 0) {
> + if (fwts_method_package_count_equal(fw, name, "_BIX", obj, 20) != FWTS_OK)
> + return;
> +
> + if (fwts_method_package_elements_type(fw, name, "_BIX", obj, elements, 20) != FWTS_OK)
> + return;
> + } else if (revision == 1) {
> + if (fwts_method_package_count_equal(fw, name, "_BIX", obj, 21) != FWTS_OK)
> + return;
> +
> + if (fwts_method_package_elements_type(fw, name, "_BIX", obj, elements_v1, 21) != FWTS_OK)
> + return;
> + } else {
> + fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + "Method_BIXRevision",
> + "%s: Expected %s (Element 0) to be "
> + "0 or 1, got 0x%8.8" PRIx64 ".",
> + name, elements_v1[0].name,
> + (uint64_t)obj->Package.Elements[0].Integer.Value);
> + failed = true;
> + }
>
> /* Sanity check each field */
> /* Power Unit */
> @@ -4179,6 +4225,18 @@ static void method_test_BIX_return(
> }
>
> #endif
> +
> + /* Swapping Capability */
> + if (revision == 1 && obj->Package.Elements[20].Integer.Value > 2) {
> + fwts_failed(fw, LOG_LEVEL_CRITICAL,
> + "Method_BIXSwappingCapability",
> + "%s: %s (Element 20) "
> + "is unknown: 0x%8.8" PRIx64 ".",
> + name, elements_v1[20].name,
> + obj->Package.Elements[20].Integer.Value);
> + failed = true;
> + }
> +
> if (failed)
> fwts_advice(fw,
> "Battery %s package contains errors. It is "
>
More information about the fwts-devel
mailing list