linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/4] ACPI: PCI: Unify printing of messages
@ 2021-02-19 18:14 Rafael J. Wysocki
  2021-02-19 18:15 ` [PATCH v1 1/4] ACPI: PCI: IRQ: Consolidate printing diagnostic messages Rafael J. Wysocki
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-02-19 18:14 UTC (permalink / raw)
  To: Linux ACPI; +Cc: Linux PCI, LKML, Bjorn Helgaas, Hanjun Guo

Hi All,

This series gets rid of ACPICA debugging from non-ACPICA code related to PCI
(patches [1-3/4]) and replaces direct printk() usage in pci_link.c with
pr_*() or acpi_handle_*().

Please refer to the patch changelogs for details.

Thanks!




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v1 1/4] ACPI: PCI: IRQ: Consolidate printing diagnostic messages
  2021-02-19 18:14 [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Rafael J. Wysocki
@ 2021-02-19 18:15 ` Rafael J. Wysocki
  2021-02-19 18:16 ` [PATCH v1 2/4] ACPI: PCI: Replace ACPI_DEBUG_PRINT() and ACPI_EXCEPTION() Rafael J. Wysocki
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-02-19 18:15 UTC (permalink / raw)
  To: Linux ACPI; +Cc: Linux PCI, LKML, Bjorn Helgaas, Hanjun Guo

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The code in pci_irq.c prints diagnostic messages using different
and inconsistent methods.  The majority of them are printed with
the help of the dev_*() familiy of logging functions, but
ACPI_DEBUG_PRINT() and ACPI_DEBUG_PRINT_RAW() are still used in
some places which requires the ACPICA debug to be enabled
additionally which is a nuisance and one message is printed
using the raw printk().

To consolidate the printing of messages in that code, convert all of
the ACPI_DEBUG_PRINT() instances in it into dev_dbg(), which is
consistent with the way the other messages are printed by it,
replace the only ACPI_DEBUG_PRINT_RAW() instance with pr_debug() and
make it use pr_warn() istead of printk(KERN_WARNING ).

Also add a pr_fmt() definition to that file and drop the
_COMPONENT and ACPI_MODULE_NAME() definitions that are not used
any more after the above changes.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/pci_irq.c |   34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

Index: linux-pm/drivers/acpi/pci_irq.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_irq.c
+++ linux-pm/drivers/acpi/pci_irq.c
@@ -9,6 +9,7 @@
  *	Bjorn Helgaas <bjorn.helgaas@hp.com>
  */
 
+#define pr_fmt(fmt) "ACPI: PCI: " fmt
 
 #include <linux/dmi.h>
 #include <linux/kernel.h>
@@ -22,11 +23,6 @@
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 
-#define PREFIX "ACPI: "
-
-#define _COMPONENT		ACPI_PCI_COMPONENT
-ACPI_MODULE_NAME("pci_irq");
-
 struct acpi_prt_entry {
 	struct acpi_pci_id	id;
 	u8			pin;
@@ -126,7 +122,7 @@ static void do_prt_fixups(struct acpi_pr
 		    entry->pin == quirk->pin &&
 		    !strcmp(prt->source, quirk->source) &&
 		    strlen(prt->source) >= strlen(quirk->actual_source)) {
-			printk(KERN_WARNING PREFIX "firmware reports "
+			pr_warn("Firmware reports "
 				"%04x:%02x:%02x PCI INT %c connected to %s; "
 				"changing to %s\n",
 				entry->id.segment, entry->id.bus,
@@ -191,12 +187,9 @@ static int acpi_pci_irq_check_entry(acpi
 	 * the IRQ value, which is hardwired to specific interrupt inputs on
 	 * the interrupt controller.
 	 */
-
-	ACPI_DEBUG_PRINT_RAW((ACPI_DB_INFO,
-			      "      %04x:%02x:%02x[%c] -> %s[%d]\n",
-			      entry->id.segment, entry->id.bus,
-			      entry->id.device, pin_name(entry->pin),
-			      prt->source, entry->index));
+	pr_debug("%04x:%02x:%02x[%c] -> %s[%d]\n",
+		 entry->id.segment, entry->id.bus, entry->id.device,
+		 pin_name(entry->pin), prt->source, entry->index);
 
 	*entry_ptr = entry;
 
@@ -307,8 +300,7 @@ static struct acpi_prt_entry *acpi_pci_i
 #ifdef CONFIG_X86_IO_APIC
 		acpi_reroute_boot_interrupt(dev, entry);
 #endif /* CONFIG_X86_IO_APIC */
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found %s[%c] _PRT entry\n",
-				  pci_name(dev), pin_name(pin)));
+		dev_dbg(&dev->dev, "Found [%c] _PRT entry\n", pin_name(pin));
 		return entry;
 	}
 
@@ -324,9 +316,7 @@ static struct acpi_prt_entry *acpi_pci_i
 			/* PC card has the same IRQ as its cardbridge */
 			bridge_pin = bridge->pin;
 			if (!bridge_pin) {
-				ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-						  "No interrupt pin configured for device %s\n",
-						  pci_name(bridge)));
+				dev_dbg(&bridge->dev, "No interrupt pin configured\n");
 				return NULL;
 			}
 			pin = bridge_pin;
@@ -334,10 +324,8 @@ static struct acpi_prt_entry *acpi_pci_i
 
 		ret = acpi_pci_irq_find_prt_entry(bridge, pin, &entry);
 		if (!ret && entry) {
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-					 "Derived GSI for %s INT %c from %s\n",
-					 pci_name(dev), pin_name(orig_pin),
-					 pci_name(bridge)));
+			dev_dbg(&dev->dev, "Derived GSI INT %c from %s\n",
+				pin_name(orig_pin), pci_name(bridge));
 			return entry;
 		}
 
@@ -413,9 +401,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 
 	pin = dev->pin;
 	if (!pin) {
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-				  "No interrupt pin configured for device %s\n",
-				  pci_name(dev)));
+		dev_dbg(&dev->dev, "No interrupt pin configured\n");
 		return 0;
 	}
 




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v1 2/4] ACPI: PCI: Replace ACPI_DEBUG_PRINT() and ACPI_EXCEPTION()
  2021-02-19 18:14 [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Rafael J. Wysocki
  2021-02-19 18:15 ` [PATCH v1 1/4] ACPI: PCI: IRQ: Consolidate printing diagnostic messages Rafael J. Wysocki
