2017-04-25 08:49:21

by Frank Rowand

[permalink] [raw]
Subject: [PATCH v3 0/4] of: remove *phandle properties from expanded device tree

From: Frank Rowand <[email protected]>

Remove "phandle" and "linux,phandle" properties from the internal
device tree. The phandle will still be in the struct device_node
phandle field.

This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *. As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.

[1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html

Patch 1 is the phandle related changes.

Patches 2 - 4 are minor fixups for issues that became visible
while implementing patch 1.

Changes from v1:
- patch 1: Remove check in __of_add_phandle_sysfs() that would not
add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)

Changes from v1:
- Remove phandle properties in of_attach_node(), before attaching
the node to the live tree.
- Add the phandle sysfs entry for the node in of_attach_node().
- When creating an overlay changeset, duplicate the node phandle in
__of_node_dup().

Frank Rowand (4):
of: remove *phandle properties from expanded device tree
of: make __of_attach_node() static
of: be consistent in form of file mode
of: detect invalid phandle in overlay

drivers/of/base.c | 50 +++++++++++++++++++++++++++++++++++++++----
drivers/of/dynamic.c | 56 ++++++++++++++++++++++++++++++++++++-------------
drivers/of/fdt.c | 40 +++++++++++++++++++++--------------
drivers/of/of_private.h | 2 +-
drivers/of/overlay.c | 8 ++++---
drivers/of/resolver.c | 23 +-------------------
include/linux/of.h | 1 +
7 files changed, 120 insertions(+), 60 deletions(-)

--
Frank Rowand <[email protected]>


2017-04-25 08:49:32

by Frank Rowand

[permalink] [raw]
Subject: [PATCH v3 1/4] of: remove *phandle properties from expanded device tree

From: Frank Rowand <[email protected]>

Remove "phandle", "linux,phandle", and "ibm,phandle" properties from
the internal device tree. The phandle will still be in the struct
device_node phandle field.

This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *. As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.

[1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html

- Add sysfs infrastructure to report np->phandle, as if it was a property.
- Do not create "phandle" "ibm,phandle", and "linux,phandle" properties
in the expanded device tree.
- Remove phandle properties in of_attach_node(), for nodes dynamically
attached to the live tree. Add the phandle sysfs entry for these nodes.
- When creating an overlay changeset, duplicate the node phandle in
__of_node_dup().
- Remove no longer needed checks to exclude "phandle" and "linux,phandle"
properties in several locations.
- A side effect of these changes is that the obsolete "linux,phandle" and
"ibm,phandle" properties will no longer appear in /proc/device-tree (they
will appear as "phandle").

Signed-off-by: Frank Rowand <[email protected]>
---
drivers/of/base.c | 48 ++++++++++++++++++++++++++++++++++++++++---
drivers/of/dynamic.c | 54 +++++++++++++++++++++++++++++++++++++------------
drivers/of/fdt.c | 40 +++++++++++++++++++++---------------
drivers/of/of_private.h | 1 +
drivers/of/overlay.c | 4 +---
drivers/of/resolver.c | 23 +--------------------
include/linux/of.h | 1 +
7 files changed, 114 insertions(+), 57 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d7c4629a3a2d..41473b1e2445 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -116,6 +116,19 @@ static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
}

+static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t offset, size_t count)
+{
+ phandle phandle;
+ struct device_node *np;
+
+ np = container_of(bin_attr, struct device_node, attr_phandle);
+ phandle = cpu_to_be32(np->phandle);
+ return memory_read_from_buffer(buf, count, &offset, &phandle,
+ sizeof(phandle));
+}
+
/* always return newly allocated name, caller must free after use */
static const char *safe_name(struct kobject *kobj, const char *orig_name)
{
@@ -164,6 +177,35 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)
return rc;
}

