[PATCH 0/3] of: overlay: load overlay symbols into live device tree

[PATCH 0/3] of: overlay: load overlay symbols into live device tree

[frowand.list-Re5JQEeQqe8AvxtiuMwx3w]

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

Symbols in a loaded overlay are not currently available to subsequently
loaded overlays because the properties in the overlay's __symbols__
node are not loaded into the live device tree.

Patch 1 is unittests to test patches 2 and 3.

Patch 2 fixes a problem discovered while developing patch 3.  If
a node name in an overlay has a unit-address then the overlay
code does not correctly match the node name against an existing
node in the live tree.

Patch 3 adds the properties in an overlay's __symbol__ node to
the overlay changeset.

This patch set was created on top of v4.12, with Rob's pull request
for 4.13 added (https://lkml.org/lkml/2017/7/6/691).  Thus this
series should apply on 4.13-rc1 when that becomes available.  I will
re-test on 4.13-rc1.


Frank Rowand (3):
  of: overlay: add overlay unittest data for node names and symbols
  of: overlay: correctly apply overlay node with unit-address
  of: overlay: add overlay symbols to live device tree

 drivers/of/overlay.c                            | 136 ++++++++++++++++++++++--
 drivers/of/unittest-data/Makefile               |   3 +
 drivers/of/unittest-data/overlay.dts            |  19 +++-
 drivers/of/unittest-data/overlay_bad_symbol.dts |  22 ++++
 drivers/of/unittest-data/overlay_base.dts       |   7 ++
 drivers/of/unittest.c                           |   6 ++
 6 files changed, 182 insertions(+), 11 deletions(-)
 create mode 100644 drivers/of/unittest-data/overlay_bad_symbol.dts

-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] of: overlay: add overlay unittest data for node names and symbols

[frowand.list-Re5JQEeQqe8AvxtiuMwx3w]

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

Add nodes and properties to overlay_base and overlay dts files to
test for
   - incorrect existing node name detection when overlay node name
     has a unit-address
   - adding overlay __symbols__ properties to live tree when an
     overlay is added to the live tree

Expected result from patch 2/3 is overlay will update the nodes and
properties for /testcase-data-2/fairway-1/ride@100/

Before patch 2/3 is applied:

   Console error message near end of unittest:
      OF: Duplicate name in fairway-1, renamed to "ride@100#1"

   $ cd /proc/device-tree/testcase-data-2/fairway-1/
   $ # extra node: ride@100#1
   $ ls
   #address-cells  linux,phandle   phandle         ride@200
   #size-cells     name            ride@100        status
   compatible      orientation     ride@100#1
   $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
   $ ls track@3/incline_up
   ls: track@3/incline_up: No such file or directory
   $ ls track@4/incline_up
   ls: track@4/incline_up: No such file or directory

After patch 2/3 is applied:

   Console error message no longer occurs

   $ cd /proc/device-tree/testcase-data-2/fairway-1/
   $ # no extra node: ride@100#1
   $ ls
   #address-cells  compatible      name            phandle         ride@200
   #size-cells     linux,phandle   orientation     ride@100        status
   $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
   $ ls track@3/incline_up
   track@3/incline_up
   $ ls track@4/incline_up
   track@4/incline_up

Expected result from patch 3/3 is new __symbols__ entries for labels
from the overlay _after_ the add overlay symbols patch is also applied.

Before patch 3/3 is applied:

   Console error message near end of unittest:
      ### dt-test ### FAIL of_unittest_overlay_high_level():2296 Adding overlay 'overlay_bad_symbol' failed
      ### dt-test ### end of unittest - 190 passed, 1 failed

   The new unittest "fails" because the expected result of loading the
   new overlay is an error instead of success.

   $ # node hvac-medium-2 exists because the overlay loaded
   $ # since the duplicate symbol was not detected
   $ cd /proc/device-tree/testcase-data-2/substation@100/
   $ ls
   compatible     hvac-medium-2  motor-8        reg
   hvac-large-1   linux,phandle  name           status
   hvac-medium-1  motor-1        phandle

   $ cd /proc/device-tree/__symbols__/
   $ ls
   electric_1   lights_1     name         rides_1      spin_ctrl_2
   hvac_1       lights_2     retail_1     spin_ctrl_1