@ 2021-02-19 18:16 ` Rafael J. Wysocki
  2021-02-19 18:16 ` [PATCH v1 3/4] ACPI: PCI: Drop ACPI_PCI_COMPONENT that is not used any more Rafael J. Wysocki
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-02-19 18:16 UTC (permalink / raw)
  To: Linux ACPI; +Cc: Linux PCI, LKML, Bjorn Helgaas, Hanjun Guo

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The ACPI_DEBUG_PRINT() and ACPI_EXCEPTION() macros are used for
message printing in the ACPICA code and they should not be used
elsewhere.  Special configuration (either kernel command line or
sysfs-based) is needed to see the messages printed by them and
the format of those messages is also special and convoluted.

For this reason, replace all of the ACPI_DEBUG_PRINT() and
ACPI_EXCEPTION() instances in pci_link.c with acpi_handle_*() calls
relative to the ACPI handle of the given link device (wherever that
handle is readily available) or pr_debug() invocations.

While at it, make acpi_pci_link_check_current() print all messages
with pr_debug(), because all of them are in the same category (_CRS
return buffer issues) and they all should be printed at the same log
level.

Also make acpi_pci_link_set() use acpi_handle_*() for printing all
messages for consistency.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/pci_link.c |   80 +++++++++++++++++++++---------------------------
 1 file changed, 36 insertions(+), 44 deletions(-)

Index: linux-pm/drivers/acpi/pci_link.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_link.c
+++ linux-pm/drivers/acpi/pci_link.c
@@ -27,8 +27,6 @@
 
 #include "internal.h"
 
-#define _COMPONENT			ACPI_PCI_COMPONENT
-ACPI_MODULE_NAME("pci_link");
 #define ACPI_PCI_LINK_CLASS		"pci_irq_routing"
 #define ACPI_PCI_LINK_DEVICE_NAME	"PCI Interrupt Link"
 #define ACPI_PCI_LINK_MAX_POSSIBLE	16
