diff mbox

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

Message ID 1443720528-14499-1-git-send-email-prarit@redhat.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Prarit Bhargava Oct. 1, 2015, 5:28 p.m. UTC
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(-)

Comments

Moore, Robert Oct. 1, 2015, 5:57 p.m. UTC | #1
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

--
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
Wysocki, Rafael J Oct. 1, 2015, 10:28 p.m. UTC | #2
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

--
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
Prarit Bhargava Oct. 7, 2015, 3:28 p.m. UTC | #3
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.
--
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
Moore, Robert Oct. 7, 2015, 3:52 p.m. UTC | #4
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.
--
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
Prarit Bhargava Oct. 7, 2015, 3:57 p.m. UTC | #5
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.
--
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
Moore, Robert Oct. 7, 2015, 4:27 p.m. UTC | #6
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.
--
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
diff mbox

Patch

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);
 }