PCI, PNP: print resources consistently

PATCH Applied Bjorn Helgaas 4 participants 13 messages Oct 6, 2009 – Oct 26, 2009

Subsystems (6)

x86 ACPI PCI Library code +2 more subsystems

lore

Share & tools

lore

Bjorn Helgaas

author

2009-10-06 21:34 UTC #0 lore raw

cover PCI, PNP: print resources consistently

These enhance %pR so we can print resource types and flags more easily. This doesn't really add anything (other than a couple new messages about host bridge apertures), but hopefully it will make things more consistent and a bit easier to debug. Sample change (with "pci=use_crs"):

    -pci 0000:00:03.0: reg 10 32bit mmio: [0xf6000000-0xf6ffffff]
    -pci 0000:00:03.0: reg 14 io port: [0x2400-0x24ff]
    -pci 0000:00:03.0: reg 18 32bit mmio: [0xf5ff0000-0xf5ff0fff]
    -pci 0000:00:03.0: reg 30 32bit mmio pref: [0x000000-0x01ffff]
    +pci_root PNP0A03:00: host bridge window: [io  0x0000-0x0cff]
    +pci_root PNP0A03:00: host bridge window: [io  0x0000-0x2cfe]
    +pci_root PNP0A03:00: host bridge window: [io  0x03b0-0x03bb]
    +pci_root PNP0A03:00: host bridge window: [io  0x03c0-0x03df]
    +pci_root PNP0A03:00: host bridge window: [mem 0xf5d00000-0xf6ffffff]
    +pci_root PNP0A03:00: host bridge window: [mem 0x000a0000-0x000bffff]
    +pci 0000:00:03.0: reg 10: [mem 0xf6000000-0xf6ffffff]
    +pci 0000:00:03.0: reg 14: [io  0x2400-0x24ff]
    +pci 0000:00:03.0: reg 18: [mem 0xf5ff0000-0xf5ff0fff]
    +pci 0000:00:03.0: reg 30: [mem 0x00000000-0x0001ffff pref]

---

Bjorn Helgaas (7):
vsprintf: fix io/mem resource width
vsprintf: add %pR support for IRQ and DMA resources
vsprintf: add %pRt, %pRf to print struct resource details
PCI: print resources consistently with %pRt x86/PCI: print resources consistently with %pRt ia64/PCI: print resources consistently with %pRt
PNP: print resources consistently with %pRt

 arch/ia64/pci/pci.c     |   21 +++++++++---
 arch/x86/pci/acpi.c     |   14 ++++++--
 arch/x86/pci/i386.c     |   12 +++----
 drivers/pci/pci.c       |    4 +-
 drivers/pci/probe.c     |   26 +++++----------
 drivers/pci/setup-bus.c |    9 ++---
 drivers/pci/setup-res.c |   28 +++++++---------
 drivers/pnp/quirks.c    |   12 ++-----
 drivers/pnp/resource.c  |   10 ++----
 drivers/pnp/support.c   |   43 +++----------------------
 drivers/pnp/system.c    |   14 ++++----
 lib/vsprintf.c          |   80 ++++++++++++++++++++++++++++++++++++++---------
 12 files changed, 138 insertions(+), 135 deletions(-)

Signature · 1 line

Bjorn

Bjorn Helgaas

author

2009-10-06 21:34 UTC #1 lore raw

PATCH 1/7 vsprintf: fix io/mem resource width

The leading "0x" consumes field width, so leave space for it in addition to the 4 or 8 hex digits. This means we'll print "0x0000-0x01df" rather than "0x00-0x1df", for example.

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

---
 lib/vsprintf.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

lib/vsprintf.c +2 -2

