* [PATCH 0/4] DT printf format specifiers @ 2017-06-14 20:30 Rob Herring [not found] ` <20170614203025.7581-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> ` (3 more replies) 0 siblings, 4 replies; 19+ messages in thread From: Rob Herring @ 2017-06-14 20:30 UTC (permalink / raw) To: Frank Rowand, Mark Rutland Cc: Pantelis Antoniou, Joe Perches, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA This resurrects an old patch[1] from Pantelis adding printf format specifiers for DT nodes. The previous versions didn't get applied after debate about the what character(s) to use. Grant suggested %pO for base kobject and %pOF for struct device_node. Everyone agreed, but no new version was posted. I ended up re-writing the core implementation to be more inline with how other format specifiers are written which allowed removing #define code fragments. The other 3 patches convert the core DT code to use %pOF and prepare for changing device_node.full_name to stop storing the full path for every node. My plan is to merge this series for v4.13 and post follow-up patches to convert all arches and subsystems to %pOF for v4.14. The full series is available here[2]. I tested this on QEMU running the DT unittests. Rob [1] https://patchwork.kernel.org/patch/6127521/ [2] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dt-printf Pantelis Antoniou (1): of: Custom printk format specifier for device node Rob Herring (3): of: use kbasename instead of open coding of: find_node_by_full_name rewrite to compare each level of: Convert to using %pOF instead of full_name Documentation/printk-formats.txt | 31 +++++++++ drivers/of/address.c | 21 +++--- drivers/of/base.c | 76 ++++++++++++---------- drivers/of/device.c | 2 +- drivers/of/dynamic.c | 33 +++++----- drivers/of/irq.c | 10 +-- drivers/of/of_mdio.c | 10 +-- drivers/of/of_pci.c | 29 ++++----- drivers/of/of_private.h | 3 + drivers/of/overlay.c | 21 +++--- drivers/of/platform.c | 34 +++++----- drivers/of/resolver.c | 34 ++-------- drivers/of/unittest.c | 66 +++++++++++-------- lib/vsprintf.c | 135 ++++++++++++++++++++++++++++++++++++++- scripts/checkpatch.pl | 2 +- 15 files changed, 331 insertions(+), 176 deletions(-) -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <20170614203025.7581-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* [PATCH 1/4] of: use kbasename instead of open coding [not found] ` <20170614203025.7581-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-06-14 20:30 ` Rob Herring [not found] ` <20170614203025.7581-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 19+ messages in thread From: Rob Herring @ 2017-06-14 20:30 UTC (permalink / raw) To: Frank Rowand, Mark Rutland Cc: Pantelis Antoniou, Joe Perches, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Several places in DT code open code the equivalent of kbasename. Replace them. The behavior for root nodes in node_name_cmp will be slightly different. Instead of comparing "/", "" will be compared. The comparison will be the same. Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- drivers/of/base.c | 5 +---- drivers/of/platform.c | 2 +- drivers/of/resolver.c | 4 ++-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 28d5f53bc631..054159ccd5f8 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -773,10 +773,7 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent, return NULL; __for_each_child_of_node(parent, child) { - const char *name = strrchr(child->full_name, '/'); - if (WARN(!name, "malformed device_node %s\n", child->full_name)) - continue; - name++; + const char *name = kbasename(child->full_name); if (strncmp(path, name, len) == 0 && (strlen(name) == len)) return child; } diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 71fecc2debfc..8f73413fa243 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -99,7 +99,7 @@ static void of_device_make_bus_id(struct device *dev) /* format arguments only used if dev_name() resolves to NULL */ dev_set_name(dev, dev_name(dev) ? "%s:%s" : "%s", - strrchr(node->full_name, '/') + 1, dev_name(dev)); + kbasename(node->full_name), dev_name(dev)); node = node->parent; } } diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 771f4844c781..63626d7d9adb 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -165,8 +165,8 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay, static int node_name_cmp(const struct device_node *dn1, const struct device_node *dn2) { - const char *n1 = strrchr(dn1->full_name, '/') ? : "/"; - const char *n2 = strrchr(dn2->full_name, '/') ? : "/"; + const char *n1 = kbasename(dn1->full_name); + const char *n2 = kbasename(dn2->full_name); return of_node_cmp(n1, n2); } -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 19+ messages in thread
[parent not found: <20170614203025.7581-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 1/4] of: use kbasename instead of open coding [not found] ` <20170614203025.7581-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-06-17 17:30 ` Andy Shevchenko 0 siblings, 0 replies; 19+ messages in thread From: Andy Shevchenko @ 2017-06-17 17:30 UTC (permalink / raw) To: Rob Herring Cc: Frank Rowand, Mark Rutland, Pantelis Antoniou, Joe Perches, devicetree, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Wed, Jun 14, 2017 at 11:30 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > Several places in DT code open code the equivalent of kbasename. > Replace them. > > The behavior for root nodes in node_name_cmp will be slightly different. > Instead of comparing "/", "" will be compared. The comparison will be > the same. Reviewed-by: Andy Shevchenko <andy.shevhchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > --- > drivers/of/base.c | 5 +---- > drivers/of/platform.c | 2 +- > drivers/of/resolver.c | 4 ++-- > 3 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index 28d5f53bc631..054159ccd5f8 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -773,10 +773,7 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent, > return NULL; > > __for_each_child_of_node(parent, child) { > - const char *name = strrchr(child->full_name, '/'); > - if (WARN(!name, "malformed device_node %s\n", child->full_name)) > - continue; > - name++; > + const char *name = kbasename(child->full_name); > if (strncmp(path, name, len) == 0 && (strlen(name) == len)) > return child; > } > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 71fecc2debfc..8f73413fa243 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -99,7 +99,7 @@ static void of_device_make_bus_id(struct device *dev) > > /* format arguments only used if dev_name() resolves to NULL */ > dev_set_name(dev, dev_name(dev) ? "%s:%s" : "%s", > - strrchr(node->full_name, '/') + 1, dev_name(dev)); > + kbasename(node->full_name), dev_name(dev)); > node = node->parent; > } > } > diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c > index 771f4844c781..63626d7d9adb 100644 > --- a/drivers/of/resolver.c > +++ b/drivers/of/resolver.c > @@ -165,8 +165,8 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay, > static int node_name_cmp(const struct device_node *dn1, > const struct device_node *dn2) > { > - const char *n1 = strrchr(dn1->full_name, '/') ? : "/"; > - const char *n2 = strrchr(dn2->full_name, '/') ? : "/"; > + const char *n1 = kbasename(dn1->full_name); > + const char *n2 = kbasename(dn2->full_name); > > return of_node_cmp(n1, n2); > } > -- > 2.11.0 > -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/4] of: find_node_by_full_name rewrite to compare each level 2017-06-14 20:30 [PATCH 0/4] DT printf format specifiers Rob Herring [not found] ` <20170614203025.7581-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-06-14 20:30 ` Rob Herring 2017-06-14 20:30 ` [PATCH 3/4] of: Custom printk format specifier for device node Rob Herring 2017-06-14 20:30 ` [PATCH 4/4] of: Convert to using %pOF instead of full_name Rob Herring 3 siblings, 0 replies; 19+ messages in thread From: Rob Herring @ 2017-06-14 20:30 UTC (permalink / raw) To: Frank Rowand, Mark Rutland Cc: Pantelis Antoniou, Joe Perches, devicetree, linux-kernel find_node_by_full_name() does the same thing as of_find_node_by_path(), but takes no locks and doesn't work on aliases. Refactor of_find_node_opts_by_path() into __of_find_node_by_full_path() and replace find_node_by_full_name() with it. Signed-off-by: Rob Herring <robh@kernel.org> --- drivers/of/base.c | 29 +++++++++++++++++++---------- drivers/of/of_private.h | 3 +++ drivers/of/resolver.c | 30 +++--------------------------- 3 files changed, 25 insertions(+), 37 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 054159ccd5f8..02be991a29c5 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -780,6 +780,24 @@ static struct device_node *__of_find_node_by_path(struct device_node *parent, return NULL; } +struct device_node *__of_find_node_by_full_path(struct device_node *node, + const char *path) +{ + const char *separator = strchr(path, ':'); + + while (node && *path == '/') { + struct device_node *tmp = node; + + path++; /* Increment past '/' delimiter */ + node = __of_find_node_by_path(node, path); + of_node_put(tmp); + path = strchrnul(path, '/'); + if (separator && separator < path) + break; + } + return node; +} + /** * of_find_node_opts_by_path - Find a node matching a full OF path * @path: Either the full path to match, or if the path does not @@ -839,16 +857,7 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt raw_spin_lock_irqsave(&devtree_lock, flags); if (!np) np = of_node_get(of_root); - while (np && *path == '/') { - struct device_node *tmp = np; - - path++; /* Increment past '/' delimiter */ - np = __of_find_node_by_path(np, path); - of_node_put(tmp); - path = strchrnul(path, '/'); - if (separator && separator < path) - break; - } + np =__of_find_node_by_full_path(np, path); raw_spin_unlock_irqrestore(&devtree_lock, flags); return np; } diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 4ebb0149d118..e7bbfa3e9632 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -77,6 +77,9 @@ extern void *__unflatten_device_tree(const void *blob, struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags); __printf(2, 3) struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...); +struct device_node *__of_find_node_by_full_path(struct device_node *node, + const char *path); + extern const void *__of_get_property(const struct device_node *np, const char *name, int *lenp); extern int __of_add_property(struct device_node *np, struct property *prop); diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 63626d7d9adb..b3f987c1301b 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -20,35 +20,11 @@ #include <linux/errno.h> #include <linux/slab.h> +#include "of_private.h" + /* illegal phandle value (set when unresolved) */ #define OF_PHANDLE_ILLEGAL 0xdeadbeef -/** - * Find a node with the give full name by recursively following any of - * the child node links. - */ -static struct device_node *find_node_by_full_name(struct device_node *node, - const char *full_name) -{ - struct device_node *child, *found; - - if (!node) - return NULL; - - if (!of_node_cmp(node->full_name, full_name)) - return of_node_get(node); - - for_each_child_of_node(node, child) { - found = find_node_by_full_name(child, full_name); - if (found != NULL) { - of_node_put(child); - return found; - } - } - - return NULL; -} - static phandle live_tree_max_phandle(void) { struct device_node *node; @@ -138,7 +114,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay, if (err) goto err_fail; - refnode = find_node_by_full_name(overlay, node_path); + refnode = __of_find_node_by_full_path(overlay, node_path); if (!refnode) continue; -- 2.11.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/4] of: Custom printk format specifier for device node 2017-06-14 20:30 [PATCH 0/4] DT printf format specifiers Rob Herring [not found] ` <20170614203025.7581-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-14 20:30 ` [PATCH 2/4] of: find_node_by_full_name rewrite to compare each level Rob Herring @ 2017-06-14 20:30 ` Rob Herring [not found] ` <20170614203025.7581-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-14 20:30 ` [PATCH 4/4] of: Convert to using %pOF instead of full_name Rob Herring 3 siblings, 1 reply; 19+ messages in thread From: Rob Herring @ 2017-06-14 20:30 UTC (permalink / raw) To: Frank Rowand, Mark Rutland Cc: Pantelis Antoniou, Joe Perches, devicetree, linux-kernel From: Pantelis Antoniou <pantelis.antoniou@konsulko.com> 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 %pOF\n", node); More fine-grained control of formatting includes printing the name, flag, path-spec name, reference count and others, explained in the documentation entry. Originally written by Pantelis, but pretty much rewrote the core function using existing string/number functions. The 2 passes were unnecessary and have been removed. Also, updated the checkpatch.pl check. Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com> Signed-off-by: Rob Herring <robh@kernel.org> --- Documentation/printk-formats.txt | 31 +++++++++ lib/vsprintf.c | 135 ++++++++++++++++++++++++++++++++++++++- scripts/checkpatch.pl | 2 +- 3 files changed, 166 insertions(+), 2 deletions(-) diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt index 5962949944fd..07bc088ba5f5 100644 --- a/Documentation/printk-formats.txt +++ b/Documentation/printk-formats.txt @@ -275,6 +275,37 @@ struct va_format: Passed by reference. +Device tree nodes: + + %pOF[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: + + %pOF /foo/bar@0 - Node full name + %pOFf /foo/bar@0 - Same as above + %pOFfp /foo/bar@0|10 - Node full name + phandle + %pOFfcF /foo/bar@0|foo,device|--P- - Node full name + + major compatible string + + node flags + D - dynamic + d - detached + P - Populated + B - Populated bus + + Passed by reference. + struct clk: %pC pll1 diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 2d41de3f98a1..80b3e237f8d1 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -31,6 +31,7 @@ #include <linux/dcache.h> #include <linux/cred.h> #include <linux/uuid.h> +#include <linux/of.h> #include <net/addrconf.h> #ifdef CONFIG_BLOCK #include <linux/blkdev.h> @@ -650,7 +651,7 @@ char *bdev_name(char *buf, char *end, struct block_device *bdev, struct printf_spec spec, const char *fmt) { struct gendisk *hd = bdev->bd_disk; - + buf = string(buf, end, hd->disk_name, spec); if (bdev->bd_part->partno) { if (isdigit(hd->disk_name[strlen(hd->disk_name)-1])) { @@ -1470,6 +1471,123 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) return format_flags(buf, end, flags, names); } +static noinline_for_stack +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) +{ + int len, ret; + + if (!np || !np->parent) + return buf; + + buf = device_node_gen_full_name(np->parent, buf, end); + + if (buf < end) + len = end - buf; + else + len = 0; + ret = snprintf(buf, len, "/%s", kbasename(np->full_name)); + if (ret <= 0) + return buf; + else if (len == 0 || ret < len) + return buf + ret; + return buf + len; +} + +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 ret; + char *buf_start = buf; + struct property *prop; + bool has_mult, pass; + const struct printf_spec num_spec = { + .flags = SMALL, + .field_width = -1, + .precision = -1, + .base = 10, + }; + + struct printf_spec str_spec = spec; + str_spec.field_width = -1; + + if (!IS_ENABLED(CONFIG_OF)) + return string(buf, end, "(!OF)", spec); + + if ((unsigned long)dn < PAGE_SIZE) + return string(buf, end, "(null)", spec); + + /* simple case without anything any more format specifiers */ + if (fmt[1] == '\0' || strcspn(fmt + 1,"fnpPFcCr") > 0) + fmt = "Ff"; + + for (fmtp = fmt + 1, pass = false; strspn(fmtp,"fnpPFcCr"); fmtp++, pass = true) { + if (pass && (*fmtp != 'f')) { + if (buf < end) + *buf = '|'; + buf++; + } + + switch (*fmtp) { + case 'f': /* full_name */ + if (pass) { + if (buf < end) + *buf = ':'; + buf++; + } + buf = device_node_gen_full_name(dn, buf, end); + break; + case 'n': /* name */ + buf = string(buf, end, dn->name, str_spec); + break; + case 'p': /* phandle */ + buf = number(buf, end, (unsigned int)dn->phandle, num_spec); + break; + case 'P': /* path-spec */ + buf = string(buf, end, kbasename(of_node_full_name(dn)), str_spec); + break; + case 'F': /* flags */ + 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' : '-'); + buf = string(buf, end, tbuf, str_spec); + break; + case 'c': /* major compatible string */ + ret = of_property_read_string(dn, "compatible", &p); + if (!ret) + buf = string(buf, end, p, str_spec); + break; + case 'C': /* full compatible string */ + has_mult = false; + of_property_for_each_string(dn, "compatible", prop, p) { + if (has_mult) + buf = string(buf, end, ",", str_spec); + buf = string(buf, end, "\"", str_spec); + buf = string(buf, end, p, str_spec); + buf = string(buf, end, "\"", str_spec); + + has_mult = true; + } + break; + case 'r': /* node reference count */ + buf = number(buf, end, refcount_read(&dn->kobj.kref.refcount), num_spec); + break; + default: + break; + } + } + + return widen_string(buf, buf - buf_start, end, spec); +} + int kptr_restrict __read_mostly; /* @@ -1566,6 +1684,16 @@ int kptr_restrict __read_mostly; * p page flags (see struct page) given as pointer to unsigned long * g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t * v vma flags (VM_*) given as pointer to unsigned long + * - 'OF[fnpPcCFr]' For an DT device node + * Without any optional arguments prints the full_name + * 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 * * ** Please update also Documentation/printk-formats.txt when making changes ** * @@ -1721,6 +1849,11 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, case 'G': return flags_string(buf, end, ptr, fmt); + case 'O': + switch (fmt[1]) { + case 'F': + return device_node_string(buf, end, ptr, spec, fmt + 1); + } } spec.flags |= SMALL; if (spec.field_width == -1) { diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4b9569fa931b..411f2098fa6b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5709,7 +5709,7 @@ sub process { for (my $count = $linenr; $count <= $lc; $count++) { my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; - if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGN]).)/) { + if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) { $bad_extension = $1; last; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
[parent not found: <20170614203025.7581-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 3/4] of: Custom printk format specifier for device node [not found] ` <20170614203025.7581-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-06-14 20:56 ` Joe Perches [not found] ` <1497473808.18751.70.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> 2017-06-22 20:44 ` [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree Rob Herring 1 sibling, 1 reply; 19+ messages in thread From: Joe Perches @ 2017-06-14 20:56 UTC (permalink / raw) To: Rob Herring, Frank Rowand, Mark Rutland Cc: Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Wed, 2017-06-14 at 15:30 -0500, Rob Herring wrote: > From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> I think the commit subject is wrong. It adds an "of" specific bit to vsprintf.c. The subject should be 'vsprintf: Add %p extension "%pO" for device tree' > 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 %pOF\n", node); Somehow I think this example is poor as node->full_name is a pretty obvious to read use. %pOF requires you to look up or know what the output is going to be. > More fine-grained control of formatting includes printing the name, > flag, path-spec name, reference count and others, explained in the > documentation entry. > > Originally written by Pantelis, but pretty much rewrote the core > function using existing string/number functions. The 2 passes were > unnecessary and have been removed. Also, updated the checkpatch.pl > check. Some comments about the code. > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > [] > @@ -1470,6 +1471,123 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) > return format_flags(buf, end, flags, names); > } > > +static noinline_for_stack > +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) > +{ > + int len, ret; > + > + if (!np || !np->parent) > + return buf; > + > + buf = device_node_gen_full_name(np->parent, buf, end); This is recursive. How many levels of parents could there be? Perhaps there should be a recursion limit. > + > + if (buf < end) > + len = end - buf; > + else > + len = 0; > + ret = snprintf(buf, len, "/%s", kbasename(np->full_name)); > + if (ret <= 0) > + return buf; > + else if (len == 0 || ret < len) > + return buf + ret; > + return buf + len; > +} Does this work with %p<len>OF for a right justified or padded length string? Perhaps widen_string should be added. > +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 ret; > + char *buf_start = buf; > + struct property *prop; > + bool has_mult, pass; > + const struct printf_spec num_spec = { > + .flags = SMALL, > + .field_width = -1, > + .precision = -1, > + .base = 10, > + }; > + > + struct printf_spec str_spec = spec; > + str_spec.field_width = -1; > + > + if (!IS_ENABLED(CONFIG_OF)) > + return string(buf, end, "(!OF)", spec); > + > + if ((unsigned long)dn < PAGE_SIZE) > + return string(buf, end, "(null)", spec); > + > + /* simple case without anything any more format specifiers */ > + if (fmt[1] == '\0' || strcspn(fmt + 1,"fnpPFcCr") > 0) > + fmt = "Ff"; > + > + for (fmtp = fmt + 1, pass = false; strspn(fmtp,"fnpPFcCr"); fmtp++, pass = true) { why not while (isalpha(*++fmt)) like ip6 or isalnum like FORMAT_TYPE_PTR uses? > + if (pass && (*fmtp != 'f')) { > + if (buf < end) > + *buf = '|'; > + buf++; > + } > + > + switch (*fmtp) { > + case 'f': /* full_name */ > + if (pass) { > + if (buf < end) > + *buf = ':'; > + buf++; > + } > + buf = device_node_gen_full_name(dn, buf, end); > + break; > + case 'n': /* name */ > + buf = string(buf, end, dn->name, str_spec); > + break; > + case 'p': /* phandle */ > + buf = number(buf, end, (unsigned int)dn->phandle, num_spec); > + break; > + case 'P': /* path-spec */ > + buf = string(buf, end, kbasename(of_node_full_name(dn)), str_spec); > + break; > + case 'F': /* flags */ > + 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' : '-'); I'd try to avoid all uses of snprintf as it's effectively another fairly large stack frame. It's probably better to avoid more recursion stack depth use and just use *buf++ as appropriate. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <1497473808.18751.70.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH 3/4] of: Custom printk format specifier for device node [not found] ` <1497473808.18751.70.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> @ 2017-06-15 12:30 ` Rob Herring [not found] ` <CAL_JsqJ_cc46Hf2XdZoXkgZyOh+0KXVXfeWYe1100E9vuRt12A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-06-15 21:26 ` Rob Herring 1 sibling, 1 reply; 19+ messages in thread From: Rob Herring @ 2017-06-15 12:30 UTC (permalink / raw) To: Joe Perches Cc: Frank Rowand, Mark Rutland, Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Wed, Jun 14, 2017 at 3:56 PM, Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote: > On Wed, 2017-06-14 at 15:30 -0500, Rob Herring wrote: >> From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> > > I think the commit subject is wrong. > It adds an "of" specific bit to vsprintf.c. > The subject should be > 'vsprintf: Add %p extension "%pO" for device tree' Okay, but it was good enough for the 2-3 versions Pantelis did before... >> 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 %pOF\n", node); > > Somehow I think this example is poor as node->full_name > is a pretty obvious to read use. %pOF requires you to > look up or know what the output is going to be. So %pOFfullname? We've beat this one to death IMO. > >> More fine-grained control of formatting includes printing the name, >> flag, path-spec name, reference count and others, explained in the >> documentation entry. >> >> Originally written by Pantelis, but pretty much rewrote the core >> function using existing string/number functions. The 2 passes were >> unnecessary and have been removed. Also, updated the checkpatch.pl >> check. > > Some comments about the code. > >> diff --git a/lib/vsprintf.c b/lib/vsprintf.c >> [] >> @@ -1470,6 +1471,123 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) >> return format_flags(buf, end, flags, names); >> } >> >> +static noinline_for_stack >> +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) >> +{ >> + int len, ret; >> + >> + if (!np || !np->parent) >> + return buf; >> + >> + buf = device_node_gen_full_name(np->parent, buf, end); > > This is recursive. How many levels of parents could there be? > Perhaps there should be a recursion limit. 2-6 I'd say is typical. The FDT unflattening code limits things to 64 (which is probably way more than needed). I could re-write it to be non-recursive, but then I'll just have the max sized array of pointers on the stack. > >> + >> + if (buf < end) >> + len = end - buf; >> + else >> + len = 0; >> + ret = snprintf(buf, len, "/%s", kbasename(np->full_name)); I can replace this one too with a strcat and save some stack space. >> + if (ret <= 0) >> + return buf; >> + else if (len == 0 || ret < len) >> + return buf + ret; >> + return buf + len; >> +} > > Does this work with %p<len>OF for a right justified or padded > length string? Perhaps widen_string should be added. widen_string is called at the end of device_node_string. >> +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 ret; >> + char *buf_start = buf; >> + struct property *prop; >> + bool has_mult, pass; >> + const struct printf_spec num_spec = { >> + .flags = SMALL, >> + .field_width = -1, >> + .precision = -1, >> + .base = 10, >> + }; >> + >> + struct printf_spec str_spec = spec; >> + str_spec.field_width = -1; >> + >> + if (!IS_ENABLED(CONFIG_OF)) >> + return string(buf, end, "(!OF)", spec); >> + >> + if ((unsigned long)dn < PAGE_SIZE) >> + return string(buf, end, "(null)", spec); >> + >> + /* simple case without anything any more format specifiers */ >> + if (fmt[1] == '\0' || strcspn(fmt + 1,"fnpPFcCr") > 0) >> + fmt = "Ff"; >> + >> + for (fmtp = fmt + 1, pass = false; strspn(fmtp,"fnpPFcCr"); fmtp++, pass = true) { > > why not > while (isalpha(*++fmt)) > like ip6 or isalnum like FORMAT_TYPE_PTR uses? Okay. > >> + if (pass && (*fmtp != 'f')) { >> + if (buf < end) >> + *buf = '|'; >> + buf++; >> + } >> + >> + switch (*fmtp) { >> + case 'f': /* full_name */ >> + if (pass) { >> + if (buf < end) >> + *buf = ':'; >> + buf++; >> + } >> + buf = device_node_gen_full_name(dn, buf, end); >> + break; >> + case 'n': /* name */ >> + buf = string(buf, end, dn->name, str_spec); >> + break; >> + case 'p': /* phandle */ >> + buf = number(buf, end, (unsigned int)dn->phandle, num_spec); >> + break; >> + case 'P': /* path-spec */ >> + buf = string(buf, end, kbasename(of_node_full_name(dn)), str_spec); >> + break; >> + case 'F': /* flags */ >> + 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' : '-'); > > I'd try to avoid all uses of snprintf as it's effectively > another fairly > large stack frame. Okay. > It's probably better to avoid more recursion stack depth use > and just use *buf++ as appropriate. You can't use *buf++ as this code must work and increment buf even when buf is NULL. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <CAL_JsqJ_cc46Hf2XdZoXkgZyOh+0KXVXfeWYe1100E9vuRt12A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 3/4] of: Custom printk format specifier for device node [not found] ` <CAL_JsqJ_cc46Hf2XdZoXkgZyOh+0KXVXfeWYe1100E9vuRt12A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2017-06-15 16:51 ` Joe Perches 0 siblings, 0 replies; 19+ messages in thread From: Joe Perches @ 2017-06-15 16:51 UTC (permalink / raw) To: Rob Herring Cc: Frank Rowand, Mark Rutland, Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Thu, 2017-06-15 at 07:30 -0500, Rob Herring wrote: > On Wed, Jun 14, 2017 at 3:56 PM, Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote: > > On Wed, 2017-06-14 at 15:30 -0500, Rob Herring wrote: > > > From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> > > > > I think the commit subject is wrong. > > It adds an "of" specific bit to vsprintf.c. > > The subject should be > > 'vsprintf: Add %p extension "%pO" for device tree' > > Okay, but it was good enough for the 2-3 versions Pantelis did before... Which were not applied. > > > 90% of the usage of device node's full_name is printing it out > > > in a kernel message. Preparing for the eventual delayed allocation The "eventual delayed allocation" bit doesn't mean anything to me. > > > 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 %pOF\n", node); > > > > Somehow I think this example is poor as node->full_name > > is a pretty obvious to read use. %pOF requires you to > > look up or know what the output is going to be. > > So %pOFfullname? We've beat this one to death IMO. I don't doubt the utility, just the example. Just mention that full_name is going away. > > > More fine-grained control of formatting includes printing the name, > > > flag, path-spec name, reference count and others, explained in the > > > documentation entry. > > > > > > Originally written by Pantelis, but pretty much rewrote the core > > > function using existing string/number functions. The 2 passes were > > > unnecessary and have been removed. Also, updated the checkpatch.pl > > > check. > > > > Some comments about the code. > > > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > > > [] > > > @@ -1470,6 +1471,123 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) > > > return format_flags(buf, end, flags, names); > > > } > > > > > > +static noinline_for_stack > > > +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) > > > +{ > > > + int len, ret; > > > + > > > + if (!np || !np->parent) > > > + return buf; > > > + > > > + buf = device_node_gen_full_name(np->parent, buf, end); > > > > This is recursive. How many levels of parents could there be? > > Perhaps there should be a recursion limit. > > 2-6 I'd say is typical. The FDT unflattening code limits things to 64 > (which is probably way more than needed). > > I could re-write it to be non-recursive, but then I'll just have the > max sized array of pointers on the stack. Which would be less stack than how many recursive calls? 5? In any case, 64 * 8 for pointers or 5+ stack frames is a fair amount of stack. Maybe too much. > > > + case 'F': /* flags */ > > > + 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' : '-'); > > > > I'd try to avoid all uses of snprintf as it's effectively > > another fairly large stack frame. > > Okay. > > > It's probably better to avoid more recursion stack depth use > > and just use *buf++ as appropriate. > > You can't use *buf++ as this code must work and increment buf even > when buf is NULL. tbuf then. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] of: Custom printk format specifier for device node [not found] ` <1497473808.18751.70.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> 2017-06-15 12:30 ` Rob Herring @ 2017-06-15 21:26 ` Rob Herring 2017-06-15 21:50 ` Joe Perches 1 sibling, 1 reply; 19+ messages in thread From: Rob Herring @ 2017-06-15 21:26 UTC (permalink / raw) To: Joe Perches Cc: Frank Rowand, Mark Rutland, Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Wed, Jun 14, 2017 at 01:56:48PM -0700, Joe Perches wrote: > On Wed, 2017-06-14 at 15:30 -0500, Rob Herring wrote: > > From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> > > I think the commit subject is wrong. > It adds an "of" specific bit to vsprintf.c. > The subject should be > 'vsprintf: Add %p extension "%pO" for device tree' > > > 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 %pOF\n", node); > > Somehow I think this example is poor as node->full_name > is a pretty obvious to read use. %pOF requires you to > look up or know what the output is going to be. > > > More fine-grained control of formatting includes printing the name, > > flag, path-spec name, reference count and others, explained in the > > documentation entry. > > > > Originally written by Pantelis, but pretty much rewrote the core > > function using existing string/number functions. The 2 passes were > > unnecessary and have been removed. Also, updated the checkpatch.pl > > check. > > Some comments about the code. > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > > [] > > @@ -1470,6 +1471,123 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) > > return format_flags(buf, end, flags, names); > > } > > > > +static noinline_for_stack > > +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) > > +{ > > + int len, ret; > > + > > + if (!np || !np->parent) > > + return buf; > > + > > + buf = device_node_gen_full_name(np->parent, buf, end); > > This is recursive. How many levels of parents could there be? > Perhaps there should be a recursion limit. Okay, unlike unflattening code, we can easily calculate the depth and then allocate an array on the stack. So this is what I've ended up with: static int device_node_calc_depth(const struct device_node *np) { int d; for (d = 0; np; d++) np = np->parent; return d; } static noinline_for_stack char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) { int i; int depth = device_node_calc_depth(np); const struct device_node *nodes[depth]; const struct printf_spec strspec = { .field_width = -1, .precision = -1, }; if (!depth) return buf; /* special case for root node */ if (depth == 1) return string(buf, end, "/", strspec); depth--; for (i = depth - 1; i >= 0; i--) { nodes[i] = np; np = np->parent; } for (i = 0; i < depth; i++) { buf = string(buf, end, "/", strspec); buf = string(buf, end, kbasename(nodes[i]->full_name), strspec); } return buf; } > > + /* simple case without anything any more format specifiers */ > > + if (fmt[1] == '\0' || strcspn(fmt + 1,"fnpPFcCr") > 0) > > + fmt = "Ff"; > > + > > + for (fmtp = fmt + 1, pass = false; strspn(fmtp,"fnpPFcCr"); fmtp++, pass = true) { > > why not > while (isalpha(*++fmt)) > like ip6 or isalnum like FORMAT_TYPE_PTR uses? This case is more complicated with the field separators. If we have something like %pOFfxyz where xyz are not valid format specifiers, we'd end up with extra field separators. I did simplify things a bit and got rid of fmtp and just use fmt instead. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] of: Custom printk format specifier for device node 2017-06-15 21:26 ` Rob Herring @ 2017-06-15 21:50 ` Joe Perches 0 siblings, 0 replies; 19+ messages in thread From: Joe Perches @ 2017-06-15 21:50 UTC (permalink / raw) To: Rob Herring Cc: Frank Rowand, Mark Rutland, Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thu, 2017-06-15 at 16:26 -0500, Rob Herring wrote: > On Wed, Jun 14, 2017 at 01:56:48PM -0700, Joe Perches wrote: > > On Wed, 2017-06-14 at 15:30 -0500, Rob Herring wrote: > > > From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> [] > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > > > [] > > > @@ -1470,6 +1471,123 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) > > > return format_flags(buf, end, flags, names); > > > } > > > > > > +static noinline_for_stack > > > +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) > > > +{ > > > + int len, ret; > > > + > > > + if (!np || !np->parent) > > > + return buf; > > > + > > > + buf = device_node_gen_full_name(np->parent, buf, end); > > > > This is recursive. How many levels of parents could there be? > > Perhaps there should be a recursion limit. > > Okay, unlike unflattening code, we can easily calculate the depth and > then allocate an array on the stack. So this is what I've ended up > with: > > static int device_node_calc_depth(const struct device_node *np) > { > int d; > > for (d = 0; np; d++) > np = np->parent; > > return d; > } > > static noinline_for_stack > char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) > { > int i; > int depth = device_node_calc_depth(np); > const struct device_node *nodes[depth]; > const struct printf_spec strspec = { > .field_width = -1, > .precision = -1, > }; static const struct printf_spec strspec = { etc... and please move strspec above *nodes > > if (!depth) > return buf; > /* special case for root node */ > if (depth == 1) > return string(buf, end, "/", strspec); > > depth--; > for (i = depth - 1; i >= 0; i--) { > nodes[i] = np; > np = np->parent; > } > for (i = 0; i < depth; i++) { > buf = string(buf, end, "/", strspec); > buf = string(buf, end, kbasename(nodes[i]->full_name), strspec); > } > return buf; > } -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree [not found] ` <20170614203025.7581-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-14 20:56 ` Joe Perches @ 2017-06-22 20:44 ` Rob Herring 2017-06-22 22:44 ` Randy Dunlap ` (2 more replies) 1 sibling, 3 replies; 19+ messages in thread From: Rob Herring @ 2017-06-22 20:44 UTC (permalink / raw) To: Grant Likely, Frank Rowand, Mark Rutland, Joe Perches Cc: Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> 90% of the usage of device node's full_name is printing it out in a kernel message. However, storing the full path for every node is wasteful and redundant. With a custom format specifier, we can generate the full path at run-time and eventually remove the full path from every node. For instance typical use is: pr_info("Frobbing node %s\n", node->full_name); Which can be written now as: pr_info("Frobbing node %pOF\n", node); More fine-grained control of formatting includes printing the name, flags, path-spec name and others, explained in the documentation entry. Originally written by Pantelis, but pretty much rewrote the core function using existing string/number functions. The 2 passes were unnecessary and have been removed. Also, updated the checkpatch.pl check. The unittest code was written by Grant Likely. Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> --- I missed some comments and changes Grant had done and incorporated them. v2: - Change subject - Rewrite device_node_gen_full_name() to avoid recursion. - Avoid using sprintf. - Add unittests Grant L. wrote. - Drop ref count printing (from discussion 2 years ago). - Remove fmtp local var. Documentation/printk-formats.txt | 30 ++++++ drivers/of/unittest-data/tests-platform.dtsi | 4 +- drivers/of/unittest.c | 58 +++++++++++ lib/vsprintf.c | 140 +++++++++++++++++++++++++++ scripts/checkpatch.pl | 2 +- 5 files changed, 232 insertions(+), 2 deletions(-) diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt index 5962949944fd..c7af38188f12 100644 --- a/Documentation/printk-formats.txt +++ b/Documentation/printk-formats.txt @@ -275,6 +275,36 @@ struct va_format: Passed by reference. +Device tree nodes: + + %pOn[fnpPcCF] + + 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 + Without any arguments prints full_name (same as %pOFf) + The separator when using multiple arguments is ':' + + Examples: + + %pOn /foo/bar@0 - Node full name + %pOnf /foo/bar@0 - Same as above + %pOnfp /foo/bar@0:10 - Node full name + phandle + %pOnfcF /foo/bar@0:foo,device:--P- - Node full name + + major compatible string + + node flags + D - dynamic + d - detached + P - Populated + B - Populated bus + + Passed by reference. + struct clk: %pC pll1 diff --git a/drivers/of/unittest-data/tests-platform.dtsi b/drivers/of/unittest-data/tests-platform.dtsi index eb20eeb2b062..a0c93822aee3 100644 --- a/drivers/of/unittest-data/tests-platform.dtsi +++ b/drivers/of/unittest-data/tests-platform.dtsi @@ -26,7 +26,9 @@ #size-cells = <0>; dev@100 { - compatible = "test-sub-device"; + compatible = "test-sub-device", + "test-compat2", + "test-compat3"; reg = <0x100>; }; }; diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 987a1530282a..8f611e9844db 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -239,6 +239,63 @@ static void __init of_unittest_check_tree_linkage(void) pr_debug("allnodes list size (%i); sibling lists size (%i)\n", allnode_count, child_count); } +static void __init of_unittest_printf_one(struct device_node *np, const char *fmt, + const char *expected) +{ + char buf[strlen(expected)+10]; + int size, i; + + /* Baseline; check conversion with a large size limit */ + memset(buf, 0xff, sizeof(buf)); + size = snprintf(buf, sizeof(buf) - 2, fmt, np); + + /* use strcmp() instead of strncmp() here to be absolutely sure strings match */ + unittest((strcmp(buf, expected) == 0) && (buf[size+1] == 0xff), + "sprintf failed; fmt='%s' expected='%s' rslt='%s'\n", + fmt, expected, buf); + + /* Make sure length limits work */ + size++; + for (i = 0; i < 2; i++, size--) { + /* Clear the buffer, and make sure it works correctly still */ + memset(buf, 0xff, sizeof(buf)); + snprintf(buf, size+1, fmt, np); + unittest(strncmp(buf, expected, size) == 0 && (buf[size+1] == 0xff), + "snprintf failed; size=%i fmt='%s' expected='%s' rslt='%s'\n", + size, fmt, expected, buf); + } +} + +static void __init of_unittest_printf(void) +{ + struct device_node *np; + const char *full_name = "/testcase-data/platform-tests/test-device@1/dev@100"; + char phandle_str[16] = ""; + + np = of_find_node_by_path(full_name); + if (!np) { + unittest(np, "testcase data missing\n"); + return; + } + + num_to_str(phandle_str, sizeof(phandle_str), np->phandle); + + of_unittest_printf_one(np, "%pOF", full_name); + of_unittest_printf_one(np, "%pOFf", full_name); + of_unittest_printf_one(np, "%pOFp", phandle_str); + of_unittest_printf_one(np, "%pOFP", "dev@100"); + of_unittest_printf_one(np, "ABC %pOFP ABC", "ABC dev@100 ABC"); + of_unittest_printf_one(np, "%10pOFP", " dev@100"); + of_unittest_printf_one(np, "%-10pOFP", "dev@100 "); + of_unittest_printf_one(of_root, "%pOFP", "/"); + of_unittest_printf_one(np, "%pOFF", "----"); + of_unittest_printf_one(np, "%pOFPF", "dev@100:----"); + of_unittest_printf_one(np, "%pOFPFPc", "dev@100:----:dev@100:test-sub-device"); + of_unittest_printf_one(np, "%pOFc", "test-sub-device"); + of_unittest_printf_one(np, "%pOFC", + "\"test-sub-device\",\"test-compat2\",\"test-compat3\""); +} + struct node_hash { struct hlist_node node; struct device_node *np; @@ -2269,6 +2326,7 @@ static int __init of_unittest(void) of_unittest_find_node_by_name(); of_unittest_dynamic(); of_unittest_parse_phandle_with_args(); + of_unittest_printf(); of_unittest_property_string(); of_unittest_property_copy(); of_unittest_changeset(); diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 2d41de3f98a1..2343b2ca47c5 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -31,6 +31,7 @@ #include <linux/dcache.h> #include <linux/cred.h> #include <linux/uuid.h> +#include <linux/of.h> #include <net/addrconf.h> #ifdef CONFIG_BLOCK #include <linux/blkdev.h> @@ -1470,6 +1471,131 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) return format_flags(buf, end, flags, names); } +static int device_node_calc_depth(const struct device_node *np) +{ + int d; + + for (d = 0; np; d++) + np = np->parent; + + return d; +} + +static noinline_for_stack +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) +{ + int i; + int depth = device_node_calc_depth(np); + static const struct printf_spec strspec = { + .field_width = -1, + .precision = -1, + }; + const struct device_node *nodes[depth]; + + if (!depth) + return buf; + /* special case for root node */ + if (depth == 1) + return string(buf, end, "/", strspec); + + depth--; + for (i = depth - 1; i >= 0; i--) { + nodes[i] = np; + np = np->parent; + } + for (i = 0; i < depth; i++) { + buf = string(buf, end, "/", strspec); + buf = string(buf, end, kbasename(nodes[i]->full_name), strspec); + } + return buf; +} + +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("xxxx") + 1]; + const char *p; + int ret; + char *buf_start = buf; + struct property *prop; + bool has_mult, pass; + static const struct printf_spec num_spec = { + .flags = SMALL, + .field_width = -1, + .precision = -1, + .base = 10, + }; + + struct printf_spec str_spec = spec; + str_spec.field_width = -1; + + if (!IS_ENABLED(CONFIG_OF)) + return string(buf, end, "(!OF)", spec); + + if ((unsigned long)dn < PAGE_SIZE) + return string(buf, end, "(null)", spec); + + /* simple case without anything any more format specifiers */ + fmt++; + if (fmt[0] == '\0' || strcspn(fmt,"fnpPFcC") > 0) + fmt = "f"; + + for (pass = false; strspn(fmt,"fnpPFcC"); fmt++, pass = true) { + if (pass) { + if (buf < end) + *buf = ':'; + buf++; + } + + switch (*fmt) { + case 'f': /* full_name */ + buf = device_node_gen_full_name(dn, buf, end); + break; + case 'n': /* name */ + buf = string(buf, end, dn->name, str_spec); + break; + case 'p': /* phandle */ + buf = number(buf, end, (unsigned int)dn->phandle, num_spec); + break; + case 'P': /* path-spec */ + p = kbasename(of_node_full_name(dn)); + if (!p[1]) + p = "/"; + buf = string(buf, end, p, str_spec); + break; + case 'F': /* flags */ + tbuf[0] = of_node_check_flag(dn, OF_DYNAMIC) ? 'D' : '-'; + tbuf[1] = of_node_check_flag(dn, OF_DETACHED) ? 'd' : '-'; + tbuf[2] = of_node_check_flag(dn, OF_POPULATED) ? 'P' : '-'; + tbuf[3] = of_node_check_flag(dn, OF_POPULATED_BUS) ? 'B' : '-'; + buf = string(buf, end, tbuf, str_spec); + break; + case 'c': /* major compatible string */ + ret = of_property_read_string(dn, "compatible", &p); + if (!ret) + buf = string(buf, end, p, str_spec); + break; + case 'C': /* full compatible string */ + has_mult = false; + of_property_for_each_string(dn, "compatible", prop, p) { + if (has_mult) + buf = string(buf, end, ",", str_spec); + buf = string(buf, end, "\"", str_spec); + buf = string(buf, end, p, str_spec); + buf = string(buf, end, "\"", str_spec); + + has_mult = true; + } + break; + default: + break; + } + } + + return widen_string(buf, buf - buf_start, end, spec); +} + int kptr_restrict __read_mostly; /* @@ -1566,6 +1692,15 @@ int kptr_restrict __read_mostly; * p page flags (see struct page) given as pointer to unsigned long * g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t * v vma flags (VM_*) given as pointer to unsigned long + * - 'OF[fnpPcCF]' For a device tree object + * Without any optional arguments prints the full_name + * 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 * * ** Please update also Documentation/printk-formats.txt when making changes ** * @@ -1721,6 +1856,11 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, case 'G': return flags_string(buf, end, ptr, fmt); + case 'O': + switch (fmt[1]) { + case 'F': + return device_node_string(buf, end, ptr, spec, fmt + 1); + } } spec.flags |= SMALL; if (spec.field_width == -1) { diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4b9569fa931b..411f2098fa6b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5709,7 +5709,7 @@ sub process { for (my $count = $linenr; $count <= $lc; $count++) { my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; - if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGN]).)/) { + if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) { $bad_extension = $1; last; } -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree 2017-06-22 20:44 ` [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree Rob Herring @ 2017-06-22 22:44 ` Randy Dunlap [not found] ` <d22444aa-39da-ec74-42a1-63f1fa40c7d3-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> [not found] ` <20170622204445.14930-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-23 17:30 ` [PATCH v3] " Rob Herring 2 siblings, 1 reply; 19+ messages in thread From: Randy Dunlap @ 2017-06-22 22:44 UTC (permalink / raw) To: Rob Herring, Grant Likely, Frank Rowand, Mark Rutland, Joe Perches Cc: Pantelis Antoniou, devicetree, linux-kernel On 06/22/2017 01:44 PM, Rob Herring wrote: > From: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > > 90% of the usage of device node's full_name is printing it out in a > kernel message. However, storing the full path for every node is > wasteful and redundant. With a custom format specifier, we can generate > the full path at run-time and eventually remove the full path from every > node. > > For instance typical use is: > pr_info("Frobbing node %s\n", node->full_name); > > Which can be written now as: > pr_info("Frobbing node %pOF\n", node); isn't OF for flags -- and Of for full name? Typo or a change in the last 2 years? > More fine-grained control of formatting includes printing the name, > flags, path-spec name and others, explained in the documentation entry. > > Originally written by Pantelis, but pretty much rewrote the core > function using existing string/number functions. The 2 passes were > unnecessary and have been removed. Also, updated the checkpatch.pl > check. The unittest code was written by Grant Likely. > > Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com> > Signed-off-by: Rob Herring <robh@kernel.org> > --- > I missed some comments and changes Grant had done and incorporated them. > > v2: > - Change subject > - Rewrite device_node_gen_full_name() to avoid recursion. > - Avoid using sprintf. > - Add unittests Grant L. wrote. > - Drop ref count printing (from discussion 2 years ago). > - Remove fmtp local var. > > > Documentation/printk-formats.txt | 30 ++++++ > drivers/of/unittest-data/tests-platform.dtsi | 4 +- > drivers/of/unittest.c | 58 +++++++++++ > lib/vsprintf.c | 140 +++++++++++++++++++++++++++ > scripts/checkpatch.pl | 2 +- > 5 files changed, 232 insertions(+), 2 deletions(-) > > diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt > index 5962949944fd..c7af38188f12 100644 > --- a/Documentation/printk-formats.txt > +++ b/Documentation/printk-formats.txt > @@ -275,6 +275,36 @@ struct va_format: > > Passed by reference. > > +Device tree nodes: > + > + %pOn[fnpPcCF] > + > + 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 > + Without any arguments prints full_name (same as %pOFf) > + The separator when using multiple arguments is ':' > + > + Examples: > + > + %pOn /foo/bar@0 - Node full name > + %pOnf /foo/bar@0 - Same as above > + %pOnfp /foo/bar@0:10 - Node full name + phandle > + %pOnfcF /foo/bar@0:foo,device:--P- - Node full name + > + major compatible string + > + node flags > + D - dynamic > + d - detached > + P - Populated > + B - Populated bus > + > + Passed by reference. > + > struct clk: > > %pC pll1 -- ~Randy ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <d22444aa-39da-ec74-42a1-63f1fa40c7d3-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]
* Re: [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree [not found] ` <d22444aa-39da-ec74-42a1-63f1fa40c7d3-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> @ 2017-06-23 14:08 ` Rob Herring 0 siblings, 0 replies; 19+ messages in thread From: Rob Herring @ 2017-06-23 14:08 UTC (permalink / raw) To: Randy Dunlap Cc: Grant Likely, Frank Rowand, Mark Rutland, Joe Perches, Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Thu, Jun 22, 2017 at 5:44 PM, Randy Dunlap <rdunlap-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote: > On 06/22/2017 01:44 PM, Rob Herring wrote: >> From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> >> >> 90% of the usage of device node's full_name is printing it out in a >> kernel message. However, storing the full path for every node is >> wasteful and redundant. With a custom format specifier, we can generate >> the full path at run-time and eventually remove the full path from every >> node. >> >> For instance typical use is: >> pr_info("Frobbing node %s\n", node->full_name); >> >> Which can be written now as: >> pr_info("Frobbing node %pOF\n", node); > > > isn't OF for flags -- and Of for full name? > Typo or a change in the last 2 years? It changed in the discussion 2 years ago. The intent is: %pO - kobj %pOF - device_node, defaulting to full name There's no support of base kobj's, but I guess I should make that intent clear in the documentation. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <20170622204445.14930-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree [not found] ` <20170622204445.14930-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-06-23 3:01 ` Joe Perches [not found] ` <1498186912.24295.9.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 19+ messages in thread From: Joe Perches @ 2017-06-23 3:01 UTC (permalink / raw) To: Rob Herring, Grant Likely, Frank Rowand, Mark Rutland Cc: Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thu, 2017-06-22 at 15:44 -0500, Rob Herring wrote: > From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> > > 90% of the usage of device node's full_name is printing it out in a > kernel message. However, storing the full path for every node is > wasteful and redundant. With a custom format specifier, we can generate > the full path at run-time and eventually remove the full path from every > node. > > For instance typical use is: > pr_info("Frobbing node %s\n", node->full_name); > > Which can be written now as: > pr_info("Frobbing node %pOF\n", node); I still think this should use another identifier like %pO for object then another letter for type of object maybe N for node. And F is flags, f is name > diff --git a/lib/vsprintf.c b/lib/vsprintf.c [] > @@ -1470,6 +1471,131 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) > return format_flags(buf, end, flags, names); > } > > +static int device_node_calc_depth(const struct device_node *np) > +{ > + int d; > + > + for (d = 0; np; d++) > + np = np->parent; > + > + return d; > +} > + > +static noinline_for_stack > +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) > +{ > + int i; > + int depth = device_node_calc_depth(np); > + static const struct printf_spec strspec = { > + .field_width = -1, > + .precision = -1, > + }; > + const struct device_node *nodes[depth]; > + > + if (!depth) > + returnuf; > + /* special case for root node */ > + if (depth == 1) > + return string(buf, end, "/", strspec); > + > + depth--; > + for (i = depth - 1; i >= 0; i--) { > + nodes[i] = np; > + np = np->parent; > + } > + for (i = 0; i < depth; i++) { > + buf = string(buf, end, "/", strspec); > + buf = string(buf, end, kbasename(nodes[i]->full_name), strspec); > + } > + return buf; > +} I think there should be another function to get a particular struct device_node * at a particular depth to avoid all the pointers on stack. Basically, adapt the for (i = depth - 1 loop to a function that returns nodes[i] -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <1498186912.24295.9.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree [not found] ` <1498186912.24295.9.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> @ 2017-06-23 14:13 ` Rob Herring 0 siblings, 0 replies; 19+ messages in thread From: Rob Herring @ 2017-06-23 14:13 UTC (permalink / raw) To: Joe Perches Cc: Grant Likely, Frank Rowand, Mark Rutland, Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Thu, Jun 22, 2017 at 10:01 PM, Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote: > On Thu, 2017-06-22 at 15:44 -0500, Rob Herring wrote: >> From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> >> >> 90% of the usage of device node's full_name is printing it out in a >> kernel message. However, storing the full path for every node is >> wasteful and redundant. With a custom format specifier, we can generate >> the full path at run-time and eventually remove the full path from every >> node. >> >> For instance typical use is: >> pr_info("Frobbing node %s\n", node->full_name); >> >> Which can be written now as: >> pr_info("Frobbing node %pOF\n", node); > > I still think this should use another identifier like > %pO for object then another letter for type of object > maybe N for node. It is. O is for kobj and F is for device node. It doesn't make a much sense separately, but OF together makes sense as Open Firmware. > > And F is flags, f is name > >> diff --git a/lib/vsprintf.c b/lib/vsprintf.c > [] >> @@ -1470,6 +1471,131 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) >> return format_flags(buf, end, flags, names); >> } >> >> +static int device_node_calc_depth(const struct device_node *np) >> +{ >> + int d; >> + >> + for (d = 0; np; d++) >> + np = np->parent; >> + >> + return d; >> +} >> + >> +static noinline_for_stack >> +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) >> +{ >> + int i; >> + int depth = device_node_calc_depth(np); >> + static const struct printf_spec strspec = { >> + .field_width = -1, >> + .precision = -1, >> + }; >> + const struct device_node *nodes[depth]; >> + >> + if (!depth) >> + returnuf; >> + /* special case for root node */ >> + if (depth == 1) >> + return string(buf, end, "/", strspec); >> + >> + depth--; >> + for (i = depth - 1; i >= 0; i--) { >> + nodes[i] = np; >> + np = np->parent; >> + } >> + for (i = 0; i < depth; i++) { >> + buf = string(buf, end, "/", strspec); >> + buf = string(buf, end, kbasename(nodes[i]->full_name), strspec); >> + } >> + return buf; >> +} > > I think there should be another function to get > a particular struct device_node * at a particular > depth to avoid all the pointers on stack. > > Basically, adapt the for (i = depth - 1 loop to > a function that returns nodes[i] Okay. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3] vsprintf: Add %p extension "%pOF" for device tree 2017-06-22 20:44 ` [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree Rob Herring 2017-06-22 22:44 ` Randy Dunlap [not found] ` <20170622204445.14930-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-06-23 17:30 ` Rob Herring [not found] ` <20170623173053.636-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2 siblings, 1 reply; 19+ messages in thread From: Rob Herring @ 2017-06-23 17:30 UTC (permalink / raw) To: Frank Rowand, Mark Rutland, Joe Perches, Randy Dunlap, Grant Likely Cc: Pantelis Antoniou, devicetree, linux-kernel From: Pantelis Antoniou <pantelis.antoniou@konsulko.com> 90% of the usage of device node's full_name is printing it out in a kernel message. However, storing the full path for every node is wasteful and redundant. With a custom format specifier, we can generate the full path at run-time and eventually remove the full path from every node. For instance typical use is: pr_info("Frobbing node %s\n", node->full_name); Which can be written now as: pr_info("Frobbing node %pOF\n", node); '%pO' is the base specifier to represent kobjects with '%pOF' representing struct device_node. Currently, struct device_node is the only supported type of kobject. More fine-grained control of formatting includes printing the name, flags, path-spec name and others, explained in the documentation entry. Originally written by Pantelis, but pretty much rewrote the core function using existing string/number functions. The 2 passes were unnecessary and have been removed. Also, updated the checkpatch.pl check. The unittest code was written by Grant Likely. Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com> Signed-off-by: Rob Herring <robh@kernel.org> --- v3: - Fix missing documentation updates using '%pOF' as the device_node specifier. - Update the commit msg and documentation to clearly define '%pO' is the base specifier for kobjects. - Rework device_node_gen_full_name() to avoid creating an array of node pointers on the stack. v2: - Change subject - Rewrite device_node_gen_full_name() to avoid recursion. - Avoid using sprintf. - Add unittests Grant L. wrote. - Drop ref count printing (from discussion 2 years ago). - Remove fmtp local var. Documentation/printk-formats.txt | 36 +++++++ drivers/of/unittest-data/tests-platform.dtsi | 4 +- drivers/of/unittest.c | 58 ++++++++++++ lib/vsprintf.c | 135 +++++++++++++++++++++++++++ scripts/checkpatch.pl | 2 +- 5 files changed, 233 insertions(+), 2 deletions(-) diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt index 5962949944fd..619cdffa5d44 100644 --- a/Documentation/printk-formats.txt +++ b/Documentation/printk-formats.txt @@ -275,6 +275,42 @@ struct va_format: Passed by reference. +kobjects: + %pO + + Base specifier for kobject based structs. Must be followed with + character for specific type of kobject as listed below: + + Device tree nodes: + + %pOF[fnpPcCF] + + 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 + Without any arguments prints full_name (same as %pOFf) + The separator when using multiple arguments is ':' + + Examples: + + %pOF /foo/bar@0 - Node full name + %pOFf /foo/bar@0 - Same as above + %pOFfp /foo/bar@0:10 - Node full name + phandle + %pOFfcF /foo/bar@0:foo,device:--P- - Node full name + + major compatible string + + node flags + D - dynamic + d - detached + P - Populated + B - Populated bus + + Passed by reference. + struct clk: %pC pll1 diff --git a/drivers/of/unittest-data/tests-platform.dtsi b/drivers/of/unittest-data/tests-platform.dtsi index eb20eeb2b062..a0c93822aee3 100644 --- a/drivers/of/unittest-data/tests-platform.dtsi +++ b/drivers/of/unittest-data/tests-platform.dtsi @@ -26,7 +26,9 @@ #size-cells = <0>; dev@100 { - compatible = "test-sub-device"; + compatible = "test-sub-device", + "test-compat2", + "test-compat3"; reg = <0x100>; }; }; diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 987a1530282a..8f611e9844db 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -239,6 +239,63 @@ static void __init of_unittest_check_tree_linkage(void) pr_debug("allnodes list size (%i); sibling lists size (%i)\n", allnode_count, child_count); } +static void __init of_unittest_printf_one(struct device_node *np, const char *fmt, + const char *expected) +{ + char buf[strlen(expected)+10]; + int size, i; + + /* Baseline; check conversion with a large size limit */ + memset(buf, 0xff, sizeof(buf)); + size = snprintf(buf, sizeof(buf) - 2, fmt, np); + + /* use strcmp() instead of strncmp() here to be absolutely sure strings match */ + unittest((strcmp(buf, expected) == 0) && (buf[size+1] == 0xff), + "sprintf failed; fmt='%s' expected='%s' rslt='%s'\n", + fmt, expected, buf); + + /* Make sure length limits work */ + size++; + for (i = 0; i < 2; i++, size--) { + /* Clear the buffer, and make sure it works correctly still */ + memset(buf, 0xff, sizeof(buf)); + snprintf(buf, size+1, fmt, np); + unittest(strncmp(buf, expected, size) == 0 && (buf[size+1] == 0xff), + "snprintf failed; size=%i fmt='%s' expected='%s' rslt='%s'\n", + size, fmt, expected, buf); + } +} + +static void __init of_unittest_printf(void) +{ + struct device_node *np; + const char *full_name = "/testcase-data/platform-tests/test-device@1/dev@100"; + char phandle_str[16] = ""; + + np = of_find_node_by_path(full_name); + if (!np) { + unittest(np, "testcase data missing\n"); + return; + } + + num_to_str(phandle_str, sizeof(phandle_str), np->phandle); + + of_unittest_printf_one(np, "%pOF", full_name); + of_unittest_printf_one(np, "%pOFf", full_name); + of_unittest_printf_one(np, "%pOFp", phandle_str); + of_unittest_printf_one(np, "%pOFP", "dev@100"); + of_unittest_printf_one(np, "ABC %pOFP ABC", "ABC dev@100 ABC"); + of_unittest_printf_one(np, "%10pOFP", " dev@100"); + of_unittest_printf_one(np, "%-10pOFP", "dev@100 "); + of_unittest_printf_one(of_root, "%pOFP", "/"); + of_unittest_printf_one(np, "%pOFF", "----"); + of_unittest_printf_one(np, "%pOFPF", "dev@100:----"); + of_unittest_printf_one(np, "%pOFPFPc", "dev@100:----:dev@100:test-sub-device"); + of_unittest_printf_one(np, "%pOFc", "test-sub-device"); + of_unittest_printf_one(np, "%pOFC", + "\"test-sub-device\",\"test-compat2\",\"test-compat3\""); +} + struct node_hash { struct hlist_node node; struct device_node *np; @@ -2269,6 +2326,7 @@ static int __init of_unittest(void) of_unittest_find_node_by_name(); of_unittest_dynamic(); of_unittest_parse_phandle_with_args(); + of_unittest_printf(); of_unittest_property_string(); of_unittest_property_copy(); of_unittest_changeset(); diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 2d41de3f98a1..9e2e9306fd24 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -31,6 +31,7 @@ #include <linux/dcache.h> #include <linux/cred.h> #include <linux/uuid.h> +#include <linux/of.h> #include <net/addrconf.h> #ifdef CONFIG_BLOCK #include <linux/blkdev.h> @@ -1470,6 +1471,125 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt) return format_flags(buf, end, flags, names); } +static const char *device_node_name_for_depth(const struct device_node *np, int depth) +{ + for ( ; np && depth; depth--) + np = np->parent; + + return kbasename(np->full_name); +} + +static noinline_for_stack +char *device_node_gen_full_name(const struct device_node *np, char *buf, char *end) +{ + int depth; + const struct device_node *parent = np->parent; + static const struct printf_spec strspec = { + .field_width = -1, + .precision = -1, + }; + + /* special case for root node */ + if (!parent) + return string(buf, end, "/", strspec); + + for (depth = 0; parent->parent; depth++) + parent = parent->parent; + + for ( ; depth >= 0; depth--) { + buf = string(buf, end, "/", strspec); + buf = string(buf, end, device_node_name_for_depth(np, depth), + strspec); + } + return buf; +} + +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("xxxx") + 1]; + const char *p; + int ret; + char *buf_start = buf; + struct property *prop; + bool has_mult, pass; + static const struct printf_spec num_spec = { + .flags = SMALL, + .field_width = -1, + .precision = -1, + .base = 10, + }; + + struct printf_spec str_spec = spec; + str_spec.field_width = -1; + + if (!IS_ENABLED(CONFIG_OF)) + return string(buf, end, "(!OF)", spec); + + if ((unsigned long)dn < PAGE_SIZE) + return string(buf, end, "(null)", spec); + + /* simple case without anything any more format specifiers */ + fmt++; + if (fmt[0] == '\0' || strcspn(fmt,"fnpPFcC") > 0) + fmt = "f"; + + for (pass = false; strspn(fmt,"fnpPFcC"); fmt++, pass = true) { + if (pass) { + if (buf < end) + *buf = ':'; + buf++; + } + + switch (*fmt) { + case 'f': /* full_name */ + buf = device_node_gen_full_name(dn, buf, end); + break; + case 'n': /* name */ + buf = string(buf, end, dn->name, str_spec); + break; + case 'p': /* phandle */ + buf = number(buf, end, (unsigned int)dn->phandle, num_spec); + break; + case 'P': /* path-spec */ + p = kbasename(of_node_full_name(dn)); + if (!p[1]) + p = "/"; + buf = string(buf, end, p, str_spec); + break; + case 'F': /* flags */ + tbuf[0] = of_node_check_flag(dn, OF_DYNAMIC) ? 'D' : '-'; + tbuf[1] = of_node_check_flag(dn, OF_DETACHED) ? 'd' : '-'; + tbuf[2] = of_node_check_flag(dn, OF_POPULATED) ? 'P' : '-'; + tbuf[3] = of_node_check_flag(dn, OF_POPULATED_BUS) ? 'B' : '-'; + buf = string(buf, end, tbuf, str_spec); + break; + case 'c': /* major compatible string */ + ret = of_property_read_string(dn, "compatible", &p); + if (!ret) + buf = string(buf, end, p, str_spec); + break; + case 'C': /* full compatible string */ + has_mult = false; + of_property_for_each_string(dn, "compatible", prop, p) { + if (has_mult) + buf = string(buf, end, ",", str_spec); + buf = string(buf, end, "\"", str_spec); + buf = string(buf, end, p, str_spec); + buf = string(buf, end, "\"", str_spec); + + has_mult = true; + } + break; + default: + break; + } + } + + return widen_string(buf, buf - buf_start, end, spec); +} + int kptr_restrict __read_mostly; /* @@ -1566,6 +1686,16 @@ int kptr_restrict __read_mostly; * p page flags (see struct page) given as pointer to unsigned long * g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t * v vma flags (VM_*) given as pointer to unsigned long + * - 'O' For a kobject based struct. Must be one of the following: + * - 'OF[fnpPcCF]' For a device tree object + * Without any optional arguments prints the full_name + * 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 * * ** Please update also Documentation/printk-formats.txt when making changes ** * @@ -1721,6 +1851,11 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, case 'G': return flags_string(buf, end, ptr, fmt); + case 'O': + switch (fmt[1]) { + case 'F': + return device_node_string(buf, end, ptr, spec, fmt + 1); + } } spec.flags |= SMALL; if (spec.field_width == -1) { diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4b9569fa931b..411f2098fa6b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5709,7 +5709,7 @@ sub process { for (my $count = $linenr; $count <= $lc; $count++) { my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; - if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGN]).)/) { + if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) { $bad_extension = $1; last; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
[parent not found: <20170623173053.636-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH v3] vsprintf: Add %p extension "%pOF" for device tree [not found] ` <20170623173053.636-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-06-23 17:38 ` Joe Perches 0 siblings, 0 replies; 19+ messages in thread From: Joe Perches @ 2017-06-23 17:38 UTC (permalink / raw) To: Rob Herring, Frank Rowand, Mark Rutland, Randy Dunlap, Grant Likely Cc: Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Fri, 2017-06-23 at 12:30 -0500, Rob Herring wrote: > From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> > > 90% of the usage of device node's full_name is printing it out in a > kernel message. However, storing the full path for every node is > wasteful and redundant. With a custom format specifier, we can generate > the full path at run-time and eventually remove the full path from every > node. > > For instance typical use is: > pr_info("Frobbing node %s\n", node->full_name); > > Which can be written now as: > pr_info("Frobbing node %pOF\n", node); > > '%pO' is the base specifier to represent kobjects with '%pOF' > representing struct device_node. Currently, struct device_node is the > only supported type of kobject. > > More fine-grained control of formatting includes printing the name, > flags, path-spec name and others, explained in the documentation entry. > > Originally written by Pantelis, but pretty much rewrote the core > function using existing string/number functions. The 2 passes were > unnecessary and have been removed. Also, updated the checkpatch.pl > check. The unittest code was written by Grant Likely. > > Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > --- > v3: > - Fix missing documentation updates using '%pOF' as the device_node > specifier. > - Update the commit msg and documentation to clearly define '%pO' is the > base specifier for kobjects. > - Rework device_node_gen_full_name() to avoid creating an array of node > pointers on the stack. Thanks Rob. This all seems sensible to me now. If you want: Acked-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/4] of: Convert to using %pOF instead of full_name 2017-06-14 20:30 [PATCH 0/4] DT printf format specifiers Rob Herring ` (2 preceding siblings ...) 2017-06-14 20:30 ` [PATCH 3/4] of: Custom printk format specifier for device node Rob Herring @ 2017-06-14 20:30 ` Rob Herring [not found] ` <20170614203025.7581-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 3 siblings, 1 reply; 19+ messages in thread From: Rob Herring @ 2017-06-14 20:30 UTC (permalink / raw) To: Frank Rowand, Mark Rutland Cc: Pantelis Antoniou, Joe Perches, devicetree, linux-kernel Now that we have a custom printf format specifier, convert users of full_name to use %pOF instead. This is preparation to remove storing of the full path string for each node. Signed-off-by: Rob Herring <robh@kernel.org> --- drivers/of/address.c | 21 ++++++++-------- drivers/of/base.c | 42 ++++++++++++++++---------------- drivers/of/device.c | 2 +- drivers/of/dynamic.c | 33 +++++++++++++------------- drivers/of/irq.c | 10 ++++---- drivers/of/of_mdio.c | 10 ++++---- drivers/of/of_pci.c | 29 +++++++++++----------- drivers/of/overlay.c | 21 ++++++++-------- drivers/of/platform.c | 32 ++++++++++++------------- drivers/of/unittest.c | 66 +++++++++++++++++++++++++++++---------------------- 10 files changed, 136 insertions(+), 130 deletions(-) diff --git a/drivers/of/address.c b/drivers/of/address.c index 72914cdfce2a..6cce3c750a8f 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -559,7 +559,7 @@ static u64 __of_translate_address(struct device_node *dev, int na, ns, pna, pns; u64 result = OF_BAD_ADDR; - pr_debug("** translation for device %s **\n", of_node_full_name(dev)); + pr_debug("** translation for device %pOF **\n", dev); /* Increase refcount at current level */ of_node_get(dev); @@ -573,13 +573,13 @@ static u64 __of_translate_address(struct device_node *dev, /* Count address cells & copy address locally */ bus->count_cells(dev, &na, &ns); if (!OF_CHECK_COUNTS(na, ns)) { - pr_debug("Bad cell count for %s\n", of_node_full_name(dev)); + pr_debug("Bad cell count for %pOF\n", dev); goto bail; } memcpy(addr, in_addr, na * 4); - pr_debug("bus is %s (na=%d, ns=%d) on %s\n", - bus->name, na, ns, of_node_full_name(parent)); + pr_debug("bus is %s (na=%d, ns=%d) on %pOF\n", + bus->name, na, ns, parent); of_dump_addr("translating address:", addr, na); /* Translate */ @@ -600,13 +600,12 @@ static u64 __of_translate_address(struct device_node *dev, pbus = of_match_bus(parent); pbus->count_cells(dev, &pna, &pns); if (!OF_CHECK_COUNTS(pna, pns)) { - pr_err("Bad cell count for %s\n", - of_node_full_name(dev)); + pr_err("Bad cell count for %pOF\n", dev); break; } - pr_debug("parent bus is %s (na=%d, ns=%d) on %s\n", - pbus->name, pna, pns, of_node_full_name(parent)); + pr_debug("parent bus is %s (na=%d, ns=%d) on %pOF\n", + pbus->name, pna, pns, parent); /* Apply bus translation */ if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop)) @@ -855,7 +854,7 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz } if (!ranges) { - pr_debug("no dma-ranges found for node(%s)\n", np->full_name); + pr_debug("no dma-ranges found for node(%pOF)\n", np); ret = -ENODEV; goto out; } @@ -872,8 +871,8 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz dmaaddr = of_read_number(ranges, naddr); *paddr = of_translate_dma_address(np, ranges); if (*paddr == OF_BAD_ADDR) { - pr_err("translation of DMA address(%pad) to CPU address failed node(%s)\n", - dma_addr, np->full_name); + pr_err("translation of DMA address(%pad) to CPU address failed node(%pOF)\n", + dma_addr, np); ret = -EINVAL; goto out; } diff --git a/drivers/of/base.c b/drivers/of/base.c index 02be991a29c5..bbbaf1d080c2 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -160,7 +160,7 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp) pp->attr.read = of_node_property_read; rc = sysfs_create_bin_file(&np->kobj, &pp->attr); - WARN(rc, "error adding attribute %s to node %s\n", pp->name, np->full_name); + WARN(rc, "error adding attribute %s to node %pOF\n", pp->name, np); return rc; } @@ -1142,8 +1142,8 @@ int of_property_count_elems_of_size(const struct device_node *np, return -ENODATA; if (prop->length % elem_size != 0) { - pr_err("size of %s in node %s is not a multiple of %d\n", - propname, np->full_name, elem_size); + pr_err("size of %s in node %pOF is not a multiple of %d\n", + propname, np, elem_size); return -EINVAL; } @@ -1574,7 +1574,7 @@ EXPORT_SYMBOL_GPL(of_property_read_string_helper); void of_print_phandle_args(const char *msg, const struct of_phandle_args *args) { int i; - printk("%s %s", msg, of_node_full_name(args->np)); + printk("%s %pOF", msg, args->np); for (i = 0; i < args->args_count; i++) { const char delim = i ? ',' : ':'; @@ -1635,17 +1635,17 @@ int of_phandle_iterator_next(struct of_phandle_iterator *it) if (it->cells_name) { if (!it->node) { - pr_err("%s: could not find phandle\n", - it->parent->full_name); + pr_err("%pOF: could not find phandle\n", + it->parent); goto err; } if (of_property_read_u32(it->node, it->cells_name, &count)) { - pr_err("%s: could not get %s for %s\n", - it->parent->full_name, + pr_err("%pOF: could not get %s for %pOF\n", + it->parent, it->cells_name, - it->node->full_name); + it->node); goto err; } } else { @@ -1657,8 +1657,8 @@ int of_phandle_iterator_next(struct of_phandle_iterator *it) * property data length */ if (it->cur + count > it->list_end) { - pr_err("%s: arguments longer than property\n", - it->parent->full_name); + pr_err("%pOF: arguments longer than property\n", + it->parent); goto err; } } @@ -2089,8 +2089,8 @@ static void of_alias_add(struct alias_prop *ap, struct device_node *np, strncpy(ap->stem, stem, stem_len); ap->stem[stem_len] = 0; list_add_tail(&ap->link, &aliases_lookup); - pr_debug("adding DT alias:%s: stem=%s id=%i node=%s\n", - ap->alias, ap->stem, ap->id, of_node_full_name(np)); + pr_debug("adding DT alias:%s: stem=%s id=%i node=%pOF\n", + ap->alias, ap->stem, ap->id, np); } /** @@ -2344,8 +2344,8 @@ int of_graph_parse_endpoint(const struct device_node *node, { struct device_node *port_node = of_get_parent(node); - WARN_ONCE(!port_node, "%s(): endpoint %s has no parent node\n", - __func__, node->full_name); + WARN_ONCE(!port_node, "%s(): endpoint %pOF has no parent node\n", + __func__, node); memset(endpoint, 0, sizeof(*endpoint)); @@ -2428,14 +2428,14 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, of_node_put(node); if (!port) { - pr_err("graph: no port node found in %s\n", - parent->full_name); + pr_err("graph: no port node found in %pOF\n", + parent); return NULL; } } else { port = of_get_parent(prev); - if (WARN_ONCE(!port, "%s(): endpoint %s has no parent node\n", - __func__, prev->full_name)) + if (WARN_ONCE(!port, "%s(): endpoint %pOF has no parent node\n", + __func__, prev)) return NULL; } @@ -2551,8 +2551,8 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node, endpoint_node = of_graph_get_endpoint_by_regs(node, port, endpoint); if (!endpoint_node) { - pr_debug("no valid endpoint (%d, %d) for node %s\n", - port, endpoint, node->full_name); + pr_debug("no valid endpoint (%d, %d) for node %pOF\n", + port, endpoint, node); return NULL; } diff --git a/drivers/of/device.c b/drivers/of/device.c index 9416d052cb89..cce2f48c92be 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -294,7 +294,7 @@ void of_device_uevent(struct device *dev, struct kobj_uevent_env *env) return; add_uevent_var(env, "OF_NAME=%s", dev->of_node->name); - add_uevent_var(env, "OF_FULLNAME=%s", dev->of_node->full_name); + add_uevent_var(env, "OF_FULLNAME=%pOF", dev->of_node); if (dev->of_node->type && strcmp("<NULL>", dev->of_node->type) != 0) add_uevent_var(env, "OF_TYPE=%s", dev->of_node->type); diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 888fdbc09992..4ee1bd9bd79c 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -98,14 +98,14 @@ int of_reconfig_notify(unsigned long action, struct of_reconfig_data *p) switch (action) { case OF_RECONFIG_ATTACH_NODE: case OF_RECONFIG_DETACH_NODE: - pr_debug("notify %-15s %s\n", action_names[action], - pr->dn->full_name); + pr_debug("notify %-15s %pOF\n", action_names[action], + pr->dn); break; case OF_RECONFIG_ADD_PROPERTY: case OF_RECONFIG_REMOVE_PROPERTY: case OF_RECONFIG_UPDATE_PROPERTY: - pr_debug("notify %-15s %s:%s\n", action_names[action], - pr->dn->full_name, pr->prop->name); + pr_debug("notify %-15s %pOF:%s\n", action_names[action], + pr->dn, pr->prop->name); break; } @@ -328,11 +328,10 @@ void of_node_release(struct kobject *kobj) /* We should never be releasing nodes that haven't been detached. */ if (!of_node_check_flag(node, OF_DETACHED)) { - pr_err("ERROR: Bad of_node_put() on %s\n", node->full_name); + pr_err("ERROR: Bad of_node_put() on %pOF\n", node); dump_stack(); return; } - if (!of_node_check_flag(node, OF_DYNAMIC)) return; @@ -462,13 +461,13 @@ static void __of_changeset_entry_dump(struct of_changeset_entry *ce) case OF_RECONFIG_ADD_PROPERTY: case OF_RECONFIG_REMOVE_PROPERTY: case OF_RECONFIG_UPDATE_PROPERTY: - pr_debug("cset<%p> %-15s %s/%s\n", ce, action_names[ce->action], - ce->np->full_name, ce->prop->name); + pr_debug("cset<%p> %-15s %pOF/%s\n", ce, action_names[ce->action], + ce->np, ce->prop->name); break; case OF_RECONFIG_ATTACH_NODE: case OF_RECONFIG_DETACH_NODE: - pr_debug("cset<%p> %-15s %s\n", ce, action_names[ce->action], - ce->np->full_name); + pr_debug("cset<%p> %-15s %pOF\n", ce, action_names[ce->action], + ce->np); break; } } @@ -539,7 +538,7 @@ static void __of_changeset_entry_notify(struct of_changeset_entry *ce, bool reve } if (ret) - pr_err("changeset notifier error @%s\n", ce->np->full_name); + pr_err("changeset notifier error @%pOF\n", ce->np); } static int __of_changeset_entry_apply(struct of_changeset_entry *ce) @@ -570,8 +569,8 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) ret = __of_add_property(ce->np, ce->prop); if (ret) { - pr_err("changeset: add_property failed @%s/%s\n", - ce->np->full_name, + pr_err("changeset: add_property failed @%pOF/%s\n", + ce->np, ce->prop->name); break; } @@ -579,8 +578,8 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) case OF_RECONFIG_REMOVE_PROPERTY: ret = __of_remove_property(ce->np, ce->prop); if (ret) { - pr_err("changeset: remove_property failed @%s/%s\n", - ce->np->full_name, + pr_err("changeset: remove_property failed @%pOF/%s\n", + ce->np, ce->prop->name); break; } @@ -598,8 +597,8 @@ static int __of_changeset_entry_apply(struct of_changeset_entry *ce) ret = __of_update_property(ce->np, ce->prop, &old_prop); if (ret) { - pr_err("changeset: update_property failed @%s/%s\n", - ce->np->full_name, + pr_err("changeset: update_property failed @%pOF/%s\n", + ce->np, ce->prop->name); break; } diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 6ce72aa65425..9c4b32522bc2 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -131,7 +131,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq) goto fail; } - pr_debug("of_irq_parse_raw: ipar=%s, size=%d\n", of_node_full_name(ipar), intsize); + pr_debug("of_irq_parse_raw: ipar=%pOF, size=%d\n", ipar, intsize); if (out_irq->args_count != intsize) goto fail; @@ -269,7 +269,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq) skiplevel: /* Iterate again with new parent */ out_irq->np = newpar; - pr_debug(" -> new parent: %s\n", of_node_full_name(newpar)); + pr_debug(" -> new parent: %pOF\n", newpar); of_node_put(ipar); ipar = newpar; newpar = NULL; @@ -301,7 +301,7 @@ int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_ar u32 intsize, intlen; int i, res; - pr_debug("of_irq_parse_one: dev=%s, index=%d\n", of_node_full_name(device), index); + pr_debug("of_irq_parse_one: dev=%pOF, index=%d\n", device, index); /* OldWorld mac stuff is "special", handle out of line */ if (of_irq_workarounds & OF_IMAP_OLDWORLD_MAC) @@ -555,8 +555,8 @@ void __init of_irq_init(const struct of_device_id *matches) of_node_set_flag(desc->dev, OF_POPULATED); - pr_debug("of_irq_init: init %s (%p), parent %p\n", - desc->dev->full_name, + pr_debug("of_irq_init: init %pOF (%p), parent %p\n", + desc->dev, desc->dev, desc->interrupt_parent); ret = desc->irq_init_cb(desc->dev, desc->interrupt_parent); diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c index 7e4c80f9b6cd..f0a32be9cc61 100644 --- a/drivers/of/of_mdio.c +++ b/drivers/of/of_mdio.c @@ -126,14 +126,14 @@ int of_mdio_parse_addr(struct device *dev, const struct device_node *np) ret = of_property_read_u32(np, "reg", &addr); if (ret < 0) { - dev_err(dev, "%s has invalid PHY address\n", np->full_name); + dev_err(dev, "%pOF has invalid PHY address\n", np); return ret; } /* A PHY must have a reg property in the range [0-31] */ if (addr >= PHY_MAX_ADDR) { - dev_err(dev, "%s PHY address %i is too large\n", - np->full_name, addr); + dev_err(dev, "%pOF PHY address %i is too large\n", + np, addr); return -EINVAL; } @@ -188,8 +188,8 @@ static bool of_mdiobus_child_is_phy(struct device_node *child) if (of_match_node(whitelist_phys, child)) { pr_warn(FW_WARN - "%s: Whitelisted compatible string. Please remove\n", - child->full_name); + "%pOF: Whitelisted compatible string. Please remove\n", + child); return true; } diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c index c9d4d3a7b0fe..3d4cb7090878 100644 --- a/drivers/of/of_pci.c +++ b/drivers/of/of_pci.c @@ -204,15 +204,15 @@ int of_pci_get_host_bridge_resources(struct device_node *dev, if (!bus_range) return -ENOMEM; - pr_info("host bridge %s ranges:\n", dev->full_name); + pr_info("host bridge %pOF ranges:\n", dev); err = of_pci_parse_bus_range(dev, bus_range); if (err) { bus_range->start = busno; bus_range->end = bus_max; bus_range->flags = IORESOURCE_BUS; - pr_info(" No bus range found for %s, using %pR\n", - dev->full_name, bus_range); + pr_info(" No bus range found for %pOF, using %pR\n", + dev, bus_range); } else { if (bus_range->end > bus_range->start + bus_max) bus_range->end = bus_range->start + bus_max; @@ -258,14 +258,14 @@ int of_pci_get_host_bridge_resources(struct device_node *dev, if (resource_type(res) == IORESOURCE_IO) { if (!io_base) { - pr_err("I/O range found for %s. Please provide an io_base pointer to save CPU base address\n", - dev->full_name); + pr_err("I/O range found for %pOF. Please provide an io_base pointer to save CPU base address\n", + dev); err = -EINVAL; goto conversion_failed; } if (*io_base != (resource_size_t)OF_BAD_ADDR) - pr_warn("More than one I/O resource converted for %s. CPU base address for old range lost!\n", - dev->full_name); + pr_warn("More than one I/O resource converted for %pOF. CPU base address for old range lost!\n", + dev); *io_base = range.cpu_addr; } @@ -325,7 +325,7 @@ int of_pci_map_rid(struct device_node *np, u32 rid, } if (!map_len || map_len % (4 * sizeof(*map))) { - pr_err("%s: Error: Bad %s length: %d\n", np->full_name, + pr_err("%pOF: Error: Bad %s length: %d\n", np, map_name, map_len); return -EINVAL; } @@ -349,8 +349,8 @@ int of_pci_map_rid(struct device_node *np, u32 rid, u32 rid_len = be32_to_cpup(map + 3); if (rid_base & ~map_mask) { - pr_err("%s: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n", - np->full_name, map_name, map_name, + pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n", + np, map_name, map_name, map_mask, rid_base); return -EFAULT; } @@ -375,14 +375,13 @@ int of_pci_map_rid(struct device_node *np, u32 rid, if (id_out) *id_out = masked_rid - rid_base + out_base; - pr_debug("%s: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n", - np->full_name, map_name, map_mask, rid_base, out_base, + pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n", + np, map_name, map_mask, rid_base, out_base, rid_len, rid, *id_out); return 0; } - pr_err("%s: Invalid %s translation - no match for rid 0x%x on %s\n", - np->full_name, map_name, rid, - target && *target ? (*target)->full_name : "any target"); + pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n", + np, map_name, rid, target && *target ? *target : NULL); return -EFAULT; } diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 7827786718d8..5948eac07f34 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -137,7 +137,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov, of_node_put(tchild); } else { /* create empty tree as a target */ - tchild = __of_node_dup(child, "%s/%s", target->full_name, cname); + tchild = __of_node_dup(child, "%pOF/%s", target, cname); if (!tchild) return -ENOMEM; @@ -173,8 +173,8 @@ static int of_overlay_apply_one(struct of_overlay *ov, for_each_property_of_node(overlay, prop) { ret = of_overlay_apply_single_property(ov, target, prop); if (ret) { - pr_err("Failed to apply prop @%s/%s\n", - target->full_name, prop->name); + pr_err("Failed to apply prop @%pOF/%s\n", + target, prop->name); return ret; } } @@ -182,8 +182,8 @@ static int of_overlay_apply_one(struct of_overlay *ov, for_each_child_of_node(overlay, child) { ret = of_overlay_apply_single_device_node(ov, target, child); if (ret != 0) { - pr_err("Failed to apply single node @%s/%s\n", - target->full_name, child->name); + pr_err("Failed to apply single node @%pOF/%s\n", + target, child->name); of_node_put(child); return ret; } @@ -211,7 +211,7 @@ static int of_overlay_apply(struct of_overlay *ov) err = of_overlay_apply_one(ov, ovinfo->target, ovinfo->overlay); if (err != 0) { - pr_err("apply failed '%s'\n", ovinfo->target->full_name); + pr_err("apply failed '%pOF'\n", ovinfo->target); return err; } } @@ -396,8 +396,8 @@ int of_overlay_create(struct device_node *tree) /* build the overlay info structures */ err = of_build_overlay_info(ov, tree); if (err) { - pr_err("of_build_overlay_info() failed for tree@%s\n", - tree->full_name); + pr_err("of_build_overlay_info() failed for tree@%pOF\n", + tree); goto err_free_idr; } @@ -476,9 +476,8 @@ static int overlay_is_topmost(struct of_overlay *ov, struct device_node *dn) /* check against each subtree affected by this overlay */ list_for_each_entry(ce, &ovt->cset.entries, node) { if (overlay_subtree_check(ce->np, dn)) { - pr_err("%s: #%d clashes #%d @%s\n", - __func__, ov->id, ovt->id, - dn->full_name); + pr_err("%s: #%d clashes #%d @%pOF\n", + __func__, ov->id, ovt->id, dn); return 0; } } diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 8f73413fa243..09045265b1b9 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -228,7 +228,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node, const void *prop; int i, ret; - pr_debug("Creating amba device %s\n", node->full_name); + pr_debug("Creating amba device %pOF\n", node); if (!of_device_is_available(node) || of_node_test_and_set_flag(node, OF_POPULATED)) @@ -259,15 +259,15 @@ static struct amba_device *of_amba_device_create(struct device_node *node, ret = of_address_to_resource(node, 0, &dev->res); if (ret) { - pr_err("amba: of_address_to_resource() failed (%d) for %s\n", - ret, node->full_name); + pr_err("amba: of_address_to_resource() failed (%d) for %pOF\n", + ret, node); goto err_free; } ret = amba_device_add(dev, &iomem_resource); if (ret) { - pr_err("amba_device_add() failed (%d) for %s\n", - ret, node->full_name); + pr_err("amba_device_add() failed (%d) for %pOF\n", + ret, node); goto err_free; } @@ -310,7 +310,7 @@ static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *l if (!of_address_to_resource(np, 0, &res)) if (res.start != auxdata->phys_addr) continue; - pr_debug("%s: devname=%s\n", np->full_name, auxdata->name); + pr_debug("%pOF: devname=%s\n", np, auxdata->name); return auxdata; } @@ -323,7 +323,7 @@ static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *l if (!of_device_is_compatible(np, auxdata->compatible)) continue; if (!auxdata->phys_addr && !auxdata->name) { - pr_debug("%s: compatible match\n", np->full_name); + pr_debug("%pOF: compatible match\n", np); return auxdata; } } @@ -356,14 +356,14 @@ static int of_platform_bus_create(struct device_node *bus, /* Make sure it has a compatible property */ if (strict && (!of_get_property(bus, "compatible", NULL))) { - pr_debug("%s() - skipping %s, no compatible prop\n", - __func__, bus->full_name); + pr_debug("%s() - skipping %pOF, no compatible prop\n", + __func__, bus); return 0; } if (of_node_check_flag(bus, OF_POPULATED_BUS)) { - pr_debug("%s() - skipping %s, already populated\n", - __func__, bus->full_name); + pr_debug("%s() - skipping %pOF, already populated\n", + __func__, bus); return 0; } @@ -387,7 +387,7 @@ static int of_platform_bus_create(struct device_node *bus, return 0; for_each_child_of_node(bus, child) { - pr_debug(" create child: %s\n", child->full_name); + pr_debug(" create child: %pOF\n", child); rc = of_platform_bus_create(child, matches, lookup, &dev->dev, strict); if (rc) { of_node_put(child); @@ -419,7 +419,7 @@ int of_platform_bus_probe(struct device_node *root, return -EINVAL; pr_debug("%s()\n", __func__); - pr_debug(" starting at: %s\n", root->full_name); + pr_debug(" starting at: %pOF\n", root); /* Do a self check of bus type, if there's a match, create children */ if (of_match_node(matches, root)) { @@ -471,7 +471,7 @@ int of_platform_populate(struct device_node *root, return -EINVAL; pr_debug("%s()\n", __func__); - pr_debug(" starting at: %s\n", root->full_name); + pr_debug(" starting at: %pOF\n", root); for_each_child_of_node(root, child) { rc = of_platform_bus_create(child, matches, lookup, parent, true); @@ -659,8 +659,8 @@ static int of_platform_notify(struct notifier_block *nb, of_dev_put(pdev_parent); if (pdev == NULL) { - pr_err("%s: failed to create for '%s'\n", - __func__, rd->dn->full_name); + pr_err("%s: failed to create for '%pOF'\n", + __func__, rd->dn); /* of_platform_device_create tosses the error code */ return notifier_from_errno(-EINVAL); } diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 987a1530282a..038fec3e3463 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -46,46 +46,54 @@ static struct unittest_results { static void __init of_unittest_find_node_by_name(void) { struct device_node *np; - const char *options; + const char *options, *name; np = of_find_node_by_path("/testcase-data"); - unittest(np && !strcmp("/testcase-data", np->full_name), + name = kasprintf(GFP_KERNEL, "%pOF", np); + unittest(np && !strcmp("/testcase-data", name), "find /testcase-data failed\n"); of_node_put(np); + kfree(name); /* Test if trailing '/' works */ np = of_find_node_by_path("/testcase-data/"); unittest(!np, "trailing '/' on /testcase-data/ should fail\n"); np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); - unittest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", np->full_name), + name = kasprintf(GFP_KERNEL, "%pOF", np); + unittest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", name), "find /testcase-data/phandle-tests/consumer-a failed\n"); of_node_put(np); + kfree(name); np = of_find_node_by_path("testcase-alias"); - unittest(np && !strcmp("/testcase-data", np->full_name), + name = kasprintf(GFP_KERNEL, "%pOF", np); + unittest(np && !strcmp("/testcase-data", name), "find testcase-alias failed\n"); of_node_put(np); + kfree(name); /* Test if trailing '/' works on aliases */ np = of_find_node_by_path("testcase-alias/"); unittest(!np, "trailing '/' on testcase-alias/ should fail\n"); np = of_find_node_by_path("testcase-alias/phandle-tests/consumer-a"); - unittest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", np->full_name), + name = kasprintf(GFP_KERNEL, "%pOF", np); + unittest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", name), "find testcase-alias/phandle-tests/consumer-a failed\n"); of_node_put(np); + kfree(name); np = of_find_node_by_path("/testcase-data/missing-path"); - unittest(!np, "non-existent path returned node %s\n", np->full_name); + unittest(!np, "non-existent path returned node %pOF\n", np); of_node_put(np); np = of_find_node_by_path("missing-alias"); - unittest(!np, "non-existent alias returned node %s\n", np->full_name); + unittest(!np, "non-existent alias returned node %pOF\n", np); of_node_put(np); np = of_find_node_by_path("testcase-alias/missing-path"); - unittest(!np, "non-existent alias with relative path returned node %s\n", np->full_name); + unittest(!np, "non-existent alias with relative path returned node %pOF\n", np); of_node_put(np); np = of_find_node_opts_by_path("/testcase-data:testoption", &options); @@ -258,8 +266,8 @@ static void __init of_unittest_check_phandles(void) hash_for_each_possible(phandle_ht, nh, node, np->phandle) { if (nh->np->phandle == np->phandle) { - pr_info("Duplicate phandle! %i used by %s and %s\n", - np->phandle, nh->np->full_name, np->full_name); + pr_info("Duplicate phandle! %i used by %pOF and %pOF\n", + np->phandle, nh->np, np); dup_count++; break; } @@ -349,8 +357,8 @@ static void __init of_unittest_parse_phandle_with_args(void) passed = false; } - unittest(passed, "index %i - data error on node %s rc=%i\n", - i, args.np->full_name, rc); + unittest(passed, "index %i - data error on node %pOF rc=%i\n", + i, args.np, rc); } /* Check for missing list property */ @@ -533,7 +541,7 @@ static void __init of_unittest_changeset(void) /* Make sure node names are constructed correctly */ unittest((np = of_find_node_by_path("/testcase-data/changeset/n2/n21")), - "'%s' not added\n", n21->full_name); + "'%pOF' not added\n", n21); of_node_put(np); unittest(!of_changeset_revert(&chgset), "revert failed\n"); @@ -564,8 +572,8 @@ static void __init of_unittest_parse_interrupts(void) passed &= (args.args_count == 1); passed &= (args.args[0] == (i + 1)); - unittest(passed, "index %i - data error on node %s rc=%i\n", - i, args.np->full_name, rc); + unittest(passed, "index %i - data error on node %pOF rc=%i\n", + i, args.np, rc); } of_node_put(np); @@ -610,8 +618,8 @@ static void __init of_unittest_parse_interrupts(void) default: passed = false; } - unittest(passed, "index %i - data error on node %s rc=%i\n", - i, args.np->full_name, rc); + unittest(passed, "index %i - data error on node %pOF rc=%i\n", + i, args.np, rc); } of_node_put(np); } @@ -680,8 +688,8 @@ static void __init of_unittest_parse_interrupts_extended(void) passed = false; } - unittest(passed, "index %i - data error on node %s rc=%i\n", - i, args.np->full_name, rc); + unittest(passed, "index %i - data error on node %pOF rc=%i\n", + i, args.np, rc); } of_node_put(np); } @@ -861,7 +869,9 @@ static int attach_node_and_children(struct device_node *np) struct device_node *next, *dup, *child; unsigned long flags; - dup = of_find_node_by_path(np->full_name); + full_name = kasprintf(GFP_KERNEL, "%pOF", np); + dup = of_find_node_by_path(full_name); + kfree(full_name); if (dup) { update_node_properties(np, dup); return 0; @@ -966,7 +976,7 @@ static int unittest_probe(struct platform_device *pdev) } - dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name); + dev_dbg(dev, "%s for node @%pOF\n", __func__, np); of_platform_populate(np, NULL, NULL, &pdev->dev); @@ -978,7 +988,7 @@ static int unittest_remove(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; - dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name); + dev_dbg(dev, "%s for node @%pOF\n", __func__, np); return 0; } @@ -1592,7 +1602,7 @@ static int unittest_i2c_bus_probe(struct platform_device *pdev) } - dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name); + dev_dbg(dev, "%s for node @%pOF\n", __func__, np); std = devm_kzalloc(dev, sizeof(*std), GFP_KERNEL); if (!std) { @@ -1630,7 +1640,7 @@ static int unittest_i2c_bus_remove(struct platform_device *pdev) struct device_node *np = dev->of_node; struct unittest_i2c_bus_data *std = platform_get_drvdata(pdev); - dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name); + dev_dbg(dev, "%s for node @%pOF\n", __func__, np); i2c_del_adapter(&std->adap); return 0; @@ -1661,7 +1671,7 @@ static int unittest_i2c_dev_probe(struct i2c_client *client, return -EINVAL; } - dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name); + dev_dbg(dev, "%s for node @%pOF\n", __func__, np); return 0; }; @@ -1671,7 +1681,7 @@ static int unittest_i2c_dev_remove(struct i2c_client *client) struct device *dev = &client->dev; struct device_node *np = client->dev.of_node; - dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name); + dev_dbg(dev, "%s for node @%pOF\n", __func__, np); return 0; } @@ -1706,7 +1716,7 @@ static int unittest_i2c_mux_probe(struct i2c_client *client, struct i2c_mux_core *muxc; u32 reg, max_reg; - dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name); + dev_dbg(dev, "%s for node @%pOF\n", __func__, np); if (!np) { dev_err(dev, "No OF node\n"); @@ -1751,7 +1761,7 @@ static int unittest_i2c_mux_remove(struct i2c_client *client) struct device_node *np = client->dev.of_node; struct i2c_mux_core *muxc = i2c_get_clientdata(client); - dev_dbg(dev, "%s for node @%s\n", __func__, np->full_name); + dev_dbg(dev, "%s for node @%pOF\n", __func__, np); i2c_mux_del_adapters(muxc); return 0; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
[parent not found: <20170614203025.7581-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 4/4] of: Convert to using %pOF instead of full_name [not found] ` <20170614203025.7581-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2017-06-14 20:58 ` Joe Perches 0 siblings, 0 replies; 19+ messages in thread From: Joe Perches @ 2017-06-14 20:58 UTC (permalink / raw) To: Rob Herring, Frank Rowand, Mark Rutland Cc: Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Wed, 2017-06-14 at 15:30 -0500, Rob Herring wrote: > Now that we have a custom printf format specifier, convert users of > full_name to use %pOF instead. This is preparation to remove storing > of the full path string for each node. Well, a full_name removal makes my reply in 2/4 less sensible. Maybe 2/4 should say that full_name is going away. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-06-23 17:38 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-14 20:30 [PATCH 0/4] DT printf format specifiers Rob Herring [not found] ` <20170614203025.7581-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-14 20:30 ` [PATCH 1/4] of: use kbasename instead of open coding Rob Herring [not found] ` <20170614203025.7581-2-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-17 17:30 ` Andy Shevchenko 2017-06-14 20:30 ` [PATCH 2/4] of: find_node_by_full_name rewrite to compare each level Rob Herring 2017-06-14 20:30 ` [PATCH 3/4] of: Custom printk format specifier for device node Rob Herring [not found] ` <20170614203025.7581-4-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-14 20:56 ` Joe Perches [not found] ` <1497473808.18751.70.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> 2017-06-15 12:30 ` Rob Herring [not found] ` <CAL_JsqJ_cc46Hf2XdZoXkgZyOh+0KXVXfeWYe1100E9vuRt12A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2017-06-15 16:51 ` Joe Perches 2017-06-15 21:26 ` Rob Herring 2017-06-15 21:50 ` Joe Perches 2017-06-22 20:44 ` [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree Rob Herring 2017-06-22 22:44 ` Randy Dunlap [not found] ` <d22444aa-39da-ec74-42a1-63f1fa40c7d3-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> 2017-06-23 14:08 ` Rob Herring [not found] ` <20170622204445.14930-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-23 3:01 ` Joe Perches [not found] ` <1498186912.24295.9.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> 2017-06-23 14:13 ` Rob Herring 2017-06-23 17:30 ` [PATCH v3] " Rob Herring [not found] ` <20170623173053.636-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-23 17:38 ` Joe Perches 2017-06-14 20:30 ` [PATCH 4/4] of: Convert to using %pOF instead of full_name Rob Herring [not found] ` <20170614203025.7581-5-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2017-06-14 20:58 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).