PCI, PNP: print resources consistently

PATCH In review Bjorn Helgaas 3 participants 16 messages Oct 13, 2009 – Oct 22, 2009

Subsystems (6)

x86 ACPI PCI Library code +2 more subsystems

lore

Share & tools

lore

Bjorn Helgaas

author

2009-10-13 19:23 UTC #0 lore raw

v2 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]

Changes from initial post to v2:
- tidied vsprintf stack buffer to shrink and compute size more accurately (Joe Perches)
- use %pR for decoding, %pr for "raw" (with type and flags) instead of adding %pRt and %pRf
- split cleanup from printk changes for easier review

I plan some future patches to add more messages and change some dev_dbg() to dev_info(), so this series is strictly to change the resource formatting.

---

Bjorn Helgaas (9):
vsprintf: fix io/mem resource width
vsprintf: add %pR support for IRQ and DMA resources
vsprintf: add %pR decoding and %pr for raw struct resource
PCI: set IORESOURCE_MEM_64 before printing resource
PCI: trivial bridge resource factorization
PCI: print resources consistently with %pR x86/PCI: print resources consistently with %pR ia64/PCI: print resources consistently with %pR
PNP: print resources consistently with %pR

 arch/ia64/pci/pci.c     |   24 ++++++++++---
 arch/x86/pci/acpi.c     |   15 ++++++--
 arch/x86/pci/i386.c     |   13 +++----
 drivers/pci/pci.c       |    4 +-
 drivers/pci/probe.c     |   29 +++++-----------
 drivers/pci/quirks.c    |    2 +
 drivers/pci/setup-bus.c |   82 +++++++++++++++++++-------------------------
 drivers/pci/setup-res.c |   20 +++++------
 drivers/pnp/quirks.c    |   13 ++-----
 drivers/pnp/resource.c  |   10 ++---
 drivers/pnp/support.c   |   43 +++--------------------
 drivers/pnp/system.c    |   14 +++-----
 lib/vsprintf.c          |   87 ++++++++++++++++++++++++++++++++++++++---------
 13 files changed, 183 insertions(+), 173 deletions(-)

Signature · 1 line

Bjorn

Bjorn Helgaas

author

2009-10-13 19:22 UTC #1 lore raw

PATCH v2 1/9 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,

Bjorn Helgaas

author

2009-10-13 19:22 UTC #2 lore raw

PATCH v2 2/9 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 |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)

lib/vsprintf.c +23 ~~-11~~

index 7830576..fcbe69d 100644
@@ -604,28 +604,40 @@ 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,
+	};
+	/* 32-bit res (sizeof==4): 10 chars in dec, 10 in hex ("0x" + 8)
+	 * 64-bit res (sizeof==8): 20 chars in dec, 18 in hex ("0x" + 16) */
+#define RSRC_BUF_SIZE	((2 * sizeof(resource_size_t)) + 4)
+	char sym[2*RSRC_BUF_SIZE + sizeof("[-]")];
 	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;
+	*p = '\0';
 
 	return string(buf, end, sym, spec);
 }

Bjorn Helgaas

author

2009-10-13 19:22 UTC #3 lore raw

PATCH v2 3/9 vsprintf: add %pR decoding and %pr for raw struct resource

In reply to Bjorn Helgaas

This adds support for printing struct resource type and flag information. For example, "%pR" looks like "[mem 0xf5df0000-0xf5df3fff 64bit pref]", and "%pr" looks like "[mem 0xff5e2000-0xff5e2007 flags 0x201]".

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

---
 lib/vsprintf.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 47 insertions(+), 6 deletions(-)

lib/vsprintf.c +47 -6

index fcbe69d..6438cd5 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,12 +614,29 @@ static char *resource_string(char *buf, char *end, struct resource *res,
 		.precision = -1,
 		.flags = 0,
 	};
