ACK: [PATCH 2/2] acpi/method: remove duplicated code by a common function

Colin Ian King colin.king at canonical.com
Tue May 26 15:47:28 UTC 2020


On 15/05/2020 19:00, Alex Hung wrote:
> _CLS, _HPP, _LSI, _MBM, _FIF and _FST all returns a package containing
> all integer elements. Each individual function is placed by the new
> fwts_method_test_package_integer_return function.
> 
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>  src/acpi/method/method.c                | 158 ++----------------------
>  src/lib/include/fwts_acpi_object_eval.h |   1 -
>  src/lib/src/fwts_acpi_object_eval.c     |  24 +---
>  3 files changed, 14 insertions(+), 169 deletions(-)
> 
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 254e1519..76ce8a81 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -750,14 +750,14 @@ static int method_test_PIC(fwts_framework *fw)
>  	return ret;
>  }
>  
> -
>  /*
>   * Section 6.1 Device Identification Objects
>   */
>  static int method_test_CLS(fwts_framework *fw)
>  {
> +	uint32_t element_size = 3;
>  	return method_evaluate_method(fw, METHOD_OPTIONAL,
> -		"_CLS", NULL, 0, fwts_method_test_CLS_return, NULL);
> +		"_CLS", NULL, 0, fwts_method_test_package_integer_return, &element_size);
>  }
>  
>  static int method_test_DDN(fwts_framework *fw)
> @@ -1087,33 +1087,11 @@ static int method_test_GSB(fwts_framework *fw)
>  		"_GSB", NULL, 0, fwts_method_test_integer_return, NULL);
>  }
>  
> -static void method_test_HPP_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_PACKAGE) != FWTS_OK)
> -		return;
> -
> -	/* Must be 4 elements in the package */
> -	if (fwts_method_package_count_equal(fw, name, "_HPP", obj, 4) != FWTS_OK)
> -		return;
> -
> -	/* All 4 elements in the package must be integers */
> -	if (fwts_method_package_elements_all_type(fw, name, "_HPP", obj, ACPI_TYPE_INTEGER) != FWTS_OK)
> -		return;
> -
> -	fwts_method_passed_sane(fw, name, "package");
> -}
> -
>  static int method_test_HPP(fwts_framework *fw)
>  {
> +	uint32_t element_size = 4;
>  	return method_evaluate_method(fw, METHOD_OPTIONAL,
> -		"_HPP", NULL, 0, method_test_HPP_return, NULL);
> +		"_HPP", NULL, 0, fwts_method_test_package_integer_return, &element_size);
>  }
>  
>  static int method_test_PXM(fwts_framework *fw)
> @@ -1255,28 +1233,11 @@ static int method_test_FIT(fwts_framework *fw)
>  		"_FIT", NULL, 0, fwts_method_test_buffer_return, NULL);
>  }
>  
> -static void method_test_LSI_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_PACKAGE) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_elements_all_type(fw, name, "_LSI", obj, ACPI_TYPE_INTEGER) != FWTS_OK)
> -		return;
> -
> -	fwts_method_passed_sane(fw, name, "package");
> -}
> -
>  static int method_test_LSI(fwts_framework *fw)
>  {
> +	uint32_t element_size = 3;
>  	return method_evaluate_method(fw, METHOD_OPTIONAL,
> -		"_LSI", NULL, 0, method_test_LSI_return, NULL);
> +		"_LSI", NULL, 0, fwts_method_test_package_integer_return, &element_size);
>  }
>  
>  static int method_test_DCK(fwts_framework *fw)
> @@ -3193,44 +3154,11 @@ static int method_test_GTM(fwts_framework *fw)
>  /*
>   * Section 9.12 Memory Devices
>   */
> -
> -static void method_test_MBM_return(
> -	fwts_framework *fw,
> -	char *name,
> -	ACPI_BUFFER *buf,
> -	ACPI_OBJECT *obj,
> -	void *private)
> -{
> -	static const fwts_package_element elements[] = {
> -		{ ACPI_TYPE_INTEGER,	"Revision" },
> -		{ ACPI_TYPE_INTEGER,	"Window Size" },
> -		{ ACPI_TYPE_INTEGER,	"Sampling Interval" },
> -		{ ACPI_TYPE_INTEGER,	"Maximum Bandwidth" },
> -		{ ACPI_TYPE_INTEGER,	"Average Bandwidth" },
> -		{ ACPI_TYPE_INTEGER,	"Low Bandwidth" },
> -		{ ACPI_TYPE_INTEGER,	"Low Notification Threshold" },
> -		{ ACPI_TYPE_INTEGER,	"High Notification Threshold" },
> -	};
> -
> -	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, "_MBM", obj, 8) != FWTS_OK)
> -		return;
> -
> -	/* For now, just check types */
> -	if (fwts_method_package_elements_type(fw, name, "_MBM", obj, elements, 8) != FWTS_OK)
> -		return;
> -
> -	fwts_method_passed_sane(fw, name, "package");
> -}
> -
>  static int method_test_MBM(fwts_framework *fw)
>  {
> +	uint32_t element_size = 8;
>  	return method_evaluate_method(fw, METHOD_OPTIONAL,
> -		"_MBM", NULL, 0, method_test_MBM_return, NULL);
> +		"_MBM", NULL, 0, fwts_method_test_package_integer_return, &element_size);
>  }
>  
>  /*
> @@ -4579,42 +4507,11 @@ static int method_test_WPP(fwts_framework *fw)
>  /*
>   * Section 11.3 Fan Devices
>   */
> -static void method_test_FIF_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_PACKAGE) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_count_equal(fw, name, "_FIF", obj, 4) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_elements_all_type(fw, name, "_FIF",
> -		obj, ACPI_TYPE_INTEGER) != FWTS_OK) {
> -		fwts_advice(fw,
> -			"%s is not returning the correct "
> -			"fan information. It may be worth "
> -			"running the firmware test suite "
> -			"interactive 'fan' test to see if "
> -			"this affects the control and "
> -			"operation of the fan.", name);
> -		return;
> -	}
> -
> -	fwts_acpi_object_dump(fw, obj);
> -
> -	fwts_method_passed_sane(fw, name, "package");
> -}
> -
>  static int method_test_FIF(fwts_framework *fw)
>  {
> +	uint32_t element_size = 4;
>  	return method_evaluate_method(fw, METHOD_OPTIONAL,
> -		"_FIF", NULL, 0, method_test_FIF_return, NULL);
> +		"_FIF", NULL, 0, fwts_method_test_package_integer_return, &element_size);
>  }
>  
>  static void method_test_FPS_return(
> @@ -4713,42 +4610,11 @@ static int method_test_FSL(fwts_framework *fw)
>  		"_FSL", arg, 1, fwts_method_test_NULL_return, NULL);
>  }
>  
> -static void method_test_FST_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_PACKAGE) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_count_equal(fw, name, "_FST", obj, 3) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_elements_all_type(fw, name, "_FST",
> -		obj, ACPI_TYPE_INTEGER) != FWTS_OK) {
> -		fwts_advice(fw,
> -			"%s is not returning the correct "
> -			"fan status information. It may be "
> -			"worth running the firmware test "
> -			"suite interactive 'fan' test to see "
> -			"if this affects the control and "
> -			"operation of the fan.", name);
> -		return;
> -	}
> -
> -	fwts_acpi_object_dump(fw, obj);
> -
> -	fwts_method_passed_sane(fw, name, "package");
> -}
> -
>  static int method_test_FST(fwts_framework *fw)
>  {
> +	uint32_t element_size = 3;
>  	return method_evaluate_method(fw, METHOD_OPTIONAL,
> -		"_FST", NULL, 0, method_test_FST_return, NULL);
> +		"_FST", NULL, 0, fwts_method_test_package_integer_return, &element_size);
>  }
>  
>  
> diff --git a/src/lib/include/fwts_acpi_object_eval.h b/src/lib/include/fwts_acpi_object_eval.h
> index 92177857..080a0d65 100644
> --- a/src/lib/include/fwts_acpi_object_eval.h
> +++ b/src/lib/include/fwts_acpi_object_eval.h
> @@ -158,7 +158,6 @@ int fwts_method_test_UID(fwts_framework *fw, ACPI_HANDLE *device);
>  
>  void fwts_method_valid_CID_Type(fwts_framework *fw, char *name, ACPI_OBJECT *obj);
>  void fwts_method_test_CID_return(fwts_framework *fw, char *name,ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
> -void fwts_method_test_CLS_return(fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
>  void fwts_method_test_HID_return(fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
>  void fwts_method_test_MLS_return( fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
>  void fwts_method_test_PLD_return(fwts_framework *fw, char *name, ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private);
> diff --git a/src/lib/src/fwts_acpi_object_eval.c b/src/lib/src/fwts_acpi_object_eval.c
> index 613a6fd3..42b97d26 100644
> --- a/src/lib/src/fwts_acpi_object_eval.c
> +++ b/src/lib/src/fwts_acpi_object_eval.c
> @@ -1985,31 +1985,11 @@ int fwts_method_test_CID(fwts_framework *fw, ACPI_HANDLE *device)
>  		"_CID", NULL, 0, fwts_method_test_CID_return, NULL);
>  }
>  
> -void fwts_method_test_CLS_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_PACKAGE) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_count_equal(fw, name, "_CLS", obj, 3) != FWTS_OK)
> -		return;
> -
> -	if (fwts_method_package_elements_all_type(fw, name, "_CLS", obj, ACPI_TYPE_INTEGER) != FWTS_OK)
> -		return;
> -
> -	fwts_method_passed_sane(fw, name, "package");
> -}
> -
>  int fwts_method_test_CLS(fwts_framework *fw, ACPI_HANDLE *device)
>  {
> +	uint32_t element_size = 3;
>  	return fwts_evaluate_method(fw, METHOD_OPTIONAL, device,
> -		"_CLS", NULL, 0, fwts_method_test_CLS_return, NULL);
> +		"_CLS", NULL, 0, fwts_method_test_package_integer_return, &element_size);
>  }
>  
>  int fwts_method_test_DDN(fwts_framework *fw, ACPI_HANDLE *device)
> 
Acked-by: Colin Ian King <colin.king at canonical.com>



More information about the fwts-devel mailing list