ACK: [PATCH] acpi: update checks of _BIX return package

ivanhu ivan.hu at canonical.com
Tue Nov 14 10:07:47 UTC 2017



On 11/08/2017 04:40 PM, 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" },
> +		{ 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;
> +	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 "
>
> Acked-by:
>   			"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" },
> +		{ 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;
> +	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 "

Acked-by: Ivan Hu <ivan.hu at canonical.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20171114/7b667ec2/attachment-0001.html>


More information about the fwts-devel mailing list