+	struct printf_spec str_spec = {
+		.field_width = -1,
+		.precision = 10,
+		.flags = LEFT,
+	};
+	struct printf_spec flag_spec = {
+		.base = 16,
+		.precision = -1,
+		.flags = SPECIAL | SMALL,
+	};
+
 	/* 32-bit res (sizeof==4): 10 chars in dec, 10 in hex ("0x" + 8)
 	 * 64-bit res (sizeof==8): 20 chars in dec, 18 in hex ("0x" + 16) */
-#define RSRC_BUF_SIZE	((2 * sizeof(resource_size_t)) + 4)
-	char sym[2*RSRC_BUF_SIZE + sizeof("[-]")];
+#define RSRC_BUF_SIZE		((2 * sizeof(resource_size_t)) + 4)
+#define FLAG_BUF_SIZE		(2 * sizeof(res->flags))
+#define DECODED_BUF_SIZE	sizeof("[mem - 64bit pref disabled]")
+#define RAW_BUF_SIZE		sizeof("[mem - flags 0x]")
+	char sym[max(2*RSRC_BUF_SIZE + DECODED_BUF_SIZE,
+		     2*RSRC_BUF_SIZE + FLAG_BUF_SIZE + RAW_BUF_SIZE)];
+
 	char *p = sym, *pend = sym + sizeof(sym);
 	int size = -1, addr = 0;
+	int decode = (fmt[0] == 'R') ? 1 : 0;
 
 	if (res->flags & IORESOURCE_IO) {
 		size = IO_RSRC_PRINTK_SIZE;

@@ -630,12 +647,35 @@ static char *resource_string(char *buf, char *end, struct resource *res,
 	}
 
 	*p++ = '[';
+	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);
+	else {
+		p = string(p, pend, "??? ", str_spec);
+		decode = 0;
+	}
 	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 (decode) {
+		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);
+	} else {
+		p = string(p, pend, " flags ", str_spec);
+		p = number(p, pend, res->flags, flag_spec);
+	}
 	*p++ = ']';
 	*p = '\0';

@@ -813,8 +853,8 @@ 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 decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref]
+ * - 'r' For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201]
  * - '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

@@ -845,7 +885,8 @@ 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);
+	case 'r':
+		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);

Joe Perches

2009-10-13 19:40 UTC #4 lore raw

Re: [PATCH v2 3/9] vsprintf: add %pR decoding and %pr for raw struct resource

Quoted 3 lines · Bjorn Helgaas

This adds support for printing struct resource type and flag information. For example, "%pR" looks like "[mem 0xf5df0000-0xf5df3fff 64bit pref]", and "%pr" looks like "[mem 0xff5e2000-0xff5e2007 flags 0x201]".

%p[A-Za-z0-9] is a relatively limited resource.

Isn't it better to avoid using multiple characters for a single type?

Bjorn Helgaas

author

2009-10-13 20:07 UTC #5 lore raw

Re: [PATCH v2 3/9] vsprintf: add %pR decoding and %pr for raw struct resource

Quoted 9 lines · Joe Perches

Quoted 3 lines · Bjorn Helgaas
This adds support for printing struct resource type and flag information. For example, "%pR" looks like "[mem 0xf5df0000-0xf5df3fff 64bit pref]", and "%pr" looks like "[mem 0xff5e2000-0xff5e2007 flags 0x201]".
%p[A-Za-z0-9] is a relatively limited resource.
Isn't it better to avoid using multiple characters for a single type?

I followed the example of F/f, S/s, M/m, etc. I could easily go back to %pR/%pRf or something, but I don't know if it's worth the trouble.

Bjorn

Joe Perches

2009-10-13 20:32 UTC #6 lore raw

Re: [PATCH v2 3/9] vsprintf: add %pR decoding and %pr for raw struct resource

Quoted 10 lines · Bjorn Helgaas

