[PATCH 2/2] pci: Use new %pR to print resource ranges - Kernel

This is a discussion on [PATCH 2/2] pci: Use new %pR to print resource ranges - Kernel ; This converts things in drivers/pci to use % pR to printout the content of a struct resource instead of hand-casted %llx or other variants. Signed-off-by: Benjamin Herrenschmidt --- I didn't go down the subdirs in there and I might have ...

+ Reply to Thread
Results 1 to 17 of 17

Thread: [PATCH 2/2] pci: Use new %pR to print resource ranges

  1. [PATCH 2/2] pci: Use new %pR to print resource ranges

    This converts things in drivers/pci to use %pR to printout the
    content of a struct resource instead of hand-casted %llx or
    other variants.

    Signed-off-by: Benjamin Herrenschmidt
    ---

    I didn't go down the subdirs in there and I might have missed one
    or two but the bulk is converted.

    drivers/pci/pci.c | 5 ++---
    drivers/pci/probe.c | 28 +++++++++++++---------------
    drivers/pci/setup-bus.c | 13 ++++---------
    drivers/pci/setup-res.c | 40 +++++++++++++---------------------------
    4 files changed, 32 insertions(+), 54 deletions(-)

    --- linux-work.orig/drivers/pci/probe.c 2008-10-17 16:11:00.000000000 +1100
    +++ linux-work/drivers/pci/probe.c 2008-10-20 14:44:26.000000000 +1100
    @@ -238,9 +238,8 @@ static int __pci_read_base(struct pci_de
    } else {
    res->start = l64;
    res->end = l64 + sz64;
    - printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
    - pci_name(dev), pos, (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: %pR\n",
    + pci_name(dev), pos, res);
    }
    } else {
    sz = pci_size(l, sz, mask);
    @@ -250,9 +249,10 @@ static int __pci_read_base(struct pci_de

    res->start = l;
    res->end = l + sz;
    - printk(KERN_DEBUG "PCI: %s reg %x %s: [%llx, %llx]\n", pci_name(dev),
    - pos, (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    - (unsigned long long)res->start, (unsigned long long)res->end);
    + printk(KERN_DEBUG "PCI: %s reg %x %s: %pR\n",
    + pci_name(dev), pos,
    + (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    + res);
    }

    out:
    @@ -323,9 +323,8 @@ void __devinit pci_read_bridge_bases(str
    res->start = base;
    if (!res->end)
    res->end = limit + 0xfff;
    - printk(KERN_DEBUG "PCI: bridge %s io port: [%llx, %llx]\n",
    - pci_name(dev), (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s io port: %pR\n",
    + pci_name(dev), res);
    }

    res = child->resource[1];
    @@ -337,9 +336,8 @@ void __devinit pci_read_bridge_bases(str
    res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
    res->start = base;
    res->end = limit + 0xfffff;
    - printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: [%llx, %llx]\n",
    - pci_name(dev), (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: %pR\n",
    + pci_name(dev), res);
    }

    res = child->resource[2];
    @@ -375,9 +373,9 @@ void __devinit pci_read_bridge_bases(str
    res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
    res->start = base;
    res->end = limit + 0xfffff;
    - printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: [%llx, %llx]\n",
    - pci_name(dev), (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64" : "32",
    - (unsigned long long) res->start, (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: %pR\n",
    + pci_name(dev),
    + (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64":"32", res);
    }
    }

    Index: linux-work/drivers/pci/setup-bus.c
    ================================================== =================
    --- linux-work.orig/drivers/pci/setup-bus.c 2008-10-17 16:11:00.000000000 +1100
    +++ linux-work/drivers/pci/setup-bus.c 2008-10-20 14:44:26.000000000 +1100
    @@ -356,10 +356,7 @@ static int pbus_size_mem(struct pci_bus
    order = __ffs(align) - 20;
    if (order > 11) {
    dev_warn(&dev->dev, "BAR %d bad alignment %llx: "
    - "%#016llx-%#016llx\n", i,
    - (unsigned long long)align,
    - (unsigned long long)r->start,
    - (unsigned long long)r->end);
    + "%pR\n", i, (unsigned long long)align, r);
    r->flags = 0;
    continue;
    }
    @@ -539,11 +536,9 @@ static void pci_bus_dump_res(struct pci_
    if (!res)
    continue;

    - printk(KERN_INFO "bus: %02x index %x %s: [%llx, %llx]\n",
    - bus->number, i,
    - (res->flags & IORESOURCE_IO) ? "io port" : "mmio",
    - (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_INFO "bus: %02x index %x %s: %pR\n",
    + bus->number, i,
    + (res->flags & IORESOURCE_IO) ? "io port" : "mmio", res);
    }
    }

    Index: linux-work/drivers/pci/setup-res.c
    ================================================== =================
    --- linux-work.orig/drivers/pci/setup-res.c 2008-10-17 16:11:00.000000000 +1100
    +++ linux-work/drivers/pci/setup-res.c 2008-10-20 14:44:26.000000000 +1100
    @@ -49,10 +49,8 @@ void pci_update_resource(struct pci_dev

    pcibios_resource_to_bus(dev, &region, res);

    - dev_dbg(&dev->dev, "BAR %d: got res [%#llx-%#llx] bus [%#llx-%#llx] "
    - "flags %#lx\n", resno,
    - (unsigned long long)res->start,
    - (unsigned long long)res->end,
    + 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);
    @@ -114,13 +112,11 @@ int pci_claim_resource(struct pci_dev *d
    err = insert_resource(root, res);

    if (err) {
    - dev_err(&dev->dev, "BAR %d: %s of %s [%#llx-%#llx]\n",
    + dev_err(&dev->dev, "BAR %d: %s of %s %pR\n",
    resource,
    root ? "address space collision on" :
    "no parent found for",
    - dtype,
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dtype, res);
    }

    return err;
    @@ -139,9 +135,8 @@ int pci_assign_resource(struct pci_dev *
    align = resource_alignment(res);
    if (!align) {
    dev_err(&dev->dev, "BAR %d: can't allocate resource (bogus "
    - "alignment) [%#llx-%#llx] flags %#lx\n",
    - resno, (unsigned long long)res->start,
    - (unsigned long long)res->end, res->flags);
    + "alignment) %pR flags %#lx\n",
    + resno, res, res->flags);
    return -EINVAL;
    }

    @@ -162,11 +157,8 @@ int pci_assign_resource(struct pci_dev *
    }

    if (ret) {
    - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    - "[%#llx-%#llx]\n", resno,
    - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    } else {
    res->flags &= ~IORESOURCE_STARTALIGN;
    if (resno < PCI_BRIDGE_RESOURCES)
    @@ -202,11 +194,8 @@ int pci_assign_resource_fixed(struct pci
    }

    if (ret) {
    - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    - "[%#llx-%#llx\n]", resno,
    - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    } else if (resno < PCI_BRIDGE_RESOURCES) {
    pci_update_resource(dev, res, resno);
    }
    @@ -237,9 +226,8 @@ void pdev_sort_resources(struct pci_dev
    r_align = resource_alignment(r);
    if (!r_align) {
    dev_warn(&dev->dev, "BAR %d: bogus alignment "
    - "[%#llx-%#llx] flags %#lx\n",
    - i, (unsigned long long)r->start,
    - (unsigned long long)r->end, r->flags);
    + "%pR flags %#lx\n",
    + i, r, r->flags);
    continue;
    }
    for (list = head; ; list = list->next) {
    @@ -287,9 +275,7 @@ int pci_enable_resources(struct pci_dev

    if (!r->parent) {
    dev_err(&dev->dev, "device not available because of "
    - "BAR %d [%#llx-%#llx] collisions\n", i,
    - (unsigned long long) r->start,
    - (unsigned long long) r->end);
    + "BAR %d %pR collisions\n", i, r);
    return -EINVAL;
    }

    Index: linux-work/drivers/pci/pci.c
    ================================================== =================
    --- linux-work.orig/drivers/pci/pci.c 2008-10-17 16:11:00.000000000 +1100
    +++ linux-work/drivers/pci/pci.c 2008-10-20 14:44:26.000000000 +1100
    @@ -1358,11 +1358,10 @@ int pci_request_region(struct pci_dev *p
    return 0;

    err_out:
    - dev_warn(&pdev->dev, "BAR %d: can't reserve %s region [%#llx-%#llx]\n",
    + dev_warn(&pdev->dev, "BAR %d: can't reserve %s region %pR\n",
    bar,
    pci_resource_flags(pdev, bar) & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)pci_resource_start(pdev, bar),
    - (unsigned long long)pci_resource_end(pdev, bar));
    + &pdev->resource[bar]);
    return -EBUSY;
    }

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [PATCH] x86, ioremap: use %pR in printk


    * Benjamin Herrenschmidt wrote:

    > This converts things in drivers/pci to use %pR to printout the
    > content of a struct resource instead of hand-casted %llx or
    > other variants.
    >
    > Signed-off-by: Benjamin Herrenschmidt


    cool!

    Acked-by: Ingo Molnar

    there's also two places in arch/x86/mm/ioremap.c that could use this
    straight away - see the (untested) patch below.

    Ingo

    --------------------->
    From 3b4f8fd4ff54a2da035e972a610b8d5ac8d8eabb Mon Sep 17 00:00:00 2001
    From: Ingo Molnar
    Date: Mon, 20 Oct 2008 09:08:57 +0200
    Subject: [PATCH] x86, ioremap: use %pR in printk

    use the new %pR IO resource pointer/address/size printk type conversion
    specifier.

    Signed-off-by: Ingo Molnar
    ---
    arch/x86/mm/ioremap.c | 10 +++++-----
    1 files changed, 5 insertions(+), 5 deletions(-)

    diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
    index ae71e11..640c653 100644
    --- a/arch/x86/mm/ioremap.c
    +++ b/arch/x86/mm/ioremap.c
    @@ -207,8 +207,8 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    return NULL;

    if (!phys_addr_valid(phys_addr)) {
    - printk(KERN_WARNING "ioremap: invalid physical address %llx\n",
    - (unsigned long long)phys_addr);
    + printk(KERN_WARNING "ioremap: invalid physical address %pR\n",
    + phys_addr);
    WARN_ON_ONCE(1);
    return NULL;
    }
    @@ -267,9 +267,9 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    (prot_val == _PAGE_CACHE_WC &&
    new_prot_val == _PAGE_CACHE_WB)) {
    pr_debug(
    - "ioremap error for 0x%llx-0x%llx, requested 0x%lx, got 0x%lx\n",
    - (unsigned long long)phys_addr,
    - (unsigned long long)(phys_addr + size),
    + "ioremap error for %pR-%pR, requested 0x%lx, got 0x%lx\n",
    + phys_addr,
    + phys_addr + size,
    prot_val, new_prot_val);
    free_memtype(phys_addr, phys_addr + size);
    return NULL;
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] x86, ioremap: use %pR in printk

    On Mon, 2008-10-20 at 09:12 +0200, Ingo Molnar wrote:
    > * Benjamin Herrenschmidt wrote:
    >
    > > This converts things in drivers/pci to use %pR to printout the
    > > content of a struct resource instead of hand-casted %llx or
    > > other variants.
    > >
    > > Signed-off-by: Benjamin Herrenschmidt

    >
    > cool!
    >
    > Acked-by: Ingo Molnar
    >
    > there's also two places in arch/x86/mm/ioremap.c that could use this
    > straight away - see the (untested) patch below.


    No, you don't pass it a phys_addr_t or a resource_size_t, you pass it a
    struct resource *

    Now, I would have liked to have a way to print a resource_size_t (or
    phys_addr_t) but that's harder because we need pointers for the magic
    typechecking to work (among others).

    Maybe we could create a special format for a pointer-to-resource_size_t
    and pass &foo on callers but that's fishy...

    Cheers,
    Ben.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH] x86, ioremap: use %pR in printk


    * Benjamin Herrenschmidt wrote:

    > On Mon, 2008-10-20 at 09:12 +0200, Ingo Molnar wrote:
    > > * Benjamin Herrenschmidt wrote:
    > >
    > > > This converts things in drivers/pci to use %pR to printout the
    > > > content of a struct resource instead of hand-casted %llx or
    > > > other variants.
    > > >
    > > > Signed-off-by: Benjamin Herrenschmidt

    > >
    > > cool!
    > >
    > > Acked-by: Ingo Molnar
    > >
    > > there's also two places in arch/x86/mm/ioremap.c that could use this
    > > straight away - see the (untested) patch below.

    >
    > No, you don't pass it a phys_addr_t or a resource_size_t, you pass it a
    > struct resource *


    oops ... i only looked at:

    + char sym[4*sizeof(resource_size_t) + 8];

    and assumed that it was a resource_size_t printout format :-/

    while printing out ranges is useful too, it's by far not the only source
    of ugliness all around resource_size_t.

    > Now, I would have liked to have a way to print a resource_size_t (or
    > phys_addr_t) but that's harder because we need pointers for the magic
    > typechecking to work (among others).
    >
    > Maybe we could create a special format for a
    > pointer-to-resource_size_t and pass &foo on callers but that's
    > fishy...


    would be very nice to just have support for all the native types that
    the kernel uses. %pR is still a convenient shortcut.

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] x86, ioremap: use %pR in printk


    * Ingo Molnar wrote:

    > > No, you don't pass it a phys_addr_t or a resource_size_t, you pass
    > > it a struct resource *

    >
    > oops ... i only looked at:
    >
    > + char sym[4*sizeof(resource_size_t) + 8];
    >
    > and assumed that it was a resource_size_t printout format :-/
    >
    > while printing out ranges is useful too, it's by far not the only
    > source of ugliness all around resource_size_t.


    so how about something like the two patches below (ontop of Linus's
    patch): the first patch introduces a "small" resource pointer printout
    format: %pr - the little brother of %pR.

    The output format is [0x00001234] - minimum width is 8.

    The second patch takes advantage of it in ioremap.c.

    Not tested yet but should work. One thing that would be nice is to
    extend it to phys_addr_t as well. Right now there's no guarantee that
    sizeof(resource_size_t) == sizeof(phys_addr_t).

    What do you think? Passing it by reference makes it somewhat ugly [and
    restricts it to lvalues], but that's the safest we can get at the moment
    i guess, as it guarantees followup vararg parameter correctness, because
    gcc does not check these extensions.

    Worst-case we get gibberish output if it's used incorrectly - but we
    dont get a kernel security hole if a %s follows it in the format string
    and an attacker can control some of the parameters, etc.

    Ingo

    ---------------->
    From bec931cb306f0901b9e2017c845d5ad6016574e4 Mon Sep 17 00:00:00 2001
    From: Ingo Molnar
    Date: Mon, 20 Oct 2008 12:39:17 +0200
    Subject: [PATCH] vsprintf: implement %pr to print resource_size_t

    Signed-off-by: Ingo Molnar
    ---
    lib/vsprintf.c | 20 ++++++++++++++++++++
    1 files changed, 20 insertions(+), 0 deletions(-)

    diff --git a/lib/vsprintf.c b/lib/vsprintf.c
    index a013bbc..ddcaa6e 100644
    --- a/lib/vsprintf.c
    +++ b/lib/vsprintf.c
    @@ -581,6 +581,21 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
    return string(buf, end, sym, field_width, precision, flags);
    }

    +static char *resource_size_string(char *buf, char *end, resource_size_t *val, int field_width, int precision, int flags)
    +{
    + /* room for the actual number, the one "0x", [, ] and the final zero */
    + char sym[2*sizeof(resource_size_t) + 5];
    + char *p = sym, *pend = sym + sizeof(sym);
    + int size = 8;
    +
    + *p++ = '[';
    + p = number(p, pend, *val, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p++ = ']';
    + *p = 0;
    +
    + return string(buf, end, sym, field_width, precision, flags);
    +}
    +
    /*
    * Show a '%p' thing. A kernel extension is that the '%p' is followed
    * by an extra set of alphanumeric characters that are extended format
    @@ -592,6 +607,8 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
    * - 'S' For symbolic direct pointers
    * - 'R' For a struct resource pointer, it prints the range of
    * addresses (not the name nor the flags)
    + * - 'r' For a resource_size_t pointer, it prints the resource
    + * addresses (in %pR format)
    *
    * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
    * function pointers are really function descriptors, which contain a
    @@ -607,6 +624,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    return symbol_string(buf, end, ptr, field_width, precision, flags);
    case 'R':
    return resource_string(buf, end, ptr, field_width, precision, flags);
    + case 'r':
    + return resource_size_string(buf, end, ptr, field_width, precision, flags);
    }
    flags |= SMALL;
    if (field_width == -1) {
    @@ -627,6 +646,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    * %pS output the name of a text symbol
    * %pF output the name of a function pointer
    * %pR output the address range in a struct resource
    + * %pr output the address in a pointer to a resource_size_t type
    *
    * The return value is the number of characters which would
    * be generated for the given input, excluding the trailing

    From cfbd2eec241a53bc5c1b95bb68f269036e3e92a4 Mon Sep 17 00:00:00 2001
    From: Ingo Molnar
    Date: Mon, 20 Oct 2008 09:08:57 +0200
    Subject: [PATCH] x86, ioremap: use %pr in printk

    use the new %pr IO resource pointer/address/size printk type conversion
    specifier.

    Signed-off-by: Ingo Molnar
    ---
    arch/x86/mm/ioremap.c | 10 +++++-----
    1 files changed, 5 insertions(+), 5 deletions(-)

    diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
    index ae71e11..4725c7a 100644
    --- a/arch/x86/mm/ioremap.c
    +++ b/arch/x86/mm/ioremap.c
    @@ -207,8 +207,8 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    return NULL;

    if (!phys_addr_valid(phys_addr)) {
    - printk(KERN_WARNING "ioremap: invalid physical address %llx\n",
    - (unsigned long long)phys_addr);
    + printk(KERN_WARNING "ioremap: invalid physical address %pr\n",
    + &phys_addr);
    WARN_ON_ONCE(1);
    return NULL;
    }
    @@ -267,9 +267,9 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    (prot_val == _PAGE_CACHE_WC &&
    new_prot_val == _PAGE_CACHE_WB)) {
    pr_debug(
    - "ioremap error for 0x%llx-0x%llx, requested 0x%lx, got 0x%lx\n",
    - (unsigned long long)phys_addr,
    - (unsigned long long)(phys_addr + size),
    + "ioremap error for %pr [%lx], requested 0x%lx, got 0x%lx\n",
    + &phys_addr,
    + size,
    prot_val, new_prot_val);
    free_memtype(phys_addr, phys_addr + size);
    return NULL;
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH] x86, ioremap: use %pR in printk


    > so how about something like the two patches below (ontop of Linus's
    > patch): the first patch introduces a "small" resource pointer printout
    > format: %pr - the little brother of %pR.
    >
    > The output format is [0x00001234] - minimum width is 8.
    >
    > The second patch takes advantage of it in ioremap.c.


    Well, I did the exact same patch except I used the same function
    and just added a flag for "R" vs. "r". However, I didn't post it
    because I wasn't too happy with passing by pointer and I wasn't
    sure whether we wanted to keep the letter after p uppercase or not... In
    any case, I kept it as a thing to discuss after the first one goes in.

    At this stage, I'm tempted to go for a %pP for printing a pointer to
    a phys_addr_t (and that's the same as resource_size_t, just more generic
    nowadays, since those were consolidated).

    Still not too happy with the pointer thing but that's the best we can
    do I suppose without losing gcc type checking.

    Cheers,
    Ben.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH] x86, ioremap: use %pR in printk

    On Mon, 2008-10-20 at 22:15 +1100, Benjamin Herrenschmidt wrote:
    > > so how about something like the two patches below (ontop of Linus's
    > > patch): the first patch introduces a "small" resource pointer printout
    > > format: %pr - the little brother of %pR.
    > >
    > > The output format is [0x00001234] - minimum width is 8.
    > >
    > > The second patch takes advantage of it in ioremap.c.

    >
    > Well, I did the exact same patch except I used the same function
    > and just added a flag for "R" vs. "r". However, I didn't post it
    > because I wasn't too happy with passing by pointer and I wasn't
    > sure whether we wanted to keep the letter after p uppercase or not... In
    > any case, I kept it as a thing to discuss after the first one goes in.
    >
    > At this stage, I'm tempted to go for a %pP for printing a pointer to
    > a phys_addr_t (and that's the same as resource_size_t, just more generic
    > nowadays, since those were consolidated).
    >
    > Still not too happy with the pointer thing but that's the best we can
    > do I suppose without losing gcc type checking.


    Oh, and I didn't like having the brackets around something that isn't
    a range too but that's a minor details.

    Ben.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH] x86, ioremap: use %pR in printk


    * Ingo Molnar wrote:

    >
    > * Benjamin Herrenschmidt wrote:
    >
    > > > Still not too happy with the pointer thing but that's the best we
    > > > can do I suppose without losing gcc type checking.

    > >
    > > Oh, and I didn't like having the brackets around something that isn't
    > > a range too but that's a minor details.

    >
    > it looked quite natural in printouts to me, but no strong feelings.


    got rid of the brackets, see the patches below.

    One open question would be whether to set the width to 8 on 32-bit
    platforms and 16 on 64-bit platforms - right now it's 8 on both. Since
    this is specifically a 'physical address' thing it might make sense to
    extend that on 64-bit systems. (although it's quite a bit of screen real
    estate so i think the current width of 8 should be fine)

    Ingo

    ---------->
    commit b74e806f605f2c3eda181066f60f2bf6585d835a
    Author: Ingo Molnar
    Date: Mon Oct 20 09:08:57 2008 +0200

    x86, ioremap: use %pP in printk

    use the new %pP physical address printk type conversion specifier.

    Signed-off-by: Ingo Molnar

    diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
    index ae71e11..aaa81d9 100644
    --- a/arch/x86/mm/ioremap.c
    +++ b/arch/x86/mm/ioremap.c
    @@ -207,8 +207,8 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    return NULL;

    if (!phys_addr_valid(phys_addr)) {
    - printk(KERN_WARNING "ioremap: invalid physical address %llx\n",
    - (unsigned long long)phys_addr);
    + printk(KERN_WARNING "ioremap: invalid physical address %pP\n",
    + &phys_addr);
    WARN_ON_ONCE(1);
    return NULL;
    }
    @@ -267,9 +267,9 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    (prot_val == _PAGE_CACHE_WC &&
    new_prot_val == _PAGE_CACHE_WB)) {
    pr_debug(
    - "ioremap error for 0x%llx-0x%llx, requested 0x%lx, got 0x%lx\n",
    - (unsigned long long)phys_addr,
    - (unsigned long long)(phys_addr + size),
    + "ioremap error for %pP [%lx], requested 0x%lx, got 0x%lx\n",
    + &phys_addr,
    + size,
    prot_val, new_prot_val);
    free_memtype(phys_addr, phys_addr + size);
    return NULL;

    commit 075279167da29f6bb701597a32e6b7311faa1782
    Author: Ingo Molnar
    Date: Mon Oct 20 12:39:17 2008 +0200

    vsprintf: implement %pP to print phys_addr_t

    Add %pP to print addresses in phys_addr_t variables. Passed by reference.

    Signed-off-by: Ingo Molnar

    diff --git a/lib/vsprintf.c b/lib/vsprintf.c
    index a013bbc..aac4fff 100644
    --- a/lib/vsprintf.c
    +++ b/lib/vsprintf.c
    @@ -581,6 +581,19 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
    return string(buf, end, sym, field_width, precision, flags);
    }

    +static char *phys_addr_string(char *buf, char *end, phys_addr_t *val, int field_width, int precision, int flags)
    +{
    + /* room for the actual number, the "0x" and the final zero */
    + char sym[2*sizeof(phys_addr_t) + 2];
    + char *p = sym, *pend = sym + sizeof(sym);
    + int size = 8;
    +
    + p = number(p, pend, *val, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p = 0;
    +
    + return string(buf, end, sym, field_width, precision, flags);
    +}
    +
    /*
    * Show a '%p' thing. A kernel extension is that the '%p' is followed
    * by an extra set of alphanumeric characters that are extended format
    @@ -592,6 +605,8 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
    * - 'S' For symbolic direct pointers
    * - 'R' For a struct resource pointer, it prints the range of
    * addresses (not the name nor the flags)
    + * - 'P' For a phys_addr_t pointer, it prints the physical
    + * addresses (with a minimum width of 8 characters)
    *
    * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
    * function pointers are really function descriptors, which contain a
    @@ -607,6 +622,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    return symbol_string(buf, end, ptr, field_width, precision, flags);
    case 'R':
    return resource_string(buf, end, ptr, field_width, precision, flags);
    + case 'P':
    + return phys_addr_string(buf, end, ptr, field_width, precision, flags);
    }
    flags |= SMALL;
    if (field_width == -1) {
    @@ -627,6 +644,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    * %pS output the name of a text symbol
    * %pF output the name of a function pointer
    * %pR output the address range in a struct resource
    + * %pP output the address in a pointer to a phys_addr_t type
    *
    * The return value is the number of characters which would
    * be generated for the given input, excluding the trailing

    commit 0d391458be88baf3c079e60c3ea331ebe12902c0
    Author: Benjamin Herrenschmidt
    Date: Mon Oct 20 15:07:37 2008 +1100

    pci: use new %pR to print resource ranges

    This converts things in drivers/pci to use %pR to printout the
    content of a struct resource instead of hand-casted %llx or
    other variants.

    Signed-off-by: Benjamin Herrenschmidt
    Signed-off-by: Ingo Molnar

    diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
    index c9884bb..dbe9f39 100644
    --- a/drivers/pci/pci.c
    +++ b/drivers/pci/pci.c
    @@ -1358,11 +1358,10 @@ int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
    return 0;

    err_out:
    - dev_warn(&pdev->dev, "BAR %d: can't reserve %s region [%#llx-%#llx]\n",
    + dev_warn(&pdev->dev, "BAR %d: can't reserve %s region %pR\n",
    bar,
    pci_resource_flags(pdev, bar) & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)pci_resource_start(pdev, bar),
    - (unsigned long long)pci_resource_end(pdev, bar));
    + &pdev->resource[bar]);
    return -EBUSY;
    }

    diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
    index dd9161a..d3db8b2 100644
    --- a/drivers/pci/probe.c
    +++ b/drivers/pci/probe.c
    @@ -304,9 +304,8 @@ static int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
    } else {
    res->start = l64;
    res->end = l64 + sz64;
    - printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
    - pci_name(dev), pos, (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: %pR\n",
    + pci_name(dev), pos, res);
    }
    } else {
    sz = pci_size(l, sz, mask);
    @@ -316,9 +315,10 @@ static int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,

    res->start = l;
    res->end = l + sz;
    - printk(KERN_DEBUG "PCI: %s reg %x %s: [%llx, %llx]\n", pci_name(dev),
    - pos, (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    - (unsigned long long)res->start, (unsigned long long)res->end);
    + printk(KERN_DEBUG "PCI: %s reg %x %s: %pR\n",
    + pci_name(dev), pos,
    + (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    + res);
    }

    out:
    @@ -389,9 +389,8 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
    res->start = base;
    if (!res->end)
    res->end = limit + 0xfff;
    - printk(KERN_DEBUG "PCI: bridge %s io port: [%llx, %llx]\n",
    - pci_name(dev), (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s io port: %pR\n",
    + pci_name(dev), res);
    }

    res = child->resource[1];
    @@ -403,9 +402,8 @@ 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;
    - printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: [%llx, %llx]\n",
    - pci_name(dev), (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: %pR\n",
    + pci_name(dev), res);
    }

    res = child->resource[2];
    @@ -441,9 +439,9 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
    res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
    res->start = base;
    res->end = limit + 0xfffff;
    - printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: [%llx, %llx]\n",
    - pci_name(dev), (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64" : "32",
    - (unsigned long long) res->start, (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: %pR\n",
    + pci_name(dev),
    + (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64":"32", res);
    }
    }

    diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
    index d5e2106..471a429 100644
    --- a/drivers/pci/setup-bus.c
    +++ b/drivers/pci/setup-bus.c
    @@ -356,10 +356,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, unsigned long
    order = __ffs(align) - 20;
    if (order > 11) {
    dev_warn(&dev->dev, "BAR %d bad alignment %llx: "
    - "%#016llx-%#016llx\n", i,
    - (unsigned long long)align,
    - (unsigned long long)r->start,
    - (unsigned long long)r->end);
    + "%pR\n", i, (unsigned long long)align, r);
    r->flags = 0;
    continue;
    }
    @@ -539,11 +536,9 @@ static void pci_bus_dump_res(struct pci_bus *bus)
    if (!res)
    continue;

    - printk(KERN_INFO "bus: %02x index %x %s: [%llx, %llx]\n",
    - bus->number, i,
    - (res->flags & IORESOURCE_IO) ? "io port" : "mmio",
    - (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_INFO "bus: %02x index %x %s: %pR\n",
    + bus->number, i,
    + (res->flags & IORESOURCE_IO) ? "io port" : "mmio", res);
    }
    }

    diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
    index 1a5fc83..d4b5c69 100644
    --- a/drivers/pci/setup-res.c
    +++ b/drivers/pci/setup-res.c
    @@ -49,10 +49,8 @@ void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno)

    pcibios_resource_to_bus(dev, &region, res);

    - dev_dbg(&dev->dev, "BAR %d: got res [%#llx-%#llx] bus [%#llx-%#llx] "
    - "flags %#lx\n", resno,
    - (unsigned long long)res->start,
    - (unsigned long long)res->end,
    + 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);
    @@ -114,13 +112,11 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
    err = insert_resource(root, res);

    if (err) {
    - dev_err(&dev->dev, "BAR %d: %s of %s [%#llx-%#llx]\n",
    + dev_err(&dev->dev, "BAR %d: %s of %s %pR\n",
    resource,
    root ? "address space collision on" :
    "no parent found for",
    - dtype,
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dtype, res);
    }

    return err;
    @@ -139,9 +135,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
    align = resource_alignment(res);
    if (!align) {
    dev_err(&dev->dev, "BAR %d: can't allocate resource (bogus "
    - "alignment) [%#llx-%#llx] flags %#lx\n",
    - resno, (unsigned long long)res->start,
    - (unsigned long long)res->end, res->flags);
    + "alignment) %pR flags %#lx\n",
    + resno, res, res->flags);
    return -EINVAL;
    }

    @@ -162,11 +157,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
    }

    if (ret) {
    - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    - "[%#llx-%#llx]\n", resno,
    - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    } else {
    res->flags &= ~IORESOURCE_STARTALIGN;
    if (resno < PCI_BRIDGE_RESOURCES)
    @@ -202,11 +194,8 @@ int pci_assign_resource_fixed(struct pci_dev *dev, int resno)
    }

    if (ret) {
    - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    - "[%#llx-%#llx\n]", resno,
    - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    } else if (resno < PCI_BRIDGE_RESOURCES) {
    pci_update_resource(dev, res, resno);
    }
    @@ -237,9 +226,8 @@ void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
    r_align = resource_alignment(r);
    if (!r_align) {
    dev_warn(&dev->dev, "BAR %d: bogus alignment "
    - "[%#llx-%#llx] flags %#lx\n",
    - i, (unsigned long long)r->start,
    - (unsigned long long)r->end, r->flags);
    + "%pR flags %#lx\n",
    + i, r, r->flags);
    continue;
    }
    for (list = head; ; list = list->next) {
    @@ -287,9 +275,7 @@ int pci_enable_resources(struct pci_dev *dev, int mask)

    if (!r->parent) {
    dev_err(&dev->dev, "device not available because of "
    - "BAR %d [%#llx-%#llx] collisions\n", i,
    - (unsigned long long) r->start,
    - (unsigned long long) r->end);
    + "BAR %d %pR collisions\n", i, r);
    return -EINVAL;
    }


    commit c21f132b1eca94808fe72b31aa159c7f0ad61d25
    Author: Linus Torvalds
    Date: Mon Oct 20 15:07:34 2008 +1100

    vsprintf: implement %pR to print struct resource content

    Add a %pR option to the kernel vsnprintf that prints the range of
    addresses inside a struct resource passed by pointer.

    Padding now defaults to 4 digits for IO and 8 for MEM

    Signed-off-by: Linus Torvalds
    Signed-off-by: Benjamin Herrenschmidt
    Signed-off-by: Ingo Molnar

    diff --git a/lib/vsprintf.c b/lib/vsprintf.c
    index cceecb6..a013bbc 100644
    --- a/lib/vsprintf.c
    +++ b/lib/vsprintf.c
    @@ -24,6 +24,7 @@
    #include
    #include
    #include
    +#include

    #include /* for PAGE_SIZE */
    #include
    @@ -550,18 +551,51 @@ static char *symbol_string(char *buf, char *end, void *ptr, int field_width, int
    #endif
    }

    +static char *resource_string(char *buf, char *end, struct resource *res, int field_width, int precision, int flags)
    +{
    +#ifndef IO_RSRC_PRINTK_SIZE
    +#define IO_RSRC_PRINTK_SIZE 4
    +#endif
    +
    +#ifndef MEM_RSRC_PRINTK_SIZE
    +#define MEM_RSRC_PRINTK_SIZE 8
    +#endif
    +
    + /* room for the actual numbers, the two "0x", -, [, ] and the final zero */
    + char sym[4*sizeof(resource_size_t) + 8];
    + char *p = sym, *pend = sym + sizeof(sym);
    + int size = -1;
    +
    + if (res->flags & IORESOURCE_IO)
    + size = IO_RSRC_PRINTK_SIZE;
    + else if (res->flags & IORESOURCE_MEM)
    + size = MEM_RSRC_PRINTK_SIZE;
    +
    + *p++ = '[';
    + p = number(p, pend, res->start, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p++ = '-';
    + p = number(p, pend, res->end, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p++ = ']';
    + *p = 0;
    +
    + return string(buf, end, sym, field_width, precision, flags);
    +}
    +
    /*
    * Show a '%p' thing. A kernel extension is that the '%p' is followed
    * by an extra set of alphanumeric characters that are extended format
    * specifiers.
    *
    - * Right now we just handle 'F' (for symbolic Function descriptor pointers)
    - * and 'S' (for Symbolic direct pointers), but this can easily be
    - * extended in the future (network address types etc).
    + * Right now we handle:
    + *
    + * - 'F' For symbolic function descriptor pointers
    + * - 'S' For symbolic direct pointers
    + * - 'R' For a struct resource pointer, it prints the range of
    + * addresses (not the name nor the flags)
    *
    - * The difference between 'S' and 'F' is that on ia64 and ppc64 function
    - * pointers are really function descriptors, which contain a pointer the
    - * real address.
    + * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
    + * function pointers are really function descriptors, which contain a
    + * pointer to the real address.
    */
    static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field_width, int precision, int flags)
    {
    @@ -571,6 +605,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    /* Fallthrough */
    case 'S':
    return symbol_string(buf, end, ptr, field_width, precision, flags);
    + case 'R':
    + return resource_string(buf, end, ptr, field_width, precision, flags);
    }
    flags |= SMALL;
    if (field_width == -1) {
    @@ -590,6 +626,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    * This function follows C99 vsnprintf, but has some extensions:
    * %pS output the name of a text symbol
    * %pF output the name of a function pointer
    + * %pR output the address range in a struct resource
    *
    * The return value is the number of characters which would
    * be generated for the given input, excluding the trailing
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH] x86, ioremap: use %pR in printk


    * Benjamin Herrenschmidt wrote:

    >
    > > so how about something like the two patches below (ontop of Linus's
    > > patch): the first patch introduces a "small" resource pointer printout
    > > format: %pr - the little brother of %pR.
    > >
    > > The output format is [0x00001234] - minimum width is 8.
    > >
    > > The second patch takes advantage of it in ioremap.c.

    >
    > Well, I did the exact same patch except I used the same function and
    > just added a flag for "R" vs. "r". However, I didn't post it because I
    > wasn't too happy with passing by pointer and I wasn't sure whether we
    > wanted to keep the letter after p uppercase or not... In any case, I
    > kept it as a thing to discuss after the first one goes in.
    >
    > At this stage, I'm tempted to go for a %pP for printing a pointer to a
    > phys_addr_t (and that's the same as resource_size_t, just more generic
    > nowadays, since those were consolidated).
    >
    > Still not too happy with the pointer thing but that's the best we can
    > do I suppose without losing gcc type checking.


    %pP sounds very good, and phys_addr_t is indeed the better choice as
    recently we mapped resource_size_t to phys_addr_t (they used to be
    separate for no good reason, up to a few days ago).

    Below are the updated patches that implement this.

    Ingo

    -------------->
    commit 73301b25ba6df2a54727a9fc27614787a5143dca
    Author: Ingo Molnar
    Date: Mon Oct 20 09:08:57 2008 +0200

    x86, ioremap: use %pP in printk

    use the new %pP physical address printk type conversion specifier.

    Signed-off-by: Ingo Molnar

    diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
    index ae71e11..aaa81d9 100644
    --- a/arch/x86/mm/ioremap.c
    +++ b/arch/x86/mm/ioremap.c
    @@ -207,8 +207,8 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    return NULL;

    if (!phys_addr_valid(phys_addr)) {
    - printk(KERN_WARNING "ioremap: invalid physical address %llx\n",
    - (unsigned long long)phys_addr);
    + printk(KERN_WARNING "ioremap: invalid physical address %pP\n",
    + &phys_addr);
    WARN_ON_ONCE(1);
    return NULL;
    }
    @@ -267,9 +267,9 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    (prot_val == _PAGE_CACHE_WC &&
    new_prot_val == _PAGE_CACHE_WB)) {
    pr_debug(
    - "ioremap error for 0x%llx-0x%llx, requested 0x%lx, got 0x%lx\n",
    - (unsigned long long)phys_addr,
    - (unsigned long long)(phys_addr + size),
    + "ioremap error for %pP [%lx], requested 0x%lx, got 0x%lx\n",
    + &phys_addr,
    + size,
    prot_val, new_prot_val);
    free_memtype(phys_addr, phys_addr + size);
    return NULL;

    commit 7fd44cc79290a613c2de83ca9ac624f6b04d7908
    Author: Ingo Molnar
    Date: Mon Oct 20 12:39:17 2008 +0200

    vsprintf: implement %pP to print phys_addr_t

    Add %pP to print addresses in phys_addr_t variables. Passed by reference.

    Signed-off-by: Ingo Molnar

    diff --git a/lib/vsprintf.c b/lib/vsprintf.c
    index a013bbc..3baed89 100644
    --- a/lib/vsprintf.c
    +++ b/lib/vsprintf.c
    @@ -581,6 +581,21 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
    return string(buf, end, sym, field_width, precision, flags);
    }

    +static char *phys_addr_string(char *buf, char *end, phys_addr_t *val, int field_width, int precision, int flags)
    +{
    + /* room for the actual number, the one "0x", [, ] and the final zero */
    + char sym[2*sizeof(phys_addr_t) + 5];
    + char *p = sym, *pend = sym + sizeof(sym);
    + int size = 8;
    +
    + *p++ = '[';
    + p = number(p, pend, *val, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p++ = ']';
    + *p = 0;
    +
    + return string(buf, end, sym, field_width, precision, flags);
    +}
    +
    /*
    * Show a '%p' thing. A kernel extension is that the '%p' is followed
    * by an extra set of alphanumeric characters that are extended format
    @@ -592,6 +607,8 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
    * - 'S' For symbolic direct pointers
    * - 'R' For a struct resource pointer, it prints the range of
    * addresses (not the name nor the flags)
    + * - 'P' For a phys_addr_t pointer, it prints the physical
    + * addresses (in %pR format)
    *
    * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
    * function pointers are really function descriptors, which contain a
    @@ -607,6 +624,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    return symbol_string(buf, end, ptr, field_width, precision, flags);
    case 'R':
    return resource_string(buf, end, ptr, field_width, precision, flags);
    + case 'P':
    + return phys_addr_string(buf, end, ptr, field_width, precision, flags);
    }
    flags |= SMALL;
    if (field_width == -1) {
    @@ -627,6 +646,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    * %pS output the name of a text symbol
    * %pF output the name of a function pointer
    * %pR output the address range in a struct resource
    + * %pP output the address in a pointer to a phys_addr_t type
    *
    * The return value is the number of characters which would
    * be generated for the given input, excluding the trailing

    commit 0d391458be88baf3c079e60c3ea331ebe12902c0
    Author: Benjamin Herrenschmidt
    Date: Mon Oct 20 15:07:37 2008 +1100

    pci: use new %pR to print resource ranges

    This converts things in drivers/pci to use %pR to printout the
    content of a struct resource instead of hand-casted %llx or
    other variants.

    Signed-off-by: Benjamin Herrenschmidt
    Signed-off-by: Ingo Molnar

    diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
    index c9884bb..dbe9f39 100644
    --- a/drivers/pci/pci.c
    +++ b/drivers/pci/pci.c
    @@ -1358,11 +1358,10 @@ int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
    return 0;

    err_out:
    - dev_warn(&pdev->dev, "BAR %d: can't reserve %s region [%#llx-%#llx]\n",
    + dev_warn(&pdev->dev, "BAR %d: can't reserve %s region %pR\n",
    bar,
    pci_resource_flags(pdev, bar) & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)pci_resource_start(pdev, bar),
    - (unsigned long long)pci_resource_end(pdev, bar));
    + &pdev->resource[bar]);
    return -EBUSY;
    }

    diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
    index dd9161a..d3db8b2 100644
    --- a/drivers/pci/probe.c
    +++ b/drivers/pci/probe.c
    @@ -304,9 +304,8 @@ static int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
    } else {
    res->start = l64;
    res->end = l64 + sz64;
    - printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
    - pci_name(dev), pos, (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: %pR\n",
    + pci_name(dev), pos, res);
    }
    } else {
    sz = pci_size(l, sz, mask);
    @@ -316,9 +315,10 @@ static int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,

    res->start = l;
    res->end = l + sz;
    - printk(KERN_DEBUG "PCI: %s reg %x %s: [%llx, %llx]\n", pci_name(dev),
    - pos, (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    - (unsigned long long)res->start, (unsigned long long)res->end);
    + printk(KERN_DEBUG "PCI: %s reg %x %s: %pR\n",
    + pci_name(dev), pos,
    + (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    + res);
    }

    out:
    @@ -389,9 +389,8 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
    res->start = base;
    if (!res->end)
    res->end = limit + 0xfff;
    - printk(KERN_DEBUG "PCI: bridge %s io port: [%llx, %llx]\n",
    - pci_name(dev), (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s io port: %pR\n",
    + pci_name(dev), res);
    }

    res = child->resource[1];
    @@ -403,9 +402,8 @@ 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;
    - printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: [%llx, %llx]\n",
    - pci_name(dev), (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: %pR\n",
    + pci_name(dev), res);
    }

    res = child->resource[2];
    @@ -441,9 +439,9 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
    res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
    res->start = base;
    res->end = limit + 0xfffff;
    - printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: [%llx, %llx]\n",
    - pci_name(dev), (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64" : "32",
    - (unsigned long long) res->start, (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: %pR\n",
    + pci_name(dev),
    + (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64":"32", res);
    }
    }

    diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
    index d5e2106..471a429 100644
    --- a/drivers/pci/setup-bus.c
    +++ b/drivers/pci/setup-bus.c
    @@ -356,10 +356,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, unsigned long
    order = __ffs(align) - 20;
    if (order > 11) {
    dev_warn(&dev->dev, "BAR %d bad alignment %llx: "
    - "%#016llx-%#016llx\n", i,
    - (unsigned long long)align,
    - (unsigned long long)r->start,
    - (unsigned long long)r->end);
    + "%pR\n", i, (unsigned long long)align, r);
    r->flags = 0;
    continue;
    }
    @@ -539,11 +536,9 @@ static void pci_bus_dump_res(struct pci_bus *bus)
    if (!res)
    continue;

    - printk(KERN_INFO "bus: %02x index %x %s: [%llx, %llx]\n",
    - bus->number, i,
    - (res->flags & IORESOURCE_IO) ? "io port" : "mmio",
    - (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_INFO "bus: %02x index %x %s: %pR\n",
    + bus->number, i,
    + (res->flags & IORESOURCE_IO) ? "io port" : "mmio", res);
    }
    }

    diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
    index 1a5fc83..d4b5c69 100644
    --- a/drivers/pci/setup-res.c
    +++ b/drivers/pci/setup-res.c
    @@ -49,10 +49,8 @@ void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno)

    pcibios_resource_to_bus(dev, &region, res);

    - dev_dbg(&dev->dev, "BAR %d: got res [%#llx-%#llx] bus [%#llx-%#llx] "
    - "flags %#lx\n", resno,
    - (unsigned long long)res->start,
    - (unsigned long long)res->end,
    + 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);
    @@ -114,13 +112,11 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
    err = insert_resource(root, res);

    if (err) {
    - dev_err(&dev->dev, "BAR %d: %s of %s [%#llx-%#llx]\n",
    + dev_err(&dev->dev, "BAR %d: %s of %s %pR\n",
    resource,
    root ? "address space collision on" :
    "no parent found for",
    - dtype,
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dtype, res);
    }

    return err;
    @@ -139,9 +135,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
    align = resource_alignment(res);
    if (!align) {
    dev_err(&dev->dev, "BAR %d: can't allocate resource (bogus "
    - "alignment) [%#llx-%#llx] flags %#lx\n",
    - resno, (unsigned long long)res->start,
    - (unsigned long long)res->end, res->flags);
    + "alignment) %pR flags %#lx\n",
    + resno, res, res->flags);
    return -EINVAL;
    }

    @@ -162,11 +157,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
    }

    if (ret) {
    - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    - "[%#llx-%#llx]\n", resno,
    - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    } else {
    res->flags &= ~IORESOURCE_STARTALIGN;
    if (resno < PCI_BRIDGE_RESOURCES)
    @@ -202,11 +194,8 @@ int pci_assign_resource_fixed(struct pci_dev *dev, int resno)
    }

    if (ret) {
    - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    - "[%#llx-%#llx\n]", resno,
    - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    } else if (resno < PCI_BRIDGE_RESOURCES) {
    pci_update_resource(dev, res, resno);
    }
    @@ -237,9 +226,8 @@ void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
    r_align = resource_alignment(r);
    if (!r_align) {
    dev_warn(&dev->dev, "BAR %d: bogus alignment "
    - "[%#llx-%#llx] flags %#lx\n",
    - i, (unsigned long long)r->start,
    - (unsigned long long)r->end, r->flags);
    + "%pR flags %#lx\n",
    + i, r, r->flags);
    continue;
    }
    for (list = head; ; list = list->next) {
    @@ -287,9 +275,7 @@ int pci_enable_resources(struct pci_dev *dev, int mask)

    if (!r->parent) {
    dev_err(&dev->dev, "device not available because of "
    - "BAR %d [%#llx-%#llx] collisions\n", i,
    - (unsigned long long) r->start,
    - (unsigned long long) r->end);
    + "BAR %d %pR collisions\n", i, r);
    return -EINVAL;
    }


    commit c21f132b1eca94808fe72b31aa159c7f0ad61d25
    Author: Linus Torvalds
    Date: Mon Oct 20 15:07:34 2008 +1100

    vsprintf: implement %pR to print struct resource content

    Add a %pR option to the kernel vsnprintf that prints the range of
    addresses inside a struct resource passed by pointer.

    Padding now defaults to 4 digits for IO and 8 for MEM

    Signed-off-by: Linus Torvalds
    Signed-off-by: Benjamin Herrenschmidt
    Signed-off-by: Ingo Molnar

    diff --git a/lib/vsprintf.c b/lib/vsprintf.c
    index cceecb6..a013bbc 100644
    --- a/lib/vsprintf.c
    +++ b/lib/vsprintf.c
    @@ -24,6 +24,7 @@
    #include
    #include
    #include
    +#include

    #include /* for PAGE_SIZE */
    #include
    @@ -550,18 +551,51 @@ static char *symbol_string(char *buf, char *end, void *ptr, int field_width, int
    #endif
    }

    +static char *resource_string(char *buf, char *end, struct resource *res, int field_width, int precision, int flags)
    +{
    +#ifndef IO_RSRC_PRINTK_SIZE
    +#define IO_RSRC_PRINTK_SIZE 4
    +#endif
    +
    +#ifndef MEM_RSRC_PRINTK_SIZE
    +#define MEM_RSRC_PRINTK_SIZE 8
    +#endif
    +
    + /* room for the actual numbers, the two "0x", -, [, ] and the final zero */
    + char sym[4*sizeof(resource_size_t) + 8];
    + char *p = sym, *pend = sym + sizeof(sym);
    + int size = -1;
    +
    + if (res->flags & IORESOURCE_IO)
    + size = IO_RSRC_PRINTK_SIZE;
    + else if (res->flags & IORESOURCE_MEM)
    + size = MEM_RSRC_PRINTK_SIZE;
    +
    + *p++ = '[';
    + p = number(p, pend, res->start, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p++ = '-';
    + p = number(p, pend, res->end, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p++ = ']';
    + *p = 0;
    +
    + return string(buf, end, sym, field_width, precision, flags);
    +}
    +
    /*
    * Show a '%p' thing. A kernel extension is that the '%p' is followed
    * by an extra set of alphanumeric characters that are extended format
    * specifiers.
    *
    - * Right now we just handle 'F' (for symbolic Function descriptor pointers)
    - * and 'S' (for Symbolic direct pointers), but this can easily be
    - * extended in the future (network address types etc).
    + * Right now we handle:
    + *
    + * - 'F' For symbolic function descriptor pointers
    + * - 'S' For symbolic direct pointers
    + * - 'R' For a struct resource pointer, it prints the range of
    + * addresses (not the name nor the flags)
    *
    - * The difference between 'S' and 'F' is that on ia64 and ppc64 function
    - * pointers are really function descriptors, which contain a pointer the
    - * real address.
    + * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
    + * function pointers are really function descriptors, which contain a
    + * pointer to the real address.
    */
    static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field_width, int precision, int flags)
    {
    @@ -571,6 +605,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    /* Fallthrough */
    case 'S':
    return symbol_string(buf, end, ptr, field_width, precision, flags);
    + case 'R':
    + return resource_string(buf, end, ptr, field_width, precision, flags);
    }
    flags |= SMALL;
    if (field_width == -1) {
    @@ -590,6 +626,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    * This function follows C99 vsnprintf, but has some extensions:
    * %pS output the name of a text symbol
    * %pF output the name of a function pointer
    + * %pR output the address range in a struct resource
    *
    * The return value is the number of characters which would
    * be generated for the given input, excluding the trailing
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [PATCH] x86, ioremap: use %pR in printk


    * Benjamin Herrenschmidt wrote:

    > > Still not too happy with the pointer thing but that's the best we
    > > can do I suppose without losing gcc type checking.

    >
    > Oh, and I didn't like having the brackets around something that isn't
    > a range too but that's a minor details.


    it looked quite natural in printouts to me, but no strong feelings.

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [PATCH] x86, ioremap: use %pR in printk

    On Mon, 2008-10-20 at 13:36 +0200, Ingo Molnar wrote:

    > + /* room for the actual number, the "0x" and the final zero */
    > + char sym[2*sizeof(phys_addr_t) + 2];


    Buffer overflow.

    johannes

    -----BEGIN PGP SIGNATURE-----
    Comment: Johannes Berg (powerbook)

    iQIcBAABAgAGBQJI/IBbAAoJEKVg1VMiehFY7KgP/1FVB7epsiKR+CjLPmtRKjvK
    gtpkBfes5COvdRNJ4dwHv8zXvhC/gwWDMTJfixZoPrIEdBiC2FT4oIlEhdoBT9My
    v3T4LCqRPTOM9iWfgaE/16x855EZLvR12sDYGJiKcjixs2Tju2lBnDSZoZMJGmdx
    hLE41nsr1NmXwnV1Kc6i/IMZhLxGk4Ofr6d8kS9gzidaRgd8kYs06JSk8Ko6EBxM
    7QETMiFuHCsoJUtTikowtB/CN9yNrrZtSR1rmVM/k9m3D7mSLBukErujVpcG1Tuu
    ZEmOG0T5ofo0c4ZVyjHY3zU9svxp53TbfYlZPcuph+v5as3Bnm QzV0mVIHLg6JXE
    Gz95A0jWWkfdxBG6h/mMPPG1DsGJ8DPd95VBZoXX2YDajzfoWTwqCZXZ0QR3uXO0
    i3kPmBkV+bbCcgAgRyCRTVec+fMHhKRUKlBGq9NGR8KiPmCyHY rRxymO8dT+2DZF
    Zhw8uSb7DNaRFKWrHKWs1Y7+E6vhWSgs5jq/NTVH0iJQLmfATpU4s/+754ARRGBe
    FPwu+7u/HwX8/cO8GrcaVl0Xv4YFXtVv7t1b6Cittf8wNJWj5SvzuxThBwaplpG l
    K0D5uqiCoxh55keYuB5GeAvQGAYLgoVMPCZNJ1Ve6/jpBamFKPbZoF5wBHFQPK/N
    F0+eRoEhhiHrnIH0G8lx
    =HApQ
    -----END PGP SIGNATURE-----


  12. Re: [PATCH] x86, ioremap: use %pR in printk

    On Mon, Oct 20, 2008 at 01:36:02PM +0200, Ingo Molnar wrote:
    > One open question would be whether to set the width to 8 on 32-bit
    > platforms and 16 on 64-bit platforms - right now it's 8 on both. Since
    > this is specifically a 'physical address' thing it might make sense to
    > extend that on 64-bit systems. (although it's quite a bit of screen real
    > estate so i think the current width of 8 should be fine)


    Maybe we should let architectures configure it -- after all, they know
    the real size of a physical address. I'm thinking something like this:

    #ifndef PHYS_ADDR_T_SIZE
    #ifdef CONFIG_64BIT
    #define PHYS_ADDR_T_SIZE 64
    #else
    #define PHYS_ADDR_T_SIZE 32
    #endif
    #endif

    static char *phys_addr_string(char *buf, char *end, phys_addr_t *val, int field_width, int precision, int flags)
    {
    /* room for the actual number, the "0x" and the final zero */
    char sym[2*sizeof(phys_addr_t) + 3];
    char *p = sym, *pend = sym + sizeof(sym);
    int size = DIV_ROUND_UP(PHYS_ADDR_T_SIZE, 4);

    p = number(p, pend, *val, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    *p = 0;

    return string(buf, end, sym, field_width, precision, flags);
    }

    Architectures can define PHYS_ADDR_T_SIZE to a variable if they want
    to be able to detect it at boot-time. Or just define it to a number
    (eg 36 for PAE).

    By the way, the patch I'm replying to has a bug; you mis-sized 'sym'.
    It needs three extra bytes, not two.

    --
    Matthew Wilcox Intel Open Source Technology Centre
    "Bill, look, we understand that you're interested in selling us this
    operating system, but compare it to ours. We can't possibly take such
    a retrograde step."
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH] x86, ioremap: use %pR in printk


    * Johannes Berg wrote:

    > On Mon, 2008-10-20 at 13:36 +0200, Ingo Molnar wrote:
    >
    > > + /* room for the actual number, the "0x" and the final zero */
    > > + char sym[2*sizeof(phys_addr_t) + 2];

    >
    > Buffer overflow.


    good spotting, that should be +3. And it's still untested ;-)

    Ingo

    -------------------->
    commit e8613ba0d3912d526c9de2317f40ca2a236c026d
    Author: Ingo Molnar
    Date: Mon Oct 20 09:08:57 2008 +0200

    x86, ioremap: use %pP in printk

    use the new %pP physical address printk type conversion specifier.

    Signed-off-by: Ingo Molnar

    diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
    index ae71e11..aaa81d9 100644
    --- a/arch/x86/mm/ioremap.c
    +++ b/arch/x86/mm/ioremap.c
    @@ -207,8 +207,8 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    return NULL;

    if (!phys_addr_valid(phys_addr)) {
    - printk(KERN_WARNING "ioremap: invalid physical address %llx\n",
    - (unsigned long long)phys_addr);
    + printk(KERN_WARNING "ioremap: invalid physical address %pP\n",
    + &phys_addr);
    WARN_ON_ONCE(1);
    return NULL;
    }
    @@ -267,9 +267,9 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
    (prot_val == _PAGE_CACHE_WC &&
    new_prot_val == _PAGE_CACHE_WB)) {
    pr_debug(
    - "ioremap error for 0x%llx-0x%llx, requested 0x%lx, got 0x%lx\n",
    - (unsigned long long)phys_addr,
    - (unsigned long long)(phys_addr + size),
    + "ioremap error for %pP [%lx], requested 0x%lx, got 0x%lx\n",
    + &phys_addr,
    + size,
    prot_val, new_prot_val);
    free_memtype(phys_addr, phys_addr + size);
    return NULL;

    commit 97cc157df4144c20ff2939f2acd4eaf96907ed55
    Author: Ingo Molnar
    Date: Mon Oct 20 12:39:17 2008 +0200

    vsprintf: implement %pP to print phys_addr_t

    Add %pP to print addresses in phys_addr_t variables. Passed by reference.

    Signed-off-by: Ingo Molnar

    diff --git a/lib/vsprintf.c b/lib/vsprintf.c
    index a013bbc..d0a3e2c 100644
    --- a/lib/vsprintf.c
    +++ b/lib/vsprintf.c
    @@ -581,6 +581,19 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
    return string(buf, end, sym, field_width, precision, flags);
    }

    +static char *phys_addr_string(char *buf, char *end, phys_addr_t *val, int field_width, int precision, int flags)
    +{
    + /* room for the actual number, the "0x" and the final zero */
    + char sym[2*sizeof(phys_addr_t) + 3];
    + char *p = sym, *pend = sym + sizeof(sym);
    + int size = 8;
    +
    + p = number(p, pend, *val, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p = 0;
    +
    + return string(buf, end, sym, field_width, precision, flags);
    +}
    +
    /*
    * Show a '%p' thing. A kernel extension is that the '%p' is followed
    * by an extra set of alphanumeric characters that are extended format
    @@ -592,6 +605,8 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
    * - 'S' For symbolic direct pointers
    * - 'R' For a struct resource pointer, it prints the range of
    * addresses (not the name nor the flags)
    + * - 'P' For a phys_addr_t pointer, it prints the physical
    + * addresses (with a minimum width of 8 characters)
    *
    * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
    * function pointers are really function descriptors, which contain a
    @@ -607,6 +622,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    return symbol_string(buf, end, ptr, field_width, precision, flags);
    case 'R':
    return resource_string(buf, end, ptr, field_width, precision, flags);
    + case 'P':
    + return phys_addr_string(buf, end, ptr, field_width, precision, flags);
    }
    flags |= SMALL;
    if (field_width == -1) {
    @@ -627,6 +644,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    * %pS output the name of a text symbol
    * %pF output the name of a function pointer
    * %pR output the address range in a struct resource
    + * %pP output the address in a pointer to a phys_addr_t type
    *
    * The return value is the number of characters which would
    * be generated for the given input, excluding the trailing

    commit 0d391458be88baf3c079e60c3ea331ebe12902c0
    Author: Benjamin Herrenschmidt
    Date: Mon Oct 20 15:07:37 2008 +1100

    pci: use new %pR to print resource ranges

    This converts things in drivers/pci to use %pR to printout the
    content of a struct resource instead of hand-casted %llx or
    other variants.

    Signed-off-by: Benjamin Herrenschmidt
    Signed-off-by: Ingo Molnar

    diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
    index c9884bb..dbe9f39 100644
    --- a/drivers/pci/pci.c
    +++ b/drivers/pci/pci.c
    @@ -1358,11 +1358,10 @@ int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
    return 0;

    err_out:
    - dev_warn(&pdev->dev, "BAR %d: can't reserve %s region [%#llx-%#llx]\n",
    + dev_warn(&pdev->dev, "BAR %d: can't reserve %s region %pR\n",
    bar,
    pci_resource_flags(pdev, bar) & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)pci_resource_start(pdev, bar),
    - (unsigned long long)pci_resource_end(pdev, bar));
    + &pdev->resource[bar]);
    return -EBUSY;
    }

    diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
    index dd9161a..d3db8b2 100644
    --- a/drivers/pci/probe.c
    +++ b/drivers/pci/probe.c
    @@ -304,9 +304,8 @@ static int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
    } else {
    res->start = l64;
    res->end = l64 + sz64;
    - printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
    - pci_name(dev), pos, (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: %pR\n",
    + pci_name(dev), pos, res);
    }
    } else {
    sz = pci_size(l, sz, mask);
    @@ -316,9 +315,10 @@ static int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,

    res->start = l;
    res->end = l + sz;
    - printk(KERN_DEBUG "PCI: %s reg %x %s: [%llx, %llx]\n", pci_name(dev),
    - pos, (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    - (unsigned long long)res->start, (unsigned long long)res->end);
    + printk(KERN_DEBUG "PCI: %s reg %x %s: %pR\n",
    + pci_name(dev), pos,
    + (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    + res);
    }

    out:
    @@ -389,9 +389,8 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
    res->start = base;
    if (!res->end)
    res->end = limit + 0xfff;
    - printk(KERN_DEBUG "PCI: bridge %s io port: [%llx, %llx]\n",
    - pci_name(dev), (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s io port: %pR\n",
    + pci_name(dev), res);
    }

    res = child->resource[1];
    @@ -403,9 +402,8 @@ 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;
    - printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: [%llx, %llx]\n",
    - pci_name(dev), (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: %pR\n",
    + pci_name(dev), res);
    }

    res = child->resource[2];
    @@ -441,9 +439,9 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
    res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
    res->start = base;
    res->end = limit + 0xfffff;
    - printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: [%llx, %llx]\n",
    - pci_name(dev), (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64" : "32",
    - (unsigned long long) res->start, (unsigned long long) res->end);
    + printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: %pR\n",
    + pci_name(dev),
    + (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64":"32", res);
    }
    }

    diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
    index d5e2106..471a429 100644
    --- a/drivers/pci/setup-bus.c
    +++ b/drivers/pci/setup-bus.c
    @@ -356,10 +356,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, unsigned long
    order = __ffs(align) - 20;
    if (order > 11) {
    dev_warn(&dev->dev, "BAR %d bad alignment %llx: "
    - "%#016llx-%#016llx\n", i,
    - (unsigned long long)align,
    - (unsigned long long)r->start,
    - (unsigned long long)r->end);
    + "%pR\n", i, (unsigned long long)align, r);
    r->flags = 0;
    continue;
    }
    @@ -539,11 +536,9 @@ static void pci_bus_dump_res(struct pci_bus *bus)
    if (!res)
    continue;

    - printk(KERN_INFO "bus: %02x index %x %s: [%llx, %llx]\n",
    - bus->number, i,
    - (res->flags & IORESOURCE_IO) ? "io port" : "mmio",
    - (unsigned long long) res->start,
    - (unsigned long long) res->end);
    + printk(KERN_INFO "bus: %02x index %x %s: %pR\n",
    + bus->number, i,
    + (res->flags & IORESOURCE_IO) ? "io port" : "mmio", res);
    }
    }

    diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
    index 1a5fc83..d4b5c69 100644
    --- a/drivers/pci/setup-res.c
    +++ b/drivers/pci/setup-res.c
    @@ -49,10 +49,8 @@ void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno)

    pcibios_resource_to_bus(dev, &region, res);

    - dev_dbg(&dev->dev, "BAR %d: got res [%#llx-%#llx] bus [%#llx-%#llx] "
    - "flags %#lx\n", resno,
    - (unsigned long long)res->start,
    - (unsigned long long)res->end,
    + 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);
    @@ -114,13 +112,11 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
    err = insert_resource(root, res);

    if (err) {
    - dev_err(&dev->dev, "BAR %d: %s of %s [%#llx-%#llx]\n",
    + dev_err(&dev->dev, "BAR %d: %s of %s %pR\n",
    resource,
    root ? "address space collision on" :
    "no parent found for",
    - dtype,
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dtype, res);
    }

    return err;
    @@ -139,9 +135,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
    align = resource_alignment(res);
    if (!align) {
    dev_err(&dev->dev, "BAR %d: can't allocate resource (bogus "
    - "alignment) [%#llx-%#llx] flags %#lx\n",
    - resno, (unsigned long long)res->start,
    - (unsigned long long)res->end, res->flags);
    + "alignment) %pR flags %#lx\n",
    + resno, res, res->flags);
    return -EINVAL;
    }

    @@ -162,11 +157,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
    }

    if (ret) {
    - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    - "[%#llx-%#llx]\n", resno,
    - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    } else {
    res->flags &= ~IORESOURCE_STARTALIGN;
    if (resno < PCI_BRIDGE_RESOURCES)
    @@ -202,11 +194,8 @@ int pci_assign_resource_fixed(struct pci_dev *dev, int resno)
    }

    if (ret) {
    - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    - "[%#llx-%#llx\n]", resno,
    - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    - (unsigned long long)res->start,
    - (unsigned long long)res->end);
    + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    } else if (resno < PCI_BRIDGE_RESOURCES) {
    pci_update_resource(dev, res, resno);
    }
    @@ -237,9 +226,8 @@ void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
    r_align = resource_alignment(r);
    if (!r_align) {
    dev_warn(&dev->dev, "BAR %d: bogus alignment "
    - "[%#llx-%#llx] flags %#lx\n",
    - i, (unsigned long long)r->start,
    - (unsigned long long)r->end, r->flags);
    + "%pR flags %#lx\n",
    + i, r, r->flags);
    continue;
    }
    for (list = head; ; list = list->next) {
    @@ -287,9 +275,7 @@ int pci_enable_resources(struct pci_dev *dev, int mask)

    if (!r->parent) {
    dev_err(&dev->dev, "device not available because of "
    - "BAR %d [%#llx-%#llx] collisions\n", i,
    - (unsigned long long) r->start,
    - (unsigned long long) r->end);
    + "BAR %d %pR collisions\n", i, r);
    return -EINVAL;
    }


    commit c21f132b1eca94808fe72b31aa159c7f0ad61d25
    Author: Linus Torvalds
    Date: Mon Oct 20 15:07:34 2008 +1100

    vsprintf: implement %pR to print struct resource content

    Add a %pR option to the kernel vsnprintf that prints the range of
    addresses inside a struct resource passed by pointer.

    Padding now defaults to 4 digits for IO and 8 for MEM

    Signed-off-by: Linus Torvalds
    Signed-off-by: Benjamin Herrenschmidt
    Signed-off-by: Ingo Molnar

    diff --git a/lib/vsprintf.c b/lib/vsprintf.c
    index cceecb6..a013bbc 100644
    --- a/lib/vsprintf.c
    +++ b/lib/vsprintf.c
    @@ -24,6 +24,7 @@
    #include
    #include
    #include
    +#include

    #include /* for PAGE_SIZE */
    #include
    @@ -550,18 +551,51 @@ static char *symbol_string(char *buf, char *end, void *ptr, int field_width, int
    #endif
    }

    +static char *resource_string(char *buf, char *end, struct resource *res, int field_width, int precision, int flags)
    +{
    +#ifndef IO_RSRC_PRINTK_SIZE
    +#define IO_RSRC_PRINTK_SIZE 4
    +#endif
    +
    +#ifndef MEM_RSRC_PRINTK_SIZE
    +#define MEM_RSRC_PRINTK_SIZE 8
    +#endif
    +
    + /* room for the actual numbers, the two "0x", -, [, ] and the final zero */
    + char sym[4*sizeof(resource_size_t) + 8];
    + char *p = sym, *pend = sym + sizeof(sym);
    + int size = -1;
    +
    + if (res->flags & IORESOURCE_IO)
    + size = IO_RSRC_PRINTK_SIZE;
    + else if (res->flags & IORESOURCE_MEM)
    + size = MEM_RSRC_PRINTK_SIZE;
    +
    + *p++ = '[';
    + p = number(p, pend, res->start, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p++ = '-';
    + p = number(p, pend, res->end, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    + *p++ = ']';
    + *p = 0;
    +
    + return string(buf, end, sym, field_width, precision, flags);
    +}
    +
    /*
    * Show a '%p' thing. A kernel extension is that the '%p' is followed
    * by an extra set of alphanumeric characters that are extended format
    * specifiers.
    *
    - * Right now we just handle 'F' (for symbolic Function descriptor pointers)
    - * and 'S' (for Symbolic direct pointers), but this can easily be
    - * extended in the future (network address types etc).
    + * Right now we handle:
    + *
    + * - 'F' For symbolic function descriptor pointers
    + * - 'S' For symbolic direct pointers
    + * - 'R' For a struct resource pointer, it prints the range of
    + * addresses (not the name nor the flags)
    *
    - * The difference between 'S' and 'F' is that on ia64 and ppc64 function
    - * pointers are really function descriptors, which contain a pointer the
    - * real address.
    + * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
    + * function pointers are really function descriptors, which contain a
    + * pointer to the real address.
    */
    static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field_width, int precision, int flags)
    {
    @@ -571,6 +605,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    /* Fallthrough */
    case 'S':
    return symbol_string(buf, end, ptr, field_width, precision, flags);
    + case 'R':
    + return resource_string(buf, end, ptr, field_width, precision, flags);
    }
    flags |= SMALL;
    if (field_width == -1) {
    @@ -590,6 +626,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    * This function follows C99 vsnprintf, but has some extensions:
    * %pS output the name of a text symbol
    * %pF output the name of a function pointer
    + * %pR output the address range in a struct resource
    *
    * The return value is the number of characters which would
    * be generated for the given input, excluding the trailing
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [PATCH] x86, ioremap: use %pR in printk

    On Mon, 2008-10-20 at 13:36 +0200, Ingo Molnar wrote:
    > got rid of the brackets, see the patches below.
    >
    > One open question would be whether to set the width to 8 on 32-bit
    > platforms and 16 on 64-bit platforms - right now it's 8 on both. Since
    > this is specifically a 'physical address' thing it might make sense to
    > extend that on 64-bit systems. (although it's quite a bit of screen real
    > estate so i think the current width of 8 should be fine)


    A -lot- of 64-bit platforms (though not all of them) have most of their
    stuff still in the 32 bit space, especially when IO is concerned.
    Keeping it to 8 thus makes the output nicer on those, and as Linus
    mentioned before, it's not like we lose digits anyway.

    If you want, you can re-use the #ifdef/#define I did for resources and
    thus give archs the option to have a different default.

    Cheers,
    Ben.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [PATCH] x86, ioremap: use %pR in printk


    > +static char *phys_addr_string(char *buf, char *end, phys_addr_t *val, int field_width, int precision, int flags)
    > +{
    > + /* room for the actual number, the "0x" and the final zero */
    > + char sym[2*sizeof(phys_addr_t) + 2];


    Aren't you missing one byte here ?

    Cheers,
    Ben.

    > + char *p = sym, *pend = sym + sizeof(sym);
    > + int size = 8;
    > +
    > + p = number(p, pend, *val, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    > + *p = 0;
    > +
    > + return string(buf, end, sym, field_width, precision, flags);
    > +}
    > +
    > /*
    > * Show a '%p' thing. A kernel extension is that the '%p' is followed
    > * by an extra set of alphanumeric characters that are extended format
    > @@ -592,6 +605,8 @@ static char *resource_string(char *buf, char *end, struct resource *res, int fie
    > * - 'S' For symbolic direct pointers
    > * - 'R' For a struct resource pointer, it prints the range of
    > * addresses (not the name nor the flags)
    > + * - 'P' For a phys_addr_t pointer, it prints the physical
    > + * addresses (with a minimum width of 8 characters)
    > *
    > * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
    > * function pointers are really function descriptors, which contain a
    > @@ -607,6 +622,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    > return symbol_string(buf, end, ptr, field_width, precision, flags);
    > case 'R':
    > return resource_string(buf, end, ptr, field_width, precision, flags);
    > + case 'P':
    > + return phys_addr_string(buf, end, ptr, field_width, precision, flags);
    > }
    > flags |= SMALL;
    > if (field_width == -1) {
    > @@ -627,6 +644,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    > * %pS output the name of a text symbol
    > * %pF output the name of a function pointer
    > * %pR output the address range in a struct resource
    > + * %pP output the address in a pointer to a phys_addr_t type
    > *
    > * The return value is the number of characters which would
    > * be generated for the given input, excluding the trailing
    >
    > commit 0d391458be88baf3c079e60c3ea331ebe12902c0
    > Author: Benjamin Herrenschmidt
    > Date: Mon Oct 20 15:07:37 2008 +1100
    >
    > pci: use new %pR to print resource ranges
    >
    > This converts things in drivers/pci to use %pR to printout the
    > content of a struct resource instead of hand-casted %llx or
    > other variants.
    >
    > Signed-off-by: Benjamin Herrenschmidt
    > Signed-off-by: Ingo Molnar
    >
    > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
    > index c9884bb..dbe9f39 100644
    > --- a/drivers/pci/pci.c
    > +++ b/drivers/pci/pci.c
    > @@ -1358,11 +1358,10 @@ int pci_request_region(struct pci_dev *pdev, int bar, const char *res_name)
    > return 0;
    >
    > err_out:
    > - dev_warn(&pdev->dev, "BAR %d: can't reserve %s region [%#llx-%#llx]\n",
    > + dev_warn(&pdev->dev, "BAR %d: can't reserve %s region %pR\n",
    > bar,
    > pci_resource_flags(pdev, bar) & IORESOURCE_IO ? "I/O" : "mem",
    > - (unsigned long long)pci_resource_start(pdev, bar),
    > - (unsigned long long)pci_resource_end(pdev, bar));
    > + &pdev->resource[bar]);
    > return -EBUSY;
    > }
    >
    > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
    > index dd9161a..d3db8b2 100644
    > --- a/drivers/pci/probe.c
    > +++ b/drivers/pci/probe.c
    > @@ -304,9 +304,8 @@ static int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
    > } else {
    > res->start = l64;
    > res->end = l64 + sz64;
    > - printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: [%llx, %llx]\n",
    > - pci_name(dev), pos, (unsigned long long)res->start,
    > - (unsigned long long)res->end);
    > + printk(KERN_DEBUG "PCI: %s reg %x 64bit mmio: %pR\n",
    > + pci_name(dev), pos, res);
    > }
    > } else {
    > sz = pci_size(l, sz, mask);
    > @@ -316,9 +315,10 @@ static int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
    >
    > res->start = l;
    > res->end = l + sz;
    > - printk(KERN_DEBUG "PCI: %s reg %x %s: [%llx, %llx]\n", pci_name(dev),
    > - pos, (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    > - (unsigned long long)res->start, (unsigned long long)res->end);
    > + printk(KERN_DEBUG "PCI: %s reg %x %s: %pR\n",
    > + pci_name(dev), pos,
    > + (res->flags & IORESOURCE_IO) ? "io port":"32bit mmio",
    > + res);
    > }
    >
    > out:
    > @@ -389,9 +389,8 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
    > res->start = base;
    > if (!res->end)
    > res->end = limit + 0xfff;
    > - printk(KERN_DEBUG "PCI: bridge %s io port: [%llx, %llx]\n",
    > - pci_name(dev), (unsigned long long) res->start,
    > - (unsigned long long) res->end);
    > + printk(KERN_DEBUG "PCI: bridge %s io port: %pR\n",
    > + pci_name(dev), res);
    > }
    >
    > res = child->resource[1];
    > @@ -403,9 +402,8 @@ 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;
    > - printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: [%llx, %llx]\n",
    > - pci_name(dev), (unsigned long long) res->start,
    > - (unsigned long long) res->end);
    > + printk(KERN_DEBUG "PCI: bridge %s 32bit mmio: %pR\n",
    > + pci_name(dev), res);
    > }
    >
    > res = child->resource[2];
    > @@ -441,9 +439,9 @@ void __devinit pci_read_bridge_bases(struct pci_bus *child)
    > res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
    > res->start = base;
    > res->end = limit + 0xfffff;
    > - printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: [%llx, %llx]\n",
    > - pci_name(dev), (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64" : "32",
    > - (unsigned long long) res->start, (unsigned long long) res->end);
    > + printk(KERN_DEBUG "PCI: bridge %s %sbit mmio pref: %pR\n",
    > + pci_name(dev),
    > + (res->flags & PCI_PREF_RANGE_TYPE_64) ? "64":"32", res);
    > }
    > }
    >
    > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
    > index d5e2106..471a429 100644
    > --- a/drivers/pci/setup-bus.c
    > +++ b/drivers/pci/setup-bus.c
    > @@ -356,10 +356,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, unsigned long
    > order = __ffs(align) - 20;
    > if (order > 11) {
    > dev_warn(&dev->dev, "BAR %d bad alignment %llx: "
    > - "%#016llx-%#016llx\n", i,
    > - (unsigned long long)align,
    > - (unsigned long long)r->start,
    > - (unsigned long long)r->end);
    > + "%pR\n", i, (unsigned long long)align, r);
    > r->flags = 0;
    > continue;
    > }
    > @@ -539,11 +536,9 @@ static void pci_bus_dump_res(struct pci_bus *bus)
    > if (!res)
    > continue;
    >
    > - printk(KERN_INFO "bus: %02x index %x %s: [%llx, %llx]\n",
    > - bus->number, i,
    > - (res->flags & IORESOURCE_IO) ? "io port" : "mmio",
    > - (unsigned long long) res->start,
    > - (unsigned long long) res->end);
    > + printk(KERN_INFO "bus: %02x index %x %s: %pR\n",
    > + bus->number, i,
    > + (res->flags & IORESOURCE_IO) ? "io port" : "mmio", res);
    > }
    > }
    >
    > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
    > index 1a5fc83..d4b5c69 100644
    > --- a/drivers/pci/setup-res.c
    > +++ b/drivers/pci/setup-res.c
    > @@ -49,10 +49,8 @@ void pci_update_resource(struct pci_dev *dev, struct resource *res, int resno)
    >
    > pcibios_resource_to_bus(dev, &region, res);
    >
    > - dev_dbg(&dev->dev, "BAR %d: got res [%#llx-%#llx] bus [%#llx-%#llx] "
    > - "flags %#lx\n", resno,
    > - (unsigned long long)res->start,
    > - (unsigned long long)res->end,
    > + 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);
    > @@ -114,13 +112,11 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
    > err = insert_resource(root, res);
    >
    > if (err) {
    > - dev_err(&dev->dev, "BAR %d: %s of %s [%#llx-%#llx]\n",
    > + dev_err(&dev->dev, "BAR %d: %s of %s %pR\n",
    > resource,
    > root ? "address space collision on" :
    > "no parent found for",
    > - dtype,
    > - (unsigned long long)res->start,
    > - (unsigned long long)res->end);
    > + dtype, res);
    > }
    >
    > return err;
    > @@ -139,9 +135,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
    > align = resource_alignment(res);
    > if (!align) {
    > dev_err(&dev->dev, "BAR %d: can't allocate resource (bogus "
    > - "alignment) [%#llx-%#llx] flags %#lx\n",
    > - resno, (unsigned long long)res->start,
    > - (unsigned long long)res->end, res->flags);
    > + "alignment) %pR flags %#lx\n",
    > + resno, res, res->flags);
    > return -EINVAL;
    > }
    >
    > @@ -162,11 +157,8 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
    > }
    >
    > if (ret) {
    > - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    > - "[%#llx-%#llx]\n", resno,
    > - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    > - (unsigned long long)res->start,
    > - (unsigned long long)res->end);
    > + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    > + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    > } else {
    > res->flags &= ~IORESOURCE_STARTALIGN;
    > if (resno < PCI_BRIDGE_RESOURCES)
    > @@ -202,11 +194,8 @@ int pci_assign_resource_fixed(struct pci_dev *dev, int resno)
    > }
    >
    > if (ret) {
    > - dev_err(&dev->dev, "BAR %d: can't allocate %s resource "
    > - "[%#llx-%#llx\n]", resno,
    > - res->flags & IORESOURCE_IO ? "I/O" : "mem",
    > - (unsigned long long)res->start,
    > - (unsigned long long)res->end);
    > + dev_err(&dev->dev, "BAR %d: can't allocate %s resource %pR\n",
    > + resno, res->flags & IORESOURCE_IO ? "I/O" : "mem", res);
    > } else if (resno < PCI_BRIDGE_RESOURCES) {
    > pci_update_resource(dev, res, resno);
    > }
    > @@ -237,9 +226,8 @@ void pdev_sort_resources(struct pci_dev *dev, struct resource_list *head)
    > r_align = resource_alignment(r);
    > if (!r_align) {
    > dev_warn(&dev->dev, "BAR %d: bogus alignment "
    > - "[%#llx-%#llx] flags %#lx\n",
    > - i, (unsigned long long)r->start,
    > - (unsigned long long)r->end, r->flags);
    > + "%pR flags %#lx\n",
    > + i, r, r->flags);
    > continue;
    > }
    > for (list = head; ; list = list->next) {
    > @@ -287,9 +275,7 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
    >
    > if (!r->parent) {
    > dev_err(&dev->dev, "device not available because of "
    > - "BAR %d [%#llx-%#llx] collisions\n", i,
    > - (unsigned long long) r->start,
    > - (unsigned long long) r->end);
    > + "BAR %d %pR collisions\n", i, r);
    > return -EINVAL;
    > }
    >
    >
    > commit c21f132b1eca94808fe72b31aa159c7f0ad61d25
    > Author: Linus Torvalds
    > Date: Mon Oct 20 15:07:34 2008 +1100
    >
    > vsprintf: implement %pR to print struct resource content
    >
    > Add a %pR option to the kernel vsnprintf that prints the range of
    > addresses inside a struct resource passed by pointer.
    >
    > Padding now defaults to 4 digits for IO and 8 for MEM
    >
    > Signed-off-by: Linus Torvalds
    > Signed-off-by: Benjamin Herrenschmidt
    > Signed-off-by: Ingo Molnar
    >
    > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
    > index cceecb6..a013bbc 100644
    > --- a/lib/vsprintf.c
    > +++ b/lib/vsprintf.c
    > @@ -24,6 +24,7 @@
    > #include
    > #include
    > #include
    > +#include
    >
    > #include /* for PAGE_SIZE */
    > #include
    > @@ -550,18 +551,51 @@ static char *symbol_string(char *buf, char *end, void *ptr, int field_width, int
    > #endif
    > }
    >
    > +static char *resource_string(char *buf, char *end, struct resource *res, int field_width, int precision, int flags)
    > +{
    > +#ifndef IO_RSRC_PRINTK_SIZE
    > +#define IO_RSRC_PRINTK_SIZE 4
    > +#endif
    > +
    > +#ifndef MEM_RSRC_PRINTK_SIZE
    > +#define MEM_RSRC_PRINTK_SIZE 8
    > +#endif
    > +
    > + /* room for the actual numbers, the two "0x", -, [, ] and the final zero */
    > + char sym[4*sizeof(resource_size_t) + 8];
    > + char *p = sym, *pend = sym + sizeof(sym);
    > + int size = -1;
    > +
    > + if (res->flags & IORESOURCE_IO)
    > + size = IO_RSRC_PRINTK_SIZE;
    > + else if (res->flags & IORESOURCE_MEM)
    > + size = MEM_RSRC_PRINTK_SIZE;
    > +
    > + *p++ = '[';
    > + p = number(p, pend, res->start, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    > + *p++ = '-';
    > + p = number(p, pend, res->end, 16, size, -1, SPECIAL | SMALL | ZEROPAD);
    > + *p++ = ']';
    > + *p = 0;
    > +
    > + return string(buf, end, sym, field_width, precision, flags);
    > +}
    > +
    > /*
    > * Show a '%p' thing. A kernel extension is that the '%p' is followed
    > * by an extra set of alphanumeric characters that are extended format
    > * specifiers.
    > *
    > - * Right now we just handle 'F' (for symbolic Function descriptor pointers)
    > - * and 'S' (for Symbolic direct pointers), but this can easily be
    > - * extended in the future (network address types etc).
    > + * Right now we handle:
    > + *
    > + * - 'F' For symbolic function descriptor pointers
    > + * - 'S' For symbolic direct pointers
    > + * - 'R' For a struct resource pointer, it prints the range of
    > + * addresses (not the name nor the flags)
    > *
    > - * The difference between 'S' and 'F' is that on ia64 and ppc64 function
    > - * pointers are really function descriptors, which contain a pointer the
    > - * real address.
    > + * Note: The difference between 'S' and 'F' is that on ia64 and ppc64
    > + * function pointers are really function descriptors, which contain a
    > + * pointer to the real address.
    > */
    > static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field_width, int precision, int flags)
    > {
    > @@ -571,6 +605,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    > /* Fallthrough */
    > case 'S':
    > return symbol_string(buf, end, ptr, field_width, precision, flags);
    > + case 'R':
    > + return resource_string(buf, end, ptr, field_width, precision, flags);
    > }
    > flags |= SMALL;
    > if (field_width == -1) {
    > @@ -590,6 +626,7 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
    > * This function follows C99 vsnprintf, but has some extensions:
    > * %pS output the name of a text symbol
    > * %pF output the name of a function pointer
    > + * %pR output the address range in a struct resource
    > *
    > * The return value is the number of characters which would
    > * be generated for the given input, excluding the trailing


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [PATCH] x86, ioremap: use %pR in printk

    On Mon, 2008-10-20 at 14:58 +0200, Johannes Berg wrote:
    > On Mon, 2008-10-20 at 13:36 +0200, Ingo Molnar wrote:
    >
    > > + /* room for the actual number, the "0x" and the final zero */
    > > + char sym[2*sizeof(phys_addr_t) + 2];

    >
    > Buffer overflow.


    It's actually not going to overflow as we pass the end of the buffer to
    "number" but we'll miss a digit.

    Ben.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [PATCH] x86, ioremap: use %pR in printk

    On Tue, 2008-10-21 at 08:35 +1100, Benjamin Herrenschmidt wrote:
    > On Mon, 2008-10-20 at 14:58 +0200, Johannes Berg wrote:
    > > On Mon, 2008-10-20 at 13:36 +0200, Ingo Molnar wrote:
    > >
    > > > + /* room for the actual number, the "0x" and the final zero */
    > > > + char sym[2*sizeof(phys_addr_t) + 2];

    > >
    > > Buffer overflow.

    >
    > It's actually not going to overflow as we pass the end of the buffer to
    > "number" but we'll miss a digit.


    "buf" won't overflow, but "sym" will, no? Anyway, +3 is the correct
    thing to do.

    johannes

    -----BEGIN PGP SIGNATURE-----
    Comment: Johannes Berg (powerbook)

    iQIcBAABAgAGBQJI/Xr9AAoJEKVg1VMiehFYAtIP/iJV5VlO+Gnth9KzRnKG4B8k
    bC0LF9Q5uIxxSv4pZgLOiLAfVsEpMb7N8BztQ+9DplkFbFOGDe VTpBgCaQb2arIO
    lsxN9hhP2bVwjqQdMnOzB3i69rLpFG8vFMXWajQg+RF3+a/ntLyqnKtlIdQAikKS
    PsVoEliwrTHf1tIKcjvVycGVYYayhlB3EOUh7MAjNIy4HT1CbP yKcaXt0hc22w3u
    jWZzZ7ceU2uFlMKLPpeI+eX3GXHCGWMPmTVVFXzGd+nkv0aD42 edUGIktYQNR9N8
    r/BZVYxvo58UrsRKGqBbAdhyHOXnLoyNAdxXgcvDMCr8HCp0tJPp ejchrj0kjuAa
    GtkJ6oH/0RBSBLfqp2Ib2kuWlg19KkHSX1ctEKzFxWsL4GqKXofEzUqpG2 fqocZX
    U9SXQwyG/4l+cZJwZD3Qybj5GZbJXeee5fVinrqssSh42jh9/kQFMmUJah9Dcyi+
    h2QNu8fFrlgqumK+wJ+KFmQZ9AdAZ76PH39jOKq7Hmn/GU1S6uFCDQPlhd2zY5Dd
    bohXJ6UqN6voE41EAbQgdWdIR+/pfvC6h0cnXjJBoLOiIqSyE3Fn+ZXXCzyijgFM
    8qX6Di3VXIjnMLLu201OLQ2BJ0TQf9okWTkKuTtzB/Qb4tFy+051wFZ8h35T9Ise
    btYZuWGR+mNr+472dz3i
    =Gl+r
    -----END PGP SIGNATURE-----


+ Reply to Thread