From 1000096b06ea53487b3457eb1d0d1704276c1c62 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:23 -0600
Subject: dtoc: Return a non-zero exit code when tests fail

At present 'dtoc -t' return a success code even if some of the tests fail.
Fix this by checking the test result and setting the exit code. This
allows 'make qcheck' to function as expected.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/dtoc/dtoc.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/dtoc/dtoc.py b/tools/dtoc/dtoc.py
index c1a1d3534d4..514e0dd4a34 100755
--- a/tools/dtoc/dtoc.py
+++ b/tools/dtoc/dtoc.py
@@ -71,6 +71,10 @@ def run_tests(args):
         print(err)
     for _, err in result.failures:
         print(err)
+    if result.errors or result.failures:
+        print('dtoc tests FAILED')
+        return 1
+    return 0
 
 def RunTestCoverage():
     """Run the tests and check that we get 100% coverage"""
@@ -101,7 +105,8 @@ parser.add_option('-T', '--test-coverage', action='store_true',
 
 # Run our meagre tests
 if options.test:
-    run_tests(args)
+    ret_code = run_tests(args)
+    sys.exit(ret_code)
 
 elif options.test_coverage:
     RunTestCoverage()
-- 
cgit v1.2.3


From b88e81c622fd683f59498b03d34c88ce7fe68184 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:24 -0600
Subject: binman: Move image-processing code into a function

The Binman() function is very long. Split out the image code to make it
more manageable.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/control.py | 103 +++++++++++++++++++++++++++---------------------
 1 file changed, 57 insertions(+), 46 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index dc898be6179..a92056846d0 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -169,6 +169,62 @@ def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
     return einfos
 
 
+def ProcessImage(image, update_fdt, write_map):
+    """Perform all steps for this image, including checking and # writing it.
+
+    This means that errors found with a later image will be reported after
+    earlier images are already completed and written, but that does not seem
+    important.
+
+    Args:
+        image: Image to process
+        update_fdt: True to update the FDT wth entry offsets, etc.
+        write_map: True to write a map file
+    """
+    image.GetEntryContents()
+    image.GetEntryOffsets()
+
+    # We need to pack the entries to figure out where everything
+    # should be placed. This sets the offset/size of each entry.
+    # However, after packing we call ProcessEntryContents() which
+    # may result in an entry changing size. In that case we need to
+    # do another pass. Since the device tree often contains the
+    # final offset/size information we try to make space for this in
+    # AddMissingProperties() above. However, if the device is
+    # compressed we cannot know this compressed size in advance,
+    # since changing an offset from 0x100 to 0x104 (for example) can
+    # alter the compressed size of the device tree. So we need a
+    # third pass for this.
+    passes = 3
+    for pack_pass in range(passes):
+        try:
+            image.PackEntries()
+            image.CheckSize()
+            image.CheckEntries()
+        except Exception as e:
+            if write_map:
+                fname = image.WriteMap()
+                print("Wrote map file '%s' to show errors"  % fname)
+            raise
+        image.SetImagePos()
+        if update_fdt:
+            image.SetCalculatedProperties()
+            for dtb_item in state.GetFdts():
+                dtb_item.Sync()
+        sizes_ok = image.ProcessEntryContents()
+        if sizes_ok:
+            break
+        image.ResetForPack()
+    if not sizes_ok:
+        image.Raise('Entries expanded after packing (tried %s passes)' %
+                    passes)
+
+    image.WriteSymbols()
+    image.BuildImage()
+    if write_map:
+        image.WriteMap()
+
+
 def Binman(args):
     """The main control code for binman
 
@@ -279,52 +335,7 @@ def Binman(args):
                 dtb_item.Flush()
 
             for image in images.values():
-                # Perform all steps for this image, including checking and
-                # writing it. This means that errors found with a later
-                # image will be reported after earlier images are already
-                # completed and written, but that does not seem important.
-                image.GetEntryContents()
-                image.GetEntryOffsets()
-
-                # We need to pack the entries to figure out where everything
-                # should be placed. This sets the offset/size of each entry.
-                # However, after packing we call ProcessEntryContents() which
-                # may result in an entry changing size. In that case we need to
-                # do another pass. Since the device tree often contains the
-                # final offset/size information we try to make space for this in
-                # AddMissingProperties() above. However, if the device is
-                # compressed we cannot know this compressed size in advance,
-                # since changing an offset from 0x100 to 0x104 (for example) can
-                # alter the compressed size of the device tree. So we need a
-                # third pass for this.
-                passes = 3
-                for pack_pass in range(passes):
-                    try:
-                        image.PackEntries()
-                        image.CheckSize()
-                        image.CheckEntries()
-                    except Exception as e:
-                        if args.map:
-                            fname = image.WriteMap()
-                            print("Wrote map file '%s' to show errors"  % fname)
-                        raise
-                    image.SetImagePos()
-                    if args.update_fdt:
-                        image.SetCalculatedProperties()
-                        for dtb_item in state.GetFdts():
-                            dtb_item.Sync()
-                    sizes_ok = image.ProcessEntryContents()
-                    if sizes_ok:
-                        break
-                    image.ResetForPack()
-                if not sizes_ok:
-                    image.Raise('Entries expanded after packing (tried %s passes)' %
-                                passes)
-
-                image.WriteSymbols()
-                image.BuildImage()
-                if args.map:
-                    image.WriteMap()
+                ProcessImage(image, args.update_fdt, args.map)
 
             # Write the updated FDTs to our output files
             for dtb_item in state.GetFdts():
-- 
cgit v1.2.3


From 935461262e322fce6fcfc48caf2b9a6ec05caaae Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:25 -0600
Subject: binman: Move GetFdtSet() into blob_dtb

At present we check the filename to see if an entry holds a device-tree
file. It is easier to use the base class designed for this purpose.

Move this method implementation into Entry_blob_dtb and update the default
one to return an empty set.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/entry.py          | 5 -----
 tools/binman/etype/blob_dtb.py | 9 +++++++++
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 1c382f3b852..276035ed324 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -192,11 +192,6 @@ class Entry(object):
             Set containing the filename from this entry, if it is a .dtb, else
             an empty set
         """
-        fname = self.GetDefaultFilename()
-        # It would be better to use isinstance(self, Entry_blob_dtb) here but
-        # we cannot access Entry_blob_dtb
-        if fname and fname.endswith('.dtb'):
-            return set([fname])
         return set()
 
     def ExpandEntries(self):
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index 88ed55d8865..b242c2da38a 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -31,3 +31,12 @@ class Entry_blob_dtb(Entry_blob):
         _, indata = state.GetFdtContents(self._filename)
         data = self.CompressData(indata)
         return self.ProcessContentsUpdate(data)
+
+    def GetFdtSet(self):
+        """Get the set of device trees used by this entry
+
+        Returns:
+            Set containing the filename from this entry
+        """
+        fname = self.GetDefaultFilename()
+        return set([fname])
-- 
cgit v1.2.3


From 7b773167c083752eabf88bd37d55976244a48f6c Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:26 -0600
Subject: binman: Use print() to print output

At present tout writes directly to stdout. This is not necessary and it
prevents tests from redirecting output. Change it to use print() for the
non-progress output.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/patman/tout.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/patman/tout.py b/tools/patman/tout.py
index 15acce28cb9..ae04c30f1db 100644
--- a/tools/patman/tout.py
+++ b/tools/patman/tout.py
@@ -4,6 +4,8 @@
 # Terminal output logging.
 #
 
+from __future__ import print_function
+
 import sys
 
 import terminal
@@ -87,7 +89,7 @@ def _Output(level, msg, color=None):
         ClearProgress()
         if color:
             msg = _color.Color(color, msg)
-        _stdout.write(msg + '\n')
+        print(msg)
 
 def DoOutput(level, msg):
     """Output a message to the terminal.
-- 
cgit v1.2.3


From a8573c4c8fb07fbd7cc8db9828aae90fcfd5145d Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:27 -0600
Subject: binman: Move image/fdt code into PrepareImagesAndDtbs()

Further reduce the size of the main Binman() function by moving this setup
code into its own function.

Note that the 'images' value is accessed from other modules so must be made
a global.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/control.py | 125 +++++++++++++++++++++++++++---------------------
 1 file changed, 71 insertions(+), 54 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index a92056846d0..de9f29e2246 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -169,6 +169,75 @@ def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
     return einfos
 
 
+def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt):
+    """Prepare the images to be processed and select the device tree
+
+    This function:
+    - reads in the device tree
+    - finds and scans the binman node to create all entries
+    - selects which images to build
+    - Updates the device tress with placeholder properties for offset,
+        image-pos, etc.
+
+    Args:
+        dtb_fname: Filename of the device tree file to use (.dts or .dtb)
+        selected_images: List of images to output, or None for all
+        update_fdt: True to update the FDT wth entry offsets, etc.
+    """
+    # Import these here in case libfdt.py is not available, in which case
+    # the above help option still works.
+    import fdt
+    import fdt_util
+    global images
+
+    # Get the device tree ready by compiling it and copying the compiled
+    # output into a file in our output directly. Then scan it for use
+    # in binman.
+    dtb_fname = fdt_util.EnsureCompiled(dtb_fname)
+    fname = tools.GetOutputFilename('u-boot.dtb.out')
+    tools.WriteFile(fname, tools.ReadFile(dtb_fname))
+    dtb = fdt.FdtScan(fname)
+
+    node = _FindBinmanNode(dtb)
+    if not node:
+        raise ValueError("Device tree '%s' does not have a 'binman' "
+                            "node" % dtb_fname)
+
+    images = _ReadImageDesc(node)
+
+    if select_images:
+        skip = []
+        new_images = OrderedDict()
+        for name, image in images.items():
+            if name in select_images:
+                new_images[name] = image
+            else:
+                skip.append(name)
+        images = new_images
+        tout.Notice('Skipping images: %s' % ', '.join(skip))
+
+    state.Prepare(images, dtb)
+
+    # Prepare the device tree by making sure that any missing
+    # properties are added (e.g. 'pos' and 'size'). The values of these
+    # may not be correct yet, but we add placeholders so that the
+    # size of the device tree is correct. Later, in
+    # SetCalculatedProperties() we will insert the correct values
+    # without changing the device-tree size, thus ensuring that our
+    # entry offsets remain the same.
+    for image in images.values():
+        image.ExpandEntries()
+        if update_fdt:
+            image.AddMissingProperties()
+        image.ProcessFdt(dtb)
+
+    for dtb_item in state.GetFdts():
+        dtb_item.Sync(auto_resize=True)
+        dtb_item.Pack()
+        dtb_item.Flush()
+    return images
+
+
 def ProcessImage(image, update_fdt, write_map):
     """Perform all steps for this image, including checking and # writing it.
 
@@ -234,8 +303,6 @@ def Binman(args):
     Args:
         args: Command line arguments Namespace object
     """
-    global images
-
     if args.full_help:
         pager = os.getenv('PAGER')
         if not pager:
@@ -272,11 +339,6 @@ def Binman(args):
         args.indir.append(board_pathname)
 
     try:
-        # Import these here in case libfdt.py is not available, in which case
-        # the above help option still works.
-        import fdt
-        import fdt_util
-
         tout.Init(args.verbosity)
         elf.debug = args.debug
         cbfs_util.VERBOSE = args.verbosity > 2
@@ -287,53 +349,8 @@ def Binman(args):
             tools.SetToolPaths(args.toolpath)
             state.SetEntryArgs(args.entry_arg)
 
-            # Get the device tree ready by compiling it and copying the compiled
-            # output into a file in our output directly. Then scan it for use
-            # in binman.
-            dtb_fname = fdt_util.EnsureCompiled(dtb_fname)
-            fname = tools.GetOutputFilename('u-boot.dtb.out')
-            tools.WriteFile(fname, tools.ReadFile(dtb_fname))
-            dtb = fdt.FdtScan(fname)
-
-            node = _FindBinmanNode(dtb)
-            if not node:
-                raise ValueError("Device tree '%s' does not have a 'binman' "
-                                 "node" % dtb_fname)
-
-            images = _ReadImageDesc(node)
-
-            if args.image:
-                skip = []
-                new_images = OrderedDict()
-                for name, image in images.items():
-                    if name in args.image:
-                        new_images[name] = image
-                    else:
-                        skip.append(name)
-                images = new_images
-                if skip and args.verbosity >= 2:
-                    print('Skipping images: %s' % ', '.join(skip))
-
-            state.Prepare(images, dtb)
-
-            # Prepare the device tree by making sure that any missing
-            # properties are added (e.g. 'pos' and 'size'). The values of these
-            # may not be correct yet, but we add placeholders so that the
-            # size of the device tree is correct. Later, in
-            # SetCalculatedProperties() we will insert the correct values
-            # without changing the device-tree size, thus ensuring that our
-            # entry offsets remain the same.
-            for image in images.values():
-                image.ExpandEntries()
-                if args.update_fdt:
-                    image.AddMissingProperties()
-                image.ProcessFdt(dtb)
-
-            for dtb_item in state.GetFdts():
-                dtb_item.Sync(auto_resize=True)
-                dtb_item.Pack()
-                dtb_item.Flush()
-
+            images = PrepareImagesAndDtbs(dtb_fname, args.image,
+                                          args.update_fdt)
             for image in images.values():
                 ProcessImage(image, args.update_fdt, args.map)
 
-- 
cgit v1.2.3


From a8adb6dfebad9a08c2df9d9ee8f79b9518e57496 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:28 -0600
Subject: binman: Convert GetFdtSet() to use a dict

At present this function returns a set of device-tree filenames. It has no
way of returning the actual device-tree object. Change it to a dictionary
so that we can add this feature in a future patch.

Also drop fdt_set since it is no-longer used.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/entry.py          | 12 +++++++-----
 tools/binman/etype/blob_dtb.py | 11 ++++++-----
 tools/binman/etype/section.py  |  8 ++++----
 tools/binman/state.py          | 14 ++++++--------
 4 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 276035ed324..2ed9dc0d6f4 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -185,14 +185,16 @@ class Entry(object):
     def GetDefaultFilename(self):
         return None
 
-    def GetFdtSet(self):
-        """Get the set of device trees used by this entry
+    def GetFdts(self):
+        """Get the device trees used by this entry
 
         Returns:
-            Set containing the filename from this entry, if it is a .dtb, else
-            an empty set
+            Empty dict, if this entry is not a .dtb, otherwise:
+            Dict:
+                key: Filename from this entry (without the path)
+                value: Fdt object for this dtb, or None if not available
         """
-        return set()
+        return {}
 
     def ExpandEntries(self):
         pass
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index b242c2da38a..b70c3d3a3a2 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -32,11 +32,12 @@ class Entry_blob_dtb(Entry_blob):
         data = self.CompressData(indata)
         return self.ProcessContentsUpdate(data)
 
-    def GetFdtSet(self):
-        """Get the set of device trees used by this entry
+    def GetFdts(self):
+        """Get the device trees used by this entry
 
         Returns:
-            Set containing the filename from this entry
+            Dict:
+                key: Filename from this entry (without the path)
+                value: Fdt object for this dtb, or None if not available
         """
-        fname = self.GetDefaultFilename()
-        return set([fname])
+        return {self.GetDefaultFilename(): None}
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 6db3c7a6f03..cdd8618c48b 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -95,11 +95,11 @@ class Entry_section(Entry):
             entry.SetPrefix(self._name_prefix)
             self._entries[node.name] = entry
 
-    def GetFdtSet(self):
-        fdt_set = set()
+    def GetFdts(self):
+        fdts = {}
         for entry in self._entries.values():
-            fdt_set.update(entry.GetFdtSet())
-        return fdt_set
+            fdts.update(entry.GetFdts())
+        return fdts
 
     def ProcessFdt(self, fdt):
         """Allow entries to adjust the device tree
diff --git a/tools/binman/state.py b/tools/binman/state.py
index 382bda32219..5b9e005df96 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -22,11 +22,8 @@ entry_args = {}
 # ftest.py)
 use_fake_dtb = False
 
-# Set of all device tree files references by images
-fdt_set = set()
-
-# Same as above, but excluding the main one
-fdt_subset = set()
+# Dict of device trees, keyed by filename, but excluding the main one
+fdt_subset = {}
 
 # The DTB which contains the full image information
 main_dtb = None
@@ -140,11 +137,12 @@ def Prepare(images, dtb):
     main_dtb = dtb
     fdt_files.clear()
     fdt_files['u-boot.dtb'] = dtb
-    fdt_subset = set()
+    fdt_subset = {}
     if not use_fake_dtb:
         for image in images.values():
-            fdt_subset.update(image.GetFdtSet())
-        fdt_subset.discard('u-boot.dtb')
+            fdt_subset.update(image.GetFdts())
+        if 'u-boot.dtb' in fdt_subset:
+            del fdt_subset['u-boot.dtb']
         for other_fname in fdt_subset:
             infile = tools.GetInputFilename(other_fname)
             other_fname_dtb = fdt_util.EnsureCompiled(infile)
-- 
cgit v1.2.3


From 4bdd1159805d99888ca60aa63efefbbd307f0cf2 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:29 -0600
Subject: binman: Rename state.GetFdts()

This function name conflicts with Entry.GetFdts() which has a different
purpose. Rename it to avoid confusion. Also update a stale comment
relating to this function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/control.py | 6 +++---
 tools/binman/state.py   | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index de9f29e2246..8700f48ad55 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -231,7 +231,7 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt):
             image.AddMissingProperties()
         image.ProcessFdt(dtb)
 
-    for dtb_item in state.GetFdts():
+    for dtb_item in state.GetAllFdts():
         dtb_item.Sync(auto_resize=True)
         dtb_item.Pack()
         dtb_item.Flush()
@@ -278,7 +278,7 @@ def ProcessImage(image, update_fdt, write_map):
         image.SetImagePos()
         if update_fdt:
             image.SetCalculatedProperties()
-            for dtb_item in state.GetFdts():
+            for dtb_item in state.GetAllFdts():
                 dtb_item.Sync()
         sizes_ok = image.ProcessEntryContents()
         if sizes_ok:
@@ -355,7 +355,7 @@ def Binman(args):
                 ProcessImage(image, args.update_fdt, args.map)
 
             # Write the updated FDTs to our output files
-            for dtb_item in state.GetFdts():
+            for dtb_item in state.GetAllFdts():
                 tools.WriteFile(dtb_item._fname, dtb_item.GetContents())
 
         finally:
diff --git a/tools/binman/state.py b/tools/binman/state.py
index 5b9e005df96..77c7024f5a7 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -117,8 +117,8 @@ def GetEntryArg(name):
 def Prepare(images, dtb):
     """Get device tree files ready for use
 
-    This sets up a set of device tree files that can be retrieved by GetFdts().
-    At present there is only one, that for U-Boot proper.
+    This sets up a set of device tree files that can be retrieved by
+    GetAllFdts(). This includes U-Boot proper and any SPL device trees.
 
     Args:
         images: List of images being used
@@ -152,7 +152,7 @@ def Prepare(images, dtb):
             other_dtb = fdt.FdtScan(out_fname)
             fdt_files[other_fname] = other_dtb
 
-def GetFdts():
+def GetAllFdts():
     """Yield all device tree files being used by binman
 
     Yields:
-- 
cgit v1.2.3


From 726e2961291dec2c46d773866c5923210c3bac0f Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:30 -0600
Subject: binman: Rename state.GetFdt()

This function name conflicts with Fdt.Node.GetFdt() which has a different
purpose. Rename it to avoid confusion.

The new name suggests it is indexed by entry type rather than filename.
This will be tidied up in a future commit.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/etype/u_boot_dtb_with_ucode.py | 2 +-
 tools/binman/state.py                       | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py
index 188888e022b..9224004efeb 100644
--- a/tools/binman/etype/u_boot_dtb_with_ucode.py
+++ b/tools/binman/etype/u_boot_dtb_with_ucode.py
@@ -54,7 +54,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb):
 
         # Remove the microcode
         fname = self.GetDefaultFilename()
-        fdt = state.GetFdt(fname)
+        fdt = state.GetFdtForEtype(fname)
         self.ucode = fdt.GetNode('/microcode')
         if not self.ucode:
             raise self.Raise("No /microcode node found in '%s'" % fname)
diff --git a/tools/binman/state.py b/tools/binman/state.py
index 77c7024f5a7..c8175a30334 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -33,7 +33,7 @@ main_dtb = None
 # Entry.ProcessContentsUpdate()
 allow_entry_expansion = True
 
-def GetFdt(fname):
+def GetFdtForEtype(fname):
     """Get the Fdt object for a particular device-tree filename
 
     Binman keeps track of at least one device-tree file called u-boot.dtb but
@@ -51,7 +51,8 @@ def GetFdt(fname):
 def GetFdtPath(fname):
     """Get the full pathname of a particular Fdt object
 
-    Similar to GetFdt() but returns the pathname associated with the Fdt.
+    Similar to GetFdtForEtype() but returns the pathname associated with the
+    Fdt.
 
     Args:
         fname: Filename to look up (e.g. 'u-boot.dtb').
@@ -78,7 +79,7 @@ def GetFdtContents(fname='u-boot.dtb'):
     """
     if fname in fdt_files and not use_fake_dtb:
         pathname = GetFdtPath(fname)
-        data = GetFdt(fname).GetContents()
+        data = GetFdtForEtype(fname).GetContents()
     else:
         pathname = tools.GetInputFilename(fname)
         data = tools.ReadFile(pathname)
-- 
cgit v1.2.3


From 4bdd30055ca3a9096f462177758b97e55c15f1e7 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:31 -0600
Subject: binman: Adjust GetFdt() to be keyed by etype

At present the FDTs are keyed by their default filename (not their actual
filename). It seems easier to key by the entry type, since this is always
the same for each FDT type.

To do this, add a new Entry method called GetFdtEtype(). This is necessary
since some entry types contain a device tree which are not the simple
three entry types 'u-boot-dtb', 'u-boot-spl' or 'u-boot-tpl'.

The code already returns a dict for GetFdt(). Update the value of that
dict to include the filename so that existing code can work.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/entry.py                           |  4 +++-
 tools/binman/entry_test.py                      |  9 +++++++++
 tools/binman/etype/blob_dtb.py                  | 16 ++++++++++++++--
 tools/binman/etype/u_boot_dtb.py                |  3 +++
 tools/binman/etype/u_boot_dtb_with_ucode.py     |  3 +++
 tools/binman/etype/u_boot_spl_dtb.py            |  3 +++
 tools/binman/etype/u_boot_tpl_dtb.py            |  3 +++
 tools/binman/etype/u_boot_tpl_dtb_with_ucode.py |  3 +++
 tools/binman/state.py                           | 14 +++++++++-----
 9 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 2ed9dc0d6f4..dd2daadf16f 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -192,7 +192,9 @@ class Entry(object):
             Empty dict, if this entry is not a .dtb, otherwise:
             Dict:
                 key: Filename from this entry (without the path)
-                value: Fdt object for this dtb, or None if not available
+                value: Tuple:
+                    Fdt object for this dtb, or None if not available
+                    Filename of file containing this dtb
         """
         return {}
 
diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py
index b6ad3edb8dc..460171ba899 100644
--- a/tools/binman/entry_test.py
+++ b/tools/binman/entry_test.py
@@ -84,5 +84,14 @@ class TestEntry(unittest.TestCase):
         base_entry = entry.Entry(None, None, None, read_node=False)
         self.assertIsNone(base_entry.GetDefaultFilename())
 
+    def testBlobFdt(self):
+        """Test the GetFdtEtype() method of the blob-dtb entries"""
+        base = entry.Entry.Create(None, self.GetNode(), 'blob-dtb')
+        self.assertIsNone(base.GetFdtEtype())
+
+        dtb = entry.Entry.Create(None, self.GetNode(), 'u-boot-dtb')
+        self.assertEqual('u-boot-dtb', dtb.GetFdtEtype())
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index b70c3d3a3a2..b9ccf9a9540 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -32,12 +32,24 @@ class Entry_blob_dtb(Entry_blob):
         data = self.CompressData(indata)
         return self.ProcessContentsUpdate(data)
 
+    def GetFdtEtype(self):
+        """Get the entry type of this device tree
+
+        This can be 'u-boot-dtb', 'u-boot-spl-dtb' or 'u-boot-tpl-dtb'
+        Returns:
+            Entry type if any, e.g. 'u-boot-dtb'
+        """
+        return None
+
     def GetFdts(self):
         """Get the device trees used by this entry
 
         Returns:
             Dict:
                 key: Filename from this entry (without the path)
-                value: Fdt object for this dtb, or None if not available
+                value: Tuple:
+                    Fdt object for this dtb, or None if not available
+                    Filename of file containing this dtb
         """
-        return {self.GetDefaultFilename(): None}
+        fname = self.GetDefaultFilename()
+        return {self.GetFdtEtype(): [self, fname]}
diff --git a/tools/binman/etype/u_boot_dtb.py b/tools/binman/etype/u_boot_dtb.py
index 6263c4ebee3..6c805a666da 100644
--- a/tools/binman/etype/u_boot_dtb.py
+++ b/tools/binman/etype/u_boot_dtb.py
@@ -26,3 +26,6 @@ class Entry_u_boot_dtb(Entry_blob_dtb):
 
     def GetDefaultFilename(self):
         return 'u-boot.dtb'
+
+    def GetFdtEtype(self):
+        return 'u-boot-dtb'
diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py
index 9224004efeb..ff7f80421a3 100644
--- a/tools/binman/etype/u_boot_dtb_with_ucode.py
+++ b/tools/binman/etype/u_boot_dtb_with_ucode.py
@@ -36,6 +36,9 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb):
     def GetDefaultFilename(self):
         return 'u-boot.dtb'
 
+    def GetFdtEtype(self):
+        return 'u-boot-dtb'
+
     def ProcessFdt(self, fdt):
         # So the module can be loaded without it
         import fdt
diff --git a/tools/binman/etype/u_boot_spl_dtb.py b/tools/binman/etype/u_boot_spl_dtb.py
index e7354646f13..1bcd449bf3a 100644
--- a/tools/binman/etype/u_boot_spl_dtb.py
+++ b/tools/binman/etype/u_boot_spl_dtb.py
@@ -23,3 +23,6 @@ class Entry_u_boot_spl_dtb(Entry_blob_dtb):
 
     def GetDefaultFilename(self):
         return 'spl/u-boot-spl.dtb'
+
+    def GetFdtEtype(self):
+        return 'u-boot-spl-dtb'
diff --git a/tools/binman/etype/u_boot_tpl_dtb.py b/tools/binman/etype/u_boot_tpl_dtb.py
index bdeb0f75a24..81a39704598 100644
--- a/tools/binman/etype/u_boot_tpl_dtb.py
+++ b/tools/binman/etype/u_boot_tpl_dtb.py
@@ -23,3 +23,6 @@ class Entry_u_boot_tpl_dtb(Entry_blob_dtb):
 
     def GetDefaultFilename(self):
         return 'tpl/u-boot-tpl.dtb'
+
+    def GetFdtEtype(self):
+        return 'u-boot-tpl-dtb'
diff --git a/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py b/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py
index 71e04fcd44f..ce19a49e2e6 100644
--- a/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py
+++ b/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py
@@ -23,3 +23,6 @@ class Entry_u_boot_tpl_dtb_with_ucode(Entry_u_boot_dtb_with_ucode):
 
     def GetDefaultFilename(self):
         return 'tpl/u-boot-tpl.dtb'
