ACK: [PATCH 3/4] acpi: move acpi battery common methods to lib

Colin Ian King colin.king at canonical.com
Thu Jan 21 17:42:37 UTC 2021


On 20/01/2021 23:23, Alex Hung wrote:
> move common functions in method.c and battery.c to lib
> 
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>  src/acpi/devices/battery/battery.c      | 434 +-----------------------
>  src/acpi/method/method.c                | 428 +----------------------
>  src/lib/include/fwts_acpi_object_eval.h |   5 +
>  src/lib/src/fwts_acpi_object_eval.c     | 423 +++++++++++++++++++++++
>  4 files changed, 439 insertions(+), 851 deletions(-)
> 
> diff --git a/src/acpi/devices/battery/battery.c b/src/acpi/devices/battery/battery.c
> index 049479fb..d71c7332 100644
> --- a/src/acpi/devices/battery/battery.c
> +++ b/src/acpi/devices/battery/battery.c
> @@ -77,130 +77,6 @@ static int acpi_battery_init(fwts_framework *fw)
>  	return FWTS_OK;
>  }
>  
> -static void method_test_BIF_return(
> -	fwts_framework *fw,
> -	char *name,
> -	ACPI_BUFFER *buf,
> -	ACPI_OBJECT *obj,
> -	void *private)
> -{
> -	bool failed = false;
> -
> -	static const fwts_package_element elements[] = {
> -		{ 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" },
> -		{ 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" }
> -	};
> -
> -	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, obj, 13) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_elements_type(fw, name, obj, elements) != FWTS_OK)
> -		return;
> -
> -	/* Sanity check each field */
> -	/* Power Unit */
> -	if (obj->Package.Elements[0].Integer.Value > 0x00000002) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIFBadUnits",
> -			"%s: Expected Power Unit (Element 0) to be "
> -			"0 (mWh) or 1 (mAh), got 0x%8.8" PRIx64 ".",
> -			name, (uint64_t)obj->Package.Elements[0].Integer.Value);
> -		failed = true;
> -	}
> -#ifdef FWTS_METHOD_PEDANDTIC
> -	/*
> -	 * 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[1].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIFBadCapacity",
> -			"%s: Design Capacity (Element 1) is "
> -			"unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[1].Integer.Value);
> -		failed = true;
> -	}
> -	/* Last Full Charge Capacity */
> -	if (obj->Package.Elements[2].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIFChargeCapacity",
> -			"%s: Last Full Charge Capacity (Element 2) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[2].Integer.Value);
> -		failed = true;
> -	}
> -#endif
> -	/* Battery Technology */
> -	if (obj->Package.Elements[3].Integer.Value > 0x00000002) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"Method_BIFBatTechUnit",
> -			"%s: Expected Battery Technology Unit "
> -			"(Element 3) to be 0 (Primary) or 1 "
> -			"(Secondary), got 0x%8.8" PRIx64 ".",
> -			name, (uint64_t)obj->Package.Elements[3].Integer.Value);
> -		failed = true;
> -	}
> -#ifdef FWTS_METHOD_PEDANDTIC
> -	/*
> -	 * 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[4].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIFDesignVoltage",
> -			"%s: Design Voltage (Element 4) is "
> -			"unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[4].Integer.Value);
> -		failed = true;
> -	}
> -	/* Design capacity warning */
> -	if (obj->Package.Elements[5].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIFDesignCapacityE5",
> -			"%s: Design Capacity Warning (Element 5) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[5].Integer.Value);
> -		failed = true;
> -	}
> -	/* Design capacity low */
> -	if (obj->Package.Elements[6].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIFDesignCapacityE6",
> -			"%s: Design Capacity Warning (Element 6) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[6].Integer.Value);
> -		failed = true;
> -	}
> -#endif
> -	if (failed)
> -		fwts_advice(fw,
> -			"Battery %s package contains errors. It is "
> -			"worth running the firmware test suite "
> -			"interactive 'battery' test to see if this "
> -			"is problematic.  This is a bug an needs to "
> -			"be fixed.", name);
> -	else
> -		fwts_method_passed_sane(fw, name, "package");
> -}
> -
>  static int method_test_BIF(fwts_framework *fw)
>  {
>  	ACPI_STATUS status;
> @@ -210,223 +86,12 @@ static int method_test_BIF(fwts_framework *fw)
>  	status = AcpiGetHandle (device, "_BIX", &method);
>  	if (ACPI_SUCCESS(status))
>  		return fwts_evaluate_method(fw, METHOD_OPTIONAL, &device,
> -			"_BIF", NULL, 0, method_test_BIF_return, NULL);
> +			"_BIF", NULL, 0, fwts_method_test_BIF_return, NULL);
>  	else
>  		return fwts_evaluate_method(fw, METHOD_MANDATORY, &device,
> -			"_BIF", NULL, 0, method_test_BIF_return, NULL);
> +			"_BIF", NULL, 0, fwts_method_test_BIF_return, NULL);
>  }
>  
> -static void method_test_BIX_return(
> -	fwts_framework *fw,
> -	char *name,
> -	ACPI_BUFFER *buf,
> -	ACPI_OBJECT *obj,
> -	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" },
> -		{ 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 Capacity of Low" },
> -		{ 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" }
> -	};
> -
> -	/* 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 Capacity of Low" },
> -		{ 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 (obj->Package.Count > 1 && obj->Package.Elements[0].Type == ACPI_TYPE_INTEGER)
> -		revision = obj->Package.Elements[0].Integer.Value;
> -
> -	switch (revision) {
> -	case 0:
> -		if (fwts_method_package_count_equal(fw, name, obj, 20) != FWTS_OK)
> -			return;
> -
> -		if (fwts_method_package_elements_type(fw, name, obj, elements) != FWTS_OK)
> -			return;
> -		break;
> -	case 1:
> -		if (fwts_method_package_count_equal(fw, name, obj, 21) != FWTS_OK)
> -			return;
> -
> -		if (fwts_method_package_elements_type(fw, name, obj, elements_v1) != FWTS_OK)
> -			return;
> -		break;
> -	default:
> -		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);
> -		return;
> -	}
> -
> -	/* Sanity check each field */
> -	/* Power Unit */
> -	if (obj->Package.Elements[1].Integer.Value > 0x00000002) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIXPowerUnit",
> -			"%s: Expected %s (Element 1) to be "
> -			"0 (mWh) or 1 (mAh), got 0x%8.8" PRIx64 ".",
> -			name, elements[1].name,
> -			(uint64_t)obj->Package.Elements[1].Integer.Value);
> -		failed = true;
> -	}
> -#ifdef FWTS_METHOD_PEDANDTIC
> -	/*
> -	 * 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,
> -			"Method_BIXDesignCapacity",
> -			"%s: %s (Element 2) is "
> -			"unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[2].name,
> -			obj->Package.Elements[2].Integer.Value);
> -		failed = true;
> -	}
> -	/* Last Full Charge Capacity */
> -	if (obj->Package.Elements[3].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIXFullChargeCapacity",
> -			"%s: %s (Element 3) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[3].name,
> -			obj->Package.Elements[3].Integer.Value);
> -		failed = true;
> -	}
> -#endif
> -	/* Battery Technology */
> -	if (obj->Package.Elements[4].Integer.Value > 0x00000002) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"Method_BIXBatteryTechUnit",
> -			"%s: %s "
> -			"(Element 4) to be 0 (Primary) or 1 "
> -			"(Secondary), got 0x%8.8" PRIx64 ".",
> -			name, elements[4].name,
> -			(uint64_t)obj->Package.Elements[4].Integer.Value);
> -		failed = true;
> -	}
> -#ifdef FWTS_METHOD_PEDANDTIC
> -	/*
> -	 * 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,
> -			"Method_BIXDesignVoltage",
> -			"%s: %s (Element 5) is unknown: "
> -			"0x%8.8" PRIx64 ".",
> -			name, elements[5].name,
> -			obj->Package.Elements[5].Integer.Value);
> -		failed = true;
> -	}
> -	/* Design capacity warning */
> -	if (obj->Package.Elements[6].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIXDesignCapacityE6",
> -			"%s: %s (Element 6) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[6].name,
> -			obj->Package.Elements[6].Integer.Value);
> -		failed = true;
> -	}
> -	/* Design capacity low */
> -	if (obj->Package.Elements[7].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIXDesignCapacityE7",
> -			"%s: %s (Element 7) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[7].name,
> -			obj->Package.Elements[7].Integer.Value);
> -		failed = true;
> -	}
> -	/* Cycle Count: value = 0 ~ 0xFFFFFFFE or 0xFFFFFFFF (unknown) */
> -
> -	/* Measurement Accuracy */
> -	if (obj->Package.Elements[9].Integer.Value > 100000) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIXMeasurementAccuracy",
> -			"%s: %s (Element 9) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[9].name,
> -			obj->Package.Elements[9].Integer.Value);
> -		failed = true;
> -	}
> -
> -#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 "
> -			"worth running the firmware test suite "
> -			"interactive 'battery' test to see if this "
> -			"is problematic.  This is a bug an needs to "
> -			"be fixed.", name);
> -	else
> -		fwts_method_passed_sane(fw, name, "package");
> -}
>  
>  static int method_test_BIX(fwts_framework *fw)
>  {
> @@ -437,10 +102,10 @@ static int method_test_BIX(fwts_framework *fw)
>  	status = AcpiGetHandle (device, "_BIF", &method);
>  	if (ACPI_SUCCESS(status))
>  		return fwts_evaluate_method(fw, METHOD_OPTIONAL, &device,
> -			"_BIX", NULL, 0, method_test_BIX_return, NULL);
> +			"_BIX", NULL, 0, fwts_method_test_BIX_return, NULL);
>  	else
>  		return fwts_evaluate_method(fw, METHOD_MANDATORY, &device,
> -			"_BIX", NULL, 0, method_test_BIX_return, NULL);
> +			"_BIX", NULL, 0, fwts_method_test_BIX_return, NULL);
>  }
>  
>  static int method_test_BMA(fwts_framework *fw)
> @@ -463,64 +128,11 @@ static int method_test_BMS(fwts_framework *fw)
>  		"_BMS", arg, 1, fwts_method_test_passed_failed_return, "_BMS");
>  }
>  
> -static void method_test_BST_return(
> -	fwts_framework *fw,
> -	char *name,
> -	ACPI_BUFFER *buf,
> -	ACPI_OBJECT *obj,
> -	void *private)
> -{
> -	bool failed = false;
> -
> -	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, obj, 4) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_elements_all_type(fw, name, obj, ACPI_TYPE_INTEGER) != FWTS_OK)
> -		return;
> -
> -	/* Sanity check each field */
> -	/* Battery State */
> -	if ((obj->Package.Elements[0].Integer.Value) > 7) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BSTBadState",
> -			"%s: Expected Battery State (Element 0) to "
> -			"be 0..7, got 0x%8.8" PRIx64 ".",
> -			name, (uint64_t)obj->Package.Elements[0].Integer.Value);
> -		failed = true;
> -	}
> -	/* Ensure bits 0 (discharging) and 1 (charging) are not both set, see 10.2.2.6 */
> -	if (((obj->Package.Elements[0].Integer.Value) & 3) == 3) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BSTBadState",
> -			"%s: Battery State (Element 0) is "
> -			"indicating both charging and discharginng "
> -			"which is not allowed. Got value 0x%8.8" PRIx64 ".",
> -			name, (uint64_t)obj->Package.Elements[0].Integer.Value);
> -		failed = true;
> -	}
> -	/* Battery Present Rate - cannot check, pulled from EC */
> -	/* Battery Remaining Capacity - cannot check, pulled from EC */
> -	/* Battery Present Voltage - cannot check, pulled from EC */
> -	if (failed)
> -		fwts_advice(fw,
> -			"Battery %s package contains errors. It is "
> -			"worth running the firmware test suite "
> -			"interactive 'battery' test to see if this "
> -			"is problematic.  This is a bug an needs to "
> -			"be fixed.", name);
> -		else
> -			fwts_method_passed_sane(fw, name, "package");
> -}
>  
>  static int method_test_BST(fwts_framework *fw)
>  {
>  	return fwts_evaluate_method(fw, METHOD_MANDATORY, &device,
> -		"_BST", NULL, 0, method_test_BST_return, NULL);
> +		"_BST", NULL, 0, fwts_method_test_BST_return, NULL);
>  }
>  
>  static int method_test_BTH(fwts_framework *fw)
> @@ -558,7 +170,6 @@ static int method_test_BTP(fwts_framework *fw)
>  	return FWTS_OK;
>  }
>  
> -
>  static void method_test_PCL_return(fwts_framework *fw,
>  	char *name,
>  	ACPI_BUFFER *buf,
> @@ -582,43 +193,10 @@ static int method_test_PCL(fwts_framework *fw)
>  		 "_PCL", NULL, 0, method_test_PCL_return, NULL);
>  }
>  
> -static void method_test_STA_return(
> -	fwts_framework *fw,
> -	char *name,
> -	ACPI_BUFFER *buf,
> -	ACPI_OBJECT *obj,
> -	void *private)
> -{
> -	bool failed = false;
> -
> -	FWTS_UNUSED(private);
> -
> -	if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) != FWTS_OK)
> -		return;
> -
> -	if ((obj->Integer.Value & 3) == 2) {
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"Method_STAEnabledNotPresent",
> -			"%s indicates that the device is enabled "
> -			"but not present, which is impossible.", name);
> -		failed = true;
> -	}
> -	if ((obj->Integer.Value & ~0x1f) != 0) {
> -		fwts_failed(fw, LOG_LEVEL_HIGH,
> -			"Method_STAReservedBitsSet",
> -			"%s is returning non-zero reserved "
> -			"bits 5-31. These should be zero.", name);
> -		failed = true;
> -	}
> -
> -	if (!failed)
> -		fwts_method_passed_sane_uint64(fw, name, obj->Integer.Value);
> -}
> -
>  static int method_test_STA(fwts_framework *fw)
>  {
>  	return fwts_evaluate_method(fw, METHOD_OPTIONAL, &device,
> -		"_STA", NULL, 0, method_test_STA_return, NULL);
> +		"_STA", NULL, 0, fwts_method_test_STA_return, NULL);
>  }
>  
>  static int method_test_BTM(fwts_framework *fw)
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 204cff9d..f4b24bb1 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -1160,42 +1160,14 @@ static int method_test_RMV(fwts_framework *fw)
>  		NULL, 0, fwts_method_test_passed_failed_return, "_RMV");
>  }
>  
> -static void method_test_STA_return(
> -	fwts_framework *fw,
> -	char *name,
> -	ACPI_BUFFER *buf,
> -	ACPI_OBJECT *obj,
> -	void *private)
> -{
> -	bool passed = true;
> -
> -	FWTS_UNUSED(private);
> -
> -	if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) != FWTS_OK)
> -		return;
> -
> -	if ((obj->Integer.Value & 3) == 2) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"Method_STAEnabledNotPresent",
> -			"%s indicates that the device is enabled "
> -			"but not present, which is impossible.", name);
> -		passed = false;
> -	}
> -	fwts_acpi_reserved_bits_check(fw, "_STA", "Reserved Bits",
> -		obj->Integer.Value, sizeof(uint32_t), 5, 31, &passed);
> -
> -	if (passed)
> -		fwts_method_passed_sane_uint64(fw, name, obj->Integer.Value);
> -}
> -
>  static int method_test_STA(fwts_framework *fw)
>  {
>  	if (fw->flags & FWTS_FLAG_TEST_SBBR)
>  		return method_evaluate_method(fw, METHOD_MANDATORY, "_STA",
> -			NULL, 0, method_test_STA_return, "_STA");
> +			NULL, 0, fwts_method_test_STA_return, "_STA");
>  	else
>  		return method_evaluate_method(fw, METHOD_OPTIONAL, "_STA",
> -			NULL, 0, method_test_STA_return, "_STA");
> +			NULL, 0, fwts_method_test_STA_return, "_STA");
>  }
>  
>  
> @@ -3445,352 +3417,16 @@ static int method_test_BCT(fwts_framework *fw)
>  		"_BCT", arg, 1, fwts_method_test_integer_return, NULL);
>  }
>  
> -static void method_test_BIF_return(
> -	fwts_framework *fw,
> -	char *name,
> -	ACPI_BUFFER *buf,
> -	ACPI_OBJECT *obj,
> -	void *private)
> -{
> -	bool failed = false;
> -
> -	static const fwts_package_element elements[] = {
> -		{ 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" },
> -		{ 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" }
> -	};
> -
> -	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, obj, 13) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_elements_type(fw, name, obj, elements) != FWTS_OK)
> -		return;
> -
> -	/* Sanity check each field */
> -	/* Power Unit */
> -	if (obj->Package.Elements[0].Integer.Value > 0x00000002) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"Method_BIFBadUnits",
> -			"%s: Expected Power Unit (Element 0) to be "
> -			"0 (mWh) or 1 (mAh), got 0x%8.8" PRIx64 ".",
> -			name, (uint64_t)obj->Package.Elements[0].Integer.Value);
> -		failed = true;
> -	}
> -#ifdef FWTS_METHOD_PEDANDTIC
> -	/*
> -	 * 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[1].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"Method_BIFBadCapacity",
> -			"%s: Design Capacity (Element 1) is "
> -			"unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[1].Integer.Value);
> -		failed = true;
> -	}
> -	/* Last Full Charge Capacity */
> -	if (obj->Package.Elements[2].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"Method_BIFChargeCapacity",
> -			"%s: Last Full Charge Capacity (Element 2) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[2].Integer.Value);
> -		failed = true;
> -	}
> -#endif
> -	/* Battery Technology */
> -	if (obj->Package.Elements[3].Integer.Value > 0x00000002) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"Method_BIFBatTechUnit",
> -			"%s: Expected Battery Technology Unit "
> -			"(Element 3) to be 0 (Primary) or 1 "
> -			"(Secondary), got 0x%8.8" PRIx64 ".",
> -			name, (uint64_t)obj->Package.Elements[3].Integer.Value);
> -		failed = true;
> -	}
> -#ifdef FWTS_METHOD_PEDANDTIC
> -	/*
> - 	 * 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[4].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"Method_BIFDesignVoltage",
> -			"%s: Design Voltage (Element 4) is "
> -			"unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[4].Integer.Value);
> -		failed = true;
> -	}
> -	/* Design capacity warning */
> -	if (obj->Package.Elements[5].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"Method_BIFDesignCapacityE5",
> -			"%s: Design Capacity Warning (Element 5) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[5].Integer.Value);
> -		failed = true;
> -	}
> -	/* Design capacity low */
> -	if (obj->Package.Elements[6].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"Method_BIFDesignCapacityE6",
> -			"%s: Design Capacity Warning (Element 6) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, obj->Package.Elements[6].Integer.Value);
> -		failed = true;
> -	}
> -#endif
> -	if (failed)
> -		fwts_advice(fw,
> -			"Battery %s package contains errors. It is "
> -			"worth running the firmware test suite "
> -			"interactive 'battery' test to see if this "
> -			"is problematic.  This is a bug an needs to "
> -			"be fixed.", name);
> -	else
> -		fwts_method_passed_sane(fw, name, "package");
> -}
> -
>  static int method_test_BIF(fwts_framework *fw)
>  {
>  	return method_evaluate_method(fw, METHOD_MOBILE,
> -		"_BIF", NULL, 0, method_test_BIF_return, NULL);
> -}
> -
> -static void method_test_BIX_return(
> -	fwts_framework *fw,
> -	char *name,
> -	ACPI_BUFFER *buf,
> -	ACPI_OBJECT *obj,
> -	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" },
> -		{ 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 Capacity of Low" },
> -		{ 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" }
> -	};
> -
> -	/* 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 Capacity of Low" },
> -		{ 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 (obj->Package.Count > 1 && obj->Package.Elements[0].Type == ACPI_TYPE_INTEGER)
> -		revision = obj->Package.Elements[0].Integer.Value;
> -
> -	switch (revision) {
> -	case 0:
> -		if (fwts_method_package_count_equal(fw, name, obj, 20) != FWTS_OK)
> -			return;
> -
> -		if (fwts_method_package_elements_type(fw, name, obj, elements) != FWTS_OK)
> -			return;
> -		break;
> -	case 1:
> -		if (fwts_method_package_count_equal(fw, name, obj, 21) != FWTS_OK)
> -			return;
> -
> -		if (fwts_method_package_elements_type(fw, name, obj, elements_v1) != FWTS_OK)
> -			return;
> -		break;
> -	default:
> -		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);
> -		return;
> -	}
> -
> -	/* Sanity check each field */
> -	/* Power Unit */
> -	if (obj->Package.Elements[1].Integer.Value > 0x00000002) {
> -		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 ".",
> -			name, elements[1].name,
> -			(uint64_t)obj->Package.Elements[1].Integer.Value);
> -		failed = true;
> -	}
> -#ifdef FWTS_METHOD_PEDANDTIC
> -	/*
> -	 * 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_LOW,
> -			"Method_BIXDesignCapacity",
> -			"%s: %s (Element 2) is "
> -			"unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[2].name,
> -			obj->Package.Elements[2].Integer.Value);
> -		failed = true;
> -	}
> -	/* Last Full Charge Capacity */
> -	if (obj->Package.Elements[3].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"Method_BIXFullChargeCapacity",
> -			"%s: %s (Element 3) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[3].name,
> -			obj->Package.Elements[3].Integer.Value);
> -		failed = true;
> -	}
> -#endif
> -	/* Battery Technology */
> -	if (obj->Package.Elements[4].Integer.Value > 0x00000002) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"Method_BIXBatteryTechUnit",
> -			"%s: %s "
> -			"(Element 4) to be 0 (Primary) or 1 "
> -			"(Secondary), got 0x%8.8" PRIx64 ".",
> -			name, elements[4].name,
> -			(uint64_t)obj->Package.Elements[4].Integer.Value);
> -		failed = true;
> -	}
> -#ifdef FWTS_METHOD_PEDANDTIC
> -	/*
> -	 * 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_LOW,
> -			"Method_BIXDesignVoltage",
> -			"%s: %s (Element 5) is unknown: "
> -			"0x%8.8" PRIx64 ".",
> -			name, elements[5].name,
> -			obj->Package.Elements[5].Integer.Value);
> -		failed = true;
> -	}
> -	/* Design capacity warning */
> -	if (obj->Package.Elements[6].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"Method_BIXDesignCapacityE6",
> -			"%s: %s (Element 6) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[6].name,
> -			obj->Package.Elements[6].Integer.Value);
> -		failed = true;
> -	}
> -	/* Design capacity low */
> -	if (obj->Package.Elements[7].Integer.Value > 0x7fffffff) {
> -		fwts_failed(fw, LOG_LEVEL_LOW,
> -			"Method_BIXDesignCapacityE7",
> -			"%s: %s (Element 7) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[7].name,
> -			obj->Package.Elements[7].Integer.Value);
> -		failed = true;
> -	}
> -	/* Cycle Count: value = 0 ~ 0xFFFFFFFE or 0xFFFFFFFF (unknown) */
> -
> -	/* Measurement Accuracy */
> -	if (obj->Package.Elements[9].Integer.Value > 100000) {
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BIXMeasurementAccuracy",
> -			"%s: %s (Element 9) "
> -			"is unknown: 0x%8.8" PRIx64 ".",
> -			name, elements[9].name,
> -			obj->Package.Elements[9].Integer.Value);
> -		failed = true;
> -	}
> -
> -#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 "
> -			"worth running the firmware test suite "
> -			"interactive 'battery' test to see if this "
> -			"is problematic.  This is a bug an needs to "
> -			"be fixed.", name);
> -	else
> -		fwts_method_passed_sane(fw, name, "package");
> +		"_BIF", NULL, 0, fwts_method_test_BIF_return, NULL);
>  }
>  
>  static int method_test_BIX(fwts_framework *fw)
>  {
>  	return method_evaluate_method(fw, METHOD_MOBILE,
> -		"_BIX", NULL, 0, method_test_BIX_return, NULL);
> +		"_BIX", NULL, 0, fwts_method_test_BIX_return, NULL);
>  }
>  
>  static int method_test_BMA(fwts_framework *fw)
> @@ -3813,64 +3449,10 @@ static int method_test_BMS(fwts_framework *fw)
>  		"_BMS", arg, 1, fwts_method_test_passed_failed_return, "_BMS");
>  }
>  
> -static void method_test_BST_return(
> -	fwts_framework *fw,
> -	char *name,
> -	ACPI_BUFFER *buf,
> -	ACPI_OBJECT *obj,
> -	void *private)
> -{
> -	bool failed = false;
> -
> -	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, obj, 4) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_elements_all_type(fw, name, obj, ACPI_TYPE_INTEGER) != FWTS_OK)
> -		return;
> -
> -	/* Sanity check each field */
> -	/* Battery State */
> -	if ((obj->Package.Elements[0].Integer.Value) > 7) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"Method_BSTBadState",
> -			"%s: Expected Battery State (Element 0) to "
> -			"be 0..7, got 0x%8.8" PRIx64 ".",
> -			name, (uint64_t)obj->Package.Elements[0].Integer.Value);
> -		failed = true;
> -	}
> -	/* Ensure bits 0 (discharging) and 1 (charging) are not both set, see 10.2.2.6 */
> -	if (((obj->Package.Elements[0].Integer.Value) & 3) == 3) {
> -		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -			"Method_BSTBadState",
> -			"%s: Battery State (Element 0) is "
> -			"indicating both charging and discharginng "
> -			"which is not allowed. Got value 0x%8.8" PRIx64 ".",
> -			name, (uint64_t)obj->Package.Elements[0].Integer.Value);
> -		failed = true;
> -	}
> -	/* Battery Present Rate - cannot check, pulled from EC */
> -	/* Battery Remaining Capacity - cannot check, pulled from EC */
> -	/* Battery Present Voltage - cannot check, pulled from EC */
> -	if (failed)
> -		fwts_advice(fw,
> -			"Battery %s package contains errors. It is "
> -			"worth running the firmware test suite "
> -			"interactive 'battery' test to see if this "
> -			"is problematic.  This is a bug an needs to "
> -			"be fixed.", name);
> -		else
> -			fwts_method_passed_sane(fw, name, "package");
> -}
> -
>  static int method_test_BST(fwts_framework *fw)
>  {
>  	return method_evaluate_method(fw, METHOD_MOBILE,
> -		"_BST", NULL, 0, method_test_BST_return, NULL);
> +		"_BST", NULL, 0, fwts_method_test_BST_return, NULL);
>  }
>  
>  static int method_test_BTP(fwts_framework *fw)
> diff --git a/src/lib/include/fwts_acpi_object_eval.h b/src/lib/include/fwts_acpi_object_eval.h
> index 34660286..996de1fe 100644
> --- a/src/lib/include/fwts_acpi_object_eval.h
> +++ b/src/lib/include/fwts_acpi_object_eval.h
> @@ -178,4 +178,9 @@ void fwts_method_test_NIC_return(fwts_framework *fw, char *name, ACPI_BUFFER *bu
>  void fwts_method_test_NIH_return(fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
>  void fwts_method_test_NIG_return(fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
>  
> +void fwts_method_test_STA_return(fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
> +void fwts_method_test_BIF_return(fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
> +void fwts_method_test_BIX_return(fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
> +void fwts_method_test_BST_return(fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
> +
>  #endif
> diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c
> index a9ee84b3..fbca0d46 100644
> --- a/src/lib/src/fwts_acpi_object_eval.c
> +++ b/src/lib/src/fwts_acpi_object_eval.c
> @@ -2647,4 +2647,427 @@ void fwts_method_test_NIG_return(
>  		fwts_method_passed_sane(fw, name, "buffer");
>  }
>  
> +void fwts_method_test_STA_return(
> +	fwts_framework *fw,
> +	char *name,
> +	ACPI_BUFFER *buf,
> +	ACPI_OBJECT *obj,
> +	void *private)
> +{
> +	bool passed = true;
> +
> +	FWTS_UNUSED(private);
> +
> +	if (fwts_method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) != FWTS_OK)
> +		return;
> +
> +	if ((obj->Integer.Value & 3) == 2) {
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +			"Method_STAEnabledNotPresent",
> +			"%s indicates that the device is enabled "
> +			"but not present, which is impossible.", name);
> +		passed = false;
> +	}
> +	fwts_acpi_reserved_bits_check(fw, "_STA", "Reserved Bits",
> +		obj->Integer.Value, sizeof(uint32_t), 5, 31, &passed);
> +
> +	if (passed)
> +		fwts_method_passed_sane_uint64(fw, name, obj->Integer.Value);
> +}
> +
> +/*
> + *  Section 10.2 Control Method Batteries
> + */
> +
> +void fwts_method_test_BIF_return(
> +	fwts_framework *fw,
> +	char *name,
> +	ACPI_BUFFER *buf,
> +	ACPI_OBJECT *obj,
> +	void *private)
> +{
> +	bool failed = false;
> +
> +	static const fwts_package_element elements[] = {
> +		{ 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" },
> +		{ 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" }
> +	};
> +
> +	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, obj, 13) != FWTS_OK)
> +		return;
> +
> +	if (fwts_method_package_elements_type(fw, name, obj, elements) != FWTS_OK)
> +		return;
> +
> +	/* Sanity check each field */
> +	/* Power Unit */
> +	if (obj->Package.Elements[0].Integer.Value > 0x00000002) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIFBadUnits",
> +			"%s: Expected Power Unit (Element 0) to be "
> +			"0 (mWh) or 1 (mAh), got 0x%8.8" PRIx64 ".",
> +			name, (uint64_t)obj->Package.Elements[0].Integer.Value);
> +		failed = true;
> +	}
> +#ifdef FWTS_METHOD_PEDANDTIC
> +	/*
> +	 * 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[1].Integer.Value > 0x7fffffff) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIFBadCapacity",
> +			"%s: Design Capacity (Element 1) is "
> +			"unknown: 0x%8.8" PRIx64 ".",
> +			name, obj->Package.Elements[1].Integer.Value);
> +		failed = true;
> +	}
> +	/* Last Full Charge Capacity */
> +	if (obj->Package.Elements[2].Integer.Value > 0x7fffffff) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIFChargeCapacity",
> +			"%s: Last Full Charge Capacity (Element 2) "
> +			"is unknown: 0x%8.8" PRIx64 ".",
> +			name, obj->Package.Elements[2].Integer.Value);
> +		failed = true;
> +	}
> +#endif
> +	/* Battery Technology */
> +	if (obj->Package.Elements[3].Integer.Value > 0x00000002) {
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +			"Method_BIFBatTechUnit",
> +			"%s: Expected Battery Technology Unit "
> +			"(Element 3) to be 0 (Primary) or 1 "
> +			"(Secondary), got 0x%8.8" PRIx64 ".",
> +			name, (uint64_t)obj->Package.Elements[3].Integer.Value);
> +		failed = true;
> +	}
> +#ifdef FWTS_METHOD_PEDANDTIC
> +	/*
> +	 * 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[4].Integer.Value > 0x7fffffff) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIFDesignVoltage",
> +			"%s: Design Voltage (Element 4) is "
> +			"unknown: 0x%8.8" PRIx64 ".",
> +			name, obj->Package.Elements[4].Integer.Value);
> +		failed = true;
> +	}
> +	/* Design capacity warning */
> +	if (obj->Package.Elements[5].Integer.Value > 0x7fffffff) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIFDesignCapacityE5",
> +			"%s: Design Capacity Warning (Element 5) "
> +			"is unknown: 0x%8.8" PRIx64 ".",
> +			name, obj->Package.Elements[5].Integer.Value);
> +		failed = true;
> +	}
> +	/* Design capacity low */
> +	if (obj->Package.Elements[6].Integer.Value > 0x7fffffff) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIFDesignCapacityE6",
> +			"%s: Design Capacity Warning (Element 6) "
> +			"is unknown: 0x%8.8" PRIx64 ".",
> +			name, obj->Package.Elements[6].Integer.Value);
> +		failed = true;
> +	}
> +#endif
> +	if (failed)
> +		fwts_advice(fw,
> +			"Battery %s package contains errors. It is "
> +			"worth running the firmware test suite "
> +			"interactive 'battery' test to see if this "
> +			"is problematic.  This is a bug an needs to "
> +			"be fixed.", name);
> +	else
> +		fwts_method_passed_sane(fw, name, "package");
> +}
> +
> +void fwts_method_test_BIX_return(
> +	fwts_framework *fw,
> +	char *name,
> +	ACPI_BUFFER *buf,
> +	ACPI_OBJECT *obj,
> +	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" },
> +		{ 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 Capacity of Low" },
> +		{ 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" }
> +	};
> +
> +	/* 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 Capacity of Low" },
> +		{ 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 (obj->Package.Count > 1 && obj->Package.Elements[0].Type == ACPI_TYPE_INTEGER)
> +		revision = obj->Package.Elements[0].Integer.Value;
> +
> +	switch (revision) {
> +	case 0:
> +		if (fwts_method_package_count_equal(fw, name, obj, 20) != FWTS_OK)
> +			return;
> +
> +		if (fwts_method_package_elements_type(fw, name, obj, elements) != FWTS_OK)
> +			return;
> +		break;
> +	case 1:
> +		if (fwts_method_package_count_equal(fw, name, obj, 21) != FWTS_OK)
> +			return;
> +
> +		if (fwts_method_package_elements_type(fw, name, obj, elements_v1) != FWTS_OK)
> +			return;
> +		break;
> +	default:
> +		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);
> +		return;
> +	}
> +
> +	/* Sanity check each field */
> +	/* Power Unit */
> +	if (obj->Package.Elements[1].Integer.Value > 0x00000002) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIXPowerUnit",
> +			"%s: Expected %s (Element 1) to be "
> +			"0 (mWh) or 1 (mAh), got 0x%8.8" PRIx64 ".",
> +			name, elements[1].name,
> +			(uint64_t)obj->Package.Elements[1].Integer.Value);
> +		failed = true;
> +	}
> +#ifdef FWTS_METHOD_PEDANDTIC
> +	/*
> +	 * 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,
> +			"Method_BIXDesignCapacity",
> +			"%s: %s (Element 2) is "
> +			"unknown: 0x%8.8" PRIx64 ".",
> +			name, elements[2].name,
> +			obj->Package.Elements[2].Integer.Value);
> +		failed = true;
> +	}
> +	/* Last Full Charge Capacity */
> +	if (obj->Package.Elements[3].Integer.Value > 0x7fffffff) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIXFullChargeCapacity",
> +			"%s: %s (Element 3) "
> +			"is unknown: 0x%8.8" PRIx64 ".",
> +			name, elements[3].name,
> +			obj->Package.Elements[3].Integer.Value);
> +		failed = true;
> +	}
> +#endif
> +	/* Battery Technology */
> +	if (obj->Package.Elements[4].Integer.Value > 0x00000002) {
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +			"Method_BIXBatteryTechUnit",
> +			"%s: %s "
> +			"(Element 4) to be 0 (Primary) or 1 "
> +			"(Secondary), got 0x%8.8" PRIx64 ".",
> +			name, elements[4].name,
> +			(uint64_t)obj->Package.Elements[4].Integer.Value);
> +		failed = true;
> +	}
> +#ifdef FWTS_METHOD_PEDANDTIC
> +	/*
> +	 * 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,
> +			"Method_BIXDesignVoltage",
> +			"%s: %s (Element 5) is unknown: "
> +			"0x%8.8" PRIx64 ".",
> +			name, elements[5].name,
> +			obj->Package.Elements[5].Integer.Value);
> +		failed = true;
> +	}
> +	/* Design capacity warning */
> +	if (obj->Package.Elements[6].Integer.Value > 0x7fffffff) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIXDesignCapacityE6",
> +			"%s: %s (Element 6) "
> +			"is unknown: 0x%8.8" PRIx64 ".",
> +			name, elements[6].name,
> +			obj->Package.Elements[6].Integer.Value);
> +		failed = true;
> +	}
> +	/* Design capacity low */
> +	if (obj->Package.Elements[7].Integer.Value > 0x7fffffff) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIXDesignCapacityE7",
> +			"%s: %s (Element 7) "
> +			"is unknown: 0x%8.8" PRIx64 ".",
> +			name, elements[7].name,
> +			obj->Package.Elements[7].Integer.Value);
> +		failed = true;
> +	}
> +	/* Cycle Count: value = 0 ~ 0xFFFFFFFE or 0xFFFFFFFF (unknown) */
> +
> +	/* Measurement Accuracy */
> +	if (obj->Package.Elements[9].Integer.Value > 100000) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BIXMeasurementAccuracy",
> +			"%s: %s (Element 9) "
> +			"is unknown: 0x%8.8" PRIx64 ".",
> +			name, elements[9].name,
> +			obj->Package.Elements[9].Integer.Value);
> +		failed = true;
> +	}
> +
> +#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 "
> +			"worth running the firmware test suite "
> +			"interactive 'battery' test to see if this "
> +			"is problematic.  This is a bug an needs to "
> +			"be fixed.", name);
> +	else
> +		fwts_method_passed_sane(fw, name, "package");
> +}
> +
> +void fwts_method_test_BST_return(
> +	fwts_framework *fw,
> +	char *name,
> +	ACPI_BUFFER *buf,
> +	ACPI_OBJECT *obj,
> +	void *private)
> +{
> +	bool failed = false;
> +
> +	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, obj, 4) != FWTS_OK)
> +		return;
> +
> +	if (fwts_method_package_elements_all_type(fw, name, obj, ACPI_TYPE_INTEGER) != FWTS_OK)
> +		return;
> +
> +	/* Sanity check each field */
> +	/* Battery State */
> +	if ((obj->Package.Elements[0].Integer.Value) > 7) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BSTBadState",
> +			"%s: Expected Battery State (Element 0) to "
> +			"be 0..7, got 0x%8.8" PRIx64 ".",
> +			name, (uint64_t)obj->Package.Elements[0].Integer.Value);
> +		failed = true;
> +	}
> +	/* Ensure bits 0 (discharging) and 1 (charging) are not both set, see 10.2.2.6 */
> +	if (((obj->Package.Elements[0].Integer.Value) & 3) == 3) {
> +		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +			"Method_BSTBadState",
> +			"%s: Battery State (Element 0) is "
> +			"indicating both charging and discharginng "
> +			"which is not allowed. Got value 0x%8.8" PRIx64 ".",
> +			name, (uint64_t)obj->Package.Elements[0].Integer.Value);
> +		failed = true;
> +	}
> +	/* Battery Present Rate - cannot check, pulled from EC */
> +	/* Battery Remaining Capacity - cannot check, pulled from EC */
> +	/* Battery Present Voltage - cannot check, pulled from EC */
> +	if (failed)
> +		fwts_advice(fw,
> +			"Battery %s package contains errors. It is "
> +			"worth running the firmware test suite "
> +			"interactive 'battery' test to see if this "
> +			"is problematic.  This is a bug an needs to "
> +			"be fixed.", name);
> +		else
> +			fwts_method_passed_sane(fw, name, "package");
> +}
> +
> +
>  #endif
> 
Makes sense.

Acked-by: Colin Ian King <colin.king at canonical.com>



More information about the fwts-devel mailing list