2009-10-13 19:23:47

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 0/9] 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(-)

--
Bjorn


2009-10-13 19:22:41

by Bjorn Helgaas

[permalink] [raw]
Subject: [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(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 33bed5e..7830576 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -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,

2009-10-13 19:24:01

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 2/9] vsprintf: add %pR support for IRQ and DMA resources

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(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 7830576..fcbe69d 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -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);
}

2009-10-13 19:22:53

by Bjorn Helgaas

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

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(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index fcbe69d..6438cd5 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -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);

2009-10-13 19:24:36

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 4/9] PCI: set IORESOURCE_MEM_64 before printing resource

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(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8105e32..f760c19 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -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);

2009-10-13 19:23:03

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 5/9] PCI: trivial bridge resource factorization

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(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 0959430..fd66a89 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -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);

2009-10-13 19:23:07

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 6/9] PCI: print resources consistently with %pR

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(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 3835871..832243f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -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;
}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index f760c19..6f0a6fb 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -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);
}
}

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index efa6534..d996a2f 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -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);
}
}

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index fd66a89..63544a9 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -84,9 +84,7 @@ void pci_setup_cardbus(struct pci_bus *bus)
* The IO resource is allocated a range twice as large as it
* would normally need. This allows us to set both IO regs.
*/
- dev_info(&bridge->dev, " IO window: %#08lx-%#08lx\n",
- (unsigned long)region.start,
- (unsigned long)region.end);
+ dev_info(&bridge->dev, " bridge window %pR\n", res);
pci_write_config_dword(bridge, PCI_CB_IO_BASE_0,
region.start);
pci_write_config_dword(bridge, PCI_CB_IO_LIMIT_0,
@@ -96,9 +94,7 @@ void pci_setup_cardbus(struct pci_bus *bus)
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);
+ dev_info(&bridge->dev, " bridge window %pR\n", res);
pci_write_config_dword(bridge, PCI_CB_IO_BASE_1,
region.start);
pci_write_config_dword(bridge, PCI_CB_IO_LIMIT_1,
@@ -108,9 +104,7 @@ void pci_setup_cardbus(struct pci_bus *bus)
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);
+ dev_info(&bridge->dev, " bridge window %pR\n", res);
pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_0,
region.start);
pci_write_config_dword(bridge, PCI_CB_MEMORY_LIMIT_0,
@@ -120,9 +114,7 @@ void pci_setup_cardbus(struct pci_bus *bus)
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);
+ dev_info(&bridge->dev, " bridge window %pR\n", res);
pci_write_config_dword(bridge, PCI_CB_MEMORY_BASE_1,
region.start);
pci_write_config_dword(bridge, PCI_CB_MEMORY_LIMIT_1,
@@ -166,15 +158,13 @@ static void pci_setup_bridge(struct pci_bus *bus)
l |= region.end & 0xf000;
/* Set up upper 16 bits of I/O base/limit. */
io_upper16 = (region.end & 0xffff0000) | (region.start >> 16);
- dev_info(&bridge->dev, " IO window: %#04lx-%#04lx\n",
- (unsigned long)region.start,
- (unsigned long)region.end);
+ dev_info(&bridge->dev, " bridge window %pR\n", res);
}
else {
/* Clear upper 16 bits of I/O base/limit. */
io_upper16 = 0;
l = 0x00f0;
- dev_info(&bridge->dev, " IO window: disabled\n");
+ dev_info(&bridge->dev, " bridge window [io disabled]\n");
}
/* Temporarily disable the I/O range before updating PCI_IO_BASE. */
pci_write_config_dword(bridge, PCI_IO_BASE_UPPER16, 0x0000ffff);
@@ -190,13 +180,11 @@ static void pci_setup_bridge(struct pci_bus *bus)
if (res->flags & IORESOURCE_MEM) {
l = (region.start >> 16) & 0xfff0;
l |= region.end & 0xfff00000;
- dev_info(&bridge->dev, " MEM window: %#08lx-%#08lx\n",
- (unsigned long)region.start,
- (unsigned long)region.end);
+ dev_info(&bridge->dev, " bridge window %pR\n", res);
}
else {
l = 0x0000fff0;
- dev_info(&bridge->dev, " MEM window: disabled\n");
+ dev_info(&bridge->dev, " bridge window [mem disabled]\n");
}
pci_write_config_dword(bridge, PCI_MEMORY_BASE, l);

@@ -211,22 +199,18 @@ static void pci_setup_bridge(struct pci_bus *bus)
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 (res->flags & IORESOURCE_MEM_64) {
pref_mem64 = 1;
bu = upper_32_bits(region.start);
lu = upper_32_bits(region.end);
- width = 16;
}
- dev_info(&bridge->dev, " PREFETCH window: %#0*llx-%#0*llx\n",
- width, (unsigned long long)region.start,
- width, (unsigned long long)region.end);
+ dev_info(&bridge->dev, " bridge window %pR\n", res);
}
else {
l = 0x0000fff0;
- dev_info(&bridge->dev, " PREFETCH window: disabled\n");
+ dev_info(&bridge->dev, " bridge window [mem pref disabled]\n");
}
pci_write_config_dword(bridge, PCI_PREF_MEMORY_BASE, l);