+
+    def GetFdtEtype(self):
+        return 'u-boot-tpl-dtb'
diff --git a/tools/binman/state.py b/tools/binman/state.py
index c8175a30334..ee11ba470e0 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -22,7 +22,10 @@ entry_args = {}
 # ftest.py)
 use_fake_dtb = False
 
-# Dict of device trees, keyed by filename, but excluding the main one
+# Dict of device trees, keyed by entry type, but excluding the main one
+# The value is as returned by Entry.GetFdts(), i.e. a tuple:
+#     Fdt object for this dtb, or None if not available
+#     Filename of file containing this dtb
 fdt_subset = {}
 
 # The DTB which contains the full image information
@@ -142,9 +145,10 @@ def Prepare(images, dtb):
     if not use_fake_dtb:
         for image in images.values():
             fdt_subset.update(image.GetFdts())
-        if 'u-boot.dtb' in fdt_subset:
-            del fdt_subset['u-boot.dtb']
-        for other_fname in fdt_subset:
+        if 'u-boot-dtb' in fdt_subset:
+            del fdt_subset['u-boot-dtb']
+        for etype, other in fdt_subset.items():
+            _, other_fname = other
             infile = tools.GetInputFilename(other_fname)
             other_fname_dtb = fdt_util.EnsureCompiled(infile)
             out_fname = tools.GetOutputFilename('%s.out' %
@@ -160,7 +164,7 @@ def GetAllFdts():
         Device trees being used (U-Boot proper, SPL, TPL)
     """
     yield main_dtb
-    for other_fname in fdt_subset:
+    for etype, other_fname in fdt_subset.values():
         yield fdt_files[other_fname]
 
 def GetUpdateNodes(node):
-- 
cgit v1.2.3


From fb5e8b163e2332297cb2c0821bc2e24ef4818816 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:32 -0600
Subject: binman: Adjust state.fdt_files to be keyed by entry type

It makes more sense to use entry type as the key for this dictionary,
since the filename can in principle be anything. Make this change and also
rename fdt_files and add a comment to explain it better.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/etype/blob_dtb.py              |  4 +-
 tools/binman/etype/u_boot_dtb_with_ucode.py |  6 +--
 tools/binman/state.py                       | 57 +++++++++++++++++------------
 3 files changed, 38 insertions(+), 29 deletions(-)

diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index b9ccf9a9540..a3b548eef20 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -23,12 +23,12 @@ class Entry_blob_dtb(Entry_blob):
     def ObtainContents(self):
         """Get the device-tree from the list held by the 'state' module"""
         self._filename = self.GetDefaultFilename()
-        self._pathname, _ = state.GetFdtContents(self._filename)
+        self._pathname, _ = state.GetFdtContents(self.GetFdtEtype())
         return Entry_blob.ReadBlobContents(self)
 
     def ProcessContents(self):
         """Re-read the DTB contents so that we get any calculated properties"""
-        _, indata = state.GetFdtContents(self._filename)
+        _, indata = state.GetFdtContents(self.GetFdtEtype())
         data = self.CompressData(indata)
         return self.ProcessContentsUpdate(data)
 
diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py
index ff7f80421a3..cb6c3730d79 100644
--- a/tools/binman/etype/u_boot_dtb_with_ucode.py
+++ b/tools/binman/etype/u_boot_dtb_with_ucode.py
@@ -56,11 +56,11 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb):
             return True
 
         # Remove the microcode
-        fname = self.GetDefaultFilename()
-        fdt = state.GetFdtForEtype(fname)
+        etype = self.GetFdtEtype()
+        fdt = state.GetFdtForEtype(etype)
         self.ucode = fdt.GetNode('/microcode')
         if not self.ucode:
-            raise self.Raise("No /microcode node found in '%s'" % fname)
+            raise self.Raise("No /microcode node found in '%s'" % etype)
 
         # There's no need to collate it (move all microcode into one place)
         # if we only have one chunk of microcode.
diff --git a/tools/binman/state.py b/tools/binman/state.py
index ee11ba470e0..0278f87df22 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -11,9 +11,15 @@ import re
 import os
 import tools
 
-# Records the device-tree files known to binman, keyed by filename (e.g.
-# 'u-boot-spl.dtb')
-fdt_files = {}
+# Records the device-tree files known to binman, keyed by entry type (e.g.
+# 'u-boot-spl-dtb'). These are the output FDT files, which can be updated by
+# binman. They have been copied to <xxx>.out files.
+#
+#   key: entry type
+#   value: tuple:
+#       Fdt object
+#       Filename
+output_fdt_files = {}
 
 # Arguments passed to binman to provide arguments to entries
 entry_args = {}
@@ -36,36 +42,36 @@ main_dtb = None
 # Entry.ProcessContentsUpdate()
 allow_entry_expansion = True
 
-def GetFdtForEtype(fname):
-    """Get the Fdt object for a particular device-tree filename
+def GetFdtForEtype(etype):
+    """Get the Fdt object for a particular device-tree entry
 
     Binman keeps track of at least one device-tree file called u-boot.dtb but
     can also have others (e.g. for SPL). This function looks up the given
-    filename and returns the associated Fdt object.
+    entry and returns the associated Fdt object.
 
     Args:
-        fname: Filename to look up (e.g. 'u-boot.dtb').
+        etype: Entry type of device tree (e.g. 'u-boot-dtb')
 
     Returns:
-        Fdt object associated with the filename
+        Fdt object associated with the entry type
     """
-    return fdt_files[fname]
+    return output_fdt_files[etype][0]
 
-def GetFdtPath(fname):
+def GetFdtPath(etype):
     """Get the full pathname of a particular Fdt object
 
     Similar to GetFdtForEtype() but returns the pathname associated with the
     Fdt.
 
     Args:
-        fname: Filename to look up (e.g. 'u-boot.dtb').
+        etype: Entry type of device tree (e.g. 'u-boot-dtb')
 
     Returns:
         Full path name to the associated Fdt
     """
-    return fdt_files[fname]._fname
+    return output_fdt_files[etype][0]._fname
 
-def GetFdtContents(fname='u-boot.dtb'):
+def GetFdtContents(etype='u-boot-dtb'):
     """Looks up the FDT pathname and contents
 
     This is used to obtain the Fdt pathname and contents when needed by an
@@ -73,17 +79,18 @@ def GetFdtContents(fname='u-boot.dtb'):
     the real dtb.
 
     Args:
-        fname: Filename to look up (e.g. 'u-boot.dtb').
+        etype: Entry type to look up (e.g. 'u-boot.dtb').
 
     Returns:
         tuple:
             pathname to Fdt
             Fdt data (as bytes)
     """
-    if fname in fdt_files and not use_fake_dtb:
-        pathname = GetFdtPath(fname)
-        data = GetFdtForEtype(fname).GetContents()
+    if etype in output_fdt_files and not use_fake_dtb:
+        pathname = GetFdtPath(etype)
+        data = GetFdtForEtype(etype).GetContents()
     else:
+        fname = output_fdt_files[etype][1]
         pathname = tools.GetInputFilename(fname)
         data = tools.ReadFile(pathname)
     return pathname, data
@@ -128,7 +135,7 @@ def Prepare(images, dtb):
         images: List of images being used
         dtb: Main dtb
     """
-    global fdt_set, fdt_subset, fdt_files, main_dtb
+    global fdt_set, fdt_subset, output_fdt_files, main_dtb
     # Import these here in case libfdt.py is not available, in which case
     # the above help option still works.
     import fdt
@@ -139,8 +146,10 @@ def Prepare(images, dtb):
     # since it is assumed to be the one passed in with options.dt, and
     # was handled just above.
     main_dtb = dtb
-    fdt_files.clear()
-    fdt_files['u-boot.dtb'] = dtb
+    output_fdt_files.clear()
+    output_fdt_files['u-boot-dtb'] = [dtb, 'u-boot.dtb']
+    output_fdt_files['u-boot-spl-dtb'] = [dtb, 'spl/u-boot-spl.dtb']
+    output_fdt_files['u-boot-tpl-dtb'] = [dtb, 'tpl/u-boot-tpl.dtb']
     fdt_subset = {}
     if not use_fake_dtb:
         for image in images.values():
@@ -155,7 +164,7 @@ def Prepare(images, dtb):
                     os.path.split(other_fname)[1])
             tools.WriteFile(out_fname, tools.ReadFile(other_fname_dtb))
             other_dtb = fdt.FdtScan(out_fname)
-            fdt_files[other_fname] = other_dtb
+            output_fdt_files[etype] = [other_dtb, other_fname]
 
 def GetAllFdts():
     """Yield all device tree files being used by binman
@@ -164,8 +173,8 @@ def GetAllFdts():
         Device trees being used (U-Boot proper, SPL, TPL)
     """
     yield main_dtb
-    for etype, other_fname in fdt_subset.values():
-        yield fdt_files[other_fname]
+    for etype in fdt_subset:
+        yield output_fdt_files[etype][0]
 
 def GetUpdateNodes(node):
     """Yield all the nodes that need to be updated in all device trees
@@ -182,7 +191,7 @@ def GetUpdateNodes(node):
             is node, SPL and TPL)
     """
     yield node
-    for dtb in fdt_files.values():
+    for dtb, fname in output_fdt_files.values():
         if dtb != node.GetFdt():
             other_node = dtb.GetNode(node.path)
             if other_node:
-- 
cgit v1.2.3


From 77e4ef1bf48eed22efe42f007d60c90b2035e101 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:33 -0600
Subject: binman: Simplify state.fdt_subset

At present this excludes the device tree passed in to binman since it
is always returned first by GetAllFdts(). However, this is easy to ensure
by adding a check in that function. Change this dict to includes all
device trees, and rename it to fdt_set.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/state.py | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/tools/binman/state.py b/tools/binman/state.py
index 0278f87df22..46c1c8d613a 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -28,11 +28,12 @@ entry_args = {}
 # ftest.py)
 use_fake_dtb = False
 
-# Dict of device trees, keyed by entry type, but excluding the main one
+# Dict of device trees, keyed by entry type. These are the input device trees,
+# before any modification by U-Boot
 # The value is as returned by Entry.GetFdts(), i.e. a tuple:
 #     Fdt object for this dtb, or None if not available
 #     Filename of file containing this dtb
-fdt_subset = {}
+fdt_set = {}
 
 # The DTB which contains the full image information
 main_dtb = None
@@ -135,7 +136,7 @@ def Prepare(images, dtb):
         images: List of images being used
         dtb: Main dtb
     """
-    global fdt_set, fdt_subset, output_fdt_files, main_dtb
+    global fdt_set, output_fdt_files, main_dtb
     # Import these here in case libfdt.py is not available, in which case
     # the above help option still works.
     import fdt
@@ -150,13 +151,11 @@ def Prepare(images, dtb):
     output_fdt_files['u-boot-dtb'] = [dtb, 'u-boot.dtb']
     output_fdt_files['u-boot-spl-dtb'] = [dtb, 'spl/u-boot-spl.dtb']
     output_fdt_files['u-boot-tpl-dtb'] = [dtb, 'tpl/u-boot-tpl.dtb']
-    fdt_subset = {}
+    fdt_set = {}
     if not use_fake_dtb:
         for image in images.values():
-            fdt_subset.update(image.GetFdts())
-        if 'u-boot-dtb' in fdt_subset:
-            del fdt_subset['u-boot-dtb']
-        for etype, other in fdt_subset.items():
+            fdt_set.update(image.GetFdts())
+        for etype, other in fdt_set.items():
             _, other_fname = other
             infile = tools.GetInputFilename(other_fname)
             other_fname_dtb = fdt_util.EnsureCompiled(infile)
@@ -173,8 +172,10 @@ def GetAllFdts():
         Device trees being used (U-Boot proper, SPL, TPL)
     """
     yield main_dtb
-    for etype in fdt_subset:
-        yield output_fdt_files[etype][0]
+    for etype in fdt_set:
+        dtb = output_fdt_files[etype][0]
+        if dtb != main_dtb:
+            yield dtb
 
 def GetUpdateNodes(node):
     """Yield all the nodes that need to be updated in all device trees
-- 
cgit v1.2.3


From f49462e547495aa314795f77904ee8ca389b3d40 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:34 -0600
Subject: binman: Drop state.fdt_set as this is not needed

We can iterate through the output files so don't need this global anymore.
Remove it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/state.py | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/tools/binman/state.py b/tools/binman/state.py
index 46c1c8d613a..7c3a987723e 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -28,13 +28,6 @@ entry_args = {}
 # ftest.py)
 use_fake_dtb = False
 
-# Dict of device trees, keyed by entry type. These are the input device trees,
-# before any modification by U-Boot
-# The value is as returned by Entry.GetFdts(), i.e. a tuple:
-#     Fdt object for this dtb, or None if not available
-#     Filename of file containing this dtb
-fdt_set = {}
-
 # The DTB which contains the full image information
 main_dtb = None
 
@@ -136,7 +129,7 @@ def Prepare(images, dtb):
         images: List of images being used
         dtb: Main dtb
     """
-    global fdt_set, output_fdt_files, main_dtb
+    global output_fdt_files, main_dtb
     # Import these here in case libfdt.py is not available, in which case
     # the above help option still works.
     import fdt
@@ -151,8 +144,8 @@ def Prepare(images, dtb):
     output_fdt_files['u-boot-dtb'] = [dtb, 'u-boot.dtb']
     output_fdt_files['u-boot-spl-dtb'] = [dtb, 'spl/u-boot-spl.dtb']
     output_fdt_files['u-boot-tpl-dtb'] = [dtb, 'tpl/u-boot-tpl.dtb']
-    fdt_set = {}
     if not use_fake_dtb:
+        fdt_set = {}
         for image in images.values():
             fdt_set.update(image.GetFdts())
         for etype, other in fdt_set.items():
@@ -172,7 +165,7 @@ def GetAllFdts():
         Device trees being used (U-Boot proper, SPL, TPL)
     """
     yield main_dtb
-    for etype in fdt_set:
+    for etype in output_fdt_files:
         dtb = output_fdt_files[etype][0]
         if dtb != main_dtb:
             yield dtb
-- 
cgit v1.2.3


From fd07336211c3b3088dce20b1c22a99e6303c68c2 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:35 -0600
Subject: patman: Update tout to avoid open-coding the debug levels

Use the debug level constants instead of open-coding them in the file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/patman/tout.py | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/tools/patman/tout.py b/tools/patman/tout.py
index ae04c30f1db..2a384851b0d 100644
--- a/tools/patman/tout.py
+++ b/tools/patman/tout.py
@@ -11,11 +11,7 @@ import sys
 import terminal
 
 # Output verbosity levels that we support
-ERROR = 0
-WARNING = 1
-NOTICE = 2
-INFO = 3
-DEBUG = 4
+ERROR, WARNING, NOTICE, INFO, DETAIL, DEBUG = range(6)
 
 in_progress = False
 
@@ -107,7 +103,7 @@ def Error(msg):
     Args:
         msg; Message to display.
     """
-    _Output(0, msg, _color.RED)
+    _Output(ERROR, msg, _color.RED)
 
 def Warning(msg):
     """Display a warning message
@@ -115,7 +111,7 @@ def Warning(msg):
     Args:
         msg; Message to display.
     """
-    _Output(1, msg, _color.YELLOW)
+    _Output(WARNING, msg, _color.YELLOW)
 
 def Notice(msg):
     """Display an important infomation message
@@ -123,7 +119,7 @@ def Notice(msg):
     Args:
         msg; Message to display.
     """
-    _Output(2, msg)
+    _Output(NOTICE, msg)
 
 def Info(msg):
     """Display an infomation message
@@ -131,7 +127,7 @@ def Info(msg):
     Args:
         msg; Message to display.
     """
-    _Output(3, msg)
+    _Output(INFO, msg)
 
 def Detail(msg):
     """Display a detailed message
@@ -139,7 +135,7 @@ def Detail(msg):
     Args:
         msg; Message to display.
     """
-    _Output(4, msg)
+    _Output(DETAIL, msg)
 
 def Debug(msg):
     """Display a debug message
@@ -147,7 +143,7 @@ def Debug(msg):
     Args:
         msg; Message to display.
     """
-    _Output(5, msg)
+    _Output(DEBUG, msg)
 
 def UserOutput(msg):
     """Display a message regardless of the current output level.
-- 
cgit v1.2.3


From 9f297b09c06f2212cb59dac5950ae61de5fe3a9f Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:36 -0600
Subject: binman: Add a bit of logging in entries when packing

Use the new logging feature to log information about progress with
packing. This is useful to see how binman is figuring things out.

Also update elf.py to use the same feature.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/elf.py           |  9 +++------
 tools/binman/elf_test.py      | 19 +++++++++++--------
 tools/binman/entry.py         | 20 ++++++++++++++++++--
 tools/binman/etype/fdtmap.py  |  3 +++
 tools/binman/etype/section.py |  2 ++
 tools/patman/tools.py         | 16 ++++++++++++++++
 6 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/tools/binman/elf.py b/tools/binman/elf.py
index 8147b3437dd..af40024ceab 100644
--- a/tools/binman/elf.py
+++ b/tools/binman/elf.py
@@ -17,6 +17,7 @@ import struct
 import tempfile
 
 import tools
+import tout
 
 ELF_TOOLS = True
 try:
@@ -25,9 +26,6 @@ try:
 except:  # pragma: no cover
     ELF_TOOLS = False
 
-# This is enabled from control.py
-debug = False
-
 Symbol = namedtuple('Symbol', ['section', 'address', 'size', 'weak'])
 
 # Information about an ELF file:
@@ -143,9 +141,8 @@ def LookupAndWriteSymbols(elf_fname, entry, section):
                 value = -1
                 pack_string = pack_string.lower()
             value_bytes = struct.pack(pack_string, value)
-            if debug:
-                print('%s:\n   insert %s, offset %x, value %x, length %d' %
-                      (msg, name, offset, value, len(value_bytes)))
+            tout.Debug('%s:\n   insert %s, offset %x, value %x, length %d' %
+                       (msg, name, offset, value, len(value_bytes)))
             entry.data = (entry.data[:offset] + value_bytes +
                         entry.data[offset + sym.size:])
 
diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
index e2506377f26..416e43baf0e 100644
--- a/tools/binman/elf_test.py
+++ b/tools/binman/elf_test.py
@@ -14,6 +14,7 @@ import command
 import elf
 import test_util
 import tools
+import tout
 
 binman_dir = os.path.dirname(os.path.realpath(sys.argv[0]))
 
@@ -130,14 +131,16 @@ class TestElf(unittest.TestCase):
 
     def testDebug(self):
         """Check that enabling debug in the elf module produced debug output"""
-        elf.debug = True
-        entry = FakeEntry(20)
-        section = FakeSection()
-        elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms')
-        with test_util.capture_sys_output() as (stdout, stderr):
-            syms = elf.LookupAndWriteSymbols(elf_fname, entry, section)
-        elf.debug = False
-        self.assertTrue(len(stdout.getvalue()) > 0)
+        try:
+            tout.Init(tout.DEBUG)
+            entry = FakeEntry(20)
+            section = FakeSection()
+            elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms')
+            with test_util.capture_sys_output() as (stdout, stderr):
+                syms = elf.LookupAndWriteSymbols(elf_fname, entry, section)
+            self.assertTrue(len(stdout.getvalue()) > 0)
+        finally:
+            tout.Init(tout.WARNING)
 
     def testMakeElf(self):
         """Test for the MakeElf function"""
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index dd2daadf16f..e3c64348225 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -23,6 +23,7 @@ import sys
 import fdt_util
 import state
 import tools
+from tools import ToHex, ToHexSize
 import tout
 
 modules = {}
@@ -272,8 +273,9 @@ class Entry(object):
         new_size = len(data)
         if state.AllowEntryExpansion():
             if new_size > self.contents_size:
-                tout.Debug("Entry '%s' size change from %#x to %#x" % (
-                    self._node.path, self.contents_size, new_size))
+                tout.Debug("Entry '%s' size change from %s to %s" % (
+                    self._node.path, ToHex(self.contents_size),
+                    ToHex(new_size)))
                 # self.data will indicate the new size needed
                 size_ok = False
         elif new_size != self.contents_size:
@@ -294,6 +296,9 @@ class Entry(object):
 
     def ResetForPack(self):
         """Reset offset/size fields so that packing can be done again"""
+        self.Detail('ResetForPack: offset %s->%s, size %s->%s' %
+                    (ToHex(self.offset), ToHex(self.orig_offset),
+                     ToHex(self.size), ToHex(self.orig_size)))
         self.offset = self.orig_offset
         self.size = self.orig_size
 
@@ -315,6 +320,9 @@ class Entry(object):
         Returns:
             New section offset pointer (after this entry)
         """
+        self.Detail('Packing: offset=%s, size=%s, content_size=%x' %
+                    (ToHex(self.offset), ToHex(self.size),
+                     self.contents_size))
         if self.offset is None:
             if self.offset_unset:
                 self.Raise('No offset set with offset-unset: should another '
@@ -346,6 +354,8 @@ class Entry(object):
         if self.offset != tools.Align(self.offset, self.align):
             self.Raise("Offset %#x (%d) does not match align %#x (%d)" %
                   (self.offset, self.offset, self.align, self.align))
+        self.Detail('   - packed: offset=%#x, size=%#x, content_size=%#x, next_offset=%x' %
+                    (self.offset, self.size, self.contents_size, new_offset))
 
         return new_offset
 
@@ -353,6 +363,11 @@ class Entry(object):
         """Convenience function to raise an error referencing a node"""
         raise ValueError("Node '%s': %s" % (self._node.path, msg))
 
+    def Detail(self, msg):
+        """Convenience function to log detail referencing a node"""
+        tag = "Node '%s'" % self._node.path
+        tout.Detail('%30s: %s' % (tag, msg))
+
     def GetEntryArgsOrProps(self, props, required=False):
         """Return the values of a set of properties
 
@@ -389,6 +404,7 @@ class Entry(object):
         return self._node.path
 
     def GetData(self):
+        self.Detail('GetData: size %s' % ToHexSize(self.data))
         return self.data
 
     def GetOffsets(self):
diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
index ddb9738e5cb..229b4a1bb69 100644
--- a/tools/binman/etype/fdtmap.py
+++ b/tools/binman/etype/fdtmap.py
@@ -14,6 +14,7 @@ from entry import Entry
 from fdt import Fdt
 import state
 import tools
+import tout
 
 FDTMAP_MAGIC   = b'_FDTMAP_'
 FDTMAP_HDR_LEN = 16
@@ -98,6 +99,8 @@ class Entry_fdtmap(Entry):
 
         # Find the node for the image containing the Fdt-map entry
         path = self.section.GetPath()
