diff mbox

[V3] ACPI/PCI: pci_link: allow the absence of _PSR and change log level

Message ID 1519364624-4363-1-git-send-email-alex.hung@canonical.com (mailing list archive)
State Not Applicable, archived
Delegated to: Rafael Wysocki
Headers show

Commit Message

Alex Hung Feb. 23, 2018, 5:43 a.m. UTC
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 messenges all the time but only when debug is enabled, and
acpi_pci_link_get_possible should not return -ENODEV when _PRS is
absent.

Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 drivers/acpi/pci_link.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Bjorn Helgaas Feb. 23, 2018, 2:50 p.m. UTC | #1
In subject:

s/allow the/Allow the/ to match drivers/acpi capitalization convention
s/_PSR/_PRS/ to fix typo

On Thu, Feb 22, 2018 at 09:43:44PM -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 messenges all the time but only when debug is enabled, and
> acpi_pci_link_get_possible should not return -ENODEV when _PRS is
> absent.
> 
> Signed-off-by: Alex Hung <alex.hung@canonical.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/acpi/pci_link.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index bc3d914..65f1908 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -173,8 +173,8 @@ 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"));
> -		return -ENODEV;
> +		acpi_handle_debug(link->device->handle, "_PRS not present or invalid");

The rest of this file uses ACPI_EXCEPTION(), ACPI_DEBUG_PRINT(), etc.
My personal preference would be to keep the file consistent, e.g.,
maybe use the style that matches the file in this patch, and have a
second patch that changes all the logging in the file to use
acpi_handle_*().

But I'm fine with whatever you and Rafael want to do about this.

> +		return 0;
>  	}
>  
>  	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> -- 
> 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
--
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
Rafael J. Wysocki March 14, 2018, 11:43 a.m. UTC | #2
On Friday, February 23, 2018 3:50:26 PM CET Bjorn Helgaas wrote:
> In subject:
> 
> s/allow the/Allow the/ to match drivers/acpi capitalization convention
> s/_PSR/_PRS/ to fix typo
> 
> On Thu, Feb 22, 2018 at 09:43:44PM -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 messenges all the time but only when debug is enabled, and
> > acpi_pci_link_get_possible should not return -ENODEV when _PRS is
> > absent.
> > 
> > Signed-off-by: Alex Hung <alex.hung@canonical.com>
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> > ---
> >  drivers/acpi/pci_link.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> > index bc3d914..65f1908 100644
> > --- a/drivers/acpi/pci_link.c
> > +++ b/drivers/acpi/pci_link.c
> > @@ -173,8 +173,8 @@ 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"));
> > -		return -ENODEV;
> > +		acpi_handle_debug(link->device->handle, "_PRS not present or invalid");
> 
> The rest of this file uses ACPI_EXCEPTION(), ACPI_DEBUG_PRINT(), etc.
> My personal preference would be to keep the file consistent, e.g.,
> maybe use the style that matches the file in this patch, and have a
> second patch that changes all the logging in the file to use
> acpi_handle_*().
> 
> But I'm fine with whatever you and Rafael want to do about this.

I wanted it to be changed.

Using ACPI_DEBUG_PRINT() and similar outside of ACPICA code is
questionable and plain problematic with some combinations of Kconfig
options, so I'd actually prefer them to go away from the ACPI code
outside of ACPICA.

Patch applied with the subject typo fixed, thanks!

--
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/pci_link.c b/drivers/acpi/pci_link.c
index bc3d914..65f1908 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -173,8 +173,8 @@  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"));
-		return -ENODEV;
+		acpi_handle_debug(link->device->handle, "_PRS not present or invalid");
+		return 0;
 	}
 
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,