After patch 3/3 is applied:

   Previous console error message no longer occurs, but expected error
   occurs:
      OF: overlay: Failed to apply prop @/__symbols__/hvac_1
      OF: overlay: apply failed '/__symbols__'
      ### dt-test ### end of unittest - 191 passed, 0 failed

   $ # node hvac-medium-2 does not exist because the overlay
   $ # properly failed to load due to the duplicate symbol
   $ cd /proc/device-tree/testcase-data-2/substation@100/
   $ ls
   compatible     hvac-medium-1  motor-1        name           reg
   hvac-large-1   linux,phandle  motor-8        phandle        status

   $ cd /proc/device-tree/__symbols__/
   $ ls
   electric_1      lights_1        retail_1        ride_200_right  spin_ctrl_2
   hvac_1          lights_2        ride_200        rides_1
   hvac_2          name            ride_200_left   spin_ctrl_1
   $ cat ride_200; echo
   /testcase-data-2/fairway-1/ride@200
   $ cat ride_200_left ; echo
   /testcase-data-2/fairway-1/ride@200/track@1
   $ cat ride_200_right ; echo
   /testcase-data-2/fairway-1/ride@200/track@2

Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
 drivers/of/unittest-data/Makefile               |  3 +++
 drivers/of/unittest-data/overlay.dts            | 19 ++++++++++++++++++-
 drivers/of/unittest-data/overlay_bad_symbol.dts | 22 ++++++++++++++++++++++
 drivers/of/unittest-data/overlay_base.dts       |  7 +++++++
 drivers/of/unittest.c                           |  6 ++++++
 5 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 drivers/of/unittest-data/overlay_bad_symbol.dts

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 6e00a9c69e58..dae2fe23cd2e 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -1,11 +1,13 @@
 obj-y += testcases.dtb.o
 obj-y += overlay.dtb.o
 obj-y += overlay_bad_phandle.dtb.o
+obj-y += overlay_bad_symbol.dtb.o
 obj-y += overlay_base.dtb.o
 
 targets += testcases.dtb testcases.dtb.S
 targets += overlay.dtb overlay.dtb.S
 targets += overlay_bad_phandle.dtb overlay_bad_phandle.dtb.S
+targets += overlay_bad_symbol.dtb overlay_bad_symbol.dtb.S
 targets += overlay_base.dtb overlay_base.dtb.S
 
 .PRECIOUS: \
@@ -15,4 +17,5 @@ targets += overlay_base.dtb overlay_base.dtb.S
 # enable creation of __symbols__ node
 DTC_FLAGS_overlay := -@
 DTC_FLAGS_overlay_bad_phandle := -@
+DTC_FLAGS_overlay_bad_symbol := -@
 DTC_FLAGS_overlay_base := -@
diff --git a/drivers/of/unittest-data/overlay.dts b/drivers/of/unittest-data/overlay.dts
index 6cd7e6a0c13e..81140adbe770 100644
--- a/drivers/of/unittest-data/overlay.dts
+++ b/drivers/of/unittest-data/overlay.dts
@@ -25,7 +25,18 @@
 			#size-cells = <1>;
 			status = "ok";
 
-			ride@200 {
+			ride@100 {
+
+				track@3 {
+					incline_up = < 48 32 16 >;
+				};
+
+				track@4 {
+					incline_up = < 47 31 15 >;
+				};
+			};
+
+			ride_200: ride@200 {
 				compatible = "ot,ferris-wheel";
 				reg = < 0x00000200 0x100 >;
 				hvac-provider = < &hvac_2 >;
@@ -36,6 +47,12 @@
 				spin-rph = < 30 >;
 				gondolas = < 16 >;
 				gondola-capacity = < 6 >;
+
+				ride_200_left: track@1 {
+				};
+
+				ride_200_right: track@2 {
+				};
 			};
 		};
 	};