+        self.Detail("Fdtmap: Using section '%s' (path '%s')" %
+                    (self.section.name, path))
         node = infdt.GetNode(path)
         if not node:
             self.Raise("Internal error: Cannot locate node for path '%s'" %
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index cdd8618c48b..f29784c1bbf 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -149,6 +149,8 @@ class Entry_section(Entry):
             base = self.pad_before + entry.offset - self._skip_at_start
             section_data = (section_data[:base] + data +
                             section_data[base + len(data):])
+        self.Detail('GetData: %d entries, total size %#x' %
+                    (len(self._entries), len(section_data)))
         return section_data
 
     def GetOffsets(self):
diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index e945b54fa28..f492dc8f8e3 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -473,3 +473,19 @@ def RunIfwiTool(ifwi_file, cmd, fname=None, subpart=None, entry_name=None):
     if entry_name:
         args += ['-d', '-e', entry_name]
     Run(*args)
+
+def ToHex(val):
+    """Convert an integer value (or None) to a string
+
+    Returns:
+        hex value, or 'None' if the value is None
+    """
+    return 'None' if val is None else '%#x' % val
+
+def ToHexSize(val):
+    """Return the size of an object in hex
+
+    Returns:
+        hex value of size, or 'None' if the value is None
+    """
+    return 'None' if val is None else '%#x' % len(val)
-- 
cgit v1.2.3


From d9dad10e3c656d930041d8ec8db853d7fafab755 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:37 -0600
Subject: binman: Show a helpful error when a DT property is missing

At present a Python exception is raised which does not show the node
information. Add a more helpful exception in this case.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/dtoc/fdt.py      | 23 ++++++++++++++++++++---
 tools/dtoc/test_fdt.py | 21 +++++++++++++++++++++
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index d9471c43819..3870eb1fa1b 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -362,6 +362,23 @@ class Node:
         value = tools.GetBytes(0, len)
         self.props[prop_name] = Prop(self, None, prop_name, value)
 
+    def _CheckProp(self, prop_name):
+        """Check if a property is present
+
+        Args:
+            prop_name: Name of property
+
+        Returns:
+            self
+
+        Raises:
+            ValueError if the property is missing
+        """
+        if prop_name not in self.props:
+            raise ValueError("Fdt '%s', node '%s': Missing property '%s'" %
+                             (self._fdt._fname, self.path, prop_name))
+        return self
+
     def SetInt(self, prop_name, val):
         """Update an integer property int the device tree.
 
@@ -374,7 +391,7 @@ class Node:
             prop_name: Name of property
             val: Value to set
         """
-        self.props[prop_name].SetInt(val)
+        self._CheckProp(prop_name).props[prop_name].SetInt(val)
 
     def SetData(self, prop_name, val):
         """Set the data value of a property
@@ -386,7 +403,7 @@ class Node:
             prop_name: Name of property to set
             val: Data value to set
         """
-        self.props[prop_name].SetData(val)
+        self._CheckProp(prop_name).props[prop_name].SetData(val)
 
     def SetString(self, prop_name, val):
         """Set the string value of a property
@@ -400,7 +417,7 @@ class Node:
         """
         if sys.version_info[0] >= 3:  # pragma: no cover
             val = bytes(val, 'utf-8')
-        self.props[prop_name].SetData(val + b'\0')
+        self._CheckProp(prop_name).props[prop_name].SetData(val + b'\0')
 
     def AddString(self, prop_name, val):
         """Add a new string property to a node
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index bf469dbd54c..c25248ca1f9 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -421,6 +421,27 @@ class TestProp(unittest.TestCase):
         self.dtb.Sync(auto_resize=True)
         self.assertTrue(dtb2.GetContents() != self.dtb.GetContents())
 
+    def testMissingSetInt(self):
+        """Test handling of a missing property with SetInt"""
+        with self.assertRaises(ValueError) as e:
+            self.node.SetInt('one', 1)
+        self.assertIn("node '/spl-test': Missing property 'one'",
+                      str(e.exception))
+
+    def testMissingSetData(self):
+        """Test handling of a missing property with SetData"""
+        with self.assertRaises(ValueError) as e:
+            self.node.SetData('one', b'data')
+        self.assertIn("node '/spl-test': Missing property 'one'",
+                      str(e.exception))
+
+    def testMissingSetString(self):
+        """Test handling of a missing property with SetString"""
+        with self.assertRaises(ValueError) as e:
+            self.node.SetString('one', 1)
+        self.assertIn("node '/spl-test': Missing property 'one'",
+                      str(e.exception))
+
 
 class TestFdtUtil(unittest.TestCase):
     """Tests for the fdt_util module
-- 
cgit v1.2.3


From 880e9ee650f829ed4772003633feb01ced94d227 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:38 -0600
Subject: dtoc: Update Fdt.FromData() to allow a name

It is confusing when something goes wrong with a device tree which was
created from data rather than a file, since there is no identifying
filename. Add an option to provide this. Use the filename as the name,
where available

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/dtoc/fdt.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index 3870eb1fa1b..b341ef3f83b 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -498,29 +498,35 @@ class Fdt:
     Properties:
       fname: Filename of fdt
       _root: Root of device tree (a Node object)
+      name: Helpful name for this Fdt for the user (useful when creating the
+        DT from data rather than a file)
     """
     def __init__(self, fname):
         self._fname = fname
         self._cached_offsets = False
         self.phandle_to_node = {}
+        self.name = ''
         if self._fname:
+            self.name = self._fname
             self._fname = fdt_util.EnsureCompiled(self._fname)
 
             with open(self._fname, 'rb') as fd:
                 self._fdt_obj = libfdt.Fdt(fd.read())
 
     @staticmethod
-    def FromData(data):
+    def FromData(data, name=''):
         """Create a new Fdt object from the given data
 
         Args:
             data: Device-tree data blob
+            name: Helpful name for this Fdt for the user
 
         Returns:
             Fdt object containing the data
         """
         fdt = Fdt(None)
         fdt._fdt_obj = libfdt.Fdt(bytes(data))
+        fdt.name = name
         return fdt
 
     def LookupPhandle(self, phandle):
-- 
cgit v1.2.3


From e44bc831e262c2ca7f307ad12074c2eb9adab615 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:39 -0600
Subject: dtoc: Update Fdt.GetNode() to handle the root node

This function currently fails if the root node is requested. Requesting
the root node is sometimes useful, so fix the bug.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/dtoc/fdt.py      | 2 ++
 tools/dtoc/test_fdt.py | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index b341ef3f83b..cd7673c7da0 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -574,6 +574,8 @@ class Fdt:
         parts = path.split('/')
         if len(parts) < 2:
             return None
+        if len(parts) == 2 and parts[1] == '':
+            return node
         for part in parts[1:]:
             node = node.FindNode(part)
             if not node:
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index c25248ca1f9..ed2d982a8fc 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -77,11 +77,16 @@ class TestFdt(unittest.TestCase):
         """Test the GetNode() method"""
         node = self.dtb.GetNode('/spl-test')
         self.assertTrue(isinstance(node, fdt.Node))
+
         node = self.dtb.GetNode('/i2c@0/pmic@9')
         self.assertTrue(isinstance(node, fdt.Node))
         self.assertEqual('pmic@9', node.name)
         self.assertIsNone(self.dtb.GetNode('/i2c@0/pmic@9/missing'))
 
+        node = self.dtb.GetNode('/')
+        self.assertTrue(isinstance(node, fdt.Node))
+        self.assertEqual(0, node.Offset())
+
     def testFlush(self):
         """Check that we can flush the device tree out to its file"""
         fname = self.dtb._fname
-- 
cgit v1.2.3


From 589d8f917e718f702142d1fdba51723d45237b44 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:40 -0600
Subject: binman: Store image fdtmap when loading from a file

This data provides all the information about the position and size of each
entry. Store it for later use when loading an image. It can be reused as
is if the image is modified without changing offsets/sizes.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/image.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/binman/image.py b/tools/binman/image.py
index fb6e591ca60..232e752258c 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -32,6 +32,9 @@ class Image(section.Entry_section):
 
     Attributes:
         filename: Output filename for image
+        image_node: Name of node containing the description for this image
+        fdtmap_dtb: Fdt object for the fdtmap when loading from a file
+        fdtmap_data: Contents of the fdtmap when loading from a file
 
     Args:
         test: True if this is being called from a test of Images. This this case
@@ -44,6 +47,8 @@ class Image(section.Entry_section):
         self.name = 'main-section'
         self.image_name = name
         self._filename = '%s.bin' % self.image_name
+        self.fdtmap_dtb = None
+        self.fdtmap_data = None
         if not test:
             filename = fdt_util.GetString(self._node, 'filename')
             if filename:
@@ -82,7 +87,11 @@ class Image(section.Entry_section):
         dtb.Scan()
 
         # Return an Image with the associated nodes
-        image = Image('image', dtb.GetRoot())
+        root = dtb.GetRoot()
+        image = Image('image', root)
+        image.image_node = fdt_util.GetString(root, 'image-node', 'image')
+        image.fdtmap_dtb = dtb
+        image.fdtmap_data = fdtmap_data
         image._data = data
         return image
 
-- 
cgit v1.2.3


From d5079330f588a1aeedc734197124543bbc4e2d3c Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:41 -0600
Subject: binman: Support loading entry data from a file

When modifying an image it is convenient to load the data from the file
into each entry so that it can be reprocessed. Add a new LoadData() method
to handle this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/entry.py         | 5 +++++
 tools/binman/etype/section.py | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index e3c64348225..6436384254d 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -695,3 +695,8 @@ features to produce new behaviours.
                   (self.GetPath(), self.offset, self.offset + self.size,
                    self.size, len(data)))
         return data[self.offset:self.offset + self.size]
+
+    def LoadData(self, decomp=True):
+        data = self.ReadData(decomp)
+        self.ProcessContentsUpdate(data)
+        self.Detail('Loaded data size %x' % len(data))
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index f29784c1bbf..cd623821a34 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -462,3 +462,8 @@ class Entry_section(Entry):
                            self.image_pos, None, self.offset, self)
         for entry in self._entries.values():
             entry.ListEntries(entries, indent + 1)
+
+    def LoadData(self, decomp=True):
+        for entry in self._entries.values():
+            entry.LoadData(decomp)
+        self.Detail('Loaded data')
-- 
cgit v1.2.3


From 6a3b5b54110980a42284beb05865436652113772 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:42 -0600
Subject: binman: Allow state functions to fail to return data

At present these state functions raise an exception if they cannot find
what is requested. But in some cases the information is optional (e.g. an
fdtmap in a coming patch) so it is better to return gracefully.

Update these two functions to return None when the data cannot be found.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/state.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/binman/state.py b/tools/binman/state.py
index 7c3a987723e..87674100665 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -49,7 +49,10 @@ def GetFdtForEtype(etype):
     Returns:
         Fdt object associated with the entry type
     """
-    return output_fdt_files[etype][0]
+    value = output_fdt_files.get(etype);
+    if not value:
+        return None
+    return value[0]
 
 def GetFdtPath(etype):
     """Get the full pathname of a particular Fdt object
@@ -80,7 +83,9 @@ def GetFdtContents(etype='u-boot-dtb'):
             pathname to Fdt
             Fdt data (as bytes)
     """
-    if etype in output_fdt_files and not use_fake_dtb:
+    if etype not in output_fdt_files:
+        return None, None
+    if not use_fake_dtb:
         pathname = GetFdtPath(etype)
         data = GetFdtForEtype(etype).GetContents()
     else:
-- 
cgit v1.2.3


From 6ca0dcba5eac47ca73d4a9e77d262e29057910b7 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:43 -0600
Subject: binman: Store the entry in output_fdt_files

In some cases we want to access the Entry object for a particular device
tree. This allows us to read its contents or update it. Add this
information to output_fdt_files and provide a function to read it.

Also rename output_fdt_files since its name is no-longer descriptive.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/state.py | 49 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/tools/binman/state.py b/tools/binman/state.py
index 87674100665..34907d9d43c 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -19,7 +19,8 @@ import tools
 #   value: tuple:
 #       Fdt object
 #       Filename
-output_fdt_files = {}
+#       Entry object, or None if not known
+output_fdt_info = {}
 
 # Arguments passed to binman to provide arguments to entries
 entry_args = {}
@@ -49,11 +50,29 @@ def GetFdtForEtype(etype):
     Returns:
         Fdt object associated with the entry type
     """
-    value = output_fdt_files.get(etype);
+    value = output_fdt_info.get(etype);
     if not value:
         return None
     return value[0]
 
+def GetEntryForEtype(etype):
+    """Get the Entry for a particular device-tree filename
+
+    Binman keeps track of at least one device-tree file called u-boot.dtb but
+    can also have others (e.g. for SPL). This function looks up the given
+    filename and returns the associated Fdt object.
+
+    Args:
+        etype: Entry type of device tree (e.g. 'u-boot-dtb')
+
+    Returns:
+        Entry object associated with the entry type, if present in the image
+    """
+    value = output_fdt_info.get(etype);
+    if not value:
+        return None
+    return value[2]
+
 def GetFdtPath(etype):
     """Get the full pathname of a particular Fdt object
 
@@ -66,7 +85,7 @@ def GetFdtPath(etype):
     Returns:
         Full path name to the associated Fdt
     """
-    return output_fdt_files[etype][0]._fname
+    return output_fdt_info[etype][0]._fname
 
 def GetFdtContents(etype='u-boot-dtb'):
     """Looks up the FDT pathname and contents
@@ -83,13 +102,13 @@ def GetFdtContents(etype='u-boot-dtb'):
             pathname to Fdt
             Fdt data (as bytes)
     """
-    if etype not in output_fdt_files:
+    if etype not in output_fdt_info:
         return None, None
     if not use_fake_dtb:
         pathname = GetFdtPath(etype)
         data = GetFdtForEtype(etype).GetContents()
     else:
-        fname = output_fdt_files[etype][1]
+        fname = output_fdt_info[etype][1]
         pathname = tools.GetInputFilename(fname)
         data = tools.ReadFile(pathname)
     return pathname, data
@@ -134,7 +153,7 @@ def Prepare(images, dtb):
         images: List of images being used
         dtb: Main dtb
     """
-    global output_fdt_files, main_dtb
+    global output_fdt_info, main_dtb
     # Import these here in case libfdt.py is not available, in which case
     # the above help option still works.
     import fdt
@@ -145,23 +164,23 @@ def Prepare(images, dtb):
     # since it is assumed to be the one passed in with options.dt, and
     # was handled just above.
     main_dtb = dtb
-    output_fdt_files.clear()
-    output_fdt_files['u-boot-dtb'] = [dtb, 'u-boot.dtb']
-    output_fdt_files['u-boot-spl-dtb'] = [dtb, 'spl/u-boot-spl.dtb']
-    output_fdt_files['u-boot-tpl-dtb'] = [dtb, 'tpl/u-boot-tpl.dtb']
+    output_fdt_info.clear()
+    output_fdt_info['u-boot-dtb'] = [dtb, 'u-boot.dtb', None]
+    output_fdt_info['u-boot-spl-dtb'] = [dtb, 'spl/u-boot-spl.dtb', None]
+    output_fdt_info['u-boot-tpl-dtb'] = [dtb, 'tpl/u-boot-tpl.dtb', None]
     if not use_fake_dtb:
         fdt_set = {}
         for image in images.values():
             fdt_set.update(image.GetFdts())
         for etype, other in fdt_set.items():
-            _, other_fname = other
+            entry, other_fname = other
             infile = tools.GetInputFilename(other_fname)
             other_fname_dtb = fdt_util.EnsureCompiled(infile)
             out_fname = tools.GetOutputFilename('%s.out' %
                     os.path.split(other_fname)[1])
             tools.WriteFile(out_fname, tools.ReadFile(other_fname_dtb))
             other_dtb = fdt.FdtScan(out_fname)
-            output_fdt_files[etype] = [other_dtb, other_fname]
+            output_fdt_info[etype] = [other_dtb, out_fname, entry]
 
 def GetAllFdts():
     """Yield all device tree files being used by binman
@@ -170,8 +189,8 @@ def GetAllFdts():
         Device trees being used (U-Boot proper, SPL, TPL)
     """
     yield main_dtb
-    for etype in output_fdt_files:
-        dtb = output_fdt_files[etype][0]
+    for etype in output_fdt_info:
+        dtb = output_fdt_info[etype][0]
         if dtb != main_dtb:
             yield dtb
 
@@ -190,7 +209,7 @@ def GetUpdateNodes(node):
             is node, SPL and TPL)
     """
     yield node
-    for dtb, fname in output_fdt_files.values():
+    for dtb, fname, _ in output_fdt_info.values():
         if dtb != node.GetFdt():
             other_node = dtb.GetNode(node.path)
             if other_node:
-- 
cgit v1.2.3


From 1411ac8d162eaf97714b17848a2da7be1f01fa98 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:44 -0600
Subject: binman: Add an image name into the fdtmap

Since binman supports multiple images it is useful to know which one
created the image that has been read. Then it is possible to look up that
name in the 'master' device tree (containing the description of all
images).

Add a property for this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/etype/fdtmap.py | 2 ++
 tools/binman/ftest.py        | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
index 229b4a1bb69..a55c9c899bf 100644
--- a/tools/binman/etype/fdtmap.py
+++ b/tools/binman/etype/fdtmap.py
@@ -60,6 +60,7 @@ class Entry_fdtmap(Entry):
     Example output for a simple image with U-Boot and an FDT map:
 
     / {
+        image-name = "binman";
         size = <0x00000112>;
         image-pos = <0x00000000>;
         offset = <0x00000000>;
@@ -110,6 +111,7 @@ class Entry_fdtmap(Entry):
         fsw = libfdt.FdtSw()
         fsw.finish_reservemap()
         with fsw.add_node(''):
+            fsw.property_string('image-node', node.name)
             _AddNode(node)
         fdt = fsw.as_fdt()
 
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 6a40d1fdbb4..08a1df03077 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2369,7 +2369,7 @@ class TestFunctional(unittest.TestCase):
 '      u-boot            138     4  u-boot            38',
 '      u-boot-dtb        180   10f  u-boot-dtb        80          3c9',
 '    u-boot-dtb          500   %x  u-boot-dtb       400          3c9' % fdt_size,
-'  fdtmap                %x   395  fdtmap           %x' %
+'  fdtmap                %x   3b4  fdtmap           %x' %
         (fdtmap_offset, fdtmap_offset),
 '  image-header          bf8     8  image-header     bf8',
             ]
-- 
cgit v1.2.3


From c6bd6e235ac6d6a35e9ad8f3db49db7ba27f7650 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:45 -0600
Subject: binman: Adjust Entry to read the node in a separate call

At present the Entry constructor sets up the object and then immediately
reads its device-tree node to obtain its properties.

This breaks a convention that constructors should not do any processing.
A consequence is that we must pass all arguments to the constructor and
cannot have the node-reading proceed in a different way unless we pass
flags to that constructor. We already have a 'test' flag in a few cases,
and now need to control whether the 'orig_offset' and 'orig_size'
properties are set or not.

Adjust the code to require a separate call to ReadNode() after
construction. The Image class remains as it was.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/entry.py            |  6 +++---
 tools/binman/entry_test.py       |  8 ++++----
 tools/binman/etype/_testing.py   |  3 +++
 tools/binman/etype/cbfs.py       |  1 +
 tools/binman/etype/fill.py       |  3 +++
 tools/binman/etype/intel_ifwi.py |  1 +
 tools/binman/etype/section.py    | 29 +++++++++++++++--------------
 tools/binman/image.py            | 10 +++++++---
 8 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 6436384254d..c4ddb43b318 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -70,7 +70,7 @@ class Entry(object):
         orig_offset: Original offset value read from node
         orig_size: Original size value read from node
     """
-    def __init__(self, section, etype, node, read_node=True, name_prefix=''):
+    def __init__(self, section, etype, node, name_prefix=''):
         self.section = section
         self.etype = etype
         self._node = node
@@ -89,8 +89,6 @@ class Entry(object):
         self.image_pos = None
         self._expand_size = False
         self.compress = 'none'
-        if read_node:
-            self.ReadNode()
 
     @staticmethod
     def Lookup(node_path, etype):
@@ -155,6 +153,8 @@ class Entry(object):
     def ReadNode(self):
         """Read entry information from the node
 
+        This must be called as the first thing after the Entry is created.
+
         This reads all the fields we recognise from the node, ready for use.
         """
         if 'pos' in self._node.props:
diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py
index 460171ba899..ee729f37519 100644
--- a/tools/binman/entry_test.py
+++ b/tools/binman/entry_test.py
@@ -57,7 +57,7 @@ class TestEntry(unittest.TestCase):
     def testEntryContents(self):
         """Test the Entry bass class"""
         import entry
-        base_entry = entry.Entry(None, None, None, read_node=False)
+        base_entry = entry.Entry(None, None, None)
         self.assertEqual(True, base_entry.ObtainContents())
 
     def testUnknownEntry(self):
@@ -73,15 +73,15 @@ class TestEntry(unittest.TestCase):
         """Test Entry.GetUniqueName"""
         Node = collections.namedtuple('Node', ['name', 'parent'])
         base_node = Node('root', None)
-        base_entry = entry.Entry(None, None, base_node, read_node=False)
+        base_entry = entry.Entry(None, None, base_node)
         self.assertEqual('root', base_entry.GetUniqueName())
         sub_node = Node('subnode', base_node)
-        sub_entry = entry.Entry(None, None, sub_node, read_node=False)
+        sub_entry = entry.Entry(None, None, sub_node)
         self.assertEqual('root.subnode', sub_entry.GetUniqueName())
 
     def testGetDefaultFilename(self):
         """Trivial test for this base class function"""
-        base_entry = entry.Entry(None, None, None, read_node=False)
+        base_entry = entry.Entry(None, None, None)
         self.assertIsNone(base_entry.GetDefaultFilename())
 
     def testBlobFdt(self):
diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py
index ae24fe8105a..4a2e4eb7caf 100644
--- a/tools/binman/etype/_testing.py
+++ b/tools/binman/etype/_testing.py
@@ -42,6 +42,9 @@ class Entry__testing(Entry):
     """
     def __init__(self, section, etype, node):
         Entry.__init__(self, section, etype, node)
+
+    def ReadNode(self):
+        Entry.ReadNode(self)
         self.return_invalid_entry = fdt_util.GetBool(self._node,
                                                      'return-invalid-entry')
         self.return_unknown_contents = fdt_util.GetBool(self._node,
diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index edf2189fd26..d73c706c444 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -203,6 +203,7 @@ class Entry_cbfs(Entry):
         """Read the subnodes to find out what should go in this IFWI"""
         for node in self._node.subnodes:
             entry = Entry.Create(self.section, node)
+            entry.ReadNode()
             entry._cbfs_name = fdt_util.GetString(node, 'cbfs-name', entry.name)
             entry._type = fdt_util.GetString(node, 'cbfs-type')
             compress = fdt_util.GetString(node, 'cbfs-compress', 'none')
diff --git a/tools/binman/etype/fill.py b/tools/binman/etype/fill.py
index 68efe42ec0b..623b7f4e95e 100644
--- a/tools/binman/etype/fill.py
+++ b/tools/binman/etype/fill.py
@@ -23,6 +23,9 @@ class Entry_fill(Entry):
     """
     def __init__(self, section, etype, node):
         Entry.__init__(self, section, etype, node)
+
+    def ReadNode(self):
+        Entry.ReadNode(self)
         if self.size is None:
             self.Raise("'fill' entry must have a size property")
         self.fill_value = fdt_util.GetByte(self._node, 'fill-byte', 0)
diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py
index 8c79b2dd291..9cbdf3698a3 100644
--- a/tools/binman/etype/intel_ifwi.py
+++ b/tools/binman/etype/intel_ifwi.py
@@ -94,6 +94,7 @@ class Entry_intel_ifwi(Entry_blob):
         """Read the subnodes to find out what should go in this IFWI"""
         for node in self._node.subnodes:
             entry = Entry.Create(self.section, node)
+            entry.ReadNode()
             entry._ifwi_replace = fdt_util.GetBool(node, 'replace')
             entry._ifwi_subpart = fdt_util.GetString(node, 'ifwi-subpart')
             entry._ifwi_entry_name = fdt_util.GetString(node, 'ifwi-entry')
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index cd623821a34..c875a79f1ff 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -52,22 +52,10 @@ class Entry_section(Entry):
         self._sort = False
         self._skip_at_start = None
         self._end_4gb = False
-        if not test:
-            self._ReadNode()
-            self._ReadEntries()
-
-    def _Raise(self, msg):
-        """Raises an error for this section
-
-        Args:
-            msg: Error message to use in the raise string
-        Raises:
-            ValueError()
-        """
-        raise ValueError("Section '%s': %s" % (self._node.path, msg))
 
-    def _ReadNode(self):
+    def ReadNode(self):
         """Read properties from the image node"""
+        Entry.ReadNode(self)
         self._pad_byte = fdt_util.GetInt(self._node, 'pad-byte', 0)
         self._sort = fdt_util.GetBool(self._node, 'sort-by-offset')
         self._end_4gb = fdt_util.GetBool(self._node, 'end-at-4gb')
@@ -87,14 +75,27 @@ class Entry_section(Entry):
         if filename:
             self._filename = filename
 
+        self._ReadEntries()
+
     def _ReadEntries(self):
         for node in self._node.subnodes:
             if node.name == 'hash':
                 continue
             entry = Entry.Create(self, node)
+            entry.ReadNode()
             entry.SetPrefix(self._name_prefix)
             self._entries[node.name] = entry
 
+    def _Raise(self, msg):
+        """Raises an error for this section
+
+        Args:
+            msg: Error message to use in the raise string
+        Raises:
+            ValueError()
+        """
+        raise ValueError("Section '%s': %s" % (self._node.path, msg))
+
     def GetFdts(self):
         fdts = {}
         for entry in self._entries.values():
diff --git a/tools/binman/image.py b/tools/binman/image.py
index 232e752258c..970d33f7110 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -50,9 +50,13 @@ class Image(section.Entry_section):
         self.fdtmap_dtb = None
         self.fdtmap_data = None
         if not test:
-            filename = fdt_util.GetString(self._node, 'filename')
-            if filename:
-                self._filename = filename
+            self.ReadNode()
+
+    def ReadNode(self):
+        section.Entry_section.ReadNode(self)
+        filename = fdt_util.GetString(self._node, 'filename')
+        if filename:
+            self._filename = filename
 
     @classmethod
     def FromFile(cls, fname):
-- 
cgit v1.2.3


From c5ad04b72169c40e3646ed5bba28832eed2c5d4f Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:46 -0600
Subject: binman: Add a function to obtain the image for an Entry

At present we have an 'image' property in the entry for this purpose, but
this is not necessary and seems error-prone in the presence of
inheritance. Add a function instead. The Entry_section class overrides
this with a special version, since top-level sections are in fact images,
since Image inherits Entry_section.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/entry.py         |  8 ++++++++
 tools/binman/etype/fmap.py    |  2 +-
 tools/binman/etype/section.py | 17 ++++++++++++++---
 tools/binman/image.py         |  1 -
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index c4ddb43b318..ddf52d8e103 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -700,3 +700,11 @@ features to produce new behaviours.
         data = self.ReadData(decomp)
         self.ProcessContentsUpdate(data)
         self.Detail('Loaded data size %x' % len(data))
+
+    def GetImage(self):
+        """Get the image containing this entry
+
+        Returns:
+            Image object containing this entry
+        """
+        return self.section.GetImage()
diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py
index f8d8d866f13..56677cbac1c 100644
--- a/tools/binman/etype/fmap.py
+++ b/tools/binman/etype/fmap.py
@@ -49,7 +49,7 @@ class Entry_fmap(Entry):
                 areas.append(fmap_util.FmapArea(pos or 0, entry.size or 0,
                                             tools.FromUnicode(entry.name), 0))
 
-        entries = self.section.image.GetEntries()
+        entries = self.GetImage().GetEntries()
         areas = []
         for entry in entries.values():
             _AddEntries(areas, entry)
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index c875a79f1ff..0fb81419cea 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -45,8 +45,6 @@ class Entry_section(Entry):
     def __init__(self, section, etype, node, test=False):
         if not test:
             Entry.__init__(self, section, etype, node)
-        if section:
-            self.image = section.image
         self._entries = OrderedDict()
         self._pad_byte = 0
         self._sort = False
@@ -374,7 +372,7 @@ class Entry_section(Entry):
             Image size as an integer number of bytes, which may be None if the
                 image size is dynamic and its sections have not yet been packed
         """
-        return self.image.size
+        return self.GetImage().size
 
     def FindEntryType(self, etype):
         """Find an entry type in the section
@@ -468,3 +466,16 @@ class Entry_section(Entry):
         for entry in self._entries.values():
             entry.LoadData(decomp)
         self.Detail('Loaded data')
+
+    def GetImage(self):
+        """Get the image containing this section
+
+        Note that a top-level section is actually an Image, so this function may
+        return self.
+
+        Returns:
+            Image object containing this section
+        """
+        if not self.section:
+            return self
+        return self.section.GetImage()
diff --git a/tools/binman/image.py b/tools/binman/image.py
index 970d33f7110..c9908187343 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -42,7 +42,6 @@ class Image(section.Entry_section):
             we create a section manually.
     """
     def __init__(self, name, node, test=False):
-        self.image = self
         section.Entry_section.__init__(self, None, 'section', node, test)
         self.name = 'main-section'
         self.image_name = name
-- 
cgit v1.2.3


From 6ccbfcdd96b9eade93379ad00ee11c7d055d1690 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:47 -0600
Subject: binman: Add a constant for common entry properties

We use this same combination of properties several times in tests. Add a
constant for it to avoid typos, etc.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/ftest.py | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 08a1df03077..bb886266277 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -69,8 +69,12 @@ FILES_DATA            = (b"sorry I'm late\nOh, don't bother apologising, I'm " +
 COMPRESS_DATA         = b'compress xxxxxxxxxxxxxxxxxxxxxx data'
 REFCODE_DATA          = b'refcode'
 
+# The expected size for the device tree in some tests
 EXTRACT_DTB_SIZE = 0x3c9
 
+# Properties expected to be in the device tree when update_dtb is used
+BASE_DTB_PROPS = ['offset', 'size', 'image-pos']
+
 
 class TestFunctional(unittest.TestCase):
     """Functional tests for binman
@@ -1240,7 +1244,7 @@ class TestFunctional(unittest.TestCase):
                                                      update_dtb=True)
         dtb = fdt.Fdt(out_dtb_fname)
         dtb.Scan()