Quoted 6 lines · Joe Perches
Quoted 3 lines · Bjorn Helgaas
This adds support for printing struct resource type and flag information. For example, "%pR" looks like "[mem 0xf5df0000-0xf5df3fff 64bit pref]", and "%pr" looks like "[mem 0xff5e2000-0xff5e2007 flags 0x201]".
Isn't it better to avoid using multiple characters for a single type?
I followed the example of F/f, S/s, M/m, etc. I could easily go back to %pR/%pRf or something, but I don't know if it's worth the trouble.

I'd prefer %pR and %pRr for raw, but maybe that's just me.

cheers, Joe

Bjorn Helgaas

author

2009-10-13 19:22 UTC #7 lore raw

PATCH v2 4/9 PCI: set IORESOURCE_MEM_64 before printing resource

In reply to Bjorn Helgaas

Set the IORESOURCE_MEM_64 struct resource flag before printing the resource, so when %pR decodes the resource, we get the "64bit" text.

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

---
 drivers/pci/probe.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

drivers/pci/probe.c +4 -3

index 8105e32..f760c19 100644
@@ -225,7 +225,10 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 		if ((sizeof(resource_size_t) < 8) && (sz64 > 0x100000000ULL)) {
 			dev_err(&dev->dev, "can't handle 64-bit BAR\n");
 			goto fail;
-		} else if ((sizeof(resource_size_t) < 8) && l) {
+		}
+		
+		res->flags |= IORESOURCE_MEM_64;
+		if ((sizeof(resource_size_t) < 8) && l) {
 			/* Address above 32-bit boundary; disable the BAR */
 			pci_write_config_dword(dev, pos, 0);
 			pci_write_config_dword(dev, pos + 4, 0);

@@ -240,8 +243,6 @@ int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 					"64bit mmio pref" : "64bit mmio",
 				 res);
 		}
