2009-10-06 21:34:04

by Bjorn Helgaas

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

--
Bjorn


2009-10-06 21:34:11

by Bjorn Helgaas

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

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-06 21:36:34

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 2/7] 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 | 31 +++++++++++++++++++++----------
1 files changed, 21 insertions(+), 10 deletions(-)

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

2009-10-06 21:34:20

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 3/7] vsprintf: add %pRt, %pRf to print struct resource details

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

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1b60aed..a6e1951 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,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);

2009-10-06 21:34:26

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 4/7] PCI: print resources consistently with %pRt

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

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6edecff..d760776 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -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;
}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ca08416..94b2b9c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -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);
}
}

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

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

2009-10-06 21:34:31

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 5/7] x86/PCI: print resources consistently with %pRt

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

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index 2297280..4d4f1df 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,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,
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index b22d13b..a70a85d 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 %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);

2009-10-06 21:34:35

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 6/7] ia64/PCI: print resources consistently with %pRt

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

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 7de76dd..0f90259 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,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,

2009-10-06 21:34:40

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH 7/7] PNP: print resources consistently with %pRt

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

diff --git a/drivers/pnp/quirks.c b/drivers/pnp/quirks.c
index 8473fe5..3a2031b 100644
--- a/drivers/pnp/quirks.c
+++ b/drivers/pnp/quirks.c
@@ -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;
}
}
diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c
index ba97654..18557d7 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 %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;
}

diff --git a/drivers/pnp/support.c b/drivers/pnp/support.c
index 63087d5..1f8a33b 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, "%pRf\n", &pnp_res->res);
}
}

diff --git a/drivers/pnp/system.c b/drivers/pnp/system.c
index 59b9092..242d3a8 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, "%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);
}
}

2009-10-07 18:43:48

by Yinghai Lu

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

On Tue, Oct 6, 2009 at 2:33 PM, Bjorn Helgaas <[email protected]> wrote:
> 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(-)
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index 2297280..4d4f1df 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,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 ?

YH

2009-10-07 19:15:56

by Bjorn Helgaas

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

On Wednesday 07 October 2009 12:43:09 pm Yinghai Lu wrote:
> On Tue, Oct 6, 2009 at 2:33 PM, Bjorn Helgaas <[email protected]> wrote:
> > 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(-)
> >
> > diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> > index 2297280..4d4f1df 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,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 ?

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

2009-10-09 21:14:59

by Len Brown

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


> 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

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

2009-10-10 15:40:54

by Bjorn Helgaas

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

On Fri, 2009-10-09 at 17:14 -0400, Len Brown wrote:
> > 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
>
> 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.

> 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

2009-10-26 20:40:46

by Jesse Barnes

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

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.

--
Jesse Barnes, Intel Open Source Technology Center