-        props = self._GetPropTree(dtb, ['offset', 'size', 'image-pos'])
+        props = self._GetPropTree(dtb, BASE_DTB_PROPS)
         self.assertEqual({
             'image-pos': 0,
             'offset': 0,
@@ -1583,8 +1587,7 @@ class TestFunctional(unittest.TestCase):
         for item in ['', 'spl', 'tpl']:
             dtb = fdt.Fdt.FromData(data[start:])
             dtb.Scan()
-            props = self._GetPropTree(dtb, ['offset', 'size', 'image-pos',
-                                            'spl', 'tpl'])
+            props = self._GetPropTree(dtb, BASE_DTB_PROPS + ['spl', 'tpl'])
             expected = dict(base_expected)
             if item:
                 expected[item] = 0
@@ -2052,8 +2055,7 @@ class TestFunctional(unittest.TestCase):
         fdt_data = fdtmap_data[16:]
         dtb = fdt.Fdt.FromData(fdt_data)
         dtb.Scan()
-        props = self._GetPropTree(dtb, ['offset', 'size', 'image-pos'],
-                                  prefix='/')
+        props = self._GetPropTree(dtb, BASE_DTB_PROPS, prefix='/')
         self.assertEqual({
             'image-pos': 0,
             'offset': 0,
@@ -2172,8 +2174,7 @@ class TestFunctional(unittest.TestCase):
                                                         update_dtb=True)
         dtb = fdt.Fdt(out_dtb_fname)
         dtb.Scan()
-        props = self._GetPropTree(dtb, ['offset', 'size', 'image-pos',
-                                        'uncomp-size'])
+        props = self._GetPropTree(dtb, BASE_DTB_PROPS + ['uncomp-size'])
         del props['cbfs/u-boot:size']
         self.assertEqual({
             'offset': 0,
-- 
cgit v1.2.3


From 02fd463cfeeb3ec693539885dbc58e0439d3b4de Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:48 -0600
Subject: binman: Allow the fdtmap to remain unchanged

When updating an existing image where the size of all entries remains the
same, we should not need to regenerate the fdtmap. Update the entry to
return the same fdtmap as was read from the image.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/etype/fdtmap.py | 55 ++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
index a55c9c899bf..1271b50036a 100644
--- a/tools/binman/etype/fdtmap.py
+++ b/tools/binman/etype/fdtmap.py
@@ -93,31 +93,36 @@ class Entry_fdtmap(Entry):
                 with fsw.add_node(subnode.name):
                     _AddNode(subnode)
 
-        # Get the FDT data into an Fdt object
-        data = state.GetFdtContents()[1]
-        infdt = Fdt.FromData(data)
-        infdt.Scan()
-
-        # Find the node for the image containing the Fdt-map entry
-        path = self.section.GetPath()
-        self.Detail("Fdtmap: Using section '%s' (path '%s')" %
-                    (self.section.name, path))
-        node = infdt.GetNode(path)
-        if not node:
-            self.Raise("Internal error: Cannot locate node for path '%s'" %
-                       path)
-
-        # Build a new tree with all nodes and properties starting from that node
-        fsw = libfdt.FdtSw()
-        fsw.finish_reservemap()
-        with fsw.add_node(''):
-            fsw.property_string('image-node', node.name)
-            _AddNode(node)
-        fdt = fsw.as_fdt()
-
-        # Pack this new FDT and return its contents
-        fdt.pack()
-        outfdt = Fdt.FromData(fdt.as_bytearray())
+        outfdt = self.GetImage().fdtmap_dtb
+        # If we have an fdtmap it means that we are using this as the
+        # read-only fdtmap for this image.
+        if not outfdt:
+            # Get the FDT data into an Fdt object
+            data = state.GetFdtContents()[1]
+            infdt = Fdt.FromData(data)
+            infdt.Scan()
+
+            # Find the node for the image containing the Fdt-map entry
+            path = self.section.GetPath()
+            self.Detail("Fdtmap: Using section '%s' (path '%s')" %
+                        (self.section.name, path))
+            node = infdt.GetNode(path)
+            if not node:
+                self.Raise("Internal error: Cannot locate node for path '%s'" %
+                           path)
+
+            # Build a new tree with all nodes and properties starting from that
+            # node
+            fsw = libfdt.FdtSw()
+            fsw.finish_reservemap()
+            with fsw.add_node(''):
+                fsw.property_string('image-node', node.name)
+                _AddNode(node)
+            fdt = fsw.as_fdt()
+
+            # Pack this new FDT and return its contents
+            fdt.pack()
+            outfdt = Fdt.FromData(fdt.as_bytearray())
         data = FDTMAP_MAGIC + tools.GetBytes(0, 8) + outfdt.GetContents()
         return data
 
-- 
cgit v1.2.3


From a004f29464d14f3535ed8db22e5dfed02c8fc9d8 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:49 -0600
Subject: binman: Tidy up _SetupDtb() to use its own temporary file

At present EnsureCompiled() uses an file from the 'output' directory (in
the tools module) when compiling the device tree. This is fine in most
cases, allowing useful inspection of the output files from binman.

However in functional tests, _SetupDtb() creates an output directory and
immediately removes it afterwards. This serves no benefit and just
confuses things, since the 'official' output directory is supposed to be
created and destroyed in control.Binman().

Add a new parameter for the optional temporary directory to use, and use a
separate temporary directory in _SetupDtb().

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/ftest.py  |  6 +++---
 tools/dtoc/fdt_util.py | 12 +++++++++---
 tools/dtoc/test_fdt.py | 17 ++++++++++++++++-
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index bb886266277..47153285922 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -287,12 +287,12 @@ class TestFunctional(unittest.TestCase):
         Returns:
             Contents of device-tree binary
         """
-        tools.PrepareOutputDir(None)
-        dtb = fdt_util.EnsureCompiled(self.TestFile(fname))
+        tmpdir = tempfile.mkdtemp(prefix='binmant.')
+        dtb = fdt_util.EnsureCompiled(self.TestFile(fname), tmpdir)
         with open(dtb, 'rb') as fd:
             data = fd.read()
             TestFunctional._MakeInputFile(outfile, data)
-        tools.FinaliseOutputDir()
+        shutil.rmtree(tmpdir)
         return data
 
     def _GetDtbContentsForSplTpl(self, dtb_data, name):
diff --git a/tools/dtoc/fdt_util.py b/tools/dtoc/fdt_util.py
index f47879ac006..b105faec749 100644
--- a/tools/dtoc/fdt_util.py
+++ b/tools/dtoc/fdt_util.py
@@ -43,12 +43,14 @@ def fdt_cells_to_cpu(val, cells):
         out = out << 32 | fdt32_to_cpu(val[1])
     return out
 
-def EnsureCompiled(fname, capture_stderr=False):
+def EnsureCompiled(fname, tmpdir=None, capture_stderr=False):
     """Compile an fdt .dts source file into a .dtb binary blob if needed.
 
     Args:
         fname: Filename (if .dts it will be compiled). It not it will be
             left alone
+        tmpdir: Temporary directory for output files, or None to use the
+            tools-module output directory
 
     Returns:
         Filename of resulting .dtb file
@@ -57,8 +59,12 @@ def EnsureCompiled(fname, capture_stderr=False):
     if ext != '.dts':
         return fname
 
-    dts_input = tools.GetOutputFilename('source.dts')
-    dtb_output = tools.GetOutputFilename('source.dtb')
+    if tmpdir:
+        dts_input = os.path.join(tmpdir, 'source.dts')
+        dtb_output = os.path.join(tmpdir, 'source.dtb')
+    else:
+        dts_input = tools.GetOutputFilename('source.dts')
+        dtb_output = tools.GetOutputFilename('source.dtb')
 
     search_paths = [os.path.join(os.getcwd(), 'include')]
     root, _ = os.path.splitext(fname)
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index ed2d982a8fc..16a4430892e 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -9,7 +9,9 @@ from __future__ import print_function
 from optparse import OptionParser
 import glob
 import os
+import shutil
 import sys
+import tempfile
 import unittest
 
 # Bring in the patman libraries
@@ -540,10 +542,23 @@ class TestFdtUtil(unittest.TestCase):
         self.assertEqual(0x12345678, fdt_util.fdt_cells_to_cpu(val, 1))
 
     def testEnsureCompiled(self):
-        """Test a degenerate case of this function"""
+        """Test a degenerate case of this function (file already compiled)"""
         dtb = fdt_util.EnsureCompiled('tools/dtoc/dtoc_test_simple.dts')
         self.assertEqual(dtb, fdt_util.EnsureCompiled(dtb))
 
+    def testEnsureCompiledTmpdir(self):
+        """Test providing a temporary directory"""
+        try:
+            old_outdir = tools.outdir
+            tools.outdir= None
+            tmpdir = tempfile.mkdtemp(prefix='test_fdt.')
+            dtb = fdt_util.EnsureCompiled('tools/dtoc/dtoc_test_simple.dts',
+                                          tmpdir)
+            self.assertEqual(tmpdir, os.path.dirname(dtb))
+            shutil.rmtree(tmpdir)
+        finally:
+            tools.outdir= old_outdir
+
 
 def RunTestCoverage():
     """Run the tests and check that we get 100% coverage"""
-- 
cgit v1.2.3


From 10f9d0066b9e9e14327922fa62c2a1b6bea50785 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:50 -0600
Subject: binman: Support updating entries in an existing image

While it is useful and efficient to build images in a single pass from a
unified description, it is sometimes desirable to update the image later.

Add support for replace an existing file with one of the same size. This
avoids needing to repack the file. Support for more advanced updates will
come in future patches.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/README                     |  13 +++-
 tools/binman/control.py                 |  33 ++++++++++-
 tools/binman/entry.py                   |  23 +++++++
 tools/binman/ftest.py                   | 102 +++++++++++++++++++++++++++++++-
 tools/binman/image.py                   |   3 +
 tools/binman/state.py                   |  74 ++++++++++++++++-------
 tools/binman/test/132_replace.dts       |  21 +++++++
 tools/binman/test/133_replace_multi.dts |  33 +++++++++++
 8 files changed, 277 insertions(+), 25 deletions(-)
 create mode 100644 tools/binman/test/132_replace.dts
 create mode 100644 tools/binman/test/133_replace_multi.dts

diff --git a/tools/binman/README b/tools/binman/README
index 756c6a0010e..35223545194 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -557,6 +557,18 @@ or just a selection:
     $ binman extract -i image.bin "*u-boot*" -O outdir
 
 
+Replacing files in an image
+---------------------------
+
+You can replace files in an existing firmware image created by binman, provided
+that there is an 'fdtmap' entry in the image. For example:
+
+    $ binman replace -i image.bin section/cbfs/u-boot
+
+which will write the contents of the file 'u-boot' from the current directory
+to the that entry. The file must be the same size as the entry being replaced.
+
+
 Logging
 -------
 
@@ -909,7 +921,6 @@ Some ideas:
 - Allow easy building of images by specifying just the board name
 - Support building an image for a board (-b) more completely, with a
   configurable build directory
-- Support updating binaries in an image (with no size change / repacking)
 - Support updating binaries in an image (with repacking)
 - Support adding FITs to an image
 - Support for ARM Trusted Firmware (ATF)
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 8700f48ad55..ab94f9d4829 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -118,6 +118,32 @@ def ReadEntry(image_fname, entry_path, decomp=True):
     return entry.ReadData(decomp)
 
 
+def WriteEntry(image_fname, entry_path, data, decomp=True):
+    """Replace an entry in an image
+
+    This replaces the data in a particular entry in an image. This size of the
+    new data must match the size of the old data
+
+    Args:
+        image_fname: Image filename to process
+        entry_path: Path to entry to extract
+        data: Data to replace with
+        decomp: True to compress the data if needed, False if data is
+            already compressed so should be used as is
+    """
+    tout.Info("WriteEntry '%s', file '%s'" % (entry_path, image_fname))
+    image = Image.FromFile(image_fname)
+    entry = image.FindEntryPath(entry_path)
+    state.PrepareFromLoadedData(image)
+    image.LoadData()
+    tout.Info('Writing data to %s' % entry.GetPath())
+    if not entry.WriteData(data, decomp):
+        entry.Raise('Entry data size does not match, but resize is disabled')
+    tout.Info('Processing image')
+    ProcessImage(image, update_fdt=True, write_map=False, get_contents=False)
+    tout.Info('WriteEntry done')
+
+
 def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
                    decomp=True):
     """Extract the data from one or more entries and write it to files
@@ -238,7 +264,7 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt):
     return images
 
 
-def ProcessImage(image, update_fdt, write_map):
+def ProcessImage(image, update_fdt, write_map, get_contents=True):
     """Perform all steps for this image, including checking and # writing it.
 
     This means that errors found with a later image will be reported after
@@ -249,8 +275,11 @@ def ProcessImage(image, update_fdt, write_map):
         image: Image to process
         update_fdt: True to update the FDT wth entry offsets, etc.
         write_map: True to write a map file
+        get_contents: True to get the image contents from files, etc., False if
+            the contents is already present
     """
-    image.GetEntryContents()
+    if get_contents:
+        image.GetEntryContents()
     image.GetEntryOffsets()
 
     # We need to pack the entries to figure out where everything
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index ddf52d8e103..07d5838c1a2 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -698,6 +698,7 @@ features to produce new behaviours.
 
     def LoadData(self, decomp=True):
         data = self.ReadData(decomp)
+        self.contents_size = len(data)
         self.ProcessContentsUpdate(data)
         self.Detail('Loaded data size %x' % len(data))
 
@@ -708,3 +709,25 @@ features to produce new behaviours.
             Image object containing this entry
         """
         return self.section.GetImage()
+
+    def WriteData(self, data, decomp=True):
+        """Write the data to an entry in the image
+
+        This is used when the image has been read in and we want to replace the
+        data for a particular entry in that image.
+
+        The image must be re-packed and written out afterwards.
+
+        Args:
+            data: Data to replace it with
+            decomp: True to compress the data if needed, False if data is
+                already compressed so should be used as is
+
+        Returns:
+            True if the data did not result in a resize of this entry, False if
+                 the entry must be resized
+        """
+        self.contents_size = self.size
+        ok = self.ProcessContentsUpdate(data)
+        self.Detail('WriteData: size=%x, ok=%s' % (len(data), ok))
+        return ok
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 47153285922..e201b741c6f 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2334,7 +2334,7 @@ class TestFunctional(unittest.TestCase):
         image_fname = tools.GetOutputFilename('image.bin')
         image = Image.FromFile(image_fname)
         self.assertTrue(isinstance(image, Image))
-        self.assertEqual('image', image.image_name)
+        self.assertEqual('image', image.image_name[-5:])
 
     def testReadImageFail(self):
         """Test failing to read an image image's FDT map"""
@@ -2720,6 +2720,106 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(len(U_BOOT_DATA), entry.contents_size)
         self.assertEqual(len(U_BOOT_DATA), entry.size)
 
+    def _RunReplaceCmd(self, entry_name, data, decomp=True):
+        """Replace an entry in an image
+
+        This writes the entry data to update it, then opens the updated file and
+        returns the value that it now finds there.
+
+        Args:
+            entry_name: Entry name to replace
+            data: Data to replace it with
+            decomp: True to compress the data if needed, False if data is
+                already compressed so should be used as is
+
+        Returns:
+            Tuple:
+                data from entry
+                data from fdtmap (excluding header)
+        """
+        dtb_data = self._DoReadFileDtb('132_replace.dts', use_real_dtb=True,
+                                       update_dtb=True)[1]
+
+        self.assertIn('image', control.images)
+        image = control.images['image']
+        entries = image.GetEntries()
+        orig_dtb_data = entries['u-boot-dtb'].data
+        orig_fdtmap_data = entries['fdtmap'].data
+
+        image_fname = tools.GetOutputFilename('image.bin')
+        updated_fname = tools.GetOutputFilename('image-updated.bin')
+        tools.WriteFile(updated_fname, tools.ReadFile(image_fname))
+        control.WriteEntry(updated_fname, entry_name, data, decomp)
+        data = control.ReadEntry(updated_fname, entry_name, decomp)
+
+        # The DT data should not change
+        new_dtb_data = entries['u-boot-dtb'].data
+        self.assertEqual(new_dtb_data, orig_dtb_data)
+        new_fdtmap_data = entries['fdtmap'].data
+        self.assertEqual(new_fdtmap_data, orig_fdtmap_data)
+
+        return data, orig_fdtmap_data[fdtmap.FDTMAP_HDR_LEN:]
+
+    def testReplaceSimple(self):
+        """Test replacing a single file"""
+        expected = b'x' * len(U_BOOT_DATA)
+        data, expected_fdtmap = self._RunReplaceCmd('u-boot', expected)
+        self.assertEqual(expected, data)
+
+        # Test that the state looks right. There should be an FDT for the fdtmap
+        # that we jsut read back in, and it should match what we find in the
+        # 'control' tables. Checking for an FDT that does not exist should
+        # return None.
+        path, fdtmap = state.GetFdtContents('fdtmap')
+        self.assertIsNone(path)
+        self.assertEqual(expected_fdtmap, fdtmap)
+
+        dtb = state.GetFdtForEtype('fdtmap')
+        self.assertEqual(dtb.GetContents(), fdtmap)
+
+        missing_path, missing_fdtmap = state.GetFdtContents('missing')
+        self.assertIsNone(missing_path)
+        self.assertIsNone(missing_fdtmap)
+
+        missing_dtb = state.GetFdtForEtype('missing')
+        self.assertIsNone(missing_dtb)
+
+        self.assertEqual('/binman', state.fdt_path_prefix)
+
+    def testReplaceResizeFail(self):
+        """Test replacing a file by something larger"""
+        expected = U_BOOT_DATA + b'x'
+        with self.assertRaises(ValueError) as e:
+            self._RunReplaceCmd('u-boot', expected)
+        self.assertIn("Node '/u-boot': Entry data size does not match, but resize is disabled",
+                      str(e.exception))
+
+    def testReplaceMulti(self):
+        """Test replacing entry data where multiple images are generated"""
+        data = self._DoReadFileDtb('133_replace_multi.dts', use_real_dtb=True,
+                                   update_dtb=True)[0]
+        expected = b'x' * len(U_BOOT_DATA)
+        updated_fname = tools.GetOutputFilename('image-updated.bin')
+        tools.WriteFile(updated_fname, data)
+        entry_name = 'u-boot'
+        control.WriteEntry(updated_fname, entry_name, expected)
+        data = control.ReadEntry(updated_fname, entry_name)
+        self.assertEqual(expected, data)
+
+        # Check the state looks right.
+        self.assertEqual('/binman/image', state.fdt_path_prefix)
+
+        # Now check we can write the first image
+        image_fname = tools.GetOutputFilename('first-image.bin')
+        updated_fname = tools.GetOutputFilename('first-updated.bin')
+        tools.WriteFile(updated_fname, tools.ReadFile(image_fname))
+        entry_name = 'u-boot'
+        control.WriteEntry(updated_fname, entry_name, expected)
+        data = control.ReadEntry(updated_fname, entry_name)
+        self.assertEqual(expected, data)
+
+        # Check the state looks right.
+        self.assertEqual('/binman/first-image', state.fdt_path_prefix)
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/image.py b/tools/binman/image.py
index c9908187343..5185b68990a 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -10,6 +10,7 @@ from __future__ import print_function
 from collections import OrderedDict
 import fnmatch
 from operator import attrgetter
+import os
 import re
 import sys
 
@@ -96,6 +97,8 @@ class Image(section.Entry_section):
         image.fdtmap_dtb = dtb
         image.fdtmap_data = fdtmap_data
         image._data = data
+        image._filename = fname
+        image.image_name, _ = os.path.splitext(fname)
         return image
 
     def Raise(self, msg):
diff --git a/tools/binman/state.py b/tools/binman/state.py
index 34907d9d43c..08e627985de 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -8,8 +8,10 @@
 import hashlib
 import re
 
+import fdt
 import os
 import tools
+import tout
 
 # Records the device-tree files known to binman, keyed by entry type (e.g.
 # 'u-boot-spl-dtb'). These are the output FDT files, which can be updated by
@@ -22,6 +24,9 @@ import tools
 #       Entry object, or None if not known
 output_fdt_info = {}
 
+# Prefix to add to an fdtmap path to turn it into a path to the /binman node
+fdt_path_prefix = ''
+
 # Arguments passed to binman to provide arguments to entries
 entry_args = {}
 
@@ -55,24 +60,6 @@ def GetFdtForEtype(etype):
         return None
     return value[0]
 
-def GetEntryForEtype(etype):
-    """Get the Entry for a particular device-tree filename
-
-    Binman keeps track of at least one device-tree file called u-boot.dtb but
-    can also have others (e.g. for SPL). This function looks up the given
-    filename and returns the associated Fdt object.
-
-    Args:
-        etype: Entry type of device tree (e.g. 'u-boot-dtb')
-
-    Returns:
-        Entry object associated with the entry type, if present in the image
-    """
-    value = output_fdt_info.get(etype);
-    if not value:
-        return None
-    return value[2]
-
 def GetFdtPath(etype):
     """Get the full pathname of a particular Fdt object
 
@@ -153,7 +140,7 @@ def Prepare(images, dtb):
         images: List of images being used
         dtb: Main dtb
     """
-    global output_fdt_info, main_dtb
+    global output_fdt_info, main_dtb, fdt_path_prefix
     # Import these here in case libfdt.py is not available, in which case
     # the above help option still works.
     import fdt
@@ -165,6 +152,7 @@ def Prepare(images, dtb):
     # was handled just above.
     main_dtb = dtb
     output_fdt_info.clear()
+    fdt_path_prefix = ''
     output_fdt_info['u-boot-dtb'] = [dtb, 'u-boot.dtb', None]
     output_fdt_info['u-boot-spl-dtb'] = [dtb, 'spl/u-boot-spl.dtb', None]
     output_fdt_info['u-boot-tpl-dtb'] = [dtb, 'tpl/u-boot-tpl.dtb', None]
@@ -182,13 +170,57 @@ def Prepare(images, dtb):
             other_dtb = fdt.FdtScan(out_fname)
             output_fdt_info[etype] = [other_dtb, out_fname, entry]
 
+def PrepareFromLoadedData(image):
+    """Get device tree files ready for use with a loaded image
+
+    Loaded images are different from images that are being created by binman,
+    since there is generally already an fdtmap and we read the description from
+    that. This provides the position and size of every entry in the image with
+    no calculation required.
+
+    This function uses the same output_fdt_info[] as Prepare(). It finds the
+    device tree files, adds a reference to the fdtmap and sets the FDT path
+    prefix to translate from the fdtmap (where the root node is the image node)
+    to the normal device tree (where the image node is under a /binman node).
+
+    Args:
+        images: List of images being used
+    """
+    global output_fdt_info, main_dtb, fdt_path_prefix
+
+    tout.Info('Preparing device trees')
+    output_fdt_info.clear()
+    fdt_path_prefix = ''
+    output_fdt_info['fdtmap'] = [image.fdtmap_dtb, 'u-boot.dtb', None]
+    main_dtb = None
+    tout.Info("   Found device tree type 'fdtmap' '%s'" % image.fdtmap_dtb.name)
+    for etype, value in image.GetFdts().items():
+        entry, fname = value
+        out_fname = tools.GetOutputFilename('%s.dtb' % entry.etype)
+        tout.Info("   Found device tree type '%s' at '%s' path '%s'" %
+                  (etype, out_fname, entry.GetPath()))
+        entry._filename = entry.GetDefaultFilename()
+        data = entry.ReadData()
+
+        tools.WriteFile(out_fname, data)
+        dtb = fdt.Fdt(out_fname)
+        dtb.Scan()
+        image_node = dtb.GetNode('/binman')
+        if 'multiple-images' in image_node.props:
+            image_node = dtb.GetNode('/binman/%s' % image.image_node)
+        fdt_path_prefix = image_node.path
+        output_fdt_info[etype] = [dtb, None, entry]
+    tout.Info("   FDT path prefix '%s'" % fdt_path_prefix)
+
+
 def GetAllFdts():
     """Yield all device tree files being used by binman
 
     Yields:
         Device trees being used (U-Boot proper, SPL, TPL)
     """
-    yield main_dtb
+    if main_dtb:
+        yield main_dtb
     for etype in output_fdt_info:
         dtb = output_fdt_info[etype][0]
         if dtb != main_dtb:
@@ -211,7 +243,7 @@ def GetUpdateNodes(node):
     yield node
     for dtb, fname, _ in output_fdt_info.values():
         if dtb != node.GetFdt():
-            other_node = dtb.GetNode(node.path)
+            other_node = dtb.GetNode(fdt_path_prefix + node.path)
             if other_node:
                 yield other_node
 
diff --git a/tools/binman/test/132_replace.dts b/tools/binman/test/132_replace.dts
new file mode 100644
index 00000000000..6ebdcda45c5
--- /dev/null
+++ b/tools/binman/test/132_replace.dts
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xc00>;
+		u-boot {
+		};
+		fdtmap {
+		};
+		u-boot-dtb {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};
diff --git a/tools/binman/test/133_replace_multi.dts b/tools/binman/test/133_replace_multi.dts
new file mode 100644
index 00000000000..38b2f39d020
--- /dev/null
+++ b/tools/binman/test/133_replace_multi.dts
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		multiple-images;
+		first-image {
+			size = <0xc00>;
+			u-boot {
+			};
+			fdtmap {
+			};
+			u-boot-dtb {
+			};
+			image-header {
+				location = "end";
+			};
+		};
+
+		image {
+			fdtmap {
+			};
+			u-boot {
+			};
+			u-boot-dtb {
+			};
+		};
+	};
+};
-- 
cgit v1.2.3


From 12bb1a99c20e9c21a40ad447947c0bc898f390da Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:51 -0600
Subject: binman: Add info to allow safely repacking an image later

At present it is not possible to discover the contraints to repacking an
image (e.g. maximum section size) since this information is not preserved
from the original image description.

Add new 'orig-offset' and 'orig-size' properties to hold this. Add them to
the main device tree in the image.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/README                             | 23 ++++++++
 tools/binman/README.entries                     |  8 ++-
 tools/binman/entry.py                           | 18 ++++++-
 tools/binman/etype/fdtmap.py                    |  3 ++
 tools/binman/ftest.py                           | 70 ++++++++++++++++++++++++-
 tools/binman/image.py                           | 13 +++--
 tools/binman/state.py                           | 21 +++++---
 tools/binman/test/134_fdt_update_all_repack.dts | 23 ++++++++
 8 files changed, 165 insertions(+), 14 deletions(-)
 create mode 100644 tools/binman/test/134_fdt_update_all_repack.dts

diff --git a/tools/binman/README b/tools/binman/README
index 35223545194..6a1cd110a4c 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -481,6 +481,29 @@ name-prefix:
 	distinguish binaries with otherwise identical names.
 
 
+Image Properties
+----------------
+
+Image nodes act like sections but also have a few extra properties:
+
+filename:
+	Output filename for the image. This defaults to image.bin (or in the
+	case of multiple images <nodename>.bin where <nodename> is the name of
+	the image node.
+
+allow-repack:
+	Create an image that can be repacked. With this option it is possible
+	to change anything in the image after it is created, including updating
+	the position and size of image components. By default this is not
+	permitted since it is not possibly to know whether this might violate a
+	constraint in the image description. For example, if a section has to
+	increase in size to hold a larger binary, that might cause the section
+	to fall out of its allow region (e.g. read-only portion of flash).
+
+	Adding this property causes the original offset and size values in the
+	image description to be stored in the FDT and fdtmap.
+
+
 Entry Documentation
 -------------------
 
diff --git a/tools/binman/README.entries b/tools/binman/README.entries
index 7ce88ee5da8..37b8b4c4f98 100644
--- a/tools/binman/README.entries
+++ b/tools/binman/README.entries
@@ -230,7 +230,9 @@ Properties / Entry arguments:
     None
 
 An FDT map is just a header followed by an FDT containing a list of all the
-entries in the image.
+entries in the image. The root node corresponds to the image node in the
+original FDT, and an image-name property indicates the image name in that
+original tree.
 
 The header is the string _FDTMAP_ followed by 8 unused bytes.
 
@@ -244,6 +246,7 @@ FDT with the position of each entry.
 Example output for a simple image with U-Boot and an FDT map:
 
 / {
+    image-name = "binman";
     size = <0x00000112>;
     image-pos = <0x00000000>;
     offset = <0x00000000>;
@@ -259,6 +262,9 @@ Example output for a simple image with U-Boot and an FDT map:
     };
 };
 
+If allow-repack is used then 'orig-offset' and 'orig-size' properties are
+added as necessary. See the binman README.
+
 
 
 Entry: files: Entry containing a set of files
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 07d5838c1a2..74e280849c4 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -161,8 +161,11 @@ class Entry(object):
             self.Raise("Please use 'offset' instead of 'pos'")
         self.offset = fdt_util.GetInt(self._node, 'offset')
         self.size = fdt_util.GetInt(self._node, 'size')
-        self.orig_offset = self.offset
-        self.orig_size = self.size
+        self.orig_offset = fdt_util.GetInt(self._node, 'orig-offset')
+        self.orig_size = fdt_util.GetInt(self._node, 'orig-size')
+        if self.GetImage().copy_to_orig:
+            self.orig_offset = self.offset
+            self.orig_size = self.size
 
         # These should not be set in input files, but are set in an FDT map,
         # which is also read by this code.
@@ -207,6 +210,12 @@ class Entry(object):
         for prop in ['offset', 'size', 'image-pos']:
             if not prop in self._node.props:
                 state.AddZeroProp(self._node, prop)
+        if self.GetImage().allow_repack:
+            if self.orig_offset is not None:
+                state.AddZeroProp(self._node, 'orig-offset', True)
+            if self.orig_size is not None:
+                state.AddZeroProp(self._node, 'orig-size', True)
+
         if self.compress != 'none':
             state.AddZeroProp(self._node, 'uncomp-size')
         err = state.CheckAddHashProp(self._node)
@@ -219,6 +228,11 @@ class Entry(object):
         state.SetInt(self._node, 'size', self.size)
         base = self.section.GetRootSkipAtStart() if self.section else 0
         state.SetInt(self._node, 'image-pos', self.image_pos - base)
+        if self.GetImage().allow_repack:
+            if self.orig_offset is not None:
+                state.SetInt(self._node, 'orig-offset', self.orig_offset, True)
+            if self.orig_size is not None:
+                state.SetInt(self._node, 'orig-size', self.orig_size, True)
         if self.uncomp_size is not None:
             state.SetInt(self._node, 'uncomp-size', self.uncomp_size)
         state.CheckSetHashValue(self._node, self.GetData)
diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
index 1271b50036a..ff3f1ae8129 100644
--- a/tools/binman/etype/fdtmap.py
+++ b/tools/binman/etype/fdtmap.py
@@ -75,6 +75,9 @@ class Entry_fdtmap(Entry):
             offset = <0x00000004>;
         };
     };
