* [PATCH 0/2] of: change overlay apply input data from EDT to FDT
@ 2018-01-29 2:53 frowand.list
2018-01-29 2:53 ` [PATCH 1/2] " frowand.list
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: frowand.list @ 2018-01-29 2:53 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: devicetree, linux-kernel, geert
From: Frank Rowand <frank.rowand@sony.com>
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 device tree (aka expanded device tree, EDT) now
belongs to devicetree code, which is thus responsible for freeing
the EDT.
These ownership changes prevent early freeing of the duplicated FDT
or the EDT, 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.
Frank Rowand (2):
of: change overlay apply input data from EDT to FDT
of: convert unittest overlay devicetree source to sugar syntax
drivers/of/of_private.h | 1 +
drivers/of/overlay.c | 124 +++++++++-
drivers/of/resolver.c | 6 -
drivers/of/unittest-data/Makefile | 16 ++
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 | 27 +--
drivers/of/unittest-data/tests-overlay.dtsi | 217 +----------------
drivers/of/unittest.c | 296 +++++++++++------------
include/linux/of.h | 7 -
25 files changed, 564 insertions(+), 480 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 <frank.rowand@sony.com>
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/2] of: change overlay apply input data from EDT to FDT 2018-01-29 2:53 [PATCH 0/2] of: change overlay apply input data from EDT to FDT frowand.list @ 2018-01-29 2:53 ` frowand.list 2018-01-29 3:21 ` Masahiro Yamada 2018-01-29 14:42 ` Rob Herring 2018-01-29 2:53 ` [PATCH 2/2] of: convert unittest overlay devicetree source to sugar syntax frowand.list 2018-01-29 14:08 ` [PATCH 0/2] of: change overlay apply input data from EDT to FDT Geert Uytterhoeven 2 siblings, 2 replies; 16+ messages in thread From: frowand.list @ 2018-01-29 2:53 UTC (permalink / raw) To: Rob Herring, pantelis.antoniou, Pantelis Antoniou Cc: devicetree, linux-kernel, geert From: Frank Rowand <frank.rowand@sony.com> 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 device tree (aka expanded device tree, EDT) now belongs to devicetree code, which is thus responsible for freeing the EDT. These ownership changes prevent early freeing of the duplicated FDT or the EDT, 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 overlay EDT 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. Errors while developing the patch exposed inadequate error messages to debug problems when overlay devicetree fragment nodes contain an invalid target path. Improved the messages in find_target_node(). Signed-off-by: Frank Rowand <frank.rowand@sony.com> --- Dependency, a patch already applied by robh for next: From: Masahiro Yamada <yamada.masahiro@socionext.com> Subject: [PATCH] of: unittest: refactor Makefile https://lkml.kernel.org/r/<1515153330-11810-1-git-send-email-yamada.masahiro@socionext.com> Created on 4.15. I will respin this patch on top of 4.15-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 | 124 ++++++++++-- drivers/of/resolver.c | 6 - drivers/of/unittest-data/Makefile | 16 ++ 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 | 296 ++++++++++++++-------------- include/linux/of.h | 7 - 22 files changed, 558 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 92a9a3687446..4328662e9d8d 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -81,6 +81,7 @@ static inline void __of_detach_node_sysfs(struct device_node *np) {} #endif #if defined(CONFIG_OF_OVERLAY) +int of_overlay_fdt_apply(void *overlay_fdt, int *ovcs_id); void of_overlay_mutex_lock(void); void of_overlay_mutex_unlock(void); #else diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 3981b7da4fa9..7f26ff47c012 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -15,10 +15,12 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/of_fdt.h> #include <linux/string.h> #include <linux/ctype.h> #include <linux/errno.h> #include <linux/slab.h> +#include <linux/libfdt.h> #include <linux/err.h> #include <linux/idr.h> @@ -36,7 +38,9 @@ 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 * @overlay_tree: expanded device tree that contains the fragment nodes * @count: count of fragment structures * @fragments: fragment nodes in the overlay expanded device tree @@ -46,6 +50,7 @@ struct fragment { struct overlay_changeset { int id; struct list_head ovcs_list; + const void *fdt; struct device_node *overlay_tree; int count; struct fragment *fragments; @@ -486,17 +491,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); @@ -506,7 +520,8 @@ static struct device_node *find_target_node(struct device_node *info_node) /** * init_overlay_changeset() - initialize overlay changeset from overlay tree - * @ovcs Overlay changeset to build + * @ovcs: Overlay changeset to build + * @fdt: the FDT that was unflattened to create @tree * @tree: Contains all the overlay fragments and overlay fixup nodes * * Initialize @ovcs. Populate @ovcs->fragments with node information from @@ -517,7 +532,7 @@ static struct device_node *find_target_node(struct device_node *info_node) * detected in @tree, or -ENOSPC if idr_alloc() error. */ static int init_overlay_changeset(struct overlay_changeset *ovcs, - struct device_node *tree) + const void *fdt, struct device_node *tree) { struct device_node *node, *overlay_node; struct fragment *fragment; @@ -538,6 +553,7 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs, pr_debug("%s() tree is not root\n", __func__); ovcs->overlay_tree = tree; + ovcs->fdt = fdt; INIT_LIST_HEAD(&ovcs->ovcs_list); @@ -609,6 +625,7 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs, } if (!cnt) { + pr_err("no fragments or symbols in overlay\n"); ret = -EINVAL; goto err_free_fragments; } @@ -645,11 +662,24 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) } kfree(ovcs->fragments); + /* + * TODO + * + * would like to: kfree(ovcs->overlay_tree); + * but can not since drivers may have pointers into this data + * + * would like to: kfree(ovcs->fdt); + * but can not since drivers may have pointers into this data + */ + kfree(ovcs); } -/** +/* + * internal documentation + * * of_overlay_apply() - Create and apply an overlay changeset + * @fdt: the FDT that was unflattened to create @tree * @tree: Expanded overlay device tree * @ovcs_id: Pointer to overlay changeset id * @@ -688,21 +718,29 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) * id is returned to *ovcs_id. */ -int of_overlay_apply(struct device_node *tree, int *ovcs_id) +static int of_overlay_apply(const void *fdt, struct device_node *tree, + int *ovcs_id) { struct overlay_changeset *ovcs; int ret = 0, ret_revert, ret_tmp; - *ovcs_id = 0; + /* + * As of this point, fdt and tree belong to the overlay changeset. + * overlay changeset code is responsible for freeing them. + */ if (devicetree_corrupt()) { pr_err("devicetree state suspect, refuse to apply overlay\n"); + kfree(fdt); + kfree(tree); ret = -EBUSY; goto out; } ovcs = kzalloc(sizeof(*ovcs), GFP_KERNEL); if (!ovcs) { + kfree(fdt); + kfree(tree); ret = -ENOMEM; goto out; } @@ -712,12 +750,17 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id) ret = of_resolve_phandles(tree); if (ret) - goto err_free_overlay_changeset; + goto err_free_tree; - ret = init_overlay_changeset(ovcs, tree); + ret = init_overlay_changeset(ovcs, fdt, tree); if (ret) - goto err_free_overlay_changeset; + goto err_free_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 + */ ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY); if (ret) { pr_err("overlay changeset pre-apply notify error %d\n", ret); @@ -757,6 +800,9 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id) goto out_unlock; +err_free_tree: + kfree(tree); + err_free_overlay_changeset: free_overlay_changeset(ovcs); @@ -769,7 +815,59 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id) return ret; } -EXPORT_SYMBOL_GPL(of_overlay_apply); + +int of_overlay_fdt_apply(void *overlay_fdt, int *ovcs_id) +{ + const void *new_fdt; + int ret; + u32 size; + struct device_node *overlay_root; + + *ovcs_id = 0; + ret = 0; + + if (fdt_check_header(overlay_fdt)) { + pr_err("Invalid overlay_fdt header\n"); + return -EINVAL; + } + + size = fdt_totalsize(overlay_fdt); + + /* + * Must create permanent copy of FDT because of_fdt_unflatten_tree() + * will create pointers to the passed in FDT in the EDT. + */ + new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL); + if (!new_fdt) + return -ENOMEM; + + of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root); + if (!overlay_root) { + pr_err("unable to unflatten overlay_fdt\n"); + ret = -EINVAL; + goto out_free_new_fdt; + } + + ret = of_overlay_apply(new_fdt, overlay_root, ovcs_id); + if (ret < 0) { + /* + * new_fdt and overlay_root now belong to the overlay + * changeset. + * overlay changeset code is responsible for freeing them. + */ + goto out; + } + + return 0; + + +out_free_new_fdt: + kfree(new_fdt); + +out: + return ret; +} +EXPORT_SYMBOL_GPL(of_overlay_fdt_apply); /* * Find @np in @tree. diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index cfaeef5f6cb1..dcf34853c536 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -272,17 +272,11 @@ int of_resolve_phandles(struct device_node *overlay) goto out; } -#if 0 - Temporarily disable check so that old style overlay unittests - do not fail when of_resolve_phandles() is moved into - of_overlay_apply(). - if (!of_node_check_flag(overlay, OF_DETACHED)) { pr_err("overlay not detached\n"); err = -EINVAL; goto out; } -#endif phandle_delta = live_tree_max_phandle() + 1; adjust_overlay_phandles(overlay, phandle_delta); diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile index df697976740a..2b7ee68c908e 100644 --- a/drivers/of/unittest-data/Makefile +++ b/drivers/of/unittest-data/Makefile @@ -3,6 +3,21 @@ DTC_FLAGS_testcases := -Wno-interrupts_property obj-y += testcases.dtb.o obj-$(CONFIG_OF_OVERLAY) += overlay.dtb.o \ + overlay_0.dtb.o \ + overlay_1.dtb.o \ + overlay_2.dtb.o \ + overlay_3.dtb.o \ + overlay_4.dtb.o \ + overlay_5.dtb.o \ + overlay_6.dtb.o \ + overlay_7.dtb.o \ + overlay_8.dtb.o \ + overlay_9.dtb.o \ + overlay_10.dtb.o \ + overlay_11.dtb.o \ + overlay_12.dtb.o \ + overlay_13.dtb.o \ + overlay_15.dtb.o \ overlay_bad_phandle.dtb.o \ overlay_bad_symbol.dtb.o \ overlay_base.dtb.o @@ -14,6 +29,7 @@ DTC_FLAGS_overlay := -@ DTC_FLAGS_overlay_bad_phandle := -@ DTC_FLAGS_overlay_bad_symbol := -@ DTC_FLAGS_overlay_base := -@ +DTC_FLAGS_testcases := -@ .PRECIOUS: \ $(obj)/%.dtb.S \ diff --git a/drivers/of/unittest-data/overlay_0.dts b/drivers/of/unittest-data/overlay_0.dts new file mode 100644 index 000000000000..ac0f9e0fe65f --- /dev/null +++ b/drivers/of/unittest-data/overlay_0.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_0 - enable using absolute target path */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/test-unittest0"; + __overlay__ { + status = "okay"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_1.dts b/drivers/of/unittest-data/overlay_1.dts new file mode 100644 index 000000000000..e92a626e2948 --- /dev/null +++ b/drivers/of/unittest-data/overlay_1.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_1 - disable using absolute target path */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/test-unittest1"; + __overlay__ { + status = "disabled"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_10.dts b/drivers/of/unittest-data/overlay_10.dts new file mode 100644 index 000000000000..445925a10cd3 --- /dev/null +++ b/drivers/of/unittest-data/overlay_10.dts @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_10 */ + /* overlays 8, 9, 10, 11 application and removal in bad sequence */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus"; + __overlay__ { + + /* suppress DTC warning */ + #address-cells = <1>; + #size-cells = <0>; + + test-unittest10 { + compatible = "unittest"; + status = "okay"; + reg = <10>; + + #address-cells = <1>; + #size-cells = <0>; + + test-unittest101 { + compatible = "unittest"; + status = "okay"; + reg = <1>; + }; + + }; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_11.dts b/drivers/of/unittest-data/overlay_11.dts new file mode 100644 index 000000000000..c1d14f34359e --- /dev/null +++ b/drivers/of/unittest-data/overlay_11.dts @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_11 */ + /* overlays 8, 9, 10, 11 application and removal in bad sequence */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus"; + __overlay__ { + + /* suppress DTC warning */ + #address-cells = <1>; + #size-cells = <0>; + + test-unittest11 { + compatible = "unittest"; + status = "okay"; + reg = <11>; + + #address-cells = <1>; + #size-cells = <0>; + + test-unittest111 { + compatible = "unittest"; + status = "okay"; + reg = <1>; + }; + + }; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_12.dts b/drivers/of/unittest-data/overlay_12.dts new file mode 100644 index 000000000000..ca3441e2cbec --- /dev/null +++ b/drivers/of/unittest-data/overlay_12.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_12 - enable using absolute target path (i2c) */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest12"; + __overlay__ { + status = "okay"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_13.dts b/drivers/of/unittest-data/overlay_13.dts new file mode 100644 index 000000000000..3c30dec63894 --- /dev/null +++ b/drivers/of/unittest-data/overlay_13.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_13 - disable using absolute target path (i2c) */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest13"; + __overlay__ { + status = "disabled"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_15.dts b/drivers/of/unittest-data/overlay_15.dts new file mode 100644 index 000000000000..44e44c62b739 --- /dev/null +++ b/drivers/of/unittest-data/overlay_15.dts @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_15 - mux overlay */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/i2c-test-bus"; + __overlay__ { + #address-cells = <1>; + #size-cells = <0>; + test-unittest15 { + reg = <11>; + compatible = "unittest-i2c-mux"; + status = "okay"; + + #address-cells = <1>; + #size-cells = <0>; + + i2c@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + + test-mux-dev { + reg = <32>; + compatible = "unittest-i2c-dev"; + status = "okay"; + }; + }; + }; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_2.dts b/drivers/of/unittest-data/overlay_2.dts new file mode 100644 index 000000000000..cf1e4245b7ce --- /dev/null +++ b/drivers/of/unittest-data/overlay_2.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_2 - enable using label */ + + fragment@0 { + target = <&unittest2>; + __overlay__ { + status = "okay"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_3.dts b/drivers/of/unittest-data/overlay_3.dts new file mode 100644 index 000000000000..158dc44fc20a --- /dev/null +++ b/drivers/of/unittest-data/overlay_3.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_3 - disable using label */ + + fragment@0 { + target = <&unittest3>; + __overlay__ { + status = "disabled"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_4.dts b/drivers/of/unittest-data/overlay_4.dts new file mode 100644 index 000000000000..b4a2e6c6b016 --- /dev/null +++ b/drivers/of/unittest-data/overlay_4.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_4 - test insertion of a full node */ + + fragment@0 { + target = <&unittestbus>; + __overlay__ { + + /* suppress DTC warning */ + #address-cells = <1>; + #size-cells = <0>; + + test-unittest4 { + compatible = "unittest"; + status = "okay"; + reg = <4>; + }; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_5.dts b/drivers/of/unittest-data/overlay_5.dts new file mode 100644 index 000000000000..02ad25c1f19c --- /dev/null +++ b/drivers/of/unittest-data/overlay_5.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_5 - test overlay apply revert */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/test-unittest5"; + __overlay__ { + status = "okay"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_6.dts b/drivers/of/unittest-data/overlay_6.dts new file mode 100644 index 000000000000..a14e965f5497 --- /dev/null +++ b/drivers/of/unittest-data/overlay_6.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_6 */ + /* overlays 6, 7 application and removal in sequence */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/test-unittest6"; + __overlay__ { + status = "okay"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_7.dts b/drivers/of/unittest-data/overlay_7.dts new file mode 100644 index 000000000000..4bd7e423209c --- /dev/null +++ b/drivers/of/unittest-data/overlay_7.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_7 */ + /* overlays 6, 7 application and removal in sequence */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/test-unittest7"; + __overlay__ { + status = "okay"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_8.dts b/drivers/of/unittest-data/overlay_8.dts new file mode 100644 index 000000000000..5b21c53945a9 --- /dev/null +++ b/drivers/of/unittest-data/overlay_8.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_8 */ + /* overlays 8, 9, 10, 11 application and removal in bad sequence */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/test-unittest8"; + __overlay__ { + status = "okay"; + }; + }; +}; diff --git a/drivers/of/unittest-data/overlay_9.dts b/drivers/of/unittest-data/overlay_9.dts new file mode 100644 index 000000000000..20ff055a5349 --- /dev/null +++ b/drivers/of/unittest-data/overlay_9.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +/plugin/; + +/ { + /* overlay_9 */ + /* overlays 8, 9, 10, 11 application and removal in bad sequence */ + + fragment@0 { + target-path = "/testcase-data/overlay-node/test-bus/test-unittest8"; + __overlay__ { + property-foo = "bar"; + }; + }; +}; diff --git a/drivers/of/unittest-data/tests-overlay.dtsi b/drivers/of/unittest-data/tests-overlay.dtsi index 7b8001ab9f3a..fa2fb43bccac 100644 --- a/drivers/of/unittest-data/tests-overlay.dtsi +++ b/drivers/of/unittest-data/tests-overlay.dtsi @@ -113,218 +113,5 @@ }; }; }; - - /* test enable using absolute target path */ - overlay0 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest0"; - __overlay__ { - status = "okay"; - }; - }; - }; - - /* test disable using absolute target path */ - overlay1 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest1"; - __overlay__ { - status = "disabled"; - }; - }; - }; - - /* test enable using label */ - overlay2 { - fragment@0 { - target = <&unittest2>; - __overlay__ { - status = "okay"; - }; - }; - }; - - /* test disable using label */ - overlay3 { - fragment@0 { - target = <&unittest3>; - __overlay__ { - status = "disabled"; - }; - }; - }; - - /* test insertion of a full node */ - overlay4 { - fragment@0 { - target = <&unittestbus>; - __overlay__ { - - /* suppress DTC warning */ - #address-cells = <1>; - #size-cells = <0>; - - test-unittest4 { - compatible = "unittest"; - status = "okay"; - reg = <4>; - }; - }; - }; - }; - - /* test overlay apply revert */ - overlay5 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest5"; - __overlay__ { - status = "okay"; - }; - }; - }; - - /* test overlays application and removal in sequence */ - overlay6 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest6"; - __overlay__ { - status = "okay"; - }; - }; - }; - overlay7 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest7"; - __overlay__ { - status = "okay"; - }; - }; - }; - - /* test overlays application and removal in bad sequence */ - overlay8 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest8"; - __overlay__ { - status = "okay"; - }; - }; - }; - overlay9 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest8"; - __overlay__ { - property-foo = "bar"; - }; - }; - }; - - overlay10 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus"; - __overlay__ { - - /* suppress DTC warning */ - #address-cells = <1>; - #size-cells = <0>; - - test-unittest10 { - compatible = "unittest"; - status = "okay"; - reg = <10>; - - #address-cells = <1>; - #size-cells = <0>; - - test-unittest101 { - compatible = "unittest"; - status = "okay"; - reg = <1>; - }; - - }; - }; - }; - }; - - overlay11 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus"; - __overlay__ { - - /* suppress DTC warning */ - #address-cells = <1>; - #size-cells = <0>; - - test-unittest11 { - compatible = "unittest"; - status = "okay"; - reg = <11>; - - #address-cells = <1>; - #size-cells = <0>; - - test-unittest111 { - compatible = "unittest"; - status = "okay"; - reg = <1>; - }; - - }; - }; - }; - }; - - /* test enable using absolute target path (i2c) */ - overlay12 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest12"; - __overlay__ { - status = "okay"; - }; - }; - }; - - /* test disable using absolute target path (i2c) */ - overlay13 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/i2c-test-bus/test-unittest13"; - __overlay__ { - status = "disabled"; - }; - }; - }; - - /* test mux overlay */ - overlay15 { - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/i2c-test-bus"; - __overlay__ { - #address-cells = <1>; - #size-cells = <0>; - test-unittest15 { - reg = <11>; - compatible = "unittest-i2c-mux"; - status = "okay"; - - #address-cells = <1>; - #size-cells = <0>; - - i2c@0 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0>; - - test-mux-dev { - reg = <32>; - compatible = "unittest-i2c-dev"; - status = "okay"; - }; - }; - }; - }; - }; - }; - }; }; diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 0f8052f1355c..e220d27e53bb 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -44,6 +44,8 @@ failed; \ }) +static int __init overlay_data_apply(const char *overlay_name, int *overlay_id); + static void __init of_unittest_find_node_by_name(void) { struct device_node *np; @@ -996,8 +998,7 @@ static int __init unittest_data_add(void) } /* - * This lock normally encloses of_overlay_apply() as well as - * of_resolve_phandles(). + * This lock normally encloses of_resolve_phandles() */ of_overlay_mutex_lock(); @@ -1190,12 +1191,12 @@ static int of_unittest_device_exists(int unittest_nr, enum overlay_type ovtype) return 0; } -static const char *overlay_path(int nr) +static const char *overlay_name_from_nr(int nr) { static char buf[256]; snprintf(buf, sizeof(buf) - 1, - "/testcase-data/overlay%d", nr); + "overlay_%d", nr); buf[sizeof(buf) - 1] = '\0'; return buf; @@ -1262,25 +1263,19 @@ static void of_unittest_destroy_tracked_overlays(void) } while (defers > 0); } -static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr, +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; - np = of_find_node_by_path(overlay_path(overlay_nr)); - if (np == NULL) { - unittest(0, "could not find overlay node @\"%s\"\n", - overlay_path(overlay_nr)); - ret = -EINVAL; - goto out; - } + overlay_name = overlay_name_from_nr(overlay_nr); - *overlay_id = 0; - ret = of_overlay_apply(np, overlay_id); - if (ret < 0) { - unittest(0, "could not create overlay from \"%s\"\n", - overlay_path(overlay_nr)); + ret = overlay_data_apply(overlay_name, overlay_id); + if (!ret) { + unittest(0, "could not apply overlay \"%s\"\n", + overlay_name); goto out; } of_unittest_track_overlay(*overlay_id); @@ -1294,15 +1289,16 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr, } /* apply an overlay while checking before and after states */ -static int of_unittest_apply_overlay_check(int overlay_nr, int unittest_nr, - int before, int after, enum overlay_type ovtype) +static int __init of_unittest_apply_overlay_check(int overlay_nr, + int unittest_nr, int before, int after, + enum overlay_type ovtype) { int ret, ovcs_id; /* unittest device must not be in before state */ if (of_unittest_device_exists(unittest_nr, ovtype) != before) { - unittest(0, "overlay @\"%s\" with device @\"%s\" %s\n", - overlay_path(overlay_nr), + unittest(0, "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), unittest_path(unittest_nr, ovtype), !before ? "enabled" : "disabled"); return -EINVAL; @@ -1317,8 +1313,8 @@ static int of_unittest_apply_overlay_check(int overlay_nr, int unittest_nr, /* unittest device must be to set to after state */ if (of_unittest_device_exists(unittest_nr, ovtype) != after) { - unittest(0, "overlay @\"%s\" failed to create @\"%s\" %s\n", - overlay_path(overlay_nr), + unittest(0, "%s failed to create @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), unittest_path(unittest_nr, ovtype), !after ? "enabled" : "disabled"); return -EINVAL; @@ -1336,8 +1332,8 @@ static int of_unittest_apply_revert_overlay_check(int overlay_nr, /* unittest device must be in before state */ if (of_unittest_device_exists(unittest_nr, ovtype) != before) { - unittest(0, "overlay @\"%s\" with device @\"%s\" %s\n", - overlay_path(overlay_nr), + unittest(0, "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), unittest_path(unittest_nr, ovtype), !before ? "enabled" : "disabled"); return -EINVAL; @@ -1353,8 +1349,8 @@ static int of_unittest_apply_revert_overlay_check(int overlay_nr, /* unittest device must be in after state */ if (of_unittest_device_exists(unittest_nr, ovtype) != after) { - unittest(0, "overlay @\"%s\" failed to create @\"%s\" %s\n", - overlay_path(overlay_nr), + unittest(0, "%s failed to create @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), unittest_path(unittest_nr, ovtype), !after ? "enabled" : "disabled"); return -EINVAL; @@ -1362,16 +1358,16 @@ static int of_unittest_apply_revert_overlay_check(int overlay_nr, ret = of_overlay_remove(&ovcs_id); if (ret != 0) { - unittest(0, "overlay @\"%s\" failed to be destroyed @\"%s\"\n", - overlay_path(overlay_nr), + unittest(0, "%s failed to be destroyed @\"%s\"\n", + overlay_name_from_nr(overlay_nr), unittest_path(unittest_nr, ovtype)); return ret; } /* unittest device must be again in before state */ if (of_unittest_device_exists(unittest_nr, PDEV_OVERLAY) != before) { - unittest(0, "overlay @\"%s\" with device @\"%s\" %s\n", - overlay_path(overlay_nr), + unittest(0, "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), unittest_path(unittest_nr, ovtype), !before ? "enabled" : "disabled"); return -EINVAL; @@ -1381,7 +1377,7 @@ static int of_unittest_apply_revert_overlay_check(int overlay_nr, } /* test activation of device */ -static void of_unittest_overlay_0(void) +static void __init of_unittest_overlay_0(void) { int ret; @@ -1394,7 +1390,7 @@ static void of_unittest_overlay_0(void) } /* test deactivation of device */ -static void of_unittest_overlay_1(void) +static void __init of_unittest_overlay_1(void) { int ret; @@ -1407,7 +1403,7 @@ static void of_unittest_overlay_1(void) } /* test activation of device */ -static void of_unittest_overlay_2(void) +static void __init of_unittest_overlay_2(void) { int ret; @@ -1420,7 +1416,7 @@ static void of_unittest_overlay_2(void) } /* test deactivation of device */ -static void of_unittest_overlay_3(void) +static void __init of_unittest_overlay_3(void) { int ret; @@ -1433,7 +1429,7 @@ static void of_unittest_overlay_3(void) } /* test activation of a full device node */ -static void of_unittest_overlay_4(void) +static void __init of_unittest_overlay_4(void) { int ret; @@ -1459,19 +1455,19 @@ static void of_unittest_overlay_5(void) } /* test overlay application in sequence */ -static void of_unittest_overlay_6(void) +static void __init of_unittest_overlay_6(void) { - struct device_node *np; int ret, i, ov_id[2], ovcs_id; int overlay_nr = 6, unittest_nr = 6; int before = 0, after = 1; + const char *overlay_name; /* unittest device must be in before state */ for (i = 0; i < 2; i++) { if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY) != before) { - unittest(0, "overlay @\"%s\" with device @\"%s\" %s\n", - overlay_path(overlay_nr + i), + unittest(0, "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr + i), unittest_path(unittest_nr + i, PDEV_OVERLAY), !before ? "enabled" : "disabled"); @@ -1482,18 +1478,12 @@ static void of_unittest_overlay_6(void) /* apply the overlays */ for (i = 0; i < 2; i++) { - np = of_find_node_by_path(overlay_path(overlay_nr + i)); - if (np == NULL) { - unittest(0, "could not find overlay node @\"%s\"\n", - overlay_path(overlay_nr + i)); - return; - } + overlay_name = overlay_name_from_nr(overlay_nr + i); - ovcs_id = 0; - ret = of_overlay_apply(np, &ovcs_id); - if (ret < 0) { - unittest(0, "could not create overlay from \"%s\"\n", - overlay_path(overlay_nr + i)); + ret = overlay_data_apply(overlay_name, &ovcs_id); + if (!ret) { + unittest(0, "could not apply overlay \"%s\"\n", + overlay_name); return; } ov_id[i] = ovcs_id; @@ -1505,7 +1495,7 @@ static void of_unittest_overlay_6(void) if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY) != after) { unittest(0, "overlay @\"%s\" failed @\"%s\" %s\n", - overlay_path(overlay_nr + i), + overlay_name_from_nr(overlay_nr + i), unittest_path(unittest_nr + i, PDEV_OVERLAY), !after ? "enabled" : "disabled"); @@ -1517,8 +1507,8 @@ static void of_unittest_overlay_6(void) ovcs_id = ov_id[i]; ret = of_overlay_remove(&ovcs_id); if (ret != 0) { - unittest(0, "overlay @\"%s\" failed destroy @\"%s\"\n", - overlay_path(overlay_nr + i), + unittest(0, "%s failed destroy @\"%s\"\n", + overlay_name_from_nr(overlay_nr + i), unittest_path(unittest_nr + i, PDEV_OVERLAY)); return; @@ -1530,8 +1520,8 @@ static void of_unittest_overlay_6(void) /* unittest device must be again in before state */ if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY) != before) { - unittest(0, "overlay @\"%s\" with device @\"%s\" %s\n", - overlay_path(overlay_nr + i), + unittest(0, "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr + i), unittest_path(unittest_nr + i, PDEV_OVERLAY), !before ? "enabled" : "disabled"); @@ -1543,29 +1533,23 @@ static void of_unittest_overlay_6(void) } /* test overlay application in sequence */ -static void of_unittest_overlay_8(void) +static void __init of_unittest_overlay_8(void) { - struct device_node *np; int ret, i, ov_id[2], ovcs_id; int overlay_nr = 8, unittest_nr = 8; + const char *overlay_name; /* we don't care about device state in this test */ /* apply the overlays */ for (i = 0; i < 2; i++) { - np = of_find_node_by_path(overlay_path(overlay_nr + i)); - if (np == NULL) { - unittest(0, "could not find overlay node @\"%s\"\n", - overlay_path(overlay_nr + i)); - return; - } + overlay_name = overlay_name_from_nr(overlay_nr + i); - ovcs_id = 0; - ret = of_overlay_apply(np, &ovcs_id); + ret = overlay_data_apply(overlay_name, &ovcs_id); if (ret < 0) { - unittest(0, "could not create overlay from \"%s\"\n", - overlay_path(overlay_nr + i)); + unittest(0, "could not apply overlay \"%s\"\n", + overlay_name); return; } ov_id[i] = ovcs_id; @@ -1576,8 +1560,8 @@ static void of_unittest_overlay_8(void) ovcs_id = ov_id[0]; ret = of_overlay_remove(&ovcs_id); if (ret == 0) { - unittest(0, "overlay @\"%s\" was destroyed @\"%s\"\n", - overlay_path(overlay_nr + 0), + unittest(0, "%s was destroyed @\"%s\"\n", + overlay_name_from_nr(overlay_nr + 0), unittest_path(unittest_nr, PDEV_OVERLAY)); return; @@ -1588,8 +1572,8 @@ static void of_unittest_overlay_8(void) ovcs_id = ov_id[i]; ret = of_overlay_remove(&ovcs_id); if (ret != 0) { - unittest(0, "overlay @\"%s\" not destroyed @\"%s\"\n", - overlay_path(overlay_nr + i), + unittest(0, "%s not destroyed @\"%s\"\n", + overlay_name_from_nr(overlay_nr + i), unittest_path(unittest_nr, PDEV_OVERLAY)); return; @@ -1601,7 +1585,7 @@ static void of_unittest_overlay_8(void) } /* test insertion of a bus with parent devices */ -static void of_unittest_overlay_10(void) +static void __init of_unittest_overlay_10(void) { int ret; char *child_path; @@ -1890,7 +1874,7 @@ static void of_unittest_overlay_i2c_cleanup(void) i2c_del_driver(&unittest_i2c_dev_driver); } -static void of_unittest_overlay_i2c_12(void) +static void __init of_unittest_overlay_i2c_12(void) { int ret; @@ -1903,7 +1887,7 @@ static void of_unittest_overlay_i2c_12(void) } /* test deactivation of device */ -static void of_unittest_overlay_i2c_13(void) +static void __init of_unittest_overlay_i2c_13(void) { int ret; @@ -1920,7 +1904,7 @@ static void of_unittest_overlay_i2c_14(void) { } -static void of_unittest_overlay_i2c_15(void) +static void __init of_unittest_overlay_i2c_15(void) { int ret; @@ -2022,23 +2006,38 @@ static inline void __init of_unittest_overlay(void) { } extern uint8_t __dtb_##name##_begin[]; \ extern uint8_t __dtb_##name##_end[] -#define OVERLAY_INFO(name, expected) \ -{ .dtb_begin = __dtb_##name##_begin, \ - .dtb_end = __dtb_##name##_end, \ - .expected_result = expected, \ +#define OVERLAY_INFO(overlay_name, expected) \ +{ .dtb_begin = __dtb_##overlay_name##_begin, \ + .dtb_end = __dtb_##overlay_name##_end, \ + .expected_result = expected, \ + .name = #overlay_name, \ } struct overlay_info { - uint8_t *dtb_begin; - uint8_t *dtb_end; - void *data; - struct device_node *np_overlay; - int expected_result; - int overlay_id; + uint8_t *dtb_begin; + uint8_t *dtb_end; + int expected_result; + int overlay_id; + char *name; }; OVERLAY_INFO_EXTERN(overlay_base); OVERLAY_INFO_EXTERN(overlay); +OVERLAY_INFO_EXTERN(overlay_0); +OVERLAY_INFO_EXTERN(overlay_1); +OVERLAY_INFO_EXTERN(overlay_2); +OVERLAY_INFO_EXTERN(overlay_3); +OVERLAY_INFO_EXTERN(overlay_4); +OVERLAY_INFO_EXTERN(overlay_5); +OVERLAY_INFO_EXTERN(overlay_6); +OVERLAY_INFO_EXTERN(overlay_7); +OVERLAY_INFO_EXTERN(overlay_8); +OVERLAY_INFO_EXTERN(overlay_9); +OVERLAY_INFO_EXTERN(overlay_10); +OVERLAY_INFO_EXTERN(overlay_11); +OVERLAY_INFO_EXTERN(overlay_12); +OVERLAY_INFO_EXTERN(overlay_13); +OVERLAY_INFO_EXTERN(overlay_15); OVERLAY_INFO_EXTERN(overlay_bad_phandle); OVERLAY_INFO_EXTERN(overlay_bad_symbol); @@ -2046,6 +2045,21 @@ struct overlay_info { static struct overlay_info overlays[] = { OVERLAY_INFO(overlay_base, -9999), OVERLAY_INFO(overlay, 0), + OVERLAY_INFO(overlay_0, 0), + OVERLAY_INFO(overlay_1, 0), + OVERLAY_INFO(overlay_2, 0), + OVERLAY_INFO(overlay_3, 0), + OVERLAY_INFO(overlay_4, 0), + OVERLAY_INFO(overlay_5, 0), + OVERLAY_INFO(overlay_6, 0), + OVERLAY_INFO(overlay_7, 0), + OVERLAY_INFO(overlay_8, 0), + OVERLAY_INFO(overlay_9, 0), + OVERLAY_INFO(overlay_10, 0), + OVERLAY_INFO(overlay_11, 0), + OVERLAY_INFO(overlay_12, 0), + OVERLAY_INFO(overlay_13, 0), + OVERLAY_INFO(overlay_15, 0), OVERLAY_INFO(overlay_bad_phandle, -EINVAL), OVERLAY_INFO(overlay_bad_symbol, -EINVAL), {} @@ -2071,6 +2085,7 @@ void __init unittest_unflatten_overlay_base(void) { struct overlay_info *info; u32 data_size; + void *new_fdt; u32 size; info = &overlays[0]; @@ -2092,22 +2107,21 @@ void __init unittest_unflatten_overlay_base(void) return; } - info->data = early_init_dt_alloc_memory_arch(size, + new_fdt = early_init_dt_alloc_memory_arch(size, roundup_pow_of_two(FDT_V17_SIZE)); - if (!info->data) { + if (!new_fdt) { pr_err("alloc for dtb 'overlay_base' failed"); return; } - memcpy(info->data, info->dtb_begin, size); + memcpy(new_fdt, info->dtb_begin, size); - __unflatten_device_tree(info->data, NULL, &info->np_overlay, + __unflatten_device_tree(new_fdt, NULL, &overlay_base_root, early_init_dt_alloc_memory_arch, true); - overlay_base_root = info->np_overlay; } /* - * The purpose of of_unittest_overlay_data_add is to add an + * The purpose of of_unittest overlay_data_apply() is to add an * overlay in the normal fashion. This is a test of the whole * picture, instead of testing individual elements. * @@ -2117,73 +2131,44 @@ void __init unittest_unflatten_overlay_base(void) * * Return 0 on unexpected error. */ -static int __init overlay_data_add(int onum) +static int __init overlay_data_apply(const char *overlay_name, int *overlay_id) { struct overlay_info *info; + int found = 0; int k; int ret; u32 size; - u32 size_from_header; - for (k = 0, info = overlays; info; info++, k++) { - if (k == onum) + for (k = 0, info = overlays; info && info->name; info++, k++) { + if (!strcmp(overlay_name, info->name)) { + found = 1; break; + } } - if (onum > k) + if (!found) { + pr_err("no overlay data for %s\n", overlay_name); return 0; + } size = info->dtb_end - info->dtb_begin; if (!size) { - pr_err("no overlay to attach, %d\n", onum); + pr_err("no overlay data for %s\n", overlay_name); ret = 0; } - size_from_header = fdt_totalsize(info->dtb_begin); - if (size_from_header != size) { - pr_err("overlay header totalsize != actual size, %d", onum); - return 0; - } - - /* - * Must create permanent copy of FDT because of_fdt_unflatten_tree() - * will create pointers to the passed in FDT in the EDT. - */ - info->data = kmemdup(info->dtb_begin, size, GFP_KERNEL); - if (!info->data) { - pr_err("unable to allocate memory for data, %d\n", onum); - return 0; - } - - of_fdt_unflatten_tree(info->data, NULL, &info->np_overlay); - if (!info->np_overlay) { - pr_err("unable to unflatten overlay, %d\n", onum); - ret = 0; - goto out_free_data; - } - - info->overlay_id = 0; - ret = of_overlay_apply(info->np_overlay, &info->overlay_id); - if (ret < 0) { - pr_err("of_overlay_apply() (ret=%d), %d\n", ret, onum); - goto out_free_np_overlay; - } - - pr_debug("__dtb_overlay_begin applied, overlay id %d\n", ret); - - goto out; - -out_free_np_overlay: - /* - * info->np_overlay is the unflattened device tree - * It has not been spliced into the live tree. - */ - - /* todo: function to free unflattened device tree */ + ret = of_overlay_fdt_apply(info->dtb_begin, &info->overlay_id); + if (overlay_id) + *overlay_id = info->overlay_id; + if (ret < 0) + goto out; -out_free_data: - kfree(info->data); + pr_debug("%s applied\n", overlay_name); out: + if (ret != info->expected_result) + pr_err("of_overlay_fdt_apply() expected %d, ret=%d, %s\n", + info->expected_result, ret, overlay_name); + return (ret == info->expected_result); } @@ -2285,18 +2270,29 @@ static __init void of_unittest_overlay_high_level(void) __of_attach_node_sysfs(np); if (of_symbols) { + struct property *new_prop; for_each_property_of_node(overlay_base_symbols, prop) { - ret = __of_add_property(of_symbols, prop); + + new_prop = __of_prop_dup(prop, GFP_KERNEL); + if (!new_prop) { + unittest(0, "__of_prop_dup() of '%s' from overlay_base node __symbols__", + prop->name); + goto err_unlock; + } + ret = __of_add_property(of_symbols, new_prop); if (ret) { - unittest(0, - "duplicate property '%s' in overlay_base node __symbols__", + if (!strcmp(new_prop->name, "name")) { + /* auto-generated by unflatten */ + ret = 0; + continue; + } + unittest(0, "duplicate property '%s' in overlay_base node __symbols__", prop->name); goto err_unlock; } - ret = __of_add_property_sysfs(of_symbols, prop); + ret = __of_add_property_sysfs(of_symbols, new_prop); if (ret) { - unittest(0, - "unable to add property '%s' in overlay_base node __symbols__ to sysfs", + unittest(0, "unable to add property '%s' in overlay_base node __symbols__ to sysfs", prop->name); goto err_unlock; } @@ -2308,13 +2304,13 @@ static __init void of_unittest_overlay_high_level(void) /* now do the normal overlay usage test */ - unittest(overlay_data_add(1), + unittest(overlay_data_apply("overlay", NULL), "Adding overlay 'overlay' failed\n"); - unittest(overlay_data_add(2), + unittest(overlay_data_apply("overlay_bad_phandle", NULL), "Adding overlay 'overlay_bad_phandle' failed\n"); - unittest(overlay_data_add(3), + unittest(overlay_data_apply("overlay_bad_symbol", NULL), "Adding overlay 'overlay_bad_symbol' failed\n"); return; diff --git a/include/linux/of.h b/include/linux/of.h index d3dea1d1e3a9..1ce26c65b50b 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -1356,8 +1356,6 @@ struct of_overlay_notify_data { #ifdef CONFIG_OF_OVERLAY -/* ID based overlays; the API for external users */ -int of_overlay_apply(struct device_node *tree, int *ovcs_id); int of_overlay_remove(int *ovcs_id); int of_overlay_remove_all(void); @@ -1366,11 +1364,6 @@ struct of_overlay_notify_data { #else -static inline int of_overlay_apply(struct device_node *tree, int *ovcs_id) -{ - return -ENOTSUPP; -} - static inline int of_overlay_remove(int *ovcs_id) { return -ENOTSUPP; -- Frank Rowand <frank.rowand@sony.com> ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] of: change overlay apply input data from EDT to FDT 2018-01-29 2:53 ` [PATCH 1/2] " frowand.list @ 2018-01-29 3:21 ` Masahiro Yamada 2018-01-29 8:12 ` Frank Rowand 2018-01-29 14:42 ` Rob Herring 1 sibling, 1 reply; 16+ messages in thread From: Masahiro Yamada @ 2018-01-29 3:21 UTC (permalink / raw) To: Frank Rowand Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, devicetree, Linux Kernel Mailing List, Geert Uytterhoeven Hi Frank, 2018-01-29 11:53 GMT+09:00 <frowand.list@gmail.com>: > From: Frank Rowand <frank.rowand@sony.com> > diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile > index df697976740a..2b7ee68c908e 100644 > --- a/drivers/of/unittest-data/Makefile > +++ b/drivers/of/unittest-data/Makefile > @@ -3,6 +3,21 @@ DTC_FLAGS_testcases := -Wno-interrupts_property > obj-y += testcases.dtb.o > > obj-$(CONFIG_OF_OVERLAY) += overlay.dtb.o \ > + overlay_0.dtb.o \ > + overlay_1.dtb.o \ > + overlay_2.dtb.o \ > + overlay_3.dtb.o \ > + overlay_4.dtb.o \ > + overlay_5.dtb.o \ > + overlay_6.dtb.o \ > + overlay_7.dtb.o \ > + overlay_8.dtb.o \ > + overlay_9.dtb.o \ > + overlay_10.dtb.o \ > + overlay_11.dtb.o \ > + overlay_12.dtb.o \ > + overlay_13.dtb.o \ > + overlay_15.dtb.o \ > overlay_bad_phandle.dtb.o \ > overlay_bad_symbol.dtb.o \ > overlay_base.dtb.o > @@ -14,6 +29,7 @@ DTC_FLAGS_overlay := -@ > DTC_FLAGS_overlay_bad_phandle := -@ > DTC_FLAGS_overlay_bad_symbol := -@ > DTC_FLAGS_overlay_base := -@ > +DTC_FLAGS_testcases := -@ > This overwrites '-Wno-interrupts_property' a few lines above. Perhaps, do you want to do like this? DTC_FLAGS_testcases += -@ or DTC_FLAGS_testcases := -Wno-interrupts_property -@ -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] of: change overlay apply input data from EDT to FDT 2018-01-29 3:21 ` Masahiro Yamada @ 2018-01-29 8:12 ` Frank Rowand 0 siblings, 0 replies; 16+ messages in thread From: Frank Rowand @ 2018-01-29 8:12 UTC (permalink / raw) To: Masahiro Yamada Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, devicetree, Linux Kernel Mailing List, Geert Uytterhoeven On 01/28/18 19:21, Masahiro Yamada wrote: > Hi Frank, > > 2018-01-29 11:53 GMT+09:00 <frowand.list@gmail.com>: >> From: Frank Rowand <frank.rowand@sony.com> >> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile >> index df697976740a..2b7ee68c908e 100644 >> --- a/drivers/of/unittest-data/Makefile >> +++ b/drivers/of/unittest-data/Makefile >> @@ -3,6 +3,21 @@ DTC_FLAGS_testcases := -Wno-interrupts_property >> obj-y += testcases.dtb.o >> >> obj-$(CONFIG_OF_OVERLAY) += overlay.dtb.o \ >> + overlay_0.dtb.o \ >> + overlay_1.dtb.o \ >> + overlay_2.dtb.o \ >> + overlay_3.dtb.o \ >> + overlay_4.dtb.o \ >> + overlay_5.dtb.o \ >> + overlay_6.dtb.o \ >> + overlay_7.dtb.o \ >> + overlay_8.dtb.o \ >> + overlay_9.dtb.o \ >> + overlay_10.dtb.o \ >> + overlay_11.dtb.o \ >> + overlay_12.dtb.o \ >> + overlay_13.dtb.o \ >> + overlay_15.dtb.o \ >> overlay_bad_phandle.dtb.o \ >> overlay_bad_symbol.dtb.o \ >> overlay_base.dtb.o >> @@ -14,6 +29,7 @@ DTC_FLAGS_overlay := -@ >> DTC_FLAGS_overlay_bad_phandle := -@ >> DTC_FLAGS_overlay_bad_symbol := -@ >> DTC_FLAGS_overlay_base := -@ >> +DTC_FLAGS_testcases := -@ >> > > This overwrites '-Wno-interrupts_property' a few lines above. > > Perhaps, do you want to do like this? > > DTC_FLAGS_testcases += -@ > > or > > DTC_FLAGS_testcases := -Wno-interrupts_property -@ Yes, thanks for catching that! I'll fix it. -Frank ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] of: change overlay apply input data from EDT to FDT 2018-01-29 2:53 ` [PATCH 1/2] " frowand.list 2018-01-29 3:21 ` Masahiro Yamada @ 2018-01-29 14:42 ` Rob Herring 2018-01-29 15:05 ` Geert Uytterhoeven 2018-01-30 0:01 ` Frank Rowand 1 sibling, 2 replies; 16+ messages in thread From: Rob Herring @ 2018-01-29 14:42 UTC (permalink / raw) To: Frank Rowand Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, devicetree, Linux Kernel Mailing List, Geert Uytterhoeven On Sun, Jan 28, 2018 at 8:53 PM, <frowand.list@gmail.com> wrote: > From: Frank Rowand <frank.rowand@sony.com> > > 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 device tree (aka expanded device tree, EDT) now Not really a fan of a new acronym. > belongs to devicetree code, which is thus responsible for freeing > the EDT. > > These ownership changes prevent early freeing of the duplicated FDT > or the EDT, 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. Why the rearranging? That should be a separate patch. > > - 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. For this? > - 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 overlay > EDT 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. > > Errors while developing the patch exposed inadequate error messages > to debug problems when overlay devicetree fragment nodes contain > an invalid target path. Improved the messages in find_target_node(). Sounds like another separate patch. Rob ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] of: change overlay apply input data from EDT to FDT 2018-01-29 14:42 ` Rob Herring @ 2018-01-29 15:05 ` Geert Uytterhoeven 2018-01-30 0:13 ` Frank Rowand 2018-01-30 0:01 ` Frank Rowand 1 sibling, 1 reply; 16+ messages in thread From: Geert Uytterhoeven @ 2018-01-29 15:05 UTC (permalink / raw) To: Rob Herring Cc: Frank Rowand, Rob Herring, Pantelis Antoniou, Pantelis Antoniou, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List On Mon, Jan 29, 2018 at 3:42 PM, Rob Herring <robh@kernel.org> wrote: > On Sun, Jan 28, 2018 at 8:53 PM, <frowand.list@gmail.com> wrote: >> From: Frank Rowand <frank.rowand@sony.com> >> >> 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 device tree (aka expanded device tree, EDT) now > > Not really a fan of a new acronym. FWIW, it has already sneaked in: $ git grep -w EDT -- drivers/of drivers/of/unittest.c: * will create pointers to the passed in FDT in the EDT. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] of: change overlay apply input data from EDT to FDT 2018-01-29 15:05 ` Geert Uytterhoeven @ 2018-01-30 0:13 ` Frank Rowand 0 siblings, 0 replies; 16+ messages in thread From: Frank Rowand @ 2018-01-30 0:13 UTC (permalink / raw) To: Geert Uytterhoeven, Rob Herring Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List On 01/29/18 07:05, Geert Uytterhoeven wrote: > On Mon, Jan 29, 2018 at 3:42 PM, Rob Herring <robh@kernel.org> wrote: >> On Sun, Jan 28, 2018 at 8:53 PM, <frowand.list@gmail.com> wrote: >>> From: Frank Rowand <frank.rowand@sony.com> >>> >>> 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 device tree (aka expanded device tree, EDT) now >> >> Not really a fan of a new acronym. > > FWIW, it has already sneaked in: > > $ git grep -w EDT -- drivers/of > drivers/of/unittest.c: * will create pointers to the passed in FDT in the EDT. Rob will be glad to know that this comment is removed by this patch series. :-) > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] of: change overlay apply input data from EDT to FDT 2018-01-29 14:42 ` Rob Herring 2018-01-29 15:05 ` Geert Uytterhoeven @ 2018-01-30 0:01 ` Frank Rowand 2018-02-05 6:07 ` Rob Herring 1 sibling, 1 reply; 16+ messages in thread From: Frank Rowand @ 2018-01-30 0:01 UTC (permalink / raw) To: Rob Herring Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, devicetree, Linux Kernel Mailing List, Geert Uytterhoeven On 01/29/18 06:42, Rob Herring wrote: > On Sun, Jan 28, 2018 at 8:53 PM, <frowand.list@gmail.com> wrote: >> From: Frank Rowand <frank.rowand@sony.com> >> >> 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 device tree (aka expanded device tree, EDT) now > > Not really a fan of a new acronym. > >> belongs to devicetree code, which is thus responsible for freeing >> the EDT. >> >> These ownership changes prevent early freeing of the duplicated FDT >> or the EDT, 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. I should have reversed the cause and effect in that sentence to instead be: The changes to drivers/of/unittest.c require the test overlays to be in FDT form instead of unflattened devicetree form. Move overlay fragments from artificial locations in drivers/of/unittest-data/tests-overlay.dtsi into one devicetree source file per overlay and thus create one FDT per overlay. > Why the rearranging? That should be a separate patch. Bisectability. I can make the changes to the devicetree files in a second patch. After the first patch, there will be 29 self test fails. I will make the change unless you respond back to this saying not to. >> - 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. > > For this? > >> - 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 overlay >> EDT 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. >> >> Errors while developing the patch exposed inadequate error messages >> to debug problems when overlay devicetree fragment nodes contain >> an invalid target path. Improved the messages in find_target_node(). > > Sounds like another separate patch. Will do. > > Rob > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] of: change overlay apply input data from EDT to FDT 2018-01-30 0:01 ` Frank Rowand @ 2018-02-05 6:07 ` Rob Herring 0 siblings, 0 replies; 16+ messages in thread From: Rob Herring @ 2018-02-05 6:07 UTC (permalink / raw) To: Frank Rowand Cc: Pantelis Antoniou, Pantelis Antoniou, devicetree, Linux Kernel Mailing List, Geert Uytterhoeven On Mon, Jan 29, 2018 at 04:01:38PM -0800, Frank Rowand wrote: > On 01/29/18 06:42, Rob Herring wrote: > > On Sun, Jan 28, 2018 at 8:53 PM, <frowand.list@gmail.com> wrote: > >> From: Frank Rowand <frank.rowand@sony.com> > >> > >> 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 device tree (aka expanded device tree, EDT) now > > > > Not really a fan of a new acronym. > > > >> belongs to devicetree code, which is thus responsible for freeing > >> the EDT. > >> > >> These ownership changes prevent early freeing of the duplicated FDT > >> or the EDT, 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. > > I should have reversed the cause and effect in that sentence to > instead be: > > The changes to drivers/of/unittest.c require the test overlays > to be in FDT form instead of unflattened devicetree form. Move > overlay fragments from artificial locations in > drivers/of/unittest-data/tests-overlay.dtsi into one > devicetree source file per overlay and thus create > one FDT per overlay. > > > > Why the rearranging? That should be a separate patch. > Bisectability. > > I can make the changes to the devicetree files in a second patch. > After the first patch, there will be 29 self test fails. > > I will make the change unless you respond back to this saying not to. One patch is fine for me. Unit test is special. Rob ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/2] of: convert unittest overlay devicetree source to sugar syntax 2018-01-29 2:53 [PATCH 0/2] of: change overlay apply input data from EDT to FDT frowand.list 2018-01-29 2:53 ` [PATCH 1/2] " frowand.list @ 2018-01-29 2:53 ` frowand.list 2018-01-29 10:37 ` Geert Uytterhoeven 2018-01-29 14:08 ` [PATCH 0/2] of: change overlay apply input data from EDT to FDT Geert Uytterhoeven 2 siblings, 1 reply; 16+ messages in thread From: frowand.list @ 2018-01-29 2:53 UTC (permalink / raw) To: Rob Herring, pantelis.antoniou, Pantelis Antoniou Cc: devicetree, linux-kernel, geert From: Frank Rowand <frank.rowand@sony.com> 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 <frank.rowand@sony.com> --- 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 | 27 +++--- drivers/of/unittest-data/tests-overlay.dtsi | 4 +- 15 files changed, 149 insertions(+), 234 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 = < 0x00000200 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 = < 0x00000010 0x10 >; - }; + track@30 { + incline-up = < 48 32 16 >; + }; - ride_200_right: track@20 { - reg = < 0x00000020 0x10 >; - }; - }; + track@40 { + incline-up = < 47 31 15 >; }; }; - fragment@2 { - target = <&lights_2>; + ride_200: ride@200 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "ot,ferris-wheel"; + reg = < 0x00000200 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_200_left: track@10 { + reg = < 0x00000010 0x10 >; + }; - __overlay__ { - status = "okay"; - color = "purple", "white", "red", "green"; - rate = < 3 256 >; + ride_200_right: track@20 { + reg = < 0x00000020 0x10 >; }; }; +}; + +&lights_2 { + status = "okay"; + color = "purple", "white", "red", "green"; + rate = < 3 256 >; }; diff --git a/drivers/of/unittest-data/overlay_10.dts b/drivers/of/unittest-data/overlay_10.dts index 445925a10cd3..73993bf23bf8 100644 --- a/drivers/of/unittest-data/overlay_10.dts +++ b/drivers/of/unittest-data/overlay_10.dts @@ -2,33 +2,26 @@ /dts-v1/; /plugin/; -/ { - /* overlay_10 */ - /* overlays 8, 9, 10, 11 application and removal in bad sequence */ +/* overlay_10 */ +/* overlays 8, 9, 10, 11 application and removal in bad sequence */ - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus"; - __overlay__ { +&unittest_test_bus { + /* suppress DTC warning */ + #address-cells = <1>; + #size-cells = <0>; - /* suppress DTC warning */ - #address-cells = <1>; - #size-cells = <0>; + test-unittest10 { + compatible = "unittest"; + status = "okay"; + reg = <10>; - test-unittest10 { - compatible = "unittest"; - status = "okay"; - reg = <10>; + #address-cells = <1>; + #size-cells = <0>; - #address-cells = <1>; - #size-cells = <0>; - - test-unittest101 { - compatible = "unittest"; - status = "okay"; - reg = <1>; - }; - - }; + test-unittest101 { + compatible = "unittest"; + status = "okay"; + reg = <1>; }; }; }; diff --git a/drivers/of/unittest-data/overlay_11.dts b/drivers/of/unittest-data/overlay_11.dts index c1d14f34359e..9a79b253a809 100644 --- a/drivers/of/unittest-data/overlay_11.dts +++ b/drivers/of/unittest-data/overlay_11.dts @@ -2,33 +2,27 @@ /dts-v1/; /plugin/; -/ { - /* overlay_11 */ - /* overlays 8, 9, 10, 11 application and removal in bad sequence */ +/* overlay_11 */ +/* overlays 8, 9, 10, 11 application and removal in bad sequence */ - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus"; - __overlay__ { +&unittest_test_bus { + /* suppress DTC warning */ + #address-cells = <1>; + #size-cells = <0>; - /* suppress DTC warning */ - #address-cells = <1>; - #size-cells = <0>; + test-unittest11 { + compatible = "unittest"; + status = "okay"; + reg = <11>; - test-unittest11 { - compatible = "unittest"; - status = "okay"; - reg = <11>; + #address-cells = <1>; + #size-cells = <0>; - #address-cells = <1>; - #size-cells = <0>; - - test-unittest111 { - compatible = "unittest"; - status = "okay"; - reg = <1>; - }; - - }; + test-unittest111 { + compatible = "unittest"; + status = "okay"; + reg = <1>; }; + }; }; diff --git a/drivers/of/unittest-data/overlay_15.dts b/drivers/of/unittest-data/overlay_15.dts index 44e44c62b739..b98f2514df4b 100644 --- a/drivers/of/unittest-data/overlay_15.dts +++ b/drivers/of/unittest-data/overlay_15.dts @@ -2,33 +2,28 @@ /dts-v1/; /plugin/; -/ { - /* overlay_15 - mux overlay */ +/* overlay_15 - mux overlay */ - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/i2c-test-bus"; - __overlay__ { - #address-cells = <1>; - #size-cells = <0>; - test-unittest15 { - reg = <11>; - compatible = "unittest-i2c-mux"; - status = "okay"; +&unittest_i2c_test_bus { + #address-cells = <1>; + #size-cells = <0>; + test-unittest15 { + reg = <11>; + compatible = "unittest-i2c-mux"; + status = "okay"; - #address-cells = <1>; - #size-cells = <0>; + #address-cells = <1>; + #size-cells = <0>; - i2c@0 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0>; + i2c@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; - test-mux-dev { - reg = <32>; - compatible = "unittest-i2c-dev"; - status = "okay"; - }; - }; + test-mux-dev { + reg = <32>; + compatible = "unittest-i2c-dev"; + status = "okay"; }; }; }; diff --git a/drivers/of/unittest-data/overlay_2.dts b/drivers/of/unittest-data/overlay_2.dts index cf1e4245b7ce..db8684ba89d9 100644 --- a/drivers/of/unittest-data/overlay_2.dts +++ b/drivers/of/unittest-data/overlay_2.dts @@ -2,13 +2,8 @@ /dts-v1/; /plugin/; -/ { - /* overlay_2 - enable using label */ +/* overlay_2 - enable using label */ - fragment@0 { - target = <&unittest2>; - __overlay__ { - status = "okay"; - }; - }; +&unittest2 { + status = "okay"; }; diff --git a/drivers/of/unittest-data/overlay_3.dts b/drivers/of/unittest-data/overlay_3.dts index 158dc44fc20a..40f289e7c237 100644 --- a/drivers/of/unittest-data/overlay_3.dts +++ b/drivers/of/unittest-data/overlay_3.dts @@ -2,13 +2,8 @@ /dts-v1/; /plugin/; -/ { - /* overlay_3 - disable using label */ +/* overlay_3 - disable using label */ - fragment@0 { - target = <&unittest3>; - __overlay__ { - status = "disabled"; - }; - }; +&unittest3 { + status = "disabled"; }; diff --git a/drivers/of/unittest-data/overlay_4.dts b/drivers/of/unittest-data/overlay_4.dts index b4a2e6c6b016..a8a77ddf9abe 100644 --- a/drivers/of/unittest-data/overlay_4.dts +++ b/drivers/of/unittest-data/overlay_4.dts @@ -2,22 +2,17 @@ /dts-v1/; /plugin/; -/ { - /* overlay_4 - test insertion of a full node */ +/* overlay_4 - test insertion of a full node */ - fragment@0 { - target = <&unittestbus>; - __overlay__ { +&unittest_test_bus { - /* suppress DTC warning */ - #address-cells = <1>; - #size-cells = <0>; + /* suppress DTC warning */ + #address-cells = <1>; + #size-cells = <0>; - test-unittest4 { - compatible = "unittest"; - status = "okay"; - reg = <4>; - }; - }; + test-unittest4 { + compatible = "unittest"; + status = "okay"; + reg = <4>; }; }; diff --git a/drivers/of/unittest-data/overlay_5.dts b/drivers/of/unittest-data/overlay_5.dts index 02ad25c1f19c..706f5f1b737c 100644 --- a/drivers/of/unittest-data/overlay_5.dts +++ b/drivers/of/unittest-data/overlay_5.dts @@ -2,13 +2,8 @@ /dts-v1/; /plugin/; -/ { - /* overlay_5 - test overlay apply revert */ +/* overlay_5 - test overlay apply revert */ - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest5"; - __overlay__ { - status = "okay"; - }; - }; +&unittest5 { + status = "okay"; }; diff --git a/drivers/of/unittest-data/overlay_6.dts b/drivers/of/unittest-data/overlay_6.dts index a14e965f5497..21a7fa4ca45e 100644 --- a/drivers/of/unittest-data/overlay_6.dts +++ b/drivers/of/unittest-data/overlay_6.dts @@ -2,14 +2,9 @@ /dts-v1/; /plugin/; -/ { - /* overlay_6 */ - /* overlays 6, 7 application and removal in sequence */ +/* overlay_6 */ +/* overlays 6, 7 application and removal in sequence */ - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest6"; - __overlay__ { - status = "okay"; - }; - }; +&unittest6 { + status = "okay"; }; diff --git a/drivers/of/unittest-data/overlay_7.dts b/drivers/of/unittest-data/overlay_7.dts index 4bd7e423209c..58ba1bb51b50 100644 --- a/drivers/of/unittest-data/overlay_7.dts +++ b/drivers/of/unittest-data/overlay_7.dts @@ -2,14 +2,9 @@ /dts-v1/; /plugin/; -/ { - /* overlay_7 */ - /* overlays 6, 7 application and removal in sequence */ +/* overlay_7 */ +/* overlays 6, 7 application and removal in sequence */ - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest7"; - __overlay__ { - status = "okay"; - }; - }; +&unittest7 { + status = "okay"; }; diff --git a/drivers/of/unittest-data/overlay_8.dts b/drivers/of/unittest-data/overlay_8.dts index 5b21c53945a9..e9718d118e38 100644 --- a/drivers/of/unittest-data/overlay_8.dts +++ b/drivers/of/unittest-data/overlay_8.dts @@ -2,14 +2,9 @@ /dts-v1/; /plugin/; -/ { - /* overlay_8 */ - /* overlays 8, 9, 10, 11 application and removal in bad sequence */ +/* overlay_8 */ +/* overlays 8, 9, 10, 11 application and removal in bad sequence */ - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest8"; - __overlay__ { - status = "okay"; - }; - }; +&unittest8 { + status = "okay"; }; diff --git a/drivers/of/unittest-data/overlay_9.dts b/drivers/of/unittest-data/overlay_9.dts index 20ff055a5349..b35e23edae50 100644 --- a/drivers/of/unittest-data/overlay_9.dts +++ b/drivers/of/unittest-data/overlay_9.dts @@ -2,14 +2,9 @@ /dts-v1/; /plugin/; -/ { - /* overlay_9 */ - /* overlays 8, 9, 10, 11 application and removal in bad sequence */ +/* overlay_9 */ +/* overlays 8, 9, 10, 11 application and removal in bad sequence */ - fragment@0 { - target-path = "/testcase-data/overlay-node/test-bus/test-unittest8"; - __overlay__ { - property-foo = "bar"; - }; - }; +&unittest8 { + property-foo = "bar"; }; diff --git a/drivers/of/unittest-data/overlay_bad_phandle.dts b/drivers/of/unittest-data/overlay_bad_phandle.dts index 4d5b99723bad..83b797360318 100644 --- a/drivers/of/unittest-data/overlay_bad_phandle.dts +++ b/drivers/of/unittest-data/overlay_bad_phandle.dts @@ -2,20 +2,13 @@ /dts-v1/; /plugin/; -/ { - - fragment@0 { - target = <&electric_1>; - - __overlay__ { - - // This label should cause an error when the overlay - // is applied. There is already a phandle value - // in the base tree for motor-1. - spin_ctrl_1_conflict: motor-1 { - accelerate = < 3 >; - decelerate = < 5 >; - }; - }; +&electric_1 { + + // This label should cause an error when the overlay + // is applied. There is already a phandle value + // in the base tree for motor-1. + spin_ctrl_1_conflict: motor-1 { + accelerate = < 3 >; + decelerate = < 5 >; }; }; diff --git a/drivers/of/unittest-data/overlay_bad_symbol.dts b/drivers/of/unittest-data/overlay_bad_symbol.dts index 09261cb9a67e..3b3a2c08edd7 100644 --- a/drivers/of/unittest-data/overlay_bad_symbol.dts +++ b/drivers/of/unittest-data/overlay_bad_symbol.dts @@ -1,22 +1,15 @@ /dts-v1/; /plugin/; -/ { - - fragment@0 { - target = <&electric_1>; - - __overlay__ { - - // This label should cause an error when the overlay - // is applied. There is already a symbol hvac_1 - // in the base tree - hvac_1: hvac-medium-2 { - compatible = "ot,hvac-medium"; - heat-range = < 50 75 >; - cool-range = < 60 80 >; - }; - - }; +&electric_1 { + + // This label should cause an error when the overlay + // is applied. There is already a symbol hvac_1 + // in the base tree + hvac_1: hvac-medium-2 { + compatible = "ot,hvac-medium"; + heat-range = < 50 75 >; + cool-range = < 60 80 >; }; + }; diff --git a/drivers/of/unittest-data/tests-overlay.dtsi b/drivers/of/unittest-data/tests-overlay.dtsi index fa2fb43bccac..25cf397b8f6b 100644 --- a/drivers/of/unittest-data/tests-overlay.dtsi +++ b/drivers/of/unittest-data/tests-overlay.dtsi @@ -5,7 +5,7 @@ overlay-node { /* test bus */ - unittestbus: test-bus { + unittest_test_bus: test-bus { compatible = "simple-bus"; #address-cells = <1>; #size-cells = <0>; @@ -70,7 +70,7 @@ reg = <8>; }; - i2c-test-bus { + unittest_i2c_test_bus: i2c-test-bus { compatible = "unittest-i2c-bus"; status = "okay"; reg = <50>; -- Frank Rowand <frank.rowand@sony.com> ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] of: convert unittest overlay devicetree source to sugar syntax 2018-01-29 2:53 ` [PATCH 2/2] of: convert unittest overlay devicetree source to sugar syntax frowand.list @ 2018-01-29 10:37 ` Geert Uytterhoeven 2018-01-30 0:14 ` Frank Rowand 0 siblings, 1 reply; 16+ messages in thread From: Geert Uytterhoeven @ 2018-01-29 10:37 UTC (permalink / raw) To: Frank Rowand Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List Hi Frank, On Mon, Jan 29, 2018 at 3:53 AM, <frowand.list@gmail.com> wrote: > From: Frank Rowand <frank.rowand@sony.com> > > 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. Cool! I had heard about the new syntax, but hadn't realized it was merged in v4.15-rc1. Have you used your overlay_convert_old_to_new script to convert them? > 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 And I guess you want to keep at least a few tests using the non-sugar syntax anyway? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] of: convert unittest overlay devicetree source to sugar syntax 2018-01-29 10:37 ` Geert Uytterhoeven @ 2018-01-30 0:14 ` Frank Rowand 0 siblings, 0 replies; 16+ messages in thread From: Frank Rowand @ 2018-01-30 0:14 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List On 01/29/18 02:37, Geert Uytterhoeven wrote: > Hi Frank, > > On Mon, Jan 29, 2018 at 3:53 AM, <frowand.list@gmail.com> wrote: >> From: Frank Rowand <frank.rowand@sony.com> >> >> 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. > > Cool! I had heard about the new syntax, but hadn't realized it was merged in > v4.15-rc1. > > Have you used your overlay_convert_old_to_new script to convert them? > >> 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 > > And I guess you want to keep at least a few tests using the non-sugar syntax > anyway? Not really. But not something I care about at the moment. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] of: change overlay apply input data from EDT to FDT 2018-01-29 2:53 [PATCH 0/2] of: change overlay apply input data from EDT to FDT frowand.list 2018-01-29 2:53 ` [PATCH 1/2] " frowand.list 2018-01-29 2:53 ` [PATCH 2/2] of: convert unittest overlay devicetree source to sugar syntax frowand.list @ 2018-01-29 14:08 ` Geert Uytterhoeven 2018-01-30 0:22 ` Frank Rowand 2 siblings, 1 reply; 16+ messages in thread From: Geert Uytterhoeven @ 2018-01-29 14:08 UTC (permalink / raw) To: Frank Rowand Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List Hi Frank, On Mon, Jan 29, 2018 at 3:53 AM, <frowand.list@gmail.com> wrote: > From: Frank Rowand <frank.rowand@sony.com> > > 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 device tree (aka expanded device tree, EDT) now > belongs to devicetree code, which is thus responsible for freeing > the EDT. > > These ownership changes prevent early freeing of the duplicated FDT > or the EDT, 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. Thanks for your series! > Frank Rowand (2): > of: change overlay apply input data from EDT to FDT > of: convert unittest overlay devicetree source to sugar syntax Do you plan to update Documentation/devicetree/overlay-notes.txt and Documentation/devicetree/bindings/fpga/fpga-region.txt, too? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] of: change overlay apply input data from EDT to FDT 2018-01-29 14:08 ` [PATCH 0/2] of: change overlay apply input data from EDT to FDT Geert Uytterhoeven @ 2018-01-30 0:22 ` Frank Rowand 2018-01-30 0:35 ` Frank Rowand 0 siblings, 1 reply; 16+ messages in thread From: Frank Rowand @ 2018-01-30 0:22 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List On 01/29/18 06:08, Geert Uytterhoeven wrote: > Hi Frank, > > On Mon, Jan 29, 2018 at 3:53 AM, <frowand.list@gmail.com> wrote: >> From: Frank Rowand <frank.rowand@sony.com> >> >> 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 device tree (aka expanded device tree, EDT) now >> belongs to devicetree code, which is thus responsible for freeing >> the EDT. >> >> These ownership changes prevent early freeing of the duplicated FDT >> or the EDT, 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. > > Thanks for your series! > >> Frank Rowand (2): >> of: change overlay apply input data from EDT to FDT >> of: convert unittest overlay devicetree source to sugar syntax > > Do you plan to update Documentation/devicetree/overlay-notes.txt > and Documentation/devicetree/bindings/fpga/fpga-region.txt, too? Thanks for the pointers. I will add updates to Documentation/devicetree/overlay-notes.txt in this series. The changes to Documentation/devicetree/bindings/fpga/fpga-region.txt are related to the relatively new sugar syntax in dtc, not to the changes introduced by this patch series. I'll create a patch outside this series to update this documentation. > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] of: change overlay apply input data from EDT to FDT 2018-01-30 0:22 ` Frank Rowand @ 2018-01-30 0:35 ` Frank Rowand 2018-01-30 16:28 ` Alan Tull 0 siblings, 1 reply; 16+ messages in thread From: Frank Rowand @ 2018-01-30 0:35 UTC (permalink / raw) To: Geert Uytterhoeven, Alan Tull Cc: Rob Herring, Pantelis Antoniou, Pantelis Antoniou, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List Hi Alan, In this patch series one of the changes was to change some devicetree unittest overlay source to use the new sugar syntax to specify overlay nodes instead of hand coding the fragment nodes. One reviewer reminded me to update Documentation/ files that show the hand coding of overlay fragment nodes. One of those files is Documentation/devicetree/bindings/fpga/fpga-region.txt. I plan to submit a patch to update the example devicetree source to use the sugar syntax. There is one place where I can not make that change, which is the "Overlay DTS Format" section. In this section, the use of 'target-path="/path" is shown as one of two ways to specify the target of the fragment. The sugar syntax creates a fragment using the 'target=<phandle>' form instead of 'target-path="/path"' form. With sugar syntax, there is no way to specify target-path. Is the target-path form needed by the fpga subsystem, or can this be removed? -Frank On 01/29/18 16:22, Frank Rowand wrote: > On 01/29/18 06:08, Geert Uytterhoeven wrote: >> Hi Frank, >> >> On Mon, Jan 29, 2018 at 3:53 AM, <frowand.list@gmail.com> wrote: >>> From: Frank Rowand <frank.rowand@sony.com> >>> >>> 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 device tree (aka expanded device tree, EDT) now >>> belongs to devicetree code, which is thus responsible for freeing >>> the EDT. >>> >>> These ownership changes prevent early freeing of the duplicated FDT >>> or the EDT, 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. >> >> Thanks for your series! >> >>> Frank Rowand (2): >>> of: change overlay apply input data from EDT to FDT >>> of: convert unittest overlay devicetree source to sugar syntax >> >> Do you plan to update Documentation/devicetree/overlay-notes.txt >> and Documentation/devicetree/bindings/fpga/fpga-region.txt, too? > > Thanks for the pointers. > > I will add updates to Documentation/devicetree/overlay-notes.txt in > this series. > > The changes to Documentation/devicetree/bindings/fpga/fpga-region.txt > are related to the relatively new sugar syntax in dtc, not to the > changes introduced by this patch series. I'll create a patch outside > this series to update this documentation. > > >> Gr{oetje,eeting}s, >> >> Geert >> >> -- >> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org >> >> In personal conversations with technical people, I call myself a hacker. But >> when I'm talking to journalists I just say "programmer" or something like that. >> -- Linus Torvalds >> > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] of: change overlay apply input data from EDT to FDT 2018-01-30 0:35 ` Frank Rowand @ 2018-01-30 16:28 ` Alan Tull 0 siblings, 0 replies; 16+ messages in thread From: Alan Tull @ 2018-01-30 16:28 UTC (permalink / raw) To: Frank Rowand Cc: Geert Uytterhoeven, Rob Herring, Pantelis Antoniou, Pantelis Antoniou, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List On Mon, Jan 29, 2018 at 6:35 PM, Frank Rowand <frowand.list@gmail.com> wrote: Hi Frank, > Hi Alan, > > In this patch series one of the changes was to change some > devicetree unittest overlay source to use the new sugar syntax to > specify overlay nodes instead of hand coding the fragment nodes. > > One reviewer reminded me to update Documentation/ files that > show the hand coding of overlay fragment nodes. One of those > files is Documentation/devicetree/bindings/fpga/fpga-region.txt. > I plan to submit a patch to update the example devicetree source > to use the sugar syntax. There is one place where I can not > make that change, which is the "Overlay DTS Format" section. > In this section, the use of 'target-path="/path" is shown as > one of two ways to specify the target of the fragment. The > sugar syntax creates a fragment using the 'target=<phandle>' > form instead of 'target-path="/path"' form. With sugar > syntax, there is no way to specify target-path. Is the > target-path form needed by the fpga subsystem, or can this > be removed? Removing target-path won't be a loss of functionality as far as I can see. Currently the examples in Documentation/devicetree/bindings/fpga/fpga-region.txt all specify 'target = <phandle> while the descriptive text says that target and target-path are both options (3 line edits are needed to remove target-path from this file plus other edits to change it to sugar format without __overlay__ and 'fragment@0'). Thanks for asking! Alan > > -Frank > > On 01/29/18 16:22, Frank Rowand wrote: >> On 01/29/18 06:08, Geert Uytterhoeven wrote: >>> Hi Frank, >>> >>> On Mon, Jan 29, 2018 at 3:53 AM, <frowand.list@gmail.com> wrote: >>>> From: Frank Rowand <frank.rowand@sony.com> >>>> >>>> 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 device tree (aka expanded device tree, EDT) now >>>> belongs to devicetree code, which is thus responsible for freeing >>>> the EDT. >>>> >>>> These ownership changes prevent early freeing of the duplicated FDT >>>> or the EDT, 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. >>> >>> Thanks for your series! >>> >>>> Frank Rowand (2): >>>> of: change overlay apply input data from EDT to FDT >>>> of: convert unittest overlay devicetree source to sugar syntax >>> >>> Do you plan to update Documentation/devicetree/overlay-notes.txt >>> and Documentation/devicetree/bindings/fpga/fpga-region.txt, too? >> >> Thanks for the pointers. >> >> I will add updates to Documentation/devicetree/overlay-notes.txt in >> this series. >> >> The changes to Documentation/devicetree/bindings/fpga/fpga-region.txt >> are related to the relatively new sugar syntax in dtc, not to the >> changes introduced by this patch series. I'll create a patch outside >> this series to update this documentation. >> >> >>> Gr{oetje,eeting}s, >>> >>> Geert >>> >>> -- >>> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org >>> >>> In personal conversations with technical people, I call myself a hacker. But >>> when I'm talking to journalists I just say "programmer" or something like that. >>> -- Linus Torvalds >>> >> >> > ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-02-05 6:07 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-29 2:53 [PATCH 0/2] of: change overlay apply input data from EDT to FDT frowand.list 2018-01-29 2:53 ` [PATCH 1/2] " frowand.list 2018-01-29 3:21 ` Masahiro Yamada 2018-01-29 8:12 ` Frank Rowand 2018-01-29 14:42 ` Rob Herring 2018-01-29 15:05 ` Geert Uytterhoeven 2018-01-30 0:13 ` Frank Rowand 2018-01-30 0:01 ` Frank Rowand 2018-02-05 6:07 ` Rob Herring 2018-01-29 2:53 ` [PATCH 2/2] of: convert unittest overlay devicetree source to sugar syntax frowand.list 2018-01-29 10:37 ` Geert Uytterhoeven 2018-01-30 0:14 ` Frank Rowand 2018-01-29 14:08 ` [PATCH 0/2] of: change overlay apply input data from EDT to FDT Geert Uytterhoeven 2018-01-30 0:22 ` Frank Rowand 2018-01-30 0:35 ` Frank Rowand 2018-01-30 16:28 ` Alan Tull
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox