[PATCH v4 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
From: Frank Rowand The Devicetree standard specifies an 8 byte alignment of the FDT. Code in libfdt expects this alignment for an FDT image in memory. kmemdup() returns 4 byte alignment on openrisc. Replace kmemdup() with kmalloc(), align pointer, memcpy() to get proper alignment. The 4 byte alignment exposed a related bug which triggered a crash on openrisc with: commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9") as reported in: https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/ Reported-by: Guenter Roeck Signed-off-by: Frank Rowand --- changes since version 1: - use pointer from kmalloc() for kfree() instead of using pointer that has been modified for FDT alignment changes since version 2: - version 1 was a work in progress version, I failed to commit the following final changes - reorder first two arguments of of_overlay_apply() changes since version 3: - size of memory allocation and size of copy after pointer alignment differ, use separate variables with correct values for each case - edit comment to more clearly describe that ovcs->fdt is the allocated memory region, which may be different than where the aligned pointer points - remove unused parameter from of_overlay_apply() drivers/of/of_private.h | 2 ++ drivers/of/overlay.c| 27 +-- drivers/of/unittest.c | 13 ++--- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index d9e6a324de0a..d717efbd637d 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -8,6 +8,8 @@ * Copyright (C) 1996-2005 Paul Mackerras. */ +#define FDT_ALIGN_SIZE 8 + /** * struct alias_prop - Alias property in 'aliases' node * @link: List node to link the structure in aliases_lookup list diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 50bbe0edf538..ecf967c57900 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -57,7 +57,7 @@ struct fragment { * struct overlay_changeset * @id:changeset identifier * @ovcs_list: list on which we are located - * @fdt: FDT that was unflattened to create @overlay_tree + * @fdt: base of memory allocated to hold aligned FDT that was unflattened to create @overlay_tree * @overlay_tree: expanded device tree that contains the fragment nodes * @count: count of fragment structures * @fragments: fragment nodes in the overlay expanded device tree @@ -719,8 +719,8 @@ static struct device_node *find_target(struct device_node *info_node) /** * init_overlay_changeset() - initialize overlay changeset from overlay tree * @ovcs: Overlay changeset to build - * @fdt: the FDT that was unflattened to create @tree - * @tree: Contains all the overlay fragments and overlay fixup nodes + * @fdt: base of memory allocated to hold aligned FDT that was unflattened to create @tree + * @tree: Contains the overlay fragments and overlay fixup nodes * * Initialize @ovcs. Populate @ovcs->fragments with node information from * the top level of @tree. The relevant top level nodes are the fragment @@ -873,7 +873,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) * internal documentation * * of_overlay_apply() - Create and apply an overlay changeset - * @fdt: the FDT that was unflattened to create @tree + * @fdt: base of memory allocated to hold the aligned FDT * @tree: Expanded overlay device tree * @ovcs_id: Pointer to overlay changeset id * @@ -913,7 +913,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) */ static int of_overlay_apply(const void *fdt, struct device_node *tree, - int *ovcs_id) + int *ovcs_id) { struct overlay_changeset *ovcs; int ret = 0, ret_revert, ret_tmp; @@ -953,7 +953,9 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree, /* * after overlay_notify(), ovcs->overlay_tree related pointers may have * leaked to drivers, so can not kfree() tree, aka ovcs->overlay_tree; -* and can not free fdt, aka ovcs->fdt +* and can not free memory containing aligned fdt. The aligned fdt +* is contained within the memory at ovcs->fdt, possibly at an offset +* from ovcs->fdt. */ ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY); if (ret) { @@ -1014,9 +1016,10 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree, int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size, int *ovcs_id) { - const void *new_fdt; + void *new_fdt; + void *new_fdt_align; int ret; - u32 size; + u32 size, size_alloc; struct device_node *overlay_root; *ovcs_id =
[PATCH v3 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
From: Frank Rowand The Devicetree standard specifies an 8 byte alignment of the FDT. Code in libfdt expects this alignment for an FDT image in memory. kmemdup() returns 4 byte alignment on openrisc. Replace kmemdup() with kmalloc(), align pointer, memcpy() to get proper alignment. The 4 byte alignment exposed a related bug which triggered a crash on openrisc with: commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9") as reported in: https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/ Reported-by: Guenter Roeck Signed-off-by: Frank Rowand --- changes since version 1: - use pointer from kmalloc() for kfree() instead of using pointer that has been modified for FDT alignment changes since version 2: - version 1 was a work in progress version, I failed to commit the following final changes - reorder first two arguments of of_overlay_apply() drivers/of/of_private.h | 2 ++ drivers/of/overlay.c| 28 +--- drivers/of/unittest.c | 12 +--- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index d9e6a324de0a..d717efbd637d 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -8,6 +8,8 @@ * Copyright (C) 1996-2005 Paul Mackerras. */ +#define FDT_ALIGN_SIZE 8 + /** * struct alias_prop - Alias property in 'aliases' node * @link: List node to link the structure in aliases_lookup list diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 50bbe0edf538..cf770452e1e5 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -57,7 +57,7 @@ struct fragment { * struct overlay_changeset * @id:changeset identifier * @ovcs_list: list on which we are located - * @fdt: FDT that was unflattened to create @overlay_tree + * @fdt: base of memory allocated to hold aligned FDT that was unflattened to create @overlay_tree * @overlay_tree: expanded device tree that contains the fragment nodes * @count: count of fragment structures * @fragments: fragment nodes in the overlay expanded device tree @@ -719,8 +719,8 @@ static struct device_node *find_target(struct device_node *info_node) /** * init_overlay_changeset() - initialize overlay changeset from overlay tree * @ovcs: Overlay changeset to build - * @fdt: the FDT that was unflattened to create @tree - * @tree: Contains all the overlay fragments and overlay fixup nodes + * @fdt: base of memory allocated to hold aligned FDT that was unflattened to create @tree + * @tree: Contains the overlay fragments and overlay fixup nodes * * Initialize @ovcs. Populate @ovcs->fragments with node information from * the top level of @tree. The relevant top level nodes are the fragment @@ -873,7 +873,8 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) * internal documentation * * of_overlay_apply() - Create and apply an overlay changeset - * @fdt: the FDT that was unflattened to create @tree + * @fdt: base of memory allocated to hold *@fdt_align + * @fdt_align: the FDT that was unflattened to create @tree, aligned * @tree: Expanded overlay device tree * @ovcs_id: Pointer to overlay changeset id * @@ -912,8 +913,8 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) * id is returned to *ovcs_id. */ -static int of_overlay_apply(const void *fdt, struct device_node *tree, - int *ovcs_id) +static int of_overlay_apply(const void *fdt, const void *fdt_align, + struct device_node *tree, int *ovcs_id) { struct overlay_changeset *ovcs; int ret = 0, ret_revert, ret_tmp; @@ -953,7 +954,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree, /* * after overlay_notify(), ovcs->overlay_tree related pointers may have * leaked to drivers, so can not kfree() tree, aka ovcs->overlay_tree; -* and can not free fdt, aka ovcs->fdt +* and can not free memory containing aligned fdt, aka ovcs->fdt */ ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY); if (ret) { @@ -1014,7 +1015,8 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree, int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size, int *ovcs_id) { - const void *new_fdt; + void *new_fdt; + void *new_fdt_align; int ret; u32 size; struct device_node *overlay_root; @@ -1036,18 +1038,22 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size, * Must create permanent copy of FDT because of_fdt_unflatten_tree() * will create pointers to the passed in FDT in the unflattened tree. */ - new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL); + size += FDT_ALIGN_SIZ
[PATCH v2 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
From: Frank Rowand The Devicetree standard specifies an 8 byte alignment of the FDT. Code in libfdt expects this alignment for an FDT image in memory. kmemdup() returns 4 byte alignment on openrisc. Replace kmemdup() with kmalloc(), align pointer, memcpy() to get proper alignment. The 4 byte alignment exposed a related bug which triggered a crash on openrisc with: commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9") as reported in: https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/ Reported-by: Guenter Roeck Signed-off-by: Frank Rowand --- Please review carefully, I am not yet fully awake... changes since version 1: - use pointer from kmalloc() for kfree() instead of using pointer that has been modified for FDT alignment drivers/of/of_private.h | 2 ++ drivers/of/overlay.c| 28 +--- drivers/of/unittest.c | 12 +--- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index d9e6a324de0a..d717efbd637d 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -8,6 +8,8 @@ * Copyright (C) 1996-2005 Paul Mackerras. */ +#define FDT_ALIGN_SIZE 8 + /** * struct alias_prop - Alias property in 'aliases' node * @link: List node to link the structure in aliases_lookup list diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 50bbe0edf538..e0397d70d531 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -57,7 +57,7 @@ struct fragment { * struct overlay_changeset * @id:changeset identifier * @ovcs_list: list on which we are located - * @fdt: FDT that was unflattened to create @overlay_tree + * @fdt: base of memory allocated to hold aligned FDT that was unflattened to create @overlay_tree * @overlay_tree: expanded device tree that contains the fragment nodes * @count: count of fragment structures * @fragments: fragment nodes in the overlay expanded device tree @@ -719,8 +719,8 @@ static struct device_node *find_target(struct device_node *info_node) /** * init_overlay_changeset() - initialize overlay changeset from overlay tree * @ovcs: Overlay changeset to build - * @fdt: the FDT that was unflattened to create @tree - * @tree: Contains all the overlay fragments and overlay fixup nodes + * @fdt: base of memory allocated to hold aligned FDT that was unflattened to create @tree + * @tree: Contains the overlay fragments and overlay fixup nodes * * Initialize @ovcs. Populate @ovcs->fragments with node information from * the top level of @tree. The relevant top level nodes are the fragment @@ -873,7 +873,8 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) * internal documentation * * of_overlay_apply() - Create and apply an overlay changeset - * @fdt: the FDT that was unflattened to create @tree + * @fdt_align: the FDT that was unflattened to create @tree, aligned + * @fdt: base of memory allocated to hold *@fdt_align * @tree: Expanded overlay device tree * @ovcs_id: Pointer to overlay changeset id * @@ -912,8 +913,8 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) * id is returned to *ovcs_id. */ -static int of_overlay_apply(const void *fdt, struct device_node *tree, - int *ovcs_id) +static int of_overlay_apply(const void *fdt_align, const void *fdt, + struct device_node *tree, int *ovcs_id) { struct overlay_changeset *ovcs; int ret = 0, ret_revert, ret_tmp; @@ -953,7 +954,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree, /* * after overlay_notify(), ovcs->overlay_tree related pointers may have * leaked to drivers, so can not kfree() tree, aka ovcs->overlay_tree; -* and can not free fdt, aka ovcs->fdt +* and can not free memory containing aligned fdt, aka ovcs->fdt */ ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY); if (ret) { @@ -1014,7 +1015,8 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree, int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size, int *ovcs_id) { - const void *new_fdt; + void *new_fdt; + void *new_fdt_align; int ret; u32 size; struct device_node *overlay_root; @@ -1036,18 +1038,22 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size, * Must create permanent copy of FDT because of_fdt_unflatten_tree() * will create pointers to the passed in FDT in the unflattened tree. */ - new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL); + size += FDT_ALIGN_SIZE; + new_fdt = kmalloc(size, GFP_KERNEL); if (!new_fdt) return -ENOMEM; - of_fdt_unf
[PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
From: Frank Rowand The Devicetree standard specifies an 8 byte alignment of the FDT. Code in libfdt expects this alignment for an FDT image in memory. kmemdup() returns 4 byte alignment on openrisc. Replace kmemdup() with kmalloc(), align pointer, memcpy() to get proper alignment. The 4 byte alignment exposed a related bug which triggered a crash on openrisc with: commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9") as reported in: https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/ Reported-by: Guenter Roeck Signed-off-by: Frank Rowand --- drivers/of/of_private.h | 2 ++ drivers/of/overlay.c| 8 ++-- drivers/of/unittest.c | 9 +++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index d9e6a324de0a..d717efbd637d 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -8,6 +8,8 @@ * Copyright (C) 1996-2005 Paul Mackerras. */ +#define FDT_ALIGN_SIZE 8 + /** * struct alias_prop - Alias property in 'aliases' node * @link: List node to link the structure in aliases_lookup list diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 50bbe0edf538..8b40711ed202 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -1014,7 +1014,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree, int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size, int *ovcs_id) { - const void *new_fdt; + void *new_fdt; int ret; u32 size; struct device_node *overlay_root; @@ -1036,10 +1036,14 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size, * Must create permanent copy of FDT because of_fdt_unflatten_tree() * will create pointers to the passed in FDT in the unflattened tree. */ - new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL); + size += FDT_ALIGN_SIZE; + new_fdt = kmalloc(size, GFP_KERNEL); if (!new_fdt) return -ENOMEM; + new_fdt = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE); + memcpy(new_fdt, overlay_fdt, size); + of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root); if (!overlay_root) { pr_err("unable to unflatten overlay_fdt\n"); diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index eb100627c186..edd6ce807691 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -1415,7 +1416,7 @@ static int __init unittest_data_add(void) */ extern uint8_t __dtb_testcases_begin[]; extern uint8_t __dtb_testcases_end[]; - const int size = __dtb_testcases_end - __dtb_testcases_begin; + u32 size = __dtb_testcases_end - __dtb_testcases_begin; int rc; if (!size) { @@ -1425,10 +1426,14 @@ static int __init unittest_data_add(void) } /* creating copy */ - unittest_data = kmemdup(__dtb_testcases_begin, size, GFP_KERNEL); + size += FDT_ALIGN_SIZE; + unittest_data = kmalloc(size, GFP_KERNEL); if (!unittest_data) return -ENOMEM; + unittest_data = PTR_ALIGN(unittest_data, FDT_ALIGN_SIZE); + memcpy(unittest_data, __dtb_testcases_begin, size); + of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node); if (!unittest_data_node) { pr_warn("%s: No tree to attach; not running tests\n", __func__); -- Frank Rowand
[PATCH 1/1] of: properly check for error returned by fdt_get_name()
From: Frank Rowand fdt_get_name() returns error values via a parameter pointer instead of in function return. Fix check for this error value in populate_node() and callers of populate_node(). Chasing up the caller tree showed callers of various functions failing to initialize the value of pointer parameters that can return error values. Initialize those values to NULL. The bug was introduced by commit e6a6928c3ea1 ("of/fdt: Convert FDT functions to use libfdt") but this patch can not be backported directly to that commit because the relevant code has further been restructured by commit dfbd4c6eff35 ("drivers/of: Split unflatten_dt_node()") The bug became visible by triggering a crash on openrisc with: commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9") as reported in: https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/ Fixes: commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9") Reported-by: Guenter Roeck Signed-off-by: Frank Rowand --- This patch papers over the unaligned pointer passed to of_fdt_unflatten_tree() bug that Guenter reported in https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/ I will create a separate patch to fix that problem. drivers/of/fdt.c | 36 +++- drivers/of/overlay.c | 2 +- drivers/of/unittest.c | 15 ++- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index dcc1dd96911a..adb26aff481d 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -205,7 +205,7 @@ static void populate_properties(const void *blob, *pprev = NULL; } -static bool populate_node(const void *blob, +static int populate_node(const void *blob, int offset, void **mem, struct device_node *dad, @@ -214,24 +214,24 @@ static bool populate_node(const void *blob, { struct device_node *np; const char *pathp; - unsigned int l, allocl; + int len; - pathp = fdt_get_name(blob, offset, &l); + pathp = fdt_get_name(blob, offset, &len); if (!pathp) { *pnp = NULL; - return false; + return len; } - allocl = ++l; + len++; - np = unflatten_dt_alloc(mem, sizeof(struct device_node) + allocl, + np = unflatten_dt_alloc(mem, sizeof(struct device_node) + len, __alignof__(struct device_node)); if (!dryrun) { char *fn; of_node_init(np); np->full_name = fn = ((char *)np) + sizeof(*np); - memcpy(fn, pathp, l); + memcpy(fn, pathp, len); if (dad != NULL) { np->parent = dad; @@ -295,6 +295,7 @@ static int unflatten_dt_nodes(const void *blob, struct device_node *nps[FDT_MAX_DEPTH]; void *base = mem; bool dryrun = !base; + int ret; if (nodepp) *nodepp = NULL; @@ -322,9 +323,10 @@ static int unflatten_dt_nodes(const void *blob, !of_fdt_device_is_available(blob, offset)) continue; - if (!populate_node(blob, offset, &mem, nps[depth], - &nps[depth+1], dryrun)) - return mem - base; + ret = populate_node(blob, offset, &mem, nps[depth], + &nps[depth+1], dryrun); + if (ret < 0) + return ret; if (!dryrun && nodepp && !*nodepp) *nodepp = nps[depth+1]; @@ -372,6 +374,10 @@ void *__unflatten_device_tree(const void *blob, { int size; void *mem; + int ret; + + if (mynodes) + *mynodes = NULL; pr_debug(" -> unflatten_device_tree()\n"); @@ -392,7 +398,7 @@ void *__unflatten_device_tree(const void *blob, /* First pass, scan for size */ size = unflatten_dt_nodes(blob, NULL, dad, NULL); - if (size < 0) + if (size <= 0) return NULL; size = ALIGN(size, 4); @@ -410,12 +416,16 @@ void *__unflatten_device_tree(const void *blob, pr_debug(" unflattening %p...\n", mem); /* Second pass, do actual unflattening */ - unflatten_dt_nodes(blob, mem, dad, mynodes); + ret = unflatten_dt_nodes(blob, mem, dad, mynodes); + if (be32_to_cpup(mem + size) != 0xdeadbeef) pr_warn("End of tree marker overwritten: %08x\n", be32_to_cpup(mem + size)); - if (detached && mynodes) { + if (ret <= 0) + return NULL; + + if (detached && mynodes && *mynodes) { of_node_set_flag(*mynodes, OF_DETACHED); pr_debug("unflattened tree is detached\n"); } dif
[PATCH 1/1] of: unittest: rename overlay source files from .dts to .dtso
From: Frank Rowand Add Makefile rule to build .dtbo.o assembly file from overlay .dtso source file. Rename unittest .dts overlay source files to use .dtso suffix. Update Makefile to build .dtbo.o objects instead of .dtb.o from unittest overlay source files. Modify unitest.c to use .dtbo.o based symbols instead of .dtb.o Modify Makefile.lib %.dtbo rule to depend upon %.dtso instead of %.dts Documentation/devicetree/of_unittest.rst was already out of date. This commit would make it even more out of date. Delete the document instead of continuing the maintenance burden of keeping the document in sync with the source. Signed-off-by: Frank Rowand --- This patch applies on top of Viresh Kumar's patches 1, 2, 4, and 5 that Rob has already accepted at: https://lore.kernel.org/r/20210315171357.ga1063...@robh.at.kernel.org Viresh Kumar's series is "[PATCH V11 0/5] dt: Add fdtoverlay rule and statically build unittest" at: https://lore.kernel.org/linux-devicetree/cover.1615354376.git.viresh.ku...@linaro.org/ There are checkpatch warnings. I have reviewed them and feel they can be ignored. Documentation/devicetree/index.rst| 1 - Documentation/devicetree/of_unittest.rst | 205 -- drivers/of/unittest-data/Makefile | 76 --- .../{overlay.dts => overlay.dtso} | 0 .../{overlay_0.dts => overlay_0.dtso} | 0 .../{overlay_1.dts => overlay_1.dtso} | 0 .../{overlay_10.dts => overlay_10.dtso} | 0 .../{overlay_11.dts => overlay_11.dtso} | 0 .../{overlay_12.dts => overlay_12.dtso} | 0 .../{overlay_13.dts => overlay_13.dtso} | 0 .../{overlay_15.dts => overlay_15.dtso} | 0 .../{overlay_2.dts => overlay_2.dtso} | 0 .../{overlay_3.dts => overlay_3.dtso} | 0 .../{overlay_4.dts => overlay_4.dtso} | 0 .../{overlay_5.dts => overlay_5.dtso} | 0 .../{overlay_6.dts => overlay_6.dtso} | 0 .../{overlay_7.dts => overlay_7.dtso} | 0 .../{overlay_8.dts => overlay_8.dtso} | 0 .../{overlay_9.dts => overlay_9.dtso} | 0 ...node.dts => overlay_bad_add_dup_node.dtso} | 0 ...prop.dts => overlay_bad_add_dup_prop.dtso} | 0 ...d_phandle.dts => overlay_bad_phandle.dtso} | 0 ...bad_symbol.dts => overlay_bad_symbol.dtso} | 0 .../{overlay_base.dts => overlay_base.dtso} | 0 ...erlay_gpio_01.dts => overlay_gpio_01.dtso} | 0 ...lay_gpio_02a.dts => overlay_gpio_02a.dtso} | 0 ...lay_gpio_02b.dts => overlay_gpio_02b.dtso} | 0 ...erlay_gpio_03.dts => overlay_gpio_03.dtso} | 0 ...lay_gpio_04a.dts => overlay_gpio_04a.dtso} | 0 ...lay_gpio_04b.dts => overlay_gpio_04b.dtso} | 0 .../{testcases.dts => testcases.dtso} | 0 drivers/of/unittest.c | 48 ++-- scripts/Makefile.lib | 2 +- 33 files changed, 73 insertions(+), 259 deletions(-) delete mode 100644 Documentation/devicetree/of_unittest.rst rename drivers/of/unittest-data/{overlay.dts => overlay.dtso} (100%) rename drivers/of/unittest-data/{overlay_0.dts => overlay_0.dtso} (100%) rename drivers/of/unittest-data/{overlay_1.dts => overlay_1.dtso} (100%) rename drivers/of/unittest-data/{overlay_10.dts => overlay_10.dtso} (100%) rename drivers/of/unittest-data/{overlay_11.dts => overlay_11.dtso} (100%) rename drivers/of/unittest-data/{overlay_12.dts => overlay_12.dtso} (100%) rename drivers/of/unittest-data/{overlay_13.dts => overlay_13.dtso} (100%) rename drivers/of/unittest-data/{overlay_15.dts => overlay_15.dtso} (100%) rename drivers/of/unittest-data/{overlay_2.dts => overlay_2.dtso} (100%) rename drivers/of/unittest-data/{overlay_3.dts => overlay_3.dtso} (100%) rename drivers/of/unittest-data/{overlay_4.dts => overlay_4.dtso} (100%) rename drivers/of/unittest-data/{overlay_5.dts => overlay_5.dtso} (100%) rename drivers/of/unittest-data/{overlay_6.dts => overlay_6.dtso} (100%) rename drivers/of/unittest-data/{overlay_7.dts => overlay_7.dtso} (100%) rename drivers/of/unittest-data/{overlay_8.dts => overlay_8.dtso} (100%) rename drivers/of/unittest-data/{overlay_9.dts => overlay_9.dtso} (100%) rename drivers/of/unittest-data/{overlay_bad_add_dup_node.dts => overlay_bad_add_dup_node.dtso} (100%) rename drivers/of/unittest-data/{overlay_bad_add_dup_prop.dts => overlay_bad_add_dup_prop.dtso} (100%) rename drivers/of/unittest-data/{overlay_bad_phandle.dts => overlay_bad_phandle.dtso} (100%) rename drivers/of/unittest-data/{overlay_bad_symbol.dts => overlay_bad_symbol.dtso} (100%) rename drivers/of/unittest-data/{overlay_base.dts => overlay_base.dtso} (100%) rename drivers/of/unittest-data/{overlay_gpio_01.dts => overlay_gpio_01.dtso} (100%) rename drivers/of/unittest-data/{overlay_gpio_02a.dts => overlay_gpio_02a.dtso} (100%) rename drivers/of/unittest-data/{overlay_gpio_02b.dts => overlay_gpio_02b.dtso} (100%) rename drivers/of/unittest-data/{overlay_gpio_03.dts
re: [PATCH] of: unittest: Statically apply overlays using fdtoverlay
From: Frank Rowand These changes apply on top of the patches in: [PATCH] of: unittest: Statically apply overlays using fdtoverlay Message-Id: <1e42183ccafa1afba33b3e79a4e3efd3329fd133.1610095159.git.viresh.ku...@linaro.org> There are still some issues to be cleaned up, so not ready for acceptance. I have not used the construct "always-$(CONFIG_OF_OVERLAY)" before, and have not looked into the proper usage of it. I tested this using a hand build libfdt and fdtoverlay from the dtc-compiler upstream project. For my testing I added LD_LIBRARY_PATH to the body of "cmd_fdtoverlay" to reference my hand built libfdt. The kernel build system will have to instead use a libfdt that is built in the kernel tree. I have not run this through checkpatch, or my checks for build warnings. I have not run unittests on my target with these patches applied. --- drivers/of/unittest-data/Makefile | 67 ++- 1 file changed, 48 insertions(+), 19 deletions(-) diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile index f17bce85f65f..28614a123d1e 100644 --- a/drivers/of/unittest-data/Makefile +++ b/drivers/of/unittest-data/Makefile @@ -39,25 +39,54 @@ DTC_FLAGS_testcases += -@ # suppress warnings about intentional errors DTC_FLAGS_testcases += -Wno-interrupts_property -# Apply overlays statically with fdtoverlay -intermediate-overlay := overlay.dtb -master := overlay_0.dtb overlay_1.dtb overlay_2.dtb \ - overlay_3.dtb overlay_4.dtb overlay_5.dtb \ - overlay_6.dtb overlay_7.dtb overlay_8.dtb \ - overlay_9.dtb overlay_10.dtb overlay_11.dtb \ - overlay_12.dtb overlay_13.dtb overlay_15.dtb \ - overlay_gpio_01.dtb overlay_gpio_02a.dtb \ - overlay_gpio_02b.dtb overlay_gpio_03.dtb \ - overlay_gpio_04a.dtb overlay_gpio_04b.dtb \ - intermediate-overlay.dtb - -quiet_cmd_fdtoverlay = fdtoverlay $@ - cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $^ - -$(obj)/intermediate-overlay.dtb: $(obj)/overlay_base.dtb $(addprefix $(obj)/,$(intermediate-overlay)) - $(call if_changed,fdtoverlay) +# Apply overlays statically with fdtoverlay. This is a build time test that +# the overlays can be applied successfully by fdtoverlay. This does not +# guarantee that the overlays can be applied successfully at run time by +# unittest, but it provides a bit of build time test coverage for those +# who do not execute unittest. +# +# The overlays are applied on top of testcases.dtb to create static_test.dtb +# If fdtoverlay detects an error than the kernel build will fail. +# static_test.dtb is not consumed by unittest. +# +# Some unittest overlays deliberately contain errors that unittest checks for. +# These overlays will cause fdtoverlay to fail, and are thus not included +# in the static test: +# overlay.dtb \ +# overlay_bad_add_dup_node.dtb \ +# overlay_bad_add_dup_prop.dtb \ +# overlay_bad_phandle.dtb \ +# overlay_bad_symbol.dtb \ + +apply_static_overlay := overlay_base.dtb \ + overlay_0.dtb \ + overlay_1.dtb \ + overlay_2.dtb \ + overlay_3.dtb \ + overlay_4.dtb \ + overlay_5.dtb \ + overlay_6.dtb \ + overlay_7.dtb \ + overlay_8.dtb \ + overlay_9.dtb \ + overlay_10.dtb \ + overlay_11.dtb \ + overlay_12.dtb \ + overlay_13.dtb \ + overlay_15.dtb \ + overlay_gpio_01.dtb \ + overlay_gpio_02a.dtb \ + overlay_gpio_02b.dtb \ + overlay_gpio_03.dtb \ + overlay_gpio_04a.dtb \ + overlay_gpio_04b.dtb \ + +quiet_cmd_fdtoverlay = FDTOVERLAY $@ + +## This is not correct, need to use libfdt from the kernel tree: +cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $^ -$(obj)/master.dtb: $(obj)/testcases.dtb $(addprefix $(obj)/,$(master)) +$(obj)/static_test.dtb: $(obj)/testcases.dtb $(addprefix $(obj)/,$(apply_static_overlay)) $(call if_changed,fdtoverlay) -always-$(CONFIG_OF_OVERLAY) += intermediate-overlay.dtb master.dtb +always-$(CONFIG_OF_OVERLAY) += static_test.dtb -- Frank Rowand
[PATCH v2] tracing: initialize variable in create_dyn_event()
From: Frank Rowand Fix compile warning in create_dyn_event(): 'ret' may be used uninitialized in this function [-Wuninitialized]. Fixes: 5448d44c3855 ("tracing: Add unified dynamic event framework") Signed-off-by: Frank Rowand --- changes since v1: - initialize to -ENODEV instead of 0, as suggested by Steve kernel/trace/trace_dynevent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c index dd1f43588d70..fa100ed3b4de 100644 --- a/kernel/trace/trace_dynevent.c +++ b/kernel/trace/trace_dynevent.c @@ -74,7 +74,7 @@ int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type) static int create_dyn_event(int argc, char **argv) { struct dyn_event_operations *ops; - int ret; + int ret = -ENODEV; if (argv[0][0] == '-' || argv[0][0] == '!') return dyn_event_release(argc, argv, NULL); -- Frank Rowand
[PATCH] docs: kernel-doc: typo "if ... if" -> "if ... is"
From: Frank Rowand "If no *function* if specified" should instead be "If no *function* is specified". Reported-by: Matthew Wilcox Signed-off-by: Frank Rowand --- Documentation/doc-guide/kernel-doc.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst index 6513c16b7e4f..f96059767c8c 100644 --- a/Documentation/doc-guide/kernel-doc.rst +++ b/Documentation/doc-guide/kernel-doc.rst @@ -490,7 +490,7 @@ doc: *title* functions: *[ function ...]* Include documentation for each *function* in *source*. - If no *function* if specified, the documentation for all functions + If no *function* is specified, the documentation for all functions and types in the *source* will be included. Examples:: -- Frank Rowand
[PATCH] docs: kernel-doc: typo "documentaion"
From: Frank Rowand Fix a typo in kernel-doc.rst Signed-off-by: Frank Rowand --- Documentation/doc-guide/kernel-doc.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst index 450ac634e92e..6513c16b7e4f 100644 --- a/Documentation/doc-guide/kernel-doc.rst +++ b/Documentation/doc-guide/kernel-doc.rst @@ -490,7 +490,7 @@ doc: *title* functions: *[ function ...]* Include documentation for each *function* in *source*. - If no *function* if specified, the documentaion for all functions + If no *function* if specified, the documentation for all functions and types in the *source* will be included. Examples:: -- Frank Rowand
[PATCH] docs: kernel-doc: update commands to generate man page
From: Frank Rowand (1) The command to generate man pages is truncated in the pdf version of the document. Reformat the command into multiple lines to prevent the truncation. (2) Older versions of git do not support all variants of pathspec syntax. Provide commands to generate man pages for various alternate syntax. Signed-off-by: Frank Rowand --- Documentation/doc-guide/kernel-doc.rst | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst index 51be62aa4385..450ac634e92e 100644 --- a/Documentation/doc-guide/kernel-doc.rst +++ b/Documentation/doc-guide/kernel-doc.rst @@ -517,4 +517,17 @@ How to use kernel-doc to generate man pages If you just want to use kernel-doc to generate man pages you can do this from the kernel git tree:: - $ scripts/kernel-doc -man $(git grep -l '/\*\*' -- :^Documentation :^tools) | scripts/split-man.pl /tmp/man + $ scripts/kernel-doc -man \ +$(git grep -l '/\*\*' -- :^Documentation :^tools) \ +| scripts/split-man.pl /tmp/man + +Some older versions of git do not support some of the variants of syntax for +path exclusion. One of the following commands may work for those versions:: + + $ scripts/kernel-doc -man \ +$(git grep -l '/\*\*' -- . ':!Documentation' ':!tools') \ +| scripts/split-man.pl /tmp/man + + $ scripts/kernel-doc -man \ +$(git grep -l '/\*\*' -- . ":(exclude)Documentation" ":(exclude)tools") \ +| scripts/split-man.pl /tmp/man -- Frank Rowand
[PATCH] tracing: initialize variable in create_dyn_event()
From: Frank Rowand Fix compile warning in create_dyn_event(): 'ret' may be used uninitialized in this function [-Wuninitialized]. Fixes: 5448d44c3855 ("tracing: Add unified dynamic event framework") Signed-off-by: Frank Rowand --- Compile and boot tested only. Please verify the initialization value of zero is correct. There is also a sparse warning you might want to check into (when ARCH=arm, so 32 bit): include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000 becomes 0) kernel/trace/trace_dynevent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c index dd1f43588d70..4f633476c307 100644 --- a/kernel/trace/trace_dynevent.c +++ b/kernel/trace/trace_dynevent.c @@ -74,7 +74,7 @@ int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type) static int create_dyn_event(int argc, char **argv) { struct dyn_event_operations *ops; - int ret; + int ret = 0; if (argv[0][0] == '-' || argv[0][0] == '!') return dyn_event_release(argc, argv, NULL); -- Frank Rowand
[PATCH 1/2] of: unittest: remove report of expected error
From: Frank Rowand update_node_properties() reports an error when the test data contains a node (such as "/aliases") that already exists in the base devicetree. The error is caused by of_fdt_unflatten_tree() autogenerating the "name" property, thus both the existing node and the new node will have a property with the same name. Suppress reporting the known error. Signed-off-by: Frank Rowand --- drivers/of/unittest.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 84427384654d..3249fe259d30 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -1116,9 +1116,12 @@ static void update_node_properties(struct device_node *np, for (prop = np->properties; prop != NULL; prop = save_next) { save_next = prop->next; ret = of_add_property(dup, prop); - if (ret) + if (ret) { + if (ret == -EEXIST && !strcmp(prop->name, "name")) + continue; pr_err("unittest internal error: unable to add testdata property %pOF/%s", np, prop->name); + } } } -- Frank Rowand
[PATCH 2/2] of: unittest: add caution to function header comment
From: Frank Rowand Name of function attach_node_and_children() is misleading because if the node already exists in the livetree then only the node's properties are attached. This works for the existing test data, but add comment warning of this misleading name. Signed-off-by: Frank Rowand --- drivers/of/unittest.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 3249fe259d30..872956500c27 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -1127,7 +1127,11 @@ static void update_node_properties(struct device_node *np, /** * attach_node_and_children - attaches nodes - * and its children to live tree + * and its children to live tree. + * CAUTION: misleading function name - if node @np already exists in + * the live tree then children of @np are *not* attached to the live + * tree. This works for the current test devicetree nodes because such + * nodes do not have child nodes. * * @np:Node to attach to live tree */ -- Frank Rowand
[PATCH 0/2] of: unittest: minor cleanups
From: Frank Rowand 1) Suppress expected error message when adding unittest data to livetree. 2) While writing patch 1, noted a misleading function name. Add comment to the function header warning about possible unexpected results. Frank Rowand (2): of: unittest: remove report of expected error of: unittest: add caution to function header comment drivers/of/unittest.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) -- Frank Rowand
[PATCH RESEND] of: add dtc annotations functionality to dtx_diff
From: Frank Rowand Add -T and --annotations command line arguments to dtx_diff. These arguments will be passed through to dtc. dtc will then add source location annotations to its output. Signed-off-by: Frank Rowand --- If dtx_diff is provided with a single devicetree source file then that source file will be compiled, with all appropriate includes and pre-processing, and the result will be output as source. This provides a convenient mechanism to see what the devicetree looks like after all processing and replacements occur. The --annotations argument will cause dtc to report information about where the source was located for each line of the output. scripts/dtc/dtx_diff | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/scripts/dtc/dtx_diff b/scripts/dtc/dtx_diff index 8c4fbad2055e..0d8572008729 100755 --- a/scripts/dtc/dtx_diff +++ b/scripts/dtc/dtx_diff @@ -21,6 +21,7 @@ Usage: diff DTx_1 and DTx_2 + --annotatesynonym for -T -f print full dts in diff (--unified=9) -h synonym for --help -helpsynonym for --help @@ -28,6 +29,7 @@ Usage: -s SRCTREE linux kernel source tree is at path SRCTREE (default is current directory) -S linux kernel source tree is at root of current git repo + -T Annotate output .dts with input source file and line (-T -T for more details) -u unsorted, do not sort DTx @@ -174,6 +176,7 @@ compile_to_dts() { # - start of script +annotate="" cmd_diff=0 diff_flags="-u" dtx_file_1="" @@ -208,6 +211,14 @@ while [ $# -gt 0 ] ; do shift ;; + -T | --annotate ) + if [ "${annotate}" = "" ] ; then + annotate="-T" + elif [ "${annotate}" = "-T" ] ; then + annotate="-T -T" + fi + shift + ;; -u ) dtc_sort="" shift @@ -327,7 +338,7 @@ cpp_flags="\ DTC="\ ${DTC} \ -i ${srctree}/scripts/dtc/include-prefixes \ - -O dts -qq -f ${dtc_sort} -o -" + -O dts -qq -f ${dtc_sort} ${annotate} -o -" # - do the diff or decompile -- Frank Rowand
[PATCH v3 2/2] of: __of_detach_node() - remove node from phandle cache
From: Frank Rowand Non-overlay dynamic devicetree node removal may leave the node in the phandle cache. Subsequent calls to of_find_node_by_phandle() will incorrectly find the stale entry. Remove the node from the cache. Add paranoia checks in of_find_node_by_phandle() as a second level of defense (do not return cached node if detached, do not add node to cache if detached). Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Reported-by: Michael Bringmann Signed-off-by: Frank Rowand --- do not "cc: stable", unless the following commits are also in stable: commit e54192b48da7 ("of: fix phandle cache creation for DTs with no phandles") commit b9952b5218ad ("of: overlay: update phandle cache on overlay apply and remove") commit 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Changes since v2: - add temporary variable np in __of_free_phandle_cache_entry() to improve readability - explain reason for WARN_ON() in comment - add Fixes tag in patch comment drivers/of/base.c | 31 ++- drivers/of/dynamic.c| 3 +++ drivers/of/of_private.h | 4 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 6c33d63361b8..6d20b6dcf034 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -162,6 +162,28 @@ int of_free_phandle_cache(void) late_initcall_sync(of_free_phandle_cache); #endif +/* + * Caller must hold devtree_lock. + */ +void __of_free_phandle_cache_entry(phandle handle) +{ + phandle masked_handle; + struct device_node *np; + + if (!handle) + return; + + masked_handle = handle & phandle_cache_mask; + + if (phandle_cache) { + np = phandle_cache[masked_handle]; + if (np && handle == np->phandle) { + of_node_put(np); + phandle_cache[masked_handle] = NULL; + } + } +} + void of_populate_phandle_cache(void) { unsigned long flags; @@ -1209,11 +1231,18 @@ struct device_node *of_find_node_by_phandle(phandle handle) if (phandle_cache[masked_handle] && handle == phandle_cache[masked_handle]->phandle) np = phandle_cache[masked_handle]; + if (np && of_node_check_flag(np, OF_DETACHED)) { + WARN_ON(1); /* did not uncache np on node removal */ + of_node_put(np); + phandle_cache[masked_handle] = NULL; + np = NULL; + } } if (!np) { for_each_of_allnodes(np) - if (np->phandle == handle) { + if (np->phandle == handle && + !of_node_check_flag(np, OF_DETACHED)) { if (phandle_cache) { /* will put when removed from cache */ of_node_get(np); diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index f4f8ed9b5454..ecea92f68c87 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -268,6 +268,9 @@ void __of_detach_node(struct device_node *np) } of_node_set_flag(np, OF_DETACHED); + + /* race with of_find_node_by_phandle() prevented by devtree_lock */ + __of_free_phandle_cache_entry(np->phandle); } /** diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 5d1567025358..24786818e32e 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -84,6 +84,10 @@ static inline void __of_detach_node_sysfs(struct device_node *np) {} int of_resolve_phandles(struct device_node *tree); #endif +#if defined(CONFIG_OF_DYNAMIC) +void __of_free_phandle_cache_entry(phandle handle); +#endif + #if defined(CONFIG_OF_OVERLAY) void of_overlay_mutex_lock(void); void of_overlay_mutex_unlock(void); -- Frank Rowand
[PATCH v3 1/2] of: of_node_get()/of_node_put() nodes held in phandle cache
From: Frank Rowand The phandle cache contains struct device_node pointers. The refcount of the pointers was not incremented while in the cache, allowing use after free error after kfree() of the node. Add the proper increment and decrement of the use count. Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Signed-off-by: Frank Rowand --- do not "cc: stable", unless the following commits are also in stable: commit e54192b48da7 ("of: fix phandle cache creation for DTs with no phandles") commit b9952b5218ad ("of: overlay: update phandle cache on overlay apply and remove") commit 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") drivers/of/base.c | 70 --- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 09692c9b32a7..6c33d63361b8 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -116,9 +116,6 @@ int __weak of_node_to_nid(struct device_node *np) } #endif -static struct device_node **phandle_cache; -static u32 phandle_cache_mask; - /* * Assumptions behind phandle_cache implementation: * - phandle property values are in a contiguous range of 1..n @@ -127,6 +124,44 @@ int __weak of_node_to_nid(struct device_node *np) * - the phandle lookup overhead reduction provided by the cache * will likely be less */ + +static struct device_node **phandle_cache; +static u32 phandle_cache_mask; + +/* + * Caller must hold devtree_lock. + */ +static void __of_free_phandle_cache(void) +{ + u32 cache_entries = phandle_cache_mask + 1; + u32 k; + + if (!phandle_cache) + return; + + for (k = 0; k < cache_entries; k++) + of_node_put(phandle_cache[k]); + + kfree(phandle_cache); + phandle_cache = NULL; +} + +int of_free_phandle_cache(void) +{ + unsigned long flags; + + raw_spin_lock_irqsave(&devtree_lock, flags); + + __of_free_phandle_cache(); + + raw_spin_unlock_irqrestore(&devtree_lock, flags); + + return 0; +} +#if !defined(CONFIG_MODULES) +late_initcall_sync(of_free_phandle_cache); +#endif + void of_populate_phandle_cache(void) { unsigned long flags; @@ -136,8 +171,7 @@ void of_populate_phandle_cache(void) raw_spin_lock_irqsave(&devtree_lock, flags); - kfree(phandle_cache); - phandle_cache = NULL; + __of_free_phandle_cache(); for_each_of_allnodes(np) if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) @@ -155,30 +189,15 @@ void of_populate_phandle_cache(void) goto out; for_each_of_allnodes(np) - if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) { + of_node_get(np); phandle_cache[np->phandle & phandle_cache_mask] = np; + } out: raw_spin_unlock_irqrestore(&devtree_lock, flags); } -int of_free_phandle_cache(void) -{ - unsigned long flags; - - raw_spin_lock_irqsave(&devtree_lock, flags); - - kfree(phandle_cache); - phandle_cache = NULL; - - raw_spin_unlock_irqrestore(&devtree_lock, flags); - - return 0; -} -#if !defined(CONFIG_MODULES) -late_initcall_sync(of_free_phandle_cache); -#endif - void __init of_core_init(void) { struct device_node *np; @@ -1195,8 +1214,11 @@ struct device_node *of_find_node_by_phandle(phandle handle) if (!np) { for_each_of_allnodes(np) if (np->phandle == handle) { - if (phandle_cache) + if (phandle_cache) { + /* will put when removed from cache */ + of_node_get(np); phandle_cache[masked_handle] = np; + } break; } } -- Frank Rowand
[PATCH v3 0/2] of: phandle_cache, fix refcounts, remove stale entry
From: Frank Rowand Non-overlay dynamic devicetree node removal may leave the node in the phandle cache. Subsequent calls to of_find_node_by_phandle() will incorrectly find the stale entry. This bug exposed the foloowing phandle cache refcount bug. The refcount of phandle_cache entries is not incremented while in the cache, allowing use after free error after kfree() of the cached entry. Changes since v2: - patch 2/2: add temporary variable np in __of_free_phandle_cache_entry() to improve readability - patch 2/2: explain reason for WARN_ON() in comment - patch 2/2: add Fixes tag in patch comment Changes since v1: - make __of_free_phandle_cache() static - add WARN_ON(1) for unexpected condition in of_find_node_by_phandle() Frank Rowand (2): of: of_node_get()/of_node_put() nodes held in phandle cache of: __of_detach_node() - remove node from phandle cache drivers/of/base.c | 101 drivers/of/dynamic.c| 3 ++ drivers/of/of_private.h | 4 ++ 3 files changed, 83 insertions(+), 25 deletions(-) -- Frank Rowand
[PATCH v2 2/2] of: __of_detach_node() - remove node from phandle cache
From: Frank Rowand Non-overlay dynamic devicetree node removal may leave the node in the phandle cache. Subsequent calls to of_find_node_by_phandle() will incorrectly find the stale entry. Remove the node from the cache. Add paranoia checks in of_find_node_by_phandle() as a second level of defense (do not return cached node if detached, do not add node to cache if detached). Reported-by: Michael Bringmann Signed-off-by: Frank Rowand --- changes since v1: - add WARN_ON(1) for unexpected condition in of_find_node_by_phandle() drivers/of/base.c | 30 +- drivers/of/dynamic.c| 3 +++ drivers/of/of_private.h | 4 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 6c33d63361b8..ad71864cecf5 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -162,6 +162,27 @@ int of_free_phandle_cache(void) late_initcall_sync(of_free_phandle_cache); #endif +/* + * Caller must hold devtree_lock. + */ +void __of_free_phandle_cache_entry(phandle handle) +{ + phandle masked_handle; + + if (!handle) + return; + + masked_handle = handle & phandle_cache_mask; + + if (phandle_cache) { + if (phandle_cache[masked_handle] && + handle == phandle_cache[masked_handle]->phandle) { + of_node_put(phandle_cache[masked_handle]); + phandle_cache[masked_handle] = NULL; + } + } +} + void of_populate_phandle_cache(void) { unsigned long flags; @@ -1209,11 +1230,18 @@ struct device_node *of_find_node_by_phandle(phandle handle) if (phandle_cache[masked_handle] && handle == phandle_cache[masked_handle]->phandle) np = phandle_cache[masked_handle]; + if (np && of_node_check_flag(np, OF_DETACHED)) { + WARN_ON(1); + of_node_put(np); + phandle_cache[masked_handle] = NULL; + np = NULL; + } } if (!np) { for_each_of_allnodes(np) - if (np->phandle == handle) { + if (np->phandle == handle && + !of_node_check_flag(np, OF_DETACHED)) { if (phandle_cache) { /* will put when removed from cache */ of_node_get(np); diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index f4f8ed9b5454..ecea92f68c87 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -268,6 +268,9 @@ void __of_detach_node(struct device_node *np) } of_node_set_flag(np, OF_DETACHED); + + /* race with of_find_node_by_phandle() prevented by devtree_lock */ + __of_free_phandle_cache_entry(np->phandle); } /** diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 5d1567025358..24786818e32e 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -84,6 +84,10 @@ static inline void __of_detach_node_sysfs(struct device_node *np) {} int of_resolve_phandles(struct device_node *tree); #endif +#if defined(CONFIG_OF_DYNAMIC) +void __of_free_phandle_cache_entry(phandle handle); +#endif + #if defined(CONFIG_OF_OVERLAY) void of_overlay_mutex_lock(void); void of_overlay_mutex_unlock(void); -- Frank Rowand
[PATCH v2 1/2] of: of_node_get()/of_node_put() nodes held in phandle cache
From: Frank Rowand The phandle cache contains struct device_node pointers. The refcount of the pointers was not incremented while in the cache, allowing use after free error after kfree() of the node. Add the proper increment and decrement of the use count. Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Signed-off-by: Frank Rowand --- changes since v1 - make __of_free_phandle_cache() static drivers/of/base.c | 70 --- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 09692c9b32a7..6c33d63361b8 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -116,9 +116,6 @@ int __weak of_node_to_nid(struct device_node *np) } #endif -static struct device_node **phandle_cache; -static u32 phandle_cache_mask; - /* * Assumptions behind phandle_cache implementation: * - phandle property values are in a contiguous range of 1..n @@ -127,6 +124,44 @@ int __weak of_node_to_nid(struct device_node *np) * - the phandle lookup overhead reduction provided by the cache * will likely be less */ + +static struct device_node **phandle_cache; +static u32 phandle_cache_mask; + +/* + * Caller must hold devtree_lock. + */ +static void __of_free_phandle_cache(void) +{ + u32 cache_entries = phandle_cache_mask + 1; + u32 k; + + if (!phandle_cache) + return; + + for (k = 0; k < cache_entries; k++) + of_node_put(phandle_cache[k]); + + kfree(phandle_cache); + phandle_cache = NULL; +} + +int of_free_phandle_cache(void) +{ + unsigned long flags; + + raw_spin_lock_irqsave(&devtree_lock, flags); + + __of_free_phandle_cache(); + + raw_spin_unlock_irqrestore(&devtree_lock, flags); + + return 0; +} +#if !defined(CONFIG_MODULES) +late_initcall_sync(of_free_phandle_cache); +#endif + void of_populate_phandle_cache(void) { unsigned long flags; @@ -136,8 +171,7 @@ void of_populate_phandle_cache(void) raw_spin_lock_irqsave(&devtree_lock, flags); - kfree(phandle_cache); - phandle_cache = NULL; + __of_free_phandle_cache(); for_each_of_allnodes(np) if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) @@ -155,30 +189,15 @@ void of_populate_phandle_cache(void) goto out; for_each_of_allnodes(np) - if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) { + of_node_get(np); phandle_cache[np->phandle & phandle_cache_mask] = np; + } out: raw_spin_unlock_irqrestore(&devtree_lock, flags); } -int of_free_phandle_cache(void) -{ - unsigned long flags; - - raw_spin_lock_irqsave(&devtree_lock, flags); - - kfree(phandle_cache); - phandle_cache = NULL; - - raw_spin_unlock_irqrestore(&devtree_lock, flags); - - return 0; -} -#if !defined(CONFIG_MODULES) -late_initcall_sync(of_free_phandle_cache); -#endif - void __init of_core_init(void) { struct device_node *np; @@ -1195,8 +1214,11 @@ struct device_node *of_find_node_by_phandle(phandle handle) if (!np) { for_each_of_allnodes(np) if (np->phandle == handle) { - if (phandle_cache) + if (phandle_cache) { + /* will put when removed from cache */ + of_node_get(np); phandle_cache[masked_handle] = np; + } break; } } -- Frank Rowand
[PATCH v2 0/2] of: phandle_cache, fix refcounts, remove stale entry
From: Frank Rowand Non-overlay dynamic devicetree node removal may leave the node in the phandle cache. Subsequent calls to of_find_node_by_phandle() will incorrectly find the stale entry. This bug exposed the foloowing phandle cache refcount bug. The refcount of phandle_cache entries is not incremented while in the cache, allowing use after free error after kfree() of the cached entry. Changes since v1: - make __of_free_phandle_cache() static - add WARN_ON(1) for unexpected condition in of_find_node_by_phandle() Frank Rowand (2): of: of_node_get()/of_node_put() nodes held in phandle cache of: __of_detach_node() - remove node from phandle cache drivers/of/base.c | 100 drivers/of/dynamic.c| 3 ++ drivers/of/of_private.h | 4 ++ 3 files changed, 82 insertions(+), 25 deletions(-) -- Frank Rowand
[PATCH 2/2] of: __of_detach_node() - remove node from phandle cache
From: Frank Rowand Non-overlay dynamic devicetree node removal may leave the node in the phandle cache. Subsequent calls to of_find_node_by_phandle() will incorrectly find the stale entry. Remove the node from the cache. Add paranoia checks in of_find_node_by_phandle() as a second level of defense (do not return cached node if detached, do not add node to cache if detached). Reported-by: Michael Bringmann Signed-off-by: Frank Rowand --- drivers/of/base.c | 29 - drivers/of/dynamic.c| 3 +++ drivers/of/of_private.h | 4 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index d599367cb92a..34a5125713c8 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -162,6 +162,27 @@ int of_free_phandle_cache(void) late_initcall_sync(of_free_phandle_cache); #endif +/* + * Caller must hold devtree_lock. + */ +void __of_free_phandle_cache_entry(phandle handle) +{ + phandle masked_handle; + + if (!handle) + return; + + masked_handle = handle & phandle_cache_mask; + + if (phandle_cache) { + if (phandle_cache[masked_handle] && + handle == phandle_cache[masked_handle]->phandle) { + of_node_put(phandle_cache[masked_handle]); + phandle_cache[masked_handle] = NULL; + } + } +} + void of_populate_phandle_cache(void) { unsigned long flags; @@ -1209,11 +1230,17 @@ struct device_node *of_find_node_by_phandle(phandle handle) if (phandle_cache[masked_handle] && handle == phandle_cache[masked_handle]->phandle) np = phandle_cache[masked_handle]; + if (np && of_node_check_flag(np, OF_DETACHED)) { + of_node_put(np); + phandle_cache[masked_handle] = NULL; + np = NULL; + } } if (!np) { for_each_of_allnodes(np) - if (np->phandle == handle) { + if (np->phandle == handle && + !of_node_check_flag(np, OF_DETACHED)) { if (phandle_cache) { /* will put when removed from cache */ of_node_get(np); diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index f4f8ed9b5454..ecea92f68c87 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -268,6 +268,9 @@ void __of_detach_node(struct device_node *np) } of_node_set_flag(np, OF_DETACHED); + + /* race with of_find_node_by_phandle() prevented by devtree_lock */ + __of_free_phandle_cache_entry(np->phandle); } /** diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 5d1567025358..24786818e32e 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -84,6 +84,10 @@ static inline void __of_detach_node_sysfs(struct device_node *np) {} int of_resolve_phandles(struct device_node *tree); #endif +#if defined(CONFIG_OF_DYNAMIC) +void __of_free_phandle_cache_entry(phandle handle); +#endif + #if defined(CONFIG_OF_OVERLAY) void of_overlay_mutex_lock(void); void of_overlay_mutex_unlock(void); -- Frank Rowand
[PATCH 0/2] of: phandle_cache, fix refcounts, remove stale entry
From: Frank Rowand Non-overlay dynamic devicetree node removal may leave the node in the phandle cache. Subsequent calls to of_find_node_by_phandle() will incorrectly find the stale entry. This bug exposed the foloowing phandle cache refcount bug. The refcount of phandle_cache entries is not incremented while in the cache, allowing use after free error after kfree() of the cached entry. Frank Rowand (2): of: of_node_get()/of_node_put() nodes held in phandle cache of: __of_detach_node() - remove node from phandle cache drivers/of/base.c | 99 - drivers/of/dynamic.c| 3 ++ drivers/of/of_private.h | 4 ++ 3 files changed, 81 insertions(+), 25 deletions(-) -- Frank Rowand
[PATCH 1/2] of: of_node_get()/of_node_put() nodes held in phandle cache
From: Frank Rowand The phandle cache contains struct device_node pointers. The refcount of the pointers was not incremented while in the cache, allowing use after free error after kfree() of the node. Add the proper increment and decrement of the use count. Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Signed-off-by: Frank Rowand --- do not "cc: stable", unless the following commits are also in stable: commit e54192b48da7 ("of: fix phandle cache creation for DTs with no phandles") commit b9952b5218ad ("of: overlay: update phandle cache on overlay apply and remove") commit 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") drivers/of/base.c | 70 --- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 09692c9b32a7..d599367cb92a 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -116,9 +116,6 @@ int __weak of_node_to_nid(struct device_node *np) } #endif -static struct device_node **phandle_cache; -static u32 phandle_cache_mask; - /* * Assumptions behind phandle_cache implementation: * - phandle property values are in a contiguous range of 1..n @@ -127,6 +124,44 @@ int __weak of_node_to_nid(struct device_node *np) * - the phandle lookup overhead reduction provided by the cache * will likely be less */ + +static struct device_node **phandle_cache; +static u32 phandle_cache_mask; + +/* + * Caller must hold devtree_lock. + */ +void __of_free_phandle_cache(void) +{ + u32 cache_entries = phandle_cache_mask + 1; + u32 k; + + if (!phandle_cache) + return; + + for (k = 0; k < cache_entries; k++) + of_node_put(phandle_cache[k]); + + kfree(phandle_cache); + phandle_cache = NULL; +} + +int of_free_phandle_cache(void) +{ + unsigned long flags; + + raw_spin_lock_irqsave(&devtree_lock, flags); + + __of_free_phandle_cache(); + + raw_spin_unlock_irqrestore(&devtree_lock, flags); + + return 0; +} +#if !defined(CONFIG_MODULES) +late_initcall_sync(of_free_phandle_cache); +#endif + void of_populate_phandle_cache(void) { unsigned long flags; @@ -136,8 +171,7 @@ void of_populate_phandle_cache(void) raw_spin_lock_irqsave(&devtree_lock, flags); - kfree(phandle_cache); - phandle_cache = NULL; + __of_free_phandle_cache(); for_each_of_allnodes(np) if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) @@ -155,30 +189,15 @@ void of_populate_phandle_cache(void) goto out; for_each_of_allnodes(np) - if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) { + of_node_get(np); phandle_cache[np->phandle & phandle_cache_mask] = np; + } out: raw_spin_unlock_irqrestore(&devtree_lock, flags); } -int of_free_phandle_cache(void) -{ - unsigned long flags; - - raw_spin_lock_irqsave(&devtree_lock, flags); - - kfree(phandle_cache); - phandle_cache = NULL; - - raw_spin_unlock_irqrestore(&devtree_lock, flags); - - return 0; -} -#if !defined(CONFIG_MODULES) -late_initcall_sync(of_free_phandle_cache); -#endif - void __init of_core_init(void) { struct device_node *np; @@ -1195,8 +1214,11 @@ struct device_node *of_find_node_by_phandle(phandle handle) if (!np) { for_each_of_allnodes(np) if (np->phandle == handle) { - if (phandle_cache) + if (phandle_cache) { + /* will put when removed from cache */ + of_node_get(np); phandle_cache[masked_handle] = np; + } break; } } -- Frank Rowand
[PATCH] of: add dtc annotations functionality to dtx_diff
From: Frank Rowand Add -T and --annotations command line arguments to dtx_diff. These arguments will be passed through to dtc. dtc will then add source location annotations to its output. Signed-off-by: Frank Rowand --- This feature depends upon commit 5667e7ef9a9a ("annotations: add the annotation functionality") in the dtc git repository. To use the new flags before the new version of dtc is imported to the linux kernel, download the dtc repository, compile dtc with the make command, then add the path of the dtc repository to the shell PATH variable. scripts/dtc/dtx_diff | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/scripts/dtc/dtx_diff b/scripts/dtc/dtx_diff index 8c4fbad2055e..0d8572008729 100755 --- a/scripts/dtc/dtx_diff +++ b/scripts/dtc/dtx_diff @@ -21,6 +21,7 @@ Usage: diff DTx_1 and DTx_2 + --annotatesynonym for -T -f print full dts in diff (--unified=9) -h synonym for --help -helpsynonym for --help @@ -28,6 +29,7 @@ Usage: -s SRCTREE linux kernel source tree is at path SRCTREE (default is current directory) -S linux kernel source tree is at root of current git repo + -T Annotate output .dts with input source file and line (-T -T for more details) -u unsorted, do not sort DTx @@ -174,6 +176,7 @@ compile_to_dts() { # - start of script +annotate="" cmd_diff=0 diff_flags="-u" dtx_file_1="" @@ -208,6 +211,14 @@ while [ $# -gt 0 ] ; do shift ;; + -T | --annotate ) + if [ "${annotate}" = "" ] ; then + annotate="-T" + elif [ "${annotate}" = "-T" ] ; then + annotate="-T -T" + fi + shift + ;; -u ) dtc_sort="" shift @@ -327,7 +338,7 @@ cpp_flags="\ DTC="\ ${DTC} \ -i ${srctree}/scripts/dtc/include-prefixes \ - -O dts -qq -f ${dtc_sort} -o -" + -O dts -qq -f ${dtc_sort} ${annotate} -o -" # - do the diff or decompile -- Frank Rowand
[PATCH v4] of: overlay: user space synchronization
From: Frank Rowand When an overlay is applied or removed, the live devicetree visible in /proc/device-tree/, aka /sys/firmware/devicetree/base/, reflects the changes. There is no method for user space to determine whether the live devicetree was modified by overlay actions. Provide a sysfs file, /sys/firmware/devicetree/tree_version, to allow user space to determine if the live devicetree has remained unchanged while a series of one or more accesses of /proc/device-tree/ occur. The use of both (1) dynamic devicetree modifications and (2) overlay apply and removal are not supported during the same boot cycle. Thus non-overlay dynamic modifications are not reflected in the value of tree_version. Example shell use: # set done false (1) done=1 # keep trying until we can make it through the loop without # live tree being changed by an overlay changeset during the # 'critical region' while [ $done = 1 ] ; do pre_version=$(cat /sys/firmware/devicetree/tree_version) # 'critical region' # access /proc/device-tree/ one or more times # check that overlay did not change DT during critical region post_version=$(cat /sys/firmware/devicetree/tree_version) if [ ${post_version} = ${pre_version} ] ; then # set done true (0) done=0 fi done Signed-off-by: Frank Rowand --- changes since version 3: - Use mutex in sysfs show function for tree_version. The mutex provides the required synchronization. This also removes the need to increment tree_version twice per overlay apply or remove, simplifying the use of tree_version in user space. - Update documentation to reflect code changes. - Update documentation to show simpler user space usage. - Change tree_version from 32 bit to 64 bit to prevent problems with excessively rapid overlay applies and removes. This problem should not occur on a system that is using overlays as intended, but the cost of prevention is negligible. Documentation/ABI/testing/sysfs-firmware-ofw | 91 +--- drivers/of/base.c| 75 +++ drivers/of/of_private.h | 2 + drivers/of/overlay.c | 6 ++ 4 files changed, 167 insertions(+), 7 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-firmware-ofw b/Documentation/ABI/testing/sysfs-firmware-ofw index edcab3ccfcc0..c441d54b721c 100644 --- a/Documentation/ABI/testing/sysfs-firmware-ofw +++ b/Documentation/ABI/testing/sysfs-firmware-ofw @@ -1,4 +1,10 @@ -What: /sys/firmware/devicetree/* +What: /sys/firmware/devicetree/ +Date: November 2013 +Contact: Frank Rowand , devicet...@vger.kernel.org +Description: + Top level Open Firmware, aka devicetree, sysfs directory. + +What: /sys/firmware/devicetree/base Date: November 2013 Contact: Grant Likely , devicet...@vger.kernel.org Description: @@ -6,11 +12,6 @@ Description: hardware, the device tree structure will be exposed in this directory. - It is possible for multiple device-tree directories to exist. - Some device drivers use a separate detached device tree which - have no attachment to the system tree and will appear in a - different subdirectory under /sys/firmware/devicetree. - Userspace must not use the /sys/firmware/devicetree/base path directly, but instead should follow /proc/device-tree symlink. It is possible that the absolute path will change @@ -20,13 +21,89 @@ Description: filesystem support, and has largely the same semantics and should be compatible with existing userspace. - The contents of /sys/firmware/devicetree/ is a + The /sys/firmware/devicetree/base directory is the root + node of the devicetree. + + The contents of /sys/firmware/devicetree/base is a hierarchy of directories, one per device tree node. The directory name is the resolved path component name (node name plus address). Properties are represented as files in the directory. The contents of each file is the exact binary data from the device tree. +What: /sys/firmware/devicetree/tree_version +Date: October 2018 +KernelVersion: 4.20 +Contact: Frank Rowand , devicet...@vger.kernel.org +Description: + When an overlay is applied or removed, the live devicetree + visible in /proc/device-tree/, aka + /sys/firmware/devicetree/base/, reflects the changes. + + tree_version provides a way for user space to determine if the + live devicet
[PATCH v3] of: overlay: user space synchronization
From: Frank Rowand When an overlay is applied or removed, the live devicetree visible in /proc/device-tree/, aka /sys/firmware/devicetree/base/, reflects the changes. There is no method for user space to determine whether the live devicetree was modified by overlay actions. Provide a sysfs file, /sys/firmware/devicetree/tree_version, to allow user space to determine if the live devicetree has remained unchanged while a series of one or more accesses of /proc/device-tree/ occur. The use of both (1) dynamic devicetree modifications and (2) overlay apply and removal are not supported during the same boot cycle. Thus non-overlay dynamic modifications are not reflected in the value of tree_version. Example shell use: # set done false (1) done=1 # keep trying until we can make it through the loop without # live tree being changed by an overlay changeset during the # 'critical region' while [ $done = 1 ] ; do # loop while live tree is locked for overlay changes pre_version=$(cat /sys/firmware/devicetree/tree_version) while [ $(( ${pre_version} & 1 )) != 0 ] ; do # echo is optional, sleep value can be tuned echo "${pre_version} tree locked, sleeping 2 seconds" sleep 2; pre_version=$(cat /sys/firmware/devicetree/tree_version) done # 'critical region' # access /proc/device-tree/ one or more times # check that overlay did not change DT during critical region post_version=$(cat /sys/firmware/devicetree/tree_version) if [ ${post_version} = ${pre_version} ] ; then # set done true (0) done=0 fi done Signed-off-by: Frank Rowand --- Changes since version 2: - sysfs-firmware-ofw: add comments to shell script example - sysfs-firmware-ofw: add more information about the behavior of tree_version and how its value changes - correct error in patch comment shell script by copying the shell script from the patch body Documentation/ABI/testing/sysfs-firmware-ofw | 98 ++-- drivers/of/base.c| 66 +++ drivers/of/of_private.h | 2 + drivers/of/overlay.c | 12 4 files changed, 171 insertions(+), 7 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-firmware-ofw b/Documentation/ABI/testing/sysfs-firmware-ofw index edcab3ccfcc0..20860de3ca4c 100644 --- a/Documentation/ABI/testing/sysfs-firmware-ofw +++ b/Documentation/ABI/testing/sysfs-firmware-ofw @@ -1,4 +1,10 @@ -What: /sys/firmware/devicetree/* +What: /sys/firmware/devicetree/ +Date: November 2013 +Contact: Frank Rowand , devicet...@vger.kernel.org +Description: + Top level Open Firmware, aka devicetree, sysfs directory. + +What: /sys/firmware/devicetree/base Date: November 2013 Contact: Grant Likely , devicet...@vger.kernel.org Description: @@ -6,11 +12,6 @@ Description: hardware, the device tree structure will be exposed in this directory. - It is possible for multiple device-tree directories to exist. - Some device drivers use a separate detached device tree which - have no attachment to the system tree and will appear in a - different subdirectory under /sys/firmware/devicetree. - Userspace must not use the /sys/firmware/devicetree/base path directly, but instead should follow /proc/device-tree symlink. It is possible that the absolute path will change @@ -20,13 +21,96 @@ Description: filesystem support, and has largely the same semantics and should be compatible with existing userspace. - The contents of /sys/firmware/devicetree/ is a + The /sys/firmware/devicetree/base directory is the root + node of the devicetree. + + The contents of /sys/firmware/devicetree/base is a hierarchy of directories, one per device tree node. The directory name is the resolved path component name (node name plus address). Properties are represented as files in the directory. The contents of each file is the exact binary data from the device tree. +What: /sys/firmware/devicetree/tree_version +Date: October 2018 +KernelVersion: 4.20 +Contact: Frank Rowand , devicet...@vger.kernel.org +Description: + When an overlay is applied or removed, the live devicetree + visible in /proc/device-tree/, aka + /sys/firmware/devicetree/base/, reflects the changes. + + tree_version provides a way for user space to determine if the + live device
[PATCH v4 15/18] of: overlay: set node fields from properties when add new overlay node
From: Frank Rowand Overlay nodes added by add_changeset_node() do not have the node fields name, phandle, and type set. The node passed to __of_attach_node() when the add node changeset entry is processed does not contain any properties. The node's properties are located in add property changeset entries that will be processed after the add node changeset is applied. Set the node's fields in the node contained in the add node changeset entry and do not set them to incorrect values in add_changeset_node(). A visible symptom that is fixed by this patch is the names of nodes added by overlays that have an entry in /sys/bus/platform/drivers/*/ will contain the unit-address but the node-name will be , for example, "fc4ab000.". After applying the patch the name, in this example, for node restart@fc4ab000 is "fc4ab000.restart". Signed-off-by: Frank Rowand --- drivers/of/dynamic.c | 27 ++- drivers/of/overlay.c | 29 - 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index a94f727ec3da..a9f5d5fb3f25 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -205,15 +205,24 @@ static void __of_attach_node(struct device_node *np) const __be32 *phandle; int sz; - np->name = __of_get_property(np, "name", NULL) ? : ""; - np->type = __of_get_property(np, "device_type", NULL) ? : ""; - - phandle = __of_get_property(np, "phandle", &sz); - if (!phandle) - phandle = __of_get_property(np, "linux,phandle", &sz); - if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle) - phandle = __of_get_property(np, "ibm,phandle", &sz); - np->phandle = (phandle && (sz >= 4)) ? be32_to_cpup(phandle) : 0; + if (!of_node_check_flag(np, OF_OVERLAY)) { + np->name = __of_get_property(np, "name", NULL); + np->type = __of_get_property(np, "device_type", NULL); + if (!np->name) + np->name = ""; + if (!np->type) + np->type = ""; + + phandle = __of_get_property(np, "phandle", &sz); + if (!phandle) + phandle = __of_get_property(np, "linux,phandle", &sz); + if (IS_ENABLED(CONFIG_PPC_PSERIES) && !phandle) + phandle = __of_get_property(np, "ibm,phandle", &sz); + if (phandle && (sz >= 4)) + np->phandle = be32_to_cpup(phandle); + else + np->phandle = 0; + } np->child = NULL; np->sibling = np->parent->child; diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 01afd22566ed..d011177e5aaa 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -307,10 +307,11 @@ static int add_changeset_property(struct overlay_changeset *ovcs, int ret = 0; bool check_for_non_overlay_node = false; - 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 (target->in_livetree) + 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 (target->in_livetree) prop = of_find_property(target->np, overlay_prop->name, NULL); @@ -331,6 +332,10 @@ static int add_changeset_property(struct overlay_changeset *ovcs, if (!prop) { check_for_non_overlay_node = true; + if (!target->in_livetree) { + new_prop->next = target->np->deadprops; + target->np->deadprops = new_prop; + } ret = of_changeset_add_property(&ovcs->cset, target->np, new_prop); @@ -410,9 +415,10 @@ static int add_changeset_node(struct overlay_changeset *ovcs, struct target *target, struct device_node *node) { const char *node_kbasename; + const __be32 *phandle; struct device_node *tchild; struct target target_child; - int ret = 0; + int ret = 0, size; node_kbasename = kbasename(node->full_name); @@ -426,6 +432,19 @@ static int add_changeset_node(struct overlay_changeset *ovcs, return -ENOMEM; tchild->parent = target->np; + tchild->name = __of_get_property(node, "name", NULL); + tchild->type = __of_get_property(node, "device_type", NULL); + + if (!tchild->name) + tchild->name = ""; + if (!tchild->type) + tchild->type = ""; + + /* ignore obsolete "linux,phandle" */ +
[PATCH v4 09/18] of: overlay: validate overlay properties #address-cells and #size-cells
From: Frank Rowand If overlay properties #address-cells or #size-cells are already in the live devicetree for any given node, then the values in the overlay must match the values in the live tree. If the properties are already in the live tree then there is no need to create a changeset entry to add them since they must have the same value. This reduces the memory used by the changeset and eliminates a possible memory leak. This is verified by 12 fewer warnings during the devicetree unittest, as the possible memory leak warnings about #address-cells and Signed-off-by: Frank Rowand --- Changes since v3: - for errors of an overlay changing the value of #size-cells or #address-cells, return -EINVAL so that overlay apply will fail - for errors of an overlay changing the value of #size-cells or #address-cells, make the message more direct. Old message: OF: overlay: ERROR: overlay and/or live tree #size-cells invalid in node /soc/base_fpga_region New message: OF: overlay: ERROR: changing value of /soc/base_fpga_region/#size-cells not allowed drivers/of/overlay.c | 42 +++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 272a0d1a5e18..3e1e519c12f0 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -287,7 +287,12 @@ static struct property *dup_and_fixup_symbol_prop( * @target may be either in the live devicetree or in a new subtree that * is contained in the changeset. * - * Some special properties are not updated (no error returned). + * Some special properties are not added or updated (no error returned): + * "name", "phandle", "linux,phandle". + * + * Properties "#address-cells" and "#size-cells" are not updated if they + * are already in the live tree, but if present in the live tree, the values + * in the overlay must match the values in the live tree. * * Update of property in symbols node is not allowed. * @@ -300,6 +305,7 @@ static int add_changeset_property(struct overlay_changeset *ovcs, { struct property *new_prop = NULL, *prop; int ret = 0; + bool check_for_non_overlay_node = false; if (!of_prop_cmp(overlay_prop->name, "name") || !of_prop_cmp(overlay_prop->name, "phandle") || @@ -322,13 +328,43 @@ static int add_changeset_property(struct overlay_changeset *ovcs, if (!new_prop) return -ENOMEM; - if (!prop) + if (!prop) { + + check_for_non_overlay_node = true; ret = of_changeset_add_property(&ovcs->cset, target->np, new_prop); - else + + } else if (!of_prop_cmp(prop->name, "#address-cells")) { + + if (prop->length != 4 || new_prop->length != 4 || + *(u32 *)prop->value != *(u32 *)new_prop->value) { + pr_err("ERROR: changing value of %pOF/#address-cells is not allowed\n", + target->np); + ret = -EINVAL; + } + + } else if (!of_prop_cmp(prop->name, "#size-cells")) { + + if (prop->length != 4 || new_prop->length != 4 || + *(u32 *)prop->value != *(u32 *)new_prop->value) { + pr_err("ERROR: changing value of %pOF/#size-cells is not allowed\n", + target->np); + ret = -EINVAL; + } + + } else { + + check_for_non_overlay_node = true; ret = of_changeset_update_property(&ovcs->cset, target->np, new_prop); + } + + if (check_for_non_overlay_node && + !of_node_check_flag(target->np, OF_OVERLAY)) + pr_err("WARNING: %s(), memory leak will occur if overlay removed. Property: %pOF/%s\n", + __func__, target->np, new_prop->name); + if (ret) { kfree(new_prop->name); kfree(new_prop->value); -- Frank Rowand
[PATCH v2] of: overlay: user space synchronization
From: Frank Rowand When an overlay is applied or removed, the live devicetree visible in /proc/device-tree/, aka /sys/firmware/devicetree/base/, reflects the changes. There is no method for user space to determine whether the live devicetree was modified by overlay actions. Provide a sysfs file, /sys/firmware/devicetree/tree_version, to allow user space to determine if the live devicetree has remained unchanged while a series of one or more accesses of /proc/device-tree/ occur. The use of both dynamic devicetree modifications and overlay apply and removal are not supported during the same boot cycle. Thus non-overlay dynamic modifications are not reflected in the value of tree_version. Example shell use: done=1 while [ $done = 1 ] ; do pre_version=$(cat /sys/firmware/devicetree/tree_version) while [ $(( ${pre_version} & 1 )) != 0 ] ; do # echo is optional, sleep value can be tuned echo "${pre_version} locked, sleeping 2 seconds" sleep 2; pre_version=$(cat /sys/firmware/devicetree/tree_version) version=${pre_version} done # 'critical region' # access /proc/device-tree/ one or more times post_version=$(cat /sys/firmware/devicetree/tree_version) if [ ${post_version} = ${pre_version} ] ; then done=0 fi done Signed-off-by: Frank Rowand --- updates since v1: - removed unneeded variable "version" from shell script - explicitly state that an odd value of tree_version means the devicetree is locked for an overlay update not just in the source, but also in Documentation/ABI/testing/sysfs-firmware-ofw - document that tree_version is reported as an ascii number, the internal representation, and the resultant wrap around behavior Documentation/ABI/testing/sysfs-firmware-ofw | 67 +--- drivers/of/base.c| 66 +++ drivers/of/of_private.h | 2 + drivers/of/overlay.c | 12 + 4 files changed, 140 insertions(+), 7 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-firmware-ofw b/Documentation/ABI/testing/sysfs-firmware-ofw index edcab3ccfcc0..d2346eb61924 100644 --- a/Documentation/ABI/testing/sysfs-firmware-ofw +++ b/Documentation/ABI/testing/sysfs-firmware-ofw @@ -1,4 +1,10 @@ -What: /sys/firmware/devicetree/* +What: /sys/firmware/devicetree/ +Date: November 2013 +Contact: Frank Rowand , devicet...@vger.kernel.org +Description: + Top level Open Firmware, aka devicetree, sysfs directory. + +What: /sys/firmware/devicetree/base Date: November 2013 Contact: Grant Likely , devicet...@vger.kernel.org Description: @@ -6,11 +12,6 @@ Description: hardware, the device tree structure will be exposed in this directory. - It is possible for multiple device-tree directories to exist. - Some device drivers use a separate detached device tree which - have no attachment to the system tree and will appear in a - different subdirectory under /sys/firmware/devicetree. - Userspace must not use the /sys/firmware/devicetree/base path directly, but instead should follow /proc/device-tree symlink. It is possible that the absolute path will change @@ -20,13 +21,65 @@ Description: filesystem support, and has largely the same semantics and should be compatible with existing userspace. - The contents of /sys/firmware/devicetree/ is a + The /sys/firmware/devicetree/base directory is the root + node of the devicetree. + + The contents of /sys/firmware/devicetree/base is a hierarchy of directories, one per device tree node. The directory name is the resolved path component name (node name plus address). Properties are represented as files in the directory. The contents of each file is the exact binary data from the device tree. +What: /sys/firmware/devicetree/tree_version +Date: October 2018 +KernelVersion: 4.20 +Contact: Frank Rowand , devicet...@vger.kernel.org +Description: + When an overlay is applied or removed, the live devicetree + visible in /proc/device-tree/, aka + /sys/firmware/devicetree/base/, reflects the changes. + + tree_version provides a way for user space to determine if the + live devicetree has remained unchanged while a series of one + or more accesses of /proc/device-tree/ occur. If tre
[PATCH] of: Documentation: remove unmaintained todo file
From: Frank Rowand The todo.txt file was created by a previous maintainer and has never been updated by the current OPEN FIRMWARE AND FLATTENED DEVICE TREE maintainers. Remove the out of date file. Signed-off-by: Frank Rowand --- Documentation/devicetree/todo.txt | 10 -- 1 file changed, 10 deletions(-) delete mode 100644 Documentation/devicetree/todo.txt diff --git a/Documentation/devicetree/todo.txt b/Documentation/devicetree/todo.txt deleted file mode 100644 index b5139d1de811.. --- a/Documentation/devicetree/todo.txt +++ /dev/null @@ -1,10 +0,0 @@ -Todo list for devicetree: - -=== General structure === -- Switch from custom lists to (h)list_head for nodes and properties structure - -=== CONFIG_OF_DYNAMIC === -- Switch to RCU for tree updates and get rid of global spinlock -- Document node lifecycle for CONFIG_OF_DYNAMIC -- Always set ->full_name at of_attach_node() time -- pseries: Get rid of open-coded tree modification from arch/powerpc/platforms/pseries/dlpar.c -- Frank Rowand
[PATCH] of: overlay: update documentation to match current implementation
From: Frank Rowand The overlay information in Documentation/devicetree/ is out of date. Signed-off-by: Frank Rowand --- .../devicetree/dynamic-resolution-notes.txt| 24 -- Documentation/devicetree/overlay-notes.txt | 93 ++ 2 files changed, 42 insertions(+), 75 deletions(-) delete mode 100644 Documentation/devicetree/dynamic-resolution-notes.txt diff --git a/Documentation/devicetree/dynamic-resolution-notes.txt b/Documentation/devicetree/dynamic-resolution-notes.txt deleted file mode 100644 index c24ec366c5dc.. --- a/Documentation/devicetree/dynamic-resolution-notes.txt +++ /dev/null @@ -1,24 +0,0 @@ -Device Tree Dynamic Resolver Notes --- - -This document describes the implementation of the in-kernel -Device Tree resolver, residing in drivers/of/resolver.c - -How the resolver works --- - -The resolver is given as an input an arbitrary tree compiled with the -proper dtc option and having a /plugin/ tag. This generates the -appropriate __fixups__ & __local_fixups__ nodes. - -In sequence the resolver works by the following steps: - -1. Get the maximum device tree phandle value from the live tree + 1. -2. Adjust all the local phandles of the tree to resolve by that amount. -3. Using the __local__fixups__ node information adjust all local references - by the same amount. -4. For each property in the __fixups__ node locate the node it references - in the live tree. This is the label used to tag the node. -5. Retrieve the phandle of the target of the fixup. -6. For each fixup in the property locate the node:property:offset location - and replace it with the phandle value. diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt index 725fb8d255c1..5b34b2318c59 100644 --- a/Documentation/devicetree/overlay-notes.txt +++ b/Documentation/devicetree/overlay-notes.txt @@ -2,8 +2,8 @@ Device Tree Overlay Notes - This document describes the implementation of the in-kernel -device tree overlay functionality residing in drivers/of/overlay.c and is a -companion document to Documentation/devicetree/dynamic-resolution-notes.txt[1] +device tree overlay functionality residing in drivers/of/overlay.c and +drivers/of/resolver.c. How overlays work - @@ -34,26 +34,23 @@ Lets take an example where we have a foo board with the following base tree: }; foo.dts - -The overlay bar.dts, when loaded (and resolved as described in [1]) should +The overlay bar.dts, when processed by the devicetree resolver (as described +later in this document) and applied to the live devicetree) will result in a +live devicetree that is equivalent to foo+bar.dts. bar.dts - /plugin/; /* allow undefined label references and record them */ / { - /* various properties for loader use; i.e. part id etc. */ - fragment@0 { - target = <&ocp>; - __overlay__ { - /* bar peripheral */ - bar { - compatible = "corp,bar"; - ... /* various properties and child nodes */ - } - }; + &ocp: { + /* bar peripheral */ + bar { + compatible = "corp,bar"; + ... /* various properties and child nodes */ + } }; }; bar.dts - -result in foo+bar.dts foo+bar.dts - /* FOO platform + bar peripheral */ @@ -78,14 +75,16 @@ result in foo+bar.dts }; foo+bar.dts - -As a result of the overlay, a new device node (bar) has been created -so a bar platform device will be registered and if a matching device driver -is loaded the device will be created as expected. +As a result of the overlay, a new device node (bar) has been created. Thus +a bar platform device will have been registered. If a matching device +driver is loaded the device will have been created as expected. -Overlay in-kernel API - +An overlay may specify the location of a node in the overlay with path +notation as the label reference. In the bar.dts example above, '&ocp:' +can also be expressed as '&{/ocp/}'. -The API is quite easy to use. +Overlay in-kernel API +- 1. Call of_overlay_fdt_apply() to create and apply an overlay changeset. The return value is an error or a cookie identifying this overlay. @@ -106,34 +105,26 @@ Note that a notifier callback is not supposed to store pointers to a device tree nod
[PATCH] of: overlay: user space synchronization
From: Frank Rowand When an overlay is applied or removed, the live devicetree visible in /proc/device-tree/, aka /sys/firmware/devicetree/base/, reflects the changes. There is no method for user space to determine whether the live devicetree was modified by overlay actions. Provide a sysfs file, /sys/firmware/devicetree/tree_version, to allow user space to determine if the live devicetree has remained unchanged while a series of one or more accesses of /proc/device-tree/ occur. The use of both dynamic devicetree modifications and overlay apply and removal are not supported during the same boot cycle. Thus non-overlay dynamic modifications are not reflected in the value of tree_version. Example shell use: done=1 while [ $done = 1 ] ; do pre_version=$(cat /sys/firmware/devicetree/tree_version) version=$pre_version while [ $(( ${version} & 1 )) != 0 ] ; do # echo is optional, sleep value can be tuned echo "${version} sleeping" sleep 2; pre_version=$(cat /sys/firmware/devicetree/tree_version) version=${pre_version} done # 'critical region' # access /proc/device-tree/ one or more times post_version=$(cat /sys/firmware/devicetree/tree_version) if [ ${post_version} = ${pre_version} ] ; then done=0 fi done Signed-off-by: Frank Rowand --- Documentation/ABI/testing/sysfs-firmware-ofw | 64 --- drivers/of/base.c| 66 drivers/of/of_private.h | 2 + drivers/of/overlay.c | 12 + 4 files changed, 137 insertions(+), 7 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-firmware-ofw b/Documentation/ABI/testing/sysfs-firmware-ofw index edcab3ccfcc0..4111797e8ed5 100644 --- a/Documentation/ABI/testing/sysfs-firmware-ofw +++ b/Documentation/ABI/testing/sysfs-firmware-ofw @@ -1,4 +1,10 @@ -What: /sys/firmware/devicetree/* +What: /sys/firmware/devicetree/ +Date: November 2013 +Contact: Frank Rowand , devicet...@vger.kernel.org +Description: + Top level Open Firmware, aka devicetree, sysfs directory. + +What: /sys/firmware/devicetree/base Date: November 2013 Contact: Grant Likely , devicet...@vger.kernel.org Description: @@ -6,11 +12,6 @@ Description: hardware, the device tree structure will be exposed in this directory. - It is possible for multiple device-tree directories to exist. - Some device drivers use a separate detached device tree which - have no attachment to the system tree and will appear in a - different subdirectory under /sys/firmware/devicetree. - Userspace must not use the /sys/firmware/devicetree/base path directly, but instead should follow /proc/device-tree symlink. It is possible that the absolute path will change @@ -20,13 +21,62 @@ Description: filesystem support, and has largely the same semantics and should be compatible with existing userspace. - The contents of /sys/firmware/devicetree/ is a + The /sys/firmware/devicetree/base directory is the root + node of the devicetree. + + The contents of /sys/firmware/devicetree/base is a hierarchy of directories, one per device tree node. The directory name is the resolved path component name (node name plus address). Properties are represented as files in the directory. The contents of each file is the exact binary data from the device tree. +What: /sys/firmware/devicetree/tree_version +Date: October 2018 +KernelVersion: 4.20 +Contact: Frank Rowand , devicet...@vger.kernel.org +Description: + When an overlay is applied or removed, the live devicetree + visible in /proc/device-tree/, aka + /sys/firmware/devicetree/base/, reflects the changes. + + tree_version provides a way for user space to determine if the + live devicetree has remained unchanged while a series of one + or more accesses of /proc/device-tree/ occur. + + The use of both dynamic devicetree modifications and overlay + apply and removal are not supported during the same boot + cycle. Thus non-overlay dynamic modifications are not + reflected in the value of tree_version. + + Example shell use of tree_version: + + done=1 + + while [ $done = 1 ] ; do + +
[PATCH 6/6] ARM: dts: qcom-msm8974: change invalid flag IRQ NONE to valid value
From: Frank Rowand Change the third field of the "interrupts" property from IRQ_TYPE_NONE to the correct value. I do not have hardware documentation for these devices, so I followed a mail list suggestion to copy the flag values from the same type of node in arch/arm64/boot/dts/qcom/msm8916.dtsi Signed-off-by: Frank Rowand --- Compile and boot tested on a Qualcomm APQ8074 Dragonboard. arch/arm/boot/dts/qcom-msm8974.dtsi | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi index 1e54113d6d9a..9550f0612918 100644 --- a/arch/arm/boot/dts/qcom-msm8974.dtsi +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi @@ -586,7 +586,7 @@ blsp1_uart1: serial@f991d000 { compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm"; reg = <0xf991d000 0x1000>; - interrupts = ; + interrupts = ; clocks = <&gcc GCC_BLSP1_UART1_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; clock-names = "core", "iface"; status = "disabled"; @@ -595,7 +595,7 @@ blsp1_uart2: serial@f991e000 { compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm"; reg = <0xf991e000 0x1000>; - interrupts = ; + interrupts = ; clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; clock-names = "core", "iface"; status = "disabled"; @@ -605,8 +605,8 @@ compatible = "qcom,sdhci-msm-v4"; reg = <0xf9824900 0x11c>, <0xf9824000 0x800>; reg-names = "hc_mem", "core_mem"; - interrupts = , -; + interrupts = , +; interrupt-names = "hc_irq", "pwr_irq"; clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>, @@ -619,8 +619,8 @@ compatible = "qcom,sdhci-msm-v4"; reg = <0xf9864900 0x11c>, <0xf9864000 0x800>; reg-names = "hc_mem", "core_mem"; - interrupts = , -; + interrupts = , +; interrupt-names = "hc_irq", "pwr_irq"; clocks = <&gcc GCC_SDCC3_APPS_CLK>, <&gcc GCC_SDCC3_AHB_CLK>, @@ -633,8 +633,8 @@ compatible = "qcom,sdhci-msm-v4"; reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>; reg-names = "hc_mem", "core_mem"; - interrupts = , -; + interrupts = , +; interrupt-names = "hc_irq", "pwr_irq"; clocks = <&gcc GCC_SDCC2_APPS_CLK>, <&gcc GCC_SDCC2_AHB_CLK>, @@ -701,14 +701,14 @@ #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; - interrupts = ; + interrupts = ; }; i2c@f9924000 { status = "disabled"; compatible = "qcom,i2c-qup-v2.1.1"; reg = <0xf9924000 0x1000>; - interrupts = ; + interrupts = ; clocks = <&gcc GCC_BLSP1_QUP2_I2C_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; clock-names = "core", "iface"; #address-cells = <1>; @@ -719,7 +719,7 @@ status = "disabled"; compatible = "qcom,i2c-qup-v2.1.1"; reg = <0xf9964000 0x1000>; - interrupts = ; + interrupts = ; clocks = <&gcc GCC_BLSP2_QUP2_I2C_APPS_CLK>, <&gcc GCC_BLSP2_AHB_CLK>; clock-names = "core", "iface"; #address-cells = <1>; @@ -730,7 +730,7 @@ status = "disabled"; compatible = "qcom,i2c-qup-v2.1.1"; reg = <0xf9967000 0x1000>; - interrupts = ; + interrupts = ; clocks = <&gcc GCC_BLSP2_QUP5_I2C_APPS_CLK>, <&gcc GCC_BLSP2_AHB_CLK>; clock-names = "core", "iface"; #address-cells = <1>; @@ -746,7 +746,7 @@ <0xfc4cb000 0x1000>,
[PATCH 2/6] ARM: dts: qcom-msm8974: use named constant for interrupt type GIC_SPI
From: Frank Rowand Cosmetic change of integer value "0" in the first field of the "interrupts" property to the correct named constant. Signed-off-by: Frank Rowand --- arch/arm/boot/dts/qcom-msm8974.dtsi | 56 +++-- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi index f4f5e2df4c03..c09cc1232a6f 100644 --- a/arch/arm/boot/dts/qcom-msm8974.dtsi +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi @@ -243,7 +243,7 @@ adsp-pil { compatible = "qcom,msm8974-adsp-pil"; - interrupts-extended = <&intc 0 162 IRQ_TYPE_EDGE_RISING>, + interrupts-extended = <&intc GIC_SPI 162 IRQ_TYPE_EDGE_RISING>, <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>, <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>, <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>, @@ -275,7 +275,7 @@ qcom,smem = <443>, <429>; interrupt-parent = <&intc>; - interrupts = <0 158 IRQ_TYPE_EDGE_RISING>; + interrupts = ; qcom,ipc = <&apcs 8 10>; @@ -300,7 +300,7 @@ qcom,smem = <435>, <428>; interrupt-parent = <&intc>; - interrupts = <0 27 IRQ_TYPE_EDGE_RISING>; + interrupts = ; qcom,ipc = <&apcs 8 14>; @@ -325,7 +325,7 @@ qcom,smem = <451>, <431>; interrupt-parent = <&intc>; - interrupts = <0 143 IRQ_TYPE_EDGE_RISING>; + interrupts = ; qcom,ipc = <&apcs 8 18>; @@ -364,7 +364,7 @@ modem_smsm: modem@1 { reg = <1>; - interrupts = <0 26 IRQ_TYPE_EDGE_RISING>; + interrupts = ; interrupt-controller; #interrupt-cells = <2>; @@ -372,7 +372,7 @@ adsp_smsm: adsp@2 { reg = <2>; - interrupts = <0 157 IRQ_TYPE_EDGE_RISING>; + interrupts = ; interrupt-controller; #interrupt-cells = <2>; @@ -380,7 +380,7 @@ wcnss_smsm: wcnss@7 { reg = <7>; - interrupts = <0 144 IRQ_TYPE_EDGE_RISING>; + interrupts = ; interrupt-controller; #interrupt-cells = <2>; @@ -445,50 +445,50 @@ frame@f9021000 { frame-number = <0>; - interrupts = <0 8 0x4>, -<0 7 0x4>; + interrupts = , +; reg = <0xf9021000 0x1000>, <0xf9022000 0x1000>; }; frame@f9023000 { frame-number = <1>; - interrupts = <0 9 0x4>; + interrupts = ; reg = <0xf9023000 0x1000>; status = "disabled"; }; frame@f9024000 { frame-number = <2>; - interrupts = <0 10 0x4>; + interrupts = ; reg = <0xf9024000 0x1000>; status = "disabled"; }; frame@f9025000 { frame-number = <3>; - interrupts = <0 11 0x4>; + interrupts = ; reg = <0xf9025000 0x1000>; status = "disabled"; }; frame@f9026000 { frame-number = <4>; - interrupts = <0 12 0x4>; + interrupts = ; reg = <0xf9026000 0x1000>; status = "disabled"; }; frame@f9027000 { frame-number = <5>; - interrupts = <0 13 0x4>; + interrupts = ; reg = <0xf9027000 0x1000>; status = "disabled"; }; frame@f9028000 { frame-number = <6>; - interrupts = <0 14 0x4>; + interrupts = ;
[PATCH 1/6] ARM: dts: qcom-msm8974: use named constant for interrupt type GIC_PPI
From: Frank Rowand Cosmetic change of integer value "1" in the first field of the "interrupts" property to the correct named constant. Signed-off-by: Frank Rowand --- arch/arm/boot/dts/qcom-msm8974.dtsi | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi index d9019a49b292..f4f5e2df4c03 100644 --- a/arch/arm/boot/dts/qcom-msm8974.dtsi +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi @@ -67,7 +67,7 @@ cpus { #address-cells = <1>; #size-cells = <0>; - interrupts = <1 9 0xf04>; + interrupts = ; CPU0: cpu@0 { compatible = "qcom,krait"; @@ -214,7 +214,7 @@ cpu-pmu { compatible = "qcom,krait-pmu"; - interrupts = <1 7 0xf04>; + interrupts = ; }; clocks { @@ -233,10 +233,10 @@ timer { compatible = "arm,armv7-timer"; - interrupts = <1 2 0xf08>, -<1 3 0xf08>, -<1 4 0xf08>, -<1 1 0xf08>; + interrupts = , +, +, +; clock-frequency = <1920>; }; -- Frank Rowand
[PATCH 0/6] ARM: dts: qcom-msm8974: change invalid flag IRQ NONE to valid value
From: Frank Rowand A boot time warning of devicetree interrupts types set to the invalid value of none was added by 83a86fbb5b56 ("irqchip/gic: Loudly complain about the use of IRQ_TYPE_NONE"). This patch series fixes the devicetree source to replace IRQ_TYPE_NONE with the appropriate value. Some cosmetic changes are made by several patches. The final patch in the series replaces the IRQ_TYPE_NONE values with the proper values. The cosmetic changes are to change integer values in the third field of the "interrupts" property to the correct named constant. Bjorn suggested that I also change integer values in the first field of the "interrupts" property to the correct named constant. The series is split into several patches for ease of review. Each cosmetic patch addresses a single named constant. Frank Rowand (6): ARM: dts: qcom-msm8974: use named constant for interrupt type GIC_PPI ARM: dts: qcom-msm8974: use named constant for interrupt type GIC_SPI ARM: dts: qcom-msm8974: use named constant for interrupt flag EDGE RISING ARM: dts: qcom-msm8974: use named constant for interrupt flag LEVEL HIGH ARM: dts: qcom-msm8974: use named constant for interrupt flag NONE ARM: dts: qcom-msm8974: change invalid flag IRQ NONE to valid value arch/arm/boot/dts/qcom-msm8974.dtsi | 72 +++-- 1 file changed, 37 insertions(+), 35 deletions(-) -- Frank Rowand
[PATCH 4/6] ARM: dts: qcom-msm8974: use named constant for interrupt flag LEVEL HIGH
From: Frank Rowand Cosmetic change of integer value "4" in the third field of the "interrupts" property to the correct named constant. Signed-off-by: Frank Rowand --- arch/arm/boot/dts/qcom-msm8974.dtsi | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi index 6273b6120be0..c7198900b426 100644 --- a/arch/arm/boot/dts/qcom-msm8974.dtsi +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi @@ -445,50 +445,50 @@ frame@f9021000 { frame-number = <0>; - interrupts = , -; + interrupts = , +; reg = <0xf9021000 0x1000>, <0xf9022000 0x1000>; }; frame@f9023000 { frame-number = <1>; - interrupts = ; + interrupts = ; reg = <0xf9023000 0x1000>; status = "disabled"; }; frame@f9024000 { frame-number = <2>; - interrupts = ; + interrupts = ; reg = <0xf9024000 0x1000>; status = "disabled"; }; frame@f9025000 { frame-number = <3>; - interrupts = ; + interrupts = ; reg = <0xf9025000 0x1000>; status = "disabled"; }; frame@f9026000 { frame-number = <4>; - interrupts = ; + interrupts = ; reg = <0xf9026000 0x1000>; status = "disabled"; }; frame@f9027000 { frame-number = <5>; - interrupts = ; + interrupts = ; reg = <0xf9027000 0x1000>; status = "disabled"; }; frame@f9028000 { frame-number = <6>; - interrupts = ; + interrupts = ; reg = <0xf9028000 0x1000>; status = "disabled"; }; -- Frank Rowand
[PATCH 5/6] ARM: dts: qcom-msm8974: use named constant for interrupt flag NONE
From: Frank Rowand Cosmetic change of integer value "0" in the third field of the "interrupts" property to the correct named constant. Signed-off-by: Frank Rowand --- arch/arm/boot/dts/qcom-msm8974.dtsi | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi index c7198900b426..1e54113d6d9a 100644 --- a/arch/arm/boot/dts/qcom-msm8974.dtsi +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi @@ -586,7 +586,7 @@ blsp1_uart1: serial@f991d000 { compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm"; reg = <0xf991d000 0x1000>; - interrupts = ; + interrupts = ; clocks = <&gcc GCC_BLSP1_UART1_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; clock-names = "core", "iface"; status = "disabled"; @@ -595,7 +595,7 @@ blsp1_uart2: serial@f991e000 { compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm"; reg = <0xf991e000 0x1000>; - interrupts = ; + interrupts = ; clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>; clock-names = "core", "iface"; status = "disabled"; @@ -605,8 +605,8 @@ compatible = "qcom,sdhci-msm-v4"; reg = <0xf9824900 0x11c>, <0xf9824000 0x800>; reg-names = "hc_mem", "core_mem"; - interrupts = , -; + interrupts = , +; interrupt-names = "hc_irq", "pwr_irq"; clocks = <&gcc GCC_SDCC1_APPS_CLK>, <&gcc GCC_SDCC1_AHB_CLK>, @@ -633,8 +633,8 @@ compatible = "qcom,sdhci-msm-v4"; reg = <0xf98a4900 0x11c>, <0xf98a4000 0x800>; reg-names = "hc_mem", "core_mem"; - interrupts = , -; + interrupts = , +; interrupt-names = "hc_irq", "pwr_irq"; clocks = <&gcc GCC_SDCC2_APPS_CLK>, <&gcc GCC_SDCC2_AHB_CLK>, @@ -701,7 +701,7 @@ #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; - interrupts = ; + interrupts = ; }; i2c@f9924000 { @@ -746,7 +746,7 @@ <0xfc4cb000 0x1000>, <0xfc4ca000 0x1000>; interrupt-names = "periph_irq"; - interrupts = ; + interrupts = ; qcom,ee = <0>; qcom,channel = <0>; #address-cells = <2>; -- Frank Rowand
[PATCH 3/6] ARM: dts: qcom-msm8974: use named constant for interrupt flag EDGE RISING
From: Frank Rowand Cosmetic change of integer value "1" in the third field of the "interrupts" property to the correct named constant. Signed-off-by: Frank Rowand --- arch/arm/boot/dts/qcom-msm8974.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi index c09cc1232a6f..6273b6120be0 100644 --- a/arch/arm/boot/dts/qcom-msm8974.dtsi +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi @@ -1056,7 +1056,7 @@ }; rpm { - interrupts = ; + interrupts = ; qcom,ipc = <&apcs 8 0>; qcom,smd-edge = <15>; -- Frank Rowand
[PATCH] ARM: qcom_defconfig: Enable MAILBOX
From: Frank Rowand Problem: ab460a2e72da ("rpmsg: qcom_smd: Access APCS through mailbox framework" added a "depends on MAILBOX") to RPMSG_QCOM_SMD, thus RPMSG_QCOM_SMD becomes unset since MAILBOX was not enabled in qcom_defconfig and is not otherwise selected for the dragonboard. When the resulting kernel is booted the mmc device which contains the root file system is not available. Fix: add CONFIG_MAILBOX to qcom_defconfig Fixes: ab460a2e72da ("rpmsg: qcom_smd: Access APCS through mailbox framework" added a "depends on MAILBOX") Signed-off-by: Frank Rowand --- Bjorn suggested that multi_v7_defconfig might need this same fix but I don't have a target to test on so I have not included that change in this patch Tested on Qualcomm APQ8074 Dragonboard arch/arm/configs/qcom_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig index 6aa7046fb91f..bd6440f23493 100644 --- a/arch/arm/configs/qcom_defconfig +++ b/arch/arm/configs/qcom_defconfig @@ -207,6 +207,7 @@ CONFIG_MSM_MMCC_8974=y CONFIG_MSM_IOMMU=y CONFIG_HWSPINLOCK=y CONFIG_HWSPINLOCK_QCOM=y +CONFIG_MAILBOX=y CONFIG_REMOTEPROC=y CONFIG_QCOM_ADSP_PIL=y CONFIG_QCOM_Q6V5_PIL=y -- Frank Rowand
[PATCH] of: overlay: update phandle cache on overlay apply and remove
From: Frank Rowand A comment in the review of the patch adding the phandle cache said that the cache would have to be updated when modules are applied and removed. This patch implements the cache updates. Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Reported-by: Alan Tull Suggested-by: Alan Tull Signed-off-by: Frank Rowand --- Changes since RFC: - update code comment to mention race condition avoidance For the RFC version of this patch, the 0day test reported a general protection fault from the KASAN runtime memory debugger on x86_64 in qemu. The GPF was in a devicetree unittest. 0day tested the patch on v4.17-rc1, with some other patches applied. I was unable to replicate the GPF on v4.18-rc1 with just this patch applied. I was also unable to replicate the GPF on a clone of the v4.17-rc1 0day repository, using the 0day kernel config. I will reply to this email with the 0day GPF report. drivers/of/base.c | 6 +++--- drivers/of/of_private.h | 2 ++ drivers/of/overlay.c| 11 +++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 848f549164cd..466e3c8582f0 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -102,7 +102,7 @@ static u32 phandle_cache_mask; * - the phandle lookup overhead reduction provided by the cache * will likely be less */ -static void of_populate_phandle_cache(void) +void of_populate_phandle_cache(void) { unsigned long flags; u32 cache_entries; @@ -134,8 +134,7 @@ static void of_populate_phandle_cache(void) raw_spin_unlock_irqrestore(&devtree_lock, flags); } -#ifndef CONFIG_MODULES -static int __init of_free_phandle_cache(void) +int of_free_phandle_cache(void) { unsigned long flags; @@ -148,6 +147,7 @@ static int __init of_free_phandle_cache(void) return 0; } +#if !defined(CONFIG_MODULES) late_initcall_sync(of_free_phandle_cache); #endif diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 891d780c076a..216175d11d3d 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -79,6 +79,8 @@ int of_resolve_phandles(struct device_node *tree); #if defined(CONFIG_OF_OVERLAY) void of_overlay_mutex_lock(void); void of_overlay_mutex_unlock(void); +int of_free_phandle_cache(void); +void of_populate_phandle_cache(void); #else static inline void of_overlay_mutex_lock(void) {}; static inline void of_overlay_mutex_unlock(void) {}; diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 7baa53e5b1d7..eda57ef12fd0 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -804,6 +804,8 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree, goto err_free_overlay_changeset; } + of_populate_phandle_cache(); + ret = __of_changeset_apply_notify(&ovcs->cset); if (ret) pr_err("overlay changeset entry notify error %d\n", ret); @@ -1046,8 +1048,17 @@ int of_overlay_remove(int *ovcs_id) list_del(&ovcs->ovcs_list); + /* +* Disable phandle cache. Avoids race condition that would arise +* from removing cache entry when the associated node is deleted. +*/ + of_free_phandle_cache(); + ret_apply = 0; ret = __of_changeset_revert_entries(&ovcs->cset, &ret_apply); + + of_populate_phandle_cache(); + if (ret) { if (ret_apply) devicetree_state_flags |= DTSF_REVERT_FAIL; -- Frank Rowand
[RFC PATCH] of: overlay: update phandle cache on overlay apply and remove
From: Frank Rowand A comment in the review of the patch adding the phandle cache said that the cache would have to be updated when modules are applied and removed. This patch implements the cache updates. Fixes: 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of of_find_node_by_phandle()") Reported-by: Alan Tull Suggested-by: Alan Tull Signed-off-by: Frank Rowand --- Compiles for one configuration. NOT boot tested. Not run through my normal process to check for new warnings, etc. It is late, I'm tired, my brain is fuzzy. I need to review this more to have any confidence in it. But I wanted to get a version out for Alan to see (and test if he wants). drivers/of/base.c | 6 +++--- drivers/of/of_private.h | 2 ++ drivers/of/overlay.c| 12 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 848f549164cd..466e3c8582f0 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -102,7 +102,7 @@ static u32 phandle_cache_mask; * - the phandle lookup overhead reduction provided by the cache * will likely be less */ -static void of_populate_phandle_cache(void) +void of_populate_phandle_cache(void) { unsigned long flags; u32 cache_entries; @@ -134,8 +134,7 @@ static void of_populate_phandle_cache(void) raw_spin_unlock_irqrestore(&devtree_lock, flags); } -#ifndef CONFIG_MODULES -static int __init of_free_phandle_cache(void) +int of_free_phandle_cache(void) { unsigned long flags; @@ -148,6 +147,7 @@ static int __init of_free_phandle_cache(void) return 0; } +#if !defined(CONFIG_MODULES) late_initcall_sync(of_free_phandle_cache); #endif diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 891d780c076a..216175d11d3d 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -79,6 +79,8 @@ int of_resolve_phandles(struct device_node *tree); #if defined(CONFIG_OF_OVERLAY) void of_overlay_mutex_lock(void); void of_overlay_mutex_unlock(void); +int of_free_phandle_cache(void); +void of_populate_phandle_cache(void); #else static inline void of_overlay_mutex_lock(void) {}; static inline void of_overlay_mutex_unlock(void) {}; diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 7baa53e5b1d7..3f76e58fbec4 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -804,6 +804,8 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree, goto err_free_overlay_changeset; } + of_populate_phandle_cache(); + ret = __of_changeset_apply_notify(&ovcs->cset); if (ret) pr_err("overlay changeset entry notify error %d\n", ret); @@ -1046,8 +1048,18 @@ int of_overlay_remove(int *ovcs_id) list_del(&ovcs->ovcs_list); + /* +* Empty and disable phandle cache. Must empty here so that +* changeset notifiers do not use stale cache entry for a removed +* phandle. +*/ + of_free_phandle_cache(); + ret_apply = 0; ret = __of_changeset_revert_entries(&ovcs->cset, &ret_apply); + + of_populate_phandle_cache(); + if (ret) { if (ret_apply) devicetree_state_flags |= DTSF_REVERT_FAIL; -- Frank Rowand
[PATCH] of: overlay: validate offset from property fixups
From: Frank Rowand The smatch static checker marks the data in offset as untrusted, leading it to warn: drivers/of/resolver.c:125 update_usages_of_a_phandle_reference() error: buffer underflow 'prop->value' 's32min-s32max' Add check to verify that offset is within the property data. Reported-by: Dan Carpenter Signed-off-by: Frank Rowand --- drivers/of/resolver.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 65d0b7adfcd4..7edfac6f1914 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -122,6 +122,11 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay, goto err_fail; } + if (offset < 0 || offset + sizeof(__be32) > prop->length) { + err = -EINVAL; + goto err_fail; + } + *(__be32 *)(prop->value + offset) = cpu_to_be32(phandle); } -- Frank Rowand
[PATCH v2] MAINTAINERS: add keyword for devicetree overlay notifiers
From: Frank Rowand Devicetree overlay notifiers have a chance to potentially get pointers into the overlay unflattened devicetree and overlay FDT. The only protection against these pointers being accessed after the underlying data has been released by kfree() is by source code review of patches. Add a keyword line to the devicetree overlay maintainers entry to try to catch overlay notifier related patches. The keyword line is added to the devicetree overlay entry instead of the devicetree entry so that not all maintainers will receive the additional review traffic. Add Frank Rowand (already a maintainer in the devicetree entry) so that he will receive the additional review traffic. Signed-off-by: Frank Rowand --- changes from version 1: - removed extraneous "From:" tags at top of patch description MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 92be777d060a..aca3956694f8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10422,12 +10422,14 @@ F:drivers/infiniband/ulp/opa_vnic OPEN FIRMWARE AND DEVICE TREE OVERLAYS M: Pantelis Antoniou +M: Frank Rowand L: devicet...@vger.kernel.org S: Maintained F: Documentation/devicetree/dynamic-resolution-notes.txt F: Documentation/devicetree/overlay-notes.txt F: drivers/of/overlay.c F: drivers/of/resolver.c +K: of_overlay_notifier_ OPEN FIRMWARE AND FLATTENED DEVICE TREE M: Rob Herring -- Frank Rowand
[PATCH] MAINTAINERS: add keyword for devicetree overlay notifiers
From: Frank Rowand From: Frank Rowand Devicetree overlay notifiers have a chance to potentially get pointers into the overlay unflattened devicetree and overlay FDT. The only protection against these pointers being accessed after the underlying data has been released by kfree() is by source code review of patches. Add a keyword line to the devicetree overlay maintainers entry to try to catch overlay notifier related patches. The keyword line is added to the devicetree overlay entry instead of the devicetree entry so that not all maintainers will receive the additional review traffic. Add Frank Rowand (already a maintainer in the devicetree entry) so that he will receive the additional review traffic. Signed-off-by: Frank Rowand --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 92be777d060a..aca3956694f8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10422,12 +10422,14 @@ F:drivers/infiniband/ulp/opa_vnic OPEN FIRMWARE AND DEVICE TREE OVERLAYS M: Pantelis Antoniou +M: Frank Rowand L: devicet...@vger.kernel.org S: Maintained F: Documentation/devicetree/dynamic-resolution-notes.txt F: Documentation/devicetree/overlay-notes.txt F: drivers/of/overlay.c F: drivers/of/resolver.c +K: of_overlay_notifier_ OPEN FIRMWARE AND FLATTENED DEVICE TREE M: Rob Herring -- Frank Rowand
[PATCH 1/2] of: unittest: remove unneeded local return value variables
From: Frank Rowand A common pattern in many unittest functions is to save the return value of a function in a local variable, then test the value of the local variable, without using that return value for any further purpose. Remove the local return value variable for these cases. A second common pattern is: ret = some_test_function(many, parameters, ...); if (unittest(ret == 0, "error message format", ...)) return; This pattern is more clear when the local variable 'ret' is used, due to the long lines caused by the parameters to the test function and the long format and data parameters of unittest(). The local variable is retained in these cases. Signed-off-by: Frank Rowand --- drivers/of/unittest.c | 89 ++- 1 file changed, 24 insertions(+), 65 deletions(-) diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index a23b54780c7d..546483c0be62 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -1380,11 +1380,8 @@ static int __init of_unittest_apply_revert_overlay_check(int overlay_nr, /* test activation of device */ static void __init of_unittest_overlay_0(void) { - int ret; - /* device should enable */ - ret = of_unittest_apply_overlay_check(0, 0, 0, 1, PDEV_OVERLAY); - if (ret != 0) + if (of_unittest_apply_overlay_check(0, 0, 0, 1, PDEV_OVERLAY)) return; unittest(1, "overlay test %d passed\n", 0); @@ -1393,11 +1390,8 @@ static void __init of_unittest_overlay_0(void) /* test deactivation of device */ static void __init of_unittest_overlay_1(void) { - int ret; - /* device should disable */ - ret = of_unittest_apply_overlay_check(1, 1, 1, 0, PDEV_OVERLAY); - if (ret != 0) + if (of_unittest_apply_overlay_check(1, 1, 1, 0, PDEV_OVERLAY)) return; unittest(1, "overlay test %d passed\n", 1); @@ -1406,11 +1400,8 @@ static void __init of_unittest_overlay_1(void) /* test activation of device */ static void __init of_unittest_overlay_2(void) { - int ret; - /* device should enable */ - ret = of_unittest_apply_overlay_check(2, 2, 0, 1, PDEV_OVERLAY); - if (ret != 0) + if (of_unittest_apply_overlay_check(2, 2, 0, 1, PDEV_OVERLAY)) return; unittest(1, "overlay test %d passed\n", 2); @@ -1419,11 +1410,8 @@ static void __init of_unittest_overlay_2(void) /* test deactivation of device */ static void __init of_unittest_overlay_3(void) { - int ret; - /* device should disable */ - ret = of_unittest_apply_overlay_check(3, 3, 1, 0, PDEV_OVERLAY); - if (ret != 0) + if (of_unittest_apply_overlay_check(3, 3, 1, 0, PDEV_OVERLAY)) return; unittest(1, "overlay test %d passed\n", 3); @@ -1432,11 +1420,8 @@ static void __init of_unittest_overlay_3(void) /* test activation of a full device node */ static void __init of_unittest_overlay_4(void) { - int ret; - /* device should disable */ - ret = of_unittest_apply_overlay_check(4, 4, 0, 1, PDEV_OVERLAY); - if (ret != 0) + if (of_unittest_apply_overlay_check(4, 4, 0, 1, PDEV_OVERLAY)) return; unittest(1, "overlay test %d passed\n", 4); @@ -1445,11 +1430,8 @@ static void __init of_unittest_overlay_4(void) /* test overlay apply/revert sequence */ static void __init of_unittest_overlay_5(void) { - int ret; - /* device should disable */ - ret = of_unittest_apply_revert_overlay_check(5, 5, 0, 1, PDEV_OVERLAY); - if (ret != 0) + if (of_unittest_apply_revert_overlay_check(5, 5, 0, 1, PDEV_OVERLAY)) return; unittest(1, "overlay test %d passed\n", 5); @@ -1458,7 +1440,7 @@ static void __init of_unittest_overlay_5(void) /* test overlay application in sequence */ static void __init of_unittest_overlay_6(void) { - int ret, i, ov_id[2], ovcs_id; + int i, ov_id[2], ovcs_id; int overlay_nr = 6, unittest_nr = 6; int before = 0, after = 1; const char *overlay_name; @@ -1481,8 +1463,7 @@ static void __init of_unittest_overlay_6(void) overlay_name = overlay_name_from_nr(overlay_nr + i); - ret = overlay_data_apply(overlay_name, &ovcs_id); - if (!ret) { + if (!overlay_data_apply(overlay_name, &ovcs_id)) { unittest(0, "could not apply overlay \"%s\"\n", overlay_name); return; @@ -1506,8 +1487,7 @@ static void __init of_unittest_overlay_6(void) for (i = 1; i >= 0; i--) { ovcs_id = ov_id[i]; - ret = of_overlay_remove(&ovcs_id); - if (ret != 0) { + if (of_overlay_remove(&ovcs_id)) { unittest(0, "%s failed destroy @\"%s\"\n", overlay_name_from_nr(ove
[PATCH 2/2] of: unittest: local return value variable related cleanups
From: Frank Rowand Several more style issues became apparent while creating "of: unittest: remove unneeded local return value variables". Correct those issues. Signed-off-by: Frank Rowand --- drivers/of/unittest.c | 22 ++ 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 546483c0be62..7aef8688c365 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -1267,7 +1267,6 @@ static void of_unittest_destroy_tracked_overlays(void) static int __init of_unittest_apply_overlay(int overlay_nr, int unittest_nr, int *overlay_id) { - struct device_node *np = NULL; const char *overlay_name; int ret; @@ -1277,16 +1276,11 @@ static int __init of_unittest_apply_overlay(int overlay_nr, int unittest_nr, if (!ret) { unittest(0, "could not apply overlay \"%s\"\n", overlay_name); - goto out; + return ret; } of_unittest_track_overlay(*overlay_id); - ret = 0; - -out: - of_node_put(np); - - return ret; + return 0; } /* apply an overlay while checking before and after states */ @@ -1582,8 +1576,8 @@ static void __init of_unittest_overlay_10(void) ret = of_path_device_type_exists(child_path, PDEV_OVERLAY); kfree(child_path); - if (unittest(ret, "overlay test %d failed; no child device\n", 10)) - return; + + unittest(ret, "overlay test %d failed; no child device\n", 10); } /* test insertion of a bus with parent devices (and revert) */ @@ -1594,9 +1588,7 @@ static void __init of_unittest_overlay_11(void) /* device should disable */ ret = of_unittest_apply_revert_overlay_check(11, 11, 0, 1, PDEV_OVERLAY); - if (unittest(ret == 0, - "overlay test %d failed; overlay application\n", 11)) - return; + unittest(ret == 0, "overlay test %d failed; overlay apply\n", 11); } #if IS_BUILTIN(CONFIG_I2C) && IS_ENABLED(CONFIG_OF_OVERLAY) @@ -2120,10 +2112,8 @@ static int __init overlay_data_apply(const char *overlay_name, int *overlay_id) } size = info->dtb_end - info->dtb_begin; - if (!size) { + if (!size) pr_err("no overlay data for %s\n", overlay_name); - ret = 0; - } ret = of_overlay_fdt_apply(info->dtb_begin, size, &info->overlay_id); if (overlay_id) -- Frank Rowand
[PATCH v5 2/3] memblock: add memblock_free() alloc when CONFIG_HAVE_MEMBLOCK is not set
From: Frank Rowand When CONFIG_HAVE_MEMBLOCK is not set, an error version of memblock_alloc() exists. Add the matching memblock_free(). Signed-off-by: Frank Rowand --- Andrew or Michal, can you please ack this patch to be accepted by Rob? With "of: add early boot allocation of of_find_node_by_phandle() cache" applied, kbuild test robot reports tilex architecture build error due to no prototype for memblock_free(). The version of the patch that kbuild test robot reported was "[PATCH v4 2/2] of: add early boot allocation of of_find_node_by_phandle() cache". An updated version of that patch is now patch 3/3 of this series. include/linux/memblock.h | 4 1 file changed, 4 insertions(+) diff --git a/include/linux/memblock.h b/include/linux/memblock.h index 8be5077efb5f..d6973b1d92bc 100644 --- a/include/linux/memblock.h +++ b/include/linux/memblock.h @@ -432,6 +432,10 @@ static inline unsigned long memblock_reserved_memory_within(phys_addr_t start_ad return 0; } +static inline int memblock_free(phys_addr_t base, phys_addr_t size) +{ + return 0; +} #endif /* CONFIG_HAVE_MEMBLOCK */ #endif /* __KERNEL__ */ -- Frank Rowand
[PATCH v5 1/3] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()
From: Frank Rowand Create a cache of the nodes that contain a phandle property. Use this cache to find the node for a given phandle value instead of scanning the devicetree to find the node. If the phandle value is not found in the cache, of_find_node_by_phandle() will fall back to the tree scan algorithm. The cache is initialized in of_core_init(). The cache is freed via a late_initcall_sync() if modules are not enabled. If the devicetree is created by the dtc compiler, with all phandle property values auto generated, then the size required by the cache could be 4 * (1 + number of phandles) bytes. This results in an O(1) node lookup cost for a given phandle value. Due to a concern that the phandle property values might not be consistent with what is generated by the dtc compiler, a mask has been added to the cache lookup algorithm. To maintain the O(1) node lookup cost, the size of the cache has been increased by rounding the number of entries up to the next power of two. The overhead of finding the devicetree node containing a given phandle value has been noted by several people in the recent past, in some cases with a patch to add a hashed index of devicetree nodes, based on the phandle value of the node. One concern with this approach is the extra space added to each node. This patch takes advantage of the phandle property values auto generated by the dtc compiler, which begin with one and monotonically increase by one, resulting in a range of 1..n for n phandle values. This implementation should also provide a good reduction of overhead for any range of phandle values that are mostly in a monotonic range. Performance measurements by Chintan Pandya of several implementations of patches that are similar to this one suggest an expected reduction of boot time by ~400ms for his test system. If the cache size was decreased to 64 entries, the boot time was reduced by ~340 ms. The measurements were on a 4.9.73 kernel for arch/arm64/boot/dts/qcom/sda670-mtp.dts, contains 2371 nodes and 814 phandle values. Reported-by: Chintan Pandya Signed-off-by: Frank Rowand --- Changes since v3: - of_populate_phandle_cache(): add check for failed memory allocation Changes since v2: - add mask to calculation of phandle cache entry - which results in better overhead reduction for devicetrees with phandle properties not allocated in the monotonically increasing range of 1..n - due to mask, number of entries in cache potentially increased to next power of two - minor fixes as suggested by reviewers - no longer using live_tree_max_phandle() so do not move it from drivers/of/resolver.c to drivers/of/base.c Changes since v1: - change short description from of: cache phandle nodes to reduce cost of of_find_node_by_phandle() - rebase on v4.16-rc1 - reorder new functions in base.c to avoid forward declaration - add locking around kfree(phandle_cache) for memory ordering - add explicit check for non-null of phandle_cache in of_find_node_by_phandle(). There is already a check for !handle, which prevents accessing a null phandle_cache, but that dependency is not obvious, so this check makes it more apparent. - do not free phandle_cache if modules are enabled, so that cached phandles will be available when modules are loaded drivers/of/base.c | 86 ++--- drivers/of/of_private.h | 3 ++ drivers/of/resolver.c | 3 -- 3 files changed, 85 insertions(+), 7 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index ad28de96e13f..e71d157d7149 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -91,10 +91,72 @@ int __weak of_node_to_nid(struct device_node *np) } #endif +static struct device_node **phandle_cache; +static u32 phandle_cache_mask; + +/* + * Assumptions behind phandle_cache implementation: + * - phandle property values are in a contiguous range of 1..n + * + * If the assumptions do not hold, then + * - the phandle lookup overhead reduction provided by the cache + * will likely be less + */ +static void of_populate_phandle_cache(void) +{ + unsigned long flags; + u32 cache_entries; + struct device_node *np; + u32 phandles = 0; + + raw_spin_lock_irqsave(&devtree_lock, flags); + + kfree(phandle_cache); + phandle_cache = NULL; + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandles++; + + cache_entries = roundup_pow_of_two(phandles); + phandle_cache_mask = cache_entries - 1; + + phandle_cache = kcalloc(cache_entries, sizeof(*phandle_cache), + GFP_ATOMIC); + if (!phandle_cache) + goto out; + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandle_cache[np->phandle & phandle_cache_mask] = np; + +out: +
[PATCH v5 3/3] of: add early boot allocation of of_find_node_by_phandle() cache
From: Frank Rowand The initial implementation of the of_find_node_by_phandle() cache allocates the cache using kcalloc(). Add an early boot allocation of the cache so it will be usable during early boot. Switch over to the kcalloc() based cache once normal memory allocation becomes available. Signed-off-by: Frank Rowand --- Changes from v4: - Add checks for memblock_virt_alloc() returning zero. Changes from previous versions: - no changes Version 5 adds checks for memblock_virt_alloc() returning zero. kernel test robot reports 10 out of 10 boot failures on qemu-system-x86_64 with "[PATCH v4 2/2] of: add early boot allocation of of_find_node_by_phandle() cache" applied. The failure appears to me to be due to memblock_virt_alloc() returning zero. kernel BUG at arch/x86/mm/physaddr.c:27 invalid opcode: [#1] PREEMPT PTI CPU: 0 PID: 1 Comm: swapper Not tainted 4.16.0-rc1-8-gb013aa4 #1 RIP: 0010:__phys_addr+0x39/0x50 RSP: :880018de3ee0 EFLAGS: 00010087 RAX: 7800 RBX: 0001 RCX: 0002 RDX: 8000 RSI: 82e77239 RDI: RBP: 880018de3ee0 R08: R09: 0001 R10: 29cb R11: 63561fc2891644ad R12: 0286 R13: R14: R15: FS: () GS:8309b000() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: CR3: 03074000 CR4: 06f0 Call Trace: of_core_init+0x30/0x21f driver_init+0x36/0x38 kernel_init_freeable+0x82/0x19f ? rest_init+0x220/0x220 kernel_init+0xe/0x100 ret_from_fork+0x24/0x30 I'm not conversent in the x86 dialect of assembly, but it looks like phandle_cache has the value zero, when calling __phys_addr(): the call from of_core_init() to __pa(phandle_cache) aka __physaddr(): 186 size = (phandle_cache_mask + 1) * sizeof(*phandle_cache); 0x83334d35 <+27>:mov0x1771e1d(%rip),%eax# 0x84aa6b58 0x83334d42 <+40>:lea0x1(%rax),%r12d 0x83334d4b <+49>:shl$0x3,%r12 187 memblock_free(__pa(phandle_cache), size); 0x83334d3b <+33>:mov0x1771e1e(%rip),%rdi# 0x84aa6b60 0x83334d46 <+44>:callq 0x81049ad0 <__phys_addr> 0x83334d4f <+53>:mov%rax,%rdi 0x83334d52 <+56>:mov%r12,%rsi 0x83334d55 <+59>:callq 0x8334e45b 188 phandle_cache = NULL; 0x83334d64 <+74>:movq $0x0,0x1771df1(%rip)# 0x84aa6b60 __pa(): (gdb) disass /m __phys_addr Dump of assembler code for function __phys_addr: 15 { 0x81049ad0 <+0>: callq 0x822018e0 <__fentry__> 0x81049ad5 <+5>: push %rbp 0x81049ade <+14>:mov%rsp,%rbp 16 unsigned long y = x - __START_KERNEL_map; 0x81049ad6 <+6>: mov$0x8000,%eax 17 18 /* use the carry flag to determine if x was < __START_KERNEL_map */ 19 if (unlikely(x > y)) { 0x81049adb <+11>:add%rdi,%rax 0x81049ae1 <+17>:mov%rax,%rdx 0x81049ae4 <+20>:jae0x81049af8 <__phys_addr+40> 20 x = y + phys_base; 0x81049ae6 <+22>:add0x1e30523(%rip),%rax# 0x82e7a010 21 22 VIRTUAL_BUG_ON(y >= KERNEL_IMAGE_SIZE); 0x81049aed <+29>:cmp$0x1fff,%rdx 0x81049af4 <+36>:jbe0x81049b1e <__phys_addr+78> 0x81049af6 <+38>:ud2 23 } else { 24 x = y + (__START_KERNEL_map - PAGE_OFFSET); 0x81049af8 <+40>:movabs $0x7800,%rax 0x81049b02 <+50>:add%rdi,%rax 25 26 /* carry flag will be set if starting x was >= PAGE_OFFSET */ 27 VIRTUAL_BUG_ON((x > y) || !phys_addr_valid(x)); 0x81049b05 <+53>:cmp%rax,%rdx 0x81049b08 <+56>:jb 0x81049b1c <__phys_addr+76> 0x81049b17 <+71>:test %rdx,%rdx 0x81049b1a <+74>:je 0x81049b1e <__phys_addr+78> 0x81049b1c <+76>:ud2 28 } 29 30 return x; 31 } 0x81049b1e <+78>:pop%rbp 0x81049b1f <+79>:retq drivers/of/base.c | 38 ++ drivers/of/fdt.c| 2 ++ drivers/of/of_private.h | 2 ++ 3 files changed, 42 insertions(+) diff --git a/drivers/of/base.c b/drivers/of/base.c index e71d157d7149..48714d8e0bc9 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -16,9 +16,11 @@ #define pr_fmt(fmt)"OF: " fmt +#include #include #include #include +#include #include #include #include @@ -134,6 +136,32 @@ static void of_popul
[PATCH v5 0/3] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()
From: Frank Rowand Create a cache of the nodes that contain a phandle property. Use this cache to find the node for a given phandle value instead of scanning the devicetree to find the node. If the phandle value is not found in the cache, of_find_node_by_phandle() will fall back to the tree scan algorithm. Size and performance data is in patch 1/3 comments Changes since v4: - Add patch 2/3 "memblock: add memblock_free() alloc when CONFIG_HAVE_MEMBLOCK is not set" - patch 3/3 (previously 2/2): add checks for zero returned by memblock_virt_alloc() Changes since v3: - of_populate_phandle_cache(): add check for failed memory allocation - add patch 2/2 into series instead of as a standalone patch that was dependent on patch 1/2 of this series Changes since v2: - add mask to calculation of phandle cache entry - which results in better overhead reduction for devicetrees with phandle properties not allocated in the monotonically increasing range of 1..n - due to mask, number of entries in cache potentially increased to next power of two - minor fixes as suggested by reviewers - no longer using live_tree_max_phandle() so do not move it from drivers/of/resolver.c to drivers/of/base.c Changes since v1: - change short description from of: cache phandle nodes to reduce cost of of_find_node_by_phandle() - rebase on v4.16-rc1 - reorder new functions in base.c to avoid forward declaration - add locking around kfree(phandle_cache) for memory ordering - add explicit check for non-null of phandle_cache in of_find_node_by_phandle(). There is already a check for !handle, which prevents accessing a null phandle_cache, but that dependency is not obvious, so this check makes it more apparent. - do not free phandle_cache if modules are enabled, so that cached phandles will be available when modules are loaded Frank Rowand (3): of: cache phandle nodes to reduce cost of of_find_node_by_phandle() memblock: add memblock_free() alloc when CONFIG_HAVE_MEMBLOCK is not set of: add early boot allocation of of_find_node_by_phandle() cache drivers/of/base.c| 124 +-- drivers/of/fdt.c | 2 + drivers/of/of_private.h | 5 ++ drivers/of/resolver.c| 3 -- include/linux/memblock.h | 4 ++ 5 files changed, 131 insertions(+), 7 deletions(-) -- Frank Rowand
[PATCH v7 2/5] of: change overlay apply input data from unflattened to FDT
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. of_overlay_fdt_apply() is a private function for the anticipated overlay loader. Update unittest.c to use of_overlay_fdt_apply() instead of of_overlay_apply(). Move overlay fragments from artificial locations in drivers/of/unittest-data/tests-overlay.dtsi into one devicetree source file per overlay. This led to changes in drivers/of/unitest-data/Makefile and drivers/of/unitest.c. - Add overlay directives to the overlay devicetree source files so that dtc will compile them as true overlays into one FDT data chunk per overlay. - Set CFLAGS for drivers/of/unittest-data/testcases.dts so that symbols will be generated for overlay resolution of overlays that are no longer artificially contained in testcases.dts - Unflatten and apply each unittest overlay FDT using of_overlay_fdt_apply(). - Enable the of_resolve_phandles() check for whether the unflattened overlay is detached. This check was previously disabled because the overlays from tests-overlay.dtsi were not unflattened into detached trees. - Other changes to unittest.c infrastructure to manage multiple test FDTs built into the kernel image (access by name instead of arbitrary number). - of_unittest_overlay_high_level(): previously unused code to add properties from the overlay_base devicetree to the live tree was triggered by the restructuring of tests-overlay.dtsi and thus testcases.dts. This exposed two bugs: (1) the need to dup a property before adding it, and (2) property 'name' is auto-generated in the unflatten code and thus will be a duplicate in the __symbols__ node - do not treat this duplicate as an error. Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are "line over 80 characters" for either pre-existing long lines, or lines in devicetree source files. Rob, I have added making unittest a loadable module to my todo list. Changes from v5: - Add __init to of_unittest_apply_revert_overlay_check(), of_unittest_overlay_5(), and of_unittest_overlay_11() to fix compile warnings reported by Geert Changes from v4: - move of_overlay_fdt_apply() prototype from of_private.h to of.h - of_overlay_apply(): add kfree(fdt) if resolve phandles or initialize changeset fail - of_overlay_fdt_apply(): add parameter size of overlay_fdt and use it for some additional validation of the overlay fdt - update unittest.c with additional parameter to of_overlay_fdt_apply() Changes from v3: - OF_OVERLAY: add select OF_FLATTREE Changes from v1: - rebase on v4.16-rc1 drivers/of/Kconfig | 1 + drivers/of/overlay.c| 112 ++- drivers/of/resolver.c | 6 - drivers/of/unittest-data/Makefile | 28 ++- drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 34 drivers/of/unittest-data/overlay_11.dts | 34 drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 35 drivers/of/unittest-data/overlay_2.dts | 14 ++ drivers/of/unittest-data/overlay_3.dts | 14 ++ drivers/of/unittest-data/overlay_4.dts | 23 +++ drivers/of/unittest-data/overlay_5.dts | 14 ++ drivers/of/unittest-data/overlay_6.dts | 15 ++ drivers/of/unittest-data/overlay_7.dts | 15 ++ drivers/of/unittest-data/overlay_8.dts | 15 ++ drivers/of/unittest-data/overlay_9.dts | 15 ++ drivers/of/unittest-data/tests-overlay.dtsi | 213 drivers/of/unittest.c | 300 ++-- include/linux/of.h | 6 +- 22 files changed, 562 insertions(+), 388 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts
[PATCH v7 3/5] of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply()
From: Frank Rowand Signed-off-by: Frank Rowand --- Documentation/devicetree/overlay-notes.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt index c4aa0adf13ec..5175a24d387e 100644 --- a/Documentation/devicetree/overlay-notes.txt +++ b/Documentation/devicetree/overlay-notes.txt @@ -87,8 +87,8 @@ Overlay in-kernel API The API is quite easy to use. -1. Call of_overlay_apply() to create and apply an overlay changeset. The return -value is an error or a cookie identifying this overlay. +1. Call of_overlay_fdt_apply() to create and apply an overlay changeset. The +return value is an error or a cookie identifying this overlay. 2. Call of_overlay_remove() to remove and cleanup the overlay changeset previously created via the call to of_overlay_apply(). Removal of an overlay -- Frank Rowand
[PATCH v7 4/5] of: convert unittest overlay devicetree source to sugar syntax
From: Frank Rowand The unittest-data overlays have been pulled into proper overlay devicetree source files without changing their format. The next step is to convert them to use sugar syntax instead of hand coding overlay fragments structure. A few of the overlays can not be converted because they test absolute target paths in the overlay fragment. dtc does not generate this type of target: overlay_0.dts overlay_1.dts overlay_12.dts overlay_13.dts Two pre-existing unittest overlay devicetree source files are also converted: overlay_bad_phandle.dts overlay_bad_symbol.dts Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are pre-existing warnings of un-documented bindings of made up (fake) compatibles in devicetree source for test overlays. drivers/of/unittest-data/overlay.dts | 101 ++- drivers/of/unittest-data/overlay_10.dts | 39 - drivers/of/unittest-data/overlay_11.dts | 40 - drivers/of/unittest-data/overlay_15.dts | 41 - drivers/of/unittest-data/overlay_2.dts | 11 +-- drivers/of/unittest-data/overlay_3.dts | 11 +-- drivers/of/unittest-data/overlay_4.dts | 23 ++ drivers/of/unittest-data/overlay_5.dts | 11 +-- drivers/of/unittest-data/overlay_6.dts | 13 +-- drivers/of/unittest-data/overlay_7.dts | 13 +-- drivers/of/unittest-data/overlay_8.dts | 13 +-- drivers/of/unittest-data/overlay_9.dts | 13 +-- drivers/of/unittest-data/overlay_bad_phandle.dts | 23 ++ drivers/of/unittest-data/overlay_bad_symbol.dts | 25 ++ drivers/of/unittest-data/tests-overlay.dtsi | 4 +- 15 files changed, 148 insertions(+), 233 deletions(-) diff --git a/drivers/of/unittest-data/overlay.dts b/drivers/of/unittest-data/overlay.dts index ab5e89b5e27e..3bbc59e922fe 100644 --- a/drivers/of/unittest-data/overlay.dts +++ b/drivers/of/unittest-data/overlay.dts @@ -2,76 +2,63 @@ /dts-v1/; /plugin/; -/ { +&electric_1 { - fragment@0 { - target = <&electric_1>; + status = "okay"; - __overlay__ { - status = "okay"; - - hvac_2: hvac-large-1 { - compatible = "ot,hvac-large"; - heat-range = < 40 75 >; - cool-range = < 65 80 >; - }; - }; + hvac_2: hvac-large-1 { + compatible = "ot,hvac-large"; + heat-range = < 40 75 >; + cool-range = < 65 80 >; }; +}; - fragment@1 { - target = <&rides_1>; - - __overlay__ { - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - - ride@100 { - #address-cells = <1>; - #size-cells = <1>; - - track@30 { - incline-up = < 48 32 16 >; - }; +&rides_1 { - track@40 { - incline-up = < 47 31 15 >; - }; - }; + #address-cells = <1>; + #size-cells = <1>; + status = "okay"; - ride_200: ride@200 { - #address-cells = <1>; - #size-cells = <1>; - compatible = "ot,ferris-wheel"; - reg = < 0x0200 0x100 >; - hvac-provider = < &hvac_2 >; - hvac-thermostat = < 27 32 > ; - hvac-zones = < 12 5 >; - hvac-zone-names = "operator", "snack-bar"; - spin-controller = < &spin_ctrl_1 3 >; - spin-rph = < 30 >; - gondolas = < 16 >; - gondola-capacity = < 6 >; + ride@100 { + #address-cells = <1>; + #size-cells = <1>; - ride_200_left: track@10 { - reg = < 0x0010 0x10 >; - }; + track@30 { + incline-up = < 48 32 16 >; + }; - ride_200_right: track@20 { - reg = < 0x0020 0x10 >; - }; - }; + track@40 { + incline-up = < 47 31 15 >; }; }; - fragment@2 { - target = <&lights_2>; + ride_200: ride@200
[PATCH v7 0/5] of: change overlay apply input data from unflattened
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. These changes led to migrating some unittest overlay data into their own devicetree source files, and then converting most of them to use sugar syntax instead of hand coding fragments. Changes from v6: - patches 2-5 were previously patches 1-4 - patch 1/5 previously submitted separately as: "x86: devicetree: fix config option around x86_flattree_get_config()" https://lkml.org/lkml/2018/3/2/1148 Changes from v5: - Add __init to of_unittest_apply_revert_overlay_check(), of_unittest_overlay_5(), and of_unittest_overlay_11() to fix compile warnings reported by Geert Changes from v4: (all in patch 1/4) - move of_overlay_fdt_apply() prototype from of_private.h to of.h - of_overlay_apply(): add kfree(fdt) if resolve phandles or initialize changeset fail - of_overlay_fdt_apply(): add parameter size of overlay_fdt and use it for some additional validation of the overlay fdt - update unittest.c with additional parameter to of_overlay_fdt_apply() Changes from v3: - patch 1/4: OF_OVERLAY: add select OF_FLATTREE Changes from v2: - improve error messages in patch 4/4, as suggested by Geert Changes from v1: - rebase on v4.16-rc1 - update documentation - split out error message to a separate patch Frank Rowand (5): x86: devicetree: fix config option around x86_flattree_get_config() of: change overlay apply input data from unflattened to FDT of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply() of: convert unittest overlay devicetree source to sugar syntax of: improve reporting invalid overlay target path Documentation/devicetree/overlay-notes.txt | 4 +- arch/x86/kernel/devicetree.c | 2 +- drivers/of/Kconfig | 1 + drivers/of/overlay.c | 134 -- drivers/of/resolver.c| 6 - drivers/of/unittest-data/Makefile| 28 ++- drivers/of/unittest-data/overlay.dts | 101 drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 27 ++ drivers/of/unittest-data/overlay_11.dts | 28 +++ drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 30 +++ drivers/of/unittest-data/overlay_2.dts | 9 + drivers/of/unittest-data/overlay_3.dts | 9 + drivers/of/unittest-data/overlay_4.dts | 18 ++ drivers/of/unittest-data/overlay_5.dts | 9 + drivers/of/unittest-data/overlay_6.dts | 10 + drivers/of/unittest-data/overlay_7.dts | 10 + drivers/of/unittest-data/overlay_8.dts | 10 + drivers/of/unittest-data/overlay_9.dts | 10 + drivers/of/unittest-data/overlay_bad_phandle.dts | 23 +- drivers/of/unittest-data/overlay_bad_symbol.dts | 25 +- drivers/of/unittest-data/tests-overlay.dtsi | 217 +--- drivers/of/unittest.c| 300 +++ include/linux/of.h | 6 +- 27 files changed, 586 insertions(+), 487 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts create mode 100644 drivers/of/unittest-data/overlay_15.dts create mode 100644 drivers/of/unittest-data/overlay_2.dts create mode 100644 drivers/of/unittest-data/overlay_3.dts create mode 100644 drivers/of/unittest-data/overlay_4.dts create mode 100644 drivers/of/unittest-data/overlay_5.dts create mode 100644 drivers/of/unittest-data/overlay_6.dts create mode 100644 drivers/of/unittest-data/overlay_7.dts create mode 100644 drivers/of/unittest-data/overlay_8.dts create mode 100644 drivers/of/unittest-data/overlay_9.dts -- Frank Rowand
[PATCH v7 1/5] x86: devicetree: fix config option around x86_flattree_get_config()
From: Frank Rowand x86_flattree_get_config() is incorrectly protected by ifdef CONFIG_OF_FLATTREE. It uses of_get_flat_dt_size(), which only exists if CONFIG_OF_EARLY_FLATTREE. This issue has not been exposed previously because OF_FLATTREE did not occur unless it was selected by OF_EARLY_FLATTREE. A devicetree overlay change is selecting OF_FLATTREE directly instead of indirectly enabling it by selecting OF_EARLY_FLATTREE. This problem was exposed by a randconfig generated by the kbuild test robot, where Platform OLPC was enabled. OLPC selects OF_PROMTREE instead of OF_EARLY_FLATREE. The only other x86 platform that selects OF is X86_INTEL_CE, which does select OF_EARLY_FLATTREE. Signed-off-by: Frank Rowand --- x86 Maintainers, please ack this patch for Rob to accept This patch previously submitted separately as: "x86: devicetree: fix config option around x86_flattree_get_config()" https://lkml.org/lkml/2018/3/2/1148 so this is a jump from version 1 to version 6. No changes from version 1. arch/x86/kernel/devicetree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c index 25de5f6ca997..45416826f6ee 100644 --- a/arch/x86/kernel/devicetree.c +++ b/arch/x86/kernel/devicetree.c @@ -259,7 +259,7 @@ static void __init dtb_apic_setup(void) dtb_ioapic_setup(); } -#ifdef CONFIG_OF_FLATTREE +#ifdef CONFIG_OF_EARLY_FLATTREE static void __init x86_flattree_get_config(void) { u32 size, map_len; -- Frank Rowand
[PATCH v7 5/5] of: improve reporting invalid overlay target path
From: Frank Rowand Errors while developing the patch to create of_overlay_fdt_apply() exposed inadequate error messages to debug problems when overlay devicetree fragment nodes contain an invalid target path. Improve the messages in find_target_node() to remedy this. Signed-off-by: Frank Rowand --- changes from v2: - add fragment node name as suggested by Geert - existing error message printed short node name, thus not discriminating between fragments; change to full node name - existing error message printed node address, which is devicetree internal debugging, not useful in an overlay apply error message; remove node address from message drivers/of/overlay.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index e3d7f69a8333..b930e05d1215 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -488,20 +488,30 @@ static int build_changeset(struct overlay_changeset *ovcs) */ static struct device_node *find_target_node(struct device_node *info_node) { + struct device_node *node; const char *path; u32 val; int ret; ret = of_property_read_u32(info_node, "target", &val); - if (!ret) - return of_find_node_by_phandle(val); + if (!ret) { + node = of_find_node_by_phandle(val); + if (!node) + pr_err("find target, node: %pOF, phandle 0x%x not found\n", + info_node, val); + return node; + } ret = of_property_read_string(info_node, "target-path", &path); - if (!ret) - return of_find_node_by_path(path); + if (!ret) { + node = of_find_node_by_path(path); + if (!node) + pr_err("find target, node: %pOF, path '%s' not found\n", + info_node, path); + return node; + } - pr_err("Failed to find target for node %p (%s)\n", - info_node, info_node->name); + pr_err("find target, node: %pOF, no target property\n", info_node); return NULL; } -- Frank Rowand
[PATCH] x86: devicetree: fix config option around x86_flattree_get_config()
From: Frank Rowand x86_flattree_get_config() is incorrectly protected by ifdef CONFIG_OF_FLATTREE. It uses of_get_flat_dt_size(), which only exists if CONFIG_OF_EARLY_FLATTREE. This issue has not been exposed previously because OF_FLATTREE did not occur unless it was selected by OF_EARLY_FLATTREE. A devicetree overlay change is selecting OF_FLATTREE directly instead of indirectly enabling it by selecting OF_EARLY_FLATTREE. This problem was exposed by a randconfig generated by the kbuild test robot, where Platform OLPC was enabled. OLPC selects OF_PROMTREE instead of OF_EARLY_FLATREE. The only other x86 platform that selects OF is X86_INTEL_CE, which does select OF_EARLY_FLATTREE. Signed-off-by: Frank Rowand --- The devicetree overlay change is in patch 1/4 of the series: https://lkml.org/lkml/2018/3/2/979 The pull request for the patch series that triggered the kbuild test robot is: https://lkml.org/lkml/2018/3/2/1065 arch/x86/kernel/devicetree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c index 25de5f6ca997..45416826f6ee 100644 --- a/arch/x86/kernel/devicetree.c +++ b/arch/x86/kernel/devicetree.c @@ -259,7 +259,7 @@ static void __init dtb_apic_setup(void) dtb_ioapic_setup(); } -#ifdef CONFIG_OF_FLATTREE +#ifdef CONFIG_OF_EARLY_FLATTREE static void __init x86_flattree_get_config(void) { u32 size, map_len; -- Frank Rowand
[PATCH v6 4/4] of: improve reporting invalid overlay target path
From: Frank Rowand Errors while developing the patch to create of_overlay_fdt_apply() exposed inadequate error messages to debug problems when overlay devicetree fragment nodes contain an invalid target path. Improve the messages in find_target_node() to remedy this. Signed-off-by: Frank Rowand --- changes from v2: - add fragment node name as suggested by Geert - existing error message printed short node name, thus not discriminating between fragments; change to full node name - existing error message printed node address, which is devicetree internal debugging, not useful in an overlay apply error message; remove node address from message drivers/of/overlay.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index e3d7f69a8333..b930e05d1215 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -488,20 +488,30 @@ static int build_changeset(struct overlay_changeset *ovcs) */ static struct device_node *find_target_node(struct device_node *info_node) { + struct device_node *node; const char *path; u32 val; int ret; ret = of_property_read_u32(info_node, "target", &val); - if (!ret) - return of_find_node_by_phandle(val); + if (!ret) { + node = of_find_node_by_phandle(val); + if (!node) + pr_err("find target, node: %pOF, phandle 0x%x not found\n", + info_node, val); + return node; + } ret = of_property_read_string(info_node, "target-path", &path); - if (!ret) - return of_find_node_by_path(path); + if (!ret) { + node = of_find_node_by_path(path); + if (!node) + pr_err("find target, node: %pOF, path '%s' not found\n", + info_node, path); + return node; + } - pr_err("Failed to find target for node %p (%s)\n", - info_node, info_node->name); + pr_err("find target, node: %pOF, no target property\n", info_node); return NULL; } -- Frank Rowand
[PATCH v6 3/4] of: convert unittest overlay devicetree source to sugar syntax
From: Frank Rowand The unittest-data overlays have been pulled into proper overlay devicetree source files without changing their format. The next step is to convert them to use sugar syntax instead of hand coding overlay fragments structure. A few of the overlays can not be converted because they test absolute target paths in the overlay fragment. dtc does not generate this type of target: overlay_0.dts overlay_1.dts overlay_12.dts overlay_13.dts Two pre-existing unittest overlay devicetree source files are also converted: overlay_bad_phandle.dts overlay_bad_symbol.dts Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are pre-existing warnings of un-documented bindings of made up (fake) compatibles in devicetree source for test overlays. drivers/of/unittest-data/overlay.dts | 101 ++- drivers/of/unittest-data/overlay_10.dts | 39 - drivers/of/unittest-data/overlay_11.dts | 40 - drivers/of/unittest-data/overlay_15.dts | 41 - drivers/of/unittest-data/overlay_2.dts | 11 +-- drivers/of/unittest-data/overlay_3.dts | 11 +-- drivers/of/unittest-data/overlay_4.dts | 23 ++ drivers/of/unittest-data/overlay_5.dts | 11 +-- drivers/of/unittest-data/overlay_6.dts | 13 +-- drivers/of/unittest-data/overlay_7.dts | 13 +-- drivers/of/unittest-data/overlay_8.dts | 13 +-- drivers/of/unittest-data/overlay_9.dts | 13 +-- drivers/of/unittest-data/overlay_bad_phandle.dts | 23 ++ drivers/of/unittest-data/overlay_bad_symbol.dts | 25 ++ drivers/of/unittest-data/tests-overlay.dtsi | 4 +- 15 files changed, 148 insertions(+), 233 deletions(-) diff --git a/drivers/of/unittest-data/overlay.dts b/drivers/of/unittest-data/overlay.dts index ab5e89b5e27e..3bbc59e922fe 100644 --- a/drivers/of/unittest-data/overlay.dts +++ b/drivers/of/unittest-data/overlay.dts @@ -2,76 +2,63 @@ /dts-v1/; /plugin/; -/ { +&electric_1 { - fragment@0 { - target = <&electric_1>; + status = "okay"; - __overlay__ { - status = "okay"; - - hvac_2: hvac-large-1 { - compatible = "ot,hvac-large"; - heat-range = < 40 75 >; - cool-range = < 65 80 >; - }; - }; + hvac_2: hvac-large-1 { + compatible = "ot,hvac-large"; + heat-range = < 40 75 >; + cool-range = < 65 80 >; }; +}; - fragment@1 { - target = <&rides_1>; - - __overlay__ { - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - - ride@100 { - #address-cells = <1>; - #size-cells = <1>; - - track@30 { - incline-up = < 48 32 16 >; - }; +&rides_1 { - track@40 { - incline-up = < 47 31 15 >; - }; - }; + #address-cells = <1>; + #size-cells = <1>; + status = "okay"; - ride_200: ride@200 { - #address-cells = <1>; - #size-cells = <1>; - compatible = "ot,ferris-wheel"; - reg = < 0x0200 0x100 >; - hvac-provider = < &hvac_2 >; - hvac-thermostat = < 27 32 > ; - hvac-zones = < 12 5 >; - hvac-zone-names = "operator", "snack-bar"; - spin-controller = < &spin_ctrl_1 3 >; - spin-rph = < 30 >; - gondolas = < 16 >; - gondola-capacity = < 6 >; + ride@100 { + #address-cells = <1>; + #size-cells = <1>; - ride_200_left: track@10 { - reg = < 0x0010 0x10 >; - }; + track@30 { + incline-up = < 48 32 16 >; + }; - ride_200_right: track@20 { - reg = < 0x0020 0x10 >; - }; - }; + track@40 { + incline-up = < 47 31 15 >; }; }; - fragment@2 { - target = <&lights_2>; + ride_200: ride@200
[PATCH v6 0/4] of: change overlay apply input data from unflattened
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. These changes led to migrating some unittest overlay data into their own devicetree source files, and then converting most of them to use sugar syntax instead of hand coding fragments. Changes from v5: - Add __init to of_unittest_apply_revert_overlay_check(), of_unittest_overlay_5(), and of_unittest_overlay_11() to fix compile warnings reported by Geert Changes from v4: (all in patch 1/4) - move of_overlay_fdt_apply() prototype from of_private.h to of.h - of_overlay_apply(): add kfree(fdt) if resolve phandles or initialize changeset fail - of_overlay_fdt_apply(): add parameter size of overlay_fdt and use it for some additional validation of the overlay fdt - update unittest.c with additional parameter to of_overlay_fdt_apply() Changes from v3: - patch 1/4: OF_OVERLAY: add select OF_FLATTREE Changes from v2: - improve error messages in patch 4/4, as suggested by Geert Changes from v1: - rebase on v4.16-rc1 - update documentation - split out error message to a separate patch Frank Rowand (4): of: change overlay apply input data from unflattened to FDT of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply() of: convert unittest overlay devicetree source to sugar syntax of: improve reporting invalid overlay target path Documentation/devicetree/overlay-notes.txt | 4 +- drivers/of/Kconfig | 1 + drivers/of/overlay.c | 134 -- drivers/of/resolver.c| 6 - drivers/of/unittest-data/Makefile| 28 ++- drivers/of/unittest-data/overlay.dts | 101 drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 27 ++ drivers/of/unittest-data/overlay_11.dts | 28 +++ drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 30 +++ drivers/of/unittest-data/overlay_2.dts | 9 + drivers/of/unittest-data/overlay_3.dts | 9 + drivers/of/unittest-data/overlay_4.dts | 18 ++ drivers/of/unittest-data/overlay_5.dts | 9 + drivers/of/unittest-data/overlay_6.dts | 10 + drivers/of/unittest-data/overlay_7.dts | 10 + drivers/of/unittest-data/overlay_8.dts | 10 + drivers/of/unittest-data/overlay_9.dts | 10 + drivers/of/unittest-data/overlay_bad_phandle.dts | 23 +- drivers/of/unittest-data/overlay_bad_symbol.dts | 25 +- drivers/of/unittest-data/tests-overlay.dtsi | 217 +--- drivers/of/unittest.c| 300 +++ include/linux/of.h | 6 +- 26 files changed, 585 insertions(+), 486 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts create mode 100644 drivers/of/unittest-data/overlay_15.dts create mode 100644 drivers/of/unittest-data/overlay_2.dts create mode 100644 drivers/of/unittest-data/overlay_3.dts create mode 100644 drivers/of/unittest-data/overlay_4.dts create mode 100644 drivers/of/unittest-data/overlay_5.dts create mode 100644 drivers/of/unittest-data/overlay_6.dts create mode 100644 drivers/of/unittest-data/overlay_7.dts create mode 100644 drivers/of/unittest-data/overlay_8.dts create mode 100644 drivers/of/unittest-data/overlay_9.dts -- Frank Rowand
[PATCH v6 1/4] of: change overlay apply input data from unflattened to FDT
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. of_overlay_fdt_apply() is a private function for the anticipated overlay loader. Update unittest.c to use of_overlay_fdt_apply() instead of of_overlay_apply(). Move overlay fragments from artificial locations in drivers/of/unittest-data/tests-overlay.dtsi into one devicetree source file per overlay. This led to changes in drivers/of/unitest-data/Makefile and drivers/of/unitest.c. - Add overlay directives to the overlay devicetree source files so that dtc will compile them as true overlays into one FDT data chunk per overlay. - Set CFLAGS for drivers/of/unittest-data/testcases.dts so that symbols will be generated for overlay resolution of overlays that are no longer artificially contained in testcases.dts - Unflatten and apply each unittest overlay FDT using of_overlay_fdt_apply(). - Enable the of_resolve_phandles() check for whether the unflattened overlay is detached. This check was previously disabled because the overlays from tests-overlay.dtsi were not unflattened into detached trees. - Other changes to unittest.c infrastructure to manage multiple test FDTs built into the kernel image (access by name instead of arbitrary number). - of_unittest_overlay_high_level(): previously unused code to add properties from the overlay_base devicetree to the live tree was triggered by the restructuring of tests-overlay.dtsi and thus testcases.dts. This exposed two bugs: (1) the need to dup a property before adding it, and (2) property 'name' is auto-generated in the unflatten code and thus will be a duplicate in the __symbols__ node - do not treat this duplicate as an error. Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are "line over 80 characters" for either pre-existing long lines, or lines in devicetree source files. Rob, I have added making unittest a loadable module to my todo list. Changes from v5: - Add __init to of_unittest_apply_revert_overlay_check(), of_unittest_overlay_5(), and of_unittest_overlay_11() to fix compile warnings reported by Geert Changes from v4: - move of_overlay_fdt_apply() prototype from of_private.h to of.h - of_overlay_apply(): add kfree(fdt) if resolve phandles or initialize changeset fail - of_overlay_fdt_apply(): add parameter size of overlay_fdt and use it for some additional validation of the overlay fdt - update unittest.c with additional parameter to of_overlay_fdt_apply() Changes from v3: - OF_OVERLAY: add select OF_FLATTREE Changes from v1: - rebase on v4.16-rc1 drivers/of/Kconfig | 1 + drivers/of/overlay.c| 112 ++- drivers/of/resolver.c | 6 - drivers/of/unittest-data/Makefile | 28 ++- drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 34 drivers/of/unittest-data/overlay_11.dts | 34 drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 35 drivers/of/unittest-data/overlay_2.dts | 14 ++ drivers/of/unittest-data/overlay_3.dts | 14 ++ drivers/of/unittest-data/overlay_4.dts | 23 +++ drivers/of/unittest-data/overlay_5.dts | 14 ++ drivers/of/unittest-data/overlay_6.dts | 15 ++ drivers/of/unittest-data/overlay_7.dts | 15 ++ drivers/of/unittest-data/overlay_8.dts | 15 ++ drivers/of/unittest-data/overlay_9.dts | 15 ++ drivers/of/unittest-data/tests-overlay.dtsi | 213 drivers/of/unittest.c | 300 ++-- include/linux/of.h | 6 +- 22 files changed, 562 insertions(+), 388 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts
[PATCH v6 2/4] of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply()
From: Frank Rowand Signed-off-by: Frank Rowand --- Documentation/devicetree/overlay-notes.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt index c4aa0adf13ec..5175a24d387e 100644 --- a/Documentation/devicetree/overlay-notes.txt +++ b/Documentation/devicetree/overlay-notes.txt @@ -87,8 +87,8 @@ Overlay in-kernel API The API is quite easy to use. -1. Call of_overlay_apply() to create and apply an overlay changeset. The return -value is an error or a cookie identifying this overlay. +1. Call of_overlay_fdt_apply() to create and apply an overlay changeset. The +return value is an error or a cookie identifying this overlay. 2. Call of_overlay_remove() to remove and cleanup the overlay changeset previously created via the call to of_overlay_apply(). Removal of an overlay -- Frank Rowand
[PATCH v5 2/4] of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply()
From: Frank Rowand Signed-off-by: Frank Rowand --- Documentation/devicetree/overlay-notes.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt index c4aa0adf13ec..5175a24d387e 100644 --- a/Documentation/devicetree/overlay-notes.txt +++ b/Documentation/devicetree/overlay-notes.txt @@ -87,8 +87,8 @@ Overlay in-kernel API The API is quite easy to use. -1. Call of_overlay_apply() to create and apply an overlay changeset. The return -value is an error or a cookie identifying this overlay. +1. Call of_overlay_fdt_apply() to create and apply an overlay changeset. The +return value is an error or a cookie identifying this overlay. 2. Call of_overlay_remove() to remove and cleanup the overlay changeset previously created via the call to of_overlay_apply(). Removal of an overlay -- Frank Rowand
[PATCH v5 1/4] of: change overlay apply input data from unflattened to FDT
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. of_overlay_fdt_apply() is a private function for the anticipated overlay loader. Update unittest.c to use of_overlay_fdt_apply() instead of of_overlay_apply(). Move overlay fragments from artificial locations in drivers/of/unittest-data/tests-overlay.dtsi into one devicetree source file per overlay. This led to changes in drivers/of/unitest-data/Makefile and drivers/of/unitest.c. - Add overlay directives to the overlay devicetree source files so that dtc will compile them as true overlays into one FDT data chunk per overlay. - Set CFLAGS for drivers/of/unittest-data/testcases.dts so that symbols will be generated for overlay resolution of overlays that are no longer artificially contained in testcases.dts - Unflatten and apply each unittest overlay FDT using of_overlay_fdt_apply(). - Enable the of_resolve_phandles() check for whether the unflattened overlay is detached. This check was previously disabled because the overlays from tests-overlay.dtsi were not unflattened into detached trees. - Other changes to unittest.c infrastructure to manage multiple test FDTs built into the kernel image (access by name instead of arbitrary number). - of_unittest_overlay_high_level(): previously unused code to add properties from the overlay_base devicetree to the live tree was triggered by the restructuring of tests-overlay.dtsi and thus testcases.dts. This exposed two bugs: (1) the need to dup a property before adding it, and (2) property 'name' is auto-generated in the unflatten code and thus will be a duplicate in the __symbols__ node - do not treat this duplicate as an error. Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are "line over 80 characters" for either pre-existing long lines, or lines in devicetree source files. There are still some functions in unittest.c that should be tagged __init due to changes in this patch, but modpost is not warning of them and they are not a risk because they are only called from __init functions. A sweep of unittest.c for functions that should be tagged __init is on the todo list. Rob, the good news is that I have add making unittest a loadable module to my todo list. Changes from v4: - move of_overlay_fdt_apply() prototype from of_private.h to of.h - of_overlay_apply(): add kfree(fdt) if resolve phandles or initialize changeset fail - of_overlay_fdt_apply(): add parameter size of overlay_fdt and use it for some additional validation of the overlay fdt - update unittest.c with additional parameter to of_overlay_fdt_apply() Changes from v3: - OF_OVERLAY: add select OF_FLATTREE Changes from v1: - rebase on v4.16-rc1 drivers/of/Kconfig | 1 + drivers/of/overlay.c| 112 ++- drivers/of/resolver.c | 6 - drivers/of/unittest-data/Makefile | 28 ++- drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 34 drivers/of/unittest-data/overlay_11.dts | 34 drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 35 drivers/of/unittest-data/overlay_2.dts | 14 ++ drivers/of/unittest-data/overlay_3.dts | 14 ++ drivers/of/unittest-data/overlay_4.dts | 23 +++ drivers/of/unittest-data/overlay_5.dts | 14 ++ drivers/of/unittest-data/overlay_6.dts | 15 ++ drivers/of/unittest-data/overlay_7.dts | 15 ++ drivers/of/unittest-data/overlay_8.dts | 15 ++ drivers/of/unittest-data/overlay_9.dts | 15 ++ drivers/of/unittest-data/tests-overlay.dtsi | 213 drivers/of/unittest.c | 294 ++-- include/linux/of.h | 6 +- 22 files changed, 559 insertions(+), 385 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittes
[PATCH v5 4/4] of: improve reporting invalid overlay target path
From: Frank Rowand Errors while developing the patch to create of_overlay_fdt_apply() exposed inadequate error messages to debug problems when overlay devicetree fragment nodes contain an invalid target path. Improve the messages in find_target_node() to remedy this. Signed-off-by: Frank Rowand --- changes from v2: - add fragment node name as suggested by Geert - existing error message printed short node name, thus not discriminating between fragments; change to full node name - existing error message printed node address, which is devicetree internal debugging, not useful in an overlay apply error message; remove node address from message drivers/of/overlay.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index e3d7f69a8333..b930e05d1215 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -488,20 +488,30 @@ static int build_changeset(struct overlay_changeset *ovcs) */ static struct device_node *find_target_node(struct device_node *info_node) { + struct device_node *node; const char *path; u32 val; int ret; ret = of_property_read_u32(info_node, "target", &val); - if (!ret) - return of_find_node_by_phandle(val); + if (!ret) { + node = of_find_node_by_phandle(val); + if (!node) + pr_err("find target, node: %pOF, phandle 0x%x not found\n", + info_node, val); + return node; + } ret = of_property_read_string(info_node, "target-path", &path); - if (!ret) - return of_find_node_by_path(path); + if (!ret) { + node = of_find_node_by_path(path); + if (!node) + pr_err("find target, node: %pOF, path '%s' not found\n", + info_node, path); + return node; + } - pr_err("Failed to find target for node %p (%s)\n", - info_node, info_node->name); + pr_err("find target, node: %pOF, no target property\n", info_node); return NULL; } -- Frank Rowand
[PATCH v5 0/4] of: change overlay apply input data from unflattened
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. These changes led to migrating some unittest overlay data into their own devicetree source files, and then converting most of them to use sugar syntax instead of hand coding fragments. Changes from v4: (all in patch 1/4) - move of_overlay_fdt_apply() prototype from of_private.h to of.h - of_overlay_apply(): add kfree(fdt) if resolve phandles or initialize changeset fail - of_overlay_fdt_apply(): add parameter size of overlay_fdt and use it for some additional validation of the overlay fdt - update unittest.c with additional parameter to of_overlay_fdt_apply() Changes from v3: - patch 1/4: OF_OVERLAY: add select OF_FLATTREE Changes from v2: - improve error messages in patch 4/4, as suggested by Geert Changes from v1: - rebase on v4.16-rc1 - update documentation - split out error message to a separate patch Frank Rowand (4): of: change overlay apply input data from unflattened to FDT of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply() of: convert unittest overlay devicetree source to sugar syntax of: improve reporting invalid overlay target path Documentation/devicetree/overlay-notes.txt | 4 +- drivers/of/Kconfig | 1 + drivers/of/overlay.c | 134 +-- drivers/of/resolver.c| 6 - drivers/of/unittest-data/Makefile| 28 ++- drivers/of/unittest-data/overlay.dts | 101 drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 27 +++ drivers/of/unittest-data/overlay_11.dts | 28 +++ drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 30 +++ drivers/of/unittest-data/overlay_2.dts | 9 + drivers/of/unittest-data/overlay_3.dts | 9 + drivers/of/unittest-data/overlay_4.dts | 18 ++ drivers/of/unittest-data/overlay_5.dts | 9 + drivers/of/unittest-data/overlay_6.dts | 10 + drivers/of/unittest-data/overlay_7.dts | 10 + drivers/of/unittest-data/overlay_8.dts | 10 + drivers/of/unittest-data/overlay_9.dts | 10 + drivers/of/unittest-data/overlay_bad_phandle.dts | 23 +- drivers/of/unittest-data/overlay_bad_symbol.dts | 25 +- drivers/of/unittest-data/tests-overlay.dtsi | 217 + drivers/of/unittest.c| 294 +++ include/linux/of.h | 6 +- 26 files changed, 582 insertions(+), 483 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts create mode 100644 drivers/of/unittest-data/overlay_15.dts create mode 100644 drivers/of/unittest-data/overlay_2.dts create mode 100644 drivers/of/unittest-data/overlay_3.dts create mode 100644 drivers/of/unittest-data/overlay_4.dts create mode 100644 drivers/of/unittest-data/overlay_5.dts create mode 100644 drivers/of/unittest-data/overlay_6.dts create mode 100644 drivers/of/unittest-data/overlay_7.dts create mode 100644 drivers/of/unittest-data/overlay_8.dts create mode 100644 drivers/of/unittest-data/overlay_9.dts -- Frank Rowand
[PATCH v5 3/4] of: convert unittest overlay devicetree source to sugar syntax
From: Frank Rowand The unittest-data overlays have been pulled into proper overlay devicetree source files without changing their format. The next step is to convert them to use sugar syntax instead of hand coding overlay fragments structure. A few of the overlays can not be converted because they test absolute target paths in the overlay fragment. dtc does not generate this type of target: overlay_0.dts overlay_1.dts overlay_12.dts overlay_13.dts Two pre-existing unittest overlay devicetree source files are also converted: overlay_bad_phandle.dts overlay_bad_symbol.dts Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are pre-existing warnings of un-documented bindings of made up (fake) compatibles in devicetree source for test overlays. drivers/of/unittest-data/overlay.dts | 101 ++- drivers/of/unittest-data/overlay_10.dts | 39 - drivers/of/unittest-data/overlay_11.dts | 40 - drivers/of/unittest-data/overlay_15.dts | 41 - drivers/of/unittest-data/overlay_2.dts | 11 +-- drivers/of/unittest-data/overlay_3.dts | 11 +-- drivers/of/unittest-data/overlay_4.dts | 23 ++ drivers/of/unittest-data/overlay_5.dts | 11 +-- drivers/of/unittest-data/overlay_6.dts | 13 +-- drivers/of/unittest-data/overlay_7.dts | 13 +-- drivers/of/unittest-data/overlay_8.dts | 13 +-- drivers/of/unittest-data/overlay_9.dts | 13 +-- drivers/of/unittest-data/overlay_bad_phandle.dts | 23 ++ drivers/of/unittest-data/overlay_bad_symbol.dts | 25 ++ drivers/of/unittest-data/tests-overlay.dtsi | 4 +- 15 files changed, 148 insertions(+), 233 deletions(-) diff --git a/drivers/of/unittest-data/overlay.dts b/drivers/of/unittest-data/overlay.dts index ab5e89b5e27e..3bbc59e922fe 100644 --- a/drivers/of/unittest-data/overlay.dts +++ b/drivers/of/unittest-data/overlay.dts @@ -2,76 +2,63 @@ /dts-v1/; /plugin/; -/ { +&electric_1 { - fragment@0 { - target = <&electric_1>; + status = "okay"; - __overlay__ { - status = "okay"; - - hvac_2: hvac-large-1 { - compatible = "ot,hvac-large"; - heat-range = < 40 75 >; - cool-range = < 65 80 >; - }; - }; + hvac_2: hvac-large-1 { + compatible = "ot,hvac-large"; + heat-range = < 40 75 >; + cool-range = < 65 80 >; }; +}; - fragment@1 { - target = <&rides_1>; - - __overlay__ { - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - - ride@100 { - #address-cells = <1>; - #size-cells = <1>; - - track@30 { - incline-up = < 48 32 16 >; - }; +&rides_1 { - track@40 { - incline-up = < 47 31 15 >; - }; - }; + #address-cells = <1>; + #size-cells = <1>; + status = "okay"; - ride_200: ride@200 { - #address-cells = <1>; - #size-cells = <1>; - compatible = "ot,ferris-wheel"; - reg = < 0x0200 0x100 >; - hvac-provider = < &hvac_2 >; - hvac-thermostat = < 27 32 > ; - hvac-zones = < 12 5 >; - hvac-zone-names = "operator", "snack-bar"; - spin-controller = < &spin_ctrl_1 3 >; - spin-rph = < 30 >; - gondolas = < 16 >; - gondola-capacity = < 6 >; + ride@100 { + #address-cells = <1>; + #size-cells = <1>; - ride_200_left: track@10 { - reg = < 0x0010 0x10 >; - }; + track@30 { + incline-up = < 48 32 16 >; + }; - ride_200_right: track@20 { - reg = < 0x0020 0x10 >; - }; - }; + track@40 { + incline-up = < 47 31 15 >; }; }; - fragment@2 { - target = <&lights_2>; + ride_200: ride@200
[PATCH v4 3/4] of: convert unittest overlay devicetree source to sugar syntax
From: Frank Rowand The unittest-data overlays have been pulled into proper overlay devicetree source files without changing their format. The next step is to convert them to use sugar syntax instead of hand coding overlay fragments structure. A few of the overlays can not be converted because they test absolute target paths in the overlay fragment. dtc does not generate this type of target: overlay_0.dts overlay_1.dts overlay_12.dts overlay_13.dts Two pre-existing unittest overlay devicetree source files are also converted: overlay_bad_phandle.dts overlay_bad_symbol.dts Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are pre-existing warnings of un-documented bindings of made up (fake) compatibles in devicetree source for test overlays. drivers/of/unittest-data/overlay.dts | 101 ++- drivers/of/unittest-data/overlay_10.dts | 39 - drivers/of/unittest-data/overlay_11.dts | 40 - drivers/of/unittest-data/overlay_15.dts | 41 - drivers/of/unittest-data/overlay_2.dts | 11 +-- drivers/of/unittest-data/overlay_3.dts | 11 +-- drivers/of/unittest-data/overlay_4.dts | 23 ++ drivers/of/unittest-data/overlay_5.dts | 11 +-- drivers/of/unittest-data/overlay_6.dts | 13 +-- drivers/of/unittest-data/overlay_7.dts | 13 +-- drivers/of/unittest-data/overlay_8.dts | 13 +-- drivers/of/unittest-data/overlay_9.dts | 13 +-- drivers/of/unittest-data/overlay_bad_phandle.dts | 23 ++ drivers/of/unittest-data/overlay_bad_symbol.dts | 25 ++ drivers/of/unittest-data/tests-overlay.dtsi | 4 +- 15 files changed, 148 insertions(+), 233 deletions(-) diff --git a/drivers/of/unittest-data/overlay.dts b/drivers/of/unittest-data/overlay.dts index ab5e89b5e27e..3bbc59e922fe 100644 --- a/drivers/of/unittest-data/overlay.dts +++ b/drivers/of/unittest-data/overlay.dts @@ -2,76 +2,63 @@ /dts-v1/; /plugin/; -/ { +&electric_1 { - fragment@0 { - target = <&electric_1>; + status = "okay"; - __overlay__ { - status = "okay"; - - hvac_2: hvac-large-1 { - compatible = "ot,hvac-large"; - heat-range = < 40 75 >; - cool-range = < 65 80 >; - }; - }; + hvac_2: hvac-large-1 { + compatible = "ot,hvac-large"; + heat-range = < 40 75 >; + cool-range = < 65 80 >; }; +}; - fragment@1 { - target = <&rides_1>; - - __overlay__ { - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - - ride@100 { - #address-cells = <1>; - #size-cells = <1>; - - track@30 { - incline-up = < 48 32 16 >; - }; +&rides_1 { - track@40 { - incline-up = < 47 31 15 >; - }; - }; + #address-cells = <1>; + #size-cells = <1>; + status = "okay"; - ride_200: ride@200 { - #address-cells = <1>; - #size-cells = <1>; - compatible = "ot,ferris-wheel"; - reg = < 0x0200 0x100 >; - hvac-provider = < &hvac_2 >; - hvac-thermostat = < 27 32 > ; - hvac-zones = < 12 5 >; - hvac-zone-names = "operator", "snack-bar"; - spin-controller = < &spin_ctrl_1 3 >; - spin-rph = < 30 >; - gondolas = < 16 >; - gondola-capacity = < 6 >; + ride@100 { + #address-cells = <1>; + #size-cells = <1>; - ride_200_left: track@10 { - reg = < 0x0010 0x10 >; - }; + track@30 { + incline-up = < 48 32 16 >; + }; - ride_200_right: track@20 { - reg = < 0x0020 0x10 >; - }; - }; + track@40 { + incline-up = < 47 31 15 >; }; }; - fragment@2 { - target = <&lights_2>; + ride_200: ride@200
[PATCH v4 1/4] of: change overlay apply input data from unflattened to FDT
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. of_overlay_fdt_apply() is a private function for the anticipated overlay loader. Update unittest.c to use of_overlay_fdt_apply() instead of of_overlay_apply(). Move overlay fragments from artificial locations in drivers/of/unittest-data/tests-overlay.dtsi into one devicetree source file per overlay. This led to changes in drivers/of/unitest-data/Makefile and drivers/of/unitest.c. - Add overlay directives to the overlay devicetree source files so that dtc will compile them as true overlays into one FDT data chunk per overlay. - Set CFLAGS for drivers/of/unittest-data/testcases.dts so that symbols will be generated for overlay resolution of overlays that are no longer artificially contained in testcases.dts - Unflatten and apply each unittest overlay FDT using of_overlay_fdt_apply(). - Enable the of_resolve_phandles() check for whether the unflattened overlay is detached. This check was previously disabled because the overlays from tests-overlay.dtsi were not unflattened into detached trees. - Other changes to unittest.c infrastructure to manage multiple test FDTs built into the kernel image (access by name instead of arbitrary number). - of_unittest_overlay_high_level(): previously unused code to add properties from the overlay_base devicetree to the live tree was triggered by the restructuring of tests-overlay.dtsi and thus testcases.dts. This exposed two bugs: (1) the need to dup a property before adding it, and (2) property 'name' is auto-generated in the unflatten code and thus will be a duplicate in the __symbols__ node - do not treat this duplicate as an error. Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are "line over 80 characters" for either pre-existing long lines, or lines in devicetree source files. Changes from v3: - OF_OVERLAY: add select OF_FLATTREE Changes from v1: - rebase on v4.16-rc1 drivers/of/Kconfig | 1 + drivers/of/of_private.h | 1 + drivers/of/overlay.c| 107 +- drivers/of/resolver.c | 6 - drivers/of/unittest-data/Makefile | 28 ++- drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 34 drivers/of/unittest-data/overlay_11.dts | 34 drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 35 drivers/of/unittest-data/overlay_2.dts | 14 ++ drivers/of/unittest-data/overlay_3.dts | 14 ++ drivers/of/unittest-data/overlay_4.dts | 23 +++ drivers/of/unittest-data/overlay_5.dts | 14 ++ drivers/of/unittest-data/overlay_6.dts | 15 ++ drivers/of/unittest-data/overlay_7.dts | 15 ++ drivers/of/unittest-data/overlay_8.dts | 15 ++ drivers/of/unittest-data/overlay_9.dts | 15 ++ drivers/of/unittest-data/tests-overlay.dtsi | 213 drivers/of/unittest.c | 294 ++-- include/linux/of.h | 7 - 23 files changed, 552 insertions(+), 389 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts create mode 100644 drivers/of/unittest-data/overlay_15.dts create mode 100644 drivers/of/unittest-data/overlay_2.dts create mode 100644 drivers/of/unittest-data/overlay_3.dts create mode 100644 drivers/of/unittest-data/overlay_4.dts create mode 100644 drivers/of/unittest-data/overlay_5.dts create mode 100644 drivers/of/unittest-data/overlay_6.dts create mode 100644 drivers/of/unittest-data/overlay_7.dts create mode 100644 drivers/of/unittest-data/overlay_8.dts create mode 100644 drivers/of/unittest-data/overlay_9.dts diff --git a/drivers/of/Kconfig b/drivers/of/K
[PATCH v4 2/4] of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply()
From: Frank Rowand Signed-off-by: Frank Rowand --- Documentation/devicetree/overlay-notes.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt index c4aa0adf13ec..5175a24d387e 100644 --- a/Documentation/devicetree/overlay-notes.txt +++ b/Documentation/devicetree/overlay-notes.txt @@ -87,8 +87,8 @@ Overlay in-kernel API The API is quite easy to use. -1. Call of_overlay_apply() to create and apply an overlay changeset. The return -value is an error or a cookie identifying this overlay. +1. Call of_overlay_fdt_apply() to create and apply an overlay changeset. The +return value is an error or a cookie identifying this overlay. 2. Call of_overlay_remove() to remove and cleanup the overlay changeset previously created via the call to of_overlay_apply(). Removal of an overlay -- Frank Rowand
[PATCH v4 4/4] of: improve reporting invalid overlay target path
From: Frank Rowand Errors while developing the patch to create of_overlay_fdt_apply() exposed inadequate error messages to debug problems when overlay devicetree fragment nodes contain an invalid target path. Improve the messages in find_target_node() to remedy this. Signed-off-by: Frank Rowand --- changes from v2: - add fragment node name as suggested by Geert - existing error message printed short node name, thus not discriminating between fragments; change to full node name - existing error message printed node address, which is devicetree internal debugging, not useful in an overlay apply error message; remove node address from message drivers/of/overlay.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 5f6c5569e97d..852e566d7744 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -488,20 +488,30 @@ static int build_changeset(struct overlay_changeset *ovcs) */ static struct device_node *find_target_node(struct device_node *info_node) { + struct device_node *node; const char *path; u32 val; int ret; ret = of_property_read_u32(info_node, "target", &val); - if (!ret) - return of_find_node_by_phandle(val); + if (!ret) { + node = of_find_node_by_phandle(val); + if (!node) + pr_err("find target, node: %pOF, phandle 0x%x not found\n", + info_node, val); + return node; + } ret = of_property_read_string(info_node, "target-path", &path); - if (!ret) - return of_find_node_by_path(path); + if (!ret) { + node = of_find_node_by_path(path); + if (!node) + pr_err("find target, node: %pOF, path '%s' not found\n", + info_node, path); + return node; + } - pr_err("Failed to find target for node %p (%s)\n", - info_node, info_node->name); + pr_err("find target, node: %pOF, no target property\n", info_node); return NULL; } -- Frank Rowand
[PATCH v4 0/4] of: change overlay apply input data from unflattened
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. These changes led to migrating some unittest overlay data into their own devicetree source files, and then converting most of them to use sugar syntax instead of hand coding fragments. Changes from v3: - patch 1/4: OF_OVERLAY: add select OF_FLATTREE Changes from v2: - improve error messages in patch 4/4, as suggested by Geert Changes from v1: - rebase on v4.16-rc1 - update documentation - split out error message to a separate patch Frank Rowand (4): of: change overlay apply input data from unflattened to FDT of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply() of: convert unittest overlay devicetree source to sugar syntax of: improve reporting invalid overlay target path Documentation/devicetree/overlay-notes.txt | 4 +- drivers/of/Kconfig | 1 + drivers/of/of_private.h | 1 + drivers/of/overlay.c | 129 -- drivers/of/resolver.c| 6 - drivers/of/unittest-data/Makefile| 28 ++- drivers/of/unittest-data/overlay.dts | 101 drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 27 +++ drivers/of/unittest-data/overlay_11.dts | 28 +++ drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 30 +++ drivers/of/unittest-data/overlay_2.dts | 9 + drivers/of/unittest-data/overlay_3.dts | 9 + drivers/of/unittest-data/overlay_4.dts | 18 ++ drivers/of/unittest-data/overlay_5.dts | 9 + drivers/of/unittest-data/overlay_6.dts | 10 + drivers/of/unittest-data/overlay_7.dts | 10 + drivers/of/unittest-data/overlay_8.dts | 10 + drivers/of/unittest-data/overlay_9.dts | 10 + drivers/of/unittest-data/overlay_bad_phandle.dts | 23 +- drivers/of/unittest-data/overlay_bad_symbol.dts | 25 +- drivers/of/unittest-data/tests-overlay.dtsi | 217 + drivers/of/unittest.c| 294 +++ include/linux/of.h | 7 - 27 files changed, 575 insertions(+), 487 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts create mode 100644 drivers/of/unittest-data/overlay_15.dts create mode 100644 drivers/of/unittest-data/overlay_2.dts create mode 100644 drivers/of/unittest-data/overlay_3.dts create mode 100644 drivers/of/unittest-data/overlay_4.dts create mode 100644 drivers/of/unittest-data/overlay_5.dts create mode 100644 drivers/of/unittest-data/overlay_6.dts create mode 100644 drivers/of/unittest-data/overlay_7.dts create mode 100644 drivers/of/unittest-data/overlay_8.dts create mode 100644 drivers/of/unittest-data/overlay_9.dts -- Frank Rowand
[PATCH v4 1/2] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()
From: Frank Rowand Create a cache of the nodes that contain a phandle property. Use this cache to find the node for a given phandle value instead of scanning the devicetree to find the node. If the phandle value is not found in the cache, of_find_node_by_phandle() will fall back to the tree scan algorithm. The cache is initialized in of_core_init(). The cache is freed via a late_initcall_sync() if modules are not enabled. If the devicetree is created by the dtc compiler, with all phandle property values auto generated, then the size required by the cache could be 4 * (1 + number of phandles) bytes. This results in an O(1) node lookup cost for a given phandle value. Due to a concern that the phandle property values might not be consistent with what is generated by the dtc compiler, a mask has been added to the cache lookup algorithm. To maintain the O(1) node lookup cost, the size of the cache has been increased by rounding the number of entries up to the next power of two. The overhead of finding the devicetree node containing a given phandle value has been noted by several people in the recent past, in some cases with a patch to add a hashed index of devicetree nodes, based on the phandle value of the node. One concern with this approach is the extra space added to each node. This patch takes advantage of the phandle property values auto generated by the dtc compiler, which begin with one and monotonically increase by one, resulting in a range of 1..n for n phandle values. This implementation should also provide a good reduction of overhead for any range of phandle values that are mostly in a monotonic range. Performance measurements by Chintan Pandya of several implementations of patches that are similar to this one suggest an expected reduction of boot time by ~400ms for his test system. If the cache size was decreased to 64 entries, the boot time was reduced by ~340 ms. The measurements were on a 4.9.73 kernel for arch/arm64/boot/dts/qcom/sda670-mtp.dts, contains 2371 nodes and 814 phandle values. Reported-by: Chintan Pandya Signed-off-by: Frank Rowand --- Changes since v3: - of_populate_phandle_cache(): add check for failed memory allocation Changes since v2: - add mask to calculation of phandle cache entry - which results in better overhead reduction for devicetrees with phandle properties not allocated in the monotonically increasing range of 1..n - due to mask, number of entries in cache potentially increased to next power of two - minor fixes as suggested by reviewers - no longer using live_tree_max_phandle() so do not move it from drivers/of/resolver.c to drivers/of/base.c Changes since v1: - change short description from of: cache phandle nodes to reduce cost of of_find_node_by_phandle() - rebase on v4.16-rc1 - reorder new functions in base.c to avoid forward declaration - add locking around kfree(phandle_cache) for memory ordering - add explicit check for non-null of phandle_cache in of_find_node_by_phandle(). There is already a check for !handle, which prevents accessing a null phandle_cache, but that dependency is not obvious, so this check makes it more apparent. - do not free phandle_cache if modules are enabled, so that cached phandles will be available when modules are loaded drivers/of/base.c | 86 ++--- drivers/of/of_private.h | 3 ++ drivers/of/resolver.c | 3 -- 3 files changed, 85 insertions(+), 7 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index ad28de96e13f..e71d157d7149 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -91,10 +91,72 @@ int __weak of_node_to_nid(struct device_node *np) } #endif +static struct device_node **phandle_cache; +static u32 phandle_cache_mask; + +/* + * Assumptions behind phandle_cache implementation: + * - phandle property values are in a contiguous range of 1..n + * + * If the assumptions do not hold, then + * - the phandle lookup overhead reduction provided by the cache + * will likely be less + */ +static void of_populate_phandle_cache(void) +{ + unsigned long flags; + u32 cache_entries; + struct device_node *np; + u32 phandles = 0; + + raw_spin_lock_irqsave(&devtree_lock, flags); + + kfree(phandle_cache); + phandle_cache = NULL; + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandles++; + + cache_entries = roundup_pow_of_two(phandles); + phandle_cache_mask = cache_entries - 1; + + phandle_cache = kcalloc(cache_entries, sizeof(*phandle_cache), + GFP_ATOMIC); + if (!phandle_cache) + goto out; + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandle_cache[np->phandle & phandle_cache_mask] = np; + +out: +
[PATCH v4 2/2] of: add early boot allocation of of_find_node_by_phandle() cache
From: Frank Rowand The initial implementation of the of_find_node_by_phandle() cache allocates the cache using kcalloc(). Add an early boot allocation of the cache so it will be usable during early boot. Switch over to the kcalloc() based cache once normal memory allocation becomes available. Signed-off-by: Frank Rowand --- The previous version of this patch was a standalone patch: [PATCH] of: add early boot allocation of of_find_node_by_phandle() cache that was dependent on patch 1/2 of this series. Changes from previous versions: - no changes drivers/of/base.c | 33 + drivers/of/fdt.c| 2 ++ drivers/of/of_private.h | 2 ++ 3 files changed, 37 insertions(+) diff --git a/drivers/of/base.c b/drivers/of/base.c index e71d157d7149..eeaa270a5135 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -16,9 +16,11 @@ #define pr_fmt(fmt)"OF: " fmt +#include #include #include #include +#include #include #include #include @@ -134,6 +136,29 @@ static void of_populate_phandle_cache(void) raw_spin_unlock_irqrestore(&devtree_lock, flags); } +void __init of_populate_phandle_cache_early(void) +{ + u32 cache_entries; + struct device_node *np; + u32 phandles = 0; + size_t size; + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandles++; + + cache_entries = roundup_pow_of_two(phandles); + phandle_cache_mask = cache_entries - 1; + + size = cache_entries * sizeof(*phandle_cache); + phandle_cache = memblock_virt_alloc(size, 4); + memset(phandle_cache, 0, size); + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandle_cache[np->phandle & phandle_cache_mask] = np; +} + #ifndef CONFIG_MODULES static int __init of_free_phandle_cache(void) { @@ -153,7 +178,15 @@ static int __init of_free_phandle_cache(void) void __init of_core_init(void) { + unsigned long flags; struct device_node *np; + phys_addr_t size; + + raw_spin_lock_irqsave(&devtree_lock, flags); + size = (phandle_cache_mask + 1) * sizeof(*phandle_cache); + memblock_free(__pa(phandle_cache), size); + phandle_cache = NULL; + raw_spin_unlock_irqrestore(&devtree_lock, flags); of_populate_phandle_cache(); diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 84aa9d676375..cb320df23f26 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -1264,6 +1264,8 @@ void __init unflatten_device_tree(void) of_alias_scan(early_init_dt_alloc_memory_arch); unittest_unflatten_overlay_base(); + + of_populate_phandle_cache_early(); } /** diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index fa70650136b4..6720448c84cc 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -134,6 +134,8 @@ extern void __of_sysfs_remove_bin_file(struct device_node *np, /* illegal phandle value (set when unresolved) */ #define OF_PHANDLE_ILLEGAL 0xdeadbeef +extern void __init of_populate_phandle_cache_early(void); + /* iterators for transactions, used for overlays */ /* forward iterator */ #define for_each_transaction_entry(_oft, _te) \ -- Frank Rowand
[PATCH v4 0/2] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()
From: Frank Rowand Create a cache of the nodes that contain a phandle property. Use this cache to find the node for a given phandle value instead of scanning the devicetree to find the node. If the phandle value is not found in the cache, of_find_node_by_phandle() will fall back to the tree scan algorithm. Size and performance data is in patch 1/2 comments Changes since v3: - of_populate_phandle_cache(): add check for failed memory allocation - add patch 2/2 into series instead of as a standalone patch that was dependent on patch 1/2 of this series Changes since v2: - add mask to calculation of phandle cache entry - which results in better overhead reduction for devicetrees with phandle properties not allocated in the monotonically increasing range of 1..n - due to mask, number of entries in cache potentially increased to next power of two - minor fixes as suggested by reviewers - no longer using live_tree_max_phandle() so do not move it from drivers/of/resolver.c to drivers/of/base.c Changes since v1: - change short description from of: cache phandle nodes to reduce cost of of_find_node_by_phandle() - rebase on v4.16-rc1 - reorder new functions in base.c to avoid forward declaration - add locking around kfree(phandle_cache) for memory ordering - add explicit check for non-null of phandle_cache in of_find_node_by_phandle(). There is already a check for !handle, which prevents accessing a null phandle_cache, but that dependency is not obvious, so this check makes it more apparent. - do not free phandle_cache if modules are enabled, so that cached phandles will be available when modules are loaded Frank Rowand (2): of: cache phandle nodes to reduce cost of of_find_node_by_phandle() of: add early boot allocation of of_find_node_by_phandle() cache drivers/of/base.c | 119 ++-- drivers/of/fdt.c| 2 + drivers/of/of_private.h | 5 ++ drivers/of/resolver.c | 3 -- 4 files changed, 122 insertions(+), 7 deletions(-) -- Frank Rowand
[PATCH v2 2/2] of: overlay: do not include path in full_name of added nodes
From: Frank Rowand Struct device_node full_name no longer includes the full path name when the devicetree is created from a flattened device tree (FDT). The overlay node creation code was not modified to reflect this change. Fix the node full_name generated by overlay code to contain only the basename. Unittests call an overlay internal function to create new nodes. Fix up these calls to provide basename only instead of the full path. Fixes: a7e4cfb0a7ca ("of/fdt: only store the device node basename in full_name") Signed-off-by: Frank Rowand --- changes since version 1: - update patch one-line description and full description - no longer remove kbasename from resolver.c - add_changeset_node(): add back kbasename() when comparing nodes in the livetree against nodes in the overlay - add_changeset_node(): add header comments to document assumptions and behavior, and to explain why kbasename() is used drivers/of/dynamic.c| 21 ++--- drivers/of/of_private.h | 3 ++- drivers/of/overlay.c| 18 +++--- drivers/of/unittest.c | 6 +++--- 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 7bb33d22b4e2..f4f8ed9b5454 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -383,25 +383,24 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags) /** * __of_node_dup() - Duplicate or create an empty device node dynamically. - * @fmt: Format string (plus vargs) for new full name of the device node + * @np:if not NULL, contains properties to be duplicated in new node + * @full_name: string value to be duplicated into new node's full_name field * - * Create an device tree node, either by duplicating an empty node or by allocating - * an empty one suitable for further modification. The node data are - * dynamically allocated and all the node flags have the OF_DYNAMIC & - * OF_DETACHED bits set. Returns the newly allocated node or NULL on out of - * memory error. + * Create a device tree node, optionally duplicating the properties of + * another node. The node data are dynamically allocated and all the node + * flags have the OF_DYNAMIC & OF_DETACHED bits set. + * + * Returns the newly allocated node or NULL on out of memory error. */ -struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...) +struct device_node *__of_node_dup(const struct device_node *np, + const char *full_name) { - va_list vargs; struct device_node *node; node = kzalloc(sizeof(*node), GFP_KERNEL); if (!node) return NULL; - va_start(vargs, fmt); - node->full_name = kvasprintf(GFP_KERNEL, fmt, vargs); - va_end(vargs); + node->full_name = kstrdup(full_name, GFP_KERNEL); if (!node->full_name) { kfree(node); return NULL; diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 0c609e7d0334..26bb31beb03e 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -104,7 +104,8 @@ extern void *__unflatten_device_tree(const void *blob, * own the devtree lock or work on detached trees only. */ 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_node_dup(const struct device_node *np, + const char *full_name); struct device_node *__of_find_node_by_path(struct device_node *parent, const char *path); diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 3397d7642958..c39df1c663be 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -307,7 +307,20 @@ static int add_changeset_property(struct overlay_changeset *ovcs, * If @node has child nodes, add the children recursively via * build_changeset_next_level(). * - * NOTE: Multiple mods of created nodes not supported. + * NOTE_1: A live devicetree created from a flattened device tree (FDT) will + * not contain the full path in node->full_name. Thus an overlay + * created from an FDT also will not contain the full path in + * node->full_name. However, a live devicetree created from Open + * Firmware may have the full path in node->full_name. + * + * add_changeset_node() follows the FDT convention and does not include + * the full path in node->full_name. Even though it expects the overlay + * to not contain the full path, it uses kbasename() to remove the + * full path should it exist. It also uses kbasename() in comparisons + * to nodes in the live devicetree so that it can apply an overlay to + * a live devicetree created from Open Firmware. + * + * NOTE_2: Multiple mods of created nodes not supported. * If m
[PATCH v2 1/2] of: unittest: clean up changeset test
From: Frank Rowand In preparation for fixing __of_node_dup(), clean up the unittest function that calls it. Devicetree nodes created from a flattened device tree have a name property. Follow this convention for nodes added by a changeset. For node added by changeset, remove incorrect initialization of child node pointer. Add an additional node pointer 'changeset' to more naturally reflect where in the tree the changeset is added. Make changeset add property error messages unique. Add whitespace to break apart logic blocks. Signed-off-by: Frank Rowand --- no changes from v1 Checkpatch warnings are "line over 80 characters" and are consistent with the style of the surrounding code. drivers/of/unittest.c | 42 -- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 7a9abaae874d..490bbee0cf87 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -562,42 +562,72 @@ static void __init of_unittest_property_copy(void) static void __init of_unittest_changeset(void) { #ifdef CONFIG_OF_DYNAMIC - struct property *ppadd, padd = { .name = "prop-add", .length = 0, .value = "" }; + struct property *ppadd, padd = { .name = "prop-add", .length = 1, .value = "" }; + struct property *ppname_n1, pname_n1 = { .name = "name", .length = 3, .value = "n1" }; + struct property *ppname_n2, pname_n2 = { .name = "name", .length = 3, .value = "n2" }; + struct property *ppname_n21, pname_n21 = { .name = "name", .length = 3, .value = "n21" }; struct property *ppupdate, pupdate = { .name = "prop-update", .length = 5, .value = "abcd" }; struct property *ppremove; - struct device_node *n1, *n2, *n21, *nremove, *parent, *np; + struct device_node *n1, *n2, *n21, *nchangeset, *nremove, *parent, *np; struct of_changeset chgset; n1 = __of_node_dup(NULL, "/testcase-data/changeset/n1"); unittest(n1, "testcase setup failure\n"); + n2 = __of_node_dup(NULL, "/testcase-data/changeset/n2"); unittest(n2, "testcase setup failure\n"); + n21 = __of_node_dup(NULL, "%s/%s", "/testcase-data/changeset/n2", "n21"); unittest(n21, "testcase setup failure %p\n", n21); - nremove = of_find_node_by_path("/testcase-data/changeset/node-remove"); + + nchangeset = of_find_node_by_path("/testcase-data/changeset"); + nremove = of_get_child_by_name(nchangeset, "node-remove"); unittest(nremove, "testcase setup failure\n"); + ppadd = __of_prop_dup(&padd, GFP_KERNEL); unittest(ppadd, "testcase setup failure\n"); + + ppname_n1 = __of_prop_dup(&pname_n1, GFP_KERNEL); + unittest(ppname_n1, "testcase setup failure\n"); + + ppname_n2 = __of_prop_dup(&pname_n2, GFP_KERNEL); + unittest(ppname_n2, "testcase setup failure\n"); + + ppname_n21 = __of_prop_dup(&pname_n21, GFP_KERNEL); + unittest(ppname_n21, "testcase setup failure\n"); + ppupdate = __of_prop_dup(&pupdate, GFP_KERNEL); unittest(ppupdate, "testcase setup failure\n"); - parent = nremove->parent; + + parent = nchangeset; n1->parent = parent; n2->parent = parent; n21->parent = n2; - n2->child = n21; + ppremove = of_find_property(parent, "prop-remove", NULL); unittest(ppremove, "failed to find removal prop"); of_changeset_init(&chgset); + unittest(!of_changeset_attach_node(&chgset, n1), "fail attach n1\n"); + unittest(!of_changeset_add_property(&chgset, n1, ppname_n1), "fail add prop name\n"); + unittest(!of_changeset_attach_node(&chgset, n2), "fail attach n2\n"); + unittest(!of_changeset_add_property(&chgset, n2, ppname_n2), "fail add prop name\n"); + unittest(!of_changeset_detach_node(&chgset, nremove), "fail remove node\n"); + unittest(!of_changeset_add_property(&chgset, n21, ppname_n21), "fail add prop name\n"); + unittest(!of_changeset_attach_node(&chgset, n21), "fail attach n21\n"); - unittest(!of_changeset_add_property(&chgset, parent, ppadd), "fail add prop\n"); + + unittest(!of_changeset_add_property(&chgset, parent, ppadd), "fail add prop prop-add\n"); unittest(!of_changeset_update_property(&chgset, parent, ppupdate), "fail update prop\n"); unittest(!of_changeset_remove_property(&chgset, parent, ppremove), "fail remove prop\n"); + unittest(!of_changeset_apply(&chgset), "apply failed\n"); + of_node_put(nchangeset); + /* Make sure node names are constructed correctly */ unittest((np = of_find_node_by_path("/testcase-data/changeset/n2/n21")), "'%pOF' not added\n", n21); -- Frank Rowand
[PATCH v2 0/2] of: remove kbasename() from core
From: Frank Rowand (One line description of patch 0/2 is now misleading, but leaving intact to make it easier to find v1. One line description of other patches updated as they will be part of the git commit record.) Struct device_node full_name no longer includes the full path name. Fix some broken overlay code that was not updated to reflect this. Clean up the unittest changeset test that calls into this overlay code. Version 1 of this patch removed kbasename() for other parts of core devicetree code. But Geert kindly pointed out that a devicetree created from Open Firmware (instead from an FDT) could still contain a full path in the struct device_node full_path field. This version (v2) of the patch leaves kbasename() in place where needed. Changes since v1: - update patch 2/2 one-line description and full description - no longer remove kbasename from resolver.c - add_changeset_node(): add back kbasename() when comparing nodes in the livetree against nodes in the overlay - add_changeset_node(): add header comments to document assumptions and behavior, and to explain why kbasename() is used Frank Rowand (2): of: unittest: clean up changeset test of: overlay: do not include path in full_name of added nodes drivers/of/dynamic.c| 21 ++--- drivers/of/of_private.h | 3 ++- drivers/of/overlay.c| 18 +++--- drivers/of/unittest.c | 48 +++- 4 files changed, 66 insertions(+), 24 deletions(-) -- Frank Rowand
[PATCH 1/6] of: unittest: clean up changeset test
From: Frank Rowand In preparation for fixing __of_node_dup(), clean up the unittest function that calls it. Devicetree nodes created from a flattened device tree have a name property. Follow this convention for nodes added by a changeset. For node added by changeset, remove incorrect initialization of child node pointer. Add an additional node pointer 'changeset' to more naturally reflect where in the tree the changeset is added. Make changeset add property error messages unique. Add whitespace to break apart logic blocks. Signed-off-by: Frank Rowand --- checkpatch warnings for "line over 80 characters" are for unittest code in a function with long lines as a normal style. drivers/of/unittest.c | 42 -- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 7a9abaae874d..490bbee0cf87 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -562,42 +562,72 @@ static void __init of_unittest_property_copy(void) static void __init of_unittest_changeset(void) { #ifdef CONFIG_OF_DYNAMIC - struct property *ppadd, padd = { .name = "prop-add", .length = 0, .value = "" }; + struct property *ppadd, padd = { .name = "prop-add", .length = 1, .value = "" }; + struct property *ppname_n1, pname_n1 = { .name = "name", .length = 3, .value = "n1" }; + struct property *ppname_n2, pname_n2 = { .name = "name", .length = 3, .value = "n2" }; + struct property *ppname_n21, pname_n21 = { .name = "name", .length = 3, .value = "n21" }; struct property *ppupdate, pupdate = { .name = "prop-update", .length = 5, .value = "abcd" }; struct property *ppremove; - struct device_node *n1, *n2, *n21, *nremove, *parent, *np; + struct device_node *n1, *n2, *n21, *nchangeset, *nremove, *parent, *np; struct of_changeset chgset; n1 = __of_node_dup(NULL, "/testcase-data/changeset/n1"); unittest(n1, "testcase setup failure\n"); + n2 = __of_node_dup(NULL, "/testcase-data/changeset/n2"); unittest(n2, "testcase setup failure\n"); + n21 = __of_node_dup(NULL, "%s/%s", "/testcase-data/changeset/n2", "n21"); unittest(n21, "testcase setup failure %p\n", n21); - nremove = of_find_node_by_path("/testcase-data/changeset/node-remove"); + + nchangeset = of_find_node_by_path("/testcase-data/changeset"); + nremove = of_get_child_by_name(nchangeset, "node-remove"); unittest(nremove, "testcase setup failure\n"); + ppadd = __of_prop_dup(&padd, GFP_KERNEL); unittest(ppadd, "testcase setup failure\n"); + + ppname_n1 = __of_prop_dup(&pname_n1, GFP_KERNEL); + unittest(ppname_n1, "testcase setup failure\n"); + + ppname_n2 = __of_prop_dup(&pname_n2, GFP_KERNEL); + unittest(ppname_n2, "testcase setup failure\n"); + + ppname_n21 = __of_prop_dup(&pname_n21, GFP_KERNEL); + unittest(ppname_n21, "testcase setup failure\n"); + ppupdate = __of_prop_dup(&pupdate, GFP_KERNEL); unittest(ppupdate, "testcase setup failure\n"); - parent = nremove->parent; + + parent = nchangeset; n1->parent = parent; n2->parent = parent; n21->parent = n2; - n2->child = n21; + ppremove = of_find_property(parent, "prop-remove", NULL); unittest(ppremove, "failed to find removal prop"); of_changeset_init(&chgset); + unittest(!of_changeset_attach_node(&chgset, n1), "fail attach n1\n"); + unittest(!of_changeset_add_property(&chgset, n1, ppname_n1), "fail add prop name\n"); + unittest(!of_changeset_attach_node(&chgset, n2), "fail attach n2\n"); + unittest(!of_changeset_add_property(&chgset, n2, ppname_n2), "fail add prop name\n"); + unittest(!of_changeset_detach_node(&chgset, nremove), "fail remove node\n"); + unittest(!of_changeset_add_property(&chgset, n21, ppname_n21), "fail add prop name\n"); + unittest(!of_changeset_attach_node(&chgset, n21), "fail attach n21\n"); - unittest(!of_changeset_add_property(&chgset, parent, ppadd), "fail add prop\n"); + + unittest(!of_changeset_add_property(&chgset, parent, ppadd), "fail add prop prop-add\n"); unittest(!of_changeset_update_property(&chgset, parent, ppupdate), "fail update prop\n"); unittest(!of_changeset_remove_property(&chgset, parent, ppremove), "fail remove prop\n"); + unittest(!of_changeset_apply(&chgset), "apply failed\n"); + of_node_put(nchangeset); + /* Make sure node names are constructed correctly */ unittest((np = of_find_node_by_path("/testcase-data/changeset/n2/n21")), "'%pOF' not added\n", n21); -- Frank Rowand
[PATCH 4/6] of: remove kbasename(of->full_name) from kobj.c
From: Frank Rowand struct device_node full_name has been changed to include the basename instead of the full path. kbasename() is no longer needed to extract the basename from full_name. Signed-off-by: Frank Rowand --- drivers/of/kobj.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c index 7a0a18980b98..b0aecea35706 100644 --- a/drivers/of/kobj.c +++ b/drivers/of/kobj.c @@ -128,7 +128,7 @@ int __of_attach_node_sysfs(struct device_node *np) name = safe_name(&of_kset->kobj, "base"); parent = NULL; } else { - name = safe_name(&np->parent->kobj, kbasename(np->full_name)); + name = safe_name(&np->parent->kobj, np->full_name); parent = &np->parent->kobj; } if (!name) -- Frank Rowand
[PATCH 2/6] of: remove kbasename(of->full_name) from overlay related code
From: Frank Rowand Struct device_node full_name no longer includes the full path name. The overlay node creation code was not modified to reflect this change. Fix the node names generate by overlay code to contain only the basename. Unittests call an overlay internal function to create new nodes. Fix up these calls to provide basename only instead of the full path. Fixes: a7e4cfb0a7ca ("of/fdt: only store the device node basename in full_name") Signed-off-by: Frank Rowand --- drivers/of/dynamic.c| 21 ++--- drivers/of/of_private.h | 3 ++- drivers/of/overlay.c| 8 ++-- drivers/of/resolver.c | 5 + drivers/of/unittest.c | 6 +++--- 5 files changed, 18 insertions(+), 25 deletions(-) diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c index 7bb33d22b4e2..f4f8ed9b5454 100644 --- a/drivers/of/dynamic.c +++ b/drivers/of/dynamic.c @@ -383,25 +383,24 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags) /** * __of_node_dup() - Duplicate or create an empty device node dynamically. - * @fmt: Format string (plus vargs) for new full name of the device node + * @np:if not NULL, contains properties to be duplicated in new node + * @full_name: string value to be duplicated into new node's full_name field * - * Create an device tree node, either by duplicating an empty node or by allocating - * an empty one suitable for further modification. The node data are - * dynamically allocated and all the node flags have the OF_DYNAMIC & - * OF_DETACHED bits set. Returns the newly allocated node or NULL on out of - * memory error. + * Create a device tree node, optionally duplicating the properties of + * another node. The node data are dynamically allocated and all the node + * flags have the OF_DYNAMIC & OF_DETACHED bits set. + * + * Returns the newly allocated node or NULL on out of memory error. */ -struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...) +struct device_node *__of_node_dup(const struct device_node *np, + const char *full_name) { - va_list vargs; struct device_node *node; node = kzalloc(sizeof(*node), GFP_KERNEL); if (!node) return NULL; - va_start(vargs, fmt); - node->full_name = kvasprintf(GFP_KERNEL, fmt, vargs); - va_end(vargs); + node->full_name = kstrdup(full_name, GFP_KERNEL); if (!node->full_name) { kfree(node); return NULL; diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 0c609e7d0334..26bb31beb03e 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -104,7 +104,8 @@ extern void *__unflatten_device_tree(const void *blob, * own the devtree lock or work on detached trees only. */ 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_node_dup(const struct device_node *np, + const char *full_name); struct device_node *__of_find_node_by_path(struct device_node *parent, const char *path); diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 3397d7642958..b9df55e0a656 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -323,19 +323,15 @@ static int add_changeset_property(struct overlay_changeset *ovcs, static int add_changeset_node(struct overlay_changeset *ovcs, struct device_node *target_node, struct device_node *node) { - const char *node_kbasename; struct device_node *tchild; int ret = 0; - node_kbasename = kbasename(node->full_name); - for_each_child_of_node(target_node, tchild) - if (!of_node_cmp(node_kbasename, kbasename(tchild->full_name))) + if (!of_node_cmp(node->full_name, tchild->full_name)) break; if (!tchild) { - tchild = __of_node_dup(node, "%pOF/%s", - target_node, node_kbasename); + tchild = __of_node_dup(node, node->full_name); if (!tchild) return -ENOMEM; diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 740d19bde601..0e0969f58216 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -137,10 +137,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay, static int node_name_cmp(const struct device_node *dn1, const struct device_node *dn2) { - const char *n1 = kbasename(dn1->full_name); - const char *n2 = kbasename(dn2->full_name); - - return of_node_cmp(n1, n2); + return of_node_cmp(dn1->full_name, dn2->full_name); } /* diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 490bbee0cf8
[PATCH 6/6] of: remove kbasename(of->full_name) from platform.c
From: Frank Rowand struct device_node full_name has been changed to include the basename instead of the full path. kbasename() is no longer needed to extract the basename from full_name. Signed-off-by: Frank Rowand --- drivers/of/platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index c00d81dfac0b..17116f5ce6ba 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -94,7 +94,7 @@ static void of_device_make_bus_id(struct device *dev) /* format arguments only used if dev_name() resolves to NULL */ dev_set_name(dev, dev_name(dev) ? "%s:%s" : "%s", -kbasename(node->full_name), dev_name(dev)); +node->full_name, dev_name(dev)); node = node->parent; } } -- Frank Rowand
[PATCH 5/6] of: remove kbasename(of->full_name) from of_reserved_mem.c
From: Frank Rowand struct device_node full_name has been changed to include the basename instead of the full path. kbasename() is no longer needed to extract the basename from full_name. Signed-off-by: Frank Rowand --- drivers/of/of_reserved_mem.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 9a4f4246231d..e770cab4b95b 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -405,15 +405,13 @@ void of_reserved_mem_device_release(struct device *dev) */ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np) { - const char *name; int i; if (!np->full_name) return NULL; - name = kbasename(np->full_name); for (i = 0; i < reserved_mem_count; i++) - if (!strcmp(reserved_mem[i].name, name)) + if (!strcmp(reserved_mem[i].name, np->full_name)) return &reserved_mem[i]; return NULL; -- Frank Rowand
[PATCH 3/6] of: remove kbasename(of->full_name) from base.c
From: Frank Rowand struct device_node full_name has been changed to include the basename instead of the full path. kbasename() is no longer needed to extract the basename from full_name. Signed-off-by: Frank Rowand --- drivers/of/base.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index ad28de96e13f..926d08b8674d 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -691,8 +691,8 @@ struct device_node *__of_find_node_by_path(struct device_node *parent, return NULL; __for_each_child_of_node(parent, child) { - const char *name = kbasename(child->full_name); - if (strncmp(path, name, len) == 0 && (strlen(name) == len)) + if (strncmp(path, child->full_name, len) == 0 && + (strlen(child->full_name) == len)) return child; } return NULL; -- Frank Rowand
[PATCH 0/6] of: remove kbasename() from core
From: Frank Rowand Struct device_node full_name no longer includes the full path name. Fix some broken overlay code that was not updated to reflect this. Clean up the unittest changeset test that calls into this overlay code. kbasename(->full_name) is no longer needed, so remove kbasename() from core devicetree code. Removal of kbasename() is separated out into one patch per file as much as possible to make bisection easier if a problem occurs. Frank Rowand (6): of: unittest: clean up changeset test of: remove kbasename(of->full_name) from overlay related code of: remove kbasename(of->full_name) from base.c of: remove kbasename(of->full_name) from kobj.c of: remove kbasename(of->full_name) from of_reserved_mem.c of: remove kbasename(of->full_name) from platform.c drivers/of/base.c| 4 ++-- drivers/of/dynamic.c | 21 +-- drivers/of/kobj.c| 2 +- drivers/of/of_private.h | 3 ++- drivers/of/of_reserved_mem.c | 4 +--- drivers/of/overlay.c | 8 ++-- drivers/of/platform.c| 2 +- drivers/of/resolver.c| 5 + drivers/of/unittest.c| 48 +++- 9 files changed, 59 insertions(+), 38 deletions(-) -- Frank Rowand
[PATCH] of: Kconfig: OF_OVERLAY, select OF_EARLY_FLATTREE
From: Frank Rowand kbuild test robot reported a new warning for a recent patch: >> drivers/of/overlay.c:832:2: error: implicit declaration of function >> 'of_fdt_unflatten_tree' [-Werror=implicit-function-declaration] of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root); The cause is that the prototype for of_fdt_unflatten_tree() in include/linux/of_fdt.c is guarded by OF_EARLY_FLATTREE. This was a pre-existing problem for any overlay related caller of of_fdt_unflatten_device_tree(), who was then going to pass the unflattened tree to of_overlay_apply(). After the patch that triggered this warning, all other overlay callers of of_fdt_unflatten_device_tree() no longer exist, so adding the select to OF_OVERLAY is a sufficient fix. To reproduce the warning: Use the .config attached to https://lkml.org/lkml/2018/2/17/268 make ARCH=i386 olddefconfig make ARCH=i386 CC=gcc-7 drivers/of/overlay.o Signed-off-by: Frank Rowand --- drivers/of/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index 783e0870bd22..00a6abfaaec7 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -92,6 +92,7 @@ config OF_RESOLVE config OF_OVERLAY bool "Device Tree overlays" select OF_DYNAMIC + select OF_EARLY_FLATTREE select OF_RESOLVE help Overlays are a method to dynamically modify part of the kernel's -- Frank Rowand
[PATCH v3 4/4] of: improve reporting invalid overlay target path
From: Frank Rowand Errors while developing the patch to create of_overlay_fdt_apply() exposed inadequate error messages to debug problems when overlay devicetree fragment nodes contain an invalid target path. Improve the messages in find_target_node() to remedy this. Signed-off-by: Frank Rowand --- changes from v2: - add fragment node name as suggested by Geert - existing error message printed short node name, thus not discriminating between fragments; change to full node name - existing error message printed node address, which is devicetree internal debugging, not useful in an overlay apply error message; remove node address from message drivers/of/overlay.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 5f6c5569e97d..852e566d7744 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -488,20 +488,30 @@ static int build_changeset(struct overlay_changeset *ovcs) */ static struct device_node *find_target_node(struct device_node *info_node) { + struct device_node *node; const char *path; u32 val; int ret; ret = of_property_read_u32(info_node, "target", &val); - if (!ret) - return of_find_node_by_phandle(val); + if (!ret) { + node = of_find_node_by_phandle(val); + if (!node) + pr_err("find target, node: %pOF, phandle 0x%x not found\n", + info_node, val); + return node; + } ret = of_property_read_string(info_node, "target-path", &path); - if (!ret) - return of_find_node_by_path(path); + if (!ret) { + node = of_find_node_by_path(path); + if (!node) + pr_err("find target, node: %pOF, path '%s' not found\n", + info_node, path); + return node; + } - pr_err("Failed to find target for node %p (%s)\n", - info_node, info_node->name); + pr_err("find target, node: %pOF, no target property\n", info_node); return NULL; } -- Frank Rowand
[PATCH v3 2/4] of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply()
From: Frank Rowand Signed-off-by: Frank Rowand --- Documentation/devicetree/overlay-notes.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt index c4aa0adf13ec..5175a24d387e 100644 --- a/Documentation/devicetree/overlay-notes.txt +++ b/Documentation/devicetree/overlay-notes.txt @@ -87,8 +87,8 @@ Overlay in-kernel API The API is quite easy to use. -1. Call of_overlay_apply() to create and apply an overlay changeset. The return -value is an error or a cookie identifying this overlay. +1. Call of_overlay_fdt_apply() to create and apply an overlay changeset. The +return value is an error or a cookie identifying this overlay. 2. Call of_overlay_remove() to remove and cleanup the overlay changeset previously created via the call to of_overlay_apply(). Removal of an overlay -- Frank Rowand
[PATCH v3 1/4] of: change overlay apply input data from unflattened to FDT
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. of_overlay_fdt_apply() is a private function for the anticipated overlay loader. Update unittest.c to use of_overlay_fdt_apply() instead of of_overlay_apply(). Move overlay fragments from artificial locations in drivers/of/unittest-data/tests-overlay.dtsi into one devicetree source file per overlay. This led to changes in drivers/of/unitest-data/Makefile and drivers/of/unitest.c. - Add overlay directives to the overlay devicetree source files so that dtc will compile them as true overlays into one FDT data chunk per overlay. - Set CFLAGS for drivers/of/unittest-data/testcases.dts so that symbols will be generated for overlay resolution of overlays that are no longer artificially contained in testcases.dts - Unflatten and apply each unittest overlay FDT using of_overlay_fdt_apply(). - Enable the of_resolve_phandles() check for whether the unflattened overlay is detached. This check was previously disabled because the overlays from tests-overlay.dtsi were not unflattened into detached trees. - Other changes to unittest.c infrastructure to manage multiple test FDTs built into the kernel image (access by name instead of arbitrary number). - of_unittest_overlay_high_level(): previously unused code to add properties from the overlay_base devicetree to the live tree was triggered by the restructuring of tests-overlay.dtsi and thus testcases.dts. This exposed two bugs: (1) the need to dup a property before adding it, and (2) property 'name' is auto-generated in the unflatten code and thus will be a duplicate in the __symbols__ node - do not treat this duplicate as an error. Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are "line over 80 characters" for either pre-existing long lines, or lines in devicetree source files. drivers/of/of_private.h | 1 + drivers/of/overlay.c| 107 +- drivers/of/resolver.c | 6 - drivers/of/unittest-data/Makefile | 28 ++- drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 34 drivers/of/unittest-data/overlay_11.dts | 34 drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 35 drivers/of/unittest-data/overlay_2.dts | 14 ++ drivers/of/unittest-data/overlay_3.dts | 14 ++ drivers/of/unittest-data/overlay_4.dts | 23 +++ drivers/of/unittest-data/overlay_5.dts | 14 ++ drivers/of/unittest-data/overlay_6.dts | 15 ++ drivers/of/unittest-data/overlay_7.dts | 15 ++ drivers/of/unittest-data/overlay_8.dts | 15 ++ drivers/of/unittest-data/overlay_9.dts | 15 ++ drivers/of/unittest-data/tests-overlay.dtsi | 213 drivers/of/unittest.c | 294 ++-- include/linux/of.h | 7 - 22 files changed, 551 insertions(+), 389 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts create mode 100644 drivers/of/unittest-data/overlay_15.dts create mode 100644 drivers/of/unittest-data/overlay_2.dts create mode 100644 drivers/of/unittest-data/overlay_3.dts create mode 100644 drivers/of/unittest-data/overlay_4.dts create mode 100644 drivers/of/unittest-data/overlay_5.dts create mode 100644 drivers/of/unittest-data/overlay_6.dts create mode 100644 drivers/of/unittest-data/overlay_7.dts create mode 100644 drivers/of/unittest-data/overlay_8.dts create mode 100644 drivers/of/unittest-data/overlay_9.dts diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 0c609e7d0334..6e39dce3a979 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -77,6 +77,7 @@ static inline voi
[PATCH v3 3/4] of: convert unittest overlay devicetree source to sugar syntax
From: Frank Rowand The unittest-data overlays have been pulled into proper overlay devicetree source files without changing their format. The next step is to convert them to use sugar syntax instead of hand coding overlay fragments structure. A few of the overlays can not be converted because they test absolute target paths in the overlay fragment. dtc does not generate this type of target: overlay_0.dts overlay_1.dts overlay_12.dts overlay_13.dts Two pre-existing unittest overlay devicetree source files are also converted: overlay_bad_phandle.dts overlay_bad_symbol.dts Signed-off-by: Frank Rowand --- There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are pre-existing warnings of un-documented bindings of made up (fake) compatibles in devicetree source for test overlays. drivers/of/unittest-data/overlay.dts | 101 ++- drivers/of/unittest-data/overlay_10.dts | 39 - drivers/of/unittest-data/overlay_11.dts | 40 - drivers/of/unittest-data/overlay_15.dts | 41 - drivers/of/unittest-data/overlay_2.dts | 11 +-- drivers/of/unittest-data/overlay_3.dts | 11 +-- drivers/of/unittest-data/overlay_4.dts | 23 ++ drivers/of/unittest-data/overlay_5.dts | 11 +-- drivers/of/unittest-data/overlay_6.dts | 13 +-- drivers/of/unittest-data/overlay_7.dts | 13 +-- drivers/of/unittest-data/overlay_8.dts | 13 +-- drivers/of/unittest-data/overlay_9.dts | 13 +-- drivers/of/unittest-data/overlay_bad_phandle.dts | 23 ++ drivers/of/unittest-data/overlay_bad_symbol.dts | 25 ++ drivers/of/unittest-data/tests-overlay.dtsi | 4 +- 15 files changed, 148 insertions(+), 233 deletions(-) diff --git a/drivers/of/unittest-data/overlay.dts b/drivers/of/unittest-data/overlay.dts index ab5e89b5e27e..3bbc59e922fe 100644 --- a/drivers/of/unittest-data/overlay.dts +++ b/drivers/of/unittest-data/overlay.dts @@ -2,76 +2,63 @@ /dts-v1/; /plugin/; -/ { +&electric_1 { - fragment@0 { - target = <&electric_1>; + status = "okay"; - __overlay__ { - status = "okay"; - - hvac_2: hvac-large-1 { - compatible = "ot,hvac-large"; - heat-range = < 40 75 >; - cool-range = < 65 80 >; - }; - }; + hvac_2: hvac-large-1 { + compatible = "ot,hvac-large"; + heat-range = < 40 75 >; + cool-range = < 65 80 >; }; +}; - fragment@1 { - target = <&rides_1>; - - __overlay__ { - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - - ride@100 { - #address-cells = <1>; - #size-cells = <1>; - - track@30 { - incline-up = < 48 32 16 >; - }; +&rides_1 { - track@40 { - incline-up = < 47 31 15 >; - }; - }; + #address-cells = <1>; + #size-cells = <1>; + status = "okay"; - ride_200: ride@200 { - #address-cells = <1>; - #size-cells = <1>; - compatible = "ot,ferris-wheel"; - reg = < 0x0200 0x100 >; - hvac-provider = < &hvac_2 >; - hvac-thermostat = < 27 32 > ; - hvac-zones = < 12 5 >; - hvac-zone-names = "operator", "snack-bar"; - spin-controller = < &spin_ctrl_1 3 >; - spin-rph = < 30 >; - gondolas = < 16 >; - gondola-capacity = < 6 >; + ride@100 { + #address-cells = <1>; + #size-cells = <1>; - ride_200_left: track@10 { - reg = < 0x0010 0x10 >; - }; + track@30 { + incline-up = < 48 32 16 >; + }; - ride_200_right: track@20 { - reg = < 0x0020 0x10 >; - }; - }; + track@40 { + incline-up = < 47 31 15 >; }; }; - fragment@2 { - target = <&lights_2>; + ride_200: ride@200
[PATCH v3 0/4] of: change overlay apply input data from unflattened
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. These changes led to migrating some unittest overlay data into their own devicetree source files, and then converting most of them to use sugar syntax instead of hand coding fragments. Changes from v2: - improve error messages in patch 4/4, as suggested by Geert Changes from v1: - rebase on v4.16-rc1 - update documentation - split out error message to a separate patch Frank Rowand (4): of: change overlay apply input data from unflattened to FDT of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply() of: convert unittest overlay devicetree source to sugar syntax of: improve reporting invalid overlay target path Documentation/devicetree/overlay-notes.txt | 4 +- drivers/of/of_private.h | 1 + drivers/of/overlay.c | 129 -- drivers/of/resolver.c| 6 - drivers/of/unittest-data/Makefile| 28 ++- drivers/of/unittest-data/overlay.dts | 101 drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 27 +++ drivers/of/unittest-data/overlay_11.dts | 28 +++ drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 30 +++ drivers/of/unittest-data/overlay_2.dts | 9 + drivers/of/unittest-data/overlay_3.dts | 9 + drivers/of/unittest-data/overlay_4.dts | 18 ++ drivers/of/unittest-data/overlay_5.dts | 9 + drivers/of/unittest-data/overlay_6.dts | 10 + drivers/of/unittest-data/overlay_7.dts | 10 + drivers/of/unittest-data/overlay_8.dts | 10 + drivers/of/unittest-data/overlay_9.dts | 10 + drivers/of/unittest-data/overlay_bad_phandle.dts | 23 +- drivers/of/unittest-data/overlay_bad_symbol.dts | 25 +- drivers/of/unittest-data/tests-overlay.dtsi | 217 + drivers/of/unittest.c| 294 +++ include/linux/of.h | 7 - 26 files changed, 574 insertions(+), 487 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts create mode 100644 drivers/of/unittest-data/overlay_15.dts create mode 100644 drivers/of/unittest-data/overlay_2.dts create mode 100644 drivers/of/unittest-data/overlay_3.dts create mode 100644 drivers/of/unittest-data/overlay_4.dts create mode 100644 drivers/of/unittest-data/overlay_5.dts create mode 100644 drivers/of/unittest-data/overlay_6.dts create mode 100644 drivers/of/unittest-data/overlay_7.dts create mode 100644 drivers/of/unittest-data/overlay_8.dts create mode 100644 drivers/of/unittest-data/overlay_9.dts -- Frank Rowand
[PATCH v3] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()
From: Frank Rowand Create a cache of the nodes that contain a phandle property. Use this cache to find the node for a given phandle value instead of scanning the devicetree to find the node. If the phandle value is not found in the cache, of_find_node_by_phandle() will fall back to the tree scan algorithm. The cache is initialized in of_core_init(). The cache is freed via a late_initcall_sync() if modules are not enabled. If the devicetree is created by the dtc compiler, with all phandle property values auto generated, then the size required by the cache could be 4 * (1 + number of phandles) bytes. This results in an O(1) node lookup cost for a given phandle value. Due to a concern that the phandle property values might not be consistent with what is generated by the dtc compiler, a mask has been added to the cache lookup algorithm. To maintain the O(1) node lookup cost, the size of the cache has been increased by rounding the number of entries up to the next power of two. The overhead of finding the devicetree node containing a given phandle value has been noted by several people in the recent past, in some cases with a patch to add a hashed index of devicetree nodes, based on the phandle value of the node. One concern with this approach is the extra space added to each node. This patch takes advantage of the phandle property values auto generated by the dtc compiler, which begin with one and monotonically increase by one, resulting in a range of 1..n for n phandle values. This implementation should also provide a good reduction of overhead for any range of phandle values that are mostly in a monotonic range. Performance measurements by Chintan Pandya of several implementations of patches that are similar to this one suggest an expected reduction of boot time by ~400ms for his test system. If the cache size was decreased to 64 entries, the boot time was reduced by ~340 ms. The measurements were on a 4.9.73 kernel for arch/arm64/boot/dts/qcom/sda670-mtp.dts, contains 2371 nodes and 814 phandle values. Reported-by: Chintan Pandya Signed-off-by: Frank Rowand --- A follow on patch will add an early boot allocation of the cache. Changes since v2: - add mask to calculation of phandle cache entry - which results in better overhead reduction for devicetrees with phandle properties not allocated in the monotonically increasing range of 1..n - due to mask, number of entries in cache potentially increased to next power of two - minor fixes as suggested by reviewers - no longer using live_tree_max_phandle() so do not move it from drivers/of/resolver.c to drivers/of/base.c Changes since v1: - change short description from of: cache phandle nodes to reduce cost of of_find_node_by_phandle() - rebase on v4.16-rc1 - reorder new functions in base.c to avoid forward declaration - add locking around kfree(phandle_cache) for memory ordering - add explicit check for non-null of phandle_cache in of_find_node_by_phandle(). There is already a check for !handle, which prevents accessing a null phandle_cache, but that dependency is not obvious, so this check makes it more apparent. - do not free phandle_cache if modules are enabled, so that cached phandles will be available when modules are loaded drivers/of/base.c | 83 ++--- drivers/of/of_private.h | 3 ++ drivers/of/resolver.c | 3 -- 3 files changed, 82 insertions(+), 7 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index ad28de96e13f..ab545dfa9173 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -91,10 +91,69 @@ int __weak of_node_to_nid(struct device_node *np) } #endif +static struct device_node **phandle_cache; +static u32 phandle_cache_mask; + +/* + * Assumptions behind phandle_cache implementation: + * - phandle property values are in a contiguous range of 1..n + * + * If the assumptions do not hold, then + * - the phandle lookup overhead reduction provided by the cache + * will likely be less + */ +static void of_populate_phandle_cache(void) +{ + unsigned long flags; + u32 cache_entries; + struct device_node *np; + u32 phandles = 0; + + raw_spin_lock_irqsave(&devtree_lock, flags); + + kfree(phandle_cache); + phandle_cache = NULL; + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandles++; + + cache_entries = roundup_pow_of_two(phandles); + phandle_cache_mask = cache_entries - 1; + + phandle_cache = kcalloc(cache_entries, sizeof(*phandle_cache), + GFP_ATOMIC); + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandle_cache[np->phandle & phandle_cache_mask] = np; + + raw_spin_unlock_irqrestore(&devtree_lock, flags); +} + +#ifndef CONFIG_MODULES +sta
[PATCH] of: add early boot allocation of of_find_node_by_phandle() cache
From: Frank Rowand The initial implementation of the of_find_node_by_phandle() cache allocates the cache using kcalloc(). Add an early boot allocation of the cache so it will be usable during early boot. Switch over to the kcalloc() based cache once normal memory allocation becomes available. Signed-off-by: Frank Rowand --- This patch is optional, to be added at Rob's discretion. The extra complexity is not as much as I had feared, but the boot speed up is also likely small. drivers/of/base.c | 33 + drivers/of/fdt.c| 2 ++ drivers/of/of_private.h | 2 ++ 3 files changed, 37 insertions(+) diff --git a/drivers/of/base.c b/drivers/of/base.c index ab545dfa9173..d7b1ff1209e8 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -16,9 +16,11 @@ #define pr_fmt(fmt)"OF: " fmt +#include #include #include #include +#include #include #include #include @@ -131,6 +133,29 @@ static void of_populate_phandle_cache(void) raw_spin_unlock_irqrestore(&devtree_lock, flags); } +void __init of_populate_phandle_cache_early(void) +{ + u32 cache_entries; + struct device_node *np; + u32 phandles = 0; + size_t size; + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandles++; + + cache_entries = roundup_pow_of_two(phandles); + phandle_cache_mask = cache_entries - 1; + + size = cache_entries * sizeof(*phandle_cache); + phandle_cache = memblock_virt_alloc(size, 4); + memset(phandle_cache, 0, size); + + for_each_of_allnodes(np) + if (np->phandle && np->phandle != OF_PHANDLE_ILLEGAL) + phandle_cache[np->phandle & phandle_cache_mask] = np; +} + #ifndef CONFIG_MODULES static int __init of_free_phandle_cache(void) { @@ -150,7 +175,15 @@ static int __init of_free_phandle_cache(void) void __init of_core_init(void) { + unsigned long flags; struct device_node *np; + phys_addr_t size; + + raw_spin_lock_irqsave(&devtree_lock, flags); + size = (phandle_cache_mask + 1) * sizeof(*phandle_cache); + memblock_free(__pa(phandle_cache), size); + phandle_cache = NULL; + raw_spin_unlock_irqrestore(&devtree_lock, flags); of_populate_phandle_cache(); diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 84aa9d676375..cb320df23f26 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -1264,6 +1264,8 @@ void __init unflatten_device_tree(void) of_alias_scan(early_init_dt_alloc_memory_arch); unittest_unflatten_overlay_base(); + + of_populate_phandle_cache_early(); } /** diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index fa70650136b4..6720448c84cc 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -134,6 +134,8 @@ extern void __of_sysfs_remove_bin_file(struct device_node *np, /* illegal phandle value (set when unresolved) */ #define OF_PHANDLE_ILLEGAL 0xdeadbeef +extern void __init of_populate_phandle_cache_early(void); + /* iterators for transactions, used for overlays */ /* forward iterator */ #define for_each_transaction_entry(_oft, _te) \ -- Frank Rowand
[PATCH v2 4/4] of: improve reporting invalid overlay target path
From: Frank Rowand Errors while developing the patch to create of_overlay_fdt_apply() exposed inadequate error messages to debug problems when overlay devicetree fragment nodes contain an invalid target path. Improve the messages in find_target_node() to remedy this. Signed-off-by: Frank Rowand --- Changes from v1: - split out from patch 1/2 drivers/of/overlay.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 5f6c5569e97d..01362e7e98e3 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -488,17 +488,26 @@ static int build_changeset(struct overlay_changeset *ovcs) */ static struct device_node *find_target_node(struct device_node *info_node) { + struct device_node *node; const char *path; u32 val; int ret; ret = of_property_read_u32(info_node, "target", &val); - if (!ret) - return of_find_node_by_phandle(val); + if (!ret) { + node = of_find_node_by_phandle(val); + if (!node) + pr_err("target node find by phandle failed\n"); + return node; + } ret = of_property_read_string(info_node, "target-path", &path); - if (!ret) - return of_find_node_by_path(path); + if (!ret) { + node = of_find_node_by_path(path); + if (!node) + pr_err("target node find by path failed\n"); + return node; + } pr_err("Failed to find target for node %p (%s)\n", info_node, info_node->name); -- Frank Rowand
[PATCH v2 1/4] of: change overlay apply input data from unflattened to FDT
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. of_overlay_fdt_apply() is a private function for the anticipated overlay loader. Update unittest.c to use of_overlay_fdt_apply() instead of of_overlay_apply(). Move overlay fragments from artificial locations in drivers/of/unittest-data/tests-overlay.dtsi into one devicetree source file per overlay. This led to changes in drivers/of/unitest-data/Makefile and drivers/of/unitest.c. - Add overlay directives to the overlay devicetree source files so that dtc will compile them as true overlays into one FDT data chunk per overlay. - Set CFLAGS for drivers/of/unittest-data/testcases.dts so that symbols will be generated for overlay resolution of overlays that are no longer artificially contained in testcases.dts - Unflatten and apply each unittest overlay FDT using of_overlay_fdt_apply(). - Enable the of_resolve_phandles() check for whether the unflattened overlay is detached. This check was previously disabled because the overlays from tests-overlay.dtsi were not unflattened into detached trees. - Other changes to unittest.c infrastructure to manage multiple test FDTs built into the kernel image (access by name instead of arbitrary number). - of_unittest_overlay_high_level(): previously unused code to add properties from the overlay_base devicetree to the live tree was triggered by the restructuring of tests-overlay.dtsi and thus testcases.dts. This exposed two bugs: (1) the need to dup a property before adding it, and (2) property 'name' is auto-generated in the unflatten code and thus will be a duplicate in the __symbols__ node - do not treat this duplicate as an error. Signed-off-by: Frank Rowand --- Changes from v1: - rebase on v4.16-rc1 There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are "line over 80 characters" for either pre-existing long lines, or lines in devicetree source files. drivers/of/of_private.h | 1 + drivers/of/overlay.c| 107 +- drivers/of/resolver.c | 6 - drivers/of/unittest-data/Makefile | 28 ++- drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 34 drivers/of/unittest-data/overlay_11.dts | 34 drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 35 drivers/of/unittest-data/overlay_2.dts | 14 ++ drivers/of/unittest-data/overlay_3.dts | 14 ++ drivers/of/unittest-data/overlay_4.dts | 23 +++ drivers/of/unittest-data/overlay_5.dts | 14 ++ drivers/of/unittest-data/overlay_6.dts | 15 ++ drivers/of/unittest-data/overlay_7.dts | 15 ++ drivers/of/unittest-data/overlay_8.dts | 15 ++ drivers/of/unittest-data/overlay_9.dts | 15 ++ drivers/of/unittest-data/tests-overlay.dtsi | 213 drivers/of/unittest.c | 294 ++-- include/linux/of.h | 7 - 22 files changed, 551 insertions(+), 389 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts create mode 100644 drivers/of/unittest-data/overlay_15.dts create mode 100644 drivers/of/unittest-data/overlay_2.dts create mode 100644 drivers/of/unittest-data/overlay_3.dts create mode 100644 drivers/of/unittest-data/overlay_4.dts create mode 100644 drivers/of/unittest-data/overlay_5.dts create mode 100644 drivers/of/unittest-data/overlay_6.dts create mode 100644 drivers/of/unittest-data/overlay_7.dts create mode 100644 drivers/of/unittest-data/overlay_8.dts create mode 100644 drivers/of/unittest-data/overlay_9.dts diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 18e03c9d4b55..ddc9b1a89979 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_pri
[PATCH v2 3/4] of: convert unittest overlay devicetree source to sugar syntax
From: Frank Rowand The unittest-data overlays have been pulled into proper overlay devicetree source files without changing their format. The next step is to convert them to use sugar syntax instead of hand coding overlay fragments structure. A few of the overlays can not be converted because they test absolute target paths in the overlay fragment. dtc does not generate this type of target: overlay_0.dts overlay_1.dts overlay_12.dts overlay_13.dts Two pre-existing unittest overlay devicetree source files are also converted: overlay_bad_phandle.dts overlay_bad_symbol.dts Signed-off-by: Frank Rowand --- Changes from v1: - rebase on v4.16-rc1 There are checkpatch warnings. I have reviewed them and feel they can be ignored. They are pre-existing warnings of un-documented bindings of made up (fake) compatibles in devicetree source for test overlays. drivers/of/unittest-data/overlay.dts | 101 ++- drivers/of/unittest-data/overlay_10.dts | 39 - drivers/of/unittest-data/overlay_11.dts | 40 - drivers/of/unittest-data/overlay_15.dts | 41 - drivers/of/unittest-data/overlay_2.dts | 11 +-- drivers/of/unittest-data/overlay_3.dts | 11 +-- drivers/of/unittest-data/overlay_4.dts | 23 ++ drivers/of/unittest-data/overlay_5.dts | 11 +-- drivers/of/unittest-data/overlay_6.dts | 13 +-- drivers/of/unittest-data/overlay_7.dts | 13 +-- drivers/of/unittest-data/overlay_8.dts | 13 +-- drivers/of/unittest-data/overlay_9.dts | 13 +-- drivers/of/unittest-data/overlay_bad_phandle.dts | 23 ++ drivers/of/unittest-data/overlay_bad_symbol.dts | 25 ++ drivers/of/unittest-data/tests-overlay.dtsi | 4 +- 15 files changed, 148 insertions(+), 233 deletions(-) diff --git a/drivers/of/unittest-data/overlay.dts b/drivers/of/unittest-data/overlay.dts index ab5e89b5e27e..3bbc59e922fe 100644 --- a/drivers/of/unittest-data/overlay.dts +++ b/drivers/of/unittest-data/overlay.dts @@ -2,76 +2,63 @@ /dts-v1/; /plugin/; -/ { +&electric_1 { - fragment@0 { - target = <&electric_1>; + status = "okay"; - __overlay__ { - status = "okay"; - - hvac_2: hvac-large-1 { - compatible = "ot,hvac-large"; - heat-range = < 40 75 >; - cool-range = < 65 80 >; - }; - }; + hvac_2: hvac-large-1 { + compatible = "ot,hvac-large"; + heat-range = < 40 75 >; + cool-range = < 65 80 >; }; +}; - fragment@1 { - target = <&rides_1>; - - __overlay__ { - #address-cells = <1>; - #size-cells = <1>; - status = "okay"; - - ride@100 { - #address-cells = <1>; - #size-cells = <1>; - - track@30 { - incline-up = < 48 32 16 >; - }; +&rides_1 { - track@40 { - incline-up = < 47 31 15 >; - }; - }; + #address-cells = <1>; + #size-cells = <1>; + status = "okay"; - ride_200: ride@200 { - #address-cells = <1>; - #size-cells = <1>; - compatible = "ot,ferris-wheel"; - reg = < 0x0200 0x100 >; - hvac-provider = < &hvac_2 >; - hvac-thermostat = < 27 32 > ; - hvac-zones = < 12 5 >; - hvac-zone-names = "operator", "snack-bar"; - spin-controller = < &spin_ctrl_1 3 >; - spin-rph = < 30 >; - gondolas = < 16 >; - gondola-capacity = < 6 >; + ride@100 { + #address-cells = <1>; + #size-cells = <1>; - ride_200_left: track@10 { - reg = < 0x0010 0x10 >; - }; + track@30 { + incline-up = < 48 32 16 >; + }; - ride_200_right: track@20 { - reg = < 0x0020 0x10 >; - }; - }; + track@40 { + incline-up = < 47 31 15 >; }; }; - fragment@2 { - target
[PATCH v2 0/4] of: change overlay apply input data from unflattened
From: Frank Rowand Move duplicating and unflattening of an overlay flattened devicetree (FDT) into the overlay application code. To accomplish this, of_overlay_apply() is replaced by of_overlay_fdt_apply(). The copy of the FDT (aka "duplicate FDT") now belongs to devicetree code, which is thus responsible for freeing the duplicate FDT. The caller of of_overlay_fdt_apply() remains responsible for freeing the original FDT. The unflattened devicetree now belongs to devicetree code, which is thus responsible for freeing the unflattened devicetree. These ownership changes prevent early freeing of the duplicated FDT or the unflattened devicetree, which could result in use after free errors. These changes led to migrating some unittest overlay data into their own devicetree source files, and then converting most of them to use sugar syntax instead of hand coding fragments. Changes from v1: - rebase on v4.16-rc1 - update documentation - split out error message to a separate patch Frank Rowand (4): of: change overlay apply input data from unflattened to FDT of: Documentation: of_overlay_apply() replaced by of_overlay_fdt_apply() of: convert unittest overlay devicetree source to sugar syntax of: improve reporting invalid overlay target path Documentation/devicetree/overlay-notes.txt | 4 +- drivers/of/of_private.h | 1 + drivers/of/overlay.c | 124 +- drivers/of/resolver.c| 6 - drivers/of/unittest-data/Makefile| 28 ++- drivers/of/unittest-data/overlay.dts | 101 drivers/of/unittest-data/overlay_0.dts | 14 ++ drivers/of/unittest-data/overlay_1.dts | 14 ++ drivers/of/unittest-data/overlay_10.dts | 27 +++ drivers/of/unittest-data/overlay_11.dts | 28 +++ drivers/of/unittest-data/overlay_12.dts | 14 ++ drivers/of/unittest-data/overlay_13.dts | 14 ++ drivers/of/unittest-data/overlay_15.dts | 30 +++ drivers/of/unittest-data/overlay_2.dts | 9 + drivers/of/unittest-data/overlay_3.dts | 9 + drivers/of/unittest-data/overlay_4.dts | 18 ++ drivers/of/unittest-data/overlay_5.dts | 9 + drivers/of/unittest-data/overlay_6.dts | 10 + drivers/of/unittest-data/overlay_7.dts | 10 + drivers/of/unittest-data/overlay_8.dts | 10 + drivers/of/unittest-data/overlay_9.dts | 10 + drivers/of/unittest-data/overlay_bad_phandle.dts | 23 +- drivers/of/unittest-data/overlay_bad_symbol.dts | 25 +- drivers/of/unittest-data/tests-overlay.dtsi | 217 + drivers/of/unittest.c| 294 +++ include/linux/of.h | 7 - 26 files changed, 571 insertions(+), 485 deletions(-) create mode 100644 drivers/of/unittest-data/overlay_0.dts create mode 100644 drivers/of/unittest-data/overlay_1.dts create mode 100644 drivers/of/unittest-data/overlay_10.dts create mode 100644 drivers/of/unittest-data/overlay_11.dts create mode 100644 drivers/of/unittest-data/overlay_12.dts create mode 100644 drivers/of/unittest-data/overlay_13.dts create mode 100644 drivers/of/unittest-data/overlay_15.dts create mode 100644 drivers/of/unittest-data/overlay_2.dts create mode 100644 drivers/of/unittest-data/overlay_3.dts create mode 100644 drivers/of/unittest-data/overlay_4.dts create mode 100644 drivers/of/unittest-data/overlay_5.dts create mode 100644 drivers/of/unittest-data/overlay_6.dts create mode 100644 drivers/of/unittest-data/overlay_7.dts create mode 100644 drivers/of/unittest-data/overlay_8.dts create mode 100644 drivers/of/unittest-data/overlay_9.dts -- Frank Rowand