| Message ID | 1518217003-19637-1-git-send-email-alex.hung@canonical.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Fri, Feb 09, 2018 at 02:56:43PM -0800, Alex Hung wrote: > In recent Intel hardware the IRQs become non-configurable after BIOS > initializes them in PEI phase and _PRS objects are no longer included in > ASL. > > This is the same as "static (non-configurable) devices do not > specify a _PRS object" in ACPI spec. As a result, error messages > saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to > be in kernel messages all the time but only when debug is enabled. I agree and would even go further: _PRS is optional and I don't think there's a reason to log anything at all if it's absent. A log message like "failed to evaluate _PRS" makes people think something is wrong when in fact nothing is wrong. That leads to the mindset of treating a missing _PRS as an error when it's not. In fact, it looks like acpi_pci_link_add() *does* treat this as an error. If _PRS doesn't exist, it skips the _CRS evaluation. That seems wrong. > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > drivers/acpi/pci_link.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c > index 85ad679..9d9cf24 100644 > --- a/drivers/acpi/pci_link.c > +++ b/drivers/acpi/pci_link.c > @@ -173,7 +173,7 @@ static int acpi_pci_link_get_possible(struct acpi_pci_link *link) > status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS, > acpi_pci_link_check_possible, link); > if (ACPI_FAILURE(status)) { > - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); > + acpi_handle_debug(link->device->handle, "failed to evaluate _PRS"); > return -ENODEV; > } > > -- > 2.7.4 > -- 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 Fri, Feb 9, 2018 at 5:05 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: > On Fri, Feb 09, 2018 at 02:56:43PM -0800, Alex Hung wrote: >> In recent Intel hardware the IRQs become non-configurable after BIOS >> initializes them in PEI phase and _PRS objects are no longer included in >> ASL. >> >> This is the same as "static (non-configurable) devices do not >> specify a _PRS object" in ACPI spec. As a result, error messages >> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to >> be in kernel messages all the time but only when debug is enabled. > > I agree and would even go further: _PRS is optional and I don't think > there's a reason to log anything at all if it's absent. A log message > like "failed to evaluate _PRS" makes people think something is wrong > when in fact nothing is wrong. Bjorn, Thanks for the feedback. Rafael and I had discussion on the previous patch that removed the error message (https://patchwork.codeaurora.org/patch/440263/), and had a conclusion that using a level log of "debug" is more appropriate and less noisy for most of default setting. After all, there can be other failure types than _PRS is absent. > > That leads to the mindset of treating a missing _PRS as an error when > it's not. In fact, it looks like acpi_pci_link_add() *does* treat > this as an error. If _PRS doesn't exist, it skips the _CRS > evaluation. That seems wrong. Do you mean we can do something like status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS, acpi_pci_link_check_possible, link); if (status == AE_NOT_FOUND) { // acceptable scenario but let's still output a message acpi_handle_debug(link->device->handle, "_PRS is absent"); } else if (ACPI_FAILURE(status)) { // something indeed wrong with _PRS acpi_handle_debug(link->device->handle, "failed to evaluate _PRS"); return -ENODEV; } or status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS, acpi_pci_link_check_possible, link); if (ACPI_FAILURE(status)) { // output messages but do not return -ENODEV acpi_handle_debug(link->device->handle, "something wrong with _PRS, so let's not use it"); // or a more meaningful message } and plus other probable changes and many tests as this affects other parts of pci_link.c Perhaps we can do this in two patches: 1. fix the error messages first - low risk and nobody freaks out with the new hardware 2. refine _PRS & _CRT because of higher risk and extensive testing required Rafael and Len Any comments and suggestions on error messages or _CRS? > >> Signed-off-by: Alex Hung <alex.hung@canonical.com> >> --- >> drivers/acpi/pci_link.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c >> index 85ad679..9d9cf24 100644 >> --- a/drivers/acpi/pci_link.c >> +++ b/drivers/acpi/pci_link.c >> @@ -173,7 +173,7 @@ static int acpi_pci_link_get_possible(struct acpi_pci_link *link) >> status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS, >> acpi_pci_link_check_possible, link); >> if (ACPI_FAILURE(status)) { >> - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); >> + acpi_handle_debug(link->device->handle, "failed to evaluate _PRS"); >> return -ENODEV; >> } >> >> -- >> 2.7.4 >>
On Sat, Feb 10, 2018 at 2:05 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: > On Fri, Feb 09, 2018 at 02:56:43PM -0800, Alex Hung wrote: >> In recent Intel hardware the IRQs become non-configurable after BIOS >> initializes them in PEI phase and _PRS objects are no longer included in >> ASL. >> >> This is the same as "static (non-configurable) devices do not >> specify a _PRS object" in ACPI spec. As a result, error messages >> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to >> be in kernel messages all the time but only when debug is enabled. > > I agree and would even go further: _PRS is optional and I don't think > there's a reason to log anything at all if it's absent. A log message > like "failed to evaluate _PRS" makes people think something is wrong > when in fact nothing is wrong. _PRS is required if the link object is pointed to by a _PRT entry. > That leads to the mindset of treating a missing _PRS as an error when > it's not. In fact, it looks like acpi_pci_link_add() *does* treat > this as an error. If _PRS doesn't exist, it skips the _CRS > evaluation. That seems wrong. I agree here. _CRS still should be evaluated if _PRS is not there in general, but in some cases the lack of _PRS *is* an error. -- 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 Sat, Feb 10, 2018 at 09:08:42AM +0100, Rafael J. Wysocki wrote: > On Sat, Feb 10, 2018 at 2:05 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Fri, Feb 09, 2018 at 02:56:43PM -0800, Alex Hung wrote: > >> In recent Intel hardware the IRQs become non-configurable after BIOS > >> initializes them in PEI phase and _PRS objects are no longer included in > >> ASL. > >> > >> This is the same as "static (non-configurable) devices do not > >> specify a _PRS object" in ACPI spec. As a result, error messages > >> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to > >> be in kernel messages all the time but only when debug is enabled. > > > > I agree and would even go further: _PRS is optional and I don't think > > there's a reason to log anything at all if it's absent. A log message > > like "failed to evaluate _PRS" makes people think something is wrong > > when in fact nothing is wrong. > > _PRS is required if the link object is pointed to by a _PRT entry. The closest thing I see with a quick look is ACPI 6.0, sec 6.2.13: These objects [PCI Interrupt Link Devices] have _PRS, _CRS, _SRS, and _DIS control methods to allocate the interrupt. I don't read that as a requirement for _PRS in particular; I read it as saying that the interrupt link devices use the normal device configuration method, i.e., _CRS is required and _PRS and _SRS are optional and needed for configurable devices but not for static ones. But this is just a drive-by comment and I'm really fine with whatever you decide to do. > > That leads to the mindset of treating a missing _PRS as an error when > > it's not. In fact, it looks like acpi_pci_link_add() *does* treat > > this as an error. If _PRS doesn't exist, it skips the _CRS > > evaluation. That seems wrong. > > I agree here. _CRS still should be evaluated if _PRS is not there in > general, but in some cases the lack of _PRS *is* an error. -- 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 Sat, Feb 10, 2018 at 4:34 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: > On Sat, Feb 10, 2018 at 09:08:42AM +0100, Rafael J. Wysocki wrote: >> On Sat, Feb 10, 2018 at 2:05 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: >> > On Fri, Feb 09, 2018 at 02:56:43PM -0800, Alex Hung wrote: >> >> In recent Intel hardware the IRQs become non-configurable after BIOS >> >> initializes them in PEI phase and _PRS objects are no longer included in >> >> ASL. >> >> >> >> This is the same as "static (non-configurable) devices do not >> >> specify a _PRS object" in ACPI spec. As a result, error messages >> >> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to >> >> be in kernel messages all the time but only when debug is enabled. >> > >> > I agree and would even go further: _PRS is optional and I don't think >> > there's a reason to log anything at all if it's absent. A log message >> > like "failed to evaluate _PRS" makes people think something is wrong >> > when in fact nothing is wrong. >> >> _PRS is required if the link object is pointed to by a _PRT entry. > > The closest thing I see with a quick look is ACPI 6.0, sec 6.2.13: > > These objects [PCI Interrupt Link Devices] have _PRS, _CRS, _SRS, > and _DIS control methods to allocate the interrupt. > > I don't read that as a requirement for _PRS in particular; I read it > as saying that the interrupt link devices use the normal device > configuration method, i.e., _CRS is required and _PRS and _SRS are > optional and needed for configurable devices but not for static ones. Well, that's slightly out of context. :-) First of all, Section 6.2.13 says this: "Typically, the interrupt input that a given PCI interrupt is on is configurable. [...] In this model, each interrupt is represented in the ACPI namespace as a PCI Interrupt Link Device." Then, it says the above. Now, if _PRS is not present, the IRQ represented by the link object cannot be configured, so in fact the underlying assumption doesn't apply to it, strictly speaking. It follows from the last paragraph in Section 6.2.13 that _PRT entries representing such IRQs should not point to interrupt link device objects (there should be 0 in their Source fields). Hence, if a _PRT entry points to an interrupt link device object in the namespace, _PRS should be present under this object or at least it is reasonable to expect that it will be present in there. > But this is just a drive-by comment and I'm really fine with whatever > you decide to do. OK So I think that (a) the message should be printed using acpi_handle_debug(), except that I would make it slightly more neutral, something like "_PRS not present or invalid", and (b) 0 should be returned instead of -ENODEV when _PRS evaluation doesn't succeed. -- 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 Mon, Feb 12, 2018 at 9:51 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Sat, Feb 10, 2018 at 4:34 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: >> On Sat, Feb 10, 2018 at 09:08:42AM +0100, Rafael J. Wysocki wrote: >>> On Sat, Feb 10, 2018 at 2:05 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: >>> > On Fri, Feb 09, 2018 at 02:56:43PM -0800, Alex Hung wrote: >>> >> In recent Intel hardware the IRQs become non-configurable after BIOS >>> >> initializes them in PEI phase and _PRS objects are no longer included in >>> >> ASL. >>> >> >>> >> This is the same as "static (non-configurable) devices do not >>> >> specify a _PRS object" in ACPI spec. As a result, error messages >>> >> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to >>> >> be in kernel messages all the time but only when debug is enabled. >>> > >>> > I agree and would even go further: _PRS is optional and I don't think >>> > there's a reason to log anything at all if it's absent. A log message >>> > like "failed to evaluate _PRS" makes people think something is wrong >>> > when in fact nothing is wrong. >>> >>> _PRS is required if the link object is pointed to by a _PRT entry. >> >> The closest thing I see with a quick look is ACPI 6.0, sec 6.2.13: >> >> These objects [PCI Interrupt Link Devices] have _PRS, _CRS, _SRS, >> and _DIS control methods to allocate the interrupt. >> >> I don't read that as a requirement for _PRS in particular; I read it >> as saying that the interrupt link devices use the normal device >> configuration method, i.e., _CRS is required and _PRS and _SRS are >> optional and needed for configurable devices but not for static ones. > > Well, that's slightly out of context. :-) > > First of all, Section 6.2.13 says this: "Typically, the interrupt > input that a given PCI interrupt is on is configurable. [...] In this > model, each interrupt is represented in the ACPI namespace as a PCI > Interrupt Link Device." Then, it says the above. > > Now, if _PRS is not present, the IRQ represented by the link object > cannot be configured, so in fact the underlying assumption doesn't > apply to it, strictly speaking. It follows from the last paragraph in > Section 6.2.13 that _PRT entries representing such IRQs should not > point to interrupt link device objects (there should be 0 in their > Source fields). > > Hence, if a _PRT entry points to an interrupt link device object in > the namespace, _PRS should be present under this object or at least it > is reasonable to expect that it will be present in there. > >> But this is just a drive-by comment and I'm really fine with whatever >> you decide to do. > > OK > > So I think that (a) the message should be printed using > acpi_handle_debug(), except that I would make it slightly more > neutral, something like "_PRS not present or invalid", and (b) 0 > should be returned instead of -ENODEV when _PRS evaluation doesn't > succeed. But then, of course, we may not want to evaluate _DIS on this link and I'm not quite sure if adding it to acpi_link_list makes sense then. -- 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 Mon, Feb 12, 2018 at 12:51 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Sat, Feb 10, 2018 at 4:34 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: >> On Sat, Feb 10, 2018 at 09:08:42AM +0100, Rafael J. Wysocki wrote: >>> On Sat, Feb 10, 2018 at 2:05 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: >>> > On Fri, Feb 09, 2018 at 02:56:43PM -0800, Alex Hung wrote: >>> >> In recent Intel hardware the IRQs become non-configurable after BIOS >>> >> initializes them in PEI phase and _PRS objects are no longer included in >>> >> ASL. >>> >> >>> >> This is the same as "static (non-configurable) devices do not >>> >> specify a _PRS object" in ACPI spec. As a result, error messages >>> >> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to >>> >> be in kernel messages all the time but only when debug is enabled. >>> > >>> > I agree and would even go further: _PRS is optional and I don't think >>> > there's a reason to log anything at all if it's absent. A log message >>> > like "failed to evaluate _PRS" makes people think something is wrong >>> > when in fact nothing is wrong. >>> >>> _PRS is required if the link object is pointed to by a _PRT entry. >> >> The closest thing I see with a quick look is ACPI 6.0, sec 6.2.13: >> >> These objects [PCI Interrupt Link Devices] have _PRS, _CRS, _SRS, >> and _DIS control methods to allocate the interrupt. >> >> I don't read that as a requirement for _PRS in particular; I read it >> as saying that the interrupt link devices use the normal device >> configuration method, i.e., _CRS is required and _PRS and _SRS are >> optional and needed for configurable devices but not for static ones. > > Well, that's slightly out of context. :-) > > First of all, Section 6.2.13 says this: "Typically, the interrupt > input that a given PCI interrupt is on is configurable. [...] In this > model, each interrupt is represented in the ACPI namespace as a PCI > Interrupt Link Device." Then, it says the above. > > Now, if _PRS is not present, the IRQ represented by the link object > cannot be configured, so in fact the underlying assumption doesn't > apply to it, strictly speaking. It follows from the last paragraph in > Section 6.2.13 that _PRT entries representing such IRQs should not > point to interrupt link device objects (there should be 0 in their > Source fields). > > Hence, if a _PRT entry points to an interrupt link device object in > the namespace, _PRS should be present under this object or at least it > is reasonable to expect that it will be present in there. > >> But this is just a drive-by comment and I'm really fine with whatever >> you decide to do. > > OK > > So I think that (a) the message should be printed using > acpi_handle_debug(), except that I would make it slightly more > neutral, something like "_PRS not present or invalid", and (b) 0 > should be returned instead of -ENODEV when _PRS evaluation doesn't > succeed. Looks like we will do the following. if (ACPI_FAILURE(status)) { - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); - return -ENODEV; + acpi_handle_debug(link->device->handle, "_PRS not present or invalid"); + return 0; I will revise the patch and send it if this is all agreed. -- 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 Tue, Feb 13, 2018 at 5:03 AM, Alex Hung <alex.hung@canonical.com> wrote: > On Mon, Feb 12, 2018 at 12:51 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: >> On Sat, Feb 10, 2018 at 4:34 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: >>> On Sat, Feb 10, 2018 at 09:08:42AM +0100, Rafael J. Wysocki wrote: >>>> On Sat, Feb 10, 2018 at 2:05 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: >>>> > On Fri, Feb 09, 2018 at 02:56:43PM -0800, Alex Hung wrote: >>>> >> In recent Intel hardware the IRQs become non-configurable after BIOS >>>> >> initializes them in PEI phase and _PRS objects are no longer included in >>>> >> ASL. >>>> >> >>>> >> This is the same as "static (non-configurable) devices do not >>>> >> specify a _PRS object" in ACPI spec. As a result, error messages >>>> >> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to >>>> >> be in kernel messages all the time but only when debug is enabled. >>>> > >>>> > I agree and would even go further: _PRS is optional and I don't think >>>> > there's a reason to log anything at all if it's absent. A log message >>>> > like "failed to evaluate _PRS" makes people think something is wrong >>>> > when in fact nothing is wrong. >>>> >>>> _PRS is required if the link object is pointed to by a _PRT entry. >>> >>> The closest thing I see with a quick look is ACPI 6.0, sec 6.2.13: >>> >>> These objects [PCI Interrupt Link Devices] have _PRS, _CRS, _SRS, >>> and _DIS control methods to allocate the interrupt. >>> >>> I don't read that as a requirement for _PRS in particular; I read it >>> as saying that the interrupt link devices use the normal device >>> configuration method, i.e., _CRS is required and _PRS and _SRS are >>> optional and needed for configurable devices but not for static ones. >> >> Well, that's slightly out of context. :-) >> >> First of all, Section 6.2.13 says this: "Typically, the interrupt >> input that a given PCI interrupt is on is configurable. [...] In this >> model, each interrupt is represented in the ACPI namespace as a PCI >> Interrupt Link Device." Then, it says the above. >> >> Now, if _PRS is not present, the IRQ represented by the link object >> cannot be configured, so in fact the underlying assumption doesn't >> apply to it, strictly speaking. It follows from the last paragraph in >> Section 6.2.13 that _PRT entries representing such IRQs should not >> point to interrupt link device objects (there should be 0 in their >> Source fields). >> >> Hence, if a _PRT entry points to an interrupt link device object in >> the namespace, _PRS should be present under this object or at least it >> is reasonable to expect that it will be present in there. >> >>> But this is just a drive-by comment and I'm really fine with whatever >>> you decide to do. >> >> OK >> >> So I think that (a) the message should be printed using >> acpi_handle_debug(), except that I would make it slightly more >> neutral, something like "_PRS not present or invalid", and (b) 0 >> should be returned instead of -ENODEV when _PRS evaluation doesn't >> succeed. > > Looks like we will do the following. > > if (ACPI_FAILURE(status)) { > - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); > - return -ENODEV; > + acpi_handle_debug(link->device->handle, "_PRS not > present or invalid"); > + return 0; Yes, but this affects the caller. Please make sure that everything will work correctly in there after this change. -- 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 Tue, Feb 13, 2018 at 12:49 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: > On Tue, Feb 13, 2018 at 5:03 AM, Alex Hung <alex.hung@canonical.com> wrote: >> On Mon, Feb 12, 2018 at 12:51 AM, Rafael J. Wysocki <rafael@kernel.org> wrote: >>> On Sat, Feb 10, 2018 at 4:34 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: >>>> On Sat, Feb 10, 2018 at 09:08:42AM +0100, Rafael J. Wysocki wrote: >>>>> On Sat, Feb 10, 2018 at 2:05 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: >>>>> > On Fri, Feb 09, 2018 at 02:56:43PM -0800, Alex Hung wrote: >>>>> >> In recent Intel hardware the IRQs become non-configurable after BIOS >>>>> >> initializes them in PEI phase and _PRS objects are no longer included in >>>>> >> ASL. >>>>> >> >>>>> >> This is the same as "static (non-configurable) devices do not >>>>> >> specify a _PRS object" in ACPI spec. As a result, error messages >>>>> >> saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to >>>>> >> be in kernel messages all the time but only when debug is enabled. >>>>> > >>>>> > I agree and would even go further: _PRS is optional and I don't think >>>>> > there's a reason to log anything at all if it's absent. A log message >>>>> > like "failed to evaluate _PRS" makes people think something is wrong >>>>> > when in fact nothing is wrong. >>>>> >>>>> _PRS is required if the link object is pointed to by a _PRT entry. >>>> >>>> The closest thing I see with a quick look is ACPI 6.0, sec 6.2.13: >>>> >>>> These objects [PCI Interrupt Link Devices] have _PRS, _CRS, _SRS, >>>> and _DIS control methods to allocate the interrupt. >>>> >>>> I don't read that as a requirement for _PRS in particular; I read it >>>> as saying that the interrupt link devices use the normal device >>>> configuration method, i.e., _CRS is required and _PRS and _SRS are >>>> optional and needed for configurable devices but not for static ones. >>> >>> Well, that's slightly out of context. :-) >>> >>> First of all, Section 6.2.13 says this: "Typically, the interrupt >>> input that a given PCI interrupt is on is configurable. [...] In this >>> model, each interrupt is represented in the ACPI namespace as a PCI >>> Interrupt Link Device." Then, it says the above. >>> >>> Now, if _PRS is not present, the IRQ represented by the link object >>> cannot be configured, so in fact the underlying assumption doesn't >>> apply to it, strictly speaking. It follows from the last paragraph in >>> Section 6.2.13 that _PRT entries representing such IRQs should not >>> point to interrupt link device objects (there should be 0 in their >>> Source fields). >>> >>> Hence, if a _PRT entry points to an interrupt link device object in >>> the namespace, _PRS should be present under this object or at least it >>> is reasonable to expect that it will be present in there. >>> >>>> But this is just a drive-by comment and I'm really fine with whatever >>>> you decide to do. >>> >>> OK >>> >>> So I think that (a) the message should be printed using >>> acpi_handle_debug(), except that I would make it slightly more >>> neutral, something like "_PRS not present or invalid", and (b) 0 >>> should be returned instead of -ENODEV when _PRS evaluation doesn't >>> succeed. >> >> Looks like we will do the following. >> >> if (ACPI_FAILURE(status)) { >> - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); >> - return -ENODEV; >> + acpi_handle_debug(link->device->handle, "_PRS not >> present or invalid"); >> + return 0; > > Yes, but this affects the caller. > > Please make sure that everything will work correctly in there after this change. I tried the above changes and everything seems to work fine. I will send updated patch shortly. -- 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/pci_link.c b/drivers/acpi/pci_link.c index 85ad679..9d9cf24 100644 --- a/drivers/acpi/pci_link.c +++ b/drivers/acpi/pci_link.c @@ -173,7 +173,7 @@ static int acpi_pci_link_get_possible(struct acpi_pci_link *link) status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS, acpi_pci_link_check_possible, link); if (ACPI_FAILURE(status)) { - ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRS")); + acpi_handle_debug(link->device->handle, "failed to evaluate _PRS"); return -ENODEV; }
In recent Intel hardware the IRQs become non-configurable after BIOS initializes them in PEI phase and _PRS objects are no longer included in ASL. This is the same as "static (non-configurable) devices do not specify a _PRS object" in ACPI spec. As a result, error messages saying "ACPI Exception: AE_NOT_FOUND, Evaluating _PRS" does not need to be in kernel messages all the time but only when debug is enabled. Signed-off-by: Alex Hung <alex.hung@canonical.com> --- drivers/acpi/pci_link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)