* [PATCH v3 0/4] of: remove *phandle properties from expanded device tree @ 2017-04-25 8:47 frowand.list 2017-04-25 8:47 ` [PATCH v3 1/4] " frowand.list ` (4 more replies) 0 siblings, 5 replies; 8+ messages in thread From: frowand.list @ 2017-04-25 8:47 UTC (permalink / raw) To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel From: Frank Rowand <frank.rowand@sony.com> 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 <frank.rowand@sony.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/4] of: remove *phandle properties from expanded device tree 2017-04-25 8:47 [PATCH v3 0/4] of: remove *phandle properties from expanded device tree frowand.list @ 2017-04-25 8:47 ` frowand.list 2017-04-30 0:22 ` kbuild test robot 2017-04-25 8:47 ` [PATCH v3 2/4] of: make __of_attach_node() static frowand.list ` (3 subsequent siblings) 4 siblings, 1 reply; 8+ messages in thread From: frowand.list @ 2017-04-25 8:47 UTC (permalink / raw) To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel From: Frank Rowand <frank.rowand@sony.com> 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 <frank.rowand@sony.com> --- 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 <frank.rowand@sony.com> ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/4] of: remove *phandle properties from expanded device tree 2017-04-25 8:47 ` [PATCH v3 1/4] " frowand.list @ 2017-04-30 0:22 ` kbuild test robot 2017-04-30 20:49 ` Frank Rowand 0 siblings, 1 reply; 8+ messages in thread From: kbuild test robot @ 2017-04-30 0:22 UTC (permalink / raw) To: frowand.list Cc: kbuild-all, Rob Herring, stephen.boyd, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 8074 bytes --] 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 [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 45067 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/4] of: remove *phandle properties from expanded device tree 2017-04-30 0:22 ` kbuild test robot @ 2017-04-30 20:49 ` Frank Rowand 0 siblings, 0 replies; 8+ messages in thread From: Frank Rowand @ 2017-04-30 20:49 UTC (permalink / raw) To: kbuild test robot Cc: kbuild-all, Rob Herring, stephen.boyd, devicetree, linux-kernel 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 2/4] of: make __of_attach_node() static 2017-04-25 8:47 [PATCH v3 0/4] of: remove *phandle properties from expanded device tree frowand.list 2017-04-25 8:47 ` [PATCH v3 1/4] " frowand.list @ 2017-04-25 8:47 ` frowand.list 2017-04-25 8:47 ` [PATCH v3 3/4] of: be consistent in form of file mode frowand.list ` (2 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: frowand.list @ 2017-04-25 8:47 UTC (permalink / raw) To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel From: Frank Rowand <frank.rowand@sony.com> __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 <frank.rowand@sony.com> --- 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 <frank.rowand@sony.com> ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 3/4] of: be consistent in form of file mode 2017-04-25 8:47 [PATCH v3 0/4] of: remove *phandle properties from expanded device tree frowand.list 2017-04-25 8:47 ` [PATCH v3 1/4] " frowand.list 2017-04-25 8:47 ` [PATCH v3 2/4] of: make __of_attach_node() static frowand.list @ 2017-04-25 8:47 ` frowand.list 2017-04-25 8:47 ` [PATCH v3 4/4] of: detect invalid phandle in overlay frowand.list 2017-04-25 8:51 ` [PATCH v3 0/4] of: remove *phandle properties from expanded device tree Frank Rowand 4 siblings, 0 replies; 8+ messages in thread From: frowand.list @ 2017-04-25 8:47 UTC (permalink / raw) To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel From: Frank Rowand <frank.rowand@sony.com> 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 <frank.rowand@sony.com> --- 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 <frank.rowand@sony.com> ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 4/4] of: detect invalid phandle in overlay 2017-04-25 8:47 [PATCH v3 0/4] of: remove *phandle properties from expanded device tree frowand.list ` (2 preceding siblings ...) 2017-04-25 8:47 ` [PATCH v3 3/4] of: be consistent in form of file mode frowand.list @ 2017-04-25 8:47 ` frowand.list 2017-04-25 8:51 ` [PATCH v3 0/4] of: remove *phandle properties from expanded device tree Frank Rowand 4 siblings, 0 replies; 8+ messages in thread From: frowand.list @ 2017-04-25 8:47 UTC (permalink / raw) To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel From: Frank Rowand <frank.rowand@sony.com> 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 <frank.rowand@sony.com> --- 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 <frank.rowand@sony.com> ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/4] of: remove *phandle properties from expanded device tree 2017-04-25 8:47 [PATCH v3 0/4] of: remove *phandle properties from expanded device tree frowand.list ` (3 preceding siblings ...) 2017-04-25 8:47 ` [PATCH v3 4/4] of: detect invalid phandle in overlay frowand.list @ 2017-04-25 8:51 ` Frank Rowand 4 siblings, 0 replies; 8+ messages in thread From: Frank Rowand @ 2017-04-25 8:51 UTC (permalink / raw) To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel On 04/25/17 01:47, frowand.list@gmail.com wrote: > From: Frank Rowand <frank.rowand@sony.com> > > 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(-) > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-30 20:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-25 8:47 [PATCH v3 0/4] of: remove *phandle properties from expanded device tree frowand.list 2017-04-25 8:47 ` [PATCH v3 1/4] " frowand.list 2017-04-30 0:22 ` kbuild test robot 2017-04-30 20:49 ` Frank Rowand 2017-04-25 8:47 ` [PATCH v3 2/4] of: make __of_attach_node() static frowand.list 2017-04-25 8:47 ` [PATCH v3 3/4] of: be consistent in form of file mode frowand.list 2017-04-25 8:47 ` [PATCH v3 4/4] of: detect invalid phandle in overlay frowand.list 2017-04-25 8:51 ` [PATCH v3 0/4] of: remove *phandle properties from expanded device tree Frank Rowand
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).