diff mbox series

[v2] acpi/method: check full and battery power if acpi_video0 exists

Message ID 20190419202654.22298-1-alex.hung@canonical.com
State Accepted
Headers show
Series [v2] acpi/method: check full and battery power if acpi_video0 exists | expand

Commit Message

Alex Hung April 19, 2019, 8:26 p.m. UTC
8b9e5e4c07f4 added checks for full and battery power in _BCL list,
but ACPI spec does not necessary ask them to be included (please
see examples in ACPI spec B.5.2). However, Linux kernel relies on this
behaviours.

Let's only check full and battery power only if acpi_video0 interface is
used.

Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 src/acpi/method/method.c | 74 +++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 36 deletions(-)

Comments

Colin Ian King April 19, 2019, 9:23 p.m. UTC | #1
On 19/04/2019 21:26, Alex Hung wrote:
> 8b9e5e4c07f4 added checks for full and battery power in _BCL list,
> but ACPI spec does not necessary ask them to be included (please
> see examples in ACPI spec B.5.2). However, Linux kernel relies on this
> behaviours.
> 
> Let's only check full and battery power only if acpi_video0 interface is
> used.
> 
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
>  src/acpi/method/method.c | 74 +++++++++++++++++++++-------------------
>  1 file changed, 38 insertions(+), 36 deletions(-)
> 
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 635c4813..d1f20bb2 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -5791,42 +5791,6 @@ static void method_test_BCL_return(
>  		failed = true;
>  	}
>  
> -	matching_levels = false;
> -	for (i = 2; i < obj->Package.Count; i++) {
> -		if (obj->Package.Elements[0].Integer.Value ==
> -		    obj->Package.Elements[i].Integer.Value) {
> -			matching_levels = true;
> -			break;
> -		}
> -	}
> -
> -	if (!matching_levels) {
> -		failed = true;
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BCLFullNotInList",
> -			"brightness level on full power (%" PRIu64
> -			") is not in brightness levels.",
> -			obj->Package.Elements[0].Integer.Value);
> -	}
> -
> -	matching_levels = false;
> -	for (i = 2; i < obj->Package.Count; i++) {
> -		if (obj->Package.Elements[1].Integer.Value ==
> -		    obj->Package.Elements[i].Integer.Value) {
> -			matching_levels = true;
> -			break;
> -		}
> -	}
> -
> -	if (!matching_levels) {
> -		failed = true;
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BCLBatteryNotInList",
> -			"brightness level on battery (%" PRIu64
> -			") is not in brightness levels.",
> -			obj->Package.Elements[1].Integer.Value);
> -	}
> -
>  	fwts_log_info(fw, "Brightness levels for %s:" ,name);
>  	fwts_log_info_verbatim(fw, "  Level on full power   : %" PRIu64,
>  		(uint64_t)obj->Package.Elements[0].Integer.Value);
> @@ -5847,6 +5811,44 @@ static void method_test_BCL_return(
>  		free(str);
>  	}
>  
> +	if (!access("/sys/class/backlight/acpi_video0", R_OK)) {
> +		matching_levels = false;
> +		for (i = 2; i < obj->Package.Count; i++) {
> +			if (obj->Package.Elements[0].Integer.Value ==
> +			    obj->Package.Elements[i].Integer.Value) {
> +				matching_levels = true;
> +				break;
> +			}
> +		}
> +
> +		if (!matching_levels) {
> +			failed = true;
> +			fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +				"Method_BCLFullNotInList",
> +				"brightness level on full power (%" PRIu64
> +				") is not in brightness levels.",
> +				obj->Package.Elements[0].Integer.Value);
> +		}
> +
> +		matching_levels = false;
> +		for (i = 2; i < obj->Package.Count; i++) {
> +			if (obj->Package.Elements[1].Integer.Value ==
> +			    obj->Package.Elements[i].Integer.Value) {
> +				matching_levels = true;
> +				break;
> +			}
> +		}
> +
> +		if (!matching_levels) {
> +			failed = true;
> +			fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +				"Method_BCLBatteryNotInList",
> +				"brightness level on battery (%" PRIu64
> +				") is not in brightness levels.",
> +				obj->Package.Elements[1].Integer.Value);
> +		}
> +	}
> +
>  	if (failed)
>  		fwts_advice(fw,
>  			"%s seems to be "
> 

Thanks Alex,

Acked-by: Colin Ian King <colin.king@canonical.com>
Ivan Hu April 30, 2019, 3:24 a.m. UTC | #2
On 4/20/19 4:26 AM, Alex Hung wrote:
> 8b9e5e4c07f4 added checks for full and battery power in _BCL list,
> but ACPI spec does not necessary ask them to be included (please
> see examples in ACPI spec B.5.2). However, Linux kernel relies on this
> behaviours.
>
> Let's only check full and battery power only if acpi_video0 interface is
> used.
>
> Signed-off-by: Alex Hung <alex.hung@canonical.com>
> ---
>  src/acpi/method/method.c | 74 +++++++++++++++++++++-------------------
>  1 file changed, 38 insertions(+), 36 deletions(-)
>
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 635c4813..d1f20bb2 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -5791,42 +5791,6 @@ static void method_test_BCL_return(
>  		failed = true;
>  	}
>  
> -	matching_levels = false;
> -	for (i = 2; i < obj->Package.Count; i++) {
> -		if (obj->Package.Elements[0].Integer.Value ==
> -		    obj->Package.Elements[i].Integer.Value) {
> -			matching_levels = true;
> -			break;
> -		}
> -	}
> -
> -	if (!matching_levels) {
> -		failed = true;
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BCLFullNotInList",
> -			"brightness level on full power (%" PRIu64
> -			") is not in brightness levels.",
> -			obj->Package.Elements[0].Integer.Value);
> -	}
> -
> -	matching_levels = false;
> -	for (i = 2; i < obj->Package.Count; i++) {
> -		if (obj->Package.Elements[1].Integer.Value ==
> -		    obj->Package.Elements[i].Integer.Value) {
> -			matching_levels = true;
> -			break;
> -		}
> -	}
> -
> -	if (!matching_levels) {
> -		failed = true;
> -		fwts_failed(fw, LOG_LEVEL_CRITICAL,
> -			"Method_BCLBatteryNotInList",
> -			"brightness level on battery (%" PRIu64
> -			") is not in brightness levels.",
> -			obj->Package.Elements[1].Integer.Value);
> -	}
> -
>  	fwts_log_info(fw, "Brightness levels for %s:" ,name);
>  	fwts_log_info_verbatim(fw, "  Level on full power   : %" PRIu64,
>  		(uint64_t)obj->Package.Elements[0].Integer.Value);
> @@ -5847,6 +5811,44 @@ static void method_test_BCL_return(
>  		free(str);
>  	}
>  
> +	if (!access("/sys/class/backlight/acpi_video0", R_OK)) {
> +		matching_levels = false;
> +		for (i = 2; i < obj->Package.Count; i++) {
> +			if (obj->Package.Elements[0].Integer.Value ==
> +			    obj->Package.Elements[i].Integer.Value) {
> +				matching_levels = true;
> +				break;
> +			}
> +		}
> +
> +		if (!matching_levels) {
> +			failed = true;
> +			fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +				"Method_BCLFullNotInList",
> +				"brightness level on full power (%" PRIu64
> +				") is not in brightness levels.",
> +				obj->Package.Elements[0].Integer.Value);
> +		}
> +
> +		matching_levels = false;
> +		for (i = 2; i < obj->Package.Count; i++) {
> +			if (obj->Package.Elements[1].Integer.Value ==
> +			    obj->Package.Elements[i].Integer.Value) {
> +				matching_levels = true;
> +				break;
> +			}
> +		}
> +
> +		if (!matching_levels) {
> +			failed = true;
> +			fwts_failed(fw, LOG_LEVEL_CRITICAL,
> +				"Method_BCLBatteryNotInList",
> +				"brightness level on battery (%" PRIu64
> +				") is not in brightness levels.",
> +				obj->Package.Elements[1].Integer.Value);
> +		}
> +	}
> +
>  	if (failed)
>  		fwts_advice(fw,
>  			"%s seems to be "


Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff mbox series

Patch

diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
index 635c4813..d1f20bb2 100644
--- a/src/acpi/method/method.c
+++ b/src/acpi/method/method.c
@@ -5791,42 +5791,6 @@  static void method_test_BCL_return(
 		failed = true;
 	}
 
-	matching_levels = false;
-	for (i = 2; i < obj->Package.Count; i++) {
-		if (obj->Package.Elements[0].Integer.Value ==
-		    obj->Package.Elements[i].Integer.Value) {
-			matching_levels = true;
-			break;
-		}
-	}
-
-	if (!matching_levels) {
-		failed = true;
-		fwts_failed(fw, LOG_LEVEL_CRITICAL,
-			"Method_BCLFullNotInList",
-			"brightness level on full power (%" PRIu64
-			") is not in brightness levels.",
-			obj->Package.Elements[0].Integer.Value);
-	}
-
-	matching_levels = false;
-	for (i = 2; i < obj->Package.Count; i++) {
-		if (obj->Package.Elements[1].Integer.Value ==
-		    obj->Package.Elements[i].Integer.Value) {
-			matching_levels = true;
-			break;
-		}
-	}
-
-	if (!matching_levels) {
-		failed = true;
-		fwts_failed(fw, LOG_LEVEL_CRITICAL,
-			"Method_BCLBatteryNotInList",
-			"brightness level on battery (%" PRIu64
-			") is not in brightness levels.",
-			obj->Package.Elements[1].Integer.Value);
-	}
-
 	fwts_log_info(fw, "Brightness levels for %s:" ,name);
 	fwts_log_info_verbatim(fw, "  Level on full power   : %" PRIu64,
 		(uint64_t)obj->Package.Elements[0].Integer.Value);
@@ -5847,6 +5811,44 @@  static void method_test_BCL_return(
 		free(str);
 	}
 
+	if (!access("/sys/class/backlight/acpi_video0", R_OK)) {
+		matching_levels = false;
+		for (i = 2; i < obj->Package.Count; i++) {
+			if (obj->Package.Elements[0].Integer.Value ==
+			    obj->Package.Elements[i].Integer.Value) {
+				matching_levels = true;
+				break;
+			}
+		}
+
+		if (!matching_levels) {
+			failed = true;
+			fwts_failed(fw, LOG_LEVEL_CRITICAL,
+				"Method_BCLFullNotInList",
+				"brightness level on full power (%" PRIu64
+				") is not in brightness levels.",
+				obj->Package.Elements[0].Integer.Value);
+		}
+
+		matching_levels = false;
+		for (i = 2; i < obj->Package.Count; i++) {
+			if (obj->Package.Elements[1].Integer.Value ==
+			    obj->Package.Elements[i].Integer.Value) {
+				matching_levels = true;
+				break;
+			}
+		}
+
+		if (!matching_levels) {
+			failed = true;
+			fwts_failed(fw, LOG_LEVEL_CRITICAL,
+				"Method_BCLBatteryNotInList",
+				"brightness level on battery (%" PRIu64
+				") is not in brightness levels.",
+				obj->Package.Elements[1].Integer.Value);
+		}
+	}
+
 	if (failed)
 		fwts_advice(fw,
 			"%s seems to be "