dtoc: Support adding subnodes alongside existing ones

So far we have only needed to add subnodes to empty notds, so have not
had to deal with ordering. However this feature is needed for binman's
expanded nodes, since there may be another node in the same section.

While libfdt adds new properties after existing properties, it adds new
subnodes before existing subnodes. This means that we must reorder the
nodes in the cached version, so that the ordering remains consistent.

Update the sync implementation to sync existing subnodes first, then
add new ones, then tidy up the ordering in the cached version. Update the
test to cover this behaviour.

Also improve the comment about property syncing while we are here.

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

2 files changed, 51 insertions, 9 deletions

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index a5e1d0b52f6..63d1f68d816 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -503,9 +503,13 @@ class Node:
             auto_resize: Resize the device tree automatically if it does not
                 have enough space for the update
 
+        Returns:
+            True if the node had to be added, False if it already existed
+
         Raises:
             FdtException if auto_resize is False and there is not enough space
         """
+        added = False
         if self._offset is None:
             # The subnode doesn't exist yet, so add it
             fdt_obj = self._fdt._fdt_obj
@@ -519,23 +523,45 @@ class Node:
             else:
                 offset = fdt_obj.add_subnode(self.parent._offset, self.name)
             self._offset = offset
+            added = True
 
-        # Sync subnodes in reverse so that we don't disturb node offsets for
-        # nodes that are earlier in the DT. This avoids an O(n^2) rescan of
-        # node offsets.
+        # Sync the existing subnodes first, so that we can rely on the offsets
+        # being correct. As soon as we add new subnodes, it pushes all the
+        # existing subnodes up.
         for node in reversed(self.subnodes):
-            node.Sync(auto_resize)
+            if node._offset is not None:
+                node.Sync(auto_resize)
 
-        # Sync properties now, whose offsets should not have been disturbed.
-        # We do this after subnodes, since this disturbs the offsets of these
-        # properties. Note that new properties will have an offset of None here,
-        # which Python 3 cannot sort against int. So use a large value instead
-        # to ensure that the new properties are added first.
+        # Sync subnodes in reverse so that we get the expected order. Each
+        # new node goes at the start of the subnode list. This avoids an O(n^2)
+        # rescan of node offsets.
+        num_added = 0
+        for node in reversed(self.subnodes):
+            if node.Sync(auto_resize):
+                num_added += 1
+        if num_added:
+            # Reorder our list of nodes to put the new ones first, since that's
+            # what libfdt does
+            old_count = len(self.subnodes) - num_added
+            subnodes = self.subnodes[old_count:] + self.subnodes[:old_count]
+            self.subnodes = subnodes
+
+        # Sync properties now, whose offsets should not have been disturbed,
+        # since properties come before subnodes. This is done after all the
+        # subnode processing above, since updating properties can disturb the
+        # offsets of those subnodes.
+        # Properties are synced in reverse order, with new properties added
+        # before existing properties are synced. This ensures that the offsets
+        # of earlier properties are not disturbed.
+        # Note that new properties will have an offset of None here, which
+        # Python cannot sort against int. So use a large value instead so that
+        # new properties are added first.
         prop_list = sorted(self.props.values(),
                            key=lambda prop: prop._offset or 1 << 31,
                            reverse=True)
         for prop in prop_list:
             prop.Sync(auto_resize)
+        return added
 
 
 class Fdt:
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index 1e66e1bc353..49a2853f07f 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -237,6 +237,22 @@ class TestNode(unittest.TestCase):
         """Test adding various subnode and properies"""
         node = self.dtb.GetNode('/i2c@0')
 
+        # Add one more node next to the pmic one
+        sn1 = node.AddSubnode('node-one')
+        sn1.AddInt('integer-a', 12)
+        sn1.AddInt('integer-b', 23)
+
+        # Sync so that everything is clean
+        self.dtb.Sync(auto_resize=True)
+
+        # Add two subnodes next to pmic and node-one
+        sn2 = node.AddSubnode('node-two')
+        sn2.AddInt('integer-2a', 34)
+        sn2.AddInt('integer-2b', 45)
+
+        sn3 = node.AddSubnode('node-three')
+        sn3.AddInt('integer-3', 123)
+
         # Add a property to the node after i2c@0 to check that this is not
         # disturbed by adding a subnode to i2c@0
         orig_node = self.dtb.GetNode('/orig-node')