[PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning

[PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning

[Prarit Bhargava]

48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2
integers") changed the error handling in AcpiGetSleepTypeData such that

ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_] (20130517/hwxface-571)

is displayed on systems not implementing individual ACPI \_Sx sleep states.
Since these states are optional the loud warning is incorrect.  This patch
changes the kernel to the older behaviour of not warning on a missing \_Sx.

Before patch:

[root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
 ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_] (20130517/hwxface-571)
 ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S2_] (20130517/hwxface-571)
 ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S3_] (20130517/hwxface-571)
[root@dell-pem520-03 ~]#

After patch:

[root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
[root@dell-pem520-03 ~]#

Fixes: 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2 integers")
Cc: Robert Moore <robert.moore@intel.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/acpi/acpica/hwxface.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
index 5f97468..8f50a7f 100644
--- a/drivers/acpi/acpica/hwxface.c
+++ b/drivers/acpi/acpica/hwxface.c
@@ -508,6 +508,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8 *sleep_type_b)
 	    ACPI_CAST_PTR(char, acpi_gbl_sleep_state_names[sleep_state]);
 	status = acpi_ns_evaluate(info);
 	if (ACPI_FAILURE(status)) {
+		/* \_Sx states are optional */
 		goto cleanup;
 	}
 
@@ -517,7 +518,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8 *sleep_type_b)
 		ACPI_ERROR((AE_INFO, "No Sleep State object returned from [%s]",
 			    info->relative_pathname));
 		status = AE_AML_NO_RETURN_VALUE;
-		goto cleanup;
+		goto cleanup1;
 	}
 
 	/* Return object must be of type Package */
@@ -526,7 +527,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8 *sleep_type_b)
 		ACPI_ERROR((AE_INFO,
 			    "Sleep State return object is not a Package"));
 		status = AE_AML_OPERAND_TYPE;