@@ -85,6 +83,7 @@ static acpi_status acpi_pci_link_check_p
 						void *context)
 {
 	struct acpi_pci_link *link = context;
+	acpi_handle handle = link->device->handle;
 	u32 i;
 
 	switch (resource->type) {
@@ -95,8 +94,8 @@ static acpi_status acpi_pci_link_check_p
 		{
 			struct acpi_resource_irq *p = &resource->data.irq;
 			if (!p || !p->interrupt_count) {
-				ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-						  "Blank _PRS IRQ resource\n"));
+				acpi_handle_debug(handle,
+						  "Blank _PRS IRQ resource\n");
 				return AE_OK;
 			}
 			for (i = 0;
@@ -153,18 +152,18 @@ static acpi_status acpi_pci_link_check_p
 
 static int acpi_pci_link_get_possible(struct acpi_pci_link *link)
 {
+	acpi_handle handle = link->device->handle;
 	acpi_status status;
 
-	status = acpi_walk_resources(link->device->handle, METHOD_NAME__PRS,
+	status = acpi_walk_resources(handle, METHOD_NAME__PRS,
 				     acpi_pci_link_check_possible, link);
 	if (ACPI_FAILURE(status)) {
-		acpi_handle_debug(link->device->handle, "_PRS not present or invalid");
+		acpi_handle_debug(handle, "_PRS not present or invalid");
 		return 0;
 	}
 
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-			  "Found %d possible IRQs\n",
-			  link->irq.possible_count));
+	acpi_handle_debug(handle, "Found %d possible IRQs\n",
+			  link->irq.possible_count);
 
 	return 0;
 }
@@ -186,8 +185,7 @@ static acpi_status acpi_pci_link_check_c
 				 * IRQ descriptors may have no IRQ# bits set,
 				 * particularly those those w/ _STA disabled
 				 */
-				ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-						  "Blank _CRS IRQ resource\n"));
+				pr_debug("Blank _CRS IRQ resource\n");
 				return AE_OK;
 			}
 			*irq = p->interrupts[0];
@@ -202,8 +200,7 @@ static acpi_status acpi_pci_link_check_c
 				 * extended IRQ descriptors must
 				 * return at least 1 IRQ
 				 */
-				printk(KERN_WARNING PREFIX
-					      "Blank _CRS EXT IRQ resource\n");
+				pr_debug("Blank _CRS EXT IRQ resource\n");
 				return AE_OK;
 			}
 			*irq = p->interrupts[0];
@@ -211,8 +208,8 @@ static acpi_status acpi_pci_link_check_c
 		}
 		break;
 	default:
-		printk(KERN_ERR PREFIX "_CRS resource type 0x%x isn't an IRQ\n",
-		       resource->type);
+		pr_debug("_CRS resource type 0x%x is not IRQ\n",
+			 resource->type);
 		return AE_OK;
 	}
 
