Patchwork acpi: method: ignore _WAK return values

login
register
mail settings
Submitter Colin King
Date Dec. 12, 2012, 6:27 p.m.
Message ID <1355336845-29302-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/205602/
State Accepted
Headers show

Comments

Colin King - Dec. 12, 2012, 6:27 p.m.
From: Colin Ian King <colin.king@canonical.com>

The kernel doesn't care about the returned values from _WAK so let's
not get too fussy about checking these.  Most firmware seems to do this
wrong anyway, so it's not helpful to flag up an error when the kernel
doesn't care either.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpi/method/method.c | 35 ++---------------------------------
 1 file changed, 2 insertions(+), 33 deletions(-)
Keng-Yu Lin - Dec. 18, 2012, 2:35 a.m.
On Thu, Dec 13, 2012 at 2:27 AM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The kernel doesn't care about the returned values from _WAK so let's
> not get too fussy about checking these.  Most firmware seems to do this
> wrong anyway, so it's not helpful to flag up an error when the kernel
> doesn't care either.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/acpi/method/method.c | 35 ++---------------------------------
>  1 file changed, 2 insertions(+), 33 deletions(-)
>
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index a8669f2..be274b9 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -3414,9 +3414,10 @@ static void method_test_WAK_return(
>         ACPI_OBJECT *obj,
>         void *private)
>  {
> -       uint32_t Sstate = *(uint32_t*)private;
>         int failed = 0;
>
> +       FWTS_UNUSED(private);
> +
>         if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) == FWTS_OK) {
>                 fwts_method_dump_object(fw, obj);
>
> @@ -3439,38 +3440,6 @@ static void method_test_WAK_return(
>                                 fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
>                                 failed++;
>                         }
> -                       else {
> -                               if (obj->Package.Elements[0].Integer.Value > 0x00000002) {
> -                                       fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -                                               "Method_WAKBitField",
> -                                               "_WAK: expecting condition "
> -                                               "bit-field (element 0) of "
> -                                               "packages to be in range, "
> -                                               "got 0x%8.8" PRIx64 ".",
> -                                               obj->Package.Elements[0].Integer.Value);
> -                                       fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> -                                       failed++;
> -                               }
> -                               if (!(
> -                                   ((obj->Package.Elements[1].Integer.Value == Sstate) && (obj->Package.Elements[0].Integer.Value == 0)) ||
> -                                    ((obj->Package.Elements[1].Integer.Value == 0) && (obj->Package.Elements[0].Integer.Value != 0)) )) {
> -                                       fwts_warning(fw,
> -                                               "_WAK: expecting power supply S-state (element 1) "
> -                                               "of packages to be 0x%8.8" PRIx32
> -                                               ", got 0x%8.8" PRIx64 ".",
> -                                               Sstate, obj->Package.Elements[0].Integer.Value);
> -                                       fwts_advice(fw, "_WAK should return 0 if the wake failed and was unsuccessful (i.e. element[0] "
> -                                                       "is non-zero) OR should return the S-state. "
> -                                                       "This can confuse the operating system as this _WAK return indicates that the "
> -                                                       "S-state was not entered because of too much current being drawn from the "
> -                                                       "power supply, however, the BIOS may have actually entered this state and the "
> -                                                       "_WAK method is misinforming the operating system. Currently Linux does not "
> -                                                       "check for the return type from _WAK, so it should theoretically not affect the "
> -                                                       "operation of suspend/resume.");
> -
> -                                       failed++;
> -                               }
> -                       }
>                 }
>                 if (!failed)
>                         fwts_passed(fw,
> --
> 1.8.0
>
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Ivan Hu - Dec. 20, 2012, 2:28 a.m.
On 12/13/2012 02:27 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The kernel doesn't care about the returned values from _WAK so let's
> not get too fussy about checking these.  Most firmware seems to do this
> wrong anyway, so it's not helpful to flag up an error when the kernel
> doesn't care either.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/method/method.c | 35 ++---------------------------------
>   1 file changed, 2 insertions(+), 33 deletions(-)
>
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index a8669f2..be274b9 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -3414,9 +3414,10 @@ static void method_test_WAK_return(
>   	ACPI_OBJECT *obj,
>   	void *private)
>   {
> -	uint32_t Sstate = *(uint32_t*)private;
>   	int failed = 0;
>
> +	FWTS_UNUSED(private);
> +
>   	if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) == FWTS_OK) {
>   		fwts_method_dump_object(fw, obj);
>
> @@ -3439,38 +3440,6 @@ static void method_test_WAK_return(
>   				fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
>   				failed++;
>   			}
> -			else {
> -				if (obj->Package.Elements[0].Integer.Value > 0x00000002) {
> -					fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -						"Method_WAKBitField",
> -						"_WAK: expecting condition "
> -						"bit-field (element 0) of "
> -						"packages to be in range, "
> -						"got 0x%8.8" PRIx64 ".",
> -						obj->Package.Elements[0].Integer.Value);
> -					fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> -					failed++;
> -				}
> -				if (!(
> -				    ((obj->Package.Elements[1].Integer.Value == Sstate) && (obj->Package.Elements[0].Integer.Value == 0)) ||
> -                                    ((obj->Package.Elements[1].Integer.Value == 0) && (obj->Package.Elements[0].Integer.Value != 0)) )) {
> -					fwts_warning(fw,
> -						"_WAK: expecting power supply S-state (element 1) "
> -						"of packages to be 0x%8.8" PRIx32
> -						", got 0x%8.8" PRIx64 ".",
> -						Sstate, obj->Package.Elements[0].Integer.Value);
> -					fwts_advice(fw, "_WAK should return 0 if the wake failed and was unsuccessful (i.e. element[0] "
> -							"is non-zero) OR should return the S-state. "
> -							"This can confuse the operating system as this _WAK return indicates that the "
> -							"S-state was not entered because of too much current being drawn from the "
> -							"power supply, however, the BIOS may have actually entered this state and the "
> -							"_WAK method is misinforming the operating system. Currently Linux does not "
> -							"check for the return type from _WAK, so it should theoretically not affect the "
> -							"operation of suspend/resume.");
> -
> -					failed++;
> -				}
> -			}
>   		}
>   		if (!failed)
>   			fwts_passed(fw,
>
Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
index a8669f2..be274b9 100644
--- a/src/acpi/method/method.c
+++ b/src/acpi/method/method.c
@@ -3414,9 +3414,10 @@  static void method_test_WAK_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
-	uint32_t Sstate = *(uint32_t*)private;
 	int failed = 0;
 
+	FWTS_UNUSED(private);
+
 	if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) == FWTS_OK) {
 		fwts_method_dump_object(fw, obj);
 
@@ -3439,38 +3440,6 @@  static void method_test_WAK_return(
 				fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
 				failed++;
 			}
-			else {
-				if (obj->Package.Elements[0].Integer.Value > 0x00000002) {
-					fwts_failed(fw, LOG_LEVEL_MEDIUM,
-						"Method_WAKBitField",
-						"_WAK: expecting condition "
-						"bit-field (element 0) of "
-						"packages to be in range, "
-						"got 0x%8.8" PRIx64 ".",
-						obj->Package.Elements[0].Integer.Value);
-					fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-					failed++;
-				}
-				if (!(
-				    ((obj->Package.Elements[1].Integer.Value == Sstate) && (obj->Package.Elements[0].Integer.Value == 0)) ||
-                                    ((obj->Package.Elements[1].Integer.Value == 0) && (obj->Package.Elements[0].Integer.Value != 0)) )) {
-					fwts_warning(fw,
-						"_WAK: expecting power supply S-state (element 1) "
-						"of packages to be 0x%8.8" PRIx32
-						", got 0x%8.8" PRIx64 ".",
-						Sstate, obj->Package.Elements[0].Integer.Value);
-					fwts_advice(fw, "_WAK should return 0 if the wake failed and was unsuccessful (i.e. element[0] "
-							"is non-zero) OR should return the S-state. "
-							"This can confuse the operating system as this _WAK return indicates that the "
-							"S-state was not entered because of too much current being drawn from the "
-							"power supply, however, the BIOS may have actually entered this state and the "
-							"_WAK method is misinforming the operating system. Currently Linux does not "
-							"check for the return type from _WAK, so it should theoretically not affect the "
-							"operation of suspend/resume.");
-
-					failed++;
-				}
-			}
 		}
 		if (!failed)
 			fwts_passed(fw,