+
+    If allow-repack is used then 'orig-offset' and 'orig-size' properties are
+    added as necessary. See the binman README.
     """
     def __init__(self, section, etype, node):
         Entry.__init__(self, section, etype, node)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index e201b741c6f..b67e8a15086 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -75,6 +75,9 @@ EXTRACT_DTB_SIZE = 0x3c9
 # Properties expected to be in the device tree when update_dtb is used
 BASE_DTB_PROPS = ['offset', 'size', 'image-pos']
 
+# Extra properties expected to be in the device tree when allow-repack is used
+REPACK_DTB_PROPS = ['orig-offset', 'orig-size']
+
 
 class TestFunctional(unittest.TestCase):
     """Functional tests for binman
@@ -1244,7 +1247,7 @@ class TestFunctional(unittest.TestCase):
                                                      update_dtb=True)
         dtb = fdt.Fdt(out_dtb_fname)
         dtb.Scan()
-        props = self._GetPropTree(dtb, BASE_DTB_PROPS)
+        props = self._GetPropTree(dtb, BASE_DTB_PROPS + REPACK_DTB_PROPS)
         self.assertEqual({
             'image-pos': 0,
             'offset': 0,
@@ -1587,7 +1590,8 @@ class TestFunctional(unittest.TestCase):
         for item in ['', 'spl', 'tpl']:
             dtb = fdt.Fdt.FromData(data[start:])
             dtb.Scan()
-            props = self._GetPropTree(dtb, BASE_DTB_PROPS + ['spl', 'tpl'])
+            props = self._GetPropTree(dtb, BASE_DTB_PROPS + REPACK_DTB_PROPS +
+                                      ['spl', 'tpl'])
             expected = dict(base_expected)
             if item:
                 expected[item] = 0
@@ -2821,5 +2825,67 @@ class TestFunctional(unittest.TestCase):
         # Check the state looks right.
         self.assertEqual('/binman/first-image', state.fdt_path_prefix)
 
+    def testUpdateFdtAllRepack(self):
+        """Test that all device trees are updated with offset/size info"""
+        data = self._DoReadFileRealDtb('134_fdt_update_all_repack.dts')
+        SECTION_SIZE = 0x300
+        DTB_SIZE = 602
+        FDTMAP_SIZE = 608
+        base_expected = {
+            'offset': 0,
+            'size': SECTION_SIZE + DTB_SIZE * 2 + FDTMAP_SIZE,
+            'image-pos': 0,
+            'section:offset': 0,
+            'section:size': SECTION_SIZE,
+            'section:image-pos': 0,
+            'section/u-boot-dtb:offset': 4,
+            'section/u-boot-dtb:size': 636,
+            'section/u-boot-dtb:image-pos': 4,
+            'u-boot-spl-dtb:offset': SECTION_SIZE,
+            'u-boot-spl-dtb:size': DTB_SIZE,
+            'u-boot-spl-dtb:image-pos': SECTION_SIZE,
+            'u-boot-tpl-dtb:offset': SECTION_SIZE + DTB_SIZE,
+            'u-boot-tpl-dtb:image-pos': SECTION_SIZE + DTB_SIZE,
+            'u-boot-tpl-dtb:size': DTB_SIZE,
+            'fdtmap:offset': SECTION_SIZE + DTB_SIZE * 2,
+            'fdtmap:size': FDTMAP_SIZE,
+            'fdtmap:image-pos': SECTION_SIZE + DTB_SIZE * 2,
+        }
+        main_expected = {
+            'section:orig-size': SECTION_SIZE,
+            'section/u-boot-dtb:orig-offset': 4,
+        }
+
+        # We expect three device-tree files in the output, with the first one
+        # within a fixed-size section.
+        # Read them in sequence. We look for an 'spl' property in the SPL tree,
+        # and 'tpl' in the TPL tree, to make sure they are distinct from the
+        # main U-Boot tree. All three should have the same positions and offset
+        # except that the main tree should include the main_expected properties
+        start = 4
+        for item in ['', 'spl', 'tpl', None]:
+            if item is None:
+                start += 16  # Move past fdtmap header
+            dtb = fdt.Fdt.FromData(data[start:])
+            dtb.Scan()
+            props = self._GetPropTree(dtb,
+                BASE_DTB_PROPS + REPACK_DTB_PROPS + ['spl', 'tpl'],
+                prefix='/' if item is None else '/binman/')
+            expected = dict(base_expected)
+            if item:
+                expected[item] = 0
+            else:
+                # Main DTB and fdtdec should include the 'orig-' properties
+                expected.update(main_expected)
+            # Helpful for debugging:
+            #for prop in sorted(props):
+                #print('prop %s %s %s' % (prop, props[prop], expected[prop]))
+            self.assertEqual(expected, props)
+            if item == '':
+                start = SECTION_SIZE
+            else:
+                start += dtb._fdt_obj.totalsize()
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/image.py b/tools/binman/image.py
index 5185b68990a..c81f7e3172e 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -36,19 +36,25 @@ class Image(section.Entry_section):
         image_node: Name of node containing the description for this image
         fdtmap_dtb: Fdt object for the fdtmap when loading from a file
         fdtmap_data: Contents of the fdtmap when loading from a file
+        allow_repack: True to add properties to allow the image to be safely
+            repacked later
 
     Args:
+        copy_to_orig: Copy offset/size to orig_offset/orig_size after reading
+            from the device tree
         test: True if this is being called from a test of Images. This this case
             there is no device tree defining the structure of the section, so
             we create a section manually.
     """
-    def __init__(self, name, node, test=False):
-        section.Entry_section.__init__(self, None, 'section', node, test)
+    def __init__(self, name, node, copy_to_orig=True, test=False):
+        section.Entry_section.__init__(self, None, 'section', node, test=test)
+        self.copy_to_orig = copy_to_orig
         self.name = 'main-section'
         self.image_name = name
         self._filename = '%s.bin' % self.image_name
         self.fdtmap_dtb = None
         self.fdtmap_data = None
+        self.allow_repack = False
         if not test:
             self.ReadNode()
 
@@ -57,6 +63,7 @@ class Image(section.Entry_section):
         filename = fdt_util.GetString(self._node, 'filename')
         if filename:
             self._filename = filename
+        self.allow_repack = fdt_util.GetBool(self._node, 'allow-repack')
 
     @classmethod
     def FromFile(cls, fname):
@@ -92,7 +99,7 @@ class Image(section.Entry_section):
 
         # Return an Image with the associated nodes
         root = dtb.GetRoot()
-        image = Image('image', root)
+        image = Image('image', root, copy_to_orig=False)
         image.image_node = fdt_util.GetString(root, 'image-node', 'image')
         image.fdtmap_dtb = dtb
         image.fdtmap_data = fdtmap_data
diff --git a/tools/binman/state.py b/tools/binman/state.py
index 08e627985de..2379e24ef6d 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -226,7 +226,7 @@ def GetAllFdts():
         if dtb != main_dtb:
             yield dtb
 
-def GetUpdateNodes(node):
+def GetUpdateNodes(node, for_repack=False):
     """Yield all the nodes that need to be updated in all device trees
 
     The property referenced by this node is added to any device trees which
@@ -235,25 +235,31 @@ def GetUpdateNodes(node):
 
     Args:
         node: Node object in the main device tree to look up
+        for_repack: True if we want only nodes which need 'repack' properties
+            added to them (e.g. 'orig-offset'), False to return all nodes. We
+            don't add repack properties to SPL/TPL device trees.
 
     Yields:
         Node objects in each device tree that is in use (U-Boot proper, which
             is node, SPL and TPL)
     """
     yield node
-    for dtb, fname, _ in output_fdt_info.values():
+    for dtb, fname, entry in output_fdt_info.values():
         if dtb != node.GetFdt():
+            if for_repack and entry.etype != 'u-boot-dtb':
+                continue
             other_node = dtb.GetNode(fdt_path_prefix + node.path)
             if other_node:
                 yield other_node
 
-def AddZeroProp(node, prop):
+def AddZeroProp(node, prop, for_repack=False):
     """Add a new property to affected device trees with an integer value of 0.
 
     Args:
         prop_name: Name of property
+        for_repack: True is this property is only needed for repacking
     """
-    for n in GetUpdateNodes(node):
+    for n in GetUpdateNodes(node, for_repack):
         n.AddZeroProp(prop)
 
 def AddSubnode(node, name):
@@ -283,15 +289,18 @@ def AddString(node, prop, value):
     for n in GetUpdateNodes(node):
         n.AddString(prop, value)
 
-def SetInt(node, prop, value):
+def SetInt(node, prop, value, for_repack=False):
     """Update an integer property in affected device trees with an integer value
 
     This is not allowed to change the size of the FDT.
 
     Args:
         prop_name: Name of property
+        for_repack: True is this property is only needed for repacking
     """
-    for n in GetUpdateNodes(node):
+    for n in GetUpdateNodes(node, for_repack):
+        tout.Detail("File %s: Update node '%s' prop '%s' to %#x" %
+                    (node.GetFdt().name, node.path, prop, value))
         n.SetInt(prop, value)
 
 def CheckAddHashProp(node):
diff --git a/tools/binman/test/134_fdt_update_all_repack.dts b/tools/binman/test/134_fdt_update_all_repack.dts
new file mode 100644
index 00000000000..625d37673bd
--- /dev/null
+++ b/tools/binman/test/134_fdt_update_all_repack.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		allow-repack;
+		section {
+			size = <0x300>;
+			u-boot-dtb {
+				offset = <4>;
+			};
+		};
+		u-boot-spl-dtb {
+		};
+		u-boot-tpl-dtb {
+		};
+		fdtmap {
+		};
+	};
+};
-- 
cgit v1.2.3


From 4ab88b6f2f7d857f7a998f5aae8d52af9379fb1c Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:52 -0600
Subject: binman: Update documentation for image creation

There are a few more steps in the process now. Update the documentation to
reflect this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/README | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/tools/binman/README b/tools/binman/README
index 6a1cd110a4c..1f9e13784fc 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -679,22 +679,35 @@ large enough to hold all the entries.
 7. CheckEntries() - checks that the entries do not overlap, nor extend
 outside the image.
 
-8. SetCalculatedProperties() - update any calculated properties in the device
+8. SetImagePos() - sets the image position of every entry. This is the absolute
+position 'image-pos', as opposed to 'offset' which is relative to the containing
+section. This must be done after all offsets are known, which is why it is quite
+late in the ordering.
+
+9. SetCalculatedProperties() - update any calculated properties in the device
 tree. This sets the correct 'offset' and 'size' vaues, for example.
 
-9. ProcessEntryContents() - this calls Entry.ProcessContents() on each entry.
+10. ProcessEntryContents() - this calls Entry.ProcessContents() on each entry.
 The default implementatoin does nothing. This can be overriden to adjust the
 contents of an entry in some way. For example, it would be possible to create
 an entry containing a hash of the contents of some other entries. At this
 stage the offset and size of entries should not be adjusted unless absolutely
 necessary, since it requires a repack (going back to PackEntries()).
 
-10. WriteSymbols() - write the value of symbols into the U-Boot SPL binary.
+11. ResetForPack() - if the ProcessEntryContents() step failed, in that an entry
+has changed its size, then there is no alternative but to go back to step 5 and
+try again, repacking the entries with the updated size. ResetForPack() removes
+the fixed offset/size values added by binman, so that the packing can start from
+scratch.
+
+12. WriteSymbols() - write the value of symbols into the U-Boot SPL binary.
 See 'Access to binman entry offsets at run time' below for a description of
 what happens in this stage.
 
-11. BuildImage() - builds the image and writes it to a file. This is the final
-step.
+13. BuildImage() - builds the image and writes it to a file
+
+14. WriteMap() - writes a text file containing a map of the image. This is the
+final step.
 
 
 Automatic .dtsi inclusion
-- 
cgit v1.2.3


From 96b6c506ca162b97ece5a59c0d2619173e6bfad8 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:53 -0600
Subject: binman: Write the original input fdtmap to a file

When reading an image in, write its fdtmap to a file in the output
directory. This is useful for debugging. Update the 'ls' command to set up
the output directory; otherwise it will fail.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/control.py | 6 +++++-
 tools/binman/image.py   | 5 ++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index ab94f9d4829..f9680e3948d 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -342,7 +342,11 @@ def Binman(args):
         return 0
 
     if args.cmd == 'ls':
-        ListEntries(args.image, args.paths)
+        try:
+            tools.PrepareOutputDir(None)
+            ListEntries(args.image, args.paths)
+        finally:
+            tools.FinaliseOutputDir()
         return 0
 
     if args.cmd == 'extract':
diff --git a/tools/binman/image.py b/tools/binman/image.py
index c81f7e3172e..893e8cb4cd5 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -94,7 +94,10 @@ class Image(section.Entry_section):
             data[pos + fdtmap.FDTMAP_HDR_LEN:pos + 256])
         dtb_size = probe_dtb.GetFdtObj().totalsize()
         fdtmap_data = data[pos:pos + dtb_size + fdtmap.FDTMAP_HDR_LEN]
-        dtb = fdt.Fdt.FromData(fdtmap_data[fdtmap.FDTMAP_HDR_LEN:])
+        fdt_data = fdtmap_data[fdtmap.FDTMAP_HDR_LEN:]
+        out_fname = tools.GetOutputFilename('fdtmap.in.dtb')
+        tools.WriteFile(out_fname, fdt_data)
+        dtb = fdt.Fdt.FromData(fdt_data, out_fname)
         dtb.Scan()
 
         # Return an Image with the associated nodes
-- 
cgit v1.2.3


From 7400107e467da52c7e6772b677f69f4464f6d2ce Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:54 -0600
Subject: binman: Move Image.BuildImage() into a single function

Now that an Image is an Entry_section, there is no need for the separate
BuildSection() function. Drop it and add a bit of logging.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/image.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tools/binman/image.py b/tools/binman/image.py
index 893e8cb4cd5..fd4f5044929 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -142,16 +142,14 @@ class Image(section.Entry_section):
         """Write symbol values into binary files for access at run time"""
         section.Entry_section.WriteSymbols(self, self)
 
-    def BuildSection(self, fd, base_offset):
-        """Write the section to a file"""
-        fd.seek(base_offset)
-        fd.write(self.GetData())
-
     def BuildImage(self):
         """Write the image to a file"""
         fname = tools.GetOutputFilename(self._filename)
+        tout.Info("Writing image to '%s'" % fname)
         with open(fname, 'wb') as fd:
-            self.BuildSection(fd, 0)
+            data = self.GetData()
+            fd.write(data)
+        tout.Info("Wrote %#x bytes" % len(data))
 
     def WriteMap(self):
         """Write a map of the image to a .map file
-- 
cgit v1.2.3


From eba1f0cc942947722f70029c033b915113cec1ba Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:55 -0600
Subject: binman: Add more tests for image header position

The positioning does not currently work correctly if at the end of an
image with no fixed size. Also if the header is in the middle of an image
it can cause a gap in the image since the header position is normally at
the image end, so entries after it are placed after the end of the image.

Fix these problems and add more tests to cover these cases.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/entry.py                         | 15 +++++++++++++++
 tools/binman/etype/image_header.py            | 16 ++++++++++++++--
 tools/binman/etype/section.py                 |  9 +++++++++
 tools/binman/ftest.py                         | 25 +++++++++++++++++++++++++
 tools/binman/test/135_fdtmap_hdr_middle.dts   | 16 ++++++++++++++++
 tools/binman/test/136_fdtmap_hdr_startbad.dts | 16 ++++++++++++++++
 tools/binman/test/137_fdtmap_hdr_endbad.dts   | 16 ++++++++++++++++
 tools/binman/test/138_fdtmap_hdr_nosize.dts   | 16 ++++++++++++++++
 8 files changed, 127 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/test/135_fdtmap_hdr_middle.dts
 create mode 100644 tools/binman/test/136_fdtmap_hdr_startbad.dts
 create mode 100644 tools/binman/test/137_fdtmap_hdr_endbad.dts
 create mode 100644 tools/binman/test/138_fdtmap_hdr_nosize.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 74e280849c4..8dacc61f5a1 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -745,3 +745,18 @@ features to produce new behaviours.
         ok = self.ProcessContentsUpdate(data)
         self.Detail('WriteData: size=%x, ok=%s' % (len(data), ok))
         return ok
+
+    def GetSiblingOrder(self):
+        """Get the relative order of an entry amoung its siblings
+
+        Returns:
+            'start' if this entry is first among siblings, 'end' if last,
+                otherwise None
+        """
+        entries = list(self.section.GetEntries().values())
+        if entries:
+            if self == entries[0]:
+                return 'start'
+            elif self == entries[-1]:
+                return 'end'
+        return 'middle'
diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py
index 8f9c5aa5d9e..4b69eda1a22 100644
--- a/tools/binman/etype/image_header.py
+++ b/tools/binman/etype/image_header.py
@@ -86,8 +86,20 @@ class Entry_image_header(Entry):
             if self.location not in ['start', 'end']:
                 self.Raise("Invalid location '%s', expected 'start' or 'end'" %
                            self.location)
-            image_size = self.section.GetImageSize() or 0
-            self.offset = (0 if self.location != 'end' else image_size - 8)
+            order = self.GetSiblingOrder()
+            if self.location != order and not self.section.GetSort():
+                self.Raise("Invalid sibling order '%s' for image-header: Must be at '%s' to match location" %
+                           (order, self.location))
+            if self.location != 'end':
+                offset = 0
+            else:
+                image_size = self.section.GetImageSize()
+                if image_size is None:
+                    # We don't know the image, but this must be the last entry,
+                    # so we can assume it goes
+                    offset = offset
+                else:
+                    offset = image_size - IMAGE_HEADER_LEN
         return Entry.Pack(self, offset)
 
     def ProcessContents(self):
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 0fb81419cea..3ce013d5029 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -479,3 +479,12 @@ class Entry_section(Entry):
         if not self.section:
             return self
         return self.section.GetImage()
+
+    def GetSort(self):
+        """Check if the entries in this section will be sorted
+
+        Returns:
+            True if to be sorted, False if entries will be left in the order
+                they appear in the device tree
+        """
+        return self._sort
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index b67e8a15086..bc751893edb 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2886,6 +2886,31 @@ class TestFunctional(unittest.TestCase):
             else:
                 start += dtb._fdt_obj.totalsize()
 
+    def testFdtmapHeaderMiddle(self):
+        """Test an FDT map in the middle of an image when it should be at end"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileRealDtb('135_fdtmap_hdr_middle.dts')
+        self.assertIn("Invalid sibling order 'middle' for image-header: Must be at 'end' to match location",
+                      str(e.exception))
+
+    def testFdtmapHeaderStartBad(self):
+        """Test an FDT map in middle of an image when it should be at start"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileRealDtb('136_fdtmap_hdr_startbad.dts')
+        self.assertIn("Invalid sibling order 'end' for image-header: Must be at 'start' to match location",
+                      str(e.exception))
+
+    def testFdtmapHeaderEndBad(self):
+        """Test an FDT map at the start of an image when it should be at end"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileRealDtb('137_fdtmap_hdr_endbad.dts')
+        self.assertIn("Invalid sibling order 'start' for image-header: Must be at 'end' to match location",
+                      str(e.exception))
+
+    def testFdtmapHeaderNoSize(self):
+        """Test an image header at the end of an image with undefined size"""
+        self._DoReadFileRealDtb('138_fdtmap_hdr_nosize.dts')
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/135_fdtmap_hdr_middle.dts b/tools/binman/test/135_fdtmap_hdr_middle.dts
new file mode 100644
index 00000000000..d6211da8ae3
--- /dev/null
+++ b/tools/binman/test/135_fdtmap_hdr_middle.dts
@@ -0,0 +1,16 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		image-header {
+			location = "end";
+		};
+		fdtmap {
+		};
+	};
+};
diff --git a/tools/binman/test/136_fdtmap_hdr_startbad.dts b/tools/binman/test/136_fdtmap_hdr_startbad.dts
new file mode 100644
index 00000000000..ec5f4bc7e3a
--- /dev/null
+++ b/tools/binman/test/136_fdtmap_hdr_startbad.dts
@@ -0,0 +1,16 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "start";
+		};
+	};
+};
diff --git a/tools/binman/test/137_fdtmap_hdr_endbad.dts b/tools/binman/test/137_fdtmap_hdr_endbad.dts
new file mode 100644
index 00000000000..ebacd71eb23
--- /dev/null
+++ b/tools/binman/test/137_fdtmap_hdr_endbad.dts
@@ -0,0 +1,16 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		image-header {
+			location = "end";
+		};
+		u-boot {
+		};
+		fdtmap {
+		};
+	};
+};
diff --git a/tools/binman/test/138_fdtmap_hdr_nosize.dts b/tools/binman/test/138_fdtmap_hdr_nosize.dts
new file mode 100644
index 00000000000..c362f8fdffb
--- /dev/null
+++ b/tools/binman/test/138_fdtmap_hdr_nosize.dts
@@ -0,0 +1,16 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};
-- 
cgit v1.2.3


From 51014aabc28e497eb98e0ba9c1fa0f19e871af1b Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:56 -0600
Subject: binman: Allow updating entries that change size

So far we don't allow entries to change size when repacking. But this is
not very useful since it is common for entries to change size after an
updated binary is built, etc.

Add support for this, respecting the original offset/size/alignment
constraints of the image layout. For this to work the original image
must have been created with the 'allow-repack' property.

This does not support entry types with sub-entries such as files and
CBFS, but it does support sections.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/README                      |  4 +-
 tools/binman/control.py                  | 29 +++++++++++---
 tools/binman/etype/fdtmap.py             |  9 +++--
 tools/binman/ftest.py                    | 69 +++++++++++++++++++++++++-------
 tools/binman/image.py                    |  3 +-
 tools/binman/state.py                    |  3 +-
 tools/binman/test/139_replace_repack.dts | 22 ++++++++++
 7 files changed, 113 insertions(+), 26 deletions(-)
 create mode 100644 tools/binman/test/139_replace_repack.dts

diff --git a/tools/binman/README b/tools/binman/README
index 1f9e13784fc..af2a2314719 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -589,7 +589,9 @@ that there is an 'fdtmap' entry in the image. For example:
     $ binman replace -i image.bin section/cbfs/u-boot
 
 which will write the contents of the file 'u-boot' from the current directory
-to the that entry. The file must be the same size as the entry being replaced.
+to the that entry. If the entry size changes, you must add the 'allow-repack'
+property to the original image before generating it (see above), otherwise you
+will get an error.
 
 
 Logging
diff --git a/tools/binman/control.py b/tools/binman/control.py
index f9680e3948d..c3f358d45c4 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -118,11 +118,11 @@ def ReadEntry(image_fname, entry_path, decomp=True):
     return entry.ReadData(decomp)
 
 
-def WriteEntry(image_fname, entry_path, data, decomp=True):
+def WriteEntry(image_fname, entry_path, data, decomp=True, allow_resize=True):
     """Replace an entry in an image
 
     This replaces the data in a particular entry in an image. This size of the
-    new data must match the size of the old data
+    new data must match the size of the old data unless allow_resize is True.
 
     Args:
         image_fname: Image filename to process
@@ -130,18 +130,33 @@ def WriteEntry(image_fname, entry_path, data, decomp=True):
         data: Data to replace with
         decomp: True to compress the data if needed, False if data is
             already compressed so should be used as is
+        allow_resize: True to allow entries to change size (this does a re-pack
+            of the entries), False to raise an exception
+
+    Returns:
+        Image object that was updated
     """
     tout.Info("WriteEntry '%s', file '%s'" % (entry_path, image_fname))
     image = Image.FromFile(image_fname)
     entry = image.FindEntryPath(entry_path)
     state.PrepareFromLoadedData(image)
     image.LoadData()
+
+    # If repacking, drop the old offset/size values except for the original
+    # ones, so we are only left with the constraints.
+    if allow_resize:
+        image.ResetForPack()
     tout.Info('Writing data to %s' % entry.GetPath())
     if not entry.WriteData(data, decomp):
-        entry.Raise('Entry data size does not match, but resize is disabled')
+        if not image.allow_repack:
+            entry.Raise('Entry data size does not match, but allow-repack is not present for this image')
+        if not allow_resize:
+            entry.Raise('Entry data size does not match, but resize is disabled')
     tout.Info('Processing image')
-    ProcessImage(image, update_fdt=True, write_map=False, get_contents=False)
+    ProcessImage(image, update_fdt=True, write_map=False, get_contents=False,
+                 allow_resize=allow_resize)
     tout.Info('WriteEntry done')
+    return image
 
 
 def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
@@ -264,7 +279,8 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt):
     return images
 
 
-def ProcessImage(image, update_fdt, write_map, get_contents=True):
+def ProcessImage(image, update_fdt, write_map, get_contents=True,
+                 allow_resize=True):
     """Perform all steps for this image, including checking and # writing it.
 
     This means that errors found with a later image will be reported after
@@ -277,6 +293,8 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True):
         write_map: True to write a map file
         get_contents: True to get the image contents from files, etc., False if
             the contents is already present
+        allow_resize: True to allow entries to change size (this does a re-pack
+            of the entries), False to raise an exception
     """
     if get_contents:
         image.GetEntryContents()
@@ -309,6 +327,7 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True):
             image.SetCalculatedProperties()
             for dtb_item in state.GetAllFdts():
                 dtb_item.Sync()
+                dtb_item.Flush()
         sizes_ok = image.ProcessEntryContents()
         if sizes_ok:
             break
diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py
index ff3f1ae8129..b1810b9ddb1 100644
--- a/tools/binman/etype/fdtmap.py
+++ b/tools/binman/etype/fdtmap.py
@@ -96,10 +96,10 @@ class Entry_fdtmap(Entry):
                 with fsw.add_node(subnode.name):
                     _AddNode(subnode)
 
-        outfdt = self.GetImage().fdtmap_dtb
+        data = state.GetFdtContents('fdtmap')[1]
         # If we have an fdtmap it means that we are using this as the
-        # read-only fdtmap for this image.
-        if not outfdt:
+        # fdtmap for this image.
+        if data is None:
             # Get the FDT data into an Fdt object
             data = state.GetFdtContents()[1]
             infdt = Fdt.FromData(data)
@@ -126,7 +126,8 @@ class Entry_fdtmap(Entry):
             # Pack this new FDT and return its contents
             fdt.pack()
             outfdt = Fdt.FromData(fdt.as_bytearray())
-        data = FDTMAP_MAGIC + tools.GetBytes(0, 8) + outfdt.GetContents()
+            data = outfdt.GetContents()
+        data = FDTMAP_MAGIC + tools.GetBytes(0, 8) + data
         return data
 
     def ObtainContents(self):
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index bc751893edb..64c6c0abae1 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2724,7 +2724,8 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(len(U_BOOT_DATA), entry.contents_size)
         self.assertEqual(len(U_BOOT_DATA), entry.size)
 
-    def _RunReplaceCmd(self, entry_name, data, decomp=True):
+    def _RunReplaceCmd(self, entry_name, data, decomp=True, allow_resize=True,
+                       dts='132_replace.dts'):
         """Replace an entry in an image
 
         This writes the entry data to update it, then opens the updated file and
@@ -2735,13 +2736,16 @@ class TestFunctional(unittest.TestCase):
             data: Data to replace it with
             decomp: True to compress the data if needed, False if data is
                 already compressed so should be used as is
+            allow_resize: True to allow entries to change size, False to raise
+                an exception
 
         Returns:
             Tuple:
                 data from entry
                 data from fdtmap (excluding header)
+                Image object that was modified
         """
-        dtb_data = self._DoReadFileDtb('132_replace.dts', use_real_dtb=True,
+        dtb_data = self._DoReadFileDtb(dts, use_real_dtb=True,
                                        update_dtb=True)[1]
 
         self.assertIn('image', control.images)
@@ -2753,21 +2757,24 @@ class TestFunctional(unittest.TestCase):
         image_fname = tools.GetOutputFilename('image.bin')
         updated_fname = tools.GetOutputFilename('image-updated.bin')
         tools.WriteFile(updated_fname, tools.ReadFile(image_fname))
-        control.WriteEntry(updated_fname, entry_name, data, decomp)
+        image = control.WriteEntry(updated_fname, entry_name, data, decomp,
+                                   allow_resize)
         data = control.ReadEntry(updated_fname, entry_name, decomp)
 
-        # The DT data should not change
-        new_dtb_data = entries['u-boot-dtb'].data
-        self.assertEqual(new_dtb_data, orig_dtb_data)
-        new_fdtmap_data = entries['fdtmap'].data
-        self.assertEqual(new_fdtmap_data, orig_fdtmap_data)
+        # The DT data should not change unless resized:
+        if not allow_resize:
+            new_dtb_data = entries['u-boot-dtb'].data
+            self.assertEqual(new_dtb_data, orig_dtb_data)
+            new_fdtmap_data = entries['fdtmap'].data
+            self.assertEqual(new_fdtmap_data, orig_fdtmap_data)
 
-        return data, orig_fdtmap_data[fdtmap.FDTMAP_HDR_LEN:]
+        return data, orig_fdtmap_data[fdtmap.FDTMAP_HDR_LEN:], image
 
     def testReplaceSimple(self):
         """Test replacing a single file"""
         expected = b'x' * len(U_BOOT_DATA)
-        data, expected_fdtmap = self._RunReplaceCmd('u-boot', expected)
+        data, expected_fdtmap, _ = self._RunReplaceCmd('u-boot', expected,
+                                                    allow_resize=False)
         self.assertEqual(expected, data)
 
         # Test that the state looks right. There should be an FDT for the fdtmap
@@ -2775,7 +2782,7 @@ class TestFunctional(unittest.TestCase):
         # 'control' tables. Checking for an FDT that does not exist should
         # return None.
         path, fdtmap = state.GetFdtContents('fdtmap')
-        self.assertIsNone(path)
+        self.assertIsNotNone(path)
         self.assertEqual(expected_fdtmap, fdtmap)
 
         dtb = state.GetFdtForEtype('fdtmap')
@@ -2794,7 +2801,8 @@ class TestFunctional(unittest.TestCase):
         """Test replacing a file by something larger"""
         expected = U_BOOT_DATA + b'x'
         with self.assertRaises(ValueError) as e:
-            self._RunReplaceCmd('u-boot', expected)
+            self._RunReplaceCmd('u-boot', expected, allow_resize=False,
+                                dts='139_replace_repack.dts')
         self.assertIn("Node '/u-boot': Entry data size does not match, but resize is disabled",
                       str(e.exception))
 
@@ -2806,7 +2814,8 @@ class TestFunctional(unittest.TestCase):
         updated_fname = tools.GetOutputFilename('image-updated.bin')
         tools.WriteFile(updated_fname, data)
         entry_name = 'u-boot'
-        control.WriteEntry(updated_fname, entry_name, expected)
+        control.WriteEntry(updated_fname, entry_name, expected,
+                           allow_resize=False)
         data = control.ReadEntry(updated_fname, entry_name)
         self.assertEqual(expected, data)
 
@@ -2818,7 +2827,8 @@ class TestFunctional(unittest.TestCase):
         updated_fname = tools.GetOutputFilename('first-updated.bin')
         tools.WriteFile(updated_fname, tools.ReadFile(image_fname))
         entry_name = 'u-boot'
-        control.WriteEntry(updated_fname, entry_name, expected)
+        control.WriteEntry(updated_fname, entry_name, expected,
+                           allow_resize=False)
         data = control.ReadEntry(updated_fname, entry_name)
         self.assertEqual(expected, data)
 
@@ -2911,6 +2921,37 @@ class TestFunctional(unittest.TestCase):
         """Test an image header at the end of an image with undefined size"""
         self._DoReadFileRealDtb('138_fdtmap_hdr_nosize.dts')
 
+    def testReplaceResize(self):
+        """Test replacing a single file in an entry with a larger file"""
+        expected = U_BOOT_DATA + b'x'
+        data, _, image = self._RunReplaceCmd('u-boot', expected,
+                                             dts='139_replace_repack.dts')
+        self.assertEqual(expected, data)
+
+        entries = image.GetEntries()
+        dtb_data = entries['u-boot-dtb'].data
+        dtb = fdt.Fdt.FromData(dtb_data)
+        dtb.Scan()
+
+        # The u-boot section should now be larger in the dtb
+        node = dtb.GetNode('/binman/u-boot')
+        self.assertEqual(len(expected), fdt_util.GetInt(node, 'size'))
+
+        # Same for the fdtmap
+        fdata = entries['fdtmap'].data
+        fdtb = fdt.Fdt.FromData(fdata[fdtmap.FDTMAP_HDR_LEN:])
+        fdtb.Scan()
+        fnode = fdtb.GetNode('/u-boot')
+        self.assertEqual(len(expected), fdt_util.GetInt(fnode, 'size'))
+
+    def testReplaceResizeNoRepack(self):
+        """Test replacing an entry with a larger file when not allowed"""
+        expected = U_BOOT_DATA + b'x'
+        with self.assertRaises(ValueError) as e:
+            self._RunReplaceCmd('u-boot', expected)
+        self.assertIn('Entry data size does not match, but allow-repack is not present for this image',
+                      str(e.exception))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/image.py b/tools/binman/image.py
index fd4f5044929..7b39a1ddcec 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -97,12 +97,13 @@ class Image(section.Entry_section):
         fdt_data = fdtmap_data[fdtmap.FDTMAP_HDR_LEN:]
         out_fname = tools.GetOutputFilename('fdtmap.in.dtb')
         tools.WriteFile(out_fname, fdt_data)
-        dtb = fdt.Fdt.FromData(fdt_data, out_fname)
+        dtb = fdt.Fdt(out_fname)
         dtb.Scan()
 
         # Return an Image with the associated nodes
         root = dtb.GetRoot()
         image = Image('image', root, copy_to_orig=False)
+
         image.image_node = fdt_util.GetString(root, 'image-node', 'image')
         image.fdtmap_dtb = dtb
         image.fdtmap_data = fdtmap_data
diff --git a/tools/binman/state.py b/tools/binman/state.py
index 2379e24ef6d..65536151b44 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -249,6 +249,7 @@ def GetUpdateNodes(node, for_repack=False):
             if for_repack and entry.etype != 'u-boot-dtb':
                 continue
             other_node = dtb.GetNode(fdt_path_prefix + node.path)
+            #print('   try', fdt_path_prefix + node.path, other_node)
             if other_node:
                 yield other_node
 
@@ -300,7 +301,7 @@ def SetInt(node, prop, value, for_repack=False):
     """
     for n in GetUpdateNodes(node, for_repack):
         tout.Detail("File %s: Update node '%s' prop '%s' to %#x" %
-                    (node.GetFdt().name, node.path, prop, value))
+                    (n.GetFdt().name, n.path, prop, value))
         n.SetInt(prop, value)
 
 def CheckAddHashProp(node):