-
-		res->flags |= IORESOURCE_MEM_64;
 	} else {
 		sz = pci_size(l, sz, mask);

Bjorn Helgaas

author

2009-10-13 19:22 UTC #8 lore raw

PATCH v2 5/9 PCI: trivial bridge resource factorization

In reply to Bjorn Helgaas

This adds a temporary "res" so we don't have to repeat, e.g., "bus->resource[0]". The next patch adds more uses of "res".

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

---
 drivers/pci/setup-bus.c |   39 ++++++++++++++++++++++++---------------
 1 files changed, 24 insertions(+), 15 deletions(-)

drivers/pci/setup-bus.c +24 ~~-15~~

index 0959430..fd66a89 100644
@@ -71,13 +71,15 @@ static void pbus_assign_resources_sorted(const struct pci_bus *bus)
 void pci_setup_cardbus(struct pci_bus *bus)
 {
 	struct pci_dev *bridge = bus->self;
+	struct resource *res;
 	struct pci_bus_region region;
 
 	dev_info(&bridge->dev, "CardBus bridge, secondary bus %04x:%02x\n",
 		 pci_domain_nr(bus), bus->number);
 
-	pcibios_resource_to_bus(bridge, &region, bus->resource[0]);
-	if (bus->resource[0]->flags & IORESOURCE_IO) {
+	res = bus->resource[0];
+	pcibios_resource_to_bus(bridge, &region, res);
+	if (res->flags & IORESOURCE_IO) {
 		/*
 		 * The IO resource is allocated a range twice as large as it
 		 * would normally need.  This allows us to set both IO regs.

@@ -91,8 +93,9 @@ void pci_setup_cardbus(struct pci_bus *bus)
 					region.end);
 	}
 
-	pcibios_resource_to_bus(bridge, &region, bus->resource[1]);
-	if (bus->resource[1]->flags & IORESOURCE_IO) {
+	res = bus->resource[1];
+	pcibios_resource_to_bus(bridge, &region, res);
+	if (res->flags & IORESOURCE_IO) {
 		dev_info(&bridge->dev, "  IO window: %#08lx-%#08lx\n",
 		       (unsigned long)region.start,
 		       (unsigned long)region.end);

@@ -102,8 +105,9 @@ void pci_setup_cardbus(struct pci_bus *bus)
 					region.end);
 	}
 
-	pcibios_resource_to_bus(bridge, &region, bus->resource[2]);
-	if (bus->resource[2]->flags & IORESOURCE_MEM) {
+	res = bus->resource[2];
+	pcibios_resource_to_bus(bridge, &region, res);
+	if (res->flags & IORESOURCE_MEM) {
 		dev_info(&bridge->dev, "  PREFETCH window: %#08lx-%#08lx\n",
 		       (unsigned long)region.start,
 		       (unsigned long)region.end);

@@ -113,8 +117,9 @@ void pci_setup_cardbus(struct pci_bus *bus)
 					region.end);
 	}
 
-	pcibios_resource_to_bus(bridge, &region, bus->resource[3]);
-	if (bus->resource[3]->flags & IORESOURCE_MEM) {
+	res = bus->resource[3];
+	pcibios_resource_to_bus(bridge, &region, res);
+	if (res->flags & IORESOURCE_MEM) {
 		dev_info(&bridge->dev, "  MEM window: %#08lx-%#08lx\n",
 		       (unsigned long)region.start,
 		       (unsigned long)region.end);

@@ -140,6 +145,7 @@ EXPORT_SYMBOL(pci_setup_cardbus);
 static void pci_setup_bridge(struct pci_bus *bus)
 {
 	struct pci_dev *bridge = bus->self;
+	struct resource *res;
 	struct pci_bus_region region;
 	u32 l, bu, lu, io_upper16;
 	int pref_mem64;

@@ -151,8 +157,9 @@ static void pci_setup_bridge(struct pci_bus *bus)
 		 pci_domain_nr(bus), bus->number);
 
 	/* Set up the top and bottom of the PCI I/O segment for this bus. */
-	pcibios_resource_to_bus(bridge, &region, bus->resource[0]);
-	if (bus->resource[0]->flags & IORESOURCE_IO) {
+	res = bus->resource[0];
+	pcibios_resource_to_bus(bridge, &region, res);
+	if (res->flags & IORESOURCE_IO) {
 		pci_read_config_dword(bridge, PCI_IO_BASE, &l);
 		l &= 0xffff0000;
 		l |= (region.start >> 8) & 0x00f0;

@@ -178,8 +185,9 @@ static void pci_setup_bridge(struct pci_bus *bus)
 
 	/* Set up the top and bottom of the PCI Memory segment
 	   for this bus. */
-	pcibios_resource_to_bus(bridge, &region, bus->resource[1]);
-	if (bus->resource[1]->flags & IORESOURCE_MEM) {
+	res = bus->resource[1];
+	pcibios_resource_to_bus(bridge, &region, res);
+	if (res->flags & IORESOURCE_MEM) {
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
 		dev_info(&bridge->dev, "  MEM window: %#08lx-%#08lx\n",

@@ -200,12 +208,13 @@ static void pci_setup_bridge(struct pci_bus *bus)
 	/* Set up PREF base/limit. */
 	pref_mem64 = 0;
 	bu = lu = 0;
-	pcibios_resource_to_bus(bridge, &region, bus->resource[2]);
-	if (bus->resource[2]->flags & IORESOURCE_PREFETCH) {
+	res = bus->resource[2];
+	pcibios_resource_to_bus(bridge, &region, res);
+	if (res->flags & IORESOURCE_PREFETCH) {
 		int width = 8;
 		l = (region.start >> 16) & 0xfff0;
 		l |= region.end & 0xfff00000;
-		if (bus->resource[2]->flags & IORESOURCE_MEM_64) {
+		if (res->flags & IORESOURCE_MEM_64) {
 			pref_mem64 = 1;
 			bu = upper_32_bits(region.start);
 			lu = upper_32_bits(region.end);

Bjorn Helgaas

author

2009-10-13 19:22 UTC #9 lore raw

PATCH v2 6/9 PCI: print resources consistently with %pR

In reply to Bjorn Helgaas

This uses %pR 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     |   22 ++++++----------------
 drivers/pci/quirks.c    |    2 +-
 drivers/pci/setup-bus.c |   43 ++++++++++++-------------------------------
 drivers/pci/setup-res.c |   20 +++++++++-----------
 5 files changed, 29 insertions(+), 62 deletions(-)

drivers/pci/pci.c +1 -3

index 3835871..832243f 100644
@@ -1669,9 +1669,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 %pR\n", bar,
 		 &pdev->resource[bar]);
 	return -EBUSY;
 }

drivers/pci/probe.c +6 ~~-16~~

index f760c19..6f0a6fb 100644
@@ -237,11 +237,8 @@ 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: %pR\n",
+				   pos, res);
 		}
 	} else {
 		sz = pci_size(l, sz, mask);

@@ -252,11 +249,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: %pR\n", pos, res);
 	}
 
  out:

@@ -324,7 +317,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 %pR\n", res);
 	}
 
 	res = child->resource[1];

@@ -336,8 +329,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 %pR\n", res);
 	}
 
 	res = child->resource[2];

@@ -376,9 +368,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 %pR\n", res);
 	}
 }

drivers/pci/quirks.c +1 -1

index efa6534..d996a2f 100644
@@ -357,7 +357,7 @@ static void __devinit quirk_io_region(struct pci_dev *dev, unsigned region,
 		pcibios_bus_to_resource(dev, res, &bus_region);
 
 		pci_claim_resource(dev, nr);
-		dev_info(&dev->dev, "quirk: region %04x-%04x claimed by %s\n", region, region + size - 1, name);
+		dev_info(&dev->dev, "quirk: %pR claimed by %s\n", res, name);
 	}
 }

drivers/pci/setup-bus.c +12 ~~-31~~

115 lines not on this page · raw message

drivers/pci/setup-res.c +9 ~~-11~~

index c54526b..a00ae3b 100644
@@ -91,9 +91,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",
-		resno, (unsigned long long)region.start,
-		(unsigned long long)region.end, res->flags);
+	dev_dbg(&dev->dev, "BAR %d: moved to %pR (PCI address [%#llx-%#llx]\n",
+		resno, res, (unsigned long long)region.start,
+		(unsigned long long)region.end);
 }
 
 int pci_claim_resource(struct pci_dev *dev, int resource)

@@ -181,9 +181,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 %pR "
+			 "(bogus alignment)\n", resno, res);
 		return -EINVAL;
 	}

