* [PATCH 0/4] DT printf format specifiers
@ 2017-06-14 20:30 Rob Herring
2017-06-14 20:30 ` [PATCH 1/4] of: use kbasename instead of open coding Rob Herring
` (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, linux-kernel
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/4] of: use kbasename instead of open coding
2017-06-14 20:30 [PATCH 0/4] DT printf format specifiers Rob Herring
@ 2017-06-14 20:30 ` Rob Herring
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
` (2 subsequent siblings)
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
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@kernel.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
^ permalink raw reply related [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
2017-06-14 20:30 ` [PATCH 1/4] of: use kbasename instead of open coding Rob Herring
@ 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
2017-06-14 20:30 ` [PATCH 1/4] of: use kbasename instead of open coding Rob Herring
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
2017-06-14 20:56 ` Joe Perches
2017-06-22 20:44 ` [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree Rob Herring
2017-06-14 20:30 ` [PATCH 4/4] of: Convert to using %pOF instead of full_name Rob Herring
3 siblings, 2 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
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
* [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
2017-06-14 20:58 ` Joe Perches
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
* Re: [PATCH 3/4] of: Custom printk format specifier for device node
2017-06-14 20:30 ` [PATCH 3/4] of: Custom printk format specifier for device node Rob Herring
@ 2017-06-14 20:56 ` Joe Perches
2017-06-15 12:30 ` Rob Herring
2017-06-15 21:26 ` Rob Herring
2017-06-22 20:44 ` [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree Rob Herring
1 sibling, 2 replies; 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, linux-kernel
On Wed, 2017-06-14 at 15:30 -0500, Rob Herring wrote:
> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/4] of: Convert to using %pOF instead of full_name
2017-06-14 20:30 ` [PATCH 4/4] of: Convert to using %pOF instead of full_name Rob Herring
@ 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, linux-kernel
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] of: Custom printk format specifier for device node
2017-06-14 20:56 ` Joe Perches
@ 2017-06-15 12:30 ` Rob Herring
2017-06-15 16:51 ` Joe Perches
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, linux-kernel
On Wed, Jun 14, 2017 at 3:56 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2017-06-14 at 15:30 -0500, Rob Herring wrote:
>> From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
>
> 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] of: Custom printk format specifier for device node
2017-06-15 12:30 ` Rob Herring
@ 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, linux-kernel
On Thu, 2017-06-15 at 07:30 -0500, Rob Herring wrote:
> On Wed, Jun 14, 2017 at 3:56 PM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2017-06-14 at 15:30 -0500, Rob Herring wrote:
> > > From: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> >
> > 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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] of: Custom printk format specifier for device node
2017-06-14 20:56 ` Joe Perches
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, linux-kernel
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@konsulko.com>
>
> 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
^ 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, linux-kernel
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@konsulko.com>
[]
> > > 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;
> }
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] of: use kbasename instead of open coding
2017-06-14 20:30 ` [PATCH 1/4] of: use kbasename instead of open coding Rob Herring
@ 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
On Wed, Jun 14, 2017 at 11:30 PM, Rob Herring <robh@kernel.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@gmail.com>
>
> Signed-off-by: Rob Herring <robh@kernel.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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree
2017-06-14 20:30 ` [PATCH 3/4] of: Custom printk format specifier for device node Rob Herring
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, 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);
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
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
^ 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
2017-06-23 14:08 ` Rob Herring
2017-06-23 3:01 ` Joe Perches
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
* 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
@ 2017-06-23 3:01 ` Joe Perches
2017-06-23 14:13 ` Rob Herring
2017-06-23 17:30 ` [PATCH v3] " Rob Herring
2 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, linux-kernel
On Thu, 2017-06-22 at 15:44 -0500, 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);
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]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree
2017-06-22 22:44 ` Randy Dunlap
@ 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, linux-kernel
On Thu, Jun 22, 2017 at 5:44 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> 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?
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] vsprintf: Add %p extension "%pOF" for device tree
2017-06-23 3:01 ` Joe Perches
@ 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, linux-kernel
On Thu, Jun 22, 2017 at 10:01 PM, Joe Perches <joe@perches.com> wrote:
> On Thu, 2017-06-22 at 15:44 -0500, 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);
>
> 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
^ 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
2017-06-23 3:01 ` Joe Perches
@ 2017-06-23 17:30 ` Rob Herring
2017-06-23 17:38 ` Joe Perches
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
* Re: [PATCH v3] vsprintf: Add %p extension "%pOF" for device tree
2017-06-23 17:30 ` [PATCH v3] " Rob Herring
@ 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, linux-kernel
On Fri, 2017-06-23 at 12:30 -0500, 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);
>
> '%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.
Thanks Rob.
This all seems sensible to me now.
If you want:
Acked-by: Joe Perches <joe@perches.com>
cheers, Joe
^ 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
2017-06-14 20:30 ` [PATCH 1/4] of: use kbasename instead of open coding Rob Herring
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
2017-06-14 20:56 ` Joe Perches
2017-06-15 12:30 ` Rob Herring
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
2017-06-23 14:08 ` Rob Herring
2017-06-23 3:01 ` Joe Perches
2017-06-23 14:13 ` Rob Herring
2017-06-23 17:30 ` [PATCH v3] " Rob Herring
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
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).