diff --git a/tools/binman/test/139_replace_repack.dts b/tools/binman/test/139_replace_repack.dts
new file mode 100644
index 00000000000..a3daf6f9b46
--- /dev/null
+++ b/tools/binman/test/139_replace_repack.dts
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xc00>;
+		allow-repack;
+		u-boot {
+		};
+		fdtmap {
+		};
+		u-boot-dtb {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};
-- 
cgit v1.2.3


From 79d3c58d1268786ce40c6c0920ed2a447247fdc4 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:57 -0600
Subject: binman: Update the _testing entry to support shrinkage

Sometimes entries shrink after packing. As a start towards supporting
this, update the _testing entry to handle the test case.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/etype/_testing.py | 25 +++++++++++++++++++------
 tools/binman/ftest.py          | 16 ++++++++--------
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py
index 4a2e4eb7caf..25a6206bf33 100644
--- a/tools/binman/etype/_testing.py
+++ b/tools/binman/etype/_testing.py
@@ -31,8 +31,8 @@ class Entry__testing(Entry):
         return-invalid-entry: Return an invalid entry from GetOffsets()
         return-unknown-contents: Refuse to provide any contents (to cause a
             failure)
-        bad-update-contents: Implement ProcessContents() incorrectly so as to
-            cause a failure
+        bad-update-contents: Return a larger size in ProcessContents
+        bad-shrink-contents: Return a larger size in ProcessContents
         never-complete-process-fdt: Refund to process the FDT (to cause a
             failure)
         require-args: Require that all used args are present (generating an
@@ -51,6 +51,8 @@ class Entry__testing(Entry):
                                                      'return-unknown-contents')
         self.bad_update_contents = fdt_util.GetBool(self._node,
                                                     'bad-update-contents')
+        self.bad_shrink_contents = fdt_util.GetBool(self._node,
+                                                    'bad-shrink-contents')
         self.return_contents_once = fdt_util.GetBool(self._node,
                                                      'return-contents-once')
         self.bad_update_contents_twice = fdt_util.GetBool(self._node,
@@ -76,7 +78,7 @@ class Entry__testing(Entry):
         if self.force_bad_datatype:
             self.GetEntryArgsOrProps([EntryArg('test-bad-datatype-arg', bool)])
         self.return_contents = True
-        self.contents = b'a'
+        self.contents = b'aa'
 
     def ObtainContents(self):
         if self.return_unknown_contents or not self.return_contents:
@@ -93,14 +95,25 @@ class Entry__testing(Entry):
         return {}
 
     def ProcessContents(self):
+        data = self.contents
         if self.bad_update_contents:
             # Request to update the contents with something larger, to cause a
             # failure.
             if self.bad_update_contents_twice:
-                self.contents += b'a'
+                data = self.data + b'a'
             else:
-                self.contents = b'aa'
-            return self.ProcessContentsUpdate(self.contents)
+                data = b'aaa'
+            return self.ProcessContentsUpdate(data)
+        if self.bad_shrink_contents:
+            # Request to update the contents with something smaller, to cause a
+            # failure.
+            data = b'a'
+            return self.ProcessContentsUpdate(data)
+        if self.bad_shrink_contents:
+            # Request to update the contents with something smaller, to cause a
+            # failure.
+            data = b'a'
+            return self.ProcessContentsUpdate(data)
         return True
 
     def ProcessFdt(self, fdt):
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 64c6c0abae1..f8568d7cda1 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1236,7 +1236,7 @@ class TestFunctional(unittest.TestCase):
             state.SetAllowEntryExpansion(False)
             with self.assertRaises(ValueError) as e:
                 self._DoReadFile('059_change_size.dts', True)
-            self.assertIn("Node '/binman/_testing': Cannot update entry size from 1 to 2",
+            self.assertIn("Node '/binman/_testing': Cannot update entry size from 2 to 3",
                           str(e.exception))
         finally:
             state.SetAllowEntryExpansion(True)
@@ -1252,7 +1252,7 @@ class TestFunctional(unittest.TestCase):
             'image-pos': 0,
             'offset': 0,
             '_testing:offset': 32,
-            '_testing:size': 1,
+            '_testing:size': 2,
             '_testing:image-pos': 32,
             'section@0/u-boot:offset': 0,
             'section@0/u-boot:size': len(U_BOOT_DATA),
@@ -2135,9 +2135,9 @@ class TestFunctional(unittest.TestCase):
     def testEntryExpand(self):
         """Test expanding an entry after it is packed"""
         data = self._DoReadFile('121_entry_expand.dts')
-        self.assertEqual(b'aa', data[:2])
-        self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)])
-        self.assertEqual(b'aa', data[-2:])
+        self.assertEqual(b'aaa', data[:3])
+        self.assertEqual(U_BOOT_DATA, data[3:3 + len(U_BOOT_DATA)])
+        self.assertEqual(b'aaa', data[-3:])
 
     def testEntryExpandBad(self):
         """Test expanding an entry after it is packed, twice"""
@@ -2149,9 +2149,9 @@ class TestFunctional(unittest.TestCase):
     def testEntryExpandSection(self):
         """Test expanding an entry within a section after it is packed"""
         data = self._DoReadFile('123_entry_expand_section.dts')
-        self.assertEqual(b'aa', data[:2])
-        self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)])
-        self.assertEqual(b'aa', data[-2:])
+        self.assertEqual(b'aaa', data[:3])
+        self.assertEqual(U_BOOT_DATA, data[3:3 + len(U_BOOT_DATA)])
+        self.assertEqual(b'aaa', data[-3:])
 
     def testCompressDtb(self):
         """Test that compress of device-tree files is supported"""
-- 
cgit v1.2.3


From 61ec04f9eda413664e5c11a6099c89a44b73b5b9 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:58 -0600
Subject: binman: Support shrinking a entry after packing

Sometimes an entry may shrink after it has already been packed. In that
case we must repack the items. Of course it is always possible to just
leave the entry at its original size and waste space at the end. This is
what binman does by default, since there is the possibility of the entry
changing size every time binman calculates its contents, thus causing a
loop.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/control.py                |  2 +-
 tools/binman/entry.py                  | 28 +++++++++++++++++++---------
 tools/binman/ftest.py                  | 25 ++++++++++++++++++++++++-
 tools/binman/state.py                  | 27 +++++++++++++++++++++++++++
 tools/binman/test/140_entry_shrink.dts | 20 ++++++++++++++++++++
 5 files changed, 91 insertions(+), 11 deletions(-)
 create mode 100644 tools/binman/test/140_entry_shrink.dts

diff --git a/tools/binman/control.py b/tools/binman/control.py
index c3f358d45c4..22e3e306e54 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -333,7 +333,7 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
             break
         image.ResetForPack()
     if not sizes_ok:
-        image.Raise('Entries expanded after packing (tried %s passes)' %
+        image.Raise('Entries changed size after packing (tried %s passes)' %
                     passes)
 
     image.WriteSymbols()
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 8dacc61f5a1..90192c11b77 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -285,16 +285,26 @@ class Entry(object):
         """
         size_ok = True
         new_size = len(data)
-        if state.AllowEntryExpansion():
+        if state.AllowEntryExpansion() and new_size > self.contents_size:
+            # self.data will indicate the new size needed
+            size_ok = False
+        elif state.AllowEntryContraction() and new_size < self.contents_size:
+            size_ok = False
+
+        # If not allowed to change, try to deal with it or give up
+        if size_ok:
             if new_size > self.contents_size:
-                tout.Debug("Entry '%s' size change from %s to %s" % (
-                    self._node.path, ToHex(self.contents_size),
-                    ToHex(new_size)))
-                # self.data will indicate the new size needed
-                size_ok = False
-        elif new_size != self.contents_size:
-            self.Raise('Cannot update entry size from %d to %d' %
-                       (self.contents_size, new_size))
+                self.Raise('Cannot update entry size from %d to %d' %
+                        (self.contents_size, new_size))
+
+            # Don't let the data shrink. Pad it if necessary
+            if size_ok and new_size < self.contents_size:
+                data += tools.GetBytes(0, self.contents_size - new_size)
+
+        if not size_ok:
+            tout.Debug("Entry '%s' size change from %s to %s" % (
+                self._node.path, ToHex(self.contents_size),
+                ToHex(new_size)))
         self.SetContents(data)
         return size_ok
 
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index f8568d7cda1..11155ced709 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2143,7 +2143,7 @@ class TestFunctional(unittest.TestCase):
         """Test expanding an entry after it is packed, twice"""
         with self.assertRaises(ValueError) as e:
             self._DoReadFile('122_entry_expand_twice.dts')
-        self.assertIn("Image '/binman': Entries expanded after packing",
+        self.assertIn("Image '/binman': Entries changed size after packing",
                       str(e.exception))
 
     def testEntryExpandSection(self):
@@ -2952,6 +2952,29 @@ class TestFunctional(unittest.TestCase):
         self.assertIn('Entry data size does not match, but allow-repack is not present for this image',
                       str(e.exception))
 
+    def testEntryShrink(self):
+        """Test contracting an entry after it is packed"""
+        try:
+            state.SetAllowEntryContraction(True)
+            data = self._DoReadFileDtb('140_entry_shrink.dts',
+                                       update_dtb=True)[0]
+        finally:
+            state.SetAllowEntryContraction(False)
+        self.assertEqual(b'a', data[:1])
+        self.assertEqual(U_BOOT_DATA, data[1:1 + len(U_BOOT_DATA)])
+        self.assertEqual(b'a', data[-1:])
+
+    def testEntryShrinkFail(self):
+        """Test not being allowed to contract an entry after it is packed"""
+        data = self._DoReadFileDtb('140_entry_shrink.dts', update_dtb=True)[0]
+
+        # In this case there is a spare byte at the end of the data. The size of
+        # the contents is only 1 byte but we still have the size before it
+        # shrunk.
+        self.assertEqual(b'a\0', data[:2])
+        self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)])
+        self.assertEqual(b'a\0', data[-2:])
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/state.py b/tools/binman/state.py
index 65536151b44..f22cc82d870 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -42,6 +42,14 @@ main_dtb = None
 # Entry.ProcessContentsUpdate()
 allow_entry_expansion = True
 
+# Don't allow entries to contract after they have been packed. Instead just
+# leave some wasted space. If allowed, this is detected and forces a re-pack,
+# but may result in entries that oscillate in size, thus causing a pack error.
+# An example is a compressed device tree where the original offset values
+# result in a larger compressed size than the new ones, but then after updating
+# to the new ones, the compressed size increases, etc.
+allow_entry_contraction = False
+
 def GetFdtForEtype(etype):
     """Get the Fdt object for a particular device-tree entry
 
@@ -346,3 +354,22 @@ def AllowEntryExpansion():
             raised
     """
     return allow_entry_expansion
+
+def SetAllowEntryContraction(allow):
+    """Set whether post-pack contraction of entries is allowed
+
+    Args:
+       allow: True to allow contraction, False to raise an exception
+    """
+    global allow_entry_contraction
+
+    allow_entry_contraction = allow
+
+def AllowEntryContraction():
+    """Check whether post-pack contraction of entries is allowed
+
+    Returns:
+        True if contraction should be allowed, False if an exception should be
+            raised
+    """
+    return allow_entry_contraction
diff --git a/tools/binman/test/140_entry_shrink.dts b/tools/binman/test/140_entry_shrink.dts
new file mode 100644
index 00000000000..b750d638986
--- /dev/null
+++ b/tools/binman/test/140_entry_shrink.dts
@@ -0,0 +1,20 @@
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		_testing {
+			bad-shrink-contents;
+		};
+
+		u-boot {
+		};
+
+		_testing2 {
+			type = "_testing";
+			bad-shrink-contents;
+		};
+	};
+};
-- 
cgit v1.2.3


From 89d66907b37b2578b0e998faf3ba8ef66c6a7606 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:23:59 -0600
Subject: libfdt: Copy the struct region in fdt_resize()

At present this function appears to copy only the data before the struct
region and the data in the string region. It does not seem to copy the
struct region itself.

>From the arguments of this function it seems that it should support fdt
and buf being different. This patch attempts to fix this problem.

Upstream commit: c72fa77 libfdt: Copy the struct region in fdt_resize()

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 scripts/dtc/libfdt/fdt_sw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/dtc/libfdt/fdt_sw.c b/scripts/dtc/libfdt/fdt_sw.c
index 6d33cc29d02..d8ef748a721 100644
--- a/scripts/dtc/libfdt/fdt_sw.c
+++ b/scripts/dtc/libfdt/fdt_sw.c
@@ -114,7 +114,7 @@ int fdt_resize(void *fdt, void *buf, int bufsize)
 
 	FDT_SW_CHECK_HEADER(fdt);
 
-	headsize = fdt_off_dt_struct(fdt);
+	headsize = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
 	tailsize = fdt_size_dt_strings(fdt);
 
 	if ((headsize + tailsize) > bufsize)
-- 
cgit v1.2.3


From 95a0f3c6919e5586c23e41df46d7d41e401f13bb Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:00 -0600
Subject: binman: Adjust fmap to ignore CBFS files

The FMAP is not intended to show the files inside a CBFS. The FMAP can be
used to locate the CBFS itself, but then the CBFS must be read to find out
what is in it.

Update the FMAP to work this way and add some debugging while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/README.entries | 3 ++-
 tools/binman/etype/fmap.py  | 9 +++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/binman/README.entries b/tools/binman/README.entries
index 37b8b4c4f98..0f0e367d026 100644
--- a/tools/binman/README.entries
+++ b/tools/binman/README.entries
@@ -314,7 +314,8 @@ see www.flashrom.org/Flashrom for more information.
 
 When used, this entry will be populated with an FMAP which reflects the
 entries in the current image. Note that any hierarchy is squashed, since
-FMAP does not support this.
+FMAP does not support this. Also, CBFS entries appear as a single entry -
+the sub-entries are ignored.
 
 
 
diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py
index 56677cbac1c..835ba5012e5 100644
--- a/tools/binman/etype/fmap.py
+++ b/tools/binman/etype/fmap.py
@@ -8,6 +8,8 @@
 from entry import Entry
 import fmap_util
 import tools
+from tools import ToHexSize
+import tout
 
 
 class Entry_fmap(Entry):
@@ -26,7 +28,8 @@ class Entry_fmap(Entry):
 
     When used, this entry will be populated with an FMAP which reflects the
     entries in the current image. Note that any hierarchy is squashed, since
-    FMAP does not support this.
+    FMAP does not support this. Also, CBFS entries appear as a single entry -
+    the sub-entries are ignored.
     """
     def __init__(self, section, etype, node):
         Entry.__init__(self, section, etype, node)
@@ -39,7 +42,9 @@ class Entry_fmap(Entry):
         """
         def _AddEntries(areas, entry):
             entries = entry.GetEntries()
-            if entries:
+            tout.Debug("fmap: Add entry '%s' type '%s' (%s subentries)" %
+                       (entry.GetPath(), entry.etype, ToHexSize(entries)))
+            if entries and entry.etype != 'cbfs':
                 for subentry in entries.values():
                     _AddEntries(areas, subentry)
             else:
-- 
cgit v1.2.3


From 27145fd3a836173390c2d2adcd267fa3005b7fbe Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:01 -0600
Subject: binman: Place Intel descriptor at image start

The Intel descriptor must always appear at the start of an (x86) image,
so it is supposed to position itself there always. However there is no
explicit test for this. Add one and fix a bug introduced by the recent
change to adjust Entry to read the node in a separate call.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/etype/intel_descriptor.py      |  6 +++++-
 tools/binman/ftest.py                       |  9 +++++++++
 tools/binman/test/141_descriptor_offset.dts | 20 ++++++++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/141_descriptor_offset.dts

diff --git a/tools/binman/etype/intel_descriptor.py b/tools/binman/etype/intel_descriptor.py
index adea578080c..fb5e889ebff 100644
--- a/tools/binman/etype/intel_descriptor.py
+++ b/tools/binman/etype/intel_descriptor.py
@@ -47,8 +47,12 @@ class Entry_intel_descriptor(Entry_blob):
     def __init__(self, section, etype, node):
         Entry_blob.__init__(self, section, etype, node)
         self._regions = []
+
+    def Pack(self, offset):
+        """Put this entry at the start of the image"""
         if self.offset is None:
-            self.offset = self.section.GetStartOffset()
+            offset = self.section.GetStartOffset()
+        return Entry_blob.Pack(self, offset)
 
     def GetOffsets(self):
         offset = self.data.find(FD_SIGNATURE)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 11155ced709..d1ecd65c2c3 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2975,6 +2975,15 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(U_BOOT_DATA, data[2:2 + len(U_BOOT_DATA)])
         self.assertEqual(b'a\0', data[-2:])
 
+    def testDescriptorOffset(self):
+        """Test that the Intel descriptor is always placed at at the start"""
+        data = self._DoReadFileDtb('141_descriptor_offset.dts')
+        image = control.images['image']
+        entries = image.GetEntries()
+        desc = entries['intel-descriptor']
+        self.assertEqual(0xff800000, desc.offset);
+        self.assertEqual(0xff800000, desc.image_pos);
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/141_descriptor_offset.dts b/tools/binman/test/141_descriptor_offset.dts
new file mode 100644
index 00000000000..f9bff016aa8
--- /dev/null
+++ b/tools/binman/test/141_descriptor_offset.dts
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		sort-by-offset;
+		end-at-4gb;
+		size = <0x800000>;
+		u-boot {
+			offset = <0xffff0000>;
+		};
+		intel-descriptor {
+			filename = "descriptor.bin";
+		};
+	};
+};
-- 
cgit v1.2.3


From 513c53e4452160f51c57479f366921183ff456f7 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:02 -0600
Subject: binman: Add a few more features to the wishlist

Add mention of a few other desirable features that may be implemented in
the future.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/README | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/binman/README b/tools/binman/README
index af2a2314719..5e8f9fd4808 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -962,6 +962,8 @@ Some ideas:
 - Support updating binaries in an image (with repacking)
 - Support adding FITs to an image
 - Support for ARM Trusted Firmware (ATF)
+- Detect invalid properties in nodes
+- Sort the fdtmap by offset
 
 --
 Simon Glass <sjg@chromium.org>
-- 
cgit v1.2.3


From 17a7421ff417f21d0e3e151c992d7188ded3c0d3 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:03 -0600
Subject: binman: Add a prefix before CBFS hex offsets

Add a 0x prefix to these errors to avoid confusion.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/cbfs_util.py      | 4 ++--
 tools/binman/cbfs_util_test.py | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
index 45e16da0aaa..6d9a876ee85 100644
--- a/tools/binman/cbfs_util.py
+++ b/tools/binman/cbfs_util.py
@@ -715,7 +715,7 @@ class CbfsReader(object):
         file_pos = fd.tell()
         data = fd.read(FILE_HEADER_LEN)
         if len(data) < FILE_HEADER_LEN:
-            print('File header at %x ran out of data' % file_pos)
+            print('File header at %#x ran out of data' % file_pos)
             return False
         magic, size, ftype, attr, offset = struct.unpack(FILE_HEADER_FORMAT,
                                                          data)
@@ -724,7 +724,7 @@ class CbfsReader(object):
         pos = fd.tell()
         name = self._read_string(fd)
         if name is None:
-            print('String at %x ran out of data' % pos)
+            print('String at %#x ran out of data' % pos)
             return False
 
         if DEBUG:
diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
index 0fe4aa494ec..772c794eceb 100755
--- a/tools/binman/cbfs_util_test.py
+++ b/tools/binman/cbfs_util_test.py
@@ -372,7 +372,7 @@ class TestCbfs(unittest.TestCase):
             with io.BytesIO(newdata) as fd:
                 fd.seek(pos)
                 self.assertEqual(False, cbr._read_next_file(fd))
-        self.assertIn('File header at 0 ran out of data', stdout.getvalue())
+        self.assertIn('File header at 0x0 ran out of data', stdout.getvalue())
 
     def test_cbfs_bad_file_string(self):
         """Check handling of an incomplete filename string"""
@@ -394,7 +394,7 @@ class TestCbfs(unittest.TestCase):
             with io.BytesIO(newdata) as fd:
                 fd.seek(pos)
                 self.assertEqual(False, cbr._read_next_file(fd))
-        self.assertIn('String at %x ran out of data' %
+        self.assertIn('String at %#x ran out of data' %
                       cbfs_util.FILE_HEADER_LEN, stdout.getvalue())
 
     def test_cbfs_debug(self):
-- 
cgit v1.2.3


From a9cd39ef751efdd05a3a5ccae13e28d8a993292d Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:04 -0600
Subject: binman: Update Entry.ReadEntry() to work through classes

At present we simply extract the data directly from entries using the
image_pos information. This happens to work on current entry types, but
cannot work if the entry type encodes the data in some way. Update the
ReadData() method to provide the data by calling a new ReadChildData()
method in the parent. This allows the entry_Section class, or possibly
any other container class, to return the correct data in all cases.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/entry.py         |  7 ++-----
 tools/binman/etype/blob.py    | 12 ------------
 tools/binman/etype/cbfs.py    | 12 ++++++++++++
 tools/binman/etype/section.py | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 90192c11b77..8416214fc93 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -714,11 +714,8 @@ features to produce new behaviours.
         """
         # Use True here so that we get an uncompressed section to work from,
         # although compressed sections are currently not supported
-        data = self.section.ReadData(True)
-        tout.Info('%s: Reading data from offset %#x-%#x, size %#x (avail %#x)' %
-                  (self.GetPath(), self.offset, self.offset + self.size,
-                   self.size, len(data)))
-        return data[self.offset:self.offset + self.size]
+        data = self.section.ReadChildData(self, decomp)
+        return data
 
     def LoadData(self, decomp=True):
         data = self.ReadData(decomp)
diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
index 00cad33718c..d15d0789e52 100644
--- a/tools/binman/etype/blob.py
+++ b/tools/binman/etype/blob.py
@@ -67,15 +67,3 @@ class Entry_blob(Entry):
 
     def GetDefaultFilename(self):
         return self._filename
-
-    def ReadData(self, decomp=True):
-        indata = Entry.ReadData(self, decomp)
-        if decomp:
-            data = tools.Decompress(indata, self.compress)
-            if self.uncomp_size:
-                tout.Info("%s: Decompressing data size %#x with algo '%s' to data size %#x" %
-                          (self.GetPath(), len(indata), self.compress,
-                           len(data)))
-        else:
-            data = indata
-        return data
diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index d73c706c444..2bcdf2fd433 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -262,3 +262,15 @@ class Entry_cbfs(Entry):
 
     def GetEntries(self):
         return self._cbfs_entries
+
+    def ReadData(self, decomp=True):
+        data = Entry.ReadData(self, True)
+        return data
+
+    def ReadChildData(self, child, decomp=True):
+        if not self.reader:
+            data = Entry.ReadData(self, True)
+            self.reader = cbfs_util.CbfsReader(data)
+        reader = self.reader
+        cfile = reader.files.get(child.name)
+        return cfile.data if decomp else cfile.orig_data
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 3ce013d5029..855e291bc43 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -17,6 +17,7 @@ import sys
 from entry import Entry
 import fdt_util
 import tools
+import tout
 
 
 class Entry_section(Entry):
@@ -488,3 +489,34 @@ class Entry_section(Entry):
                 they appear in the device tree
         """
         return self._sort
+
+    def ReadData(self, decomp=True):
+        tout.Info("ReadData path='%s'" % self.GetPath())
+        parent_data = self.section.ReadData(True)
+        tout.Info('%s: Reading data from offset %#x-%#x, size %#x' %
+                  (self.GetPath(), self.offset, self.offset + self.size,
+                   self.size))
+        data = parent_data[self.offset:self.offset + self.size]
+        return data
+
+    def ReadChildData(self, child, decomp=True):
+        """Read the data for a particular child entry
+
+        Args:
+            child: Child entry to read data for
+            decomp: True to return uncompressed data, False to leave the data
+                compressed if it is compressed
+
+        Returns:
+            Data contents of entry
+        """
+        parent_data = self.ReadData(True)
+        data = parent_data[child.offset:child.offset + child.size]
+        if decomp:
+            indata = data
+            data = tools.Decompress(indata, child.compress)
+            if child.uncomp_size:
+                tout.Info("%s: Decompressing data size %#x with algo '%s' to data size %#x" %
+                            (child.GetPath(), len(indata), child.compress,
+                            len(data)))
+        return data
-- 
cgit v1.2.3


From 7210c89eac7fb68947f5f31372d9f1e93f42df0c Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:05 -0600
Subject: binman: Update Entry.WriteData() to handle special sections

At present this method assumes that the parent section does not need
to recalculate its position or adjust any metadata it may contain. But
when the entry changes size this may not be true. Also if the parent
section is more than just a container (e.g. it is a CBFS) then the
section may need to regenerate its output.

Add a new WriteChildData() method to sections and call this from the
WriteData() method, to handle this situation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/entry.py         | 21 ++++++++++++++++++++-
 tools/binman/etype/cbfs.py    |  8 ++++++--
 tools/binman/etype/section.py |  3 +++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 8416214fc93..6a2c6e0d92e 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -751,7 +751,26 @@ features to produce new behaviours.
         self.contents_size = self.size
         ok = self.ProcessContentsUpdate(data)
         self.Detail('WriteData: size=%x, ok=%s' % (len(data), ok))
-        return ok
+        section_ok = self.section.WriteChildData(self)
+        return ok and section_ok
+
+    def WriteChildData(self, child):
+        """Handle writing the data in a child entry
+
+        This should be called on the child's parent section after the child's
+        data has been updated. It
+
+        This base-class implementation does nothing, since the base Entry object
+        does not have any children.
+
+        Args:
+            child: Child Entry that was written
+
+        Returns:
+            True if the section could be updated successfully, False if the
+                data is such that the section could not updat
+        """
+        return True
 
     def GetSiblingOrder(self):
         """Get the relative order of an entry amoung its siblings
diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index 2bcdf2fd433..0109fdbb918 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -169,7 +169,7 @@ class Entry_cbfs(Entry):
         self._cbfs_entries = OrderedDict()
         self._ReadSubnodes()
 
-    def ObtainContents(self):
+    def ObtainContents(self, skip=None):
         arch = cbfs_util.find_arch(self._cbfs_arg)
         if arch is None:
             self.Raise("Invalid architecture '%s'" % self._cbfs_arg)
@@ -179,7 +179,7 @@ class Entry_cbfs(Entry):
         for entry in self._cbfs_entries.values():
             # First get the input data and put it in a file. If not available,
             # try later.
-            if not entry.ObtainContents():
+            if entry != skip and not entry.ObtainContents():
                 return False
             data = entry.GetData()
             cfile = None
@@ -274,3 +274,7 @@ class Entry_cbfs(Entry):
         reader = self.reader
         cfile = reader.files.get(child.name)
         return cfile.data if decomp else cfile.orig_data
+
+    def WriteChildData(self, child):
+        self.ObtainContents(skip=child)
+        return True
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 855e291bc43..5d34fc546af 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -520,3 +520,6 @@ class Entry_section(Entry):
                             (child.GetPath(), len(indata), child.compress,
                             len(data)))
         return data