index 33bed5e..7830576 100644
@@ -598,11 +598,11 @@ static char *resource_string(char *buf, char *end, struct resource *res,
 				struct printf_spec spec)
 {
 #ifndef IO_RSRC_PRINTK_SIZE
-#define IO_RSRC_PRINTK_SIZE	4
+#define IO_RSRC_PRINTK_SIZE	6
 #endif
 
 #ifndef MEM_RSRC_PRINTK_SIZE
-#define MEM_RSRC_PRINTK_SIZE	8
+#define MEM_RSRC_PRINTK_SIZE	10
 #endif
 	struct printf_spec num_spec = {
 		.base = 16,

Jesse Barnes

Applied

2009-10-26 20:40 UTC #2 lore raw

Re: [PATCH 1/7] vsprintf: fix io/mem resource width

Quoted 6 lines · On Tue, 06 Oct 2009 15:33:29 -0600 Bjorn Helgaas <[email protected]> wrote:

The leading "0x" consumes field width, so leave space for it in addition to the 4 or 8 hex digits. This means we'll print "0x0000-0x01df" rather than "0x00-0x1df", for example.
Signed-off-by Bjorn Helgaas <[email protected]>
---

Applied this series to my linux-next tree, thanks.

Signature · 1 line

Jesse Barnes, Intel Open Source Technology Center

Bjorn Helgaas

author

2009-10-06 21:33 UTC #3 lore raw

PATCH 2/7 vsprintf: add %pR support for IRQ and DMA resources

In reply to Bjorn Helgaas

Print addresses (IO port numbers and memory addresses) in hex, but print others (IRQs and DMA channels) in decimal. Only print the end if it's different from the start.

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

---
 lib/vsprintf.c |   31 +++++++++++++++++++++----------
 1 files changed, 21 insertions(+), 10 deletions(-)

lib/vsprintf.c +21 ~~-10~~

index 7830576..1b60aed 100644
@@ -604,26 +604,37 @@ static char *resource_string(char *buf, char *end, struct resource *res,
 #ifndef MEM_RSRC_PRINTK_SIZE
 #define MEM_RSRC_PRINTK_SIZE	10
 #endif
-	struct printf_spec num_spec = {
+	struct printf_spec hex_spec = {
 		.base = 16,
 		.precision = -1,
 		.flags = SPECIAL | SMALL | ZEROPAD,
 	};
-	/* room for the actual numbers, the two "0x", -, [, ] and the final zero */
-	char sym[4*sizeof(resource_size_t) + 8];
+	struct printf_spec dec_spec = {
+		.base = 10,
+		.precision = -1,
+		.flags = 0,
+	};
+	/* room for two actual numbers (decimal or hex), the two "0x", -, [, ]
+	 * and the final zero */
+	char sym[2*3*sizeof(resource_size_t) + 8];
 	char *p = sym, *pend = sym + sizeof(sym);
-	int size = -1;
+	int size = -1, addr = 0;
 
-	if (res->flags & IORESOURCE_IO)
+	if (res->flags & IORESOURCE_IO) {
 		size = IO_RSRC_PRINTK_SIZE;
-	else if (res->flags & IORESOURCE_MEM)
+		addr = 1;
+	} else if (res->flags & IORESOURCE_MEM) {
 		size = MEM_RSRC_PRINTK_SIZE;
+		addr = 1;
+	}
 
 	*p++ = '[';
-	num_spec.field_width = size;
-	p = number(p, pend, res->start, num_spec);
-	*p++ = '-';
-	p = number(p, pend, res->end, num_spec);
+	hex_spec.field_width = size;
+	p = number(p, pend, res->start, addr ? hex_spec : dec_spec);
+	if (res->start != res->end) {
+		*p++ = '-';
+		p = number(p, pend, res->end, addr ? hex_spec : dec_spec);
+	}
 	*p++ = ']';
 	*p = 0;

Bjorn Helgaas

author

2009-10-06 21:33 UTC #4 lore raw

PATCH 3/7 vsprintf: add %pRt, %pRf to print struct resource details

In reply to Bjorn Helgaas

This adds support for printing struct resource type and flag information. For example, "%pRt" looks like "[mem 0x80080000000-0x8008001ffff 64bit pref]", and "%pRf" looks like "[mem 0xff5e2000-0xff5e2007 pref flags 0x1]".

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

---
 lib/vsprintf.c |   51 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 44 insertions(+), 7 deletions(-)

lib/vsprintf.c +44 -7

index 1b60aed..a6e1951 100644
@@ -595,7 +595,7 @@ static char *symbol_string(char *buf, char *end, void *ptr,
 }
 
 static char *resource_string(char *buf, char *end, struct resource *res,
-				struct printf_spec spec)
+				struct printf_spec spec, const char *fmt)
 {
 #ifndef IO_RSRC_PRINTK_SIZE
 #define IO_RSRC_PRINTK_SIZE	6

@@ -614,9 +614,21 @@ static char *resource_string(char *buf, char *end, struct resource *res,
 		.precision = -1,
 		.flags = 0,
 	};
-	/* room for two actual numbers (decimal or hex), the two "0x", -, [, ]
-	 * and the final zero */
-	char sym[2*3*sizeof(resource_size_t) + 8];
+	struct printf_spec str_spec = {
+		.field_width = -1,
+		.precision = 10,
+		.flags = LEFT,
+	};
+	struct printf_spec flag_spec = {
+		.base = 16,
+		.precision = -1,
+		.flags = SPECIAL | SMALL,
+	};
+	/*
+	 * room for three actual numbers (decimal or hex), plus
+	 * "[mem 0x-0x 64bit pref disabled flags 0x]\0"
+	 */
+	char sym[3*3*sizeof(resource_size_t) + 41];
 	char *p = sym, *pend = sym + sizeof(sym);
 	int size = -1, addr = 0;

@@ -629,12 +641,35 @@ static char *resource_string(char *buf, char *end, struct resource *res,
 	}
 
 	*p++ = '[';
+	if (fmt[1] == 't' || fmt[1] == 'f') {
+		if (res->flags & IORESOURCE_IO)
+			p = string(p, pend, "io  ", str_spec);
+		else if (res->flags & IORESOURCE_MEM)
+			p = string(p, pend, "mem ", str_spec);
+		else if (res->flags & IORESOURCE_IRQ)
+			p = string(p, pend, "irq ", str_spec);
+		else if (res->flags & IORESOURCE_DMA)
+			p = string(p, pend, "dma ", str_spec);
+	}
 	hex_spec.field_width = size;
 	p = number(p, pend, res->start, addr ? hex_spec : dec_spec);
 	if (res->start != res->end) {
 		*p++ = '-';
 		p = number(p, pend, res->end, addr ? hex_spec : dec_spec);
 	}
+	if (fmt[1] == 't' || fmt[1] == 'f') {
+		if (res->flags & IORESOURCE_MEM_64)
+			p = string(p, pend, " 64bit", str_spec);
+		if (res->flags & IORESOURCE_PREFETCH)
+			p = string(p, pend, " pref", str_spec);
+		if (res->flags & IORESOURCE_DISABLED)
+			p = string(p, pend, " disabled", str_spec);
+		if (fmt[1] == 'f') {
+			p = string(p, pend, " flags ", str_spec);
+			p = number(p, pend, res->flags & ~IORESOURCE_TYPE_BITS,
+				   flag_spec);
+		}
+	}
 	*p++ = ']';
 	*p = 0;

@@ -812,8 +847,10 @@ static char *ip4_addr_string(char *buf, char *end, const u8 *addr,
  * - 'f' For simple symbolic function names without offset
  * - 'S' For symbolic direct pointers with offset
  * - 's' For symbolic direct pointers without offset
- * - 'R' For a struct resource pointer, it prints the range of
- *       addresses (not the name nor the flags)
+ * - 'R' For a struct resource pointer, print:
+ *       R   address range only ([0x0-0x1f])
+ *       Rt  type and range ([mem 0x0-0x1f 64bit pref])
+ *       Rf  type, range, and flags ([mem 0x0-0x1f 64bit pref flags 0x1])
  * - 'M' For a 6-byte MAC address, it prints the address in the
  *       usual colon-separated hex notation
  * - 'm' For a 6-byte MAC address, it prints the hex address without colons

@@ -844,7 +881,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'S':
 		return symbol_string(buf, end, ptr, spec, *fmt);
 	case 'R':
-		return resource_string(buf, end, ptr, spec);
+		return resource_string(buf, end, ptr, spec, fmt);
 	case 'M':			/* Colon separated: 00:01:02:03:04:05 */
 	case 'm':			/* Contiguous: 000102030405 */
 		return mac_address_string(buf, end, ptr, spec, fmt);

Bjorn Helgaas

author

2009-10-06 21:33 UTC #5 lore raw

PATCH 4/7 PCI: print resources consistently with %pRt

In reply to Bjorn Helgaas

This uses %pRt to print additional resource information (type, size, prefetchability, etc.) consistently.

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

---
 drivers/pci/pci.c       |    4 +---
 drivers/pci/probe.c     |   26 ++++++++------------------
 drivers/pci/setup-bus.c |    9 +++------
 drivers/pci/setup-res.c |   28 ++++++++++++----------------
 4 files changed, 24 insertions(+), 43 deletions(-)

drivers/pci/pci.c +1 -3

index 6edecff..d760776 100644
@@ -1665,9 +1665,7 @@ static int __pci_request_region(struct pci_dev *pdev, int bar, const char *res_n
 	return 0;
 
 err_out:
-	dev_warn(&pdev->dev, "BAR %d: can't reserve %s region %pR\n",
-		 bar,
-		 pci_resource_flags(pdev, bar) & IORESOURCE_IO ? "I/O" : "mem",
+	dev_warn(&pdev->dev, "BAR %d: can't reserve %pRt\n", bar,
 		 &pdev->resource[bar]);
 	return -EBUSY;
 }

drivers/pci/probe.c +8 ~~-18~~

index ca08416..94b2b9c 100644
@@ -222,6 +222,8 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 		if (!sz64)
 			goto fail;
 
+		res->flags |= IORESOURCE_MEM_64;
+
 		if ((sizeof(resource_size_t) < 8) && (sz64 > 0x100000000ULL)) {
 			dev_err(&dev->dev, "can't handle 64-bit BAR\n");
 			goto fail;

@@ -234,14 +236,9 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 		} else {
 			res->start = l64;
 			res->end = l64 + sz64;
-			dev_printk(KERN_DEBUG, &dev->dev,
-				"reg %x %s: %pR\n", pos,
-				 (res->flags & IORESOURCE_PREFETCH) ?
-					"64bit mmio pref" : "64bit mmio",
-				 res);
+			dev_printk(KERN_DEBUG, &dev->dev, "reg %x: %pRt\n",
+				   pos, res);
 		}
-
-		res->flags |= IORESOURCE_MEM_64;
 	} else {
 		sz = pci_size(l, sz, mask);

@@ -251,11 +248,7 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 		res->start = l;
 		res->end = l + sz;
 
-		dev_printk(KERN_DEBUG, &dev->dev, "reg %x %s: %pR\n", pos,
-			(res->flags & IORESOURCE_IO) ? "io port" :
-			 ((res->flags & IORESOURCE_PREFETCH) ?
-				 "32bit mmio pref" : "32bit mmio"),
-			res);
+		dev_printk(KERN_DEBUG, &dev->dev, "reg %x: %pRt\n", pos, res);
 	}
 
  out:

@@ -323,7 +316,7 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
 			res->start = base;
 		if (!res->end)
 			res->end = limit + 0xfff;
-		dev_printk(KERN_DEBUG, &dev->dev, "bridge io port: %pR\n", res);
+		dev_printk(KERN_DEBUG, &dev->dev, "bridge window: %pRt\n", res);
 	}
 
 	res = child->resource[1];

@@ -335,8 +328,7 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
 		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
 		res->start = base;
 		res->end = limit + 0xfffff;
-		dev_printk(KERN_DEBUG, &dev->dev, "bridge 32bit mmio: %pR\n",
-			res);
+		dev_printk(KERN_DEBUG, &dev->dev, "bridge window: %pRt\n", res);
 	}
 
 	res = child->resource[2];

@@ -375,9 +367,7 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
 			res->flags |= IORESOURCE_MEM_64;
 		res->start = base;
 		res->end = limit + 0xfffff;
-		dev_printk(KERN_DEBUG, &dev->dev, "bridge %sbit mmio pref: %pR\n",
-			(res->flags & PCI_PREF_RANGE_TYPE_64) ? "64" : "32",
-			res);
+		dev_printk(KERN_DEBUG, &dev->dev, "bridge window: %pRt\n", res);
 	}
 }

drivers/pci/setup-bus.c +3 -6

index cb1a027..ceb7533 100644
@@ -390,8 +390,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			align = pci_resource_alignment(dev, r);
 			order = __ffs(align) - 20;
 			if (order > 11) {
-				dev_warn(&dev->dev, "BAR %d bad alignment %llx: "
-					 "%pR\n", i, (unsigned long long)align, r);
+				dev_warn(&dev->dev, "BAR %d: bad alignment %llx: "
+					 "%pRt\n", i, (unsigned long long)align, r);
 				r->flags = 0;
 				continue;
 			}

@@ -582,10 +582,7 @@ static void pci_bus_dump_res(struct pci_bus *bus)
                 if (!res || !res->end)
                         continue;
 
-		dev_printk(KERN_DEBUG, &bus->dev, "resource %d %s %pR\n", i,
-			   (res->flags & IORESOURCE_IO) ? "io: " :
-			    ((res->flags & IORESOURCE_PREFETCH)? "pref mem":"mem:"),
-			   res);
+		dev_printk(KERN_DEBUG, &bus->dev, "resource %d %pRt\n", i, res);
         }
 }

