[PATCH v2 4/6] fdt: Allow copying phandles into templates

[Simon Glass]

Allow phandles to be copied over from a template. This can potentially
cause duplicate phandles, so detect this and report an error.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

Changes in v2:
- Handle phandle copying property and report duplicates

 tools/dtoc/fdt.py      | 28 +++++++++++++++++++---------
 tools/dtoc/test_fdt.py | 21 +++++++++++++++++----
 2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index f6a9dee0db12..5963925146a5 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -337,6 +337,11 @@ class Node:
         self.props = self._fdt.GetProps(self)
         phandle = fdt_obj.get_phandle(self.Offset())
         if phandle:
+            dup = self._fdt.phandle_to_node.get(phandle)
+            if dup:
+                raise ValueError(
+                    f'Duplicate phandle {phandle} in nodes {dup.path} and {self.path}')
+
             self._fdt.phandle_to_node[phandle] = self
 
         offset = fdt_obj.first_subnode(self.Offset(), QUIET_NOTFOUND)
@@ -707,11 +712,12 @@ class Node:
             prop.Sync(auto_resize)
         return added
 
-    def merge_props(self, src):
+    def merge_props(self, src, copy_phandles):
         """Copy missing properties (except 'phandle') from another node
 
         Args:
             src (Node): Node containing properties to copy
+            copy_phandles (bool): True to copy phandle properties in nodes
 
         Adds properties which are present in src but not in this node. Any
         'phandle' property is not copied since this might result in two nodes
@@ -720,21 +726,24 @@ class Node:
         tout.debug(f'copy to {self.path}: {src.path}')
         for name, src_prop in src.props.items():
             done = False
-            if name != 'phandle' and name not in self.props:
-                self.props[name] = Prop(self, None, name, src_prop.bytes)
-                done = True
+            if name not in self.props:
+                if copy_phandles or name != 'phandle':
+                    self.props[name] = Prop(self, None, name, src_prop.bytes)
+                    done = True
             tout.debug(f"  {name}{'' if done else '  - ignored'}")
 
-    def copy_node(self, src):
+    def copy_node(self, src, copy_phandles=False):
         """Copy a node and all its subnodes into this node
 
         Args:
             src (Node): Node to copy
+            copy_phandles (bool): True to copy phandle properties in nodes
 
         Returns:
             Node: Resulting destination node
 
-        This works recursively.
+        This works recursively, with copy_phandles being set to True for the
+        recursive calls
 
         The new node is put before all other nodes. If the node already
         exists, just its subnodes and properties are copied, placing them before
@@ -746,12 +755,12 @@ class Node:
             dst.move_to_first()
         else:
             dst = self.insert_subnode(src.name)
-        dst.merge_props(src)
+        dst.merge_props(src, copy_phandles)
 
         # Process in reverse order so that they appear correctly in the result,
         # since copy_node() puts the node first in the list
         for node in reversed(src.subnodes):
-            dst.copy_node(node)
+            dst.copy_node(node, True)
         return dst
 
     def copy_subnodes_from_phandles(self, phandle_list):
@@ -774,7 +783,7 @@ class Node:
                 dst = self.copy_node(node)
 
             tout.debug(f'merge props from {parent.path} to {dst.path}')
-            self.merge_props(parent)
+            self.merge_props(parent, False)
 
 
 class Fdt:
@@ -835,6 +844,7 @@ class Fdt:
 
         TODO(sjg at chromium.org): Implement the 'root' parameter
         """
+        self.phandle_to_node = {}
         self._cached_offsets = True
         self._root = self.Node(self, None, 0, '/', '/')
         self._root.Scan()
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index f77e48b54eaf..0b01518f3a5b 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -340,8 +340,8 @@ class TestNode(unittest.TestCase):
             over = dtb.GetNode('/dest/base/over')
             self.assertTrue(over)
 
-            # Make sure that the phandle for 'over' is not copied
-            self.assertNotIn('phandle', over.props.keys())
+            # Make sure that the phandle for 'over' is copied
+            self.assertIn('phandle', over.props.keys())
 
             second = dtb.GetNode('/dest/base/second')
             self.assertTrue(second)
@@ -349,7 +349,7 @@ class TestNode(unittest.TestCase):
                              [n.name for n in chk.subnodes])
             self.assertEqual(chk, over.parent)
             self.assertEqual(
-                {'bootph-all', 'compatible', 'reg', 'low-power'},
+                {'bootph-all', 'compatible', 'reg', 'low-power', 'phandle'},
                 over.props.keys())
 
             if expect_none:
@@ -385,9 +385,22 @@ class TestNode(unittest.TestCase):
 
         dtb.Sync(auto_resize=True)
 
-        # Now check that the FDT looks correct
+        # Now check the resulting FDT. It should have duplicate phandles since
+        # 'over' has been copied to 'dest/base/over' but still exists in its old
+        # place
         new_dtb = fdt.Fdt.FromData(dtb.GetContents())
+        with self.assertRaises(ValueError) as exc:
+            new_dtb.Scan()
+        self.assertIn(
+            'Duplicate phandle 1 in nodes /dest/base/over and /base/over',
+            str(exc.exception))
+
+        # Remove the source nodes for the copy
+        new_dtb.GetNode('/base').Delete()
+
+        # Now it should scan OK
         new_dtb.Scan()
+
         dst = new_dtb.GetNode('/dest')
         do_copy_checks(new_dtb, dst, second1_ph_val, expect_none=False)
 
-- 
2.41.0.487.g6d72f3e995-goog