+
+    def WriteChildData(self, child):
+        return True
-- 
cgit v1.2.3


From eb0f4a4cb40264a90a91e10e3ec00d1e0da7fb66 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:06 -0600
Subject: binman: Support replacing data in a cbfs

At present binman cannot replace data within a CBFS since it does not
allow rewriting of the files in that CBFS. Implement this by using the
new WriteData() method to handle the case.

Add a header to compressed data so that the amount of compressed data can
be determined without reference to the size of the containing entry. This
allows the entry to be larger that the contents, without causing errors in
decompression. This is necessary to cope with a compressed device tree
being updated in such a way that it shrinks after the entry size is
already set (an obscure case). It is not used with CBFS since it has its
own metadata for this. Increase the number of passes allowed to resolve
the position of entries, to handle this case.

Add a test for this new logic.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/cbfs_util.py              | 10 +++++----
 tools/binman/control.py                |  2 +-
 tools/binman/entry_test.py             |  5 +++++
 tools/binman/etype/cbfs.py             |  3 ++-
 tools/binman/ftest.py                  | 28 ++++++++++++++++++++++++-
 tools/binman/test/142_replace_cbfs.dts | 37 ++++++++++++++++++++++++++++++++++
 tools/patman/tools.py                  | 11 ++++++++--
 7 files changed, 87 insertions(+), 9 deletions(-)
 create mode 100644 tools/binman/test/142_replace_cbfs.dts

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
index 6d9a876ee85..99d77878c9a 100644
--- a/tools/binman/cbfs_util.py
+++ b/tools/binman/cbfs_util.py
@@ -208,6 +208,7 @@ class CbfsFile(object):
         cbfs_offset: Offset of file data in bytes from start of CBFS, or None to
             place this file anyway
         data: Contents of file, uncompressed
+        orig_data: Original data added to the file, possibly compressed
         data_len: Length of (possibly compressed) data in bytes
         ftype: File type (TYPE_...)
         compression: Compression type (COMPRESS_...)
@@ -226,6 +227,7 @@ class CbfsFile(object):
         self.offset = None
         self.cbfs_offset = cbfs_offset
         self.data = data
+        self.orig_data = data
         self.ftype = ftype
         self.compress = compress
         self.memlen = None
@@ -240,9 +242,9 @@ class CbfsFile(object):
         """Handle decompressing data if necessary"""
         indata = self.data
         if self.compress == COMPRESS_LZ4:
-            data = tools.Decompress(indata, 'lz4')
+            data = tools.Decompress(indata, 'lz4', with_header=False)
         elif self.compress == COMPRESS_LZMA:
-            data = tools.Decompress(indata, 'lzma')
+            data = tools.Decompress(indata, 'lzma', with_header=False)
         else:
             data = indata
         self.memlen = len(data)
@@ -361,9 +363,9 @@ class CbfsFile(object):
         elif self.ftype == TYPE_RAW:
             orig_data = data
             if self.compress == COMPRESS_LZ4:
-                data = tools.Compress(orig_data, 'lz4')
+                data = tools.Compress(orig_data, 'lz4', with_header=False)
             elif self.compress == COMPRESS_LZMA:
-                data = tools.Compress(orig_data, 'lzma')
+                data = tools.Compress(orig_data, 'lzma', with_header=False)
             self.memlen = len(orig_data)
             self.data_len = len(data)
             attr = struct.pack(ATTR_COMPRESSION_FORMAT,
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 22e3e306e54..9c8bc6253fc 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -311,7 +311,7 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
     # since changing an offset from 0x100 to 0x104 (for example) can
     # alter the compressed size of the device tree. So we need a
     # third pass for this.
-    passes = 3
+    passes = 5
     for pack_pass in range(passes):
         try:
             image.PackEntries()
diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py
index ee729f37519..cc1fb795da5 100644
--- a/tools/binman/entry_test.py
+++ b/tools/binman/entry_test.py
@@ -92,6 +92,11 @@ class TestEntry(unittest.TestCase):
         dtb = entry.Entry.Create(None, self.GetNode(), 'u-boot-dtb')
         self.assertEqual('u-boot-dtb', dtb.GetFdtEtype())
 
+    def testWriteChildData(self):
+        """Test the WriteChildData() method of the base class"""
+        base = entry.Entry.Create(None, self.GetNode(), 'blob-dtb')
+        self.assertTrue(base.WriteChildData(base))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index 0109fdbb918..28a9c81a8ad 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -168,6 +168,7 @@ class Entry_cbfs(Entry):
         self._cbfs_arg = fdt_util.GetString(node, 'cbfs-arch', 'x86')
         self._cbfs_entries = OrderedDict()
         self._ReadSubnodes()
+        self.reader = None
 
     def ObtainContents(self, skip=None):
         arch = cbfs_util.find_arch(self._cbfs_arg)
@@ -202,7 +203,7 @@ class Entry_cbfs(Entry):
     def _ReadSubnodes(self):
         """Read the subnodes to find out what should go in this IFWI"""
         for node in self._node.subnodes:
-            entry = Entry.Create(self.section, node)
+            entry = Entry.Create(self, node)
             entry.ReadNode()
             entry._cbfs_name = fdt_util.GetString(node, 'cbfs-name', entry.name)
             entry._type = fdt_util.GetString(node, 'cbfs-type')
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index d1ecd65c2c3..04bd9f886c5 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2485,7 +2485,7 @@ class TestFunctional(unittest.TestCase):
     def testExtractCbfsRaw(self):
         """Test extracting CBFS compressed data without decompressing it"""
         data = self._RunExtractCmd('section/cbfs/u-boot-dtb', decomp=False)
-        dtb = tools.Decompress(data, 'lzma')
+        dtb = tools.Decompress(data, 'lzma', with_header=False)
         self.assertEqual(EXTRACT_DTB_SIZE, len(dtb))
 
     def testExtractBadEntry(self):
@@ -2984,6 +2984,32 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(0xff800000, desc.offset);
         self.assertEqual(0xff800000, desc.image_pos);
 
+    def testReplaceCbfs(self):
+        """Test replacing a single file in CBFS without changing the size"""
+        self._CheckLz4()
+        expected = b'x' * len(U_BOOT_DATA)
+        data = self._DoReadFileRealDtb('142_replace_cbfs.dts')
+        updated_fname = tools.GetOutputFilename('image-updated.bin')
+        tools.WriteFile(updated_fname, data)
+        entry_name = 'section/cbfs/u-boot'
+        control.WriteEntry(updated_fname, entry_name, expected,
+                           allow_resize=True)
+        data = control.ReadEntry(updated_fname, entry_name)
+        self.assertEqual(expected, data)
+
+    def testReplaceResizeCbfs(self):
+        """Test replacing a single file in CBFS with one of a different size"""
+        self._CheckLz4()
+        expected = U_BOOT_DATA + b'x'
+        data = self._DoReadFileRealDtb('142_replace_cbfs.dts')
+        updated_fname = tools.GetOutputFilename('image-updated.bin')
+        tools.WriteFile(updated_fname, data)
+        entry_name = 'section/cbfs/u-boot'
+        control.WriteEntry(updated_fname, entry_name, expected,
+                           allow_resize=True)
+        data = control.ReadEntry(updated_fname, entry_name)
+        self.assertEqual(expected, data)
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/142_replace_cbfs.dts b/tools/binman/test/142_replace_cbfs.dts
new file mode 100644
index 00000000000..d64142f9d5c
--- /dev/null
+++ b/tools/binman/test/142_replace_cbfs.dts
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xe00>;
+		allow-repack;
+		u-boot {
+		};
+		section {
+			align = <0x100>;
+			cbfs {
+				size = <0x400>;
+				u-boot {
+					cbfs-type = "raw";
+				};
+				u-boot-dtb {
+					cbfs-type = "raw";
+					cbfs-compress = "lzma";
+					cbfs-offset = <0x80>;
+				};
+			};
+			u-boot-dtb {
+				compress = "lz4";
+			};
+		};
+		fdtmap {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};
diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index f492dc8f8e3..d615227482a 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -9,6 +9,7 @@ import command
 import glob
 import os
 import shutil
+import struct
 import sys
 import tempfile
 
@@ -377,7 +378,7 @@ def ToBytes(string):
         return string.encode('utf-8')
     return string
 
-def Compress(indata, algo):
+def Compress(indata, algo, with_header=True):
     """Compress some data using a given algorithm
 
     Note that for lzma this uses an old version of the algorithm, not that
@@ -408,9 +409,12 @@ def Compress(indata, algo):
         data = Run('gzip', '-c', fname, binary=True)
     else:
         raise ValueError("Unknown algorithm '%s'" % algo)
+    if with_header:
+        hdr = struct.pack('<I', len(data))
+        data = hdr + data
     return data
 
-def Decompress(indata, algo):
+def Decompress(indata, algo, with_header=True):
     """Decompress some data using a given algorithm
 
     Note that for lzma this uses an old version of the algorithm, not that
@@ -428,6 +432,9 @@ def Decompress(indata, algo):
     """
     if algo == 'none':
         return indata
+    if with_header:
+        data_len = struct.unpack('<I', indata[:4])[0]
+        indata = indata[4:4 + data_len]
     fname = GetOutputFilename('%s.decomp.tmp' % algo)
     with open(fname, 'wb') as fd:
         fd.write(indata)
-- 
cgit v1.2.3


From 313533018dc175a5552a8ccac9d6bcd780e824df Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:07 -0600
Subject: patman: Reset the output directory when it is removed

At present outdir remains set ever after the output directory has been
removed. Fix this to avoid trying to access it when it is not present.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/patman/tools.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/patman/tools.py b/tools/patman/tools.py
index d615227482a..0d4705db760 100644
--- a/tools/patman/tools.py
+++ b/tools/patman/tools.py
@@ -83,6 +83,7 @@ def FinaliseOutputDir():
     """Tidy up: delete output directory if temporary and not preserved."""
     if outdir and not preserve_outdir:
         _RemoveOutputDir()
+        outdir = None
 
 def GetOutputFilename(fname):
     """Return a filename within the output directory.
@@ -101,6 +102,7 @@ def _FinaliseForTest():
 
     if outdir:
         _RemoveOutputDir()
+        outdir = None
 
 def SetInputDirs(dirname):
     """Add a list of input directories, where input files are kept.
-- 
cgit v1.2.3


From f6e02497ae063b7939b0670153d22a7b17bf4408 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:08 -0600
Subject: binman: Update state when replacing device-tree entries

Since the state module holds references to all the device trees used by
binman, it must be updated when the device trees are updated. Add support
for this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/etype/blob_dtb.py |  9 +++++++++
 tools/binman/state.py          | 16 ++++++++++++++++
 tools/dtoc/fdt.py              |  8 ++++++++
 tools/dtoc/test_fdt.py         |  5 +++++
 4 files changed, 38 insertions(+)

diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index a3b548eef20..5b559967d78 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -53,3 +53,12 @@ class Entry_blob_dtb(Entry_blob):
         """
         fname = self.GetDefaultFilename()
         return {self.GetFdtEtype(): [self, fname]}
+
+    def WriteData(self, data, decomp=True):
+        ok = Entry_blob.WriteData(self, data, decomp)
+
+        # Update the state module, since it has the authoritative record of the
+        # device trees used. If we don't do this, then state.GetFdtContents()
+        # will still return the old contents
+        state.UpdateFdtContents(self.GetFdtEtype(), data)
+        return ok
diff --git a/tools/binman/state.py b/tools/binman/state.py
index f22cc82d870..d704ed2c7cd 100644
--- a/tools/binman/state.py
+++ b/tools/binman/state.py
@@ -108,6 +108,22 @@ def GetFdtContents(etype='u-boot-dtb'):
         data = tools.ReadFile(pathname)
     return pathname, data
 
+def UpdateFdtContents(etype, data):
+    """Update the contents of a particular device tree
+
+    The device tree is updated and written back to its file. This affects what
+    is returned from future called to GetFdtContents(), etc.
+
+    Args:
+        etype: Entry type (e.g. 'u-boot-dtb')
+        data: Data to replace the DTB with
+    """
+    dtb, fname, entry = output_fdt_info[etype]
+    dtb_fname = dtb.GetFilename()
+    tools.WriteFile(dtb_fname, data)
+    dtb = fdt.FdtScan(dtb_fname)
+    output_fdt_info[etype] = [dtb, fname, entry]
+
 def SetEntryArgs(args):
     """Set the value of the entry args
 
diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index cd7673c7da0..6770be79fbe 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -695,6 +695,14 @@ class Fdt:
         node = Node(fdt, parent, offset, name, path)
         return node
 
+    def GetFilename(self):
+        """Get the filename of the device tree
+
+        Returns:
+            String filename
+        """
+        return self._fname
+
 def FdtScan(fname):
     """Returns a new Fdt object"""
     dtb = Fdt(fname)
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index 16a4430892e..028c8cbaa80 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -449,6 +449,11 @@ class TestProp(unittest.TestCase):
         self.assertIn("node '/spl-test': Missing property 'one'",
                       str(e.exception))
 
+    def testGetFilename(self):
+        """Test the dtb filename can be provided"""
+        self.assertEqual(tools.GetOutputFilename('source.dtb'),
+                         self.dtb.GetFilename())
+
 
 class TestFdtUtil(unittest.TestCase):
     """Tests for the fdt_util module
-- 
cgit v1.2.3


From bf574f129be2e2c7193024e0efac15d6b3496534 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:09 -0600
Subject: binman: Add a test function to clean up the output dir

Put tearDown()'s logic into a new _CleanupOutputDir() function so that it
can be called from elsewhere.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/ftest.py | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 04bd9f886c5..a51865c5c7e 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -187,6 +187,13 @@ class TestFunctional(unittest.TestCase):
         if not self.have_lz4:
             self.skipTest('lz4 --no-frame-crc not available')
 
+    def _CleanupOutputDir(self):
+        """Remove the temporary output directory"""
+        if self.preserve_outdirs:
+            print('Preserving output dir: %s' % tools.outdir)
+        else:
+            tools._FinaliseForTest()
+
     def setUp(self):
         # Enable this to turn on debugging output
         # tout.Init(tout.DEBUG)
@@ -194,10 +201,7 @@ class TestFunctional(unittest.TestCase):
 
     def tearDown(self):
         """Remove the temporary output directory"""
-        if self.preserve_outdirs:
-            print('Preserving output dir: %s' % tools.outdir)
-        else:
-            tools._FinaliseForTest()
+        self._CleanupOutputDir()
 
     @classmethod
     def _ResetDtbs(self):
-- 
cgit v1.2.3


From f86a736349bd520fffb55bb9dbe3d63816780d67 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:10 -0600
Subject: binman: Clean up all output directories in tests

At present some tests leave behind output directories. This happens
because some tests call binman, which sets up an output directory, then
call it again, which sets up another output directory and leaves the
original one behind.

Fix this by using a separate temporary directory when binman is called
twice, or by manually removing the output directory.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/ftest.py | 51 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index a51865c5c7e..c0842c4386b 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -203,6 +203,28 @@ class TestFunctional(unittest.TestCase):
         """Remove the temporary output directory"""
         self._CleanupOutputDir()
 
+    def _SetupImageInTmpdir(self):
+        """Set up the output image in a new temporary directory
+
+        This is used when an image has been generated in the output directory,
+        but we want to run binman again. This will create a new output
+        directory and fail to delete the original one.
+
+        This creates a new temporary directory, copies the image to it (with a
+        new name) and removes the old output directory.
+
+        Returns:
+            Tuple:
+                Temporary directory to use
+                New image filename
+        """
+        image_fname = tools.GetOutputFilename('image.bin')
+        tmpdir = tempfile.mkdtemp(prefix='binman.')
+        updated_fname = os.path.join(tmpdir, 'image-updated.bin')
+        tools.WriteFile(updated_fname, tools.ReadFile(image_fname))
+        self._CleanupOutputDir()
+        return tmpdir, updated_fname
+
     @classmethod
     def _ResetDtbs(self):
         TestFunctional._MakeInputFile('u-boot.dtb', U_BOOT_DTB_DATA)
@@ -1563,6 +1585,7 @@ class TestFunctional(unittest.TestCase):
 
             self.assertFalse(os.path.exists(tools.GetOutputFilename('image1.bin')))
             self.assertTrue(os.path.exists(tools.GetOutputFilename('image2.bin')))
+            self._CleanupOutputDir()
 
     def testUpdateFdtAll(self):
         """Test that all device trees are updated with offset/size info"""
@@ -2364,9 +2387,12 @@ class TestFunctional(unittest.TestCase):
         fdt_size = entries['section'].GetEntries()['u-boot-dtb'].size
         fdtmap_offset = entries['fdtmap'].offset
 
-        image_fname = tools.GetOutputFilename('image.bin')
-        with test_util.capture_sys_output() as (stdout, stderr):
-            self._DoBinman('ls', '-i', image_fname)
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+            with test_util.capture_sys_output() as (stdout, stderr):
+                self._DoBinman('ls', '-i', updated_fname)
+        finally:
+            shutil.rmtree(tmpdir)
         lines = stdout.getvalue().splitlines()
         expected = [
 'Name              Image-pos  Size  Entry-type    Offset  Uncomp-size',
@@ -2387,9 +2413,12 @@ class TestFunctional(unittest.TestCase):
     def testListCmdFail(self):
         """Test failing to list an image"""
         self._DoReadFile('005_simple.dts')
-        image_fname = tools.GetOutputFilename('image.bin')
-        with self.assertRaises(ValueError) as e:
-            self._DoBinman('ls', '-i', image_fname)
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+            with self.assertRaises(ValueError) as e:
+                self._DoBinman('ls', '-i', updated_fname)
+        finally:
+            shutil.rmtree(tmpdir)
         self.assertIn("Cannot find FDT map in image", str(e.exception))
 
     def _RunListCmd(self, paths, expected):
@@ -2515,10 +2544,14 @@ class TestFunctional(unittest.TestCase):
         """Test extracting a file fron an image on the command line"""
         self._CheckLz4()
         self._DoReadFileRealDtb('130_list_fdtmap.dts')
-        image_fname = tools.GetOutputFilename('image.bin')
         fname = os.path.join(self._indir, 'output.extact')
-        with test_util.capture_sys_output() as (stdout, stderr):
-            self._DoBinman('extract', '-i', image_fname, 'u-boot', '-f', fname)
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+            with test_util.capture_sys_output() as (stdout, stderr):
+                self._DoBinman('extract', '-i', updated_fname, 'u-boot',
+                               '-f', fname)
+        finally:
+            shutil.rmtree(tmpdir)
         data = tools.ReadFile(fname)
         self.assertEqual(U_BOOT_DATA, data)
 
-- 
cgit v1.2.3


From 22a76b7428e74c4193f4356f6caeeda69628cf64 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:11 -0600
Subject: binman: Move control.WriteEntry further down the file

Move this function after the extraction logic so we can keep the writing
logic in one place.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/control.py | 81 ++++++++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 41 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index 9c8bc6253fc..23a3d558612 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -118,47 +118,6 @@ def ReadEntry(image_fname, entry_path, decomp=True):
     return entry.ReadData(decomp)
 
 
-def WriteEntry(image_fname, entry_path, data, decomp=True, allow_resize=True):
-    """Replace an entry in an image
-
-    This replaces the data in a particular entry in an image. This size of the
-    new data must match the size of the old data unless allow_resize is True.
-
-    Args:
-        image_fname: Image filename to process
-        entry_path: Path to entry to extract
-        data: Data to replace with
-        decomp: True to compress the data if needed, False if data is
-            already compressed so should be used as is
-        allow_resize: True to allow entries to change size (this does a re-pack
-            of the entries), False to raise an exception
-
-    Returns:
-        Image object that was updated
-    """
-    tout.Info("WriteEntry '%s', file '%s'" % (entry_path, image_fname))
-    image = Image.FromFile(image_fname)
-    entry = image.FindEntryPath(entry_path)
-    state.PrepareFromLoadedData(image)
-    image.LoadData()
-
-    # If repacking, drop the old offset/size values except for the original
-    # ones, so we are only left with the constraints.
-    if allow_resize:
-        image.ResetForPack()
-    tout.Info('Writing data to %s' % entry.GetPath())
-    if not entry.WriteData(data, decomp):
-        if not image.allow_repack:
-            entry.Raise('Entry data size does not match, but allow-repack is not present for this image')
-        if not allow_resize:
-            entry.Raise('Entry data size does not match, but resize is disabled')
-    tout.Info('Processing image')
-    ProcessImage(image, update_fdt=True, write_map=False, get_contents=False,
-                 allow_resize=allow_resize)
-    tout.Info('WriteEntry done')
-    return image
-
-
 def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
                    decomp=True):
     """Extract the data from one or more entries and write it to files
@@ -210,6 +169,46 @@ def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
     return einfos
 
 
+def WriteEntry(image_fname, entry_path, data, decomp=True, allow_resize=True):
+    """Replace an entry in an image
+
+    This replaces the data in a particular entry in an image. This size of the
+    new data must match the size of the old data unless allow_resize is True.
+
+    Args:
+        image_fname: Image filename to process
+        entry_path: Path to entry to extract
+        data: Data to replace with
+        decomp: True to compress the data if needed, False if data is
+            already compressed so should be used as is
+        allow_resize: True to allow entries to change size (this does a re-pack
+            of the entries), False to raise an exception
+
+    Returns:
+        Image object that was updated
+    """
+    tout.Info("WriteEntry '%s', file '%s'" % (entry_path, image_fname))
+    image = Image.FromFile(image_fname)
+    entry = image.FindEntryPath(entry_path)
+    state.PrepareFromLoadedData(image)
+    image.LoadData()
+
+    # If repacking, drop the old offset/size values except for the original
+    # ones, so we are only left with the constraints.
+    if allow_resize:
+        image.ResetForPack()
+    tout.Info('Writing data to %s' % entry.GetPath())
+    if not entry.WriteData(data, decomp):
+        if not image.allow_repack:
+            entry.Raise('Entry data size does not match, but allow-repack is not present for this image')
+        if not allow_resize:
+            entry.Raise('Entry data size does not match, but resize is disabled')
+    tout.Info('Processing image')
+    ProcessImage(image, update_fdt=True, write_map=False, get_contents=False,
+                 allow_resize=allow_resize)
+    tout.Info('WriteEntry done')
+    return image
+
 def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt):
     """Prepare the images to be processed and select the device tree
 
-- 
cgit v1.2.3


From 3ad804e6bd1d1b9d3a669a053796ae4341e235dc Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:12 -0600
Subject: binman: Update control.WriteEntry() to support writing the map

Add the ability to write a new map file. Also tidy up a few comments and
rename a misleading variable.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/control.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index 23a3d558612..9a706305c38 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -128,7 +128,7 @@ def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
             otherwise
         outdir: Output directory to use (for any number of files), else None
         entry_paths: List of entry paths to extract
-        decomp: True to compress the entry data
+        decomp: True to decompress the entry data
 
     Returns:
         List of EntryInfo records that were written
@@ -169,7 +169,8 @@ def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
     return einfos
 
 
-def WriteEntry(image_fname, entry_path, data, decomp=True, allow_resize=True):
+def WriteEntry(image_fname, entry_path, data, do_compress=True,
+               allow_resize=True, write_map=False):
     """Replace an entry in an image
 
     This replaces the data in a particular entry in an image. This size of the
@@ -179,10 +180,11 @@ def WriteEntry(image_fname, entry_path, data, decomp=True, allow_resize=True):
         image_fname: Image filename to process
         entry_path: Path to entry to extract
         data: Data to replace with
-        decomp: True to compress the data if needed, False if data is
+        do_compress: True to compress the data if needed, False if data is
             already compressed so should be used as is
         allow_resize: True to allow entries to change size (this does a re-pack
             of the entries), False to raise an exception
+        write_map: True to write a map file
 
     Returns:
         Image object that was updated
@@ -198,7 +200,7 @@ def WriteEntry(image_fname, entry_path, data, decomp=True, allow_resize=True):
     if allow_resize:
         image.ResetForPack()
     tout.Info('Writing data to %s' % entry.GetPath())
-    if not entry.WriteData(data, decomp):
+    if not entry.WriteData(data, do_compress):
         if not image.allow_repack:
             entry.Raise('Entry data size does not match, but allow-repack is not present for this image')
         if not allow_resize:
-- 
cgit v1.2.3


From d7fa4e4b22d8f493e6f643843f0a7aaf448d098a Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:13 -0600
Subject: binman: Split control.WriteEntryToImage() into separate functions

This code has three distinct phases:

1. The image is loaded and the state module is set up
2. The entry is written to the image
3. The image is repacked and written back to the file

Split the code out with three separate functions, one for each phase.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/control.py | 76 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 17 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index 9a706305c38..232a38d9847 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -169,6 +169,62 @@ def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
     return einfos
 
 
+def BeforeReplace(image, allow_resize):
+    """Handle getting an image ready for replacing entries in it
+
+    Args:
+        image: Image to prepare
+    """
+    state.PrepareFromLoadedData(image)
+    image.LoadData()
+
+    # If repacking, drop the old offset/size values except for the original
+    # ones, so we are only left with the constraints.
+    if allow_resize:
+        image.ResetForPack()
+
+
+def ReplaceOneEntry(image, entry, data, do_compress, allow_resize):
+    """Handle replacing a single entry an an image
+
+    Args:
+        image: Image to update
+        entry: Entry to write
+        data: Data to replace with
+        do_compress: True to compress the data if needed, False if data is
+            already compressed so should be used as is
+        allow_resize: True to allow entries to change size (this does a re-pack
+            of the entries), False to raise an exception
+    """
+    if not entry.WriteData(data, do_compress):
+        if not image.allow_repack:
+            entry.Raise('Entry data size does not match, but allow-repack is not present for this image')
+        if not allow_resize:
+            entry.Raise('Entry data size does not match, but resize is disabled')
+
+
+def AfterReplace(image, allow_resize, write_map):
+    """Handle write out an image after replacing entries in it
+
+    Args:
+        image: Image to write
+        allow_resize: True to allow entries to change size (this does a re-pack
+            of the entries), False to raise an exception
+        write_map: True to write a map file
+    """
+    tout.Info('Processing image')
+    ProcessImage(image, update_fdt=True, write_map=write_map,
+                 get_contents=False, allow_resize=allow_resize)
+
+
+def WriteEntryToImage(image, entry, data, do_compress=True, allow_resize=True,
+                      write_map=False):
+    BeforeReplace(image, allow_resize)
+    tout.Info('Writing data to %s' % entry.GetPath())
+    ReplaceOneEntry(image, entry, data, do_compress, allow_resize)
+    AfterReplace(image, allow_resize=allow_resize, write_map=write_map)
+
+
 def WriteEntry(image_fname, entry_path, data, do_compress=True,
                allow_resize=True, write_map=False):
     """Replace an entry in an image
@@ -189,26 +245,12 @@ def WriteEntry(image_fname, entry_path, data, do_compress=True,
     Returns:
         Image object that was updated
     """
-    tout.Info("WriteEntry '%s', file '%s'" % (entry_path, image_fname))
+    tout.Info("Write entry '%s', file '%s'" % (entry_path, image_fname))
     image = Image.FromFile(image_fname)
     entry = image.FindEntryPath(entry_path)
-    state.PrepareFromLoadedData(image)
-    image.LoadData()
+    WriteEntryToImage(image, entry, data, do_compress=do_compress,
+                      allow_resize=allow_resize, write_map=write_map)
 
-    # If repacking, drop the old offset/size values except for the original
-    # ones, so we are only left with the constraints.
-    if allow_resize:
-        image.ResetForPack()
-    tout.Info('Writing data to %s' % entry.GetPath())
-    if not entry.WriteData(data, do_compress):
-        if not image.allow_repack:
-            entry.Raise('Entry data size does not match, but allow-repack is not present for this image')
-        if not allow_resize:
-            entry.Raise('Entry data size does not match, but resize is disabled')
-    tout.Info('Processing image')
-    ProcessImage(image, update_fdt=True, write_map=False, get_contents=False,
-                 allow_resize=allow_resize)
-    tout.Info('WriteEntry done')
     return image
 
 def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt):
-- 
cgit v1.2.3


From bb5edc1d3c0ebc989dfaa7d1e57cdde15d61f2e0 Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:14 -0600
Subject: binman: Correct the error message for invalid path

At present this message references -o for output file. But binman uses -f
now. Fix it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/control.py | 4 ++--
 tools/binman/ftest.py   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py
index 232a38d9847..e6722c94593 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -138,9 +138,9 @@ def ExtractEntries(image_fname, output_fname, outdir, entry_paths,
     # Output an entry to a single file, as a special case
     if output_fname:
         if not entry_paths:
-            raise ValueError('Must specify an entry path to write with -o')
+            raise ValueError('Must specify an entry path to write with -f')
         if len(entry_paths) != 1:
-            raise ValueError('Must specify exactly one entry path to write with -o')
+            raise ValueError('Must specify exactly one entry path to write with -f')
         entry = image.FindEntryPath(entry_paths[0])
         data = entry.ReadData(decomp)
         tools.WriteFile(output_fname, data)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index c0842c4386b..358f095dfbf 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2683,7 +2683,7 @@ class TestFunctional(unittest.TestCase):
         image_fname = tools.GetOutputFilename('image.bin')
         with self.assertRaises(ValueError) as e:
             control.ExtractEntries(image_fname, 'fname', None, [])
-        self.assertIn('Must specify an entry path to write with -o',
+        self.assertIn('Must specify an entry path to write with -f',
                       str(e.exception))
 
     def testExtractTooManyEntryPaths(self):
@@ -2693,7 +2693,7 @@ class TestFunctional(unittest.TestCase):
         image_fname = tools.GetOutputFilename('image.bin')
         with self.assertRaises(ValueError) as e:
             control.ExtractEntries(image_fname, 'fname', None, ['a', 'b'])
-        self.assertIn('Must specify exactly one entry path to write with -o',
+        self.assertIn('Must specify exactly one entry path to write with -f',
                       str(e.exception))
 
     def testPackAlignSection(self):
-- 
cgit v1.2.3


From a6cb995096c40ecd6597e20d792851ac7f80004e Mon Sep 17 00:00:00 2001
From: Simon Glass <sjg@chromium.org>
Date: Sat, 20 Jul 2019 12:24:15 -0600
Subject: binman: Add command-line support for replacing entries

Add a 'replace' command to binman to permit entries to be replaced, either
individually or all at once (using a filter).

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 tools/binman/README                   |  22 +++-
 tools/binman/cmdline.py               |  17 +++
 tools/binman/control.py               |  75 ++++++++++++++
 tools/binman/ftest.py                 | 189 ++++++++++++++++++++++++++++++++++
 tools/binman/test/143_replace_all.dts |  28 +++++
 5 files changed, 327 insertions(+), 4 deletions(-)
 create mode 100644 tools/binman/test/143_replace_all.dts

diff --git a/tools/binman/README b/tools/binman/README
index 5e8f9fd4808..b4f6392ab74 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -589,9 +589,24 @@ that there is an 'fdtmap' entry in the image. For example:
     $ binman replace -i image.bin section/cbfs/u-boot
 
 which will write the contents of the file 'u-boot' from the current directory
-to the that entry. If the entry size changes, you must add the 'allow-repack'
-property to the original image before generating it (see above), otherwise you
-will get an error.
+to the that entry, compressing if necessary. If the entry size changes, you must
+add the 'allow-repack' property to the original image before generating it (see
+above), otherwise you will get an error.
+
+You can also use a particular file, in this case u-boot.bin:
+
+    $ binman replace -i image.bin section/cbfs/u-boot -f u-boot.bin
+
+It is possible to replace all files from a source directory which uses the same
+hierarchy as the entries:
+
+    $ binman replace -i image.bin -I indir
+
+Files that are missing will generate a warning.
+
+You can also replace just a selection of entries:
+
+    $ binman replace -i image.bin "*u-boot*" -I indir
 
 
 Logging
@@ -959,7 +974,6 @@ Some ideas:
 - Allow easy building of images by specifying just the board name
 - Support building an image for a board (-b) more completely, with a
   configurable build directory
-- Support updating binaries in an image (with repacking)
 - Support adding FITs to an image
 - Support for ARM Trusted Firmware (ATF)
 - Detect invalid properties in nodes
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index a43aec649ed..1e385935797 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -84,6 +84,23 @@ controlled by a description in the board device tree.'''
     extract_parser.add_argument('-U', '--uncompressed', action='store_true',
         help='Output raw uncompressed data for compressed entries')
 
+    replace_parser = subparsers.add_parser('replace',
+                                           help='Replace entries in an image')
+    replace_parser.add_argument('-C', '--compressed', action='store_true',
+        help='Input data is already compressed if needed for the entry')
+    replace_parser.add_argument('-i', '--image', type=str, required=True,
+                                help='Image filename to extract')
+    replace_parser.add_argument('-f', '--filename', type=str,
+                                help='Input filename to read from')
+    replace_parser.add_argument('-F', '--fix-size', action='store_true',
+        help="Don't allow entries to be resized")
+    replace_parser.add_argument('-I', '--indir', type=str, default='',
+        help='Path to directory to use for input files')
+    replace_parser.add_argument('-m', '--map', action='store_true',
+        default=False, help='Output a map file for the updated image')
+    replace_parser.add_argument('paths', type=str, nargs='*',
+                                help='Paths within file to extract (wildcard)')
+
     test_parser = subparsers.add_parser('test', help='Run tests')
     test_parser.add_argument('-P', '--processes', type=int,
         help='set number of processes to use for running tests')
diff --git a/tools/binman/control.py b/tools/binman/control.py
index e6722c94593..9e7587864ce 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -253,6 +253,71 @@ def WriteEntry(image_fname, entry_path, data, do_compress=True,
 
     return image
 
+
+def ReplaceEntries(image_fname, input_fname, indir, entry_paths,
+                   do_compress=True, allow_resize=True, write_map=False):
+    """Replace the data from one or more entries from input files
+
+    Args:
+        image_fname: Image filename to process
+        input_fname: Single input ilename to use if replacing one file, None
+            otherwise
+        indir: Input directory to use (for any number of files), else None
+        entry_paths: List of entry paths to extract
+        do_compress: True if the input data is uncompressed and may need to be
+            compressed if the entry requires it, False if the data is already
+            compressed.
+        write_map: True to write a map file
+
+    Returns:
+        List of EntryInfo records that were written
+    """
+    image = Image.FromFile(image_fname)
+
+    # Replace an entry from a single file, as a special case
+    if input_fname:
+        if not entry_paths:
+            raise ValueError('Must specify an entry path to read with -f')
+        if len(entry_paths) != 1:
+            raise ValueError('Must specify exactly one entry path to write with -f')
+        entry = image.FindEntryPath(entry_paths[0])
+        data = tools.ReadFile(input_fname)
+        tout.Notice("Read %#x bytes from file '%s'" % (len(data), input_fname))
+        WriteEntryToImage(image, entry, data, do_compress=do_compress,
+                          allow_resize=allow_resize, write_map=write_map)
+        return
+
+    # Otherwise we will input from a path given by the entry path of each entry.
+    # This means that files must appear in subdirectories if they are part of
+    # a sub-section.
+    einfos = image.GetListEntries(entry_paths)[0]
+    tout.Notice("Replacing %d matching entries in image '%s'" %
+                (len(einfos), image_fname))
+
+    BeforeReplace(image, allow_resize)
+
+    for einfo in einfos:
+        entry = einfo.entry
+        if entry.GetEntries():
+            tout.Info("Skipping section entry '%s'" % entry.GetPath())
+            continue
+
+        path = entry.GetPath()[1:]
+        fname = os.path.join(indir, path)
+
+        if os.path.exists(fname):
+            tout.Notice("Write entry '%s' from file '%s'" %
+                        (entry.GetPath(), fname))
+            data = tools.ReadFile(fname)
+            ReplaceOneEntry(image, entry, data, do_compress, allow_resize)
+        else:
+            tout.Warning("Skipping entry '%s' from missing file '%s'" %
+                         (entry.GetPath(), fname))
+
+    AfterReplace(image, allow_resize=allow_resize, write_map=write_map)
+    return image
+
+
 def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt):
     """Prepare the images to be processed and select the device tree
 
@@ -420,6 +485,16 @@ def Binman(args):
             tools.FinaliseOutputDir()
         return 0
 
+    if args.cmd == 'replace':
+        try:
+            tools.PrepareOutputDir(None)
+            ReplaceEntries(args.image, args.filename, args.indir, args.paths,
+                           do_compress=not args.compressed,
+                           allow_resize=not args.fix_size, write_map=args.map)
+        finally:
+            tools.FinaliseOutputDir()
+        return 0
+
     # Try to figure out which device tree contains our image description
     if args.dt:
         dtb_fname = args.dt
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 358f095dfbf..0f3b70b3bb8 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -3047,6 +3047,195 @@ class TestFunctional(unittest.TestCase):
         data = control.ReadEntry(updated_fname, entry_name)
         self.assertEqual(expected, data)
 
+    def _SetupForReplace(self):
+        """Set up some files to use to replace entries
+
+        This generates an image, copies it to a new file, extracts all the files
+        in it and updates some of them
+
+        Returns:
+            List
+                Image filename
+                Output directory
+                Expected values for updated entries, each a string
+        """
+        data = self._DoReadFileRealDtb('143_replace_all.dts')
+
+        updated_fname = tools.GetOutputFilename('image-updated.bin')
+        tools.WriteFile(updated_fname, data)
+
+        outdir = os.path.join(self._indir, 'extract')
+        einfos = control.ExtractEntries(updated_fname, None, outdir, [])
+
+        expected1 = b'x' + U_BOOT_DATA + b'y'
+        u_boot_fname1 = os.path.join(outdir, 'u-boot')
+        tools.WriteFile(u_boot_fname1, expected1)
+
+        expected2 = b'a' + U_BOOT_DATA + b'b'
+        u_boot_fname2 = os.path.join(outdir, 'u-boot2')
+        tools.WriteFile(u_boot_fname2, expected2)
+
+        expected_text = b'not the same text'
+        text_fname = os.path.join(outdir, 'text')
+        tools.WriteFile(text_fname, expected_text)
+
+        dtb_fname = os.path.join(outdir, 'u-boot-dtb')
+        dtb = fdt.FdtScan(dtb_fname)
+        node = dtb.GetNode('/binman/text')
+        node.AddString('my-property', 'the value')
+        dtb.Sync(auto_resize=True)
+        dtb.Flush()
+
+        return updated_fname, outdir, expected1, expected2, expected_text
+
+    def _CheckReplaceMultiple(self, entry_paths):
+        """Handle replacing the contents of multiple entries
+
+        Args:
+            entry_paths: List of entry paths to replace
+
+        Returns:
+            List
+                Dict of entries in the image:
+                    key: Entry name
+                    Value: Entry object
+            Expected values for updated entries, each a string
+        """
+        updated_fname, outdir, expected1, expected2, expected_text = (
+            self._SetupForReplace())
+        control.ReplaceEntries(updated_fname, None, outdir, entry_paths)
+
+        image = Image.FromFile(updated_fname)
+        image.LoadData()
+        return image.GetEntries(), expected1, expected2, expected_text
+
+    def testReplaceAll(self):
+        """Test replacing the contents of all entries"""
+        entries, expected1, expected2, expected_text = (
+            self._CheckReplaceMultiple([]))
+        data = entries['u-boot'].data
+        self.assertEqual(expected1, data)
+
+        data = entries['u-boot2'].data
+        self.assertEqual(expected2, data)
+
+        data = entries['text'].data
+        self.assertEqual(expected_text, data)
+
+        # Check that the device tree is updated
+        data = entries['u-boot-dtb'].data
+        dtb = fdt.Fdt.FromData(data)
+        dtb.Scan()
+        node = dtb.GetNode('/binman/text')
+        self.assertEqual('the value', node.props['my-property'].value)
+
+    def testReplaceSome(self):
+        """Test replacing the contents of a few entries"""
+        entries, expected1, expected2, expected_text = (
+            self._CheckReplaceMultiple(['u-boot2', 'text']))
+
+        # This one should not change
+        data = entries['u-boot'].data
+        self.assertEqual(U_BOOT_DATA, data)
+
+        data = entries['u-boot2'].data
+        self.assertEqual(expected2, data)
+
+        data = entries['text'].data
+        self.assertEqual(expected_text, data)
+
+    def testReplaceCmd(self):
+        """Test replacing a file fron an image on the command line"""
+        self._DoReadFileRealDtb('143_replace_all.dts')
+
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+
+            fname = os.path.join(tmpdir, 'update-u-boot.bin')
+            expected = b'x' * len(U_BOOT_DATA)
+            tools.WriteFile(fname, expected)
+
+            self._DoBinman('replace', '-i', updated_fname, 'u-boot', '-f', fname)
+            data = tools.ReadFile(updated_fname)
+            self.assertEqual(expected, data[:len(expected)])
+            map_fname = os.path.join(tmpdir, 'image-updated.map')
+            self.assertFalse(os.path.exists(map_fname))
+        finally:
+            shutil.rmtree(tmpdir)
+
+    def testReplaceCmdSome(self):
+        """Test replacing some files fron an image on the command line"""
+        updated_fname, outdir, expected1, expected2, expected_text = (
+            self._SetupForReplace())
+
+        self._DoBinman('replace', '-i', updated_fname, '-I', outdir,
+                       'u-boot2', 'text')
+
+        tools.PrepareOutputDir(None)
+        image = Image.FromFile(updated_fname)
+        image.LoadData()
+        entries = image.GetEntries()
+
+        # This one should not change
+        data = entries['u-boot'].data
+        self.assertEqual(U_BOOT_DATA, data)
+
+        data = entries['u-boot2'].data
+        self.assertEqual(expected2, data)
+
+        data = entries['text'].data
+        self.assertEqual(expected_text, data)
+
+    def testReplaceMissing(self):
+        """Test replacing entries where the file is missing"""
+        updated_fname, outdir, expected1, expected2, expected_text = (
+            self._SetupForReplace())
+
+        # Remove one of the files, to generate a warning
+        u_boot_fname1 = os.path.join(outdir, 'u-boot')
+        os.remove(u_boot_fname1)
+
+        with test_util.capture_sys_output() as (stdout, stderr):
+            control.ReplaceEntries(updated_fname, None, outdir, [])
+        self.assertIn("Skipping entry '/u-boot' from missing file",
+                      stdout.getvalue())
+
+    def testReplaceCmdMap(self):
+        """Test replacing a file fron an image on the command line"""
+        self._DoReadFileRealDtb('143_replace_all.dts')
+
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+
+            fname = os.path.join(self._indir, 'update-u-boot.bin')
+            expected = b'x' * len(U_BOOT_DATA)
+            tools.WriteFile(fname, expected)
+
+            self._DoBinman('replace', '-i', updated_fname, 'u-boot',
+                           '-f', fname, '-m')
+            map_fname = os.path.join(tmpdir, 'image-updated.map')
+            self.assertTrue(os.path.exists(map_fname))
+        finally:
+            shutil.rmtree(tmpdir)
+
+    def testReplaceNoEntryPaths(self):
+        """Test replacing an entry without an entry path"""
+        self._DoReadFileRealDtb('143_replace_all.dts')
+        image_fname = tools.GetOutputFilename('image.bin')
+        with self.assertRaises(ValueError) as e:
+            control.ReplaceEntries(image_fname, 'fname', None, [])
+        self.assertIn('Must specify an entry path to read with -f',
+                      str(e.exception))
+
+    def testReplaceTooManyEntryPaths(self):
+        """Test extracting some entries"""
+        self._DoReadFileRealDtb('143_replace_all.dts')
+        image_fname = tools.GetOutputFilename('image.bin')
+        with self.assertRaises(ValueError) as e:
+            control.ReplaceEntries(image_fname, 'fname', None, ['a', 'b'])
+        self.assertIn('Must specify exactly one entry path to write with -f',
+                      str(e.exception))
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/143_replace_all.dts b/tools/binman/test/143_replace_all.dts
new file mode 100644
index 00000000000..c5744a3c1c8
--- /dev/null
+++ b/tools/binman/test/143_replace_all.dts
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <0xc00>;
+		allow-repack;
+		u-boot {
+		};
+		fdtmap {
+		};
+		u-boot2 {
+			type = "u-boot";
+		};
+		text {
+			text = "some text";
+		};
+		u-boot-dtb {
+		};
+		image-header {
+			location = "end";
+		};
+	};
+};
-- 
cgit v1.2.3


From 4f4fb85ec0bfe45da11aa23ada565387ee676e80 Mon Sep 17 00:00:00 2001
From: Stephen Warren <swarren@nvidia.com>
Date: Fri, 19 Jul 2019 11:21:17 -0600
Subject: Makefile: fix implementation of BINMAN_DEBUG

binman only accepts the -D argument early on the command-line, yet the
Makefile currently passes it near the end. This causes the build to fail
if this feature is used. Re-order the command-line to fix this.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 704579bec19..5906871ef27 100644
--- a/Makefile
+++ b/Makefile
@@ -1196,9 +1196,10 @@ u-boot.ldr:	u-boot
 # ---------------------------------------------------------------------------
 # Use 'make BINMAN_DEBUG=1' to enable debugging
 quiet_cmd_binman = BINMAN  $@
-cmd_binman = $(srctree)/tools/binman/binman build -u -d u-boot.dtb -O . -m \
+cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
+                build -u -d u-boot.dtb -O . -m \
 		-I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
-		$(if $(BINMAN_DEBUG),-D) $(BINMAN_$(@F))
+		$(BINMAN_$(@F))
 
 OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex
 
-- 
cgit v1.2.3