-		goto cleanup1;
+		goto cleanup2;
 	}
 
 	/*
@@ -570,16 +571,17 @@ acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8 *sleep_type_b)
 		break;
 	}
 
-cleanup1:
+cleanup2:
 	acpi_ut_remove_reference(info->return_object);
 
-cleanup:
+cleanup1:
 	if (ACPI_FAILURE(status)) {
 		ACPI_EXCEPTION((AE_INFO, status,
 				"While evaluating Sleep State [%s]",
 				info->relative_pathname));
 	}
 
+cleanup:
 	ACPI_FREE(info);
 	return_ACPI_STATUS(status);
 }
-- 
1.7.9.3

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning

[Moore, Robert]

I seem to remember that this came up a long time ago.

Perhaps someone else will remember.


> -----Original Message-----
> From: Prarit Bhargava [mailto:prarit@redhat.com]
> Sent: Thursday, October 01, 2015 10:29 AM
> To: devel@acpica.org
> Cc: Prarit Bhargava; Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len
> Brown; linux-acpi@vger.kernel.org
> Subject: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should
> not result in a loud warning
> 
> 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2
> integers") changed the error handling in AcpiGetSleepTypeData such that
> 
> ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
> (20130517/hwxface-571)
> 
> is displayed on systems not implementing individual ACPI \_Sx sleep
> states.
> Since these states are optional the loud warning is incorrect.  This patch
> changes the kernel to the older behaviour of not warning on a missing
> \_Sx.
> 
> Before patch:
> 
> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
>  ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
> (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND, While evaluating
> Sleep State [\_S2_] (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND,
> While evaluating Sleep State [\_S3_] (20130517/hwxface-571)
> [root@dell-pem520-03 ~]#
> 
> After patch:
> 
> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
> [root@dell-pem520-03 ~]#
> 
> Fixes: 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either
> 1 or 2 integers")
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> ---
>  drivers/acpi/acpica/hwxface.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
> index 5f97468..8f50a7f 100644
> --- a/drivers/acpi/acpica/hwxface.c
> +++ b/drivers/acpi/acpica/hwxface.c
> @@ -508,6 +508,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> *sleep_type_a, u8 *sleep_type_b)
>  	    ACPI_CAST_PTR(char, acpi_gbl_sleep_state_names[sleep_state]);
>  	status = acpi_ns_evaluate(info);
>  	if (ACPI_FAILURE(status)) {
> +		/* \_Sx states are optional */
>  		goto cleanup;
>  	}
> 
> @@ -517,7 +518,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> *sleep_type_a, u8 *sleep_type_b)
>  		ACPI_ERROR((AE_INFO, "No Sleep State object returned from
> [%s]",
>  			    info->relative_pathname));
>  		status = AE_AML_NO_RETURN_VALUE;
> -		goto cleanup;
> +		goto cleanup1;
>  	}
> 
>  	/* Return object must be of type Package */ @@ -526,7 +527,7 @@
> acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8
> *sleep_type_b)
>  		ACPI_ERROR((AE_INFO,
>  			    "Sleep State return object is not a Package"));
>  		status = AE_AML_OPERAND_TYPE;
> -		goto cleanup1;
> +		goto cleanup2;
>  	}
> 
>  	/*
> @@ -570,16 +571,17 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> *sleep_type_a, u8 *sleep_type_b)
>  		break;
>  	}
> 
> -cleanup1:
> +cleanup2:
>  	acpi_ut_remove_reference(info->return_object);
> 
> -cleanup:
> +cleanup1:
>  	if (ACPI_FAILURE(status)) {
>  		ACPI_EXCEPTION((AE_INFO, status,
>  				"While evaluating Sleep State [%s]",
>  				info->relative_pathname));
>  	}
> 
> +cleanup:
>  	ACPI_FREE(info);
>  	return_ACPI_STATUS(status);
>  }
> --
> 1.7.9.3

Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning

[Rafael J. Wysocki]

On 10/1/2015 7:57 PM, Moore, Robert wrote:
> I seem to remember that this came up a long time ago.
>
> Perhaps someone else will remember.
>

I can't recall, sorry.

>> -----Original Message-----
>> From: Prarit Bhargava [mailto:prarit@redhat.com]
>> Sent: Thursday, October 01, 2015 10:29 AM
>> To: devel@acpica.org
>> Cc: Prarit Bhargava; Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len
>> Brown; linux-acpi@vger.kernel.org
>> Subject: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should
>> not result in a loud warning
>>
>> 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2
>> integers") changed the error handling in AcpiGetSleepTypeData such that
>>
>> ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
>> (20130517/hwxface-571)
>>
>> is displayed on systems not implementing individual ACPI \_Sx sleep
>> states.
>> Since these states are optional the loud warning is incorrect.  This patch
>> changes the kernel to the older behaviour of not warning on a missing
>> \_Sx.
>>
>> Before patch:
>>
>> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
>>   ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
>> (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND, While evaluating
>> Sleep State [\_S2_] (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND,
>> While evaluating Sleep State [\_S3_] (20130517/hwxface-571)
>> [root@dell-pem520-03 ~]#
>>
>> After patch:
>>
>> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
>> [root@dell-pem520-03 ~]#
>>
>> Fixes: 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either
>> 1 or 2 integers")
>> Cc: Robert Moore <robert.moore@intel.com>
>> Cc: Lv Zheng <lv.zheng@intel.com>
>> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: linux-acpi@vger.kernel.org
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> ---
>>   drivers/acpi/acpica/hwxface.c |   10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
>> index 5f97468..8f50a7f 100644
>> --- a/drivers/acpi/acpica/hwxface.c
>> +++ b/drivers/acpi/acpica/hwxface.c
>> @@ -508,6 +508,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
>> *sleep_type_a, u8 *sleep_type_b)
>>   	    ACPI_CAST_PTR(char, acpi_gbl_sleep_state_names[sleep_state]);
>>   	status = acpi_ns_evaluate(info);
>>   	if (ACPI_FAILURE(status)) {
>> +		/* \_Sx states are optional */
>>   		goto cleanup;
>>   	}
>>
>> @@ -517,7 +518,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
>> *sleep_type_a, u8 *sleep_type_b)
>>   		ACPI_ERROR((AE_INFO, "No Sleep State object returned from
>> [%s]",
>>   			    info->relative_pathname));
>>   		status = AE_AML_NO_RETURN_VALUE;
>> -		goto cleanup;
>> +		goto cleanup1;
>>   	}
>>
>>   	/* Return object must be of type Package */ @@ -526,7 +527,7 @@
>> acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8
>> *sleep_type_b)
>>   		ACPI_ERROR((AE_INFO,
>>   			    "Sleep State return object is not a Package"));
>>   		status = AE_AML_OPERAND_TYPE;
>> -		goto cleanup1;
>> +		goto cleanup2;
>>   	}
>>
>>   	/*
>> @@ -570,16 +571,17 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
>> *sleep_type_a, u8 *sleep_type_b)
>>   		break;
>>   	}
>>
>> -cleanup1:
>> +cleanup2:
>>   	acpi_ut_remove_reference(info->return_object);
>>
>> -cleanup:
>> +cleanup1:
>>   	if (ACPI_FAILURE(status)) {
>>   		ACPI_EXCEPTION((AE_INFO, status,
>>   				"While evaluating Sleep State [%s]",
>>   				info->relative_pathname));
>>   	}
>>
>> +cleanup:
>>   	ACPI_FREE(info);
>>   	return_ACPI_STATUS(status);
>>   }
>> --
>> 1.7.9.3

Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning

[Prarit Bhargava]

On 10/01/2015 06:28 PM, Rafael J. Wysocki wrote:
> On 10/1/2015 7:57 PM, Moore, Robert wrote:
>> I seem to remember that this came up a long time ago.
>>
>> Perhaps someone else will remember.
>>
> 
> I can't recall, sorry.

FWIW, a google search on these warnings yields tonnes of comments about them,
but with no resolution AFAICT.

P.

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning

[Moore, Robert]

It would be easy to remove (or partially remove) the warning. Hopefully the calling code will handle AE_NOT_FOUND correctly. 

> -----Original Message-----
> From: Prarit Bhargava [mailto:prarit@redhat.com]
> Sent: Wednesday, October 07, 2015 8:29 AM
> To: Wysocki, Rafael J; Moore, Robert; devel@acpica.org
> Cc: Zheng, Lv; Len Brown; linux-acpi@vger.kernel.org; Box, David E
> Subject: Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx
> should not result in a loud warning
> 
> 
> 
> On 10/01/2015 06:28 PM, Rafael J. Wysocki wrote:
> > On 10/1/2015 7:57 PM, Moore, Robert wrote:
> >> I seem to remember that this came up a long time ago.
> >>
> >> Perhaps someone else will remember.
> >>
> >
> > I can't recall, sorry.
> 
> FWIW, a google search on these warnings yields tonnes of comments about
> them, but with no resolution AFAICT.
> 
> P.

Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning

[Prarit Bhargava]

On 10/07/2015 11:52 AM, Moore, Robert wrote:
> It would be easy to remove (or partially remove) the warning. Hopefully the calling code will handle AE_NOT_FOUND correctly. 
> 

In the specific case of the \_Sx sleep states the code does handle the
AE_NOT_FOUND correctly.  The issue is solely the warning being displayed when
there should be no warning.

P.

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning

[Moore, Robert]

I'm thinking that AE_NOT_FOUND should not generate a warning, but any other exception should.


> -----Original Message-----
> From: Prarit Bhargava [mailto:prarit@redhat.com]
> Sent: Wednesday, October 07, 2015 8:57 AM
> To: Moore, Robert; Wysocki, Rafael J; devel@acpica.org
> Cc: Zheng, Lv; Len Brown; linux-acpi@vger.kernel.org; Box, David E
> Subject: Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx
> should not result in a loud warning
> 
> 
> 
> On 10/07/2015 11:52 AM, Moore, Robert wrote:
> > It would be easy to remove (or partially remove) the warning. Hopefully
> the calling code will handle AE_NOT_FOUND correctly.
> >
> 
> In the specific case of the \_Sx sleep states the code does handle the
> AE_NOT_FOUND correctly.  The issue is solely the warning being displayed
> when there should be no warning.
> 
> P.

[PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Prarit Bhargava]

48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2
integers") changed the error handling in AcpiGetSleepTypeData such that

ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_] (20130517/hwxface-571)

is displayed on systems not implementing individual ACPI \_Sx sleep states.
Since these states are optional the loud warning is incorrect.  This patch
changes the kernel to the older behaviour of not warning on a not found \_Sx.

Before patch:

[root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
 ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_] (20130517/hwxface-571)
 ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S2_] (20130517/hwxface-571)
 ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S3_] (20130517/hwxface-571)
[root@dell-pem520-03 ~]#

After patch:

[root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
[root@dell-pem520-03 ~]#

v2: Suggested by Robert Moore: Do not warn on AE_NOT_FOUND, otherwise warn loudly

Fixes: 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2 integers")
Cc: Robert Moore <robert.moore@intel.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/acpi/acpica/hwxface.c |   15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
index 5f97468..b450133 100644
--- a/drivers/acpi/acpica/hwxface.c
+++ b/drivers/acpi/acpica/hwxface.c
@@ -507,9 +507,13 @@ acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8 *sleep_type_b)
 	info->relative_pathname =
 	    ACPI_CAST_PTR(char, acpi_gbl_sleep_state_names[sleep_state]);
 	status = acpi_ns_evaluate(info);
-	if (ACPI_FAILURE(status)) {
+	if (status == AE_NOT_FOUND) {
+		/* \_Sx states are optional */
 		goto cleanup;
 	}