diff --git a/drivers/of/unittest-data/overlay_bad_symbol.dts b/drivers/of/unittest-data/overlay_bad_symbol.dts
new file mode 100644
index 000000000000..09261cb9a67e
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_bad_symbol.dts
@@ -0,0 +1,22 @@
+/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 >;
+			};
+
+		};
+	};
+};
diff --git a/drivers/of/unittest-data/overlay_base.dts b/drivers/of/unittest-data/overlay_base.dts
index 5566b27fb61a..997d807259e6 100644
--- a/drivers/of/unittest-data/overlay_base.dts
+++ b/drivers/of/unittest-data/overlay_base.dts
@@ -53,6 +53,13 @@
 				spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
 				spin-controller-names = "track_1", "track_2";
 				queues = < 2 >;
+
+				track@3 {
+				};
+
+				track@4 {
+				};
+
 			};
 		};
 
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 0107fc680335..e56b8eb220d9 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2010,6 +2010,7 @@ struct overlay_info {
 OVERLAY_INFO_EXTERN(overlay_base);
 OVERLAY_INFO_EXTERN(overlay);
 OVERLAY_INFO_EXTERN(overlay_bad_phandle);
+OVERLAY_INFO_EXTERN(overlay_bad_symbol);
 
 #ifdef CONFIG_OF_OVERLAY
 
@@ -2018,6 +2019,7 @@ struct overlay_info {
 	OVERLAY_INFO(overlay_base, -9999),
 	OVERLAY_INFO(overlay, 0),
 	OVERLAY_INFO(overlay_bad_phandle, -EINVAL),
+	OVERLAY_INFO(overlay_bad_symbol, -EINVAL),
 	{}
 };
 
@@ -2289,6 +2291,10 @@ static __init void of_unittest_overlay_high_level(void)
 
 	unittest(overlay_data_add(2),
 		 "Adding overlay 'overlay_bad_phandle' failed\n");
+
+	unittest(overlay_data_add(3),
+		 "Adding overlay 'overlay_bad_symbol' failed\n");
+
 	return;
 
 err_unlock:
-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] of: overlay: correctly apply overlay node with unit-address

[frowand.list-Re5JQEeQqe8AvxtiuMwx3w]

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

Correct existing node name detection when overlay node name has
a unit-address.

Expected test result is overlay will update the nodes and properties
for /testcase-data-2/fairway-1/ride@100/ after the patch is applied.

Before this patch is applied:

   Console error message near end of unittest:
      OF: Duplicate name in fairway-1, renamed to "ride@100#1"

   $ cd /proc/device-tree/testcase-data-2/fairway-1/
   $ # extra node: ride@100#1
   $ ls
   #address-cells  linux,phandle   phandle         ride@200
   #size-cells     name            ride@100        status
   compatible      orientation     ride@100#1
   $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
   $ ls track@3/incline_up
   ls: track@3/incline_up: No such file or directory
   $ ls track@4/incline_up
   ls: track@4/incline_up: No such file or directory

After this patch is applied:

   Console error message no longer occurs

   $ cd /proc/device-tree/testcase-data-2/fairway-1/
   $ # no extra node: ride@100#1
   $ ls
   #address-cells  compatible      name            phandle         ride@200
   #size-cells     linux,phandle   orientation     ride@100        status
   $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
   $ ls track@3/incline_up
   track@3/incline_up
   $ ls track@4/incline_up
   track@4/incline_up

Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
 drivers/of/overlay.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index c0e4ee1cd1ba..30aef51eeee5 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -118,6 +118,24 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
 	return of_changeset_update_property(&ov->cset, target, propn);
 }
 
