On Wed, 2015-01-21 at 19:06 +0200, Pantelis Antoniou wrote:
> 90% of the usage of device node's full_name is printing it out
> in a kernel message. Preparing for the eventual delayed allocation
> introduce a custom printk format specifier that is both more
> compact and more pleasant to the eye.
> 
> For instance typical use is:
>       pr_info("Frobbing node %s\n", node->full_name);
> 
> Which can be written now as:
>       pr_info("Frobbing node %pO\n", node);

Still disliking use of %p0.

> More fine-grained control of formatting includes printing the name,
> flag, path-spec name, reference count and others, explained in the
> documentation entry.
> 
> Signed-off-by: Pantelis Antoniou <pantelis.anton...@konsulko.com>
> 
> dt-print
> ---
>  Documentation/printk-formats.txt |  29 ++++++++
>  lib/vsprintf.c                   | 151 
> +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 180 insertions(+)
> 
> diff --git a/Documentation/printk-formats.txt 
> b/Documentation/printk-formats.txt
> index 5a615c1..2d42c04 100644
> --- a/Documentation/printk-formats.txt
> +++ b/Documentation/printk-formats.txt
> @@ -231,6 +231,35 @@ struct va_format:
>       Do not use this feature without some mechanism to verify the
>       correctness of the format string and va_list arguments.
>  
> +Device tree nodes:
> +
> +     %pO[fnpPcCFr]
> +
> +     For printing device tree nodes. The optional arguments are:
> +            f device node full_name
> +            n device node name
> +            p device node phandle
> +            P device node path spec (name + @unit)
> +            F device node flags
> +            c major compatible string
> +            C full compatible string
> +            r node reference count
> +     Without any arguments prints full_name (same as %pOf)
> +     The separator when using multiple arguments is '|'
> +
> +     Examples:
> +
> +     %pO     /foo/bar@0                      - Node full name
> +     %pOf    /foo/bar@0                      - Same as above
> +     %pOfp   /foo/bar@0|10                   - Node full name + phandle
> +     %pOfcF  /foo/bar@0|foo,device|--P-      - Node full name +
> +                                               major compatible string +
> +                                               node flags
> +                                                     D - dynamic
> +                                                     d - detached
> +                                                     P - Populated
> +                                                     B - Populated bus
> +
>  u64 SHOULD be printed with %llu/%llx:
>  
>       printk("%llu", u64_var);
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
[]

Add #ifdef back ?

