[PATCH] livetree: Add only new data to fixup nodes instead of complete regeneration

Removing the complete __fixups__ and __local_fixups__ tree might delete
data that should better be retained. See the added test for a situation
that was broken before.

Fixes: 915daadbb62d ("Start with empty __local_fixups__ and __fixups__ nodes")
Closes: https://github.com/dgibson/dtc/issues/170
Signed-off-by: Uwe Kleine-König <[email protected]>
---
Hello,

this fixes the fallout of commit 915daadbb62d ("Start with empty
__local_fixups__ and __fixups__ nodes") that was found while discussing
https://github.com/dgibson/dtc/pull/151 .

I'm a bit annoyed by the github workflow, so here comes a patch on the
mailing list :-)

Best regards
Uwe

 livetree.c              | 75 +++++++++++++++++++++++++++++++----------
 tests/retain-fixups.dts | 29 ++++++++++++++++
 tests/run_tests.sh      |  5 +++
 3 files changed, 92 insertions(+), 17 deletions(-)
 create mode 100644 tests/retain-fixups.dts

diff --git a/livetree.c b/livetree.c
index d51d05830b18..24f7c561d77e 100644
--- a/livetree.c
+++ b/livetree.c
@@ -356,6 +356,60 @@ void append_to_property(struct node *node,
 	}
 }
 
+static void append_unique_str_to_property(struct node *node,
+					  char *name, const char *data, int len)
+{
+	struct data d;
+	struct property *p;
+
+	p = get_property(node, name);
+	if (p) {
+		const char *s;
+
+		for (s = p->val.val; s < p->val.val + p->val.len; s = strchr(s, '\0') + 1) {
+			if (strcmp(data, s) == 0)
+				/* data already contained in node.name */
+				return;
+		}
+
+		d = data_add_marker(p->val, TYPE_STRING, name);
+		d = data_append_data(d, data, len);
+		p->val = d;
+	} else {
+		d = data_add_marker(empty_data, TYPE_STRING, name);
+		d = data_append_data(d, data, len);
+		p = build_property(name, d, NULL);
+		add_property(node, p);
+	}
+
+}
+
+static void append_unique_u32_to_property(struct node *node, char *name, fdt32_t value)
+{
+	struct data d;
+	struct property *p;
+
+	p = get_property(node, name);
+	if (p) {
+		const fdt32_t *v;
+
+		for (v = (const void *)p->val.val; (const void *)v < (const void *)(p->val.val + p->val.len - 3); v++) {
+			if (*v == value)
+				/* value already contained */
+				return;
+		}
+
+		d = data_add_marker(p->val, TYPE_UINT32, name);
+		d = data_append_data(d, &value, 4);
+		p->val = d;
+	} else {
+		d = data_add_marker(empty_data, TYPE_UINT32, name);
+		d = data_append_data(d, &value, 4);
+		p = build_property(name, d, NULL);
+		add_property(node, p);
+	}
+}
+
 struct reserve_info *build_reserve_entry(uint64_t address, uint64_t size)
 {
 	struct reserve_info *new = xmalloc(sizeof(*new));
@@ -939,7 +993,7 @@ static void add_fixup_entry(struct dt_info *dti, struct node *fn,
 
 	xasprintf(&entry, "%s:%s:%u",
 			node->fullpath, prop->name, m->offset);
-	append_to_property(fn, m->ref, entry, strlen(entry) + 1, TYPE_STRING);
+	append_unique_str_to_property(fn, m->ref, entry, strlen(entry) + 1);
 
 	free(entry);
 }
@@ -1020,7 +1074,7 @@ static void add_local_fixup_entry(struct dt_info *dti,
 	free(compp);
 
 	value_32 = cpu_to_fdt32(m->offset);
-	append_to_property(wn, prop->name, &value_32, sizeof(value_32), TYPE_UINT32);
+	append_unique_u32_to_property(wn, prop->name, value_32);
 }
 
 static void generate_local_fixups_tree_internal(struct dt_info *dti,
@@ -1056,29 +1110,16 @@ void generate_label_tree(struct dt_info *dti, const char *name, bool allocph)
 
 void generate_fixups_tree(struct dt_info *dti, const char *name)
 {
-	struct node *n = get_subnode(dti->dt, name);
-
-	/* Start with an empty __fixups__ node to not get duplicates */
-	if (n)
-		n->deleted = true;
-
 	if (!any_fixup_tree(dti, dti->dt))
 		return;
-	generate_fixups_tree_internal(dti,
-				      build_and_name_child_node(dti->dt, name),
+	generate_fixups_tree_internal(dti, build_root_node(dti->dt, name),
 				      dti->dt);
 }
 
 void generate_local_fixups_tree(struct dt_info *dti, const char *name)
 {
-	struct node *n = get_subnode(dti->dt, name);
-
-	/* Start with an empty __local_fixups__ node to not get duplicates */
-	if (n)
-		n->deleted = true;
 	if (!any_local_fixup_tree(dti, dti->dt))
 		return;
-	generate_local_fixups_tree_internal(dti,
-					    build_and_name_child_node(dti->dt, name),
+	generate_local_fixups_tree_internal(dti, build_root_node(dti->dt, name),
 					    dti->dt);
 }
diff --git a/tests/retain-fixups.dts b/tests/retain-fixups.dts
new file mode 100644
index 000000000000..7b7b52210659
--- /dev/null
+++ b/tests/retain-fixups.dts
@@ -0,0 +1,29 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+	fixup-node {
+		property = <0xffffffff>;
+		property-with-fixup = <0xffffffff>;
+		property-with-label = <&somenode>;
+		property-with-label-and-fixup = <&somenode>;
+	};
+
+	label: local-fixup-node {
+		property = <0xffffffff>;
+		property-with-local-fixup = <0xffffffff>;
+		property-with-local-label = <&label>;
+		property-with-local-label-and-fixup = <&label>;
+	};
+	
+	__fixups__ {
+		somenode = "/fixup-node:property-with-fixup:0", "/fixup-node:property-with-label-and-fixup:0";
+	};
+
+	__local_fixups__ {
+		local-fixup-node {
+			property-with-local-fixup = <0x00>;
+			property-with-local-label-and-fixup = <0x00>;
+		};
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index fecfe7cc09b3..1c9278d93689 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -666,6 +666,11 @@ dtc_tests () {
         run_test dtbs_equal_ordered $tree.test.dtb $tree.test.dts.test.dtb
     done
 
+    # Check preservation of __fixups__ and __local_fixups__
+    run_dtc_test -I dts -O dtb -o retain-fixups.test.dtb "$SRCDIR/retain-fixups.dts"
+    run_fdtget_test "/fixup-node:property-with-fixup:0 /fixup-node:property-with-label-and-fixup:0 /fixup-node:property-with-label:0" retain-fixups.test.dtb /__fixups__ somenode
+    run_fdtget_test "property-with-local-fixup\nproperty-with-local-label-and-fixup\nproperty-with-local-label" -p retain-fixups.test.dtb /__local_fixups__/local-fixup-node
+
     # Check -Oyaml output
     if ! $no_yaml; then
             for tree in type-preservation; do

base-commit: 84d9dd2fcbc865a35d7f04d9b465b05ef286d281
-- 
2.50.1