@@ -408,7 +392,7 @@ 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: "
+ dev_warn(&dev->dev, "BAR %d: bad alignment %llx: "
"%pR\n", i, (unsigned long long)align, r);
r->flags = 0;
continue;
@@ -600,10 +584,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 %pR\n", i, res);
}
}

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index c54526b..a00ae3b 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -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) {

2009-10-13 19:23:12

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 7/9] x86/PCI: print resources consistently with %pR

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(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 1014eb4..6bf8091 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -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,
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index b22d13b..f5fac0f 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -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);

2009-10-13 19:23:21

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 8/9] ia64/PCI: print resources consistently with %pR

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(-)

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 7de76dd..4704f47 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -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,

2009-10-13 19:23:40

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v2 9/9] PNP: print resources consistently with %pR

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(-)

diff --git a/drivers/pnp/quirks.c b/drivers/pnp/quirks.c
index 8473fe5..dfbd5a6 100644
--- a/drivers/pnp/quirks.c
+++ b/drivers/pnp/quirks.c
@@ -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;
}
}
diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c
index ba97654..64d0596 100644
--- a/drivers/pnp/resource.c
+++ b/drivers/pnp/resource.c
@@ -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;
}

diff --git a/drivers/pnp/support.c b/drivers/pnp/support.c
index 63087d5..9585c1c 100644
--- a/drivers/pnp/support.c
+++ b/drivers/pnp/support.c
@@ -75,47 +75,14 @@ char *pnp_resource_type_name(struct resource *res)

void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc)
{
- char buf[128];
- int len;
struct pnp_resource *pnp_res;
- struct resource *res;

- if (list_empty(&dev->resources)) {
+ if (list_empty(&dev->resources))
pnp_dbg(&dev->dev, "%s: no current resources\n", desc);
- return;
- }
-
- pnp_dbg(&dev->dev, "%s: current resources:\n", desc);
- list_for_each_entry(pnp_res, &dev->resources, list) {
- res = &pnp_res->res;
- len = 0;
-
- len += scnprintf(buf + len, sizeof(buf) - len, " %-3s ",
- pnp_resource_type_name(res));
-
- if (res->flags & IORESOURCE_DISABLED) {
- pnp_dbg(&dev->dev, "%sdisabled\n", buf);
- continue;
- }
-
- switch (pnp_resource_type(res)) {
- case IORESOURCE_IO:
- case IORESOURCE_MEM:
- len += scnprintf(buf + len, sizeof(buf) - len,
- "%#llx-%#llx flags %#lx",
- (unsigned long long) res->start,
- (unsigned long long) res->end,
- res->flags);
- break;
- case IORESOURCE_IRQ:
- case IORESOURCE_DMA:
- len += scnprintf(buf + len, sizeof(buf) - len,
- "%lld flags %#lx",
- (unsigned long long) res->start,
- res->flags);
- break;
- }
- pnp_dbg(&dev->dev, "%s\n", buf);
+ else {
+ pnp_dbg(&dev->dev, "%s: current resources:\n", desc);
+ list_for_each_entry(pnp_res, &dev->resources, list)
+ pnp_dbg(&dev->dev, "%pr\n", &pnp_res->res);
}
}

diff --git a/drivers/pnp/system.c b/drivers/pnp/system.c
index 59b9092..49c1720 100644
--- a/drivers/pnp/system.c
+++ b/drivers/pnp/system.c
@@ -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);
}
}

2009-10-13 19:40:47

by Joe Perches

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

On Tue, 2009-10-13 at 13:22 -0600, Bjorn Helgaas wrote:
> 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?

2009-10-13 20:07:48

by Bjorn Helgaas

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

On Tuesday 13 October 2009 01:40:08 pm Joe Perches wrote:
> On Tue, 2009-10-13 at 13:22 -0600, Bjorn Helgaas wrote:
> > 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

2009-10-13 20:32:40

by Joe Perches

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

On Tue, 2009-10-13 at 14:07 -0600, Bjorn Helgaas wrote:
> On Tuesday 13 October 2009 01:40:08 pm Joe Perches wrote:
> > On Tue, 2009-10-13 at 13:22 -0600, Bjorn Helgaas wrote:
> > > 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

2009-10-14 06:54:39

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH v2 0/9] PCI, PNP: print resources consistently

On Tue, Oct 13, 2009 at 12:21 PM, Bjorn Helgaas <[email protected]> wrote:
> 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]

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

2009-10-14 15:52:46

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v2 0/9] PCI, PNP: print resources consistently

On Wednesday 14 October 2009 12:47:33 am Yinghai Lu wrote:
> On Tue, Oct 13, 2009 at 12:21 PM, Bjorn Helgaas <[email protected]> wrote:
> > 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]
>
> 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.)

> 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

2009-10-22 22:53:52

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v2 0/9] PCI, PNP: print resources consistently

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