+static struct device_node *child_by_full_name(const struct device_node *np,
+		const char *cname)
+{
+	struct device_node *child;
+	struct device_node *prev;
+
+	child = np->child;
+	while (child) {
+		of_node_get(child);
+		if (!of_node_cmp(cname, kbasename(child->full_name)))
+			break;
+		prev = child;
+		child = child->sibling;
+		of_node_put(prev);
+	}
+	return child;
+}
+
 static int of_overlay_apply_single_device_node(struct of_overlay *ov,
 		struct device_node *target, struct device_node *child)
 {
@@ -130,7 +148,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
 		return -ENOMEM;
 
 	/* NOTE: Multiple mods of created nodes not supported */
-	tchild = of_get_child_by_name(target, cname);
+	tchild = child_by_full_name(target, cname);
 	if (tchild != NULL) {
 		/* new overlay phandle value conflicts with existing value */
 		if (child->phandle)
-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] of: overlay: add overlay symbols to live device tree

[frowand.list]

From: Frank Rowand <frank.rowand@sony.com>

Add overlay __symbols__ properties to live tree when an overlay
is added to the live tree so that the symbols are available to
subsequent overlays.

Expected test result is new __symbols__ entries for labels from
the overlay after the patch is applied.

Before this patch is applied:

   Console error message near end of unittest:
      ### dt-test ### FAIL of_unittest_overlay_high_level():2296 Adding overlay 'overlay_bad_symbol' failed
      ### dt-test ### end of unittest - 190 passed, 1 failed

   The new unittest "fails" because the expected result of loading the
   new overlay is an error instead of success.

   $ # node hvac-medium-2 exists because the overlay loaded
   $ # since the duplicate symbol was not detected
   $ cd /proc/device-tree/testcase-data-2/substation@100/
   $ ls
   compatible     hvac-medium-2  motor-8        reg
   hvac-large-1   linux,phandle  name           status
   hvac-medium-1  motor-1        phandle

   $ cd /proc/device-tree/__symbols__/
   $ ls
   electric_1   lights_1     name         rides_1      spin_ctrl_2
   hvac_1       lights_2     retail_1     spin_ctrl_1

After this patch is applied:

   Previous console error message no longer occurs, but expected error
   occurs:
      OF: overlay: Failed to apply prop @/__symbols__/hvac_1
      OF: overlay: apply failed '/__symbols__'
      ### dt-test ### end of unittest - 191 passed, 0 failed

   $ # node hvac-medium-2 does not exist because the overlay
   $ # properly failed to load due to the duplicate symbol
   $ cd /proc/device-tree/testcase-data-2/substation@100/
   $ ls
   compatible     hvac-medium-1  motor-1        name           reg
   hvac-large-1   linux,phandle  motor-8        phandle        status

   $ cd /proc/device-tree/__symbols__/
   $ ls
   electric_1      lights_1        retail_1        ride_200_right  spin_ctrl_2
   hvac_1          lights_2        ride_200        rides_1
   hvac_2          name            ride_200_left   spin_ctrl_1
   $ cat ride_200; echo
   /testcase-data-2/fairway-1/ride@200
   $ cat ride_200_left ; echo
   /testcase-data-2/fairway-1/ride@200/track@1
   $ cat ride_200_right ; echo
   /testcase-data-2/fairway-1/ride@200/track@2

Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
 drivers/of/overlay.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 107 insertions(+), 9 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 30aef51eeee5..c484712d124a 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -35,6 +35,7 @@
 struct of_overlay_info {
 	struct device_node *target;
 	struct device_node *overlay;
+	bool is_symbols_node;
 };
 
 /**
@@ -55,7 +56,8 @@ struct of_overlay {
 };
 
 static int of_overlay_apply_one(struct of_overlay *ov,
-		struct device_node *target, const struct device_node *overlay);
+		struct device_node *target, const struct device_node *overlay,
+		bool is_symbols_node);
 
 static BLOCKING_NOTIFIER_HEAD(of_overlay_chain);
 
@@ -92,10 +94,74 @@ static int of_overlay_notify(struct of_overlay *ov,
 	return 0;
 }
 
+static struct property *dup_and_fixup_symbol_prop(struct of_overlay *ov,
+		const struct property *prop)
+{
+	struct of_overlay_info *ovinfo;
+	struct property *new;
+	const char *overlay_name;
+	char *label_path;
+	char *symbol_path;
+	const char *target_path;
+	int k;
+	int label_path_len;
+	int overlay_name_len;
+	int target_path_len;
+
+	if (!prop->value)
+		return NULL;
+	symbol_path = prop->value;
+
+	new = kzalloc(sizeof(*new), GFP_KERNEL);
+	if (!new)
+		return NULL;
+
+	for (k = 0; k < ov->count; k++) {
+		ovinfo = &ov->ovinfo_tab[k];
+		overlay_name = ovinfo->overlay->full_name;
+		overlay_name_len = strlen(overlay_name);
+		if (!strncasecmp(symbol_path, overlay_name, overlay_name_len))
+			break;
+	}
+
+	if (k >= ov->count)
+		goto err_free;
+
+	target_path = ovinfo->target->full_name;
+	target_path_len = strlen(target_path);
+
+	label_path = symbol_path + overlay_name_len;
+	label_path_len = strlen(label_path);
+
+	new->name = kstrdup(prop->name, GFP_KERNEL);
+	new->length = target_path_len + label_path_len + 1;
+	new->value = kzalloc(new->length, GFP_KERNEL);
+
+	if (!new->name || !new->value)
+		goto err_free;
+
+	strcpy(new->value, target_path);
+	strcpy(new->value + target_path_len, label_path);
+
+	/* mark the property as dynamic */
+	of_property_set_flag(new, OF_DYNAMIC);
+
+	return new;
+
+ err_free:
+	kfree(new->name);
+	kfree(new->value);
+	kfree(new);
+	return NULL;
+
+
+}
+
 static int of_overlay_apply_single_property(struct of_overlay *ov,
-		struct device_node *target, struct property *prop)
+		struct device_node *target, struct property *prop,
+		bool is_symbols_node)
 {
-	struct property *propn, *tprop;
+	struct property *propn = NULL, *tprop;
 
 	/* NOTE: Multiple changes of single properties not supported */
 	tprop = of_find_property(target, prop->name, NULL);
@@ -106,7 +172,15 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
 	    of_prop_cmp(prop->name, "linux,phandle") == 0)
 		return 0;
 
