From a7e4cfb0a7ca4773e7d0dd1d9c018ab27a15360e Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 1 Jun 2017 18:01:47 -0500 Subject: of/fdt: only store the device node basename in full_name With dependencies on a statically allocated full path name converted to use %pOF format specifier, we can store just the basename of node, and the unflattening of the FDT can be simplified. This commit will affect the remaining users of full_name. After analyzing these users, the remaining cases should only change some print messages. The main users of full_name are providing a name for struct resource. The resource names shouldn't be important other than providing /proc/iomem names. We no longer distinguish between pre and post 0x10 dtb formats as either a full path or basename will work. However, less than 0x10 formats have been broken since the conversion to use libfdt (and no one has cared). The conversion of the unflattening code to be non-recursive also broke pre 0x10 formats as the populate_node function would return 0 in that case. Signed-off-by: Rob Herring --- drivers/of/fdt.c | 69 +++++++++----------------------------------------------- 1 file changed, 11 insertions(+), 58 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index ce30c9a588a4..27c535af0be8 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -266,74 +266,32 @@ static void populate_properties(const void *blob, *pprev = NULL; } -static unsigned int populate_node(const void *blob, - int offset, - void **mem, - struct device_node *dad, - unsigned int fpsize, - struct device_node **pnp, - bool dryrun) +static bool populate_node(const void *blob, + int offset, + void **mem, + struct device_node *dad, + struct device_node **pnp, + bool dryrun) { struct device_node *np; const char *pathp; unsigned int l, allocl; - int new_format = 0; pathp = fdt_get_name(blob, offset, &l); if (!pathp) { *pnp = NULL; - return 0; + return false; } allocl = ++l; - /* version 0x10 has a more compact unit name here instead of the full - * path. we accumulate the full path size using "fpsize", we'll rebuild - * it later. We detect this because the first character of the name is - * not '/'. - */ - if ((*pathp) != '/') { - new_format = 1; - if (fpsize == 0) { - /* root node: special case. fpsize accounts for path - * plus terminating zero. root node only has '/', so - * fpsize should be 2, but we want to avoid the first - * level nodes to have two '/' so we use fpsize 1 here - */ - fpsize = 1; - allocl = 2; - l = 1; - pathp = ""; - } else { - /* account for '/' and path size minus terminal 0 - * already in 'l' - */ - fpsize += l; - allocl = fpsize; - } - } - np = unflatten_dt_alloc(mem, sizeof(struct device_node) + allocl, __alignof__(struct device_node)); if (!dryrun) { char *fn; of_node_init(np); np->full_name = fn = ((char *)np) + sizeof(*np); - if (new_format) { - /* rebuild full path for new format */ - if (dad && dad->parent) { - strcpy(fn, dad->full_name); -#ifdef DEBUG - if ((strlen(fn) + l + 1) != allocl) { - pr_debug("%s: p: %d, l: %d, a: %d\n", - pathp, (int)strlen(fn), - l, allocl); - } -#endif - fn += strlen(fn); - } - *(fn++) = '/'; - } + memcpy(fn, pathp, l); if (dad != NULL) { @@ -355,7 +313,7 @@ static unsigned int populate_node(const void *blob, } *pnp = np; - return fpsize; + return true; } static void reverse_nodes(struct device_node *parent) @@ -399,7 +357,6 @@ static int unflatten_dt_nodes(const void *blob, struct device_node *root; int offset = 0, depth = 0, initial_depth = 0; #define FDT_MAX_DEPTH 64 - unsigned int fpsizes[FDT_MAX_DEPTH]; struct device_node *nps[FDT_MAX_DEPTH]; void *base = mem; bool dryrun = !base; @@ -418,7 +375,6 @@ static int unflatten_dt_nodes(const void *blob, depth = initial_depth = 1; root = dad; - fpsizes[depth] = dad ? strlen(of_node_full_name(dad)) : 0; nps[depth] = dad; for (offset = 0; @@ -427,11 +383,8 @@ static int unflatten_dt_nodes(const void *blob, if (WARN_ON_ONCE(depth >= FDT_MAX_DEPTH)) continue; - fpsizes[depth+1] = populate_node(blob, offset, &mem, - nps[depth], - fpsizes[depth], - &nps[depth+1], dryrun); - if (!fpsizes[depth+1]) + if (!populate_node(blob, offset, &mem, nps[depth], + &nps[depth+1], dryrun)) return mem - base; if (!dryrun && nodepp && !*nodepp) -- cgit From bd82bbf38cbe27f2c65660da801900d71bcc5cc8 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 5 Oct 2017 12:01:46 -0500 Subject: of: fix missing kobject init for !SYSFS && OF_DYNAMIC config The ref counting is broken for OF_DYNAMIC when sysfs is disabled because the kobject initialization is skipped. Only the properties add/remove/update should be skipped for !SYSFS config. Tested-by: Nicolas Pitre Reviewed-by: Frank Rowand Acked-by: Grant Likely Signed-off-by: Rob Herring --- drivers/of/base.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/base.c b/drivers/of/base.c index 260d33c0f26c..9e9bd17121fb 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -169,9 +169,6 @@ int __of_attach_node_sysfs(struct device_node *np) struct property *pp; int rc; - if (!IS_ENABLED(CONFIG_SYSFS)) - return 0; - if (!of_kset) return 0; -- cgit From 4c2bb57400394e4ec631a42dcf637e925db6202e Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 4 Oct 2017 19:30:53 -0500 Subject: of: move kobj_to_device_node() into dynamic.c The only user of kobj_to_device_node() is in dynamic.c, so move it there. This avoids having to make it conditional once kobject is configurable. Tested-by: Nicolas Pitre Reviewed-by: Frank Rowand Acked-by: Grant Likely Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 5 +++++ drivers/of/of_private.h | 6 ------ 2 files changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 301b6db2b48d..9d6ba18c529f 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -16,6 +16,11 @@ #include "of_private.h" +static struct device_node *kobj_to_device_node(struct kobject *kobj) +{ + return container_of(kobj, struct device_node, kobj); +} + /** * of_node_get() - Increment refcount of a node * @node: Node to inc refcount, NULL is supported to simplify writing of diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 3ae12ffbf547..cc86a974f35f 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -35,12 +35,6 @@ extern struct mutex of_mutex; extern struct list_head aliases_lookup; extern struct kset *of_kset; - -static inline struct device_node *kobj_to_device_node(struct kobject *kobj) -{ - return container_of(kobj, struct device_node, kobj); -} - #if defined(CONFIG_OF_DYNAMIC) extern int of_property_notify(int action, struct device_node *np, struct property *prop, struct property *old_prop); -- cgit From b56b5528f5b3c3d47e7c0ca67318c45e980d93f0 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 4 Oct 2017 14:09:40 -0500 Subject: of: make kobject and bin_attribute support configurable Having device_nodes be kobjects is only needed if sysfs or OF_DYNAMIC is enabled. Otherwise, having a kobject in struct device_node is unnecessary bloat in minimal kernel configurations. Likewise, bin_attribute is only needed in struct property when sysfs is enabled, so we can make it configurable too. Tested-by: Nicolas Pitre Reviewed-by: Frank Rowand Acked-by: Grant Likely Signed-off-by: Rob Herring --- drivers/of/Kconfig | 4 ++ drivers/of/Makefile | 1 + drivers/of/base.c | 130 -------------------------------------- drivers/of/dynamic.c | 22 ------- drivers/of/kobj.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++++ drivers/of/of_private.h | 23 +++++++ 6 files changed, 192 insertions(+), 152 deletions(-) create mode 100644 drivers/of/kobj.c (limited to 'drivers/of') diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index ba7b034b2b91..ad9a9578f9c4 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -46,10 +46,14 @@ config OF_EARLY_FLATTREE config OF_PROMTREE bool +config OF_KOBJ + def_bool SYSFS + # Hardly any platforms need this. It is safe to select, but only do so if you # need it. config OF_DYNAMIC bool "Support for dynamic device trees" if OF_UNITTEST + select OF_KOBJ help On some platforms, the device tree can be manipulated at runtime. While this option is selected automatically on such platforms, you diff --git a/drivers/of/Makefile b/drivers/of/Makefile index 97dc01c81438..83d61a7c5e82 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -1,4 +1,5 @@ obj-y = base.o device.o platform.o property.o +obj-$(CONFIG_OF_KOBJ) += kobj.o obj-$(CONFIG_OF_DYNAMIC) += dynamic.o obj-$(CONFIG_OF_FLATTREE) += fdt.o obj-$(CONFIG_OF_EARLY_FLATTREE) += fdt_address.o diff --git a/drivers/of/base.c b/drivers/of/base.c index 9e9bd17121fb..b98f3adffbb0 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -95,105 +95,6 @@ int __weak of_node_to_nid(struct device_node *np) } #endif -#ifndef CONFIG_OF_DYNAMIC -static void of_node_release(struct kobject *kobj) -{ - /* Without CONFIG_OF_DYNAMIC, no nodes gets freed */ -} -#endif /* CONFIG_OF_DYNAMIC */ - -struct kobj_type of_node_ktype = { - .release = of_node_release, -}; - -static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj, - struct bin_attribute *bin_attr, char *buf, - loff_t offset, size_t count) -{ - struct property *pp = container_of(bin_attr, struct property, attr); - return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length); -} - -/* always return newly allocated name, caller must free after use */ -static const char *safe_name(struct kobject *kobj, const char *orig_name) -{ - const char *name = orig_name; - struct kernfs_node *kn; - int i = 0; - - /* don't be a hero. After 16 tries give up */ - while (i < 16 && (kn = sysfs_get_dirent(kobj->sd, name))) { - sysfs_put(kn); - if (name != orig_name) - kfree(name); - name = kasprintf(GFP_KERNEL, "%s#%i", orig_name, ++i); - } - - if (name == orig_name) { - name = kstrdup(orig_name, GFP_KERNEL); - } else { - pr_warn("Duplicate name in %s, renamed to \"%s\"\n", - kobject_name(kobj), name); - } - return name; -} - -int __of_add_property_sysfs(struct device_node *np, struct property *pp) -{ - int rc; - - /* Important: Don't leak passwords */ - bool secure = strncmp(pp->name, "security-", 9) == 0; - - if (!IS_ENABLED(CONFIG_SYSFS)) - return 0; - - if (!of_kset || !of_node_is_attached(np)) - return 0; - - sysfs_bin_attr_init(&pp->attr); - pp->attr.attr.name = safe_name(&np->kobj, pp->name); - pp->attr.attr.mode = secure ? 0400 : 0444; - pp->attr.size = secure ? 0 : pp->length; - pp->attr.read = of_node_property_read; - - rc = sysfs_create_bin_file(&np->kobj, &pp->attr); - WARN(rc, "error adding attribute %s to node %pOF\n", pp->name, np); - return rc; -} - -int __of_attach_node_sysfs(struct device_node *np) -{ - const char *name; - struct kobject *parent; - struct property *pp; - int rc; - - if (!of_kset) - return 0; - - np->kobj.kset = of_kset; - if (!np->parent) { - /* Nodes without parents are new top level trees */ - name = safe_name(&of_kset->kobj, "base"); - parent = NULL; - } else { - name = safe_name(&np->parent->kobj, kbasename(np->full_name)); - parent = &np->parent->kobj; - } - if (!name) - return -ENOMEM; - rc = kobject_add(&np->kobj, parent, "%s", name); - kfree(name); - if (rc) - return rc; - - for_each_property_of_node(np, pp) - __of_add_property_sysfs(np, pp); - - return 0; -} - void __init of_core_init(void) { struct device_node *np; @@ -1501,22 +1402,6 @@ int __of_remove_property(struct device_node *np, struct property *prop) return 0; } -void __of_sysfs_remove_bin_file(struct device_node *np, struct property *prop) -{ - sysfs_remove_bin_file(&np->kobj, &prop->attr); - kfree(prop->attr.attr.name); -} - -void __of_remove_property_sysfs(struct device_node *np, struct property *prop) -{ - if (!IS_ENABLED(CONFIG_SYSFS)) - return; - - /* at early boot, bail here and defer setup to of_init() */ - if (of_kset && of_node_is_attached(np)) - __of_sysfs_remove_bin_file(np, prop); -} - /** * of_remove_property - Remove a property from a node. * @@ -1576,21 +1461,6 @@ int __of_update_property(struct device_node *np, struct property *newprop, return 0; } -void __of_update_property_sysfs(struct device_node *np, struct property *newprop, - struct property *oldprop) -{ - if (!IS_ENABLED(CONFIG_SYSFS)) - return; - - /* At early boot, bail out and defer setup to of_init() */ - if (!of_kset) - return; - - if (oldprop) - __of_sysfs_remove_bin_file(np, oldprop); - __of_add_property_sysfs(np, newprop); -} - /* * of_update_property - Update a property in a node, if the property does * not exist, add it. diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 9d6ba18c529f..39e8cf731764 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -48,28 +48,6 @@ void of_node_put(struct device_node *node) } EXPORT_SYMBOL(of_node_put); -void __of_detach_node_sysfs(struct device_node *np) -{ - struct property *pp; - - if (!IS_ENABLED(CONFIG_SYSFS)) - return; - - BUG_ON(!of_node_is_initialized(np)); - if (!of_kset) - return; - - /* only remove properties if on sysfs */ - if (of_node_is_attached(np)) { - for_each_property_of_node(np, pp) - __of_sysfs_remove_bin_file(np, pp); - kobject_del(&np->kobj); - } - - /* finally remove the kobj_init ref */ - of_node_put(np); -} - static BLOCKING_NOTIFIER_HEAD(of_reconfig_chain); int of_reconfig_notifier_register(struct notifier_block *nb) diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c new file mode 100644 index 000000000000..250fc7bb550f --- /dev/null +++ b/drivers/of/kobj.c @@ -0,0 +1,164 @@ +#include +#include + +#include "of_private.h" + +/* true when node is initialized */ +static int of_node_is_initialized(struct device_node *node) +{ + return node && node->kobj.state_initialized; +} + +/* true when node is attached (i.e. present on sysfs) */ +int of_node_is_attached(struct device_node *node) +{ + return node && node->kobj.state_in_sysfs; +} + + +#ifndef CONFIG_OF_DYNAMIC +static void of_node_release(struct kobject *kobj) +{ + /* Without CONFIG_OF_DYNAMIC, no nodes gets freed */ +} +#endif /* CONFIG_OF_DYNAMIC */ + +struct kobj_type of_node_ktype = { + .release = of_node_release, +}; + +static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj, + struct bin_attribute *bin_attr, char *buf, + loff_t offset, size_t count) +{ + struct property *pp = container_of(bin_attr, struct property, attr); + return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length); +} + +/* always return newly allocated name, caller must free after use */ +static const char *safe_name(struct kobject *kobj, const char *orig_name) +{ + const char *name = orig_name; + struct kernfs_node *kn; + int i = 0; + + /* don't be a hero. After 16 tries give up */ + while (i < 16 && (kn = sysfs_get_dirent(kobj->sd, name))) { + sysfs_put(kn); + if (name != orig_name) + kfree(name); + name = kasprintf(GFP_KERNEL, "%s#%i", orig_name, ++i); + } + + if (name == orig_name) { + name = kstrdup(orig_name, GFP_KERNEL); + } else { + pr_warn("Duplicate name in %s, renamed to \"%s\"\n", + kobject_name(kobj), name); + } + return name; +} + +int __of_add_property_sysfs(struct device_node *np, struct property *pp) +{ + int rc; + + /* Important: Don't leak passwords */ + bool secure = strncmp(pp->name, "security-", 9) == 0; + + if (!IS_ENABLED(CONFIG_SYSFS)) + return 0; + + if (!of_kset || !of_node_is_attached(np)) + return 0; + + sysfs_bin_attr_init(&pp->attr); + pp->attr.attr.name = safe_name(&np->kobj, pp->name); + pp->attr.attr.mode = secure ? 0400 : 0444; + pp->attr.size = secure ? 0 : pp->length; + pp->attr.read = of_node_property_read; + + rc = sysfs_create_bin_file(&np->kobj, &pp->attr); + WARN(rc, "error adding attribute %s to node %pOF\n", pp->name, np); + return rc; +} + +void __of_sysfs_remove_bin_file(struct device_node *np, struct property *prop) +{ + if (!IS_ENABLED(CONFIG_SYSFS)) + return; + + sysfs_remove_bin_file(&np->kobj, &prop->attr); + kfree(prop->attr.attr.name); +} + +void __of_remove_property_sysfs(struct device_node *np, struct property *prop) +{ + /* at early boot, bail here and defer setup to of_init() */ + if (of_kset && of_node_is_attached(np)) + __of_sysfs_remove_bin_file(np, prop); +} + +void __of_update_property_sysfs(struct device_node *np, struct property *newprop, + struct property *oldprop) +{ + /* At early boot, bail out and defer setup to of_init() */ + if (!of_kset) + return; + + if (oldprop) + __of_sysfs_remove_bin_file(np, oldprop); + __of_add_property_sysfs(np, newprop); +} + +int __of_attach_node_sysfs(struct device_node *np) +{ + const char *name; + struct kobject *parent; + struct property *pp; + int rc; + + if (!of_kset) + return 0; + + np->kobj.kset = of_kset; + if (!np->parent) { + /* Nodes without parents are new top level trees */ + name = safe_name(&of_kset->kobj, "base"); + parent = NULL; + } else { + name = safe_name(&np->parent->kobj, kbasename(np->full_name)); + parent = &np->parent->kobj; + } + if (!name) + return -ENOMEM; + rc = kobject_add(&np->kobj, parent, "%s", name); + kfree(name); + if (rc) + return rc; + + for_each_property_of_node(np, pp) + __of_add_property_sysfs(np, pp); + + return 0; +} + +void __of_detach_node_sysfs(struct device_node *np) +{ + struct property *pp; + + BUG_ON(!of_node_is_initialized(np)); + if (!of_kset) + return; + + /* only remove properties if on sysfs */ + if (of_node_is_attached(np)) { + for_each_property_of_node(np, pp) + __of_sysfs_remove_bin_file(np, pp); + kobject_del(&np->kobj); + } + + /* finally remove the kobj_init ref */ + of_node_put(np); +} + diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index cc86a974f35f..43df14f0cbce 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -49,6 +49,29 @@ static inline int of_property_notify(int action, struct device_node *np, } #endif /* CONFIG_OF_DYNAMIC */ +#if defined(CONFIG_OF_KOBJ) +int of_node_is_attached(struct device_node *node); +int __of_add_property_sysfs(struct device_node *np, struct property *pp); +void __of_remove_property_sysfs(struct device_node *np, struct property *prop); +void __of_update_property_sysfs(struct device_node *np, struct property *newprop, + struct property *oldprop); +int __of_attach_node_sysfs(struct device_node *np); +void __of_detach_node_sysfs(struct device_node *np); +#else +static inline int __of_add_property_sysfs(struct device_node *np, struct property *pp) +{ + return 0; +} +static inline void __of_remove_property_sysfs(struct device_node *np, struct property *prop) {} +static inline void __of_update_property_sysfs(struct device_node *np, + struct property *newprop, struct property *oldprop) {} +static inline int __of_attach_node_sysfs(struct device_node *np) +{ + return 0; +} +static inline void __of_detach_node_sysfs(struct device_node *np) {} +#endif + #if defined(CONFIG_OF_UNITTEST) && defined(CONFIG_OF_OVERLAY) extern void __init unittest_unflatten_overlay_base(void); #else -- cgit From ecc8a96e251dc43d13841bac44fbfa34ec20fc52 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 28 Sep 2017 19:20:32 -0500 Subject: of/fdt: add of_fdt_device_is_available function Add an equivalent function to of_device_is_available for flattened DT, and convert the one existing open coded occurrence. Tested-by: Nicolas Pitre Reviewed-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/fdt.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 27c535af0be8..f8c39705418b 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -132,6 +132,19 @@ bool of_fdt_is_big_endian(const void *blob, unsigned long node) return false; } +static bool of_fdt_device_is_available(const void *blob, unsigned long node) +{ + const char *status = fdt_getprop(blob, node, "status", NULL); + + if (!status) + return true; + + if (!strcmp(status, "ok") || !strcmp(status, "okay")) + return true; + + return false; +} + /** * of_fdt_match - Return true if node matches a list of compatible values */ @@ -605,7 +618,6 @@ static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname, int depth, void *data) { static int found; - const char *status; int err; if (!found && depth == 1 && strcmp(uname, "reserved-memory") == 0) { @@ -625,8 +637,7 @@ static int __init __fdt_scan_reserved_mem(unsigned long node, const char *uname, return 1; } - status = of_get_flat_dt_prop(node, "status", NULL); - if (status && strcmp(status, "okay") != 0 && strcmp(status, "ok") != 0) + if (!of_fdt_device_is_available(initial_boot_params, node)) return 0; err = __reserved_mem_reserve_reg(node, uname); -- cgit From 77ea8a68c54ede3fd60dadb6a49b597f6eb5aee7 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Tue, 3 Oct 2017 11:07:55 -0500 Subject: of/fdt: skip unflattening of disabled nodes For static DT usecases, we don't need the disabled nodes and can skip unflattening. This saves a significant amount of RAM in memory constrained cases. In one example on STM32F469, the RAM usage goes from 118K to 26K. There are a few cases in the kernel that modify the status property dynamically. These all are changes from enabled to disabled, depend on OF_DYNAMIC or are not FDT based (PDT based). Tested-by: Nicolas Pitre Reviewed-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/fdt.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers/of') diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index f8c39705418b..2d515b85a198 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -396,6 +396,10 @@ static int unflatten_dt_nodes(const void *blob, if (WARN_ON_ONCE(depth >= FDT_MAX_DEPTH)) continue; + if (!IS_ENABLED(CONFIG_OF_KOBJ) && + !of_fdt_device_is_available(blob, offset)) + continue; + if (!populate_node(blob, offset, &mem, nps[depth], &nps[depth+1], dryrun)) return mem - base; -- cgit From d35d623f52f9bf58b2dc2fa08156de936e1d6770 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Fri, 13 Oct 2017 00:35:57 -0700 Subject: of/resolver: Simplify to be32_add_cpu() This is the same as be32_add_cpu(), so simplify the code and remove the now unused local variable. Signed-off-by: Stephen Boyd Reviewed-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/resolver.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 99309cb7d372..2d58253bf2f7 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -165,7 +165,6 @@ static int adjust_local_phandle_references(struct device_node *local_fixups, struct property *prop_fix, *prop; int err, i, count; unsigned int off; - phandle phandle; if (!local_fixups) return 0; @@ -195,9 +194,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups, if ((off + 4) > prop->length) return -EINVAL; - phandle = be32_to_cpu(*(__be32 *)(prop->value + off)); - phandle += phandle_delta; - *(__be32 *)(prop->value + off) = cpu_to_be32(phandle); + be32_add_cpu(prop->value + off, phandle_delta); } } -- cgit From eeb09506c58a326e1c571b7f9ee30b7fdbc6239d Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Fri, 13 Oct 2017 00:35:58 -0700 Subject: of/resolver: Replace kmalloc + memcpy with kmemdup() Save one line. Signed-off-by: Stephen Boyd Reviewed-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/resolver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 2d58253bf2f7..bd21a66f6930 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -84,10 +84,9 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay, int offset, len; int err = 0; - value = kmalloc(prop_fixup->length, GFP_KERNEL); + value = kmemdup(prop_fixup->value, prop_fixup->length, GFP_KERNEL); if (!value) return -ENOMEM; - memcpy(value, prop_fixup->value, prop_fixup->length); /* prop_fixup contains a list of tuples of path:property_name:offset */ end = value + prop_fixup->length; -- cgit From f5d2da67714a48e9d06f27714967630c3d3b28a2 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Fri, 13 Oct 2017 00:41:29 -0700 Subject: of/fdt: Document detached argument to __unflatten_device_tree() This wasn't documented. Signed-off-by: Stephen Boyd Reviewed-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/fdt.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/of') diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 2d515b85a198..4675e5ac4d11 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -437,6 +437,7 @@ static int unflatten_dt_nodes(const void *blob, * @mynodes: The device_node tree created by the call * @dt_alloc: An allocator that provides a virtual address to memory * for the resulting tree + * @detached: if true set OF_DETACHED on @mynodes * * Returns NULL on failure or the memory chunk containing the unflattened * device tree on success. -- cgit From e0f4145685ec7b477f6166ee819125107388a249 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Fri, 13 Oct 2017 00:44:51 -0700 Subject: of: unittest: Remove redundant OF_DETACHED flag setting of_fdt_unflatten_tree() already sets the flag on the node to OF_DETACHED, because of_fdt_unflatten_tree() calls __unflatten_device_tree() with the detached bool set to true. Signed-off-by: Stephen Boyd Reviewed-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/unittest.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 29a35cb1da64..add02cfbfb88 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -993,7 +993,6 @@ static int __init unittest_data_add(void) pr_warn("%s: No tree to attach; not running tests\n", __func__); return -ENODATA; } - of_node_set_flag(unittest_data_node, OF_DETACHED); rc = of_resolve_phandles(unittest_data_node); if (rc) { pr_err("%s: Failed to resolve phandles (rc=%i)\n", __func__, rc); @@ -2143,7 +2142,6 @@ static int __init overlay_data_add(int onum) ret = 0; goto out_free_data; } - of_node_set_flag(info->np_overlay, OF_DETACHED); ret = of_resolve_phandles(info->np_overlay); if (ret) { -- cgit From ac0f3e30d87e593dc1460d5f0407662073281d93 Mon Sep 17 00:00:00 2001 From: Lixin Wang Date: Mon, 16 Oct 2017 17:54:32 +0800 Subject: of: overlay: fix memory leak related to duplicated property Function of_changeset_add_property or of_changeset_update_property may fails. In this case the property just allocated is never deallocated. Signed-off-by: Lixin Wang Signed-off-by: Rob Herring --- drivers/of/overlay.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 8ecfee31ab6d..af3b9a16df26 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -162,6 +162,7 @@ static int of_overlay_apply_single_property(struct of_overlay *ov, bool is_symbols_node) { struct property *propn = NULL, *tprop; + int ret = 0; /* NOTE: Multiple changes of single properties not supported */ tprop = of_find_property(target, prop->name, NULL); @@ -186,10 +187,16 @@ static int of_overlay_apply_single_property(struct of_overlay *ov, /* not found? add */ if (tprop == NULL) - return of_changeset_add_property(&ov->cset, target, propn); - - /* found? update */ - return of_changeset_update_property(&ov->cset, target, propn); + ret = of_changeset_add_property(&ov->cset, target, propn); + else /* found? update */ + ret = of_changeset_update_property(&ov->cset, target, propn); + + if (ret) { + kfree(propn->name); + kfree(propn->value); + kfree(propn); + } + return ret; } static int of_overlay_apply_single_device_node(struct of_overlay *ov, -- cgit From 646afc4ad7f01d582d00e43a4f35b1ebdb70cb4e Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:21 -0700 Subject: of: overlay.c: Remove comments that state the obvious, to reduce clutter Follows recommendations in Documentation/process/coding-style.rst, section 8, Commenting. Some in function comments are promoted to function header comments. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 47 ++++++++++++++++++----------------------------- 1 file changed, 18 insertions(+), 29 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index af3b9a16df26..a42dd7b094c4 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -143,7 +143,6 @@ static struct property *dup_and_fixup_symbol_prop(struct of_overlay *ov, strcpy(new->value, target_path); strcpy(new->value + target_path_len, label_path); - /* mark the property as dynamic */ of_property_set_flag(new, OF_DYNAMIC); return new; @@ -157,6 +156,10 @@ static struct property *dup_and_fixup_symbol_prop(struct of_overlay *ov, } +/* + * Some special properties are not updated (no error returned). + * Update of property in symbols node is not allowed. + */ static int of_overlay_apply_single_property(struct of_overlay *ov, struct device_node *target, struct property *prop, bool is_symbols_node) @@ -164,17 +167,14 @@ static int of_overlay_apply_single_property(struct of_overlay *ov, struct property *propn = NULL, *tprop; int ret = 0; - /* NOTE: Multiple changes of single properties not supported */ 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) return 0; if (is_symbols_node) { - /* changing a property in __symbols__ node not allowed */ if (tprop) return -EINVAL; propn = dup_and_fixup_symbol_prop(ov, prop); @@ -185,10 +185,9 @@ static int of_overlay_apply_single_property(struct of_overlay *ov, if (propn == NULL) return -ENOMEM; - /* not found? add */ if (tprop == NULL) ret = of_changeset_add_property(&ov->cset, target, propn); - else /* found? update */ + else ret = of_changeset_update_property(&ov->cset, target, propn); if (ret) { @@ -210,13 +209,11 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov, if (cname == NULL) return -ENOMEM; - /* NOTE: Multiple mods of created nodes not supported */ for_each_child_of_node(target, tchild) if (!of_node_cmp(cname, kbasename(tchild->full_name))) break; if (tchild != NULL) { - /* new overlay phandle value conflicts with existing value */ if (child->phandle) return -EINVAL; @@ -224,12 +221,10 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov, ret = of_overlay_apply_one(ov, tchild, child, 0); of_node_put(tchild); } else { - /* create empty tree as a target */ tchild = __of_node_dup(child, "%pOF/%s", target, cname); if (!tchild) return -ENOMEM; - /* point to parent */ tchild->parent = target; ret = of_changeset_attach_node(&ov->cset, tchild); @@ -250,6 +245,8 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov, * Note that the in case of an error the target node is left * in a inconsistent state. Error recovery should be performed * by using the changeset. + * + * Do not allow symbols node to have any children. */ static int of_overlay_apply_one(struct of_overlay *ov, struct device_node *target, const struct device_node *overlay, @@ -269,7 +266,6 @@ static int of_overlay_apply_one(struct of_overlay *ov, } } - /* do not allow symbols node to have any children */ if (is_symbols_node) return 0; @@ -299,7 +295,6 @@ static int of_overlay_apply(struct of_overlay *ov) { int i, err; - /* first we apply the overlays atomically */ for (i = 0; i < ov->count; i++) { struct of_overlay_info *ovinfo = &ov->ovinfo_tab[i]; @@ -316,10 +311,10 @@ static int of_overlay_apply(struct of_overlay *ov) /* * Find the target node using a number of different strategies - * in order of preference + * in order of preference: * - * "target" property containing the phandle of the target - * "target-path" property containing the path of the target + * 1) "target" property containing the phandle of the target + * 2) "target-path" property containing the path of the target */ static struct device_node *find_target_node(struct device_node *info_node) { @@ -327,12 +322,10 @@ static struct device_node *find_target_node(struct device_node *info_node) u32 val; int ret; - /* first try to go by using the target as a phandle */ ret = of_property_read_u32(info_node, "target", &val); if (ret == 0) return of_find_node_by_phandle(val); - /* now try to locate by path */ ret = of_property_read_string(info_node, "target-path", &path); if (ret == 0) return of_find_node_by_path(path); @@ -397,7 +390,6 @@ static int of_build_overlay_info(struct of_overlay *ov, struct of_overlay_info *ovinfo; int cnt, err; - /* worst case; every child is a node */ cnt = 0; for_each_child_of_node(tree, node) cnt++; @@ -430,7 +422,6 @@ static int of_build_overlay_info(struct of_overlay *ov, cnt++; } - /* if nothing filled, return error */ if (cnt == 0) { kfree(ovinfo); return -ENODEV; @@ -486,7 +477,6 @@ int of_overlay_create(struct device_node *tree) struct of_overlay *ov; int err, id; - /* allocate the overlay structure */ ov = kzalloc(sizeof(*ov), GFP_KERNEL); if (ov == NULL) return -ENOMEM; @@ -505,7 +495,6 @@ int of_overlay_create(struct device_node *tree) } ov->id = id; - /* build the overlay info structures */ err = of_build_overlay_info(ov, tree); if (err) { pr_err("of_build_overlay_info() failed for tree@%pOF\n", @@ -520,18 +509,15 @@ int of_overlay_create(struct device_node *tree) goto err_free_idr; } - /* apply the overlay */ err = of_overlay_apply(ov); if (err) goto err_abort_trans; - /* apply the changeset */ err = __of_changeset_apply(&ov->cset); if (err) goto err_revert_overlay; - /* add to the tail of the overlay list */ list_add_tail(&ov->node, &ov_list); of_overlay_notify(ov, OF_OVERLAY_POST_APPLY); @@ -554,13 +540,15 @@ err_destroy_trans: } EXPORT_SYMBOL_GPL(of_overlay_create); -/* check whether the given node, lies under the given tree */ +/* + * check whether the given node, lies under the given tree + * return 1 if under tree, else 0 + */ static int overlay_subtree_check(struct device_node *tree, struct device_node *dn) { struct device_node *child; - /* match? */ if (tree == dn) return 1; @@ -574,7 +562,10 @@ static int overlay_subtree_check(struct device_node *tree, return 0; } -/* check whether this overlay is the topmost */ +/* + * check whether this overlay is the topmost + * return 1 if topmost, else 0 + */ static int overlay_is_topmost(struct of_overlay *ov, struct device_node *dn) { struct of_overlay *ovt; @@ -595,7 +586,6 @@ static int overlay_is_topmost(struct of_overlay *ov, struct device_node *dn) } } - /* overlay is topmost */ return 1; } @@ -645,7 +635,6 @@ int of_overlay_destroy(int id) goto out; } - /* check whether the overlay is safe to remove */ if (!overlay_removal_is_ok(ov)) { err = -EBUSY; goto out; -- cgit From bbed8794d53b7043d7989e22bc2e1e399da305eb Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:22 -0700 Subject: of: overlay.c: Convert comparisons to zero or NULL to logical expressions Use normal shorthand for comparing a variable to zero. For variable "XXX": convert (XXX == 0) to (!XXX) convert (XXX != 0) to (XXX) Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index a42dd7b094c4..d3f4a5974a11 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -169,9 +169,9 @@ static int of_overlay_apply_single_property(struct of_overlay *ov, tprop = of_find_property(target, prop->name, NULL); - 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") || + !of_prop_cmp(prop->name, "phandle") || + !of_prop_cmp(prop->name, "linux,phandle")) return 0; if (is_symbols_node) { @@ -182,10 +182,10 @@ static int of_overlay_apply_single_property(struct of_overlay *ov, propn = __of_prop_dup(prop, GFP_KERNEL); } - if (propn == NULL) + if (!propn) return -ENOMEM; - if (tprop == NULL) + if (!tprop) ret = of_changeset_add_property(&ov->cset, target, propn); else ret = of_changeset_update_property(&ov->cset, target, propn); @@ -206,14 +206,14 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov, int ret = 0; cname = kbasename(child->full_name); - if (cname == NULL) + if (!cname) return -ENOMEM; for_each_child_of_node(target, tchild) if (!of_node_cmp(cname, kbasename(tchild->full_name))) break; - if (tchild != NULL) { + if (tchild) { if (child->phandle) return -EINVAL; @@ -271,7 +271,7 @@ 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) { + if (ret) { pr_err("Failed to apply single node @%pOF/%s\n", target, child->name); of_node_put(child); @@ -300,7 +300,7 @@ static int of_overlay_apply(struct of_overlay *ov) err = of_overlay_apply_one(ov, ovinfo->target, ovinfo->overlay, ovinfo->is_symbols_node); - if (err != 0) { + if (err) { pr_err("apply failed '%pOF'\n", ovinfo->target); return err; } @@ -323,11 +323,11 @@ static struct device_node *find_target_node(struct device_node *info_node) int ret; ret = of_property_read_u32(info_node, "target", &val); - if (ret == 0) + if (!ret) return of_find_node_by_phandle(val); ret = of_property_read_string(info_node, "target-path", &path); - if (ret == 0) + if (!ret) return of_find_node_by_path(path); pr_err("Failed to find target for node %p (%s)\n", @@ -354,11 +354,11 @@ static int of_fill_overlay_info(struct of_overlay *ov, struct device_node *info_node, struct of_overlay_info *ovinfo) { ovinfo->overlay = of_get_child_by_name(info_node, "__overlay__"); - if (ovinfo->overlay == NULL) + if (!ovinfo->overlay) goto err_fail; ovinfo->target = find_target_node(info_node); - if (ovinfo->target == NULL) + if (!ovinfo->target) goto err_fail; return 0; @@ -398,13 +398,13 @@ static int of_build_overlay_info(struct of_overlay *ov, cnt++; ovinfo = kcalloc(cnt, sizeof(*ovinfo), GFP_KERNEL); - if (ovinfo == NULL) + if (!ovinfo) return -ENOMEM; cnt = 0; for_each_child_of_node(tree, node) { err = of_fill_overlay_info(ov, node, &ovinfo[cnt]); - if (err == 0) + if (!err) cnt++; } @@ -422,7 +422,7 @@ static int of_build_overlay_info(struct of_overlay *ov, cnt++; } - if (cnt == 0) { + if (!cnt) { kfree(ovinfo); return -ENODEV; } @@ -478,7 +478,7 @@ int of_overlay_create(struct device_node *tree) int err, id; ov = kzalloc(sizeof(*ov), GFP_KERNEL); - if (ov == NULL) + if (!ov) return -ENOMEM; ov->id = -1; @@ -629,7 +629,7 @@ int of_overlay_destroy(int id) mutex_lock(&of_mutex); ov = idr_find(&ov_idr, id); - if (ov == NULL) { + if (!ov) { err = -ENODEV; pr_err("destroy: Could not find overlay #%d\n", id); goto out; -- cgit From 0290c4ca2536a35e55c53cfb9058465b1f987b17 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:23 -0700 Subject: of: overlay: rename identifiers to more reflect what they do This patch is aimed primarily at drivers/of/overlay.c, but those changes also have a small impact in a few other files. overlay.c is difficult to read and maintain. Improve readability: - Rename functions, types and variables to better reflect what they do and to be consistent with names in other places, such as the device tree overlay FDT (flattened device tree), and make the algorithms more clear - Use the same names consistently throughout the file - Update comments for name changes - Fix incorrect comments This patch is intended to not introduce any functional change. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 2 +- drivers/of/overlay.c | 530 +++++++++++++++++++++++++++----------------------- drivers/of/unittest.c | 20 +- 3 files changed, 295 insertions(+), 257 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 39e8cf731764..4bc96af4d8e2 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -758,7 +758,7 @@ int of_changeset_revert(struct of_changeset *ocs) EXPORT_SYMBOL_GPL(of_changeset_revert); /** - * of_changeset_action - Perform a changeset action + * of_changeset_action - Add an action to the tail of the changeset list * * @ocs: changeset pointer * @action: action to perform diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index d3f4a5974a11..69610637af88 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -25,67 +25,63 @@ #include "of_private.h" /** - * struct of_overlay_info - Holds a single overlay info + * struct fragment - info about fragment nodes in overlay expanded device tree * @target: target of the overlay operation - * @overlay: pointer to the overlay contents node - * - * Holds a single overlay state, including all the overlay logs & - * records. + * @overlay: pointer to the __overlay__ node */ -struct of_overlay_info { +struct fragment { struct device_node *target; struct device_node *overlay; bool is_symbols_node; }; /** - * struct of_overlay - Holds a complete overlay transaction - * @node: List on which we are located - * @count: Count of ovinfo structures - * @ovinfo_tab: Overlay info table (count sized) - * @cset: Changeset to be used - * - * Holds a complete overlay transaction + * struct overlay_changeset + * @ovcs_list: list on which we are located + * @count: count of @fragments structures + * @fragments: info about fragment nodes in overlay expanded device tree + * @cset: changeset to apply fragments to live device tree */ -struct of_overlay { +struct overlay_changeset { int id; - struct list_head node; + struct list_head ovcs_list; int count; - struct of_overlay_info *ovinfo_tab; + struct fragment *fragments; struct of_changeset cset; }; -static int of_overlay_apply_one(struct of_overlay *ov, - struct device_node *target, const struct device_node *overlay, +static int build_changeset_next_level(struct overlay_changeset *ovcs, + struct device_node *target_node, + const struct device_node *overlay_node, bool is_symbols_node); -static BLOCKING_NOTIFIER_HEAD(of_overlay_chain); +static BLOCKING_NOTIFIER_HEAD(overlay_notify_chain); int of_overlay_notifier_register(struct notifier_block *nb) { - return blocking_notifier_chain_register(&of_overlay_chain, nb); + return blocking_notifier_chain_register(&overlay_notify_chain, nb); } EXPORT_SYMBOL_GPL(of_overlay_notifier_register); int of_overlay_notifier_unregister(struct notifier_block *nb) { - return blocking_notifier_chain_unregister(&of_overlay_chain, nb); + return blocking_notifier_chain_unregister(&overlay_notify_chain, nb); } EXPORT_SYMBOL_GPL(of_overlay_notifier_unregister); -static int of_overlay_notify(struct of_overlay *ov, - enum of_overlay_notify_action action) +static int overlay_notify(struct overlay_changeset *ovcs, + enum of_overlay_notify_action action) { struct of_overlay_notify_data nd; int i, ret; - for (i = 0; i < ov->count; i++) { - struct of_overlay_info *ovinfo = &ov->ovinfo_tab[i]; + for (i = 0; i < ovcs->count; i++) { + struct fragment *fragment = &ovcs->fragments[i]; - nd.target = ovinfo->target; - nd.overlay = ovinfo->overlay; + nd.target = fragment->target; + nd.overlay = fragment->overlay; - ret = blocking_notifier_call_chain(&of_overlay_chain, + ret = blocking_notifier_call_chain(&overlay_notify_chain, action, &nd); if (ret) return notifier_to_errno(ret); @@ -94,10 +90,10 @@ static int of_overlay_notify(struct of_overlay *ov, return 0; } -static struct property *dup_and_fixup_symbol_prop(struct of_overlay *ov, - const struct property *prop) +static struct property *dup_and_fixup_symbol_prop( + struct overlay_changeset *ovcs, const struct property *prop) { - struct of_overlay_info *ovinfo; + struct fragment *fragment; struct property *new; const char *overlay_name; char *label_path; @@ -116,18 +112,18 @@ static struct property *dup_and_fixup_symbol_prop(struct of_overlay *ov, if (!new) return NULL; - for (k = 0; k < ov->count; k++) { - ovinfo = &ov->ovinfo_tab[k]; - overlay_name = ovinfo->overlay->full_name; + for (k = 0; k < ovcs->count; k++) { + fragment = &ovcs->fragments[k]; + overlay_name = fragment->overlay->full_name; overlay_name_len = strlen(overlay_name); if (!strncasecmp(symbol_path, overlay_name, overlay_name_len)) break; } - if (k >= ov->count) + if (k >= ovcs->count) goto err_free; - target_path = ovinfo->target->full_name; + target_path = fragment->target->full_name; target_path_len = strlen(target_path); label_path = symbol_path + overlay_name_len; @@ -156,82 +152,119 @@ static struct property *dup_and_fixup_symbol_prop(struct of_overlay *ov, } -/* +/** + * add_changeset_property() - add @overlay_prop to overlay changeset + * @ovcs: overlay changeset + * @target_node: where to place @overlay_prop in live tree + * @overlay_prop: property to add or update, from overlay tree + * is_symbols_node: 1 if @target_node is "/__symbols__" + * + * If @overlay_prop does not already exist in @target_node, add changeset entry + * to add @overlay_prop in @target_node, else add changeset entry to update + * value of @overlay_prop. + * * Some special properties are not updated (no error returned). + * * Update of property in symbols node is not allowed. + * + * Returns 0 on success, -ENOMEM if memory allocation failure, or -EINVAL if + * invalid @overlay. */ -static int of_overlay_apply_single_property(struct of_overlay *ov, - struct device_node *target, struct property *prop, +static int add_changeset_property(struct overlay_changeset *ovcs, + struct device_node *target_node, + struct property *overlay_prop, bool is_symbols_node) { - struct property *propn = NULL, *tprop; + struct property *new_prop = NULL, *prop; int ret = 0; - tprop = of_find_property(target, prop->name, NULL); + prop = of_find_property(target_node, overlay_prop->name, NULL); - if (!of_prop_cmp(prop->name, "name") || - !of_prop_cmp(prop->name, "phandle") || - !of_prop_cmp(prop->name, "linux,phandle")) + if (!of_prop_cmp(overlay_prop->name, "name") || + !of_prop_cmp(overlay_prop->name, "phandle") || + !of_prop_cmp(overlay_prop->name, "linux,phandle")) return 0; if (is_symbols_node) { - if (tprop) + if (prop) return -EINVAL; - propn = dup_and_fixup_symbol_prop(ov, prop); + new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop); } else { - propn = __of_prop_dup(prop, GFP_KERNEL); + new_prop = __of_prop_dup(overlay_prop, GFP_KERNEL); } - if (!propn) + if (!new_prop) return -ENOMEM; - if (!tprop) - ret = of_changeset_add_property(&ov->cset, target, propn); + if (!prop) + ret = of_changeset_add_property(&ovcs->cset, target_node, + new_prop); else - ret = of_changeset_update_property(&ov->cset, target, propn); + ret = of_changeset_update_property(&ovcs->cset, target_node, + new_prop); if (ret) { - kfree(propn->name); - kfree(propn->value); - kfree(propn); + kfree(new_prop->name); + kfree(new_prop->value); + kfree(new_prop); } return ret; } -static int of_overlay_apply_single_device_node(struct of_overlay *ov, - struct device_node *target, struct device_node *child) +/** + * add_changeset_node() - add @node (and children) to overlay changeset + * @ovcs: overlay changeset + * @target_node: where to place @node in live tree + * @node: node from within overlay device tree fragment + * + * If @node does not already exist in @target_node, add changeset entry + * to add @node in @target_node. + * + * If @node already exists in @target_node, and the existing node has + * a phandle, the overlay node is not allowed to have a phandle. + * + * If @node has child nodes, add the children recursively via + * build_changeset_next_level(). + * + * NOTE: Multiple mods of created nodes not supported. + * + * Returns 0 on success, -ENOMEM if memory allocation failure, or -EINVAL if + * invalid @overlay. + */ +static int add_changeset_node(struct overlay_changeset *ovcs, + struct device_node *target_node, struct device_node *node) { - const char *cname; + const char *node_kbasename; struct device_node *tchild; int ret = 0; - cname = kbasename(child->full_name); - if (!cname) + node_kbasename = kbasename(node->full_name); + if (!node_kbasename) return -ENOMEM; - for_each_child_of_node(target, tchild) - if (!of_node_cmp(cname, kbasename(tchild->full_name))) + for_each_child_of_node(target_node, tchild) + if (!of_node_cmp(node_kbasename, kbasename(tchild->full_name))) break; if (tchild) { - if (child->phandle) + if (node->phandle) return -EINVAL; - /* apply overlay recursively */ - ret = of_overlay_apply_one(ov, tchild, child, 0); + ret = build_changeset_next_level(ovcs, tchild, node, 0); of_node_put(tchild); } else { - tchild = __of_node_dup(child, "%pOF/%s", target, cname); + tchild = __of_node_dup(node, "%pOF/%s", + target_node, node_kbasename); if (!tchild) return -ENOMEM; - tchild->parent = target; + tchild->parent = target_node; - ret = of_changeset_attach_node(&ov->cset, tchild); + ret = of_changeset_attach_node(&ovcs->cset, tchild); if (ret) return ret; - ret = of_overlay_apply_one(ov, tchild, child, 0); + ret = build_changeset_next_level(ovcs, tchild, node, 0); if (ret) return ret; } @@ -239,29 +272,37 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov, return ret; } -/* - * Apply a single overlay node recursively. +/** + * build_changeset_next_level() - add level of overlay changeset + * @ovcs: overlay changeset + * @target_node: where to place @overlay_node in live tree + * @overlay_node: node from within an overlay device tree fragment + * @is_symbols_node: @overlay_node is node "/__symbols__" * - * Note that the in case of an error the target node is left - * in a inconsistent state. Error recovery should be performed - * by using the changeset. + * Add the properties (if any) and nodes (if any) from @overlay_node to the + * @ovcs->cset changeset. If an added node has child nodes, they will + * be added recursively. * * Do not allow symbols node to have any children. + * + * Returns 0 on success, -ENOMEM if memory allocation failure, or -EINVAL if + * invalid @overlay_node. */ -static int of_overlay_apply_one(struct of_overlay *ov, - struct device_node *target, const struct device_node *overlay, +static int build_changeset_next_level(struct overlay_changeset *ovcs, + struct device_node *target_node, + const struct device_node *overlay_node, bool is_symbols_node) { struct device_node *child; struct property *prop; int ret; - for_each_property_of_node(overlay, prop) { - ret = of_overlay_apply_single_property(ov, target, prop, - is_symbols_node); + for_each_property_of_node(overlay_node, prop) { + ret = add_changeset_property(ovcs, target_node, prop, + is_symbols_node); if (ret) { pr_err("Failed to apply prop @%pOF/%s\n", - target, prop->name); + target_node, prop->name); return ret; } } @@ -269,11 +310,11 @@ static int of_overlay_apply_one(struct of_overlay *ov, if (is_symbols_node) return 0; - for_each_child_of_node(overlay, child) { - ret = of_overlay_apply_single_device_node(ov, target, child); + for_each_child_of_node(overlay_node, child) { + ret = add_changeset_node(ovcs, target_node, child); if (ret) { - pr_err("Failed to apply single node @%pOF/%s\n", - target, child->name); + pr_err("Failed to apply node @%pOF/%s\n", + target_node, child->name); of_node_put(child); return ret; } @@ -283,26 +324,30 @@ static int of_overlay_apply_one(struct of_overlay *ov, } /** - * of_overlay_apply() - Apply @count overlays pointed at by @ovinfo_tab - * @ov: Overlay to apply + * build_changeset() - populate overlay changeset in @ovcs from @ovcs->fragments + * @ovcs: Overlay changeset * - * Applies the overlays given, while handling all error conditions - * appropriately. Either the operation succeeds, or if it fails the - * live tree is reverted to the state before the attempt. - * Returns 0, or an error if the overlay attempt failed. + * Create changeset @ovcs->cset to contain the nodes and properties of the + * overlay device tree fragments in @ovcs->fragments[]. If an error occurs, + * any portions of the changeset that were successfully created will remain + * in @ovcs->cset. + * + * Returns 0 on success, -ENOMEM if memory allocation failure, or -EINVAL if + * invalid overlay in @ovcs->fragments[]. */ -static int of_overlay_apply(struct of_overlay *ov) +static int build_changeset(struct overlay_changeset *ovcs) { - int i, err; + int i, ret; - for (i = 0; i < ov->count; i++) { - struct of_overlay_info *ovinfo = &ov->ovinfo_tab[i]; + for (i = 0; i < ovcs->count; i++) { + struct fragment *fragment = &ovcs->fragments[i]; - err = of_overlay_apply_one(ov, ovinfo->target, ovinfo->overlay, - ovinfo->is_symbols_node); - if (err) { - pr_err("apply failed '%pOF'\n", ovinfo->target); - return err; + ret = build_changeset_next_level(ovcs, fragment->target, + fragment->overlay, + fragment->is_symbols_node); + if (ret) { + pr_err("apply failed '%pOF'\n", fragment->target); + return ret; } } @@ -350,45 +395,46 @@ static struct device_node *find_target_node(struct device_node *info_node) * * Returns 0 on success, or a negative error value. */ -static int of_fill_overlay_info(struct of_overlay *ov, - struct device_node *info_node, struct of_overlay_info *ovinfo) +static int of_fill_overlay_info(struct overlay_changeset *ovcset, + struct device_node *info_node, struct fragment *fragment) { - ovinfo->overlay = of_get_child_by_name(info_node, "__overlay__"); - if (!ovinfo->overlay) + fragment->overlay = of_get_child_by_name(info_node, "__overlay__"); + if (!fragment->overlay) goto err_fail; - ovinfo->target = find_target_node(info_node); - if (!ovinfo->target) + fragment->target = find_target_node(info_node); + if (!fragment->target) goto err_fail; return 0; err_fail: - of_node_put(ovinfo->target); - of_node_put(ovinfo->overlay); + of_node_put(fragment->target); + of_node_put(fragment->overlay); - memset(ovinfo, 0, sizeof(*ovinfo)); + memset(fragment, 0, sizeof(*fragment)); return -EINVAL; } /** - * of_build_overlay_info() - Build an overlay info array - * @ov Overlay to build - * @tree: Device node containing all the overlays + * init_overlay_changeset() - initialize overlay changeset from overlay tree + * @ovcs Overlay changeset to build + * @tree: Contains all the overlay fragments and overlay fixup nodes * - * Helper function that given a tree containing overlay information, - * allocates and builds an overlay info array containing it, ready - * for use using of_overlay_apply. + * Initialize @ovcs. Populate @ovcs->fragments with node information from + * the top level of @tree. The relevant top level nodes are the fragment + * nodes and the __symbols__ node. Any other top level node will be ignored. * - * Returns 0 on success with the @cntp @ovinfop pointers valid, - * while on error a negative error value is returned. + * Returns 0 on success, -ENOMEM if memory allocation failure, -EINVAL if error + * detected in @tree, or -ENODEV if no valid nodes found. */ -static int of_build_overlay_info(struct of_overlay *ov, +static int init_overlay_changeset(struct overlay_changeset *ovcs, struct device_node *tree) { struct device_node *node; - struct of_overlay_info *ovinfo; - int cnt, err; + struct fragment *fragment; + struct fragment *fragments; + int cnt, ret; cnt = 0; for_each_child_of_node(tree, node) @@ -397,24 +443,25 @@ static int of_build_overlay_info(struct of_overlay *ov, if (of_get_child_by_name(tree, "__symbols__")) cnt++; - ovinfo = kcalloc(cnt, sizeof(*ovinfo), GFP_KERNEL); - if (!ovinfo) + fragments = kcalloc(cnt, sizeof(*fragments), GFP_KERNEL); + if (!fragments) return -ENOMEM; cnt = 0; for_each_child_of_node(tree, node) { - err = of_fill_overlay_info(ov, node, &ovinfo[cnt]); - if (!err) + ret = of_fill_overlay_info(ovcs, node, &fragments[cnt]); + if (!ret) cnt++; } node = of_get_child_by_name(tree, "__symbols__"); if (node) { - ovinfo[cnt].overlay = node; - ovinfo[cnt].target = of_find_node_by_path("/__symbols__"); - ovinfo[cnt].is_symbols_node = 1; + fragment = &fragments[cnt]; + fragment->overlay = node; + fragment->target = of_find_node_by_path("/__symbols__"); + fragment->is_symbols_node = 1; - if (!ovinfo[cnt].target) { + if (!fragment->target) { pr_err("no symbols in root of device tree.\n"); return -EINVAL; } @@ -423,137 +470,127 @@ static int of_build_overlay_info(struct of_overlay *ov, } if (!cnt) { - kfree(ovinfo); + kfree(fragments); return -ENODEV; } - ov->count = cnt; - ov->ovinfo_tab = ovinfo; + ovcs->count = cnt; + ovcs->fragments = fragments; return 0; } /** - * of_free_overlay_info() - Free an overlay info array - * @ov Overlay to free the overlay info from - * @ovinfo_tab: Array of overlay_info's to free + * free_overlay_fragments() - Free a fragments array + * @ovcs Overlay to free the overlay info from * - * Releases the memory of a previously allocated ovinfo array - * by of_build_overlay_info. - * Returns 0, or an error if the arguments are bogus. + * Frees the memory of an ovcs->fragments[] array. */ -static int of_free_overlay_info(struct of_overlay *ov) +static void free_overlay_fragments(struct overlay_changeset *ovcs) { - struct of_overlay_info *ovinfo; int i; /* do it in reverse */ - for (i = ov->count - 1; i >= 0; i--) { - ovinfo = &ov->ovinfo_tab[i]; - - of_node_put(ovinfo->target); - of_node_put(ovinfo->overlay); + for (i = ovcs->count - 1; i >= 0; i--) { + of_node_put(ovcs->fragments[i].target); + of_node_put(ovcs->fragments[i].overlay); } - kfree(ov->ovinfo_tab); - return 0; + kfree(ovcs->fragments); } -static LIST_HEAD(ov_list); -static DEFINE_IDR(ov_idr); +static LIST_HEAD(ovcs_list); +static DEFINE_IDR(ovcs_idr); /** - * of_overlay_create() - Create and apply an overlay - * @tree: Device node containing all the overlays + * of_overlay_apply() - Create and apply an overlay changeset + * @tree: Expanded overlay device tree * - * Creates and applies an overlay while also keeping track - * of the overlay in a list. This list can be used to prevent - * illegal overlay removals. + * Creates and applies an overlay changeset. If successful, the overlay + * changeset is added to the overlay changeset list. * - * Returns the id of the created overlay, or a negative error number + * Returns the id of the created overlay changeset, or a negative error number */ -int of_overlay_create(struct device_node *tree) +int of_overlay_apply(struct device_node *tree) { - struct of_overlay *ov; - int err, id; + struct overlay_changeset *ovcs; + int id, ret; - ov = kzalloc(sizeof(*ov), GFP_KERNEL); - if (!ov) + ovcs = kzalloc(sizeof(*ovcs), GFP_KERNEL); + if (!ovcs) return -ENOMEM; - ov->id = -1; + ovcs->id = -1; - INIT_LIST_HEAD(&ov->node); + INIT_LIST_HEAD(&ovcs->ovcs_list); - of_changeset_init(&ov->cset); + of_changeset_init(&ovcs->cset); mutex_lock(&of_mutex); - id = idr_alloc(&ov_idr, ov, 0, 0, GFP_KERNEL); + id = idr_alloc(&ovcs_idr, ovcs, 0, 0, GFP_KERNEL); if (id < 0) { - err = id; + ret = id; goto err_destroy_trans; } - ov->id = id; + ovcs->id = id; - err = of_build_overlay_info(ov, tree); - if (err) { - pr_err("of_build_overlay_info() failed for tree@%pOF\n", + ret = init_overlay_changeset(ovcs, tree); + if (ret) { + pr_err("init_overlay_changeset() failed for tree@%pOF\n", tree); goto err_free_idr; } - err = of_overlay_notify(ov, OF_OVERLAY_PRE_APPLY); - if (err < 0) { - pr_err("%s: Pre-apply notifier failed (err=%d)\n", - __func__, err); - goto err_free_idr; + ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY); + if (ret < 0) { + pr_err("%s: Pre-apply notifier failed (ret=%d)\n", + __func__, ret); + goto err_free_overlay_fragments; } - err = of_overlay_apply(ov); - if (err) - goto err_abort_trans; - - err = __of_changeset_apply(&ov->cset); - if (err) - goto err_revert_overlay; + ret = build_changeset(ovcs); + if (ret) + goto err_free_overlay_fragments; + ret = __of_changeset_apply(&ovcs->cset); + if (ret) + goto err_free_overlay_fragments; - list_add_tail(&ov->node, &ov_list); + list_add_tail(&ovcs->ovcs_list, &ovcs_list); - of_overlay_notify(ov, OF_OVERLAY_POST_APPLY); + overlay_notify(ovcs, OF_OVERLAY_POST_APPLY); mutex_unlock(&of_mutex); return id; -err_revert_overlay: -err_abort_trans: - of_free_overlay_info(ov); +err_free_overlay_fragments: + free_overlay_fragments(ovcs); err_free_idr: - idr_remove(&ov_idr, ov->id); + idr_remove(&ovcs_idr, ovcs->id); err_destroy_trans: - of_changeset_destroy(&ov->cset); - kfree(ov); + of_changeset_destroy(&ovcs->cset); + kfree(ovcs); mutex_unlock(&of_mutex); - return err; + return ret; } -EXPORT_SYMBOL_GPL(of_overlay_create); +EXPORT_SYMBOL_GPL(of_overlay_apply); /* - * check whether the given node, lies under the given tree - * return 1 if under tree, else 0 + * Find @np in @tree. + * + * Returns 1 if @np is @tree or is contained in @tree, else 0 */ -static int overlay_subtree_check(struct device_node *tree, - struct device_node *dn) +static int find_node(struct device_node *tree, struct device_node *np) { struct device_node *child; - if (tree == dn) + if (tree == np) return 1; for_each_child_of_node(tree, child) { - if (overlay_subtree_check(child, dn)) { + if (find_node(child, np)) { of_node_put(child); return 1; } @@ -563,30 +600,32 @@ static int overlay_subtree_check(struct device_node *tree, } /* - * check whether this overlay is the topmost - * return 1 if topmost, else 0 + * Is @remove_ce_np a child of or the same as any + * node in an overlay changeset more topmost than @remove_ovcs? + * + * Returns 1 if found, else 0 */ -static int overlay_is_topmost(struct of_overlay *ov, struct device_node *dn) +static int node_in_later_cs(struct overlay_changeset *remove_ovcs, + struct device_node *remove_ce_np) { - struct of_overlay *ovt; + struct overlay_changeset *ovcs; struct of_changeset_entry *ce; - list_for_each_entry_reverse(ovt, &ov_list, node) { - /* if we hit ourselves, we're done */ - if (ovt == ov) + list_for_each_entry_reverse(ovcs, &ovcs_list, ovcs_list) { + if (ovcs == remove_ovcs) break; - /* check against each subtree affected by this overlay */ - list_for_each_entry(ce, &ovt->cset.entries, node) { - if (overlay_subtree_check(ce->np, dn)) { + list_for_each_entry(ce, &ovcs->cset.entries, node) { + if (find_node(ce->np, remove_ce_np)) { pr_err("%s: #%d clashes #%d @%pOF\n", - __func__, ov->id, ovt->id, dn); - return 0; + __func__, remove_ovcs->id, ovcs->id, + remove_ce_np); + return 1; } } } - return 1; + return 0; } /* @@ -599,13 +638,13 @@ static int overlay_is_topmost(struct of_overlay *ov, struct device_node *dn) * the one closest to the tail. If another overlay has affected this * device node and is closest to the tail, then removal is not permited. */ -static int overlay_removal_is_ok(struct of_overlay *ov) +static int overlay_removal_is_ok(struct overlay_changeset *remove_ovcs) { - struct of_changeset_entry *ce; + struct of_changeset_entry *remove_ce; - list_for_each_entry(ce, &ov->cset.entries, node) { - if (!overlay_is_topmost(ov, ce->np)) { - pr_err("overlay #%d is not topmost\n", ov->id); + list_for_each_entry(remove_ce, &remove_ovcs->cset.entries, node) { + if (node_in_later_cs(remove_ovcs, remove_ce->np)) { + pr_err("overlay #%d is not topmost\n", remove_ovcs->id); return 0; } } @@ -614,74 +653,73 @@ static int overlay_removal_is_ok(struct of_overlay *ov) } /** - * of_overlay_destroy() - Removes an overlay - * @id: Overlay id number returned by a previous call to of_overlay_create + * of_overlay_remove() - Revert and free an overlay changeset + * @ovcs_id: Overlay changeset id number * - * Removes an overlay if it is permissible. + * Removes an overlay if it is permissible. ovcs_id was previously returned + * by of_overlay_apply(). * * Returns 0 on success, or a negative error number */ -int of_overlay_destroy(int id) +int of_overlay_remove(int ovcs_id) { - struct of_overlay *ov; - int err; + struct overlay_changeset *ovcs; + int ret = 0; mutex_lock(&of_mutex); - ov = idr_find(&ov_idr, id); - if (!ov) { - err = -ENODEV; - pr_err("destroy: Could not find overlay #%d\n", id); + ovcs = idr_find(&ovcs_idr, ovcs_id); + if (!ovcs) { + ret = -ENODEV; + pr_err("remove: Could not find overlay #%d\n", ovcs_id); goto out; } - if (!overlay_removal_is_ok(ov)) { - err = -EBUSY; + if (!overlay_removal_is_ok(ovcs)) { + ret = -EBUSY; goto out; } - of_overlay_notify(ov, OF_OVERLAY_PRE_REMOVE); - list_del(&ov->node); - __of_changeset_revert(&ov->cset); - of_overlay_notify(ov, OF_OVERLAY_POST_REMOVE); - of_free_overlay_info(ov); - idr_remove(&ov_idr, id); - of_changeset_destroy(&ov->cset); - kfree(ov); - - err = 0; + overlay_notify(ovcs, OF_OVERLAY_PRE_REMOVE); + list_del(&ovcs->ovcs_list); + __of_changeset_revert(&ovcs->cset); + overlay_notify(ovcs, OF_OVERLAY_POST_REMOVE); + free_overlay_fragments(ovcs); + idr_remove(&ovcs_idr, ovcs_id); + of_changeset_destroy(&ovcs->cset); + kfree(ovcs); out: mutex_unlock(&of_mutex); - return err; + return ret; } -EXPORT_SYMBOL_GPL(of_overlay_destroy); +EXPORT_SYMBOL_GPL(of_overlay_remove); /** - * of_overlay_destroy_all() - Removes all overlays from the system + * of_overlay_remove_all() - Reverts and frees all overlay changesets * * Removes all overlays from the system in the correct order. * * Returns 0 on success, or a negative error number */ -int of_overlay_destroy_all(void) +int of_overlay_remove_all(void) { - struct of_overlay *ov, *ovn; + struct overlay_changeset *ovcs, *ovcs_n; mutex_lock(&of_mutex); /* the tail of list is guaranteed to be safe to remove */ - list_for_each_entry_safe_reverse(ov, ovn, &ov_list, node) { - list_del(&ov->node); - __of_changeset_revert(&ov->cset); - of_free_overlay_info(ov); - idr_remove(&ov_idr, ov->id); - kfree(ov); + list_for_each_entry_safe_reverse(ovcs, ovcs_n, &ovcs_list, ovcs_list) { + list_del(&ovcs->ovcs_list); + __of_changeset_revert(&ovcs->cset); + free_overlay_fragments(ovcs); + idr_remove(&ovcs_idr, ovcs->id); + kfree(ovcs); } mutex_unlock(&of_mutex); return 0; } -EXPORT_SYMBOL_GPL(of_overlay_destroy_all); +EXPORT_SYMBOL_GPL(of_overlay_remove_all); diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index add02cfbfb88..bbdaf5606820 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -1230,7 +1230,7 @@ static void of_unittest_destroy_tracked_overlays(void) if (!(overlay_id_bits[BIT_WORD(id)] & BIT_MASK(id))) continue; - ret = of_overlay_destroy(id + overlay_first_id); + ret = of_overlay_remove(id + overlay_first_id); if (ret == -ENODEV) { pr_warn("%s: no overlay to destroy for #%d\n", __func__, id + overlay_first_id); @@ -1262,7 +1262,7 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr, goto out; } - ret = of_overlay_create(np); + ret = of_overlay_apply(np); if (ret < 0) { unittest(0, "could not create overlay from \"%s\"\n", overlay_path(overlay_nr)); @@ -1347,7 +1347,7 @@ static int of_unittest_apply_revert_overlay_check(int overlay_nr, return -EINVAL; } - ret = of_overlay_destroy(ov_id); + ret = of_overlay_remove(ov_id); if (ret != 0) { unittest(0, "overlay @\"%s\" failed to be destroyed @\"%s\"\n", overlay_path(overlay_nr), @@ -1476,7 +1476,7 @@ static void of_unittest_overlay_6(void) return; } - ret = of_overlay_create(np); + ret = of_overlay_apply(np); if (ret < 0) { unittest(0, "could not create overlay from \"%s\"\n", overlay_path(overlay_nr + i)); @@ -1500,7 +1500,7 @@ static void of_unittest_overlay_6(void) } for (i = 1; i >= 0; i--) { - ret = of_overlay_destroy(ov_id[i]); + ret = of_overlay_remove(ov_id[i]); if (ret != 0) { unittest(0, "overlay @\"%s\" failed destroy @\"%s\"\n", overlay_path(overlay_nr + i), @@ -1546,7 +1546,7 @@ static void of_unittest_overlay_8(void) return; } - ret = of_overlay_create(np); + ret = of_overlay_apply(np); if (ret < 0) { unittest(0, "could not create overlay from \"%s\"\n", overlay_path(overlay_nr + i)); @@ -1557,7 +1557,7 @@ static void of_unittest_overlay_8(void) } /* now try to remove first overlay (it should fail) */ - ret = of_overlay_destroy(ov_id[0]); + ret = of_overlay_remove(ov_id[0]); if (ret == 0) { unittest(0, "overlay @\"%s\" was destroyed @\"%s\"\n", overlay_path(overlay_nr + 0), @@ -1568,7 +1568,7 @@ static void of_unittest_overlay_8(void) /* removing them in order should work */ for (i = 1; i >= 0; i--) { - ret = of_overlay_destroy(ov_id[i]); + ret = of_overlay_remove(ov_id[i]); if (ret != 0) { unittest(0, "overlay @\"%s\" not destroyed @\"%s\"\n", overlay_path(overlay_nr + i), @@ -2149,9 +2149,9 @@ static int __init overlay_data_add(int onum) goto out_free_np_overlay; } - ret = of_overlay_create(info->np_overlay); + ret = of_overlay_apply(info->np_overlay); if (ret < 0) { - pr_err("of_overlay_create() (ret=%d), %d\n", ret, onum); + pr_err("of_overlay_apply() (ret=%d), %d\n", ret, onum); goto out_free_np_overlay; } else { info->overlay_id = ret; -- cgit From 42b2e94fe83c354b4373992c8ea28ef0ace2e633 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:24 -0700 Subject: of: overlay: rename identifiers in dup_and_fixup_symbol_prop() More renaming of identifiers to better reflect what they do. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 69610637af88..bb8867cae05b 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -90,17 +90,29 @@ static int overlay_notify(struct overlay_changeset *ovcs, return 0; } +/* + * The properties in the "/__symbols__" node are "symbols". + * + * The value of properties in the "/__symbols__" node is the path of a + * node in the subtree of a fragment node's "__overlay__" node, for + * example "/fragment@0/__overlay__/symbol_path_tail". Symbol_path_tail + * can be a single node or it may be a multi-node path. + * + * The duplicated property value will be modified by replacing the + * "/fragment_name/__overlay/" portion of the value with the target + * path from the fragment node. + */ static struct property *dup_and_fixup_symbol_prop( struct overlay_changeset *ovcs, const struct property *prop) { struct fragment *fragment; struct property *new; const char *overlay_name; - char *label_path; + char *symbol_path_tail; char *symbol_path; const char *target_path; int k; - int label_path_len; + int symbol_path_tail_len; int overlay_name_len; int target_path_len; @@ -126,18 +138,18 @@ static struct property *dup_and_fixup_symbol_prop( target_path = fragment->target->full_name; target_path_len = strlen(target_path); - label_path = symbol_path + overlay_name_len; - label_path_len = strlen(label_path); + symbol_path_tail = symbol_path + overlay_name_len; + symbol_path_tail_len = strlen(symbol_path_tail); new->name = kstrdup(prop->name, GFP_KERNEL); - new->length = target_path_len + label_path_len + 1; + new->length = target_path_len + symbol_path_tail_len + 1; new->value = kzalloc(new->length, GFP_KERNEL); if (!new->name || !new->value) goto err_free; strcpy(new->value, target_path); - strcpy(new->value + target_path_len, label_path); + strcpy(new->value + target_path_len, symbol_path_tail); of_property_set_flag(new, OF_DYNAMIC); -- cgit From 61b4de4e0b384f4a22c55c3bada604da49cec4e1 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:25 -0700 Subject: of: overlay: minor restructuring Continue improving the readability of overlay.c. The previous patches renamed identifiers. This patch is split out from the previous patches to make the previous patches easier to review. Changes are: - minor code restructuring - some initialization of an overlay changeset occurred outside of init_overlay_changeset(), move that into init_overlay_changeset() - consolidate freeing an overlay changeset into free_overlay_changeset() This patch is intended to not introduce any functional change. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 205 +++++++++++++++++++++++---------------------------- 1 file changed, 92 insertions(+), 113 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index bb8867cae05b..905916e17eec 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -55,6 +55,9 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs, const struct device_node *overlay_node, bool is_symbols_node); +static LIST_HEAD(ovcs_list); +static DEFINE_IDR(ovcs_idr); + static BLOCKING_NOTIFIER_HEAD(overlay_notify_chain); int of_overlay_notifier_register(struct notifier_block *nb) @@ -160,8 +163,6 @@ static struct property *dup_and_fixup_symbol_prop( kfree(new->value); kfree(new); return NULL; - - } /** @@ -258,13 +259,7 @@ static int add_changeset_node(struct overlay_changeset *ovcs, if (!of_node_cmp(node_kbasename, kbasename(tchild->full_name))) break; - if (tchild) { - if (node->phandle) - return -EINVAL; - - ret = build_changeset_next_level(ovcs, tchild, node, 0); - of_node_put(tchild); - } else { + if (!tchild) { tchild = __of_node_dup(node, "%pOF/%s", target_node, node_kbasename); if (!tchild) @@ -276,11 +271,15 @@ static int add_changeset_node(struct overlay_changeset *ovcs, if (ret) return ret; - ret = build_changeset_next_level(ovcs, tchild, node, 0); - if (ret) - return ret; + return build_changeset_next_level(ovcs, tchild, node, 0); } + if (node->phandle) + return -EINVAL; + + ret = build_changeset_next_level(ovcs, tchild, node, 0); + of_node_put(tchild); + return ret; } @@ -393,41 +392,6 @@ static struct device_node *find_target_node(struct device_node *info_node) return NULL; } -/** - * of_fill_overlay_info() - Fill an overlay info structure - * @ov Overlay to fill - * @info_node: Device node containing the overlay - * @ovinfo: Pointer to the overlay info structure to fill - * - * Fills an overlay info structure with the overlay information - * from a device node. This device node must have a target property - * which contains a phandle of the overlay target node, and an - * __overlay__ child node which has the overlay contents. - * Both ovinfo->target & ovinfo->overlay have their references taken. - * - * Returns 0 on success, or a negative error value. - */ -static int of_fill_overlay_info(struct overlay_changeset *ovcset, - struct device_node *info_node, struct fragment *fragment) -{ - fragment->overlay = of_get_child_by_name(info_node, "__overlay__"); - if (!fragment->overlay) - goto err_fail; - - fragment->target = find_target_node(info_node); - if (!fragment->target) - goto err_fail; - - return 0; - -err_fail: - of_node_put(fragment->target); - of_node_put(fragment->overlay); - - memset(fragment, 0, sizeof(*fragment)); - return -EINVAL; -} - /** * init_overlay_changeset() - initialize overlay changeset from overlay tree * @ovcs Overlay changeset to build @@ -438,32 +402,61 @@ err_fail: * nodes and the __symbols__ node. Any other top level node will be ignored. * * Returns 0 on success, -ENOMEM if memory allocation failure, -EINVAL if error - * detected in @tree, or -ENODEV if no valid nodes found. + * detected in @tree, or -ENOSPC if idr_alloc() error. */ static int init_overlay_changeset(struct overlay_changeset *ovcs, struct device_node *tree) { - struct device_node *node; + struct device_node *node, *overlay_node; struct fragment *fragment; struct fragment *fragments; int cnt, ret; + INIT_LIST_HEAD(&ovcs->ovcs_list); + + of_changeset_init(&ovcs->cset); + + ovcs->id = idr_alloc(&ovcs_idr, ovcs, 1, 0, GFP_KERNEL); + if (ovcs->id <= 0) + return ovcs->id; + cnt = 0; - for_each_child_of_node(tree, node) - cnt++; - if (of_get_child_by_name(tree, "__symbols__")) + /* fragment nodes */ + for_each_child_of_node(tree, node) { + overlay_node = of_get_child_by_name(node, "__overlay__"); + if (overlay_node) { + cnt++; + of_node_put(overlay_node); + } + } + + node = of_get_child_by_name(tree, "__symbols__"); + if (node) { cnt++; + of_node_put(node); + } fragments = kcalloc(cnt, sizeof(*fragments), GFP_KERNEL); - if (!fragments) - return -ENOMEM; + if (!fragments) { + ret = -ENOMEM; + goto err_free_idr; + } cnt = 0; for_each_child_of_node(tree, node) { - ret = of_fill_overlay_info(ovcs, node, &fragments[cnt]); - if (!ret) - cnt++; + fragment = &fragments[cnt]; + fragment->overlay = of_get_child_by_name(node, "__overlay__"); + if (fragment->overlay) { + fragment->target = find_target_node(node); + if (!fragment->target) { + of_node_put(fragment->overlay); + ret = -EINVAL; + goto err_free_fragments; + } else { + cnt++; + } + } } node = of_get_child_by_name(tree, "__symbols__"); @@ -475,44 +468,51 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs, if (!fragment->target) { pr_err("no symbols in root of device tree.\n"); - return -EINVAL; + ret = -EINVAL; + goto err_free_fragments; } cnt++; } if (!cnt) { - kfree(fragments); - return -ENODEV; + ret = -EINVAL; + goto err_free_fragments; } ovcs->count = cnt; ovcs->fragments = fragments; return 0; + + +err_free_fragments: + kfree(fragments); +err_free_idr: + idr_remove(&ovcs_idr, ovcs->id); + + return ret; } -/** - * free_overlay_fragments() - Free a fragments array - * @ovcs Overlay to free the overlay info from - * - * Frees the memory of an ovcs->fragments[] array. - */ -static void free_overlay_fragments(struct overlay_changeset *ovcs) +static void free_overlay_changeset(struct overlay_changeset *ovcs) { int i; - /* do it in reverse */ - for (i = ovcs->count - 1; i >= 0; i--) { + if (!ovcs->cset.entries.next) + return; + of_changeset_destroy(&ovcs->cset); + + if (ovcs->id) + idr_remove(&ovcs_idr, ovcs->id); + + for (i = 0; i < ovcs->count; i++) { of_node_put(ovcs->fragments[i].target); of_node_put(ovcs->fragments[i].overlay); } - kfree(ovcs->fragments); -} -static LIST_HEAD(ovcs_list); -static DEFINE_IDR(ovcs_idr); + kfree(ovcs); +} /** * of_overlay_apply() - Create and apply an overlay changeset @@ -526,47 +526,34 @@ static DEFINE_IDR(ovcs_idr); int of_overlay_apply(struct device_node *tree) { struct overlay_changeset *ovcs; - int id, ret; + int ret; ovcs = kzalloc(sizeof(*ovcs), GFP_KERNEL); if (!ovcs) return -ENOMEM; - ovcs->id = -1; - - INIT_LIST_HEAD(&ovcs->ovcs_list); - - of_changeset_init(&ovcs->cset); mutex_lock(&of_mutex); - id = idr_alloc(&ovcs_idr, ovcs, 0, 0, GFP_KERNEL); - if (id < 0) { - ret = id; - goto err_destroy_trans; - } - ovcs->id = id; - ret = init_overlay_changeset(ovcs, tree); if (ret) { - pr_err("init_overlay_changeset() failed for tree@%pOF\n", - tree); - goto err_free_idr; + pr_err("init_overlay_changeset() failed, ret = %d\n", ret); + goto err_free_overlay_changeset; } ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY); if (ret < 0) { pr_err("%s: Pre-apply notifier failed (ret=%d)\n", __func__, ret); - goto err_free_overlay_fragments; + goto err_free_overlay_changeset; } ret = build_changeset(ovcs); if (ret) - goto err_free_overlay_fragments; + goto err_free_overlay_changeset; ret = __of_changeset_apply(&ovcs->cset); if (ret) - goto err_free_overlay_fragments; + goto err_free_overlay_changeset; list_add_tail(&ovcs->ovcs_list, &ovcs_list); @@ -574,15 +561,11 @@ int of_overlay_apply(struct device_node *tree) mutex_unlock(&of_mutex); - return id; + return ovcs->id; + +err_free_overlay_changeset: + free_overlay_changeset(ovcs); -err_free_overlay_fragments: - free_overlay_fragments(ovcs); -err_free_idr: - idr_remove(&ovcs_idr, ovcs->id); -err_destroy_trans: - of_changeset_destroy(&ovcs->cset); - kfree(ovcs); mutex_unlock(&of_mutex); return ret; @@ -693,13 +676,14 @@ int of_overlay_remove(int ovcs_id) } overlay_notify(ovcs, OF_OVERLAY_PRE_REMOVE); + list_del(&ovcs->ovcs_list); + __of_changeset_revert(&ovcs->cset); + overlay_notify(ovcs, OF_OVERLAY_POST_REMOVE); - free_overlay_fragments(ovcs); - idr_remove(&ovcs_idr, ovcs_id); - of_changeset_destroy(&ovcs->cset); - kfree(ovcs); + + free_overlay_changeset(ovcs); out: mutex_unlock(&of_mutex); @@ -718,20 +702,15 @@ EXPORT_SYMBOL_GPL(of_overlay_remove); int of_overlay_remove_all(void) { struct overlay_changeset *ovcs, *ovcs_n; - - mutex_lock(&of_mutex); + int ret; /* the tail of list is guaranteed to be safe to remove */ list_for_each_entry_safe_reverse(ovcs, ovcs_n, &ovcs_list, ovcs_list) { - list_del(&ovcs->ovcs_list); - __of_changeset_revert(&ovcs->cset); - free_overlay_fragments(ovcs); - idr_remove(&ovcs_idr, ovcs->id); - kfree(ovcs); + ret = of_overlay_remove(ovcs->id); + if (ret) + return ret; } - mutex_unlock(&of_mutex); - return 0; } EXPORT_SYMBOL_GPL(of_overlay_remove_all); -- cgit From 24789c5ce5a373dd55640f9cd79117fcc3ccc46d Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:26 -0700 Subject: of: overlay: detect cases where device tree may become corrupt When an attempt to apply an overlay changeset fails, an effort is made to revert any partial application of the changeset. When an attempt to remove an overlay changeset fails, an effort is made to re-apply any partial reversion of the changeset. The existing code does not check for failure to recover a failed overlay changeset application or overlay changeset revert. Add the missing checks and flag the devicetree as corrupt if the state of the devicetree can not be determined. Improve and expand the returned errors to more fully reflect the result of the effort to undo the partial effects of a failed attempt to apply or remove an overlay changeset. If the device tree might be corrupt, do not allow further attempts to apply or remove an overlay changeset. When creating an overlay changeset from an overlay device tree, add some additional warnings if the state of the overlay device tree is not as expected. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 135 ++++++++++++++++++++++---- drivers/of/of_private.h | 8 +- drivers/of/overlay.c | 253 ++++++++++++++++++++++++++++++++++++++++-------- drivers/of/unittest.c | 57 +++++------ 4 files changed, 364 insertions(+), 89 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 4bc96af4d8e2..747d87619faf 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -491,11 +491,12 @@ static void __of_changeset_entry_invert(struct of_changeset_entry *ce, } } -static void __of_changeset_entry_notify(struct of_changeset_entry *ce, bool revert) +static int __of_changeset_entry_notify(struct of_changeset_entry *ce, + bool revert) { struct of_reconfig_data rd; struct of_changeset_entry ce_inverted; - int ret; + int ret = 0; if (revert) { __of_changeset_entry_invert(ce, &ce_inverted); @@ -517,11 +518,12 @@ static void __of_changeset_entry_notify(struct of_changeset_entry *ce, bool reve default: pr_err("invalid devicetree changeset action: %i\n", (int)ce->action); - return; + ret = -EINVAL; } if (ret) pr_err("changeset notifier error @%pOF\n", ce->np); + return ret; } static int __of_changeset_entry_apply(struct of_changeset_entry *ce) @@ -655,32 +657,82 @@ void of_changeset_destroy(struct of_changeset *ocs) } EXPORT_SYMBOL_GPL(of_changeset_destroy); -int __of_changeset_apply(struct of_changeset *ocs) +/* + * Apply the changeset entries in @ocs. + * If apply fails, an attempt is made to revert the entries that were + * successfully applied. + * + * If multiple revert errors occur then only the final revert error is reported. + * + * Returns 0 on success, a negative error value in case of an error. + * If a revert error occurs, it is returned in *ret_revert. + */ +int __of_changeset_apply_entries(struct of_changeset *ocs, int *ret_revert) { struct of_changeset_entry *ce; - int ret; + int ret, ret_tmp; - /* perform the rest of the work */ pr_debug("changeset: applying...\n"); list_for_each_entry(ce, &ocs->entries, node) { ret = __of_changeset_entry_apply(ce); if (ret) { pr_err("Error applying changeset (%d)\n", ret); - list_for_each_entry_continue_reverse(ce, &ocs->entries, node) - __of_changeset_entry_revert(ce); + list_for_each_entry_continue_reverse(ce, &ocs->entries, + node) { + ret_tmp = __of_changeset_entry_revert(ce); + if (ret_tmp) + *ret_revert = ret_tmp; + } return ret; } } - pr_debug("changeset: applied, emitting notifiers.\n"); + + return 0; +} + +/* + * Returns 0 on success, a negative error value in case of an error. + * + * If multiple changset entry notification errors occur then only the + * final notification error is reported. + */ +int __of_changeset_apply_notify(struct of_changeset *ocs) +{ + struct of_changeset_entry *ce; + int ret = 0, ret_tmp; + + pr_debug("changeset: emitting notifiers.\n"); /* drop the global lock while emitting notifiers */ mutex_unlock(&of_mutex); - list_for_each_entry(ce, &ocs->entries, node) - __of_changeset_entry_notify(ce, 0); + list_for_each_entry(ce, &ocs->entries, node) { + ret_tmp = __of_changeset_entry_notify(ce, 0); + if (ret_tmp) + ret = ret_tmp; + } mutex_lock(&of_mutex); pr_debug("changeset: notifiers sent.\n"); - return 0; + return ret; +} + +/* + * Returns 0 on success, a negative error value in case of an error. + * + * If a changeset entry apply fails, an attempt is made to revert any + * previous entries in the changeset. If any of the reverts fails, + * that failure is not reported. Thus the state of the device tree + * is unknown if an apply error occurs. + */ +static int __of_changeset_apply(struct of_changeset *ocs) +{ + int ret, ret_revert = 0; + + ret = __of_changeset_apply_entries(ocs, &ret_revert); + if (!ret) + ret = __of_changeset_apply_notify(ocs); + + return ret; } /** @@ -707,31 +759,74 @@ int of_changeset_apply(struct of_changeset *ocs) } EXPORT_SYMBOL_GPL(of_changeset_apply); -int __of_changeset_revert(struct of_changeset *ocs) +/* + * Revert the changeset entries in @ocs. + * If revert fails, an attempt is made to re-apply the entries that were + * successfully removed. + * + * If multiple re-apply errors occur then only the final apply error is + * reported. + * + * Returns 0 on success, a negative error value in case of an error. + * If an apply error occurs, it is returned in *ret_apply. + */ +int __of_changeset_revert_entries(struct of_changeset *ocs, int *ret_apply) { struct of_changeset_entry *ce; - int ret; + int ret, ret_tmp; pr_debug("changeset: reverting...\n"); list_for_each_entry_reverse(ce, &ocs->entries, node) { ret = __of_changeset_entry_revert(ce); if (ret) { pr_err("Error reverting changeset (%d)\n", ret); - list_for_each_entry_continue(ce, &ocs->entries, node) - __of_changeset_entry_apply(ce); + list_for_each_entry_continue(ce, &ocs->entries, node) { + ret_tmp = __of_changeset_entry_apply(ce); + if (ret_tmp) + *ret_apply = ret_tmp; + } return ret; } } - pr_debug("changeset: reverted, emitting notifiers.\n"); + + return 0; +} + +/* + * If multiple changset entry notification errors occur then only the + * final notification error is reported. + */ +int __of_changeset_revert_notify(struct of_changeset *ocs) +{ + struct of_changeset_entry *ce; + int ret = 0, ret_tmp; + + pr_debug("changeset: emitting notifiers.\n"); /* drop the global lock while emitting notifiers */ mutex_unlock(&of_mutex); - list_for_each_entry_reverse(ce, &ocs->entries, node) - __of_changeset_entry_notify(ce, 1); + list_for_each_entry_reverse(ce, &ocs->entries, node) { + ret_tmp = __of_changeset_entry_notify(ce, 1); + if (ret_tmp) + ret = ret_tmp; + } mutex_lock(&of_mutex); pr_debug("changeset: notifiers sent.\n"); - return 0; + return ret; +} + +static int __of_changeset_revert(struct of_changeset *ocs) +{ + int ret, ret_reply; + + ret_reply = 0; + ret = __of_changeset_revert_entries(ocs, &ret_reply); + + if (!ret) + ret = __of_changeset_revert_notify(ocs); + + return ret; } /** diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 43df14f0cbce..36357f571df2 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -39,8 +39,12 @@ extern struct kset *of_kset; extern int of_property_notify(int action, struct device_node *np, struct property *prop, struct property *old_prop); 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_changeset_apply_entries(struct of_changeset *ocs, + int *ret_revert); +extern int __of_changeset_apply_notify(struct of_changeset *ocs); +extern int __of_changeset_revert_entries(struct of_changeset *ocs, + int *ret_apply); +extern int __of_changeset_revert_notify(struct of_changeset *ocs); #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 905916e17eec..78c50fd57750 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -50,6 +50,22 @@ struct overlay_changeset { struct of_changeset cset; }; +/* flags are sticky - once set, do not reset */ +static int devicetree_state_flags; +#define DTSF_APPLY_FAIL 0x01 +#define DTSF_REVERT_FAIL 0x02 + +/* + * If a changeset apply or revert encounters an error, an attempt will + * be made to undo partial changes, but may fail. If the undo fails + * we do not know the state of the devicetree. + */ +static int devicetree_corrupt(void) +{ + return devicetree_state_flags & + (DTSF_APPLY_FAIL | DTSF_REVERT_FAIL); +} + static int build_changeset_next_level(struct overlay_changeset *ovcs, struct device_node *target_node, const struct device_node *overlay_node, @@ -72,6 +88,13 @@ int of_overlay_notifier_unregister(struct notifier_block *nb) } EXPORT_SYMBOL_GPL(of_overlay_notifier_unregister); +static char *of_overlay_action_name[] = { + "pre-apply", + "post-apply", + "pre-remove", + "post-remove", +}; + static int overlay_notify(struct overlay_changeset *ovcs, enum of_overlay_notify_action action) { @@ -86,8 +109,14 @@ static int overlay_notify(struct overlay_changeset *ovcs, ret = blocking_notifier_call_chain(&overlay_notify_chain, action, &nd); - if (ret) - return notifier_to_errno(ret); + if (ret == NOTIFY_STOP) + return 0; + if (ret) { + ret = notifier_to_errno(ret); + pr_err("overlay changeset %s notifier error %d, target: %pOF\n", + of_overlay_action_name[action], ret, nd.target); + return ret; + } } return 0; @@ -240,6 +269,14 @@ static int add_changeset_property(struct overlay_changeset *ovcs, * build_changeset_next_level(). * * NOTE: Multiple mods of created nodes not supported. + * If more than one fragment contains a node that does not already exist + * in the live tree, then for each fragment of_changeset_attach_node() + * will add a changeset entry to add the node. When the changeset is + * applied, __of_attach_node() will attach the node twice (once for + * each fragment). At this point the device tree will be corrupted. + * + * TODO: add integrity check to ensure that multiple fragments do not + * create the same node. * * Returns 0 on success, -ENOMEM if memory allocation failure, or -EINVAL if * invalid @overlay. @@ -312,8 +349,8 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs, ret = add_changeset_property(ovcs, target_node, prop, is_symbols_node); if (ret) { - pr_err("Failed to apply prop @%pOF/%s\n", - target_node, prop->name); + pr_debug("Failed to apply prop @%pOF/%s, err=%d\n", + target_node, prop->name, ret); return ret; } } @@ -324,8 +361,8 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs, for_each_child_of_node(overlay_node, child) { ret = add_changeset_node(ovcs, target_node, child); if (ret) { - pr_err("Failed to apply node @%pOF/%s\n", - target_node, child->name); + pr_debug("Failed to apply node @%pOF/%s, err=%d\n", + target_node, child->name, ret); of_node_put(child); return ret; } @@ -357,7 +394,7 @@ static int build_changeset(struct overlay_changeset *ovcs) fragment->overlay, fragment->is_symbols_node); if (ret) { - pr_err("apply failed '%pOF'\n", fragment->target); + pr_debug("apply failed '%pOF'\n", fragment->target); return ret; } } @@ -412,6 +449,19 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs, struct fragment *fragments; int cnt, ret; + /* + * Warn for some issues. Can not return -EINVAL for these until + * of_unittest_apply_overlay() is fixed to pass these checks. + */ + if (!of_node_check_flag(tree, OF_DYNAMIC)) + pr_debug("%s() tree is not dynamic\n", __func__); + + if (!of_node_check_flag(tree, OF_DETACHED)) + pr_debug("%s() tree is not detached\n", __func__); + + if (!of_node_is_root(tree)) + pr_debug("%s() tree is not root\n", __func__); + INIT_LIST_HEAD(&ovcs->ovcs_list); of_changeset_init(&ovcs->cset); @@ -485,12 +535,13 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs, return 0; - err_free_fragments: kfree(fragments); err_free_idr: idr_remove(&ovcs_idr, ovcs->id); + pr_err("%s() failed, ret = %d\n", __func__, ret); + return ret; } @@ -517,33 +568,71 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) /** * of_overlay_apply() - Create and apply an overlay changeset * @tree: Expanded overlay device tree + * @ovcs_id: Pointer to overlay changeset id + * + * Creates and applies an overlay changeset. * - * Creates and applies an overlay changeset. If successful, the overlay - * changeset is added to the overlay changeset list. + * If an error occurs in a pre-apply notifier, then no changes are made + * to the device tree. * - * Returns the id of the created overlay changeset, or a negative error number + + * A non-zero return value will not have created the changeset if error is from: + * - parameter checks + * - building the changeset + * - overlay changset pre-apply notifier + * + * If an error is returned by an overlay changeset pre-apply notifier + * then no further overlay changeset pre-apply notifier will be called. + * + * A non-zero return value will have created the changeset if error is from: + * - overlay changeset entry notifier + * - overlay changset post-apply notifier + * + * If an error is returned by an overlay changeset post-apply notifier + * then no further overlay changeset post-apply notifier will be called. + * + * If more than one notifier returns an error, then the last notifier + * error to occur is returned. + * + * If an error occurred while applying the overlay changeset, then an + * attempt is made to revert any changes that were made to the + * device tree. If there were any errors during the revert attempt + * then the state of the device tree can not be determined, and any + * following attempt to apply or remove an overlay changeset will be + * refused. + * + * Returns 0 on success, or a negative error number. Overlay changeset + * id is returned to *ovcs_id. */ -int of_overlay_apply(struct device_node *tree) + +int of_overlay_apply(struct device_node *tree, int *ovcs_id) { struct overlay_changeset *ovcs; - int ret; + int ret = 0, ret_revert, ret_tmp; + + *ovcs_id = 0; + + if (devicetree_corrupt()) { + pr_err("devicetree state suspect, refuse to apply overlay\n"); + ret = -EBUSY; + goto out; + } ovcs = kzalloc(sizeof(*ovcs), GFP_KERNEL); - if (!ovcs) - return -ENOMEM; + if (!ovcs) { + ret = -ENOMEM; + goto out; + } mutex_lock(&of_mutex); ret = init_overlay_changeset(ovcs, tree); - if (ret) { - pr_err("init_overlay_changeset() failed, ret = %d\n", ret); + if (ret) goto err_free_overlay_changeset; - } ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY); - if (ret < 0) { - pr_err("%s: Pre-apply notifier failed (ret=%d)\n", - __func__, ret); + if (ret) { + pr_err("overlay changeset pre-apply notify error %d\n", ret); goto err_free_overlay_changeset; } @@ -551,23 +640,46 @@ int of_overlay_apply(struct device_node *tree) if (ret) goto err_free_overlay_changeset; - ret = __of_changeset_apply(&ovcs->cset); - if (ret) + ret_revert = 0; + ret = __of_changeset_apply_entries(&ovcs->cset, &ret_revert); + if (ret) { + if (ret_revert) { + pr_debug("overlay changeset revert error %d\n", + ret_revert); + devicetree_state_flags |= DTSF_APPLY_FAIL; + } goto err_free_overlay_changeset; + } else { + ret = __of_changeset_apply_notify(&ovcs->cset); + if (ret) + pr_err("overlay changeset entry notify error %d\n", + ret); + /* fall through */ + } list_add_tail(&ovcs->ovcs_list, &ovcs_list); - - overlay_notify(ovcs, OF_OVERLAY_POST_APPLY); + *ovcs_id = ovcs->id; + + ret_tmp = overlay_notify(ovcs, OF_OVERLAY_POST_APPLY); + if (ret_tmp) { + pr_err("overlay changeset post-apply notify error %d\n", + ret_tmp); + if (!ret) + ret = ret_tmp; + } mutex_unlock(&of_mutex); - return ovcs->id; + goto out; err_free_overlay_changeset: free_overlay_changeset(ovcs); mutex_unlock(&of_mutex); +out: + pr_debug("%s() err=%d\n", __func__, ret); + return ret; } EXPORT_SYMBOL_GPL(of_overlay_apply); @@ -649,45 +761,106 @@ static int overlay_removal_is_ok(struct overlay_changeset *remove_ovcs) /** * of_overlay_remove() - Revert and free an overlay changeset - * @ovcs_id: Overlay changeset id number + * @ovcs_id: Pointer to overlay changeset id * - * Removes an overlay if it is permissible. ovcs_id was previously returned + * Removes an overlay if it is permissible. @ovcs_id was previously returned * by of_overlay_apply(). * - * Returns 0 on success, or a negative error number + * If an error occurred while attempting to revert the overlay changeset, + * then an attempt is made to re-apply any changeset entry that was + * reverted. If an error occurs on re-apply then the state of the device + * tree can not be determined, and any following attempt to apply or remove + * an overlay changeset will be refused. + * + * A non-zero return value will not revert the changeset if error is from: + * - parameter checks + * - overlay changset pre-remove notifier + * - overlay changeset entry revert + * + * If an error is returned by an overlay changeset pre-remove notifier + * then no further overlay changeset pre-remove notifier will be called. + * + * If more than one notifier returns an error, then the last notifier + * error to occur is returned. + * + * A non-zero return value will revert the changeset if error is from: + * - overlay changeset entry notifier + * - overlay changset post-remove notifier + * + * If an error is returned by an overlay changeset post-remove notifier + * then no further overlay changeset post-remove notifier will be called. + * + * Returns 0 on success, or a negative error number. *ovcs_id is set to + * zero after reverting the changeset, even if a subsequent error occurs. */ -int of_overlay_remove(int ovcs_id) +int of_overlay_remove(int *ovcs_id) { struct overlay_changeset *ovcs; - int ret = 0; + int ret, ret_apply, ret_tmp; + + ret = 0; + + if (devicetree_corrupt()) { + pr_err("suspect devicetree state, refuse to remove overlay\n"); + ret = -EBUSY; + goto out; + } mutex_lock(&of_mutex); - ovcs = idr_find(&ovcs_idr, ovcs_id); + ovcs = idr_find(&ovcs_idr, *ovcs_id); if (!ovcs) { ret = -ENODEV; - pr_err("remove: Could not find overlay #%d\n", ovcs_id); - goto out; + pr_err("remove: Could not find overlay #%d\n", *ovcs_id); + goto out_unlock; } if (!overlay_removal_is_ok(ovcs)) { ret = -EBUSY; - goto out; + goto out_unlock; } - overlay_notify(ovcs, OF_OVERLAY_PRE_REMOVE); + ret = overlay_notify(ovcs, OF_OVERLAY_PRE_REMOVE); + if (ret) { + pr_err("overlay changeset pre-remove notify error %d\n", ret); + goto out_unlock; + } list_del(&ovcs->ovcs_list); - __of_changeset_revert(&ovcs->cset); + ret_apply = 0; + ret = __of_changeset_revert_entries(&ovcs->cset, &ret_apply); + if (ret) { + if (ret_apply) + devicetree_state_flags |= DTSF_REVERT_FAIL; + goto out_unlock; + } else { + ret = __of_changeset_revert_notify(&ovcs->cset); + if (ret) { + pr_err("overlay changeset entry notify error %d\n", + ret); + /* fall through - changeset was reverted */ + } + } - overlay_notify(ovcs, OF_OVERLAY_POST_REMOVE); + *ovcs_id = 0; + + ret_tmp = overlay_notify(ovcs, OF_OVERLAY_POST_REMOVE); + if (ret_tmp) { + pr_err("overlay changeset post-remove notify error %d\n", + ret_tmp); + if (!ret) + ret = ret_tmp; + } free_overlay_changeset(ovcs); -out: +out_unlock: mutex_unlock(&of_mutex); +out: + pr_debug("%s() err=%d\n", __func__, ret); + return ret; } EXPORT_SYMBOL_GPL(of_overlay_remove); @@ -706,7 +879,7 @@ int of_overlay_remove_all(void) /* the tail of list is guaranteed to be safe to remove */ list_for_each_entry_safe_reverse(ovcs, ovcs_n, &ovcs_list, ovcs_list) { - ret = of_overlay_remove(ovcs->id); + ret = of_overlay_remove(&ovcs->id); if (ret) return ret; } diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index bbdaf5606820..3640dae4b9b2 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -1217,7 +1217,7 @@ static void of_unittest_untrack_overlay(int id) static void of_unittest_destroy_tracked_overlays(void) { - int id, ret, defers; + int id, ret, defers, ovcs_id; if (overlay_first_id < 0) return; @@ -1230,7 +1230,8 @@ static void of_unittest_destroy_tracked_overlays(void) if (!(overlay_id_bits[BIT_WORD(id)] & BIT_MASK(id))) continue; - ret = of_overlay_remove(id + overlay_first_id); + ovcs_id = id + overlay_first_id; + ret = of_overlay_remove(&ovcs_id); if (ret == -ENODEV) { pr_warn("%s: no overlay to destroy for #%d\n", __func__, id + overlay_first_id); @@ -1252,7 +1253,7 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr, int *overlay_id) { struct device_node *np = NULL; - int ret, id = -1; + int ret; np = of_find_node_by_path(overlay_path(overlay_nr)); if (np == NULL) { @@ -1262,23 +1263,20 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr, goto out; } - ret = of_overlay_apply(np); + *overlay_id = 0; + ret = of_overlay_apply(np, overlay_id); if (ret < 0) { unittest(0, "could not create overlay from \"%s\"\n", overlay_path(overlay_nr)); goto out; } - id = ret; - of_unittest_track_overlay(id); + of_unittest_track_overlay(*overlay_id); ret = 0; out: of_node_put(np); - if (overlay_id) - *overlay_id = id; - return ret; } @@ -1286,7 +1284,7 @@ out: static int of_unittest_apply_overlay_check(int overlay_nr, int unittest_nr, int before, int after, enum overlay_type ovtype) { - int ret; + int ret, ovcs_id; /* unittest device must not be in before state */ if (of_unittest_device_exists(unittest_nr, ovtype) != before) { @@ -1297,7 +1295,8 @@ static int of_unittest_apply_overlay_check(int overlay_nr, int unittest_nr, return -EINVAL; } - ret = of_unittest_apply_overlay(overlay_nr, unittest_nr, NULL); + ovcs_id = 0; + ret = of_unittest_apply_overlay(overlay_nr, unittest_nr, &ovcs_id); if (ret != 0) { /* of_unittest_apply_overlay already called unittest() */ return ret; @@ -1320,7 +1319,7 @@ static int of_unittest_apply_revert_overlay_check(int overlay_nr, int unittest_nr, int before, int after, enum overlay_type ovtype) { - int ret, ov_id; + int ret, ovcs_id; /* unittest device must be in before state */ if (of_unittest_device_exists(unittest_nr, ovtype) != before) { @@ -1332,7 +1331,8 @@ static int of_unittest_apply_revert_overlay_check(int overlay_nr, } /* apply the overlay */ - ret = of_unittest_apply_overlay(overlay_nr, unittest_nr, &ov_id); + ovcs_id = 0; + ret = of_unittest_apply_overlay(overlay_nr, unittest_nr, &ovcs_id); if (ret != 0) { /* of_unittest_apply_overlay already called unittest() */ return ret; @@ -1347,7 +1347,7 @@ static int of_unittest_apply_revert_overlay_check(int overlay_nr, return -EINVAL; } - ret = of_overlay_remove(ov_id); + ret = of_overlay_remove(&ovcs_id); if (ret != 0) { unittest(0, "overlay @\"%s\" failed to be destroyed @\"%s\"\n", overlay_path(overlay_nr), @@ -1449,7 +1449,7 @@ static void of_unittest_overlay_5(void) static void of_unittest_overlay_6(void) { struct device_node *np; - int ret, i, ov_id[2]; + int ret, i, ov_id[2], ovcs_id; int overlay_nr = 6, unittest_nr = 6; int before = 0, after = 1; @@ -1476,13 +1476,14 @@ static void of_unittest_overlay_6(void) return; } - ret = of_overlay_apply(np); + ovcs_id = 0; + ret = of_overlay_apply(np, &ovcs_id); if (ret < 0) { unittest(0, "could not create overlay from \"%s\"\n", overlay_path(overlay_nr + i)); return; } - ov_id[i] = ret; + ov_id[i] = ovcs_id; of_unittest_track_overlay(ov_id[i]); } @@ -1500,7 +1501,8 @@ static void of_unittest_overlay_6(void) } for (i = 1; i >= 0; i--) { - ret = of_overlay_remove(ov_id[i]); + ovcs_id = ov_id[i]; + ret = of_overlay_remove(&ovcs_id); if (ret != 0) { unittest(0, "overlay @\"%s\" failed destroy @\"%s\"\n", overlay_path(overlay_nr + i), @@ -1531,7 +1533,7 @@ static void of_unittest_overlay_6(void) static void of_unittest_overlay_8(void) { struct device_node *np; - int ret, i, ov_id[2]; + int ret, i, ov_id[2], ovcs_id; int overlay_nr = 8, unittest_nr = 8; /* we don't care about device state in this test */ @@ -1546,18 +1548,20 @@ static void of_unittest_overlay_8(void) return; } - ret = of_overlay_apply(np); + ovcs_id = 0; + ret = of_overlay_apply(np, &ovcs_id); if (ret < 0) { unittest(0, "could not create overlay from \"%s\"\n", overlay_path(overlay_nr + i)); return; } - ov_id[i] = ret; + ov_id[i] = ovcs_id; of_unittest_track_overlay(ov_id[i]); } /* now try to remove first overlay (it should fail) */ - ret = of_overlay_remove(ov_id[0]); + ovcs_id = ov_id[0]; + ret = of_overlay_remove(&ovcs_id); if (ret == 0) { unittest(0, "overlay @\"%s\" was destroyed @\"%s\"\n", overlay_path(overlay_nr + 0), @@ -1568,7 +1572,8 @@ static void of_unittest_overlay_8(void) /* removing them in order should work */ for (i = 1; i >= 0; i--) { - ret = of_overlay_remove(ov_id[i]); + ovcs_id = ov_id[i]; + ret = of_overlay_remove(&ovcs_id); if (ret != 0) { unittest(0, "overlay @\"%s\" not destroyed @\"%s\"\n", overlay_path(overlay_nr + i), @@ -2149,13 +2154,11 @@ static int __init overlay_data_add(int onum) goto out_free_np_overlay; } - ret = of_overlay_apply(info->np_overlay); + info->overlay_id = 0; + ret = of_overlay_apply(info->np_overlay, &info->overlay_id); if (ret < 0) { pr_err("of_overlay_apply() (ret=%d), %d\n", ret, onum); goto out_free_np_overlay; - } else { - info->overlay_id = ret; - ret = 0; } pr_debug("__dtb_overlay_begin applied, overlay id %d\n", ret); -- cgit From 87f242c119c403e8b948c8b95eca4ab6212fd1a9 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:27 -0700 Subject: of: overlay: expand check of whether overlay changeset can be removed The test of whether it is safe to remove an overlay changeset looked at whether any node in the overlay changeset was in a subtree rooted at any more recently applied overlay changeset node. The test failed to determine whether any node in the overlay changeset was the root of a subtree that contained a more recently applied overlay changeset node. Add this additional check to the test. The test is still lacking any check for any phandle dependencies. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 78c50fd57750..4cdee169a5ab 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -707,13 +707,13 @@ static int find_node(struct device_node *tree, struct device_node *np) } /* - * Is @remove_ce_np a child of or the same as any + * Is @remove_ce_node a child of, a parent of, or the same as any * node in an overlay changeset more topmost than @remove_ovcs? * * Returns 1 if found, else 0 */ -static int node_in_later_cs(struct overlay_changeset *remove_ovcs, - struct device_node *remove_ce_np) +static int node_overlaps_later_cs(struct overlay_changeset *remove_ovcs, + struct device_node *remove_ce_node) { struct overlay_changeset *ovcs; struct of_changeset_entry *ce; @@ -723,10 +723,16 @@ static int node_in_later_cs(struct overlay_changeset *remove_ovcs, break; list_for_each_entry(ce, &ovcs->cset.entries, node) { - if (find_node(ce->np, remove_ce_np)) { - pr_err("%s: #%d clashes #%d @%pOF\n", + if (find_node(ce->np, remove_ce_node)) { + pr_err("%s: #%d overlaps with #%d @%pOF\n", __func__, remove_ovcs->id, ovcs->id, - remove_ce_np); + remove_ce_node); + return 1; + } + if (find_node(remove_ce_node, ce->np)) { + pr_err("%s: #%d overlaps with #%d @%pOF\n", + __func__, remove_ovcs->id, ovcs->id, + remove_ce_node); return 1; } } @@ -750,7 +756,7 @@ static int overlay_removal_is_ok(struct overlay_changeset *remove_ovcs) struct of_changeset_entry *remove_ce; list_for_each_entry(remove_ce, &remove_ovcs->cset.entries, node) { - if (node_in_later_cs(remove_ovcs, remove_ce->np)) { + if (node_overlaps_later_cs(remove_ovcs, remove_ce->np)) { pr_err("overlay #%d is not topmost\n", remove_ovcs->id); return 0; } -- cgit From 6d0f5470dbdeb7f9b1e20fc9409bf07fab1b5ac5 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:28 -0700 Subject: of: overlay: loosen overly strict phandle clash check When an overlay contains a node that already exists in the live device tree, the overlay node is not allowed to change the phandle of the existing node. The existing check refused to allow an overlay node to set the node phandle even when the existing node did not have a phandle. Relax the check to allow an overlay node to set the phandle value if the existing node does not have a phandle. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 4cdee169a5ab..791753321ed2 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -311,10 +311,10 @@ static int add_changeset_node(struct overlay_changeset *ovcs, return build_changeset_next_level(ovcs, tchild, node, 0); } - if (node->phandle) - return -EINVAL; - - ret = build_changeset_next_level(ovcs, tchild, node, 0); + if (node->phandle && tchild->phandle) + ret = -EINVAL; + else + ret = build_changeset_next_level(ovcs, tchild, node, 0); of_node_put(tchild); return ret; -- cgit From f948d6d8b792bb90041edc12eac35faf83030994 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:29 -0700 Subject: of: overlay: avoid race condition between applying multiple overlays The process of applying an overlay consists of: - unflatten an overlay FDT (flattened device tree) into an EDT (expanded device tree) - fixup the phandle values in the overlay EDT to fit in a range above the phandle values in the live device tree - create the overlay changeset to reflect the contents of the overlay EDT - apply the overlay changeset, to modify the live device tree, potentially changing the maximum phandle value in the live device tree There is currently no protection against two overlay applies concurrently determining what range of phandle values are in use in the live device tree, and subsequently changing that range. Add a mutex to prevent multiple overlay applies from occurring simultaneously. Move of_resolve_phandles() into of_overlay_apply() so that it does not have to be duplicated by each caller of of_overlay_apply(). The test in of_resolve_phandles() that the overlay tree is detached is temporarily disabled so that old style overlay unittests do not fail. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/of_private.h | 12 ++++++++++++ drivers/of/overlay.c | 32 ++++++++++++++++++++++++++++++++ drivers/of/resolver.c | 7 +++++++ drivers/of/unittest.c | 22 ++++++++++++++++------ 4 files changed, 67 insertions(+), 6 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 36357f571df2..248730567dbe 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -76,6 +76,18 @@ static inline int __of_attach_node_sysfs(struct device_node *np) static inline void __of_detach_node_sysfs(struct device_node *np) {} #endif +#if defined(CONFIG_OF_RESOLVE) +int of_resolve_phandles(struct device_node *tree); +#endif + +#if defined(CONFIG_OF_OVERLAY) +void of_overlay_mutex_lock(void); +void of_overlay_mutex_unlock(void); +#else +static inline void of_overlay_mutex_lock(void) {}; +static inline void of_overlay_mutex_unlock(void) {}; +#endif + #if defined(CONFIG_OF_UNITTEST) && defined(CONFIG_OF_OVERLAY) extern void __init unittest_unflatten_overlay_base(void); #else diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 791753321ed2..d164f86e5541 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -71,6 +71,28 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs, const struct device_node *overlay_node, bool is_symbols_node); +/* + * of_resolve_phandles() finds the largest phandle in the live tree. + * of_overlay_apply() may add a larger phandle to the live tree. + * Do not allow race between two overlays being applied simultaneously: + * mutex_lock(&of_overlay_phandle_mutex) + * of_resolve_phandles() + * of_overlay_apply() + * mutex_unlock(&of_overlay_phandle_mutex) + */ +static DEFINE_MUTEX(of_overlay_phandle_mutex); + +void of_overlay_mutex_lock(void) +{ + mutex_lock(&of_overlay_phandle_mutex); +} + +void of_overlay_mutex_unlock(void) +{ + mutex_unlock(&of_overlay_phandle_mutex); +} + + static LIST_HEAD(ovcs_list); static DEFINE_IDR(ovcs_idr); @@ -624,6 +646,12 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id) goto out; } + of_overlay_mutex_lock(); + + ret = of_resolve_phandles(tree); + if (ret) + goto err_overlay_unlock; + mutex_lock(&of_mutex); ret = init_overlay_changeset(ovcs, tree); @@ -669,9 +697,13 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id) } mutex_unlock(&of_mutex); + of_overlay_mutex_unlock(); goto out; +err_overlay_unlock: + of_overlay_mutex_unlock(); + err_free_overlay_changeset: free_overlay_changeset(ovcs); diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index bd21a66f6930..cfaeef5f6cb1 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -271,11 +271,18 @@ int of_resolve_phandles(struct device_node *overlay) err = -EINVAL; goto out; } + +#if 0 + Temporarily disable check so that old style overlay unittests + do not fail when of_resolve_phandles() is moved into + of_overlay_apply(). + if (!of_node_check_flag(overlay, OF_DETACHED)) { pr_err("overlay not detached\n"); err = -EINVAL; goto out; } +#endif phandle_delta = live_tree_max_phandle() + 1; adjust_overlay_phandles(overlay, phandle_delta); diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 3640dae4b9b2..273d78c1520b 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -993,9 +993,17 @@ static int __init unittest_data_add(void) pr_warn("%s: No tree to attach; not running tests\n", __func__); return -ENODATA; } + + /* + * This lock normally encloses of_overlay_apply() as well as + * of_resolve_phandles(). + */ + of_overlay_mutex_lock(); + rc = of_resolve_phandles(unittest_data_node); if (rc) { pr_err("%s: Failed to resolve phandles (rc=%i)\n", __func__, rc); + of_overlay_mutex_unlock(); return -EINVAL; } @@ -1005,6 +1013,7 @@ static int __init unittest_data_add(void) __of_attach_node_sysfs(np); of_aliases = of_find_node_by_path("/aliases"); of_chosen = of_find_node_by_path("/chosen"); + of_overlay_mutex_unlock(); return 0; } @@ -1017,6 +1026,9 @@ static int __init unittest_data_add(void) attach_node_and_children(np); np = next; } + + of_overlay_mutex_unlock(); + return 0; } @@ -2148,16 +2160,11 @@ static int __init overlay_data_add(int onum) goto out_free_data; } - ret = of_resolve_phandles(info->np_overlay); - if (ret) { - pr_err("resolve ot phandles (ret=%d), %d\n", ret, onum); - goto out_free_np_overlay; - } - info->overlay_id = 0; ret = of_overlay_apply(info->np_overlay, &info->overlay_id); if (ret < 0) { pr_err("of_overlay_apply() (ret=%d), %d\n", ret, onum); + of_overlay_mutex_unlock(); goto out_free_np_overlay; } @@ -2207,7 +2214,10 @@ static __init void of_unittest_overlay_high_level(void) * Could not fixup phandles in unittest_unflatten_overlay_base() * because kmalloc() was not yet available. */ + of_overlay_mutex_lock(); of_resolve_phandles(overlay_base_root); + of_overlay_mutex_unlock(); + /* * do not allow overlay_base to duplicate any node already in -- cgit From 3912b7917ab78d75b32bec8d297ac3c46b1b2a44 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:30 -0700 Subject: of: overlay: simplify applying symbols from an overlay The code to apply symbols from an overlay to the live device tree was implemented with the intent to be minimally intrusive on the existing code. After recent restructuring of the overlay apply code, it is easier to disintangle the code that applies the symbols, and to make the overlay changeset creation code more straight forward and understandable. Remove the extra complexity, and make the code more obvious. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 91 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 26 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index d164f86e5541..602218e07ec3 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -32,21 +32,22 @@ struct fragment { struct device_node *target; struct device_node *overlay; - bool is_symbols_node; }; /** * struct overlay_changeset - * @ovcs_list: list on which we are located - * @count: count of @fragments structures - * @fragments: info about fragment nodes in overlay expanded device tree - * @cset: changeset to apply fragments to live device tree + * @ovcs_list: list on which we are located + * @count: count of fragment structures + * @fragments: fragment nodes in the overlay expanded device tree + * @symbols_fragment: last element of @fragments[] is the __symbols__ node + * @cset: changeset to apply fragments to live device tree */ struct overlay_changeset { int id; struct list_head ovcs_list; int count; struct fragment *fragments; + bool symbols_fragment; struct of_changeset cset; }; @@ -68,8 +69,7 @@ static int devicetree_corrupt(void) static int build_changeset_next_level(struct overlay_changeset *ovcs, struct device_node *target_node, - const struct device_node *overlay_node, - bool is_symbols_node); + const struct device_node *overlay_node); /* * of_resolve_phandles() finds the largest phandle in the live tree. @@ -221,7 +221,7 @@ static struct property *dup_and_fixup_symbol_prop( * @ovcs: overlay changeset * @target_node: where to place @overlay_prop in live tree * @overlay_prop: property to add or update, from overlay tree - * is_symbols_node: 1 if @target_node is "/__symbols__" + * @is_symbols_prop: 1 if @overlay_prop is from node "/__symbols__" * * If @overlay_prop does not already exist in @target_node, add changeset entry * to add @overlay_prop in @target_node, else add changeset entry to update @@ -237,7 +237,7 @@ static struct property *dup_and_fixup_symbol_prop( static int add_changeset_property(struct overlay_changeset *ovcs, struct device_node *target_node, struct property *overlay_prop, - bool is_symbols_node) + bool is_symbols_prop) { struct property *new_prop = NULL, *prop; int ret = 0; @@ -249,7 +249,7 @@ static int add_changeset_property(struct overlay_changeset *ovcs, !of_prop_cmp(overlay_prop->name, "linux,phandle")) return 0; - if (is_symbols_node) { + if (is_symbols_prop) { if (prop) return -EINVAL; new_prop = dup_and_fixup_symbol_prop(ovcs, overlay_prop); @@ -330,13 +330,13 @@ static int add_changeset_node(struct overlay_changeset *ovcs, if (ret) return ret; - return build_changeset_next_level(ovcs, tchild, node, 0); + return build_changeset_next_level(ovcs, tchild, node); } if (node->phandle && tchild->phandle) ret = -EINVAL; else - ret = build_changeset_next_level(ovcs, tchild, node, 0); + ret = build_changeset_next_level(ovcs, tchild, node); of_node_put(tchild); return ret; @@ -347,7 +347,6 @@ static int add_changeset_node(struct overlay_changeset *ovcs, * @ovcs: overlay changeset * @target_node: where to place @overlay_node in live tree * @overlay_node: node from within an overlay device tree fragment - * @is_symbols_node: @overlay_node is node "/__symbols__" * * Add the properties (if any) and nodes (if any) from @overlay_node to the * @ovcs->cset changeset. If an added node has child nodes, they will @@ -360,16 +359,14 @@ static int add_changeset_node(struct overlay_changeset *ovcs, */ static int build_changeset_next_level(struct overlay_changeset *ovcs, struct device_node *target_node, - const struct device_node *overlay_node, - bool is_symbols_node) + const struct device_node *overlay_node) { struct device_node *child; struct property *prop; int ret; for_each_property_of_node(overlay_node, prop) { - ret = add_changeset_property(ovcs, target_node, prop, - is_symbols_node); + ret = add_changeset_property(ovcs, target_node, prop, 0); if (ret) { pr_debug("Failed to apply prop @%pOF/%s, err=%d\n", target_node, prop->name, ret); @@ -377,9 +374,6 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs, } } - if (is_symbols_node) - return 0; - for_each_child_of_node(overlay_node, child) { ret = add_changeset_node(ovcs, target_node, child); if (ret) { @@ -393,6 +387,28 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs, return 0; } +/* + * Add the properties from __overlay__ node to the @ovcs->cset changeset. + */ +static int build_changeset_symbols_node(struct overlay_changeset *ovcs, + struct device_node *target_node, + const struct device_node *overlay_symbols_node) +{ + struct property *prop; + int ret; + + for_each_property_of_node(overlay_symbols_node, prop) { + ret = add_changeset_property(ovcs, target_node, prop, 1); + if (ret) { + pr_debug("Failed to apply prop @%pOF/%s, err=%d\n", + target_node, prop->name, ret); + return ret; + } + } + + return 0; +} + /** * build_changeset() - populate overlay changeset in @ovcs from @ovcs->fragments * @ovcs: Overlay changeset @@ -407,14 +423,33 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs, */ static int build_changeset(struct overlay_changeset *ovcs) { - int i, ret; + struct fragment *fragment; + int fragments_count, i, ret; - for (i = 0; i < ovcs->count; i++) { - struct fragment *fragment = &ovcs->fragments[i]; + /* + * if there is a symbols fragment in ovcs->fragments[i] it is + * the final element in the array + */ + if (ovcs->symbols_fragment) + fragments_count = ovcs->count - 1; + else + fragments_count = ovcs->count; + + for (i = 0; i < fragments_count; i++) { + fragment = &ovcs->fragments[i]; ret = build_changeset_next_level(ovcs, fragment->target, - fragment->overlay, - fragment->is_symbols_node); + fragment->overlay); + if (ret) { + pr_debug("apply failed '%pOF'\n", fragment->target); + return ret; + } + } + + if (ovcs->symbols_fragment) { + fragment = &ovcs->fragments[ovcs->count - 1]; + ret = build_changeset_symbols_node(ovcs, fragment->target, + fragment->overlay); if (ret) { pr_debug("apply failed '%pOF'\n", fragment->target); return ret; @@ -531,12 +566,16 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs, } } + /* + * if there is a symbols fragment in ovcs->fragments[i] it is + * the final element in the array + */ node = of_get_child_by_name(tree, "__symbols__"); if (node) { + ovcs->symbols_fragment = 1; fragment = &fragments[cnt]; fragment->overlay = node; fragment->target = of_find_node_by_path("/__symbols__"); - fragment->is_symbols_node = 1; if (!fragment->target) { pr_err("no symbols in root of device tree.\n"); -- cgit From e0a58f3e08d4b7fa8e2a4075c522f1a98c9e4cab Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:31 -0700 Subject: of: overlay: remove a dependency on device node full_name The "%pOF" printf format was recently added to print the full name of a device tree node, with the intent of changing the node full_name field to contain only the node name instead of the full path of the node. dup_and_fixup_symbol_prop() duplicates a property from the "/__symbols__" node of an overlay device tree. The value of each duplicated property must be fixed up to include the full path of a node in the live device tree. The current code uses the node's full_name for that purpose. Update the code to use the "%pOF" printf format to determine the node's full path. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/base.c | 2 +- drivers/of/of_private.h | 2 ++ drivers/of/overlay.c | 90 ++++++++++++++++++++++++++++++------------------- 3 files changed, 59 insertions(+), 35 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/base.c b/drivers/of/base.c index b98f3adffbb0..3de07e6e82a0 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -658,7 +658,7 @@ struct device_node *of_get_child_by_name(const struct device_node *node, } EXPORT_SYMBOL(of_get_child_by_name); -static struct device_node *__of_find_node_by_path(struct device_node *parent, +struct device_node *__of_find_node_by_path(struct device_node *parent, const char *path) { struct device_node *child; diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 248730567dbe..92a9a3687446 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -110,6 +110,8 @@ 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_path(struct device_node *parent, + const char *path); struct device_node *__of_find_node_by_full_path(struct device_node *node, const char *path); diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 602218e07ec3..0e9aefc1ff15 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -37,6 +37,7 @@ struct fragment { /** * struct overlay_changeset * @ovcs_list: list on which we are located + * @overlay_tree: expanded device tree that contains the fragment nodes * @count: count of fragment structures * @fragments: fragment nodes in the overlay expanded device tree * @symbols_fragment: last element of @fragments[] is the __symbols__ node @@ -45,6 +46,7 @@ struct fragment { struct overlay_changeset { int id; struct list_head ovcs_list; + struct device_node *overlay_tree; int count; struct fragment *fragments; bool symbols_fragment; @@ -145,12 +147,13 @@ static int overlay_notify(struct overlay_changeset *ovcs, } /* - * The properties in the "/__symbols__" node are "symbols". + * The values of properties in the "/__symbols__" node are paths in + * the ovcs->overlay_tree. When duplicating the properties, the paths + * need to be adjusted to be the correct path for the live device tree. * - * The value of properties in the "/__symbols__" node is the path of a - * node in the subtree of a fragment node's "__overlay__" node, for - * example "/fragment@0/__overlay__/symbol_path_tail". Symbol_path_tail - * can be a single node or it may be a multi-node path. + * The paths refer to a node in the subtree of a fragment node's "__overlay__" + * node, for example "/fragment@0/__overlay__/symbol_path_tail", + * where symbol_path_tail can be a single node or it may be a multi-node path. * * The duplicated property value will be modified by replacing the * "/fragment_name/__overlay/" portion of the value with the target @@ -160,59 +163,76 @@ static struct property *dup_and_fixup_symbol_prop( struct overlay_changeset *ovcs, const struct property *prop) { struct fragment *fragment; - struct property *new; - const char *overlay_name; - char *symbol_path_tail; - char *symbol_path; + struct property *new_prop; + struct device_node *fragment_node; + struct device_node *overlay_node; + const char *path; + const char *path_tail; const char *target_path; int k; - int symbol_path_tail_len; int overlay_name_len; + int path_len; + int path_tail_len; int target_path_len; if (!prop->value) return NULL; - symbol_path = prop->value; + if (strnlen(prop->value, prop->length) >= prop->length) + return NULL; + path = prop->value; + path_len = strlen(path); - new = kzalloc(sizeof(*new), GFP_KERNEL); - if (!new) + if (path_len < 1) return NULL; + fragment_node = __of_find_node_by_path(ovcs->overlay_tree, path + 1); + overlay_node = __of_find_node_by_path(fragment_node, "__overlay__/"); + of_node_put(fragment_node); + of_node_put(overlay_node); for (k = 0; k < ovcs->count; k++) { fragment = &ovcs->fragments[k]; - overlay_name = fragment->overlay->full_name; - overlay_name_len = strlen(overlay_name); - if (!strncasecmp(symbol_path, overlay_name, overlay_name_len)) + if (fragment->overlay == overlay_node) break; } - if (k >= ovcs->count) - goto err_free; + return NULL; + + overlay_name_len = snprintf(NULL, 0, "%pOF", fragment->overlay); - target_path = fragment->target->full_name; + if (overlay_name_len > path_len) + return NULL; + path_tail = path + overlay_name_len; + path_tail_len = strlen(path_tail); + + target_path = kasprintf(GFP_KERNEL, "%pOF", fragment->target); + if (!target_path) + return NULL; target_path_len = strlen(target_path); - symbol_path_tail = symbol_path + overlay_name_len; - symbol_path_tail_len = strlen(symbol_path_tail); + new_prop = kzalloc(sizeof(*new_prop), GFP_KERNEL); + if (!new_prop) + goto err_free_target_path; - new->name = kstrdup(prop->name, GFP_KERNEL); - new->length = target_path_len + symbol_path_tail_len + 1; - new->value = kzalloc(new->length, GFP_KERNEL); + new_prop->name = kstrdup(prop->name, GFP_KERNEL); + new_prop->length = target_path_len + path_tail_len + 1; + new_prop->value = kzalloc(new_prop->length, GFP_KERNEL); + if (!new_prop->name || !new_prop->value) + goto err_free_new_prop; - if (!new->name || !new->value) - goto err_free; + strcpy(new_prop->value, target_path); + strcpy(new_prop->value + target_path_len, path_tail); - strcpy(new->value, target_path); - strcpy(new->value + target_path_len, symbol_path_tail); + of_property_set_flag(new_prop, OF_DYNAMIC); - of_property_set_flag(new, OF_DYNAMIC); + return new_prop; - return new; +err_free_new_prop: + kfree(new_prop->name); + kfree(new_prop->value); + kfree(new_prop); +err_free_target_path: + kfree(target_path); - err_free: - kfree(new->name); - kfree(new->value); - kfree(new); return NULL; } @@ -519,6 +539,8 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs, if (!of_node_is_root(tree)) pr_debug("%s() tree is not root\n", __func__); + ovcs->overlay_tree = tree; + INIT_LIST_HEAD(&ovcs->ovcs_list); of_changeset_init(&ovcs->cset); -- cgit From 34ca5d76f250d4ba98c4cdc069ab79b395f9ecac Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Tue, 17 Oct 2017 16:36:32 -0700 Subject: of: overlay: remove unneeded check for NULL kbasename() kbasename() will not return NULL if passed a valid string. If the parameter passed to kbasename() in this case is already NULL then the devicetree has been corrupted. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 0e9aefc1ff15..c99842bb4b09 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -331,8 +331,6 @@ static int add_changeset_node(struct overlay_changeset *ovcs, int ret = 0; node_kbasename = kbasename(node->full_name); - if (!node_kbasename) - return -ENOMEM; for_each_child_of_node(target_node, tchild) if (!of_node_cmp(node_kbasename, kbasename(tchild->full_name))) -- cgit From a1d19bd4cf1febf0d5ff60243826a248bd20f1a5 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Thu, 19 Oct 2017 14:18:27 -0700 Subject: of: overlay: pr_err from return NOTIFY_OK to overlay apply/remove A device tree overlay notifier can return NOTIFY_OK, NOTIFY_STOP, or an embedded errno. overlay_notify() incorrectly reports an error for NOTIFY_OK. Reported-by: atull@kernel.org Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index c99842bb4b09..f5fce0fea40b 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -133,7 +133,7 @@ static int overlay_notify(struct overlay_changeset *ovcs, ret = blocking_notifier_call_chain(&overlay_notify_chain, action, &nd); - if (ret == NOTIFY_STOP) + if (ret == NOTIFY_OK || ret == NOTIFY_STOP) return 0; if (ret) { ret = notifier_to_errno(ret); -- cgit From 4ee7c0d9649d472d31969b9cbb8151161db6a807 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Thu, 19 Oct 2017 14:38:11 -0700 Subject: of: overlay: make pr_err() string unique The same error string occurs in drivers/of/resolver.c. Change the error here to more precisely describe this case, and avoid the possible confusion of looking in the wrong source location to understand the cause of an error. Signed-off-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/overlay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index f5fce0fea40b..c150abb9049d 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -598,7 +598,7 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs, fragment->target = of_find_node_by_path("/__symbols__"); if (!fragment->target) { - pr_err("no symbols in root of device tree.\n"); + pr_err("symbols in overlay, but not in live tree\n"); ret = -EINVAL; goto err_free_fragments; } -- cgit From 070ea018fa092f2ef41bef57896665a25f05d2d8 Mon Sep 17 00:00:00 2001 From: Lixin Wang Date: Mon, 23 Oct 2017 11:19:53 +0800 Subject: of: dynamic: fix memory leak related to properties of __of_node_dup If a node with no properties is dynamically added, then a property is dynamically added to the node, then the property is dynamically removed, the result will be node->properties == NULL and node->deadprops != NULL. Add a separate function to release the properties in both lists. Signed-off-by: Lixin Wang Reviewed-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/dynamic.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 747d87619faf..c454941b34ec 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -298,6 +298,18 @@ int of_detach_node(struct device_node *np) } EXPORT_SYMBOL_GPL(of_detach_node); +static void property_list_free(struct property *prop_list) +{ + struct property *prop, *next; + + for (prop = prop_list; prop != NULL; prop = next) { + next = prop->next; + kfree(prop->name); + kfree(prop->value); + kfree(prop); + } +} + /** * of_node_release() - release a dynamically allocated node * @kref: kref element of the node to be released @@ -307,7 +319,6 @@ EXPORT_SYMBOL_GPL(of_detach_node); void of_node_release(struct kobject *kobj) { struct device_node *node = kobj_to_device_node(kobj); - struct property *prop = node->properties; /* We should never be releasing nodes that haven't been detached. */ if (!of_node_check_flag(node, OF_DETACHED)) { @@ -318,18 +329,9 @@ void of_node_release(struct kobject *kobj) if (!of_node_check_flag(node, OF_DYNAMIC)) return; - while (prop) { - struct property *next = prop->next; - kfree(prop->name); - kfree(prop->value); - kfree(prop); - prop = next; + property_list_free(node->properties); + property_list_free(node->deadprops); - if (!prop) { - prop = node->deadprops; - node->deadprops = NULL; - } - } kfree(node->full_name); kfree(node->data); kfree(node); -- cgit From 10b62a2f785ab55857380f0c63d9fa468fd8c676 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 31 Oct 2017 00:33:46 +0900 Subject: .gitignore: move *.dtb and *.dtb.S patterns to the top-level .gitignore Most of DT files are compiled under arch/*/boot/dts/, but we have some other directories, like drivers/of/unittest-data/. We often miss to add gitignore patterns per directory. Since there are no source files that end with .dtb or .dtb.S, we can ignore the patterns globally. Signed-off-by: Masahiro Yamada Signed-off-by: Rob Herring --- drivers/of/unittest-data/.gitignore | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 drivers/of/unittest-data/.gitignore (limited to 'drivers/of') diff --git a/drivers/of/unittest-data/.gitignore b/drivers/of/unittest-data/.gitignore deleted file mode 100644 index 4b3cf8b16de2..000000000000 --- a/drivers/of/unittest-data/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -testcases.dtb -testcases.dtb.S -- cgit