@@ -199,8 +198,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 %pR\n",
+			 resno, res);
 
 	return ret;
 }

@@ -225,9 +224,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: %pR has bogus alignment\n",
+				 i, r);
 			continue;
 		}
 		for (list = head; ; list = list->next) {

Bjorn Helgaas

author

2009-10-13 19:22 UTC #10 lore raw

PATCH v2 7/9 x86/PCI: print resources consistently with %pR

In reply to Bjorn Helgaas

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

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

---
 arch/x86/pci/acpi.c |   15 ++++++++++++---
 arch/x86/pci/i386.c |   13 ++++++-------
 2 files changed, 18 insertions(+), 10 deletions(-)

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

index 1014eb4..6bf8091 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,19 @@ 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 host bridge window %pR\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 %pR "
+				 "(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 %pR\n", res);
 	}
 	return AE_OK;
 }

@@ -124,6 +132,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 +6 -7

index b22d13b..f5fac0f 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 %pR\n", idx, r);
 					/*
 					 * Something is wrong with the region.
 					 * Invalidate the resource to prevent

@@ -164,12 +164,11 @@ 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,
+					"BAR %d: claiming %pr (d=%d, p=%d)\n",
+					idx, 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 claim %pR\n", idx, r);
 					/* We'll assign a new address later */
 					r->end -= r->start;
 					r->start = 0;

@@ -182,7 +181,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 %pR\n", r);
 				r->flags &= ~IORESOURCE_ROM_ENABLE;
 				pci_read_config_dword(dev,
 						dev->rom_base_reg, &reg);

Bjorn Helgaas

author