> +static noinline_for_stack
> +char *device_node_string(char *buf, char *end, struct device_node *dn,
> +                      struct printf_spec spec, const char *fmt)
> +{
> +     char tbuf[sizeof("xxxxxxxxxx") + 1];
> +     const char *fmtp, *p;
> +     int len, ret, i, j, pass;
> +     char c;
> +
> +     if (!IS_ENABLED(CONFIG_OF))
> +             return string(buf, end, "(!OF)", spec);

Not very descriptive output, maybe the address would be better.

> +
> +     if ((unsigned long)dn < PAGE_SIZE)
> +             return string(buf, end, "(null)", spec);
> +
> +     /* simple case without anything any more format specifiers */
> +     if (fmt[1] == '\0' || isspace(fmt[1]))
> +             fmt = "Of";

why lower case here but upper case above?

> +
> +     len = 0;
> +
> +     /* two passes; the first calculates length, the second fills in */
> +     for (pass = 1; pass <= 2; pass++) {
> +             if (pass == 2 && !(spec.flags & LEFT)) {
> +                     /* padding */
> +                     while (len < spec.field_width--) {
> +                             if (buf < end)
> +                                     *buf = ' ';
> +                             ++buf;
> +                     }
> +             }
> +#undef _HANDLE_CH
> +#define _HANDLE_CH(_ch) \
> +     do { \
> +             if (pass == 1) \
> +                     len++; \
> +             else \
> +                     if (buf < end) \
> +                             *buf++ = (_ch); \
> +     } while (0)
> +#undef _HANDLE_STR
> +#define _HANDLE_STR(_str) \
> +     do { \
> +             const char *str = (_str); \
> +             if (pass == 1) \
> +                     len += strlen(str); \
> +             else \
> +                     while (*str && buf < end) \
> +                             *buf++ = *str++; \
> +     } while (0)

This isn't pretty.  Perhaps there's a better way?

> +
> +             for (fmtp = fmt + 1, j = 0; (c = *fmtp++) != '\0'; ) {
> +                     switch (c) {
> +                     case 'f':       /* full_name */
> +                             if (j++ > 0)
> +                                     _HANDLE_CH(':');
> +                             _HANDLE_STR(of_node_full_name(dn));
> +                             break;
> +                     case 'n':       /* name */
> +                             if (j++ > 0)
> +                                     _HANDLE_CH('|');
> +                             _HANDLE_STR(dn->name);
> +                             break;
> +                     case 'p':       /* phandle */
> +                             if (j++ > 0)
> +                                     _HANDLE_CH('|');
> +                             snprintf(tbuf, sizeof(tbuf), "%u",
> +                                             (unsigned int)dn->phandle);
> +                             _HANDLE_STR(tbuf);
> +                             break;
> +                     case 'P':       /* path-spec */
> +                             if (j++ > 0)
> +                                     _HANDLE_CH('|');
> +                             _HANDLE_STR(dn->name);
> +                             /* need to tack on the @ postfix */
> +                             p = strchr(of_node_full_name(dn), '@');
> +                             if (p)
> +                                     _HANDLE_STR(p);
> +                             break;
> +                     case 'F':       /* flags */
> +                             if (j++ > 0)
> +                                     _HANDLE_CH('|');
> +                             snprintf(tbuf, sizeof(tbuf), "%c%c%c%c",
> +                                     of_node_check_flag(dn, OF_DYNAMIC) ?
> +                                             'D' : '-',
> +                                     of_node_check_flag(dn, OF_DETACHED) ?
> +                                             'd' : '-',
> +                                     of_node_check_flag(dn, OF_POPULATED) ?
> +                                             'P' : '-',
> +                                     of_node_check_flag(dn,
> +                                             OF_POPULATED_BUS) ?  'B' : '-');
> +                             _HANDLE_STR(tbuf);
> +                             break;
> +                     case 'c':       /* major compatible string */
> +                             if (j++ > 0)
> +                                     _HANDLE_CH('|');
> +                             ret = of_property_read_string(dn, "compatible",
> +                                             &p);
> +                             if (ret == 0)
> +                                     _HANDLE_STR(p);
> +                             break;
> +                     case 'C':       /* full compatible string */
> +                             if (j++ > 0)
> +                                     _HANDLE_CH('|');
> +                             i = 0;
> +                             while (of_property_read_string_index(dn,
> +                                             "compatible", i, &p) == 0) {
> +                                     if (i == 0)
> +                                             _HANDLE_STR("\"");
> +                                     else
> +                                             _HANDLE_STR("\",\"");
> +                                     _HANDLE_STR(p);
> +                                     i++;
> +                             }
> +                             if (i > 0)
> +                                     _HANDLE_STR("\"");
> +                             break;
> +                     case 'r':       /* node reference count */
> +                             if (j++ > 0)
> +                                     _HANDLE_CH('|');
> +                             snprintf(tbuf, sizeof(tbuf), "%u",
> +                                     atomic_read(&dn->kobj.kref.refcount));
> +                             _HANDLE_STR(tbuf);
> +                             break;
> +                     default:
> +                             break;
> +                     }
> +             }
> +     }
> +     /* finish up */
> +     while (buf < end && len < spec.field_width--)
> +             *buf++ = ' ';
> +#undef _HANDLE_CH
> +#undef _HANDLE_STR


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

Reply via email to