+/*
+ * In the imported device tree (fdt), phandle is a property. In the
+ * internal data structure it is instead stored in the struct device_node.
+ * Make phandle visible in sysfs as if it was a property.
+ */
+int __of_add_phandle_sysfs(struct device_node *np)
+{
+ int rc;
+
+ if (!IS_ENABLED(CONFIG_SYSFS))
+ return 0;
+
+ if (!of_kset || !of_node_is_attached(np))
+ return 0;
+
+ if (!np->phandle || np->phandle == 0xffffffff)
+ return 0;
+
+ sysfs_bin_attr_init(&pp->attr);
+ np->attr_phandle.attr.name = "phandle";
+ np->attr_phandle.attr.mode = 0444;
+ np->attr_phandle.size = sizeof(np->phandle);
+ np->attr_phandle.read = of_node_phandle_read;
+
+ rc = sysfs_create_bin_file(&np->kobj, &np->attr_phandle);
+ WARN(rc, "error adding attribute phandle to node %s\n", np->full_name);
+ return rc;
+}
+
int __of_attach_node_sysfs(struct device_node *np)
{
const char *name;
@@ -193,6 +235,8 @@ int __of_attach_node_sysfs(struct device_node *np)
if (rc)
return rc;

+ __of_add_phandle_sysfs(np);
+
for_each_property_of_node(np, pp)
__of_add_property_sysfs(np, pp);

@@ -2097,9 +2141,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
int id, len;

/* Skip those we do not want to proceed */
- if (!strcmp(pp->name, "name") ||
- !strcmp(pp->name, "phandle") ||
- !strcmp(pp->name, "linux,phandle"))
+ if (!strcmp(pp->name, "name"))
continue;

np = of_find_node_by_path(pp->value);
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 888fdbc09992..59545b8fbf46 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -218,19 +218,6 @@ int of_property_notify(int action, struct device_node *np,

void __of_attach_node(struct device_node *np)
{
- const __be32 *phandle;
- int sz;
-
- np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
- np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
-
- phandle = __of_get_property(np, "phandle", &sz);
- if (!phandle)
- phandle = __of_get_property(np, "linux,phandle", &sz);
- if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
- phandle = __of_get_property(np, "ibm,phandle", &sz);
- np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
-
np->child = NULL;
np->sibling = np->parent->child;
np->parent->child = np;
@@ -243,11 +230,47 @@ void __of_attach_node(struct device_node *np)
int of_attach_node(struct device_node *np)
{
struct of_reconfig_data rd;
+ struct property *prev;
+ struct property *prop;
+ struct property *save_next;
unsigned long flags;
+ const __be32 *phandle;
+ int sz;

memset(&rd, 0, sizeof(rd));
rd.dn = np;

+ np->name = __of_get_property(np, "name", NULL) ? : "<NULL>";
+ np->type = __of_get_property(np, "device_type", NULL) ? : "<NULL>";
+
+ phandle = __of_get_property(np, "phandle", &sz);
+ if (!phandle)
+ phandle = __of_get_property(np, "linux,phandle", &sz);
+ if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle)
+ phandle = __of_get_property(np, "ibm,phandle", &sz);
+ np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0;
+
+ /* remove phandle properties from node */
+ prev = NULL;
+ for (prop = np->properties; prop != NULL; ) {
+ save_next = prop->next;
+ if (!strcmp(prop->name, "phandle") ||
+ !strcmp(prop->name, "ibm,phandle") ||
+ !strcmp(prop->name, "linux,phandle")) {
+ if (prev)
+ prev->next = prop->next;
+ else
+ np->properties = prop->next;
+ prop->next = np->deadprops;
+ np->deadprops = prop;
+ } else {
+ prev = prop;
+ }
+ prop = save_next;
+ }
+
+ __of_add_phandle_sysfs(np);
+
mutex_lock(&of_mutex);
raw_spin_lock_irqsave(&devtree_lock, flags);
__of_attach_node(np);
@@ -429,6 +452,7 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
/* Iterate over and duplicate all properties */
if (np) {
struct property *pp, *new_pp;
+ node->phandle = np->phandle;
for_each_property_of_node(np, pp) {
new_pp = __of_prop_dup(pp, GFP_KERNEL);
if (!new_pp)
@@ -441,6 +465,10 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
}
}
}
+
+ node->name = __of_get_property(node, "name", NULL) ? : "<NULL>";
+ node->type = __of_get_property(node, "device_type", NULL) ? : "<NULL>";
+
return node;

err_prop:
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index e5ce4b59e162..270f31b0c259 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -181,6 +181,8 @@ static void populate_properties(const void *blob,
const __be32 *val;
const char *pname;
u32 sz;
+ int prop_is_phandle = 0;
+ int prop_is_ibm_phandle = 0;

val = fdt_getprop_by_offset(blob, cur, &pname, &sz);
if (!val) {
@@ -196,11 +198,6 @@ static void populate_properties(const void *blob,
if (!strcmp(pname, "name"))
has_name = true;

- pp = unflatten_dt_alloc(mem, sizeof(struct property),
- __alignof__(struct property));
- if (dryrun)
- continue;
-
/* We accept flattened tree phandles either in
* ePAPR-style "phandle" properties, or the
* legacy "linux,phandle" properties. If both
@@ -208,23 +205,34 @@ static void populate_properties(const void *blob,
* will get weird. Don't do that.
*/
if (!strcmp(pname, "phandle") ||
- !strcmp(pname, "linux,phandle")) {
- if (!np->phandle)
- np->phandle = be32_to_cpup(val);
- }
+ !strcmp(pname, "linux,phandle"))
+ prop_is_phandle = 1;

/* And we process the "ibm,phandle" property
* used in pSeries dynamic device tree
* stuff
*/
- if (!strcmp(pname, "ibm,phandle"))
- np->phandle = be32_to_cpup(val);
+ if (!strcmp(pname, "ibm,phandle")) {
+ prop_is_phandle = 1;
+ prop_is_ibm_phandle = 1;
+ }
+
+ if (!prop_is_phandle)
+ pp = unflatten_dt_alloc(mem, sizeof(struct property),
+ __alignof__(struct property));

- pp->name = (char *)pname;
- pp->length = sz;
- pp->value = (__be32 *)val;
- *pprev = pp;
- pprev = &pp->next;
+ if (dryrun)
+ continue;
+
+ if (!prop_is_phandle) {
+ pp->name = (char *)pname;
+ pp->length = sz;
+ pp->value = (__be32 *)val;
+ *pprev = pp;
+ pprev = &pp->next;
+ } else if (prop_is_ibm_phandle || !np->phandle) {
+ np->phandle = be32_to_cpup(val);
+ }
}

/* With version 0x10 we may not have the name property,
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 18bbb4517e25..33f11a5384f3 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -47,6 +47,7 @@ extern int of_property_notify(int action, struct device_node *np,
extern void of_node_release(struct kobject *kobj);
extern int __of_changeset_apply(struct of_changeset *ocs);
extern int __of_changeset_revert(struct of_changeset *ocs);
+extern int __of_add_phandle_sysfs(struct device_node *np);
#else /* CONFIG_OF_DYNAMIC */
static inline int of_property_notify(int action, struct device_node *np,
struct property *prop, struct property *old_prop)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 7827786718d8..ca0b85f5deb1 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -101,9 +101,7 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
tprop = of_find_property(target, prop->name, NULL);

/* special properties are not meant to be updated (silent NOP) */
- if (of_prop_cmp(prop->name, "name") == 0 ||
- of_prop_cmp(prop->name, "phandle") == 0 ||
- of_prop_cmp(prop->name, "linux,phandle") == 0)
+ if (!of_prop_cmp(prop->name, "name"))
return 0;

propn = __of_prop_dup(prop, GFP_KERNEL);
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 7ae9863cb0a4..624cbaeae0a4 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -71,30 +71,11 @@ static void adjust_overlay_phandles(struct device_node *overlay,
int phandle_delta)
{
struct device_node *child;
- struct property *prop;
- phandle phandle;

/* adjust node's phandle in node */
if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
overlay->phandle += phandle_delta;

- /* copy adjusted phandle into *phandle properties */
- for_each_property_of_node(overlay, prop) {
-
- if (of_prop_cmp(prop->name, "phandle") &&
- of_prop_cmp(prop->name, "linux,phandle"))
- continue;
-
- if (prop->length < 4)
- continue;
-
- phandle = be32_to_cpup(prop->value);
- if (phandle == OF_PHANDLE_ILLEGAL)
- continue;
-
- *(uint32_t *)prop->value = cpu_to_be32(overlay->phandle);
- }
-
for_each_child_of_node(overlay, child)
adjust_overlay_phandles(child, phandle_delta);
}
@@ -197,9 +178,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
for_each_property_of_node(local_fixups, prop_fix) {

/* skip properties added automatically */
- if (!of_prop_cmp(prop_fix->name, "name") ||
- !of_prop_cmp(prop_fix->name, "phandle") ||
- !of_prop_cmp(prop_fix->name, "linux,phandle"))
+ if (!of_prop_cmp(prop_fix->name, "name"))
continue;

if ((prop_fix->length % 4) != 0 || prop_fix->length == 0)
diff --git a/include/linux/of.h b/include/linux/of.h
index 21e6323de0f3..4e86e05853f3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -61,6 +61,7 @@ struct device_node {
struct kobject kobj;
unsigned long _flags;
void *data;
+ struct bin_attribute attr_phandle;
#if defined(CONFIG_SPARC)
const char *path_component_name;
unsigned int unique_id;
--
Frank Rowand <[email protected]>

2017-04-25 08:49:48

by Frank Rowand

[permalink] [raw]
Subject: [PATCH v3 2/4] of: make __of_attach_node() static

From: Frank Rowand <[email protected]>

__of_attach_node() is not used outside of drivers/of/dynamic.c. Make
it static and remove it from drivers/of/of_private.h.

Signed-off-by: Frank Rowand <[email protected]>
---
drivers/of/dynamic.c | 2 +-
drivers/of/of_private.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 59545b8fbf46..0b9cf6d5914c 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -216,7 +216,7 @@ int of_property_notify(int action, struct device_node *np,
return of_reconfig_notify(action, &pr);
}

-void __of_attach_node(struct device_node *np)
+static void __of_attach_node(struct device_node *np)
{
np->child = NULL;
np->sibling = np->parent->child;
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 33f11a5384f3..b1ff70e1fdda 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -79,7 +79,6 @@ extern int __of_update_property(struct device_node *np,
extern void __of_update_property_sysfs(struct device_node *np,
struct property *newprop, struct property *oldprop);

-extern void __of_attach_node(struct device_node *np);
extern int __of_attach_node_sysfs(struct device_node *np);
extern void __of_detach_node(struct device_node *np);
extern void __of_detach_node_sysfs(struct device_node *np);
--
Frank Rowand <[email protected]>

2017-04-25 08:49:58

by Frank Rowand

[permalink] [raw]
Subject: [PATCH v3 4/4] of: detect invalid phandle in overlay

From: Frank Rowand <[email protected]>

Overlays are not allowed to modify phandle values of previously existing
nodes because there is no information available to allow fixup up
properties that use the previously existing phandle.

Signed-off-by: Frank Rowand <[email protected]>
---
drivers/of/overlay.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index ca0b85f5deb1..20ab49d2f7a4 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -130,6 +130,10 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
/* NOTE: Multiple mods of created nodes not supported */
tchild = of_get_child_by_name(target, cname);
if (tchild != NULL) {
+ /* new overlay phandle value conflicts with existing value */
+ if (child->phandle)
+ return -EINVAL;
+
/* apply overlay recursively */
ret = of_overlay_apply_one(ov, tchild, child);
of_node_put(tchild);
--
Frank Rowand <[email protected]>

2017-04-25 08:50:06

by Frank Rowand

[permalink] [raw]
Subject: [PATCH v3 3/4] of: be consistent in form of file mode

From: Frank Rowand <[email protected]>

checkpatch whined about using S_IRUGO instead of octal equivalent
when adding phandle sysfs code, so used octal in that patch.
Change other instances of the S_* constants in the same file to
the octal form.

Signed-off-by: Frank Rowand <[email protected]>
---
drivers/of/base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 41473b1e2445..9fe42346925b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -168,7 +168,7 @@ int __of_add_property_sysfs(struct device_node *np, struct property *pp)

sysfs_bin_attr_init(&pp->attr);
pp->attr.attr.name = safe_name(&np->kobj, pp->name);
- pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
+ pp->attr.attr.mode = secure ? 0400 : 0444;
pp->attr.size = secure ? 0 : pp->length;
pp->attr.read = of_node_property_read;

--
Frank Rowand <[email protected]>

2017-04-25 08:52:20

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] of: remove *phandle properties from expanded device tree

On 04/25/17 01:47, [email protected] wrote:
> From: Frank Rowand <[email protected]>
>
> Remove "phandle" and "linux,phandle" properties from the internal
> device tree. The phandle will still be in the struct device_node
> phandle field.
>
> This is to resolve the issue found by Stephen Boyd [1] when he changed
> the type of struct property.value from void * to const void *. As
> a result of the type change, the overlay code had compile errors
> where the resolver updates phandle values.
>
> [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
>
> Patch 1 is the phandle related changes.
>
> Patches 2 - 4 are minor fixups for issues that became visible
> while implementing patch 1.
>
> Changes from v1:

^^^^^^^^
Changes from v2


> - patch 1: Remove check in __of_add_phandle_sysfs() that would not
> add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)
>
> Changes from v1:
> - Remove phandle properties in of_attach_node(), before attaching
> the node to the live tree.
> - Add the phandle sysfs entry for the node in of_attach_node().
> - When creating an overlay changeset, duplicate the node phandle in
> __of_node_dup().
>
> Frank Rowand (4):
> of: remove *phandle properties from expanded device tree
> of: make __of_attach_node() static
> of: be consistent in form of file mode
> of: detect invalid phandle in overlay
>
> drivers/of/base.c | 50 +++++++++++++++++++++++++++++++++++++++----
> drivers/of/dynamic.c | 56 ++++++++++++++++++++++++++++++++++++-------------
> drivers/of/fdt.c | 40 +++++++++++++++++++++--------------
> drivers/of/of_private.h | 2 +-
> drivers/of/overlay.c | 8 ++++---
> drivers/of/resolver.c | 23 +-------------------
> include/linux/of.h | 1 +
> 7 files changed, 120 insertions(+), 60 deletions(-)
>

2017-04-30 00:22:43

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] of: remove *phandle properties from expanded device tree

Hi Frank,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.11-rc8 next-20170428]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/frowand-list-gmail-com/of-remove-phandle-properties-from-expanded-device-tree/20170426-000149
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: s390-allmodconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390

All errors (new ones prefixed by >>):

In file included from include/linux/kobject.h:21:0,
from include/linux/device.h:17,
from include/linux/node.h:17,
from include/linux/cpu.h:16,
from drivers/of/base.c:25:
drivers/of/base.c: In function '__of_add_phandle_sysfs':
>> drivers/of/base.c:198:23: error: 'pp' undeclared (first use in this function)
sysfs_bin_attr_init(&pp->attr);
^
include/linux/sysfs.h:54:3: note: in definition of macro 'sysfs_attr_init'
(attr)->key = &__key; \
^~~~
drivers/of/base.c:198:2: note: in expansion of macro 'sysfs_bin_attr_init'
sysfs_bin_attr_init(&pp->attr);
^~~~~~~~~~~~~~~~~~~
drivers/of/base.c:198:23: note: each undeclared identifier is reported only once for each function it appears in
sysfs_bin_attr_init(&pp->attr);
^
include/linux/sysfs.h:54:3: note: in definition of macro 'sysfs_attr_init'
(attr)->key = &__key; \
^~~~
drivers/of/base.c:198:2: note: in expansion of macro 'sysfs_bin_attr_init'
sysfs_bin_attr_init(&pp->attr);
^~~~~~~~~~~~~~~~~~~

vim +/pp +198 drivers/of/base.c

19 */
20
21 #define pr_fmt(fmt) "OF: " fmt
22
23 #include <linux/console.h>
24 #include <linux/ctype.h>
> 25 #include <linux/cpu.h>
26 #include <linux/module.h>
27 #include <linux/of.h>
28 #include <linux/of_device.h>
29 #include <linux/of_graph.h>
30 #include <linux/spinlock.h>
31 #include <linux/slab.h>
32 #include <linux/string.h>
33 #include <linux/proc_fs.h>
34
35 #include "of_private.h"
36
37 LIST_HEAD(aliases_lookup);
38
39 struct device_node *of_root;
40 EXPORT_SYMBOL(of_root);
41 struct device_node *of_chosen;
42 struct device_node *of_aliases;
43 struct device_node *of_stdout;
44 static const char *of_stdout_options;
45
46 struct kset *of_kset;
47
48 /*
49 * Used to protect the of_aliases, to hold off addition of nodes to sysfs.
50 * This mutex must be held whenever modifications are being made to the
51 * device tree. The of_{attach,detach}_node() and
52 * of_{add,remove,update}_property() helpers make sure this happens.
53 */
54 DEFINE_MUTEX(of_mutex);
55
56 /* use when traversing tree through the child, sibling,
57 * or parent members of struct device_node.
58 */
59 DEFINE_RAW_SPINLOCK(devtree_lock);
60
61 int of_n_addr_cells(struct device_node *np)
62 {
63 const __be32 *ip;
64
65 do {
66 if (np->parent)
67 np = np->parent;
68 ip = of_get_property(np, "#address-cells", NULL);
69 if (ip)
70 return be32_to_cpup(ip);
71 } while (np->parent);
72 /* No #address-cells property for the root node */
73 return OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
74 }
75 EXPORT_SYMBOL(of_n_addr_cells);
76
77 int of_n_size_cells(struct device_node *np)
78 {
79 const __be32 *ip;
80
81 do {
82 if (np->parent)
83 np = np->parent;
84 ip = of_get_property(np, "#size-cells", NULL);
85 if (ip)
86 return be32_to_cpup(ip);
87 } while (np->parent);
88 /* No #size-cells property for the root node */
89 return OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
90 }
91 EXPORT_SYMBOL(of_n_size_cells);
92
93 #ifdef CONFIG_NUMA
94 int __weak of_node_to_nid(struct device_node *np)
95 {
96 return NUMA_NO_NODE;
97 }
98 #endif
99
100 #ifndef CONFIG_OF_DYNAMIC
101 static void of_node_release(struct kobject *kobj)
102 {
103 /* Without CONFIG_OF_DYNAMIC, no nodes gets freed */
104 }
105 #endif /* CONFIG_OF_DYNAMIC */
106
107 struct kobj_type of_node_ktype = {
108 .release = of_node_release,
109 };
110
111 static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
112 struct bin_attribute *bin_attr, char *buf,
113 loff_t offset, size_t count)
114 {
115 struct property *pp = container_of(bin_attr, struct property, attr);
116 return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
117 }
118
119 static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
120 struct bin_attribute *bin_attr, char *buf,
121 loff_t offset, size_t count)
122 {
123 phandle phandle;
124 struct device_node *np;
125
126 np = container_of(bin_attr, struct device_node, attr_phandle);
127 phandle = cpu_to_be32(np->phandle);
128 return memory_read_from_buffer(buf, count, &offset, &phandle,
129 sizeof(phandle));
130 }
131
132 /* always return newly allocated name, caller must free after use */
133 static const char *safe_name(struct kobject *kobj, const char *orig_name)
134 {
135 const char *name = orig_name;
136 struct kernfs_node *kn;
137 int i = 0;
138
139 /* don't be a hero. After 16 tries give up */
140 while (i < 16 && (kn = sysfs_get_dirent(kobj->sd, name))) {
141 sysfs_put(kn);
142 if (name != orig_name)
143 kfree(name);
144 name = kasprintf(GFP_KERNEL, "%s#%i", orig_name, ++i);
145 }
146
147 if (name == orig_name) {
148 name = kstrdup(orig_name, GFP_KERNEL);
149 } else {
150 pr_warn("Duplicate name in %s, renamed to \"%s\"\n",
151 kobject_name(kobj), name);
152 }
153 return name;
154 }
155
156 int __of_add_property_sysfs(struct device_node *np, struct property *pp)
157 {
158 int rc;
159
160 /* Important: Don't leak passwords */
161 bool secure = strncmp(pp->name, "security-", 9) == 0;
162
163 if (!IS_ENABLED(CONFIG_SYSFS))
164 return 0;
165
166 if (!of_kset || !of_node_is_attached(np))
167 return 0;
168
169 sysfs_bin_attr_init(&pp->attr);
170 pp->attr.attr.name = safe_name(&np->kobj, pp->name);
171 pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
172 pp->attr.size = secure ? 0 : pp->length;
173 pp->attr.read = of_node_property_read;
174
175 rc = sysfs_create_bin_file(&np->kobj, &pp->attr);
176 WARN(rc, "error adding attribute %s to node %s\n", pp->name, np->full_name);
177 return rc;
178 }
179
180 /*
181 * In the imported device tree (fdt), phandle is a property. In the
182 * internal data structure it is instead stored in the struct device_node.
183 * Make phandle visible in sysfs as if it was a property.
184 */
185 int __of_add_phandle_sysfs(struct device_node *np)
186 {
187 int rc;
188
189 if (!IS_ENABLED(CONFIG_SYSFS))
190 return 0;
191
192 if (!of_kset || !of_node_is_attached(np))
193 return 0;
194
195 if (!np->phandle || np->phandle == 0xffffffff)
196 return 0;
197
> 198 sysfs_bin_attr_init(&pp->attr);
199 np->attr_phandle.attr.name = "phandle";
200 np->attr_phandle.attr.mode = 0444;
201 np->attr_phandle.size = sizeof(np->phandle);

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (7.88 kB)
.config.gz (44.01 kB)
Download all attachments

2017-04-30 20:49:56

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] of: remove *phandle properties from expanded device tree

On 04/29/17 17:22, kbuild test robot wrote:
> Hi Frank,
>
> [auto build test ERROR on robh/for-next]
> [also build test ERROR on v4.11-rc8 next-20170428]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/frowand-list-gmail-com/of-remove-phandle-properties-from-expanded-device-tree/20170426-000149
> base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> config: s390-allmodconfig (attached as .config)
> compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=s390
>
> All errors (new ones prefixed by >>):
>
> In file included from include/linux/kobject.h:21:0,
> from include/linux/device.h:17,
> from include/linux/node.h:17,
> from include/linux/cpu.h:16,
> from drivers/of/base.c:25:
> drivers/of/base.c: In function '__of_add_phandle_sysfs':
>>> drivers/of/base.c:198:23: error: 'pp' undeclared (first use in this function)
> sysfs_bin_attr_init(&pp->attr);
> ^

Thanks for the report!

A patch to fix this is now submitted to Rob.

-Frank

> include/linux/sysfs.h:54:3: note: in definition of macro 'sysfs_attr_init'
> (attr)->key = &__key; \
> ^~~~
> drivers/of/base.c:198:2: note: in expansion of macro 'sysfs_bin_attr_init'
> sysfs_bin_attr_init(&pp->attr);
> ^~~~~~~~~~~~~~~~~~~
> drivers/of/base.c:198:23: note: each undeclared identifier is reported only once for each function it appears in
> sysfs_bin_attr_init(&pp->attr);
> ^
> include/linux/sysfs.h:54:3: note: in definition of macro 'sysfs_attr_init'
> (attr)->key = &__key; \
> ^~~~
> drivers/of/base.c:198:2: note: in expansion of macro 'sysfs_bin_attr_init'
> sysfs_bin_attr_init(&pp->attr);
> ^~~~~~~~~~~~~~~~~~~
>
> vim +/pp +198 drivers/of/base.c
>
> 19 */
> 20
> 21 #define pr_fmt(fmt) "OF: " fmt
> 22
> 23 #include <linux/console.h>
> 24 #include <linux/ctype.h>
> > 25 #include <linux/cpu.h>
> 26 #include <linux/module.h>
> 27 #include <linux/of.h>
> 28 #include <linux/of_device.h>
> 29 #include <linux/of_graph.h>
> 30 #include <linux/spinlock.h>
> 31 #include <linux/slab.h>
> 32 #include <linux/string.h>
> 33 #include <linux/proc_fs.h>
> 34
> 35 #include "of_private.h"
> 36
> 37 LIST_HEAD(aliases_lookup);
> 38
> 39 struct device_node *of_root;
> 40 EXPORT_SYMBOL(of_root);
> 41 struct device_node *of_chosen;
> 42 struct device_node *of_aliases;
> 43 struct device_node *of_stdout;
> 44 static const char *of_stdout_options;
> 45
> 46 struct kset *of_kset;
> 47
> 48 /*
> 49 * Used to protect the of_aliases, to hold off addition of nodes to sysfs.
> 50 * This mutex must be held whenever modifications are being made to the
> 51 * device tree. The of_{attach,detach}_node() and
> 52 * of_{add,remove,update}_property() helpers make sure this happens.
> 53 */
> 54 DEFINE_MUTEX(of_mutex);
> 55
> 56 /* use when traversing tree through the child, sibling,
> 57 * or parent members of struct device_node.
> 58 */
> 59 DEFINE_RAW_SPINLOCK(devtree_lock);
> 60
> 61 int of_n_addr_cells(struct device_node *np)
> 62 {
> 63 const __be32 *ip;
> 64
> 65 do {
> 66 if (np->parent)
> 67 np = np->parent;
> 68 ip = of_get_property(np, "#address-cells", NULL);
> 69 if (ip)
> 70 return be32_to_cpup(ip);
> 71 } while (np->parent);
> 72 /* No #address-cells property for the root node */
> 73 return OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
> 74 }
> 75 EXPORT_SYMBOL(of_n_addr_cells);
> 76
> 77 int of_n_size_cells(struct device_node *np)
> 78 {
> 79 const __be32 *ip;
> 80
> 81 do {
> 82 if (np->parent)
> 83 np = np->parent;
> 84 ip = of_get_property(np, "#size-cells", NULL);
> 85 if (ip)
> 86 return be32_to_cpup(ip);
> 87 } while (np->parent);
> 88 /* No #size-cells property for the root node */
> 89 return OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
> 90 }
> 91 EXPORT_SYMBOL(of_n_size_cells);
> 92
> 93 #ifdef CONFIG_NUMA
> 94 int __weak of_node_to_nid(struct device_node *np)
> 95 {
> 96 return NUMA_NO_NODE;
> 97 }
> 98 #endif
> 99
> 100 #ifndef CONFIG_OF_DYNAMIC
> 101 static void of_node_release(struct kobject *kobj)
> 102 {
> 103 /* Without CONFIG_OF_DYNAMIC, no nodes gets freed */
> 104 }
> 105 #endif /* CONFIG_OF_DYNAMIC */
> 106
> 107 struct kobj_type of_node_ktype = {
> 108 .release = of_node_release,
> 109 };
> 110
> 111 static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
> 112 struct bin_attribute *bin_attr, char *buf,
> 113 loff_t offset, size_t count)
> 114 {
> 115 struct property *pp = container_of(bin_attr, struct property, attr);
> 116 return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
> 117 }
> 118
> 119 static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
> 120 struct bin_attribute *bin_attr, char *buf,
> 121 loff_t offset, size_t count)
> 122 {
> 123 phandle phandle;
> 124 struct device_node *np;
> 125
> 126 np = container_of(bin_attr, struct device_node, attr_phandle);
> 127 phandle = cpu_to_be32(np->phandle);
> 128 return memory_read_from_buffer(buf, count, &offset, &phandle,
> 129 sizeof(phandle));
> 130 }
> 131
> 132 /* always return newly allocated name, caller must free after use */
> 133 static const char *safe_name(struct kobject *kobj, const char *orig_name)
> 134 {
> 135 const char *name = orig_name;
> 136 struct kernfs_node *kn;
> 137 int i = 0;
> 138
> 139 /* don't be a hero. After 16 tries give up */
> 140 while (i < 16 && (kn = sysfs_get_dirent(kobj->sd, name))) {
> 141 sysfs_put(kn);
> 142 if (name != orig_name)
> 143 kfree(name);
> 144 name = kasprintf(GFP_KERNEL, "%s#%i", orig_name, ++i);
> 145 }
> 146
> 147 if (name == orig_name) {
> 148 name = kstrdup(orig_name, GFP_KERNEL);
> 149 } else {
> 150 pr_warn("Duplicate name in %s, renamed to \"%s\"\n",
> 151 kobject_name(kobj), name);
> 152 }
> 153 return name;
> 154 }
> 155
> 156 int __of_add_property_sysfs(struct device_node *np, struct property *pp)
> 157 {
> 158 int rc;
> 159
> 160 /* Important: Don't leak passwords */
> 161 bool secure = strncmp(pp->name, "security-", 9) == 0;
> 162
> 163 if (!IS_ENABLED(CONFIG_SYSFS))
> 164 return 0;
> 165
> 166 if (!of_kset || !of_node_is_attached(np))
> 167 return 0;
> 168
> 169 sysfs_bin_attr_init(&pp->attr);
> 170 pp->attr.attr.name = safe_name(&np->kobj, pp->name);
> 171 pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
> 172 pp->attr.size = secure ? 0 : pp->length;
> 173 pp->attr.read = of_node_property_read;
> 174
> 175 rc = sysfs_create_bin_file(&np->kobj, &pp->attr);
> 176 WARN(rc, "error adding attribute %s to node %s\n", pp->name, np->full_name);
> 177 return rc;
> 178 }
> 179
> 180 /*
> 181 * In the imported device tree (fdt), phandle is a property. In the
> 182 * internal data structure it is instead stored in the struct device_node.
> 183 * Make phandle visible in sysfs as if it was a property.
> 184 */
> 185 int __of_add_phandle_sysfs(struct device_node *np)
> 186 {
> 187 int rc;
> 188
> 189 if (!IS_ENABLED(CONFIG_SYSFS))
> 190 return 0;
> 191
> 192 if (!of_kset || !of_node_is_attached(np))
> 193 return 0;
> 194
> 195 if (!np->phandle || np->phandle == 0xffffffff)
> 196 return 0;
> 197
> > 198 sysfs_bin_attr_init(&pp->attr);
> 199 np->attr_phandle.attr.name = "phandle";
> 200 np->attr_phandle.attr.mode = 0444;
> 201 np->attr_phandle.size = sizeof(np->phandle);
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>