drivers/pci/setup-res.c +12 ~~-16~~

index c54526b..5e78f20 100644
@@ -51,11 +51,9 @@ void pci_update_resource(struct pci_dev *dev, int resno)
 
 	pcibios_resource_to_bus(dev, &region, res);
 
-	dev_dbg(&dev->dev, "BAR %d: got res %pR bus [%#llx-%#llx] "
-		"flags %#lx\n", resno, res,
-		 (unsigned long long)region.start,
-		 (unsigned long long)region.end,
-		 (unsigned long)res->flags);
+	dev_dbg(&dev->dev, "BAR %d: got %pRf (bus addr [%#llx-%#llx])\n",
+		resno, res, (unsigned long long)region.start,
+		(unsigned long long)region.end);
 
 	new = region.start | (res->flags & PCI_REGION_FLAG_MASK);
 	if (res->flags & IORESOURCE_IO)

@@ -91,9 +89,9 @@ void pci_update_resource(struct pci_dev *dev, int resno)
 		}
 	}
 	res->flags &= ~IORESOURCE_UNSET;
-	dev_dbg(&dev->dev, "BAR %d: moved to bus [%#llx-%#llx] flags %#lx\n",
+	dev_dbg(&dev->dev, "BAR %d: moved to bus addr [%#llx-%#llx]\n",
 		resno, (unsigned long long)region.start,
-		(unsigned long long)region.end, res->flags);
+		(unsigned long long)region.end);
 }
 
 int pci_claim_resource(struct pci_dev *dev, int resource)

