On Tue, Feb 13, 2018 at 5:03 AM, Alex Hung <alex.hung@xxxxxxxxxxxxx> wrote: > On Mon, Feb 12, 2018 at 12:51 AM, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote: >> On Sat, Feb 10, 2018 at 4:34 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html