Skip to contentSkip to conversation

dev_printk() conversions

PATCH In review Bjorn Helgaas 4 participants 12 messages Jun 13, 2008 – Jul 7, 2008

Subsystems (3)
x86 ACPI PCI
lore
Share & tools
lore

Bjorn Helgaas

author
#0 lore raw

cover dev_printk() conversions

These convert some printks to dev_printks. They are completely independent of each other; I'm just posting them together because it probably makes sense to review them together.

It'd be nice if a PCI person looked at pci_request_region(). To me, it looks like it prints the wrong BAR number (it prints "bar + 1").

Bjorn

author
#1 lore raw

PATCH 1/5 ACPI: use dev_printk when possible

Convert printks to use dev_printk(). The most obvious change will be messages like this:

-ACPI: PCI Interrupt 0000:00:04.0[A] -> GSI 31 (level, low) -> IRQ 31 +cciss 0000:00:04.0: PCI INTA# -> GSI 31 (level, low) -> IRQ 31

Signed-off-by Bjorn Helgaas <[email protected]>
drivers/acpi/pci_irq.c +17 -21
Index: work11/drivers/acpi/pci_irq.c
===================================================================
@@ -162,7 +162,7 @@ do_prt_fixups(struct acpi_prt_entry *ent
 		    !strcmp(prt->source, quirk->source) &&
 		    strlen(prt->source) >= strlen(quirk->actual_source)) {
 			printk(KERN_WARNING PREFIX "firmware reports "
-				"%04x:%02x:%02x[%c] connected to %s; "
+				"%04x:%02x:%02x PCI INT%c# connected to %s; "
 				"changing to %s\n",
 				entry->id.segment, entry->id.bus,
 				entry->id.device, 'A' + entry->pin,

@@ -429,7 +429,7 @@ acpi_pci_irq_derive(struct pci_dev *dev,
 {
 	struct pci_dev *bridge = dev;
 	int irq = -1;
-	u8 bridge_pin = 0;
+	u8 bridge_pin = 0, orig_pin = pin;
 
 
 	if (!dev)

@@ -463,8 +463,8 @@ acpi_pci_irq_derive(struct pci_dev *dev,
 	}
 
 	if (irq < 0) {
-		printk(KERN_WARNING PREFIX "Unable to derive IRQ for device %s\n",
-			      pci_name(dev));
+		dev_warn(&dev->dev, "can't derive routing for PCI INT%c#\n",
+			 'A' + orig_pin);
 		return -1;
 	}
 

@@ -487,6 +487,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 	int triggering = ACPI_LEVEL_SENSITIVE;
 	int polarity = ACPI_ACTIVE_LOW;
 	char *link = NULL;
+	char link_desc[16];
 	int rc;
 
 

@@ -503,7 +504,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 	pin--;
 
 	if (!dev->bus) {
-		printk(KERN_ERR PREFIX "Invalid (NULL) 'bus' field\n");
+		dev_err(&dev->dev, "invalid (NULL) 'bus' field\n");
 		return -ENODEV;
 	}
 

@@ -538,8 +539,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 	 * driver reported one, then use it. Exit in any case.
 	 */
 	if (irq < 0) {
-		printk(KERN_WARNING PREFIX "PCI Interrupt %s[%c]: no GSI",
-		       pci_name(dev), ('A' + pin));
+		dev_warn(&dev->dev, "PCI INT%c#: no GSI", 'A' + pin);
 		/* Interrupt Line values above 0xF are forbidden */
 		if (dev->irq > 0 && (dev->irq <= 0xF)) {
 			printk(" - using IRQ %d\n", dev->irq);

@@ -554,21 +554,21 @@ int acpi_pci_irq_enable(struct pci_dev *
 
 	rc = acpi_register_gsi(irq, triggering, polarity);
 	if (rc < 0) {
-		printk(KERN_WARNING PREFIX "PCI Interrupt %s[%c]: failed "
-		       "to register GSI\n", pci_name(dev), ('A' + pin));
+		dev_warn(&dev->dev, "PCI INT%c#: failed to register GSI\n",
+			 'A' + pin);
 		return rc;
 	}
 	dev->irq = rc;
 
-	printk(KERN_INFO PREFIX "PCI Interrupt %s[%c] -> ",
-	       pci_name(dev), 'A' + pin);
-
 	if (link)
-		printk("Link [%s] -> ", link);
+		snprintf(link_desc, sizeof(link_desc), " -> Link[%s]", link);
+	else
+		link_desc[0] = '\0';
 
-	printk("GSI %u (%s, %s) -> IRQ %d\n", irq,
-	       (triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
-	       (polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
+	dev_info(&dev->dev, "PCI INT%c#%s -> GSI %u (%s, %s) -> IRQ %d\n",
+		 'A' + pin, link_desc, irq,
+		 (triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
+		 (polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
 
 	return 0;
 }

@@ -616,10 +616,6 @@ void acpi_pci_irq_disable(struct pci_dev
 	 * (e.g. PCI_UNDEFINED_IRQ).
 	 */
 
-	printk(KERN_INFO PREFIX "PCI interrupt for device %s disabled\n",
-	       pci_name(dev));
-
+	dev_info(&dev->dev, "PCI INT%c# disabled\n", 'A' + pin);
 	acpi_unregister_gsi(gsi);
-
-	return;
 }
reviewer Acked-by on v1 1/5
#2 lore raw

Re: [patch 1/5] ACPI: use dev_printk when possible

Quoted 5 lines · Bjorn Helgaas

Convert printks to use dev_printk(). The most obvious change will be messages like this:

   -ACPI: PCI Interrupt 0000:00:04.0[A] -> GSI 31 (level, low) -> IRQ 31
   +cciss 0000:00:04.0: PCI INTA# -> GSI 31 (level, low) -> IRQ 31

I like the part identifying the driver.
I'm also okay with having fewer messages tagged with ACPI:

drivers/acpi/pci_irq.c do_prt_fixups(struct acpi_prt_entry *ent · line 165

Quoted 12 lines
Signed-off-by Bjorn Helgaas <[email protected]>

Index: work11/drivers/acpi/pci_irq.c
===================================================================

drivers/acpi/pci_irq.c +1 -1
@@ -162,7 +162,7 @@ do_prt_fixups(struct acpi_prt_entry *ent
 		    !strcmp(prt->source, quirk->source) &&
 		    strlen(prt->source) >= strlen(quirk->actual_source)) {
 			printk(KERN_WARNING PREFIX "firmware reports "
-				"%04x:%02x:%02x[%c] connected to %s; "
+				"%04x:%02x:%02x PCI INT%c# connected to %s; "

Can we delete the '#' from INTA# in this patch?
I don't think that using hardware polarity notation in kernel messages clarifies anything, particularly since it is hard-coded and thus possibly conflicting with the subsequent '(level, low)'...

otherwise,

Acked-by Len Brown <[email protected]>

thanks,
-Len

Quoted 96 lines
drivers/acpi/pci_irq.c +16 -20
 				"changing to %s\n",
 				entry->id.segment, entry->id.bus,
 				entry->id.device, 'A' + entry->pin,

@@ -429,7 +429,7 @@ acpi_pci_irq_derive(struct pci_dev *dev,
 {
 	struct pci_dev *bridge = dev;
 	int irq = -1;
-	u8 bridge_pin = 0;
+	u8 bridge_pin = 0, orig_pin = pin;
 
 
 	if (!dev)

@@ -463,8 +463,8 @@ acpi_pci_irq_derive(struct pci_dev *dev,
 	}
 
 	if (irq < 0) {
-		printk(KERN_WARNING PREFIX "Unable to derive IRQ for device %s\n",
-			      pci_name(dev));
+		dev_warn(&dev->dev, "can't derive routing for PCI INT%c#\n",
+			 'A' + orig_pin);
 		return -1;
 	}
 

@@ -487,6 +487,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 	int triggering = ACPI_LEVEL_SENSITIVE;
 	int polarity = ACPI_ACTIVE_LOW;
 	char *link = NULL;
+	char link_desc[16];
 	int rc;
 
 

@@ -503,7 +504,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 	pin--;
 
 	if (!dev->bus) {
-		printk(KERN_ERR PREFIX "Invalid (NULL) 'bus' field\n");
+		dev_err(&dev->dev, "invalid (NULL) 'bus' field\n");
 		return -ENODEV;
 	}
 

@@ -538,8 +539,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 	 * driver reported one, then use it. Exit in any case.
 	 */
 	if (irq < 0) {
-		printk(KERN_WARNING PREFIX "PCI Interrupt %s[%c]: no GSI",
-		       pci_name(dev), ('A' + pin));
+		dev_warn(&dev->dev, "PCI INT%c#: no GSI", 'A' + pin);
 		/* Interrupt Line values above 0xF are forbidden */
 		if (dev->irq > 0 && (dev->irq <= 0xF)) {
 			printk(" - using IRQ %d\n", dev->irq);

@@ -554,21 +554,21 @@ int acpi_pci_irq_enable(struct pci_dev *
 
 	rc = acpi_register_gsi(irq, triggering, polarity);
 	if (rc < 0) {
-		printk(KERN_WARNING PREFIX "PCI Interrupt %s[%c]: failed "
-		       "to register GSI\n", pci_name(dev), ('A' + pin));
+		dev_warn(&dev->dev, "PCI INT%c#: failed to register GSI\n",
+			 'A' + pin);
 		return rc;
 	}
 	dev->irq = rc;
 
-	printk(KERN_INFO PREFIX "PCI Interrupt %s[%c] -> ",
-	       pci_name(dev), 'A' + pin);
-
 	if (link)
-		printk("Link [%s] -> ", link);
+		snprintf(link_desc, sizeof(link_desc), " -> Link[%s]", link);
+	else
+		link_desc[0] = '\0';
 
-	printk("GSI %u (%s, %s) -> IRQ %d\n", irq,
-	       (triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
-	       (polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
+	dev_info(&dev->dev, "PCI INT%c#%s -> GSI %u (%s, %s) -> IRQ %d\n",
+		 'A' + pin, link_desc, irq,
+		 (triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
+		 (polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
 
 	return 0;
 }

@@ -616,10 +616,6 @@ void acpi_pci_irq_disable(struct pci_dev
 	 * (e.g. PCI_UNDEFINED_IRQ).
 	 */
 
-	printk(KERN_INFO PREFIX "PCI interrupt for device %s disabled\n",
-	       pci_name(dev));
-
+	dev_info(&dev->dev, "PCI INT%c# disabled\n", 'A' + pin);
 	acpi_unregister_gsi(gsi);
-
-	return;
 }
Signature · 1 line
--
author
#3 lore raw

PATCH 2/5 PCI: use dev_printk when possible

In reply to Bjorn Helgaas

Convert printks to use dev_printk().

I converted pr_debug() to dev_dbg(). Both use KERN_DEBUG and are enabled only when DEBUG is defined.

I converted printk(KERN_DEBUG) to dev_printk(KERN_DEBUG), not to dev_dbg(), because dev_dbg() is only enabled when DEBUG is defined.

I converted DBG(KERN_INFO) (only in setup-bus.c) to dev_info(). The DBG() name makes it sound like debug, but it's been enabled forever, so dev_info() preserves the previous behavior.

I tried to make the resource assignment formats more consistent, e.g.,
"BAR %d: got res [%#llx-%#llx] bus [%#llx-%#llx] flags %#lx\n"
instead of sometimes using "start-end" and sometimes using "size@start". I'm not attached to one or the other; I'd just like them consistent.

pci_request_region() prints "bar + 1". I don't know why the "+ 1" is there.

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/pci/msi.c       |   10 ++----
 drivers/pci/pci.c       |   40 ++++++++++++---------------
 drivers/pci/probe.c     |   28 +++++++++----------
 drivers/pci/setup-bus.c |   43 ++++++++++++-----------------
 drivers/pci/setup-irq.c |    3 --
 drivers/pci/setup-res.c |   69 ++++++++++++++++++++++--------------------------
 6 files changed, 87 insertions(+), 106 deletions(-)
drivers/pci/msi.c +4 -6
Index: work11/drivers/pci/msi.c
===================================================================
@@ -561,9 +561,8 @@ int pci_enable_msi(struct pci_dev* dev)
 
 	/* Check whether driver already requested for MSI-X irqs */
 	if (dev->msix_enabled) {
-		printk(KERN_INFO "PCI: %s: Can't enable MSI.  "
-			"Device already has MSI-X enabled\n",
-			pci_name(dev));
+		dev_info(&dev->dev, "can't enable MSI "
+			 "(MSI-X already enabled)\n");
 		return -EINVAL;
 	}
 	status = msi_capability_init(dev);

@@ -686,9 +685,8 @@ int pci_enable_msix(struct pci_dev* dev,
 
 	/* Check whether driver already requested for MSI irq */
    	if (dev->msi_enabled) {
-		printk(KERN_INFO "PCI: %s: Can't enable MSI-X.  "
-		       "Device already has an MSI irq assigned\n",
-		       pci_name(dev));
+		dev_info(&dev->dev, "can't enable MSI-X "
+		       "(MSI IRQ already assigned)\n");
 		return -EINVAL;
 	}
 	status = msix_capability_init(dev, entries, nvec);
drivers/pci/pci.c +18 -22
Index: work11/drivers/pci/pci.c
===================================================================
@@ -424,8 +424,8 @@ pci_set_power_state(struct pci_dev *dev,
 	 * to sleep if we're already in a low power state
 	 */
 	if (state != PCI_D0 && dev->current_state > state) {
-		printk(KERN_ERR "%s(): %s: state=%d, current state=%d\n",
-			__func__, pci_name(dev), state, dev->current_state);
+		dev_err(&dev->dev, "invalid power transition "
+			"(from state %d to %d)\n", dev->current_state, state);
 		return -EINVAL;
 	} else if (dev->current_state == state)
 		return 0;        /* we're already there */

@@ -433,9 +433,8 @@ pci_set_power_state(struct pci_dev *dev,
 
 	pci_read_config_word(dev,pm + PCI_PM_PMC,&pmc);
 	if ((pmc & PCI_PM_CAP_VER_MASK) > 3) {
-		printk(KERN_DEBUG
-		       "PCI: %s has unsupported PM cap regs version (%u)\n",
-		       pci_name(dev), pmc & PCI_PM_CAP_VER_MASK);
+		dev_printk(KERN_DEBUG, &dev->dev, "unsupported PM cap regs "
+			   "version (%u)\n", pmc & PCI_PM_CAP_VER_MASK);
 		return -EIO;
 	}
 

@@ -543,7 +542,8 @@ pci_power_t pci_choose_state(struct pci_
 	case PM_EVENT_HIBERNATE:
 		return PCI_D3hot;
 	default:
-		printk("Unrecognized suspend event %d\n", state.event);
+		dev_info(&dev->dev, "unrecognized suspend event %d\n",
+			 state.event);
 		BUG();
 	}
 	return PCI_D0;

@@ -568,7 +568,7 @@ static int pci_save_pcie_state(struct pc
 	else
 		found = 1;
 	if (!save_state) {
-		dev_err(&dev->dev, "Out of memory in pci_save_pcie_state\n");
+		dev_err(&dev->dev, "out of memory in pci_save_pcie_state\n");
 		return -ENOMEM;
 	}
 	cap = (u16 *)&save_state->data[0];

@@ -619,7 +619,7 @@ static int pci_save_pcix_state(struct pc
 	else
 		found = 1;
 	if (!save_state) {
-		dev_err(&dev->dev, "Out of memory in pci_save_pcie_state\n");
+		dev_err(&dev->dev, "out of memory in pci_save_pcie_state\n");
 		return -ENOMEM;
 	}
 	cap = (u16 *)&save_state->data[0];

@@ -685,10 +685,9 @@ pci_restore_state(struct pci_dev *dev)
 	for (i = 15; i >= 0; i--) {
 		pci_read_config_dword(dev, i * 4, &val);
 		if (val != dev->saved_config_space[i]) {
-			printk(KERN_DEBUG "PM: Writing back config space on "
-				"device %s at offset %x (was %x, writing %x)\n",
-				pci_name(dev), i,
-				val, (int)dev->saved_config_space[i]);
+			dev_printk(KERN_DEBUG, &dev->dev, "restoring config "
+				"space at offset %#x (was %#x, writing %#x)\n",
+				i, val, (int)dev->saved_config_space[i]);
 			pci_write_config_dword(dev,i * 4,
 				dev->saved_config_space[i]);
 		}

@@ -1116,13 +1115,11 @@ int pci_request_region(struct pci_dev *p
 	return 0;
 
 err_out:
-	printk (KERN_WARNING "PCI: Unable to reserve %s region #%d:%llx@%llx "
-		"for device %s\n",
-		pci_resource_flags(pdev, bar) & IORESOURCE_IO ? "I/O" : "mem",
+	dev_warn(&pdev->dev, "BAR %d: can't reserve %s region [%#llx-%#llx]\n",
 		bar + 1, /* PCI BAR # */
-		(unsigned long long)pci_resource_len(pdev, bar),
+		pci_resource_flags(pdev, bar) & IORESOURCE_IO ? "I/O" : "mem",
 		(unsigned long long)pci_resource_start(pdev, bar),
-		pci_name(pdev));
+		(unsigned long long)pci_resource_end(pdev, bar));
 	return -EBUSY;
 }
 

@@ -1214,7 +1211,7 @@ pci_set_master(struct pci_dev *dev)
 
 	pci_read_config_word(dev, PCI_COMMAND, &cmd);
 	if (! (cmd & PCI_COMMAND_MASTER)) {
-		pr_debug("PCI: Enabling bus mastering for device %s\n", pci_name(dev));
+		dev_dbg(&dev->dev, "enabling bus mastering\n");
 		cmd |= PCI_COMMAND_MASTER;
 		pci_write_config_word(dev, PCI_COMMAND, cmd);
 	}

@@ -1279,8 +1276,8 @@ pci_set_cacheline_size(struct pci_dev *d
 	if (cacheline_size == pci_cache_line_size)
 		return 0;
 
-	printk(KERN_DEBUG "PCI: cache line size of %d is not supported "
-	       "by device %s\n", pci_cache_line_size << 2, pci_name(dev));
+	dev_printk(KERN_DEBUG, &dev->dev, "cache line size of %d is not "
+		   "supported\n", pci_cache_line_size << 2);
 
 	return -EINVAL;
 }

@@ -1305,8 +1302,7 @@ pci_set_mwi(struct pci_dev *dev)
 
 	pci_read_config_word(dev, PCI_COMMAND, &cmd);
 	if (! (cmd & PCI_COMMAND_INVALIDATE)) {
-		pr_debug("PCI: Enabling Mem-Wr-Inval for device %s\n",
-			pci_name(dev));
+		dev_dbg(&dev->dev, "enabling Mem-Wr-Inval\n");
 		cmd |= PCI_COMMAND_INVALIDATE;
 		pci_write_config_word(dev, PCI_COMMAND, cmd);
 	}
drivers/pci/probe.c +15 -14
Index: work11/drivers/pci/probe.c
===================================================================
@@ -277,8 +277,8 @@ static void pci_read_bases(struct pci_de
 			res->end = res->start + sz64;
 #else
 			if (sz64 > 0x100000000ULL) {
-				printk(KERN_ERR "PCI: Unable to handle 64-bit "
-					"BAR for device %s\n", pci_name(dev));
+				dev_err(&dev->dev, "BAR %d: can't handle 64-bit"
+					" BAR\n", pos);
 				res->start = 0;
 				res->flags = 0;
 			} else if (lhi) {

@@ -329,7 +329,7 @@ void __devinit pci_read_bridge_bases(str
 		return;
 
 	if (dev->transparent) {
-		printk(KERN_INFO "PCI: Transparent bridge - %s\n", pci_name(dev));
+		dev_info(&dev->dev, "transparent bridge\n");
 		for(i = 3; i < PCI_BUS_NUM_RESOURCES; i++)
 			child->resource[i] = child->parent->resource[i - 3];
 	}

@@ -392,7 +392,8 @@ void __devinit pci_read_bridge_bases(str
 			limit |= ((long) mem_limit_hi) << 32;
 #else
 			if (mem_base_hi || mem_limit_hi) {
-				printk(KERN_ERR "PCI: Unable to handle 64-bit address space for bridge %s\n", pci_name(dev));
+				dev_err(&dev->dev, "can't handle 64-bit "
+					"address space for bridge\n");
 				return;
 			}
 #endif

@@ -511,8 +512,8 @@ int __devinit pci_scan_bridge(struct pci
 
 	pci_read_config_dword(dev, PCI_PRIMARY_BUS, &buses);
 
-	pr_debug("PCI: Scanning behind PCI bridge %s, config %06x, pass %d\n",
-		 pci_name(dev), buses & 0xffffff, pass);
+	dev_dbg(&dev->dev, "scanning behind bridge, config %06x, pass %d\n",
+		buses & 0xffffff, pass);
 
 	/* Disable MasterAbortMode during probing to avoid reporting
 	   of bus errors (in some architectures) */ 

@@ -535,8 +536,8 @@ int __devinit pci_scan_bridge(struct pci
 		 * ignore it.  This can happen with the i450NX chipset.
 		 */
 		if (pci_find_bus(pci_domain_nr(bus), busnr)) {
-			printk(KERN_INFO "PCI: Bus %04x:%02x already known\n",
-					pci_domain_nr(bus), busnr);
+			dev_info(&dev->dev, "bus %04x:%02x already known\n",
+				 pci_domain_nr(bus), busnr);
 			goto out;
 		}
 

@@ -720,7 +721,7 @@ static int pci_setup_device(struct pci_d
 	dev->class = class;
 	class >>= 8;
 
-	pr_debug("PCI: Found %s [%04x/%04x] %06x %02x\n", pci_name(dev),
+	dev_dbg(&dev->dev, "found [%04x/%04x] class %06x header type %02x\n",
 		 dev->vendor, dev->device, class, dev->hdr_type);
 
 	/* "Unknown power state" */

@@ -788,13 +789,13 @@ static int pci_setup_device(struct pci_d
 		break;
 
 	default:				    /* unknown header */
-		printk(KERN_ERR "PCI: device %s has unknown header type %02x, ignoring.\n",
-			pci_name(dev), dev->hdr_type);
+		dev_err(&dev->dev, "unknown header type %02x, "
+			"ignoring device\n", dev->hdr_type);
 		return -1;
 
 	bad:
-		printk(KERN_ERR "PCI: %s: class %x doesn't match header type %02x. Ignoring class.\n",
-		       pci_name(dev), class, dev->hdr_type);
+		dev_err(&dev->dev, "ignoring class %02x (doesn't match header "
+			"type %02x)\n", class, dev->hdr_type);
 		dev->class = PCI_CLASS_NOT_DEFINED;
 	}
 

@@ -927,7 +928,7 @@ static struct pci_dev *pci_scan_device(s
 			return NULL;
 		/* Card hasn't responded in 60 seconds?  Must be stuck. */
 		if (delay > 60 * 1000) {
-			printk(KERN_WARNING "Device %04x:%02x:%02x.%d not "
+			printk(KERN_WARNING "pci %04x:%02x:%02x.%d: not "
 					"responding\n", pci_domain_nr(bus),
 					bus->number, PCI_SLOT(devfn),
 					PCI_FUNC(devfn));
drivers/pci/setup-bus.c +18 -25

131 lines not on this page · raw message

drivers/pci/setup-irq.c +1 -2
Index: work11/drivers/pci/setup-irq.c
===================================================================
@@ -47,8 +47,7 @@ pdev_fixup_irq(struct pci_dev *dev,
 	}
 	dev->irq = irq;
 
-	pr_debug("PCI: fixup irq: (%s) got %d\n",
-		kobject_name(&dev->dev.kobj), dev->irq);
+	dev_dbg(&dev->dev, "fixup irq: got %d\n", dev->irq);
 
 	/* Always tell the device, so the driver knows what is
 	   the real IRQ to use; the device does not use it. */
drivers/pci/setup-res.c +33 -37

146 lines not on this page · raw message

author
#4 lore raw

PATCH 3/5 PCIE: aer: use dev_printk when possible

In reply to Bjorn Helgaas

Convert printks to use dev_printk().

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/pci/pcie/aer/aerdrv.c      |    8 +++-----
 drivers/pci/pcie/aer/aerdrv_acpi.c |    8 ++++----
 drivers/pci/pcie/aer/aerdrv_core.c |   24 ++++++++++++------------
 3 files changed, 19 insertions(+), 21 deletions(-)
drivers/pci/pcie/aer/aerdrv.c +3 -5
Index: work11/drivers/pci/pcie/aer/aerdrv.c
===================================================================
@@ -219,8 +219,7 @@ static int __devinit aer_probe (struct p
 
 	/* Alloc rpc data structure */
 	if (!(rpc = aer_alloc_rpc(dev))) {
-		printk(KERN_DEBUG "%s: Alloc rpc fails on PCIE device[%s]\n",
-			__func__, device->bus_id);
+		dev_printk(KERN_DEBUG, device, "alloc rpc failed\n");
 		aer_remove(dev);
 		return -ENOMEM;
 	}

@@ -228,8 +227,7 @@ static int __devinit aer_probe (struct p
 	/* Request IRQ ISR */
 	if ((status = request_irq(dev->irq, aer_irq, IRQF_SHARED, "aerdrv",
 				dev))) {
-		printk(KERN_DEBUG "%s: Request ISR fails on PCIE device[%s]\n",
-			__func__, device->bus_id);
+		dev_printk(KERN_DEBUG, device, "request IRQ failed\n");
 		aer_remove(dev);
 		return status;
 	}

@@ -273,7 +271,7 @@ static pci_ers_result_t aer_root_reset(s
 	 * to issue Configuration Requests to those devices.
 	 */
 	msleep(200);
-	printk(KERN_DEBUG "Complete link reset at Root[%s]\n", dev->dev.bus_id);
+	dev_printk(KERN_DEBUG, &dev->dev, "Root Port link has been reset\n");
 
 	/* Enable Root Port's interrupt in response to error messages */
 	pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, &status);
drivers/pci/pcie/aer/aerdrv_acpi.c +4 -4
Index: work11/drivers/pci/pcie/aer/aerdrv_acpi.c
===================================================================
@@ -50,10 +50,10 @@ int aer_osc_setup(struct pcie_device *pc
 	}
 
 	if (ACPI_FAILURE(status)) {
-		printk(KERN_DEBUG "AER service couldn't init device %s - %s\n",
-		    pciedev->device.bus_id,
-		    (status == AE_SUPPORT || status == AE_NOT_FOUND) ?
-		    "no _OSC support" : "Run ACPI _OSC fails");
+		dev_printk(KERN_DEBUG, &pciedev->device, "AER service couldn't "
+			   "init device: %s\n",
+			   (status == AE_SUPPORT || status == AE_NOT_FOUND) ?
+			   "no _OSC support" : "_OSC failed");
 		return -1;
 	}
drivers/pci/pcie/aer/aerdrv_core.c +12 -12
Index: work11/drivers/pci/pcie/aer/aerdrv_core.c
===================================================================
@@ -221,9 +221,9 @@ static void report_error_detected(struct
 			 * of a driver for this device is unaware of
 			 * its hw state.
 			 */
-			printk(KERN_DEBUG "Device ID[%s] has %s\n",
-					dev->dev.bus_id, (dev->driver) ?
-					"no AER-aware driver" : "no driver");
+			dev_printk(KERN_DEBUG, &dev->dev, "device has %s\n",
+				   dev->driver ?
+				   "no AER-aware driver" : "no driver");
 		}
 		return;
 	}

@@ -304,7 +304,7 @@ static pci_ers_result_t broadcast_error_
 {
 	struct aer_broadcast_data result_data;
 
-	printk(KERN_DEBUG "Broadcast %s message\n", error_mesg);
+	dev_printk(KERN_DEBUG, &dev->dev, "broadcast %s message\n", error_mesg);
 	result_data.state = state;
 	if (cb == report_error_detected)
 		result_data.result = PCI_ERS_RESULT_CAN_RECOVER;

@@ -404,18 +404,16 @@ static pci_ers_result_t reset_link(struc
 			data.aer_driver =
 				to_service_driver(aerdev->device.driver);
 		} else {
-			printk(KERN_DEBUG "No link-reset support to Device ID"
-				"[%s]\n",
-				dev->dev.bus_id);
+			dev_printk(KERN_DEBUG, &dev->dev, "no link-reset "
+				   "support\n");
 			return PCI_ERS_RESULT_DISCONNECT;
 		}
 	}
 
 	status = data.aer_driver->reset_link(udev);
 	if (status != PCI_ERS_RESULT_RECOVERED) {
-		printk(KERN_DEBUG "Link reset at upstream Device ID"
-			"[%s] failed\n",
-			udev->dev.bus_id);
+		dev_printk(KERN_DEBUG, &dev->dev, "link reset at upstream "
+			   "device %s failed\n", pci_name(udev));
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 

@@ -511,10 +509,12 @@ static void handle_error_source(struct p
 	} else {
 		status = do_recovery(aerdev, dev, info.severity);
 		if (status == PCI_ERS_RESULT_RECOVERED) {
-			printk(KERN_DEBUG "AER driver successfully recovered\n");
+			dev_printk(KERN_DEBUG, &dev->dev, "AER driver "
+				   "successfully recovered\n");
 		} else {
 			/* TODO: Should kernel panic here? */
-			printk(KERN_DEBUG "AER driver didn't recover\n");
+			dev_printk(KERN_DEBUG, &dev->dev, "AER driver didn't "
+				   "recover\n");
 		}
 	}
 }
author
#5 lore raw

PATCH 4/5 PCIE: port driver: use dev_printk when possible

In reply to Bjorn Helgaas

Convert printks to use dev_printk().

Signed-off-by Bjorn Helgaas <[email protected]>
---
 drivers/pci/pcie/portdrv_core.c |   22 +++++++++++-----------
 drivers/pci/pcie/portdrv_pci.c  |    5 ++---
 2 files changed, 13 insertions(+), 14 deletions(-)
drivers/pci/pcie/portdrv_core.c +11 -11
Index: work11/drivers/pci/pcie/portdrv_core.c
===================================================================
@@ -23,20 +23,20 @@ static int pcie_port_probe_service(struc
 {
 	struct pcie_device *pciedev;
 	struct pcie_port_service_driver *driver;
-	int status = -ENODEV;
+	int status;
 
 	if (!dev || !dev->driver)
-		return status;
+		return -ENODEV;
 
  	driver = to_service_driver(dev->driver);
 	if (!driver || !driver->probe)
-		return status;
+		return -ENODEV;
 
 	pciedev = to_pcie_device(dev);
 	status = driver->probe(pciedev, driver->id_table);
 	if (!status) {
-		printk(KERN_DEBUG "Load service driver %s on pcie device %s\n",
-			driver->name, dev->bus_id);
+		dev_printk(KERN_DEBUG, dev, "service driver %s loaded\n",
+			driver->name);
 		get_device(dev);
 	}
 	return status;

@@ -53,8 +53,8 @@ static int pcie_port_remove_service(stru
 	pciedev = to_pcie_device(dev);
  	driver = to_service_driver(dev->driver);
 	if (driver && driver->remove) { 
-		printk(KERN_DEBUG "Unload service driver %s on pcie device %s\n",
-			driver->name, dev->bus_id);
+		dev_printk(KERN_DEBUG, dev, "unloading service driver %s\n",
+			driver->name);
 		driver->remove(pciedev);
 		put_device(dev);
 	}

@@ -103,7 +103,7 @@ static int pcie_port_resume_service(stru
  */
 static void release_pcie_device(struct device *dev)
 {
-	printk(KERN_DEBUG "Free Port Service[%s]\n", dev->bus_id);
+	dev_printk(KERN_DEBUG, dev, "free port service\n");
 	kfree(to_pcie_device(dev));			
 }
 

@@ -150,7 +150,7 @@ static int assign_interrupt_mode(struct 
 	if (pos) {
 		struct msix_entry msix_entries[PCIE_PORT_DEVICE_MAXSERVICES] = 
 			{{0, 0}, {0, 1}, {0, 2}, {0, 3}};
-		printk("%s Found MSIX capability\n", __func__);
+		dev_info(&dev->dev, "found MSI-X capability\n");
 		status = pci_enable_msix(dev, msix_entries, nvec);
 		if (!status) {
 			int j = 0;

@@ -165,7 +165,7 @@ static int assign_interrupt_mode(struct 
 	if (status) {
 		pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
 		if (pos) {
-			printk("%s Found MSI capability\n", __func__);
+			dev_info(&dev->dev, "found MSI capability\n");
 			status = pci_enable_msi(dev);
 			if (!status) {
 				interrupt_mode = PCIE_PORT_MSI_MODE;

@@ -252,7 +252,7 @@ static struct pcie_device* alloc_pcie_de
 		return NULL;
 
 	pcie_device_init(parent, device, port_type, service_type, irq,irq_mode);
-	printk(KERN_DEBUG "Allocate Port Service[%s]\n", device->device.bus_id);
+	dev_printk(KERN_DEBUG, &device->device, "allocate port service\n");
 	return device;
 }
drivers/pci/pcie/portdrv_pci.c +2 -3
Index: work11/drivers/pci/pcie/portdrv_pci.c
===================================================================
@@ -91,9 +91,8 @@ static int __devinit pcie_portdrv_probe 
 	
 	pci_set_master(dev);
         if (!dev->irq && dev->pin) {
-		printk(KERN_WARNING 
-		"%s->Dev[%04x:%04x] has invalid IRQ. Check vendor BIOS\n", 
-		__func__, dev->vendor, dev->device);
+		dev_warn(&dev->dev, "device [%04x/%04x] has invalid IRQ; "
+			 "check vendor BIOS\n", dev->vendor, dev->device);
 	}
 	if (pcie_port_device_register(dev)) {
 		pci_disable_device(dev);
author
#6 lore raw

PATCH 5/5 x86 PCI: use dev_printk when possible

In reply to Bjorn Helgaas

Convert printks to use dev_printk().

I converted DBG() to dev_dbg(). This DBG() is from arch/x86/pci/pci.h and requires source-code modification to enable, so dev_dbg() seems roughly equivalent.

The printks in arch/x86/pci/irq.c were a little hairy, with lines printed in several pieces. I straightened it out a little, but another set of eyes would be good.

Signed-off-by Bjorn Helgaas <[email protected]>
---
 arch/x86/pci/fixup.c |    3 +
 arch/x86/pci/i386.c  |   26 ++++++--------
 arch/x86/pci/irq.c   |   91 ++++++++++++++++++++++-----------------------------
 arch/x86/pci/numa.c  |    5 +-
 4 files changed, 57 insertions(+), 68 deletions(-)
arch/x86/pci/fixup.c +2 -1
Index: work11/arch/x86/pci/fixup.c
===================================================================
@@ -23,7 +23,8 @@ static void __devinit pci_fixup_i450nx(s
 		pci_read_config_byte(d, reg++, &busno);
 		pci_read_config_byte(d, reg++, &suba);
 		pci_read_config_byte(d, reg++, &subb);
-		DBG("i450NX PXB %d: %02x/%02x/%02x\n", pxb, busno, suba, subb);
+		dev_dbg(&d->dev, "i450NX PXB %d: %02x/%02x/%02x\n", pxb, busno,
+			suba, subb);
 		if (busno)
 			pci_scan_bus_with_sysdata(busno);	/* Bus A */
 		if (suba < subb)
arch/x86/pci/i386.c +11 -15
Index: work11/arch/x86/pci/i386.c
===================================================================
@@ -128,10 +128,8 @@ static void __init pcibios_allocate_bus_
 				pr = pci_find_parent_resource(dev, r);
 				if (!r->start || !pr ||
 				    request_resource(pr, r) < 0) {
-					printk(KERN_ERR "PCI: Cannot allocate "
-						"resource region %d "
-						"of bridge %s\n",
-						idx, pci_name(dev));
+					dev_err(&dev->dev, "BAR %d: can't "
+						"allocate resource\n", idx);
 					/*
 					 * Something is wrong with the region.
 					 * Invalidate the resource to prevent

@@ -166,15 +164,15 @@ static void __init pcibios_allocate_reso
 			else
 				disabled = !(command & PCI_COMMAND_MEMORY);
 			if (pass == disabled) {
-				DBG("PCI: Resource %08lx-%08lx "
-				    "(f=%lx, d=%d, p=%d)\n",
-				    r->start, r->end, r->flags, disabled, pass);
+				dev_dbg(&dev->dev, "resource %#08llx-%#08llx "
+					"(f=%lx, d=%d, p=%d)\n",
+					(unsigned long long) r->start,
+					(unsigned long long) r->end,
+					r->flags, disabled, pass);
 				pr = pci_find_parent_resource(dev, r);
 				if (!pr || request_resource(pr, r) < 0) {
-					printk(KERN_ERR "PCI: Cannot allocate "
-						"resource region %d "
-						"of device %s\n",
-						idx, pci_name(dev));
+					dev_err(&dev->dev, "BAR %d: can't "
+						"allocate resource\n", idx);
 					/* We'll assign a new address later */
 					r->end -= r->start;
 					r->start = 0;

@@ -187,8 +185,7 @@ static void __init pcibios_allocate_reso
 				/* Turn the ROM off, leave the resource region,
 				 * but keep it unregistered. */
 				u32 reg;
-				DBG("PCI: Switching off ROM of %s\n",
-					pci_name(dev));
+				dev_dbg(&dev->dev, "disabling ROM\n");
 				r->flags &= ~IORESOURCE_ROM_ENABLE;
 				pci_read_config_dword(dev,
 						dev->rom_base_reg, &reg);

@@ -257,8 +254,7 @@ void pcibios_set_master(struct pci_dev *
 		lat = pcibios_max_latency;
 	else
 		return;
-	printk(KERN_DEBUG "PCI: Setting latency timer of device %s to %d\n",
-		pci_name(dev), lat);
+	dev_printk(KERN_DEBUG, &dev->dev, "setting latency timer to %d\n", lat);
 	pci_write_config_byte(dev, PCI_LATENCY_TIMER, lat);
 }
arch/x86/pci/irq.c +41 -50

226 lines not on this page · raw message

arch/x86/pci/numa.c +3 -2
Index: work11/arch/x86/pci/numa.c
===================================================================
@@ -155,13 +155,14 @@ static void __devinit pci_fixup_i450nx(s
 	u8 busno, suba, subb;
 	int quad = BUS2QUAD(d->bus->number);
 
-	printk("PCI: Searching for i450NX host bridges on %s\n", pci_name(d));
+	dev_info(&d->dev, "searching for i450NX host bridges\n");
 	reg = 0xd0;
 	for(pxb=0; pxb<2; pxb++) {
 		pci_read_config_byte(d, reg++, &busno);
 		pci_read_config_byte(d, reg++, &suba);
 		pci_read_config_byte(d, reg++, &subb);
-		DBG("i450NX PXB %d: %02x/%02x/%02x\n", pxb, busno, suba, subb);
+		dev_dbg(&d->dev, "i450NX PXB %d: %02x/%02x/%02x\n",
+			pxb, busno, suba, subb);
 		if (busno) {
 			/* Bus A */
 			pci_scan_bus_with_sysdata(QUADLOCAL2BUS(quad, busno));
reviewer
#7 lore raw

Re: [patch 5/5] x86 PCI: use dev_printk when possible

I guess the 'INTA#; rather than 'INTA' syntax I'm not fond of appears in this x86 patch too. I think if we don't really have anything useful to say about polarity, then we should not add an active low symbol.

thanks,
-Len

- printk(KERN_INFO "PCI: %s IRQ %d for device %s\n", msg, irq,
pci_name(dev));
+ dev_info(&dev->dev, "%s PCI INT%c# -> IRQ %d\n", msg, 'A' + pin,
irq);

#8 lore raw

Re: [patch 5/5] x86 PCI: use dev_printk when possible

In reply to Bjorn Helgaas
Quoted 8 lines · On Fri, 13 Jun 2008 10:52:14 -0600 Bjorn Helgaas <[email protected]> wrote:

I converted DBG() to dev_dbg(). This DBG() is from arch/x86/pci/pci.h and requires source-code modification to enable, so dev_dbg() seems roughly equivalent.

The printks in arch/x86/pci/irq.c were a little hairy, with lines printed in several pieces. I straightened it out a little, but another set of eyes would be good.

Quoted 7 lines
Signed-off-by Bjorn Helgaas <[email protected]>
---
 arch/x86/pci/fixup.c |    3 +
 arch/x86/pci/i386.c  |   26 ++++++--------
 arch/x86/pci/irq.c   |   91 ++++++++++++++++++++++-----------------------------
 arch/x86/pci/numa.c  |    5 +-

Got a bunch of rejects in arch/x86/pci/irq.c due to pending whitespace fixes. I fixed a couple of them but then I got bored, so the conversion in there is only partial.

Please prefer to raise patches against linux-next if poss, especially during late -rc's.

#9 lore raw

Re: [patch 5/5] x86 PCI: use dev_printk when possible

Quoted 20 lines · Andrew Morton

On Fri, 13 Jun 2008 10:52:14 -0600

Quoted 8 lines · Bjorn Helgaas

I converted DBG() to dev_dbg(). This DBG() is from arch/x86/pci/pci.h and requires source-code modification to enable, so dev_dbg() seems roughly equivalent.

The printks in arch/x86/pci/irq.c were a little hairy, with lines printed in several pieces. I straightened it out a little, but another set of eyes would be good.

Quoted 8 lines
Signed-off-by Bjorn Helgaas <[email protected]>
---
 arch/x86/pci/fixup.c |    3 +
 arch/x86/pci/i386.c  |   26 ++++++--------
 arch/x86/pci/irq.c   |   91

++++++++++++++++++++++----------------------------- arch/x86/pci/numa.c
| 5 +-

Quoted 6 lines

Got a bunch of rejects in arch/x86/pci/irq.c due to pending whitespace fixes. I fixed a couple of them but then I got bored, so the conversion in there is only partial.

Please prefer to raise patches against linux-next if poss, especially during late -rc's.

Yeah, the x86 tree(s) move too fast for me to keep up with. The change seems worthwhile though, maybe Ingo or one of the x86 team could just search & replace?

Jesse

author
#10 lore raw

Re: [patch 5/5] x86 PCI: use dev_printk when possible

Quoted 25 lines · Jesse Barnes
Quoted 24 lines · Andrew Morton

On Fri, 13 Jun 2008 10:52:14 -0600

Quoted 8 lines · Bjorn Helgaas

I converted DBG() to dev_dbg(). This DBG() is from arch/x86/pci/pci.h and requires source-code modification to enable, so dev_dbg() seems roughly equivalent.

The printks in arch/x86/pci/irq.c were a little hairy, with lines printed in several pieces. I straightened it out a little, but another set of eyes would be good.

Quoted 8 lines
Signed-off-by Bjorn Helgaas <[email protected]>
---
 arch/x86/pci/fixup.c |    3 +
 arch/x86/pci/i386.c  |   26 ++++++--------
 arch/x86/pci/irq.c   |   91

++++++++++++++++++++++----------------------------- arch/x86/pci/numa.c
| 5 +-

Got a bunch of rejects in arch/x86/pci/irq.c due to pending whitespace fixes. I fixed a couple of them but then I got bored, so the conversion in there is only partial.

Quoted 6 lines
Quoted 2 lines

Please prefer to raise patches against linux-next if poss, especially during late -rc's.

Yeah, the x86 tree(s) move too fast for me to keep up with. The change seems worthwhile though, maybe Ingo or one of the x86 team could just search & replace?

I'll fix it up.

Bjorn

#11 lore raw
In reply to Bjorn Helgaas
Quoted 6 lines · Bjorn Helgaas

These convert some printks to dev_printks. They are completely independent of each other; I'm just posting them together because it probably makes sense to review them together.

It'd be nice if a PCI person looked at pci_request_region(). To me, it looks like it prints the wrong BAR number (it prints "bar + 1").

Thanks Bjorn, I applied 2-4 to my linux-next tree. The x86 bits didn't apply for me, probably because my tree is slightly out of date, but those changes can come through Ingo & co.

Jesse