@@ -110,7 +108,7 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
 
 	if (err) {
 		const char *dtype = resource < PCI_BRIDGE_RESOURCES ? "device" : "bridge";
-		dev_err(&dev->dev, "BAR %d: %s of %s %pR\n",
+		dev_err(&dev->dev, "BAR %d: %s %s %pRt\n",
 			resource,
 			root ? "address space collision on" :
 				"no parent found for",

@@ -181,9 +179,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
 
 	align = pci_resource_alignment(dev, res);
 	if (!align) {
-		dev_info(&dev->dev, "BAR %d: can't allocate resource (bogus "
-			"alignment) %pR flags %#lx\n",
-			resno, res, res->flags);
+		dev_info(&dev->dev, "BAR %d: can't allocate %pRf "
+			 "(bogus alignment)\n", resno, res);
 		return -EINVAL;
 	}

@@ -199,8 +196,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
 	}
 
 	if (ret)
-		dev_info(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
-			resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
+		dev_info(&dev->dev, "BAR %d: can't allocate %pRt\n",
+			 resno, res);
 
 	return ret;
 }

@@ -225,9 +222,8 @@ void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
 
 		r_align = pci_resource_alignment(dev, r);
 		if (!r_align) {
-			dev_warn(&dev->dev, "BAR %d: bogus alignment "
-				"%pR flags %#lx\n",
-				i, r, r->flags);
+			dev_warn(&dev->dev, "BAR %d: bogus alignment %pRf\n",
+				 i, r);
 			continue;
 		}
 		for (list = head; ; list = list->next) {

Bjorn Helgaas

author

2009-10-06 21:33 UTC #6 lore raw

PATCH 5/7 x86/PCI: print resources consistently with %pRt

In reply to Bjorn Helgaas

This uses %pRt to print additional resource information (type, size, prefetchability, etc.) consistently.

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

---
 arch/x86/pci/acpi.c |   14 +++++++++++---
 arch/x86/pci/i386.c |   12 +++++-------
 2 files changed, 16 insertions(+), 10 deletions(-)

arch/x86/pci/acpi.c +11 -3

index 2297280..4d4f1df 100644
@@ -7,6 +7,7 @@
 #include <asm/pci_x86.h>
 
 struct pci_root_info {
+	struct acpi_device *bridge;
 	char *name;
 	unsigned int res_num;
 	struct resource *res;

@@ -107,12 +108,18 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
 	res->child = NULL;
 
 	if (insert_resource(root, res)) {
-		printk(KERN_ERR "PCI: Failed to allocate 0x%lx-0x%lx "
-			"from %s for %s\n", (unsigned long) res->start,
-			(unsigned long) res->end, root->name, info->name);
+		dev_err(&info->bridge->dev, "can't allocate %pRt\n", res);
 	} else {
 		info->bus->resource[info->res_num] = res;
 		info->res_num++;
+		if (addr.translation_offset)
+			dev_info(&info->bridge->dev, "host bridge window: %pRt "
+				 "(PCI address [%#llx-%#llx])\n",
+				 res, res->start - addr.translation_offset,
+				 res->end - addr.translation_offset);
+		else
+			dev_info(&info->bridge->dev,
+				 "host bridge window: %pRt\n", res);
 	}
 	return AE_OK;
 }

@@ -124,6 +131,7 @@ get_current_resources(struct acpi_device *device, int busnum,
 	struct pci_root_info info;
 	size_t size;
 
+	info.bridge = device;
 	info.bus = bus;
 	info.res_num = 0;
 	acpi_walk_resources(device->handle, METHOD_NAME__CRS, count_resource,

arch/x86/pci/i386.c +5 -7

index b22d13b..a70a85d 100644
@@ -129,7 +129,7 @@ static void __init pcibios_allocate_bus_resources(struct list_head *bus_list)
 					continue;
 				if (!r->start ||
 				    pci_claim_resource(dev, idx) < 0) {
-					dev_info(&dev->dev, "BAR %d: can't allocate resource\n", idx);
+					dev_info(&dev->dev, "BAR %d: can't allocate %pRt\n", idx, r);
 					/*
 					 * Something is wrong with the region.
 					 * Invalidate the resource to prevent

@@ -164,12 +164,10 @@ static void __init pcibios_allocate_resources(int pass)
 			else
 				disabled = !(command & PCI_COMMAND_MEMORY);
 			if (pass == disabled) {
-				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);
+				dev_dbg(&dev->dev, "%pRf (d=%d, p=%d)\n", r,
+					disabled, pass);
 				if (pci_claim_resource(dev, idx) < 0) {
-					dev_info(&dev->dev, "BAR %d: can't allocate resource\n", idx);
+					dev_info(&dev->dev, "BAR %d: can't allocate %pRt\n", idx, r);
 					/* We'll assign a new address later */
 					r->end -= r->start;
 					r->start = 0;

@@ -182,7 +180,7 @@ static void __init pcibios_allocate_resources(int pass)
 				/* Turn the ROM off, leave the resource region,
 				 * but keep it unregistered. */
 				u32 reg;
-				dev_dbg(&dev->dev, "disabling ROM\n");
+				dev_dbg(&dev->dev, "disabling ROM %pRt\n", r);
 				r->flags &= ~IORESOURCE_ROM_ENABLE;
 				pci_read_config_dword(dev,
 						dev->rom_base_reg, &reg);

Yinghai Lu

2009-10-07 18:43 UTC #7 lore raw

Re: [PATCH 5/7] x86/PCI: print resources consistently with %pRt

Quoted 22 lines · Bjorn Helgaas

This uses %pRt to print additional resource information (type, size, prefetchability, etc.) consistently.
Signed-off-by Bjorn Helgaas <[email protected]>
---
arch/x86/pci/acpi.c | 14 +++++++++++---
arch/x86/pci/i386.c | 12 +++++-------
 2 files changed, 16 insertions(+), 10 deletions(-)
arch/x86/pci/acpi.c +1 -0
index 2297280..4d4f1df 100644
@@ -7,6 +7,7 @@
  #include <asm/pci_x86.h>
 
  struct pci_root_info {
+       struct acpi_device *bridge;
        char *name;
        unsigned int res_num;
        struct resource *res;

arch/x86/pci/acpi.c setup_resource(struct acpi_resource *acpi_res, void *data) · line 111

Quoted 7 lines

arch/x86/pci/acpi.c +1 -3

@@ -107,12 +108,18 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
        res->child = NULL;
 
        if (insert_resource(root, res)) {
-               printk(KERN_ERR "PCI: Failed to allocate 0x%lx-0x%lx "
-                       "from %s for %s\n", (unsigned long) res->start,
-                       (unsigned long) res->end, root->name, info->name);
+               dev_err(&info->bridge->dev, "can't allocate %pRt\n", res);

better to keep that root name ?

Bjorn Helgaas

author

2009-10-07 19:15 UTC #8 lore raw

Re: [PATCH 5/7] x86/PCI: print resources consistently with %pRt

Quoted 25 lines · Yinghai Lu

Quoted 24 lines · Bjorn Helgaas
This uses %pRt to print additional resource information (type, size, prefetchability, etc.) consistently.
Signed-off-by Bjorn Helgaas <[email protected]>
---
arch/x86/pci/acpi.c | 14 +++++++++++---
arch/x86/pci/i386.c | 12 +++++-------
 2 files changed, 16 insertions(+), 10 deletions(-)
arch/x86/pci/acpi.c +1 -0
index 2297280..4d4f1df 100644
@@ -7,6 +7,7 @@
  #include <asm/pci_x86.h>
 
  struct pci_root_info {
+       struct acpi_device *bridge;
        char *name;
        unsigned int res_num;
        struct resource *res;
@@ -107,12 +108,18 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
        res->child = NULL;
 

Quoted 7 lines

arch/x86/pci/acpi.c setup_resource(struct acpi_resource *acpi_res, void *data) · line 111

Quoted 5 lines

arch/x86/pci/acpi.c +1 -3

        if (insert_resource(root, res)) {
-               printk(KERN_ERR "PCI: Failed to allocate 0x%lx-0x%lx "
-                       "from %s for %s\n", (unsigned long) res->start,
-                       (unsigned long) res->end, root->name, info->name);
+               dev_err(&info->bridge->dev, "can't allocate %pRt\n", res);

better to keep that root name ?

The message changes like this:

-PCI: Failed to allocate to allocate 0x0-0x3fff from PCI IO for PCI Bus 0000:00 +pci_root PNP0A03:01: can't allocate [io 0x0000-0x3fff]

I don't think changing "PCI IO" to "io" is really a problem. In fact, strictly speaking, "PCI IO" is the wrong name for ioport_resource -- we're talking about a host bridge, and the upstream side is not PCI at all.

However, I do think it would be more useful to mention the fact that we failed to allocate a *window*, e.g.,

pci_root PNP0A03:00: can't allocate host bridge window [io 0x0000-0x3fff]

I did consider keeping the PCI bus ("0000:00"), but I decided we already have that information here:

ACPI: PCI Root Bridge [PCI0] (0000:00)

and it doesn't seem worthwhile to me to repeat the bus number in all the host bridge-related messages. Right now, there's nothing to tie the PCI0 to the PNP0A03:00 (and "PCI0" shouldn't be exposed to users anyway), but someday when I finally convince Len to use dev_printk in ACPI, it could look something like this:

pci_root PNP0A03:00: PCI host bridge to pci_bus 0000:00

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Len Brown

reviewer Acked-by

2009-10-09 21:14 UTC #9 lore raw

Re: [PATCH 5/7] x86/PCI: print resources consistently with %pRt

Quoted 21 lines

The message changes like this:
-PCI: Failed to allocate to allocate 0x0-0x3fff from PCI IO for PCI Bus 0000:00 +pci_root PNP0A03:01: can't allocate [io 0x0000-0x3fff]
I don't think changing "PCI IO" to "io" is really a problem. In fact, strictly speaking, "PCI IO" is the wrong name for ioport_resource -- we're talking about a host bridge, and the upstream side is not PCI at all.
However, I do think it would be more useful to mention the fact that we failed to allocate a *window*, e.g.,
pci_root PNP0A03:00: can't allocate host bridge window [io 0x0000-0x3fff]
I did consider keeping the PCI bus ("0000:00"), but I decided we already have that information here:
ACPI: PCI Root Bridge [PCI0] (0000:00)

Quoted 7 lines

and it doesn't seem worthwhile to me to repeat the bus number in all the host bridge-related messages. Right now, there's nothing to tie the PCI0 to the PNP0A03:00 (and "PCI0" shouldn't be exposed to users anyway), but someday when I finally convince Len to use dev_printk in ACPI, it could look something like this:
pci_root PNP0A03:00: PCI host bridge to pci_bus 0000:00

The last time we looked at using dev_printk() in ACPI, it looked like the leading ACPI: would go away, and the concern was that would hinder, rather than help, people in reporting issues to the right place.

I have no problem with using dev_printk() where it makes sense, but only if it makes the message more useful rather than less useful.

BTW. I like the consistency provided by the series at hand. I assume that it will go through Jesse's, and for that, consider the relevant bits...

Acked-by Len Brown <[email protected]>

cheers,
-Len

Bjorn Helgaas

author

2009-10-10 15:40 UTC #10 lore raw

Re: [PATCH 5/7] x86/PCI: print resources consistently with %pRt

Quoted 28 lines · Len Brown

Quoted 20 lines
The message changes like this:
-PCI: Failed to allocate to allocate 0x0-0x3fff from PCI IO for PCI Bus 0000:00 +pci_root PNP0A03:01: can't allocate [io 0x0000-0x3fff]
I don't think changing "PCI IO" to "io" is really a problem. In fact, strictly speaking, "PCI IO" is the wrong name for ioport_resource -- we're talking about a host bridge, and the upstream side is not PCI at all.
However, I do think it would be more useful to mention the fact that we failed to allocate a *window*, e.g.,
pci_root PNP0A03:00: can't allocate host bridge window [io 0x0000-0x3fff]
I did consider keeping the PCI bus ("0000:00"), but I decided we already have that information here:
ACPI: PCI Root Bridge [PCI0] (0000:00)
Quoted 7 lines
and it doesn't seem worthwhile to me to repeat the bus number in all the host bridge-related messages. Right now, there's nothing to tie the PCI0 to the PNP0A03:00 (and "PCI0" shouldn't be exposed to users anyway), but someday when I finally convince Len to use dev_printk in ACPI, it could look something like this:
pci_root PNP0A03:00: PCI host bridge to pci_bus 0000:00

Quoted 8 lines

The last time we looked at using dev_printk() in ACPI, it looked like the leading ACPI: would go away, and the concern was that would hinder, rather than help, people in reporting issues to the right place.
I have no problem with using dev_printk() where it makes sense, but only if it makes the message more useful rather than less useful.

Yes, I'm sorry, that was kind of a low blow that didn't represent your concerns fairly.

Quoted 5 lines

BTW. I like the consistency provided by the series at hand. I assume that it will go through Jesse's, and for that, consider the relevant bits...
Acked-by Len Brown <[email protected]>

Thanks. I'm planning to post an updated series soon. Joe Perches suggested reducing the stack usage in vsprintf, so I'm working on that, and I might add another message or two.

Bjorn

Bjorn Helgaas

author

2009-10-06 21:33 UTC #11 lore raw

PATCH 6/7 ia64/PCI: print resources consistently with %pRt

In reply to Bjorn Helgaas

This uses %pRt to print additional resource information (type, size, prefetchability, etc.) consistently.

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

---
 arch/ia64/pci/pci.c |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

arch/ia64/pci/pci.c +16 -5

index 7de76dd..0f90259 100644
@@ -126,6 +126,7 @@ alloc_pci_controller (int seg)
 }
 
 struct pci_root_info {
+	struct acpi_device *bridge;
 	struct pci_controller *controller;
 	char *name;
 };

@@ -292,9 +293,19 @@ static __devinit acpi_status add_window(struct acpi_resource *res, void *data)
 	window->offset = offset;
 
 	if (insert_resource(root, &window->resource)) {
-		printk(KERN_ERR "alloc 0x%llx-0x%llx from %s for %s failed\n",
-			window->resource.start, window->resource.end,
-			root->name, info->name);
+		dev_err(&info->bridge->dev, "can't allocate %pRt\n",
+			&window->resource);
+	} else {
+		if (offset)
+			dev_info(&info->bridge->dev, "host bridge window: %pRt "
+				 "(PCI address [%#llx-%#llx])\n",
+				 &window->resource,
+				 window->resource.start - offset,
+				 window->resource.end - offset);
+		else
+			dev_info(&info->bridge->dev,
+				 "host bridge window: %pRt\n",
+				 &window->resource);
 	}
 
 	return AE_OK;

@@ -314,8 +325,7 @@ pcibios_setup_root_windows(struct pci_bus *bus, struct pci_controller *ctrl)
 		    (res->end - res->start < 16))
 			continue;
 		if (j >= PCI_BUS_NUM_RESOURCES) {
-			printk("Ignoring range [%#llx-%#llx] (%lx)\n",
-					res->start, res->end, res->flags);
+			dev_warn(&bus->dev, "ignoring %pRf (no space)\n", res);
 			continue;
 		}
 		bus->resource[j++] = res;

@@ -359,6 +369,7 @@ pci_acpi_scan_root(struct acpi_device *device, int domain, int bus)
 			goto out3;
 
 		sprintf(name, "PCI Bus %04x:%02x", domain, bus);
+		info.bridge = device;
 		info.controller = controller;
 		info.name = name;
 		acpi_walk_resources(device->handle, METHOD_NAME__CRS,

Bjorn Helgaas

author

2009-10-06 21:34 UTC #12 lore raw

PATCH 7/7 PNP: print resources consistently with %pRt

In reply to Bjorn Helgaas

This uses %pRt and %pRf to print additional resource information (type, size, prefetchability, etc.) consistently.

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

---
 drivers/pnp/quirks.c   |   12 +++---------
 drivers/pnp/resource.c |   10 ++++------
 drivers/pnp/support.c  |   43 +++++--------------------------------------
 drivers/pnp/system.c   |   14 ++++++--------
 4 files changed, 18 insertions(+), 61 deletions(-)

drivers/pnp/quirks.c +3 -9

index 8473fe5..3a2031b 100644
@@ -285,15 +285,9 @@ static void quirk_system_pci_resources(struct pnp_dev *dev)
 				 * the PCI region, and that might prevent a PCI
 				 * driver from requesting its resources.
 				 */
-				dev_warn(&dev->dev, "%s resource "
-					"(0x%llx-0x%llx) overlaps %s BAR %d "
-					"(0x%llx-0x%llx), disabling\n",
-					pnp_resource_type_name(res),
-					(unsigned long long) pnp_start,
-					(unsigned long long) pnp_end,
-					pci_name(pdev), i,
-					(unsigned long long) pci_start,
-					(unsigned long long) pci_end);
+				dev_warn(&dev->dev, "resource %pRt overlaps %s "
+					 "BAR %d %pRt, disabling\n", res,
+					 pci_name(pdev), i, &pdev->resource[i]);
 				res->flags |= IORESOURCE_DISABLED;
 			}
 		}

drivers/pnp/resource.c +4 -6

index ba97654..18557d7 100644
@@ -517,7 +517,7 @@ struct pnp_resource *pnp_add_irq_resource(struct pnp_dev *dev, int irq,
 	res->start = irq;
 	res->end = irq;
 
-	pnp_dbg(&dev->dev, "  add irq %d flags %#x\n", irq, flags);
+	pnp_dbg(&dev->dev, "  add %pRf\n", res);
 	return pnp_res;
 }

@@ -538,7 +538,7 @@ struct pnp_resource *pnp_add_dma_resource(struct pnp_dev *dev, int dma,
 	res->start = dma;
 	res->end = dma;
 
-	pnp_dbg(&dev->dev, "  add dma %d flags %#x\n", dma, flags);
+	pnp_dbg(&dev->dev, "  add %pRf\n", res);
 	return pnp_res;
 }

@@ -562,8 +562,7 @@ struct pnp_resource *pnp_add_io_resource(struct pnp_dev *dev,
 	res->start = start;
 	res->end = end;
 
-	pnp_dbg(&dev->dev, "  add io  %#llx-%#llx flags %#x\n",
-		(unsigned long long) start, (unsigned long long) end, flags);
+	pnp_dbg(&dev->dev, "  add %pRf\n", res);
 	return pnp_res;
 }

@@ -587,8 +586,7 @@ struct pnp_resource *pnp_add_mem_resource(struct pnp_dev *dev,
 	res->start = start;
 	res->end = end;
 
-	pnp_dbg(&dev->dev, "  add mem %#llx-%#llx flags %#x\n",
-		(unsigned long long) start, (unsigned long long) end, flags);
+	pnp_dbg(&dev->dev, "  add %pRf\n", res);
 	return pnp_res;
 }

drivers/pnp/support.c +5 ~~-38~~

52 lines not on this page · raw message

drivers/pnp/system.c +6 -8

index 59b9092..242d3a8 100644
@@ -22,11 +22,11 @@ static const struct pnp_device_id pnp_dev_table[] = {
 	{"", 0}
 };
 
-static void reserve_range(struct pnp_dev *dev, resource_size_t start,
-			  resource_size_t end, int port)
+static void reserve_range(struct pnp_dev *dev, struct resource *r, int port)
 {
 	char *regionid;
 	const char *pnpid = dev_name(&dev->dev);
+	resource_size_t start = r->start, end = r->end;
 	struct resource *res;
 
 	regionid = kmalloc(16, GFP_KERNEL);

@@ -48,10 +48,8 @@ static void reserve_range(struct pnp_dev *dev, resource_size_t start,
 	 * example do reserve stuff they know about too, so we may well
 	 * have double reservations.
 	 */
-	dev_info(&dev->dev, "%s range 0x%llx-0x%llx %s reserved\n",
-		port ? "ioport" : "iomem",
-		(unsigned long long) start, (unsigned long long) end,
-		res ? "has been" : "could not be");
+	dev_info(&dev->dev, "%pRt %s reserved\n", r,
+		 res ? "has been" : "could not be");
 }
 
 static void reserve_resources_of_dev(struct pnp_dev *dev)

@@ -77,14 +75,14 @@ static void reserve_resources_of_dev(struct pnp_dev *dev)
 		if (res->end < res->start)
 			continue;	/* invalid */
 
-		reserve_range(dev, res->start, res->end, 1);
+		reserve_range(dev, res, 1);
 	}
 
 	for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) {
 		if (res->flags & IORESOURCE_DISABLED)
 			continue;
 
-		reserve_range(dev, res->start, res->end, 0);
+		reserve_range(dev, res, 0);
 	}
 }