-	propn = __of_prop_dup(prop, GFP_KERNEL);
+	if (is_symbols_node) {
+		/* changing a property in __symbols__ node not allowed */
+		if (tprop)
+			return -EINVAL;
+		propn = dup_and_fixup_symbol_prop(ov, prop);
+	} else {
+		propn = __of_prop_dup(prop, GFP_KERNEL);
+	}
+
 	if (propn == NULL)
 		return -ENOMEM;
 
@@ -155,7 +229,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
 			return -EINVAL;
 
 		/* apply overlay recursively */
-		ret = of_overlay_apply_one(ov, tchild, child);
+		ret = of_overlay_apply_one(ov, tchild, child, 0);
 		of_node_put(tchild);
 	} else {
 		/* create empty tree as a target */
@@ -170,7 +244,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
 		if (ret)
 			return ret;
 
-		ret = of_overlay_apply_one(ov, tchild, child);
+		ret = of_overlay_apply_one(ov, tchild, child, 0);
 		if (ret)
 			return ret;
 	}
@@ -186,14 +260,16 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
  * by using the changeset.
  */
 static int of_overlay_apply_one(struct of_overlay *ov,
-		struct device_node *target, const struct device_node *overlay)
+		struct device_node *target, const struct device_node *overlay,
+		bool is_symbols_node)
 {
 	struct device_node *child;
 	struct property *prop;
 	int ret;
 
 	for_each_property_of_node(overlay, prop) {
-		ret = of_overlay_apply_single_property(ov, target, prop);
+		ret = of_overlay_apply_single_property(ov, target, prop,
+						       is_symbols_node);
 		if (ret) {
 			pr_err("Failed to apply prop @%s/%s\n",
 			       target->full_name, prop->name);
@@ -201,6 +277,10 @@ static int of_overlay_apply_one(struct of_overlay *ov,
 		}
 	}
 
+	/* do not allow symbols node to have any children */
+	if (is_symbols_node)
+		return 0;
+
 	for_each_child_of_node(overlay, child) {
 		ret = of_overlay_apply_single_device_node(ov, target, child);
 		if (ret != 0) {
@@ -231,7 +311,8 @@ static int of_overlay_apply(struct of_overlay *ov)
 	for (i = 0; i < ov->count; i++) {
 		struct of_overlay_info *ovinfo = &ov->ovinfo_tab[i];
 
-		err = of_overlay_apply_one(ov, ovinfo->target, ovinfo->overlay);
+		err = of_overlay_apply_one(ov, ovinfo->target, ovinfo->overlay,
+					   ovinfo->is_symbols_node);
 		if (err != 0) {
 			pr_err("apply failed '%s'\n", ovinfo->target->full_name);
 			return err;
@@ -329,6 +410,9 @@ static int of_build_overlay_info(struct of_overlay *ov,
 	for_each_child_of_node(tree, node)
 		cnt++;
 
+	if (of_get_child_by_name(tree, "__symbols__"))
+		cnt++;
+
 	ovinfo = kcalloc(cnt, sizeof(*ovinfo), GFP_KERNEL);
 	if (ovinfo == NULL)
 		return -ENOMEM;
@@ -340,6 +424,20 @@ static int of_build_overlay_info(struct of_overlay *ov,
 			cnt++;
 	}
 
+	node = of_get_child_by_name(tree, "__symbols__");
+	if (node) {
+		ovinfo[cnt].overlay = node;
+		ovinfo[cnt].target = of_find_node_by_path("/__symbols__");
+		ovinfo[cnt].is_symbols_node = 1;
+
+		if (!ovinfo[cnt].target) {
+			pr_err("no symbols in root of device tree.\n");
+			return -EINVAL;
+		}
+
+		cnt++;
+	}
+
 	/* if nothing filled, return error */
 	if (cnt == 0) {
 		kfree(ovinfo);
-- 
Frank Rowand <frank.rowand@sony.com>

Re: [PATCH 2/3] of: overlay: correctly apply overlay node with unit-address

[Rob Herring]

On Fri, Jul 7, 2017 at 7:28 PM,  <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@sony.com>
>
> Correct existing node name detection when overlay node name has
> a unit-address.
>
> Expected test result is overlay will update the nodes and properties
> for /testcase-data-2/fairway-1/ride@100/ after the patch is applied.
>
> Before this patch is applied:
>
>    Console error message near end of unittest:
>       OF: Duplicate name in fairway-1, renamed to "ride@100#1"
>
>    $ cd /proc/device-tree/testcase-data-2/fairway-1/
>    $ # extra node: ride@100#1
>    $ ls
>    #address-cells  linux,phandle   phandle         ride@200
>    #size-cells     name            ride@100        status
>    compatible      orientation     ride@100#1
>    $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
>    $ ls track@3/incline_up
>    ls: track@3/incline_up: No such file or directory
>    $ ls track@4/incline_up
>    ls: track@4/incline_up: No such file or directory
>
> After this patch is applied:
>
>    Console error message no longer occurs
>
>    $ cd /proc/device-tree/testcase-data-2/fairway-1/
>    $ # no extra node: ride@100#1
>    $ ls
>    #address-cells  compatible      name            phandle         ride@200
>    #size-cells     linux,phandle   orientation     ride@100        status
>    $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
>    $ ls track@3/incline_up
>    track@3/incline_up
>    $ ls track@4/incline_up
>    track@4/incline_up
>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
>  drivers/of/overlay.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index c0e4ee1cd1ba..30aef51eeee5 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -118,6 +118,24 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
>         return of_changeset_update_property(&ov->cset, target, propn);
>  }
>
> +static struct device_node *child_by_full_name(const struct device_node *np,

It's not really the full name which currently means the whole path (my
full_name work is going to change that), but the unit_name (at least
that's what dtc calls it).

> +               const char *cname)
> +{
> +       struct device_node *child;
> +       struct device_node *prev;
> +
> +       child = np->child;
> +       while (child) {

Doesn't for_each_child_of_node() work here?

> +               of_node_get(child);
> +               if (!of_node_cmp(cname, kbasename(child->full_name)))
> +                       break;
> +               prev = child;
> +               child = child->sibling;
> +               of_node_put(prev);
> +       }
> +       return child;
> +}
> +
>  static int of_overlay_apply_single_device_node(struct of_overlay *ov,
>                 struct device_node *target, struct device_node *child)
>  {
> @@ -130,7 +148,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
>                 return -ENOMEM;
>
>         /* NOTE: Multiple mods of created nodes not supported */
> -       tchild = of_get_child_by_name(target, cname);
> +       tchild = child_by_full_name(target, cname);
>         if (tchild != NULL) {
>                 /* new overlay phandle value conflicts with existing value */
>                 if (child->phandle)
> --
> Frank Rowand <frank.rowand@sony.com>
>

Re: [PATCH 2/3] of: overlay: correctly apply overlay node with unit-address

[Frank Rowand]

On 07/10/17 09:08, Rob Herring wrote:
> On Fri, Jul 7, 2017 at 7:28 PM,  <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>>
>> Correct existing node name detection when overlay node name has
>> a unit-address.
>>
>> Expected test result is overlay will update the nodes and properties
>> for /testcase-data-2/fairway-1/ride@100/ after the patch is applied.
>>
>> Before this patch is applied:
>>
>>    Console error message near end of unittest:
>>       OF: Duplicate name in fairway-1, renamed to "ride@100#1"
>>
>>    $ cd /proc/device-tree/testcase-data-2/fairway-1/
>>    $ # extra node: ride@100#1
>>    $ ls
>>    #address-cells  linux,phandle   phandle         ride@200
>>    #size-cells     name            ride@100        status
>>    compatible      orientation     ride@100#1
>>    $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
>>    $ ls track@3/incline_up
>>    ls: track@3/incline_up: No such file or directory
>>    $ ls track@4/incline_up
>>    ls: track@4/incline_up: No such file or directory
>>
>> After this patch is applied:
>>
>>    Console error message no longer occurs
>>
>>    $ cd /proc/device-tree/testcase-data-2/fairway-1/
>>    $ # no extra node: ride@100#1
>>    $ ls
>>    #address-cells  compatible      name            phandle         ride@200
>>    #size-cells     linux,phandle   orientation     ride@100        status
>>    $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
>>    $ ls track@3/incline_up
>>    track@3/incline_up
>>    $ ls track@4/incline_up
>>    track@4/incline_up
>>
>> Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>> ---
>>  drivers/of/overlay.c | 20 +++++++++++++++++++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index c0e4ee1cd1ba..30aef51eeee5 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -118,6 +118,24 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
>>         return of_changeset_update_property(&ov->cset, target, propn);
>>  }
>>
>> +static struct device_node *child_by_full_name(const struct device_node *np,
> 
> It's not really the full name which currently means the whole path (my
> full_name work is going to change that), but the unit_name (at least
> that's what dtc calls it).

Yes, thanks.  I would change this name, but your next comment below
allows me to remove this function instead.


>> +               const char *cname)
>> +{
>> +       struct device_node *child;
>> +       struct device_node *prev;
>> +
>> +       child = np->child;
>> +       while (child) {
> 
> Doesn't for_each_child_of_node() work here?

Yes, thanks.  And it makes the code compact enough that I can just
put it inside of_overlay_apply_single_device_node() instead of
creating this extra function.


>> +               of_node_get(child);
>> +               if (!of_node_cmp(cname, kbasename(child->full_name)))
>> +                       break;
>> +               prev = child;
>> +               child = child->sibling;
>> +               of_node_put(prev);
>> +       }
>> +       return child;
>> +}
>> +
>>  static int of_overlay_apply_single_device_node(struct of_overlay *ov,
>>                 struct device_node *target, struct device_node *child)
>>  {
>> @@ -130,7 +148,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
>>                 return -ENOMEM;
>>
>>         /* NOTE: Multiple mods of created nodes not supported */
>> -       tchild = of_get_child_by_name(target, cname);
>> +       tchild = child_by_full_name(target, cname);
>>         if (tchild != NULL) {
>>                 /* new overlay phandle value conflicts with existing value */
>>                 if (child->phandle)
>> --
>> Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html