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@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@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)