@@ -228,8 +225,9 @@ static acpi_status acpi_pci_link_check_c
  */
 static int acpi_pci_link_get_current(struct acpi_pci_link *link)
 {
-	int result = 0;
+	acpi_handle handle = link->device->handle;
 	acpi_status status;
+	int result = 0;
 	int irq = 0;
 
 	link->irq.active = 0;
@@ -239,12 +237,12 @@ static int acpi_pci_link_get_current(str
 		/* Query _STA, set link->device->status */
 		result = acpi_bus_get_status(link->device);
 		if (result) {
-			printk(KERN_ERR PREFIX "Unable to read status\n");
+			acpi_handle_err(handle, "Unable to read status\n");
 			goto end;
 		}
 
 		if (!link->device->status.enabled) {
-			ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Link disabled\n"));
+			acpi_handle_debug(handle, "Link disabled\n");
 			return 0;
 		}
 	}
@@ -253,22 +251,23 @@ static int acpi_pci_link_get_current(str
 	 * Query and parse _CRS to get the current IRQ assignment.
 	 */
 
-	status = acpi_walk_resources(link->device->handle, METHOD_NAME__CRS,
+	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
 				     acpi_pci_link_check_current, &irq);
 	if (ACPI_FAILURE(status)) {
-		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _CRS"));
+		acpi_handle_warn(handle, "_CRS evaluation failed: %s\n",
+				 acpi_format_exception(status));
 		result = -ENODEV;
 		goto end;
 	}
 
 	if (acpi_strict && !irq) {
-		printk(KERN_ERR PREFIX "_CRS returned 0\n");
+		acpi_handle_err(handle, "_CRS returned 0\n");
 		result = -ENODEV;
 	}
 
 	link->irq.active = irq;
 
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Link at IRQ %d \n", link->irq.active));
+	acpi_handle_debug(handle, "Link at IRQ %d \n", link->irq.active);
 
       end:
 	return result;
@@ -276,13 +275,14 @@ static int acpi_pci_link_get_current(str
 
 static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
 {
-	int result;
-	acpi_status status;
 	struct {
 		struct acpi_resource res;
 		struct acpi_resource end;
 	} *resource;
 	struct acpi_buffer buffer = { 0, NULL };
+	acpi_handle handle = link->device->handle;
+	acpi_status status;
+	int result;
 
 	if (!irq)
 		return -EINVAL;
@@ -329,7 +329,8 @@ static int acpi_pci_link_set(struct acpi
 		/* ignore resource_source, it's optional */
 		break;
 	default:
-		printk(KERN_ERR PREFIX "Invalid Resource_type %d\n", link->irq.resource_type);
+		acpi_handle_err(handle, "Invalid resource type %d\n",
+				link->irq.resource_type);
 		result = -EINVAL;
 		goto end;
 
@@ -342,7 +343,8 @@ static int acpi_pci_link_set(struct acpi
 
 	/* check for total failure */
 	if (ACPI_FAILURE(status)) {
-		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _SRS"));
+		acpi_handle_warn(handle, "_SRS evaluation failed: %s",
+				 acpi_format_exception(status));
 		result = -ENODEV;
 		goto end;
 	}
@@ -350,15 +352,11 @@ static int acpi_pci_link_set(struct acpi
 	/* Query _STA, set device->status */
 	result = acpi_bus_get_status(link->device);
 	if (result) {
-		printk(KERN_ERR PREFIX "Unable to read status\n");
+		acpi_handle_err(handle, "Unable to read status\n");
 		goto end;
 	}
-	if (!link->device->status.enabled) {
-		printk(KERN_WARNING PREFIX
-			      "%s [%s] disabled and referenced, BIOS bug\n",
-			      acpi_device_name(link->device),
-			      acpi_device_bid(link->device));
-	}
+	if (!link->device->status.enabled)
+		acpi_handle_warn(handle, "Disabled and referenced, BIOS bug\n");
 
 	/* Query _CRS, set link->irq.active */
 	result = acpi_pci_link_get_current(link);
@@ -375,14 +373,12 @@ static int acpi_pci_link_set(struct acpi
 		 * policy: when _CRS doesn't return what we just _SRS
 		 * assume _SRS worked and override _CRS value.
 		 */
-		printk(KERN_WARNING PREFIX
-			      "%s [%s] BIOS reported IRQ %d, using IRQ %d\n",
-			      acpi_device_name(link->device),
-			      acpi_device_bid(link->device), link->irq.active, irq);
+		acpi_handle_warn(handle, "BIOS reported IRQ %d, using IRQ %d\n",
+				 link->irq.active, irq);
 		link->irq.active = irq;
 	}
 
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Set IRQ %d\n", link->irq.active));
+	acpi_handle_debug(handle, "Set IRQ %d\n", link->irq.active);
 
       end:
 	kfree(resource);
@@ -656,9 +652,7 @@ int acpi_pci_link_allocate_irq(acpi_hand
 		*polarity = link->irq.polarity;
 	if (name)
 		*name = acpi_device_bid(link->device);
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-			  "Link %s is referenced\n",
-			  acpi_device_bid(link->device)));
+	acpi_handle_debug(handle, "Link is referenced\n");
 	return link->irq.active;
 }
 
@@ -702,9 +696,7 @@ int acpi_pci_link_free_irq(acpi_handle h
 	 */
 	link->refcnt--;
 #endif
-	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-			  "Link %s is dereferenced\n",
-			  acpi_device_bid(link->device)));
+	acpi_handle_debug(handle, "Link is dereferenced\n");
 
 	if (link->refcnt == 0)
 		acpi_evaluate_object(link->device->handle, "_DIS", NULL, NULL);




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v1 3/4] ACPI: PCI: Drop ACPI_PCI_COMPONENT that is not used any more
  2021-02-19 18:14 [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Rafael J. Wysocki
  2021-02-19 18:15 ` [PATCH v1 1/4] ACPI: PCI: IRQ: Consolidate printing diagnostic messages Rafael J. Wysocki
  2021-02-19 18:16 ` [PATCH v1 2/4] ACPI: PCI: Replace ACPI_DEBUG_PRINT() and ACPI_EXCEPTION() Rafael J. Wysocki
@ 2021-02-19 18:16 ` Rafael J. Wysocki
  2021-02-19 18:17 ` [PATCH v1 4/4] ACPI: PCI: Replace direct printk() invocations in pci_link.c Rafael J. Wysocki
  2021-02-20  9:24 ` [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Hanjun Guo
  4 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-02-19 18:16 UTC (permalink / raw)
  To: Linux ACPI; +Cc: Linux PCI, LKML, Bjorn Helgaas, Hanjun Guo

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After dropping all of the code using ACPI_PCI_COMPONENT drop the
definition of it too and update the documentation to remove all
ACPI_PCI_COMPONENT references from it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/admin-guide/kernel-parameters.txt |    4 +---
 Documentation/firmware-guide/acpi/debug.rst     |    5 -----
 drivers/acpi/sysfs.c                            |    1 -
 include/acpi/acpi_drivers.h                     |    1 -
 4 files changed, 1 insertion(+), 10 deletions(-)

Index: linux-pm/Documentation/admin-guide/kernel-parameters.txt
===================================================================
--- linux-pm.orig/Documentation/admin-guide/kernel-parameters.txt
+++ linux-pm/Documentation/admin-guide/kernel-parameters.txt
@@ -50,7 +50,7 @@
 			CONFIG_ACPI_DEBUG must be enabled to produce any ACPI
 			debug output.  Bits in debug_layer correspond to a
 			_COMPONENT in an ACPI source file, e.g.,
-			    #define _COMPONENT ACPI_PCI_COMPONENT
+			    #define _COMPONENT ACPI_EVENTS
 			Bits in debug_level correspond to a level in
 			ACPI_DEBUG_PRINT statements, e.g.,
 			    ACPI_DEBUG_PRINT((ACPI_DB_INFO, ...
@@ -60,8 +60,6 @@
 
 			Enable processor driver info messages:
 			    acpi.debug_layer=0x20000000
-			Enable PCI/PCI interrupt routing info messages:
-			    acpi.debug_layer=0x400000
 			Enable AML "Debug" output, i.e., stores to the Debug
 			object while interpreting AML:
 			    acpi.debug_layer=0xffffffff acpi.debug_level=0x2
Index: linux-pm/Documentation/firmware-guide/acpi/debug.rst
===================================================================
--- linux-pm.orig/Documentation/firmware-guide/acpi/debug.rst
+++ linux-pm/Documentation/firmware-guide/acpi/debug.rst
@@ -54,7 +54,6 @@ shows the supported mask values, current
     ACPI_TOOLS                      0x00002000
     ACPI_SBS_COMPONENT              0x00100000
     ACPI_FAN_COMPONENT              0x00200000
-    ACPI_PCI_COMPONENT              0x00400000
     ACPI_CONTAINER_COMPONENT        0x01000000
     ACPI_SYSTEM_COMPONENT           0x02000000
     ACPI_MEMORY_DEVICE_COMPONENT    0x08000000
@@ -127,10 +126,6 @@ AML) during boot::
 
     acpi.debug_layer=0xffffffff acpi.debug_level=0x2
 
-Enable PCI and PCI interrupt routing debug messages::
-
-    acpi.debug_layer=0x400000 acpi.debug_level=0x4
-
 Enable all ACPI hardware-related messages::
 
     acpi.debug_layer=0x2 acpi.debug_level=0xffffffff
Index: linux-pm/drivers/acpi/sysfs.c
===================================================================
--- linux-pm.orig/drivers/acpi/sysfs.c
+++ linux-pm/drivers/acpi/sysfs.c
@@ -54,7 +54,6 @@ static const struct acpi_dlayer acpi_deb
 
 	ACPI_DEBUG_INIT(ACPI_SBS_COMPONENT),
 	ACPI_DEBUG_INIT(ACPI_FAN_COMPONENT),
-	ACPI_DEBUG_INIT(ACPI_PCI_COMPONENT),
 	ACPI_DEBUG_INIT(ACPI_CONTAINER_COMPONENT),
 	ACPI_DEBUG_INIT(ACPI_SYSTEM_COMPONENT),
 	ACPI_DEBUG_INIT(ACPI_MEMORY_DEVICE_COMPONENT),
Index: linux-pm/include/acpi/acpi_drivers.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_drivers.h
+++ linux-pm/include/acpi/acpi_drivers.h
@@ -17,7 +17,6 @@
  */
 #define ACPI_SBS_COMPONENT		0x00100000
 #define ACPI_FAN_COMPONENT		0x00200000
-#define ACPI_PCI_COMPONENT		0x00400000
 #define ACPI_CONTAINER_COMPONENT	0x01000000
 #define ACPI_SYSTEM_COMPONENT		0x02000000
 #define ACPI_MEMORY_DEVICE_COMPONENT	0x08000000




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v1 4/4] ACPI: PCI: Replace direct printk() invocations in pci_link.c
  2021-02-19 18:14 [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2021-02-19 18:16 ` [PATCH v1 3/4] ACPI: PCI: Drop ACPI_PCI_COMPONENT that is not used any more Rafael J. Wysocki
@ 2021-02-19 18:17 ` Rafael J. Wysocki
  2021-02-20  9:24 ` [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Hanjun Guo
  4 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-02-19 18:17 UTC (permalink / raw)
  To: Linux ACPI; +Cc: Linux PCI, LKML, Bjorn Helgaas, Hanjun Guo

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Replace the direct printk() invocations in pci_link.c with (mostly
corresponding) acpi_handle_*() calls relative to the ACPI handle of
the given link device, which allows the AML corresponding to those
messages to be identified more easily, or with pr_*() calls.

While at it, add a pr_fmt() definition ot pci_link.c, make
acpi_pci_link_check_possible() print all messages with
acpi_handle_debug() for consistency and replace the (not-so-
reliable) KERN_CONT-based message line composition in
acpi_pci_link_add() with two pr_info() and a series of
acpi_handle_debug() calls (the latter for the possible IRQs).

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/pci_link.c |   86 +++++++++++++++++++++---------------------------
 1 file changed, 38 insertions(+), 48 deletions(-)

Index: linux-pm/drivers/acpi/pci_link.c
===================================================================
--- linux-pm.orig/drivers/acpi/pci_link.c
+++ linux-pm/drivers/acpi/pci_link.c
@@ -12,6 +12,8 @@
  *	   for IRQ management (e.g. start()->_SRS).
  */
 
+#define pr_fmt(fmt) "ACPI: PCI: " fmt
+
 #include <linux/syscore_ops.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -102,9 +104,9 @@ static acpi_status acpi_pci_link_check_p
 			     (i < p->interrupt_count
 			      && i < ACPI_PCI_LINK_MAX_POSSIBLE); i++) {
 				if (!p->interrupts[i]) {
-					printk(KERN_WARNING PREFIX
-					       "Invalid _PRS IRQ %d\n",
-					       p->interrupts[i]);
+					acpi_handle_debug(handle,
+							  "Invalid _PRS IRQ %d\n",
+							  p->interrupts[i]);
 					continue;
 				}
 				link->irq.possible[i] = p->interrupts[i];
@@ -120,17 +122,17 @@ static acpi_status acpi_pci_link_check_p
 			struct acpi_resource_extended_irq *p =
 			    &resource->data.extended_irq;
 			if (!p || !p->interrupt_count) {
-				printk(KERN_WARNING PREFIX
-					      "Blank _PRS EXT IRQ resource\n");
+				acpi_handle_debug(handle,
+						  "Blank _PRS EXT IRQ resource\n");
 				return AE_OK;
 			}
 			for (i = 0;
 			     (i < p->interrupt_count
 			      && i < ACPI_PCI_LINK_MAX_POSSIBLE); i++) {
 				if (!p->interrupts[i]) {
-					printk(KERN_WARNING PREFIX
-					       "Invalid _PRS IRQ %d\n",
-					       p->interrupts[i]);
+					acpi_handle_debug(handle,
+							  "Invalid _PRS IRQ %d\n",
+							  p->interrupts[i]);
 					continue;
 				}
 				link->irq.possible[i] = p->interrupts[i];
@@ -142,8 +144,8 @@ static acpi_status acpi_pci_link_check_p
 			break;
 		}
 	default:
-		printk(KERN_ERR PREFIX "_PRS resource type 0x%x isn't an IRQ\n",
-		       resource->type);
+		acpi_handle_debug(handle, "_PRS resource type 0x%x is not IRQ\n",
+				  resource->type);
 		return AE_OK;
 	}
 
@@ -527,6 +529,7 @@ static int acpi_irq_balance = -1;	/* 0:
 
 static int acpi_pci_link_allocate(struct acpi_pci_link *link)
 {
+	acpi_handle handle = link->device->handle;
 	int irq;
 	int i;
 
@@ -549,8 +552,8 @@ static int acpi_pci_link_allocate(struct
 	 */
 	if (i == link->irq.possible_count) {
 		if (acpi_strict)
-			printk(KERN_WARNING PREFIX "_CRS %d not found"
-				      " in _PRS\n", link->irq.active);
+			acpi_handle_warn(handle, "_CRS %d not found in _PRS\n",
+					 link->irq.active);
 		link->irq.active = 0;
 	}
 
@@ -574,28 +577,23 @@ static int acpi_pci_link_allocate(struct
 		}
 	}
 	if (acpi_irq_get_penalty(irq) >= PIRQ_PENALTY_ISA_ALWAYS) {
-		printk(KERN_ERR PREFIX "No IRQ available for %s [%s]. "
-			    "Try pci=noacpi or acpi=off\n",
-			    acpi_device_name(link->device),
-			    acpi_device_bid(link->device));
+		acpi_handle_err(handle,
+				"No IRQ available. Try pci=noacpi or acpi=off\n");
 		return -ENODEV;
 	}
 
 	/* Attempt to enable the link device at this IRQ. */
 	if (acpi_pci_link_set(link, irq)) {
-		printk(KERN_ERR PREFIX "Unable to set IRQ for %s [%s]. "
-			    "Try pci=noacpi or acpi=off\n",
-			    acpi_device_name(link->device),
-			    acpi_device_bid(link->device));
+		acpi_handle_err(handle,
+				"Unable to set IRQ. Try pci=noacpi or acpi=off\n");
 		return -ENODEV;
 	} else {
 		if (link->irq.active < ACPI_MAX_ISA_IRQS)
 			acpi_isa_irq_penalty[link->irq.active] +=
 				PIRQ_PENALTY_PCI_USING;
 
-		pr_info("%s [%s] enabled at IRQ %d\n",
-		       acpi_device_name(link->device),
-		       acpi_device_bid(link->device), link->irq.active);
+		acpi_handle_info(handle, "Enabled at IRQ %d\n",
+				 link->irq.active);
 	}
 
 	link->irq.initialized = 1;
@@ -616,19 +614,19 @@ int acpi_pci_link_allocate_irq(acpi_hand
 
 	result = acpi_bus_get_device(handle, &device);
 	if (result) {
-		printk(KERN_ERR PREFIX "Invalid link device\n");
+		acpi_handle_err(handle, "Invalid link device\n");
 		return -1;
 	}
 
 	link = acpi_driver_data(device);
 	if (!link) {
-		printk(KERN_ERR PREFIX "Invalid link context\n");
+		acpi_handle_err(handle, "Invalid link context\n");
 		return -1;
 	}
 
 	/* TBD: Support multiple index (IRQ) entries per Link Device */
 	if (index) {
-		printk(KERN_ERR PREFIX "Invalid index %d\n", index);
+		acpi_handle_err(handle, "Invalid index %d\n", index);
 		return -1;
 	}
 
@@ -640,7 +638,7 @@ int acpi_pci_link_allocate_irq(acpi_hand
 
 	if (!link->irq.active) {
 		mutex_unlock(&acpi_link_lock);
-		printk(KERN_ERR PREFIX "Link active IRQ is 0!\n");
+		acpi_handle_err(handle, "Link active IRQ is 0!\n");
 		return -1;
 	}
 	link->refcnt++;
@@ -668,20 +666,20 @@ int acpi_pci_link_free_irq(acpi_handle h
 
 	result = acpi_bus_get_device(handle, &device);
 	if (result) {
-		printk(KERN_ERR PREFIX "Invalid link device\n");
+		acpi_handle_err(handle, "Invalid link device\n");
 		return -1;
 	}
 
 	link = acpi_driver_data(device);
 	if (!link) {
-		printk(KERN_ERR PREFIX "Invalid link context\n");
+		acpi_handle_err(handle, "Invalid link context\n");
 		return -1;
 	}
 
 	mutex_lock(&acpi_link_lock);
 	if (!link->irq.initialized) {
 		mutex_unlock(&acpi_link_lock);
-		printk(KERN_ERR PREFIX "Link isn't initialized\n");
+		acpi_handle_err(handle, "Link isn't initialized\n");
 		return -1;
 	}
 #ifdef	FUTURE_USE
@@ -712,10 +710,10 @@ int acpi_pci_link_free_irq(acpi_handle h
 static int acpi_pci_link_add(struct acpi_device *device,
 			     const struct acpi_device_id *not_used)
 {
-	int result;
+	acpi_handle handle = device->handle;
 	struct acpi_pci_link *link;
+	int result;
 	int i;
-	int found = 0;
 
 	link = kzalloc(sizeof(struct acpi_pci_link), GFP_KERNEL);
 	if (!link)
@@ -734,31 +732,23 @@ static int acpi_pci_link_add(struct acpi
 	/* query and set link->irq.active */
 	acpi_pci_link_get_current(link);
 
-	printk(KERN_INFO PREFIX "%s [%s] (IRQs", acpi_device_name(device),
-	       acpi_device_bid(device));
+	pr_info("Interrupt link %s configured for IRQ %d\n",
+		acpi_device_bid(device), link->irq.active);
+
 	for (i = 0; i < link->irq.possible_count; i++) {
-		if (link->irq.active == link->irq.possible[i]) {
-			printk(KERN_CONT " *%d", link->irq.possible[i]);
-			found = 1;
-		} else
-			printk(KERN_CONT " %d", link->irq.possible[i]);
+		if (link->irq.active != link->irq.possible[i])
+			acpi_handle_debug(handle, "Possible IRQ %d\n",
+					  link->irq.possible[i]);
 	}
 
-	printk(KERN_CONT ")");
-
-	if (!found)
-		printk(KERN_CONT " *%d", link->irq.active);
-
 	if (!link->device->status.enabled)
-		printk(KERN_CONT ", disabled.");
-
-	printk(KERN_CONT "\n");
+		pr_info("Interrupt link %s disabled\n", acpi_device_bid(device));
 
 	list_add_tail(&link->list, &acpi_link_list);
 
       end:
 	/* disable all links -- to be activated on use */
-	acpi_evaluate_object(device->handle, "_DIS", NULL, NULL);
+	acpi_evaluate_object(handle, "_DIS", NULL, NULL);
 	mutex_unlock(&acpi_link_lock);
 
 	if (result)




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 0/4] ACPI: PCI: Unify printing of messages
  2021-02-19 18:14 [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2021-02-19 18:17 ` [PATCH v1 4/4] ACPI: PCI: Replace direct printk() invocations in pci_link.c Rafael J. Wysocki
@ 2021-02-20  9:24 ` Hanjun Guo
  2021-03-03 18:31   ` Rafael J. Wysocki
  4 siblings, 1 reply; 7+ messages in thread
From: Hanjun Guo @ 2021-02-20  9:24 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux ACPI; +Cc: Linux PCI, LKML, Bjorn Helgaas

On 2021/2/20 2:14, Rafael J. Wysocki wrote:
> Hi All,
> 
> This series gets rid of ACPICA debugging from non-ACPICA code related to PCI
> (patches [1-3/4]) and replaces direct printk() usage in pci_link.c with
> pr_*() or acpi_handle_*().
> 
> Please refer to the patch changelogs for details.

Looks good to me,

Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks
Hanjun

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 0/4] ACPI: PCI: Unify printing of messages
  2021-02-20  9:24 ` [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Hanjun Guo
@ 2021-03-03 18:31   ` Rafael J. Wysocki
  0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-03-03 18:31 UTC (permalink / raw)
  To: Hanjun Guo, Linux ACPI; +Cc: Rafael J. Wysocki, Linux PCI, LKML, Bjorn Helgaas

On Sat, Feb 20, 2021 at 10:25 AM Hanjun Guo <guohanjun@huawei.com> wrote:
>
> On 2021/2/20 2:14, Rafael J. Wysocki wrote:
> > Hi All,
> >
> > This series gets rid of ACPICA debugging from non-ACPICA code related to PCI
> > (patches [1-3/4]) and replaces direct printk() usage in pci_link.c with
> > pr_*() or acpi_handle_*().
> >
> > Please refer to the patch changelogs for details.
>
> Looks good to me,
>
> Reviewed-by: Hanjun Guo <guohanjun@huawei.com>

Thanks!

Given the absence of any further comments I will be queuing up this
series for 5.13, thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-03 20:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-19 18:14 [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Rafael J. Wysocki
2021-02-19 18:15 ` [PATCH v1 1/4] ACPI: PCI: IRQ: Consolidate printing diagnostic messages Rafael J. Wysocki
2021-02-19 18:16 ` [PATCH v1 2/4] ACPI: PCI: Replace ACPI_DEBUG_PRINT() and ACPI_EXCEPTION() Rafael J. Wysocki
2021-02-19 18:16 ` [PATCH v1 3/4] ACPI: PCI: Drop ACPI_PCI_COMPONENT that is not used any more Rafael J. Wysocki
2021-02-19 18:17 ` [PATCH v1 4/4] ACPI: PCI: Replace direct printk() invocations in pci_link.c Rafael J. Wysocki
2021-02-20  9:24 ` [PATCH v1 0/4] ACPI: PCI: Unify printing of messages Hanjun Guo
2021-03-03 18:31   ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).