2009-10-13 19:23 UTC #11 lore raw

PATCH v2 8/9 ia64/PCI: print resources consistently with %pR

In reply to Bjorn Helgaas

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

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

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

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

index 7de76dd..4704f47 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,20 @@ 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 host bridge window %pR\n",
+			&window->resource);
+	} else {
+		if (offset)
+			dev_info(&info->bridge->dev, "host bridge window %pR "
+				 "(PCI address [%#llx-%#llx])\n",
+				 &window->resource,
+				 window->resource.start - offset,
+				 window->resource.end - offset);
+		else
+			dev_info(&info->bridge->dev,
+				 "host bridge window %pR\n",
+				 &window->resource);
 	}
 
 	return AE_OK;

@@ -314,8 +326,9 @@ 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 host bridge window %pR (no space)\n",
+				 res);
 			continue;
 		}
 		bus->resource[j++] = res;

@@ -359,6 +372,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-13 19:23 UTC #12 lore raw

PATCH v2 9/9 PNP: print resources consistently with %pR

In reply to Bjorn Helgaas

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

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

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

drivers/pnp/quirks.c +4 -9

index 8473fe5..dfbd5a6 100644
@@ -285,15 +285,10 @@ 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,
+					 "disabling %pR because it overlaps "
+					 "%s BAR %d %pR\n", res,
+					 pci_name(pdev), i, &pdev->resource[i]);
 				res->flags |= IORESOURCE_DISABLED;
 			}
 		}

drivers/pnp/resource.c +4 -6

index ba97654..64d0596 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 %pr\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 %pr\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 %pr\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 %pr\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..49c1720 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, "%pR %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);
 	}
 }

Yinghai Lu

2009-10-14 06:54 UTC #13 lore raw

In reply to Bjorn Helgaas

Quoted 5 lines · Bjorn Helgaas

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"):

Quoted 14 lines

-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]

can you keep "io port" and "mmio" ?
so we can use
grep "io port" dmesg.txt or grep "mmio" dmesg.txt

also put "io" and "mmio" "pref' in the [ ], looks strange. [0xf5ff0000-0xf5ff0fff] is correct range expression.

YH
--
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

Bjorn Helgaas

author

2009-10-14 15:52 UTC #14 lore raw

Quoted 21 lines · Yinghai Lu

Quoted 5 lines · Bjorn Helgaas
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"):
Quoted 14 lines
-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]

Quoted 3 lines

can you keep "io port" and "mmio" ?
so we can use
grep "io port" dmesg.txt or grep "mmio" dmesg.txt

I don't think that's necessary. If you want to see only resource stuff, these greps will work perfectly:

  # dmesg | grep "\[io "
  # dmesg | grep "\[mem "

(This is much better than what's in the current tree, where resources are labelled with a hodge-podge of mem, mmio, MEM, io, I/O, io port, IO, or even nothing at all, so you really can't do the grep at all.)

Quoted 2 lines

also put "io" and "mmio" "pref' in the [ ], looks strange. [0xf5ff0000-0xf5ff0fff] is correct range expression.

I could be convinced otherwise, but right now, I don't see a correctness issue here -- it's just a matter of what the most convenient format for human readers is. And I personally like the fact that everything inside the brackets is an attribute of the struct resource.

If these are the alternatives:

bridge window [io 0x0000-0x0cff]
bridge window io [0x0000-0x0cff]

one nice thing about the first is that the brackets give a clue that you should grep for "bridge window", not "bridge window io", to find the source of the message.

Bjorn

Bjorn Helgaas

author

2009-10-22 22:53 UTC #15 lore raw

In reply to Bjorn Helgaas

I'm not planning another version of this series because I don't think anybody's raised substantive issues.

The only thing I see as a potential issue is the increased stack usage:
in resource_string(), the buffer grows from 40 to 73 bytes (64-bit) or 24 to 52 bytes (32-bit). I could imagine using a stack buffer and a mutex for this, but I'm not sure it's worth the code complexity yet.

Bjorn