| Message ID | 1444243849-27929-1-git-send-email-prarit@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 -- 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
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
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 -- 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
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 -- 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
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. -- 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
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. -- 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
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. -- 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
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 -- 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
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 -- 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
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 -- 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 --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); }
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(-)