+	if (ACPI_FAILURE(status)) {
+		goto cleanup1;
+	}
 
 	/* Must have a return object */
 
@@ -517,7 +521,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8 *sleep_type_b)
 		ACPI_ERROR((AE_INFO, "No Sleep State object returned from [%s]",
 			    info->relative_pathname));
 		status = AE_AML_NO_RETURN_VALUE;
-		goto cleanup;
+		goto cleanup1;
 	}
 
 	/* Return object must be of type Package */
@@ -526,7 +530,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8 *sleep_type_b)
 		ACPI_ERROR((AE_INFO,
 			    "Sleep State return object is not a Package"));
 		status = AE_AML_OPERAND_TYPE;
-		goto cleanup1;
+		goto cleanup2;
 	}
 
 	/*
@@ -570,16 +574,17 @@ acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8 *sleep_type_b)
 		break;
 	}
 
-cleanup1:
+cleanup2:
 	acpi_ut_remove_reference(info->return_object);
 
-cleanup:
+cleanup1:
 	if (ACPI_FAILURE(status)) {
 		ACPI_EXCEPTION((AE_INFO, status,
 				"While evaluating Sleep State [%s]",
 				info->relative_pathname));
 	}
 
+cleanup:
 	ACPI_FREE(info);
 	return_ACPI_STATUS(status);
 }
-- 
1.7.9.3

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Moore, Robert]

We'll do the ACPICA version of this, although with probably with some changes.
Thanks,
Bob


> -----Original Message-----
> From: Prarit Bhargava [mailto:prarit@redhat.com]
> Sent: Wednesday, October 07, 2015 11:51 AM
> To: devel@acpica.org
> Cc: Prarit Bhargava; Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len
> Brown; linux-acpi@vger.kernel.org
> Subject: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should
> not result in a loud warning [v2]
> 
> 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2
> integers") changed the error handling in AcpiGetSleepTypeData such that
> 
> ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
> (20130517/hwxface-571)
> 
> is displayed on systems not implementing individual ACPI \_Sx sleep
> states.
> Since these states are optional the loud warning is incorrect.  This patch
> changes the kernel to the older behaviour of not warning on a not found
> \_Sx.
> 
> Before patch:
> 
> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
>  ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
> (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND, While evaluating
> Sleep State [\_S2_] (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND,
> While evaluating Sleep State [\_S3_] (20130517/hwxface-571)
> [root@dell-pem520-03 ~]#
> 
> After patch:
> 
> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
> [root@dell-pem520-03 ~]#
> 
> v2: Suggested by Robert Moore: Do not warn on AE_NOT_FOUND, otherwise warn
> loudly
> 
> Fixes: 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either
> 1 or 2 integers")
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> ---
>  drivers/acpi/acpica/hwxface.c |   15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
> index 5f97468..b450133 100644
> --- a/drivers/acpi/acpica/hwxface.c
> +++ b/drivers/acpi/acpica/hwxface.c
> @@ -507,9 +507,13 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> *sleep_type_a, u8 *sleep_type_b)
>  	info->relative_pathname =
>  	    ACPI_CAST_PTR(char, acpi_gbl_sleep_state_names[sleep_state]);
>  	status = acpi_ns_evaluate(info);
> -	if (ACPI_FAILURE(status)) {
> +	if (status == AE_NOT_FOUND) {
> +		/* \_Sx states are optional */
>  		goto cleanup;
>  	}
> +	if (ACPI_FAILURE(status)) {
> +		goto cleanup1;
> +	}
> 
>  	/* Must have a return object */
> 
> @@ -517,7 +521,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> *sleep_type_a, u8 *sleep_type_b)
>  		ACPI_ERROR((AE_INFO, "No Sleep State object returned from
> [%s]",
>  			    info->relative_pathname));
>  		status = AE_AML_NO_RETURN_VALUE;
> -		goto cleanup;
> +		goto cleanup1;
>  	}
> 
>  	/* Return object must be of type Package */ @@ -526,7 +530,7 @@
> acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8
> *sleep_type_b)
>  		ACPI_ERROR((AE_INFO,
>  			    "Sleep State return object is not a Package"));
>  		status = AE_AML_OPERAND_TYPE;
> -		goto cleanup1;
> +		goto cleanup2;
>  	}
> 
>  	/*
> @@ -570,16 +574,17 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> *sleep_type_a, u8 *sleep_type_b)
>  		break;
>  	}
> 
> -cleanup1:
> +cleanup2:
>  	acpi_ut_remove_reference(info->return_object);
> 
> -cleanup:
> +cleanup1:
>  	if (ACPI_FAILURE(status)) {
>  		ACPI_EXCEPTION((AE_INFO, status,
>  				"While evaluating Sleep State [%s]",
>  				info->relative_pathname));
>  	}
> 
> +cleanup:
>  	ACPI_FREE(info);
>  	return_ACPI_STATUS(status);
>  }
> --
> 1.7.9.3

Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Prarit Bhargava]

On 10/08/2015 10:19 AM, Moore, Robert wrote:
> We'll do the ACPICA version of this, although with probably with some changes.
> Thanks,

Thanks Bob.  If you could let me know when this lands in github(?) that would be
great.  I'll reference it in the backport upstream.

P.

> Bob
> 
> 
>> -----Original Message-----
>> From: Prarit Bhargava [mailto:prarit@redhat.com]
>> Sent: Wednesday, October 07, 2015 11:51 AM
>> To: devel@acpica.org
>> Cc: Prarit Bhargava; Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len
>> Brown; linux-acpi@vger.kernel.org
>> Subject: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should
>> not result in a loud warning [v2]
>>
>> 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2
>> integers") changed the error handling in AcpiGetSleepTypeData such that
>>
>> ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
>> (20130517/hwxface-571)
>>
>> is displayed on systems not implementing individual ACPI \_Sx sleep
>> states.
>> Since these states are optional the loud warning is incorrect.  This patch
>> changes the kernel to the older behaviour of not warning on a not found
>> \_Sx.
>>
>> Before patch:
>>
>> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
>>  ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
>> (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND, While evaluating
>> Sleep State [\_S2_] (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND,
>> While evaluating Sleep State [\_S3_] (20130517/hwxface-571)
>> [root@dell-pem520-03 ~]#
>>
>> After patch:
>>
>> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
>> [root@dell-pem520-03 ~]#
>>
>> v2: Suggested by Robert Moore: Do not warn on AE_NOT_FOUND, otherwise warn
>> loudly
>>
>> Fixes: 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either
>> 1 or 2 integers")
>> Cc: Robert Moore <robert.moore@intel.com>
>> Cc: Lv Zheng <lv.zheng@intel.com>
>> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: linux-acpi@vger.kernel.org
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> ---
>>  drivers/acpi/acpica/hwxface.c |   15 ++++++++++-----
>>  1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
>> index 5f97468..b450133 100644
>> --- a/drivers/acpi/acpica/hwxface.c
>> +++ b/drivers/acpi/acpica/hwxface.c
>> @@ -507,9 +507,13 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
>> *sleep_type_a, u8 *sleep_type_b)
>>  	info->relative_pathname =
>>  	    ACPI_CAST_PTR(char, acpi_gbl_sleep_state_names[sleep_state]);
>>  	status = acpi_ns_evaluate(info);
>> -	if (ACPI_FAILURE(status)) {
>> +	if (status == AE_NOT_FOUND) {
>> +		/* \_Sx states are optional */
>>  		goto cleanup;
>>  	}
>> +	if (ACPI_FAILURE(status)) {
>> +		goto cleanup1;
>> +	}
>>
>>  	/* Must have a return object */
>>
>> @@ -517,7 +521,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
>> *sleep_type_a, u8 *sleep_type_b)
>>  		ACPI_ERROR((AE_INFO, "No Sleep State object returned from
>> [%s]",
>>  			    info->relative_pathname));
>>  		status = AE_AML_NO_RETURN_VALUE;
>> -		goto cleanup;
>> +		goto cleanup1;
>>  	}
>>
>>  	/* Return object must be of type Package */ @@ -526,7 +530,7 @@
>> acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8
>> *sleep_type_b)
>>  		ACPI_ERROR((AE_INFO,
>>  			    "Sleep State return object is not a Package"));
>>  		status = AE_AML_OPERAND_TYPE;
>> -		goto cleanup1;
>> +		goto cleanup2;
>>  	}
>>
>>  	/*
>> @@ -570,16 +574,17 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
>> *sleep_type_a, u8 *sleep_type_b)
>>  		break;
>>  	}
>>
>> -cleanup1:
>> +cleanup2:
>>  	acpi_ut_remove_reference(info->return_object);
>>
>> -cleanup:
>> +cleanup1:
>>  	if (ACPI_FAILURE(status)) {
>>  		ACPI_EXCEPTION((AE_INFO, status,
>>  				"While evaluating Sleep State [%s]",
>>  				info->relative_pathname));
>>  	}
>>
>> +cleanup:
>>  	ACPI_FREE(info);
>>  	return_ACPI_STATUS(status);
>>  }
>> --
>> 1.7.9.3
> 

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Zheng, Lv]

Why don't you fix this in the invoker side?
For example:
If (acpi_get_handle())
	acpi_evaluate_object()
So that the AE_NOT_FOUND warning can still be kept for the real troubles?
There are really scenarios that such warning is useful for catching bugs.

Thanks and best regards
-Lv

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Prarit Bhargava
> Sent: Thursday, October 08, 2015 10:23 PM
> To: Moore, Robert; devel@acpica.org
> Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; linux-acpi@vger.kernel.org
> Subject: Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]
> 
> 
> 
> On 10/08/2015 10:19 AM, Moore, Robert wrote:
> > We'll do the ACPICA version of this, although with probably with some changes.
> > Thanks,
> 
> Thanks Bob.  If you could let me know when this lands in github(?) that would be
> great.  I'll reference it in the backport upstream.
> 
> P.
> 
> > Bob
> >
> >
> >> -----Original Message-----
> >> From: Prarit Bhargava [mailto:prarit@redhat.com]
> >> Sent: Wednesday, October 07, 2015 11:51 AM
> >> To: devel@acpica.org
> >> Cc: Prarit Bhargava; Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len
> >> Brown; linux-acpi@vger.kernel.org
> >> Subject: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should
> >> not result in a loud warning [v2]
> >>
> >> 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2
> >> integers") changed the error handling in AcpiGetSleepTypeData such that
> >>
> >> ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
> >> (20130517/hwxface-571)
> >>
> >> is displayed on systems not implementing individual ACPI \_Sx sleep
> >> states.
> >> Since these states are optional the loud warning is incorrect.  This patch
> >> changes the kernel to the older behaviour of not warning on a not found
> >> \_Sx.
> >>
> >> Before patch:
> >>
> >> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
> >>  ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
> >> (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND, While evaluating
> >> Sleep State [\_S2_] (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND,
> >> While evaluating Sleep State [\_S3_] (20130517/hwxface-571)
> >> [root@dell-pem520-03 ~]#
> >>
> >> After patch:
> >>
> >> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
> >> [root@dell-pem520-03 ~]#
> >>
> >> v2: Suggested by Robert Moore: Do not warn on AE_NOT_FOUND, otherwise warn
> >> loudly
> >>
> >> Fixes: 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either
> >> 1 or 2 integers")
> >> Cc: Robert Moore <robert.moore@intel.com>
> >> Cc: Lv Zheng <lv.zheng@intel.com>
> >> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> >> Cc: Len Brown <lenb@kernel.org>
> >> Cc: linux-acpi@vger.kernel.org
> >> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> >> ---
> >>  drivers/acpi/acpica/hwxface.c |   15 ++++++++++-----
> >>  1 file changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
> >> index 5f97468..b450133 100644
> >> --- a/drivers/acpi/acpica/hwxface.c
> >> +++ b/drivers/acpi/acpica/hwxface.c
> >> @@ -507,9 +507,13 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> >> *sleep_type_a, u8 *sleep_type_b)
> >>  	info->relative_pathname =
> >>  	    ACPI_CAST_PTR(char, acpi_gbl_sleep_state_names[sleep_state]);
> >>  	status = acpi_ns_evaluate(info);
> >> -	if (ACPI_FAILURE(status)) {
> >> +	if (status == AE_NOT_FOUND) {
> >> +		/* \_Sx states are optional */
> >>  		goto cleanup;
> >>  	}
> >> +	if (ACPI_FAILURE(status)) {
> >> +		goto cleanup1;
> >> +	}
> >>
> >>  	/* Must have a return object */
> >>
> >> @@ -517,7 +521,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> >> *sleep_type_a, u8 *sleep_type_b)
> >>  		ACPI_ERROR((AE_INFO, "No Sleep State object returned from
> >> [%s]",
> >>  			    info->relative_pathname));
> >>  		status = AE_AML_NO_RETURN_VALUE;
> >> -		goto cleanup;
> >> +		goto cleanup1;
> >>  	}
> >>
> >>  	/* Return object must be of type Package */ @@ -526,7 +530,7 @@
> >> acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8
> >> *sleep_type_b)
> >>  		ACPI_ERROR((AE_INFO,
> >>  			    "Sleep State return object is not a Package"));
> >>  		status = AE_AML_OPERAND_TYPE;
> >> -		goto cleanup1;
> >> +		goto cleanup2;
> >>  	}
> >>
> >>  	/*
> >> @@ -570,16 +574,17 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> >> *sleep_type_a, u8 *sleep_type_b)
> >>  		break;
> >>  	}
> >>
> >> -cleanup1:
> >> +cleanup2:
> >>  	acpi_ut_remove_reference(info->return_object);
> >>
> >> -cleanup:
> >> +cleanup1:
> >>  	if (ACPI_FAILURE(status)) {
> >>  		ACPI_EXCEPTION((AE_INFO, status,
> >>  				"While evaluating Sleep State [%s]",
> >>  				info->relative_pathname));
> >>  	}
> >>
> >> +cleanup:
> >>  	ACPI_FREE(info);
> >>  	return_ACPI_STATUS(status);
> >>  }
> >> --
> >> 1.7.9.3
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Zheng, Lv]

Please ignore this.
The fix is against the caller.

Thanks and best regards
-Lv

> -----Original Message-----
> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Zheng, Lv
> Sent: Friday, October 09, 2015 10:02 AM
> To: Prarit Bhargava; Moore, Robert; devel@acpica.org
> Cc: linux-acpi@vger.kernel.org; Wysocki, Rafael J
> Subject: Re: [Devel] [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]
> 
> Why don't you fix this in the invoker side?
> For example:
> If (acpi_get_handle())
> 	acpi_evaluate_object()
> So that the AE_NOT_FOUND warning can still be kept for the real troubles?
> There are really scenarios that such warning is useful for catching bugs.
> 
> Thanks and best regards
> -Lv
> 
> > -----Original Message-----
> > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Prarit Bhargava
> > Sent: Thursday, October 08, 2015 10:23 PM
> > To: Moore, Robert; devel@acpica.org
> > Cc: Zheng, Lv; Wysocki, Rafael J; Len Brown; linux-acpi@vger.kernel.org
> > Subject: Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]
> >
> >
> >
> > On 10/08/2015 10:19 AM, Moore, Robert wrote:
> > > We'll do the ACPICA version of this, although with probably with some changes.
> > > Thanks,
> >
> > Thanks Bob.  If you could let me know when this lands in github(?) that would be
> > great.  I'll reference it in the backport upstream.
> >
> > P.
> >
> > > Bob
> > >
> > >
> > >> -----Original Message-----
> > >> From: Prarit Bhargava [mailto:prarit@redhat.com]
> > >> Sent: Wednesday, October 07, 2015 11:51 AM
> > >> To: devel@acpica.org
> > >> Cc: Prarit Bhargava; Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len
> > >> Brown; linux-acpi@vger.kernel.org
> > >> Subject: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should
> > >> not result in a loud warning [v2]
> > >>
> > >> 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2
> > >> integers") changed the error handling in AcpiGetSleepTypeData such that
> > >>
> > >> ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
> > >> (20130517/hwxface-571)
> > >>
> > >> is displayed on systems not implementing individual ACPI \_Sx sleep
> > >> states.
> > >> Since these states are optional the loud warning is incorrect.  This patch
> > >> changes the kernel to the older behaviour of not warning on a not found
> > >> \_Sx.
> > >>
> > >> Before patch:
> > >>
> > >> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
> > >>  ACPI Exception: AE_NOT_FOUND, While evaluating Sleep State [\_S1_]
> > >> (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND, While evaluating
> > >> Sleep State [\_S2_] (20130517/hwxface-571)  ACPI Exception: AE_NOT_FOUND,
> > >> While evaluating Sleep State [\_S3_] (20130517/hwxface-571)
> > >> [root@dell-pem520-03 ~]#
> > >>
> > >> After patch:
> > >>
> > >> [root@dell-pem520-03 ~]# dmesg | grep "While evaluating Sleep State"
> > >> [root@dell-pem520-03 ~]#
> > >>
> > >> v2: Suggested by Robert Moore: Do not warn on AE_NOT_FOUND, otherwise warn
> > >> loudly
> > >>
> > >> Fixes: 48ffb94 ("ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either
> > >> 1 or 2 integers")
> > >> Cc: Robert Moore <robert.moore@intel.com>
> > >> Cc: Lv Zheng <lv.zheng@intel.com>
> > >> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> > >> Cc: Len Brown <lenb@kernel.org>
> > >> Cc: linux-acpi@vger.kernel.org
> > >> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> > >> ---
> > >>  drivers/acpi/acpica/hwxface.c |   15 ++++++++++-----
> > >>  1 file changed, 10 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
> > >> index 5f97468..b450133 100644
> > >> --- a/drivers/acpi/acpica/hwxface.c
> > >> +++ b/drivers/acpi/acpica/hwxface.c
> > >> @@ -507,9 +507,13 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> > >> *sleep_type_a, u8 *sleep_type_b)
> > >>  	info->relative_pathname =
> > >>  	    ACPI_CAST_PTR(char, acpi_gbl_sleep_state_names[sleep_state]);
> > >>  	status = acpi_ns_evaluate(info);
> > >> -	if (ACPI_FAILURE(status)) {
> > >> +	if (status == AE_NOT_FOUND) {
> > >> +		/* \_Sx states are optional */
> > >>  		goto cleanup;
> > >>  	}
> > >> +	if (ACPI_FAILURE(status)) {
> > >> +		goto cleanup1;
> > >> +	}
> > >>
> > >>  	/* Must have a return object */
> > >>
> > >> @@ -517,7 +521,7 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> > >> *sleep_type_a, u8 *sleep_type_b)
> > >>  		ACPI_ERROR((AE_INFO, "No Sleep State object returned from
> > >> [%s]",
> > >>  			    info->relative_pathname));
> > >>  		status = AE_AML_NO_RETURN_VALUE;
> > >> -		goto cleanup;
> > >> +		goto cleanup1;
> > >>  	}
> > >>
> > >>  	/* Return object must be of type Package */ @@ -526,7 +530,7 @@
> > >> acpi_get_sleep_type_data(u8 sleep_state, u8 *sleep_type_a, u8
> > >> *sleep_type_b)
> > >>  		ACPI_ERROR((AE_INFO,
> > >>  			    "Sleep State return object is not a Package"));
> > >>  		status = AE_AML_OPERAND_TYPE;
> > >> -		goto cleanup1;
> > >> +		goto cleanup2;
> > >>  	}
> > >>
> > >>  	/*
> > >> @@ -570,16 +574,17 @@ acpi_get_sleep_type_data(u8 sleep_state, u8
> > >> *sleep_type_a, u8 *sleep_type_b)
> > >>  		break;
> > >>  	}
> > >>
> > >> -cleanup1:
> > >> +cleanup2:
> > >>  	acpi_ut_remove_reference(info->return_object);
> > >>
> > >> -cleanup:
> > >> +cleanup1:
> > >>  	if (ACPI_FAILURE(status)) {
> > >>  		ACPI_EXCEPTION((AE_INFO, status,
> > >>  				"While evaluating Sleep State [%s]",
> > >>  				info->relative_pathname));
> > >>  	}
> > >>
> > >> +cleanup:
> > >>  	ACPI_FREE(info);
> > >>  	return_ACPI_STATUS(status);
> > >>  }
> > >> --
> > >> 1.7.9.3
> > >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> _______________________________________________
> Devel mailing list
> Devel@acpica.org
> https://lists.acpica.org/mailman/listinfo/devel

Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Prarit Bhargava]

On 10/09/2015 01:26 AM, Zheng, Lv wrote:
> Please ignore this.
> The fix is against the caller.
> 
> Thanks and best regards
> -Lv
> 
>> -----Original Message-----
>> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Zheng, Lv
>> Sent: Friday, October 09, 2015 10:02 AM
>> To: Prarit Bhargava; Moore, Robert; devel@acpica.org
>> Cc: linux-acpi@vger.kernel.org; Wysocki, Rafael J
>> Subject: Re: [Devel] [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]
>>
>> Why don't you fix this in the invoker side?
>> For example:
>> If (acpi_get_handle())
>> 	acpi_evaluate_object()

That seems like a sloppy workaround an actual bug in ACPICA.

>> So that the AE_NOT_FOUND warning can still be kept for the real troubles?

The code is warning 100% of the time on something that is optional.

>> There are really scenarios that such warning is useful for catching bugs.
>>

What scenario is possible where this causes a problem?  Issuing an error on
something that is optional is not a good idea.

P.

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Moore, Robert]

I have no issues with the original solution. We don't warn on other ACPI_NOT_FOUND exceptions.


> -----Original Message-----
> From: Prarit Bhargava [mailto:prarit@redhat.com]
> Sent: Friday, October 09, 2015 4:30 AM
> To: Zheng, Lv; Moore, Robert; devel@acpica.org
> Cc: linux-acpi@vger.kernel.org; Wysocki, Rafael J
> Subject: Re: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx
> should not result in a loud warning [v2]
> 
> 
> 
> On 10/09/2015 01:26 AM, Zheng, Lv wrote:
> > Please ignore this.
> > The fix is against the caller.
> >
> > Thanks and best regards
> > -Lv
> >
> >> -----Original Message-----
> >> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Zheng, Lv
> >> Sent: Friday, October 09, 2015 10:02 AM
> >> To: Prarit Bhargava; Moore, Robert; devel@acpica.org
> >> Cc: linux-acpi@vger.kernel.org; Wysocki, Rafael J
> >> Subject: Re: [Devel] [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to
> >> find \_Sx should not result in a loud warning [v2]
> >>
> >> Why don't you fix this in the invoker side?
> >> For example:
> >> If (acpi_get_handle())
> >> 	acpi_evaluate_object()
> 
> That seems like a sloppy workaround an actual bug in ACPICA.
> 
> >> So that the AE_NOT_FOUND warning can still be kept for the real
> troubles?
> 
> The code is warning 100% of the time on something that is optional.
> 
> >> There are really scenarios that such warning is useful for catching
> bugs.
> >>
> 
> What scenario is possible where this causes a problem?  Issuing an error
> on something that is optional is not a good idea.
> 
> P.

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Zheng, Lv]

Hi,

> From: Prarit Bhargava [mailto:prarit@redhat.com]
> Sent: Friday, October 09, 2015 7:30 PM
> 
> On 10/09/2015 01:26 AM, Zheng, Lv wrote:
> > Please ignore this.
> > The fix is against the caller.

As I said, the previous email is sent due to lack of confirmation.
So you needn't reply the previous message.

Thanks
-Lv

> >
> > Thanks and best regards
> > -Lv
> >
> >> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Zheng, Lv
> >> Sent: Friday, October 09, 2015 10:02 AM
> >>
> >> Why don't you fix this in the invoker side?
> >> For example:
> >> If (acpi_get_handle())
> >> 	acpi_evaluate_object()
> 
> That seems like a sloppy workaround an actual bug in ACPICA.
> 
> >> So that the AE_NOT_FOUND warning can still be kept for the real troubles?
> 
> The code is warning 100% of the time on something that is optional.
> 
> >> There are really scenarios that such warning is useful for catching bugs.
> >>
> 
> What scenario is possible where this causes a problem?  Issuing an error on
> something that is optional is not a good idea.
> 
> P.

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Zheng, Lv]

Hi,

> From: Prarit Bhargava [mailto:prarit@redhat.com]
> Sent: Friday, October 09, 2015 7:30 PM
> 
> On 10/09/2015 01:26 AM, Zheng, Lv wrote:
> > Please ignore this.
> > The fix is against the caller.
> >
> > Thanks and best regards
> > -Lv
> >
> >> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Zheng, Lv
> >> Sent: Friday, October 09, 2015 10:02 AM
> >>
> >> Why don't you fix this in the invoker side?
> >> For example:
> >> If (acpi_get_handle())
> >> 	acpi_evaluate_object()
> 
> That seems like a sloppy workaround an actual bug in ACPICA.

So let me ignore this which is a reply for a useless question.

> 
> >> So that the AE_NOT_FOUND warning can still be kept for the real troubles?
> 
> The code is warning 100% of the time on something that is optional.

Let me ignore this which is a reply for a useless question.

> 
> >> There are really scenarios that such warning is useful for catching bugs.
> >>
> 
> What scenario is possible where this causes a problem?  Issuing an error on
> something that is optional is not a good idea.

There are several scenarios, let me say some known stuffs about the module level code support.
In AML grammar, module level code (non-object-definition AML opcodes outside of a control method) is legal.
While it is supported in ACPICA in a split way:
1. In the early stage, table loading code only executes those object definition opcodes and links all "if/else/while" AML fragments together.
2. In the late stage, object initialization code will execute such AML fragments before invoking all _INI methods.

1. Since code is not executed in the Windows compliant order during the early table loading stage, some objects are not defined while they should.
2. Even worse, during the early stage, ACPICA may fail the whole table loading process due to AE_NOT_FOUND, leaving us partial initialized objects (the ACPICA AML interpreter implements 2 phases to parse a table to complete the table loading).
Such issue is worked around by deleting the namespace objects created during the early stage if an exception (AE_NOT_FOUND) is encountered.

So you can see, such kind of error is sometime useful for catching AML interpreter issues or BIOS bugs.

Thanks and best regards
-Lv

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Moore, Robert]

I'm a bit confused. What does the execution of the _Sx methods have to do with module-level code? Obviously, many times the _Sx objects are implemented as simply named objects, but the table load doesn't care what is and what isn't present.


> -----Original Message-----
> From: Zheng, Lv
> Sent: Friday, October 09, 2015 6:38 PM
> To: Prarit Bhargava; Moore, Robert; devel@acpica.org
> Cc: linux-acpi@vger.kernel.org; Wysocki, Rafael J
> Subject: RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx
> should not result in a loud warning [v2]
> 
> Hi,
> 
> > From: Prarit Bhargava [mailto:prarit@redhat.com]
> > Sent: Friday, October 09, 2015 7:30 PM
> >
> > On 10/09/2015 01:26 AM, Zheng, Lv wrote:
> > > Please ignore this.
> > > The fix is against the caller.
> > >
> > > Thanks and best regards
> > > -Lv
> > >
> > >> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Zheng,
> > >> Lv
> > >> Sent: Friday, October 09, 2015 10:02 AM
> > >>
> > >> Why don't you fix this in the invoker side?
> > >> For example:
> > >> If (acpi_get_handle())
> > >> 	acpi_evaluate_object()
> >
> > That seems like a sloppy workaround an actual bug in ACPICA.
> 
> So let me ignore this which is a reply for a useless question.
> 
> >
> > >> So that the AE_NOT_FOUND warning can still be kept for the real
> troubles?
> >
> > The code is warning 100% of the time on something that is optional.
> 
> Let me ignore this which is a reply for a useless question.
> 
> >
> > >> There are really scenarios that such warning is useful for catching
> bugs.
> > >>
> >
> > What scenario is possible where this causes a problem?  Issuing an
> > error on something that is optional is not a good idea.
> 
> There are several scenarios, let me say some known stuffs about the module
> level code support.
> In AML grammar, module level code (non-object-definition AML opcodes
> outside of a control method) is legal.
> While it is supported in ACPICA in a split way:
> 1. In the early stage, table loading code only executes those object
> definition opcodes and links all "if/else/while" AML fragments together.
> 2. In the late stage, object initialization code will execute such AML
> fragments before invoking all _INI methods.
> 
> 1. Since code is not executed in the Windows compliant order during the
> early table loading stage, some objects are not defined while they should.
> 2. Even worse, during the early stage, ACPICA may fail the whole table
> loading process due to AE_NOT_FOUND, leaving us partial initialized
> objects (the ACPICA AML interpreter implements 2 phases to parse a table
> to complete the table loading).
> Such issue is worked around by deleting the namespace objects created
> during the early stage if an exception (AE_NOT_FOUND) is encountered.
> 
> So you can see, such kind of error is sometime useful for catching AML
> interpreter issues or BIOS bugs.
> 
> Thanks and best regards
> -Lv

RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]

[Zheng, Lv]

Hi, Bob

> -----Original Message-----
> From: Moore, Robert
> Sent: Wednesday, October 14, 2015 5:00 AM
> To: Zheng, Lv; Prarit Bhargava; devel@acpica.org
> Cc: linux-acpi@vger.kernel.org; Wysocki, Rafael J
> Subject: RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx should not result in a loud warning [v2]
> 
> I'm a bit confused. What does the execution of the _Sx methods have to do with module-level code? Obviously, many times the _Sx
> objects are implemented as simply named objects, but the table load doesn't care what is and what isn't present.
> 

The AE_NOT_FOUND for _Sx should have been fixed.

What we are talking is not related to the _Sx warning.
I was just replying Prarit that why such kind of warning messages could be useful.

Thanks and best regards
-Lv

> 
> > -----Original Message-----
> > From: Zheng, Lv
> > Sent: Friday, October 09, 2015 6:38 PM
> > To: Prarit Bhargava; Moore, Robert; devel@acpica.org
> > Cc: linux-acpi@vger.kernel.org; Wysocki, Rafael J
> > Subject: RE: [PATCH] ACPICA: AcpiGetSleepTypeData: Failure to find \_Sx
> > should not result in a loud warning [v2]
> >
> > Hi,
> >
> > > From: Prarit Bhargava [mailto:prarit@redhat.com]
> > > Sent: Friday, October 09, 2015 7:30 PM
> > >
> > > On 10/09/2015 01:26 AM, Zheng, Lv wrote:
> > > > Please ignore this.
> > > > The fix is against the caller.
> > > >
> > > > Thanks and best regards
> > > > -Lv
> > > >
> > > >> From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of Zheng,
> > > >> Lv
> > > >> Sent: Friday, October 09, 2015 10:02 AM
> > > >>
> > > >> Why don't you fix this in the invoker side?
> > > >> For example:
> > > >> If (acpi_get_handle())
> > > >> 	acpi_evaluate_object()
> > >
> > > That seems like a sloppy workaround an actual bug in ACPICA.
> >
> > So let me ignore this which is a reply for a useless question.
> >
> > >
> > > >> So that the AE_NOT_FOUND warning can still be kept for the real
> > troubles?
> > >
> > > The code is warning 100% of the time on something that is optional.
> >
> > Let me ignore this which is a reply for a useless question.
> >
> > >
> > > >> There are really scenarios that such warning is useful for catching
> > bugs.
> > > >>
> > >
> > > What scenario is possible where this causes a problem?  Issuing an
> > > error on something that is optional is not a good idea.
> >
> > There are several scenarios, let me say some known stuffs about the module
> > level code support.
> > In AML grammar, module level code (non-object-definition AML opcodes
> > outside of a control method) is legal.
> > While it is supported in ACPICA in a split way:
> > 1. In the early stage, table loading code only executes those object
> > definition opcodes and links all "if/else/while" AML fragments together.
> > 2. In the late stage, object initialization code will execute such AML
> > fragments before invoking all _INI methods.
> >
> > 1. Since code is not executed in the Windows compliant order during the
> > early table loading stage, some objects are not defined while they should.
> > 2. Even worse, during the early stage, ACPICA may fail the whole table
> > loading process due to AE_NOT_FOUND, leaving us partial initialized
> > objects (the ACPICA AML interpreter implements 2 phases to parse a table
> > to complete the table loading).
> > Such issue is worked around by deleting the namespace objects created
> > during the early stage if an exception (AE_NOT_FOUND) is encountered.
> >
> > So you can see, such kind of error is sometime useful for catching AML
> > interpreter issues or BIOS bugs.
> >
> > Thanks and best regards
> > -Lv