binman: Add support for passing arguments to entries

Sometimes it is useful to pass binman the value of an entry property from
the command line. For example some entries need access to files and it is
not always convenient to put these filenames in the image definition
(device tree).

Add a -a option which can be used like this:

   -a<prop>=<value>

where

   <prop> is the property to set
   <value> is the value to set it to

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

10 files changed, 264 insertions, 4 deletions

diff --git a/tools/binman/README b/tools/binman/README
index df88819a1c..d60c7fd6a3 100644
--- a/tools/binman/README
+++ b/tools/binman/README
@@ -615,6 +615,26 @@ each entry is also shown, in bytes (hex). The indentation shows the entries
 nested inside their sections.
 
 
+Passing command-line arguments to entries
+-----------------------------------------
+
+Sometimes it is useful to pass binman the value of an entry property from the
+command line. For example some entries need access to files and it is not
+always convenient to put these filenames in the image definition (device tree).
+
+The-a option supports this:
+
+    -a<prop>=<value>
+
+where
+
+    <prop> is the property to set
+    <value> is the value to set it to
+
+Not all properties can be provided this way. Only some entries support it,
+typically for filenames.
+
+
 Code coverage
 -------------
 
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index 5c9b4dfead..54e4fb13dc 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -18,6 +18,8 @@ def ParseArgs(argv):
             args is a list of string arguments
     """
     parser = OptionParser()
+    parser.add_option('-a', '--entry-arg', type='string', action='append',
+            help='Set argument value arg=value')
     parser.add_option('-b', '--board', type='string',
             help='Board name to build')
     parser.add_option('-B', '--build-dir', type='string', default='b',
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 9ac392b7e5..ab894a8aa8 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -7,6 +7,7 @@
 
 from collections import OrderedDict
 import os
+import re
 import sys
 import tools
 
@@ -25,6 +26,9 @@ images = OrderedDict()
 # 'u-boot-spl.dtb')
 fdt_files = {}
 
+# Arguments passed to binman to provide arguments to entries
+entry_args = {}
+
 
 def _ReadImageDesc(binman_node):
     """Read the image descriptions from the /binman node
@@ -76,6 +80,20 @@ def GetFdt(fname):
 def GetFdtPath(fname):
     return fdt_files[fname]._fname
 
+def SetEntryArgs(args):
+    global entry_args
+
+    entry_args = {}
+    if args:
+        for arg in args:
+            m = re.match('([^=]*)=(.*)', arg)
+            if not m:
+                raise ValueError("Invalid entry arguemnt '%s'" % arg)
+            entry_args[m.group(1)] = m.group(2)
+
+def GetEntryArg(name):
+    return entry_args.get(name)
+
 def Binman(options, args):
     """The main control code for binman
 
@@ -116,6 +134,7 @@ def Binman(options, args):
         try:
             tools.SetInputDirs(options.indir)
             tools.PrepareOutputDir(options.outdir, options.preserve)
+            SetEntryArgs(options.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
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 8004918eb5..de07f27215 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -6,6 +6,8 @@
 
 from __future__ import print_function
 
+from collections import namedtuple
+
 # importlib was introduced in Python 2.7 but there was a report of it not
 # working in 2.7.12, so we work around this:
 # http://lists.denx.de/pipermail/u-boot/2016-October/269729.html
@@ -16,6 +18,7 @@ except:
     have_importlib = False
 
 import fdt_util
+import control
 import os
 import sys
 import tools
@@ -24,6 +27,12 @@ modules = {}
 
 our_path = os.path.dirname(os.path.realpath(__file__))
 
+
+# An argument which can be passed to entries on the command line, in lieu of
+# device-tree properties.
+EntryArg = namedtuple('EntryArg', ['name', 'datatype'])
+
+
 class Entry(object):
     """An Entry in the section
 
@@ -249,6 +258,33 @@ class Entry(object):
         """Convenience function to raise an error referencing a node"""
         raise ValueError("Node '%s': %s" % (self._node.path, msg))
 
+    def GetEntryArgsOrProps(self, props, required=False):
+        """Return the values of a set of properties
+
+        Args:
+            props: List of EntryArg objects
+
+        Raises:
+            ValueError if a property is not found
+        """
+        values = []
+        missing = []
+        for prop in props:
+            python_prop = prop.name.replace('-', '_')
+            if hasattr(self, python_prop):
+                value = getattr(self, python_prop)
+            else:
+                value = None
+            if value is None:
+                value = self.GetArg(prop.name, prop.datatype)
+            if value is None and required:
+                missing.append(prop.name)
+            values.append(value)
+        if missing:
+            self.Raise('Missing required properties/entry args: %s' %
+                       (', '.join(missing)))
+        return values
+
     def GetPath(self):
         """Get the path of a node
 
@@ -307,3 +343,36 @@ class Entry(object):
             indent: Curent indent level of map (0=none, 1=one level, etc.)
         """
         self.WriteMapLine(fd, indent, self.name, self.offset, self.size)
+
+    def GetArg(self, name, datatype=str):
+        """Get the value of an entry argument or device-tree-node property
+
+        Some node properties can be provided as arguments to binman. First check
+        the entry arguments, and fall back to the device tree if not found
+
+        Args:
+            name: Argument name
+            datatype: Data type (str or int)
+
+        Returns:
+            Value of argument as a string or int, or None if no value
+
+        Raises:
+            ValueError if the argument cannot be converted to in
+        """
+        value = control.GetEntryArg(name)
+        if value is not None:
+            if datatype == int:
+                try:
+                    value = int(value)
+                except ValueError:
+                    self.Raise("Cannot convert entry arg '%s' (value '%s') to integer" %
+                               (name, value))
+            elif datatype == str:
+                pass
+            else:
+                raise ValueError("GetArg() internal error: Unknown data type '%s'" %
+                                 datatype)
+        else:
+            value = fdt_util.GetDatatype(self._node, name, datatype)
+        return value
diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py
index 31f625c026..3eeec72b36 100644
--- a/tools/binman/etype/_testing.py
+++ b/tools/binman/etype/_testing.py
@@ -5,7 +5,9 @@
 # Entry-type module for testing purposes. Not used in real images.
 #
 
-from entry import Entry
+from collections import OrderedDict
+
+from entry import Entry, EntryArg
 import fdt_util
 import tools
 
@@ -27,6 +29,21 @@ class Entry__testing(Entry):
         self.process_fdt_ready = False
         self.never_complete_process_fdt = fdt_util.GetBool(self._node,
                                                 'never-complete-process-fdt')
+        self.require_args = fdt_util.GetBool(self._node, 'require-args')
+
+        # This should be picked up by GetEntryArgsOrProps()
+        self.test_existing_prop = 'existing'
+        self.force_bad_datatype = fdt_util.GetBool(self._node,
+                                                   'force-bad-datatype')
+        (self.test_str_fdt, self.test_str_arg, self.test_int_fdt,
+         self.test_int_arg, existing) = self.GetEntryArgsOrProps([
+            EntryArg('test-str-fdt', str),
+            EntryArg('test-str-arg', str),
+            EntryArg('test-int-fdt', int),
+            EntryArg('test-int-arg', int),
+            EntryArg('test-existing-prop', str)], self.require_args)
+        if self.force_bad_datatype:
+            self.GetEntryArgsOrProps([EntryArg('test-bad-datatype-arg', bool)])
 
     def ObtainContents(self):
         if self.return_unknown_contents:
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 94a50aac16..c54cd12e71 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -146,7 +146,8 @@ class TestFunctional(unittest.TestCase):
         # options.verbosity = tout.DEBUG
         return control.Binman(options, args)
 
-    def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False):
+    def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False,
+                    entry_args=None):
         """Run binman with a given test file
 
         Args:
@@ -163,6 +164,9 @@ class TestFunctional(unittest.TestCase):
             args.append('-m')
         if update_dtb:
             args.append('-up')
+        if entry_args:
+            for arg, value in entry_args.iteritems():
+                args.append('-a%s=%s' % (arg, value))
         return self._DoBinman(*args)
 
     def _SetupDtb(self, fname, outfile='u-boot.dtb'):
@@ -188,7 +192,7 @@ class TestFunctional(unittest.TestCase):
             return data
 
     def _DoReadFileDtb(self, fname, use_real_dtb=False, map=False,
-                       update_dtb=False):
+                       update_dtb=False, entry_args=None):
         """Run binman and return the resulting image
 
         This runs binman with a given test file and then reads the resulting
@@ -220,7 +224,8 @@ class TestFunctional(unittest.TestCase):
             dtb_data = self._SetupDtb(fname)
 
         try:
-            retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb)
+            retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb,
+                                       entry_args=entry_args)
             self.assertEqual(0, retcode)
             out_dtb_fname = control.GetFdtPath('u-boot.dtb')
 
@@ -1085,5 +1090,77 @@ class TestFunctional(unittest.TestCase):
         self.assertIn('Could not complete processing of Fdt: remaining '
                       '[<_testing.Entry__testing', str(e.exception))
 
+    def testEntryArgs(self):
+        """Test passing arguments to entries from the command line"""
+        entry_args = {
+            'test-str-arg': 'test1',
+            'test-int-arg': '456',
+        }
+        self._DoReadFileDtb('62_entry_args.dts', entry_args=entry_args)
+        self.assertIn('image', control.images)
+        entry = control.images['image'].GetEntries()['_testing']
+        self.assertEqual('test0', entry.test_str_fdt)
+        self.assertEqual('test1', entry.test_str_arg)
+        self.assertEqual(123, entry.test_int_fdt)
+        self.assertEqual(456, entry.test_int_arg)
+
+    def testEntryArgsMissing(self):
+        """Test missing arguments and properties"""
+        entry_args = {
+            'test-int-arg': '456',
+        }
+        self._DoReadFileDtb('63_entry_args_missing.dts', entry_args=entry_args)
+        entry = control.images['image'].GetEntries()['_testing']
+        self.assertEqual('test0', entry.test_str_fdt)
+        self.assertEqual(None, entry.test_str_arg)
+        self.assertEqual(None, entry.test_int_fdt)
+        self.assertEqual(456, entry.test_int_arg)
+
+    def testEntryArgsRequired(self):
+        """Test missing arguments and properties"""
+        entry_args = {
+            'test-int-arg': '456',
+        }
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileDtb('64_entry_args_required.dts')
+        self.assertIn("Node '/binman/_testing': Missing required "
+            'properties/entry args: test-str-arg, test-int-fdt, test-int-arg',
+            str(e.exception))
+
+    def testEntryArgsInvalidFormat(self):
+        """Test that an invalid entry-argument format is detected"""
+        args = ['-d', self.TestFile('64_entry_args_required.dts'), '-ano-value']
+        with self.assertRaises(ValueError) as e:
+            self._DoBinman(*args)
+        self.assertIn("Invalid entry arguemnt 'no-value'", str(e.exception))
+
+    def testEntryArgsInvalidInteger(self):
+        """Test that an invalid entry-argument integer is detected"""
+        entry_args = {
+            'test-int-arg': 'abc',
+        }
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileDtb('62_entry_args.dts', entry_args=entry_args)
+        self.assertIn("Node '/binman/_testing': Cannot convert entry arg "
+                      "'test-int-arg' (value 'abc') to integer",
+            str(e.exception))
+
+    def testEntryArgsInvalidDatatype(self):
+        """Test that an invalid entry-argument datatype is detected
+
+        This test could be written in entry_test.py except that it needs
+        access to control.entry_args, which seems more than that module should
+        be able to see.
+        """
+        entry_args = {
+            'test-bad-datatype-arg': '12',
+        }
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFileDtb('65_entry_args_unknown_datatype.dts',
+                                entry_args=entry_args)
+        self.assertIn('GetArg() internal error: Unknown data type ',
+                      str(e.exception))
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/62_entry_args.dts b/tools/binman/test/62_entry_args.dts
new file mode 100644
index 0000000000..4d4f102d60
--- /dev/null
+++ b/tools/binman/test/62_entry_args.dts
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		_testing {
+			test-str-fdt = "test0";
+			test-int-fdt = <123>;
+		};
+	};
+};
diff --git a/tools/binman/test/63_entry_args_missing.dts b/tools/binman/test/63_entry_args_missing.dts
new file mode 100644
index 0000000000..1644e2fef3
--- /dev/null
+++ b/tools/binman/test/63_entry_args_missing.dts
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		_testing {
+			test-str-fdt = "test0";
+		};
+	};
+};
diff --git a/tools/binman/test/64_entry_args_required.dts b/tools/binman/test/64_entry_args_required.dts
new file mode 100644
index 0000000000..705be10069
--- /dev/null
+++ b/tools/binman/test/64_entry_args_required.dts
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		_testing {
+			require-args;
+			test-str-fdt = "test0";
+		};
+	};
+};
diff --git a/tools/binman/test/65_entry_args_unknown_datatype.dts b/tools/binman/test/65_entry_args_unknown_datatype.dts
new file mode 100644
index 0000000000..3e4838f4ff
--- /dev/null
+++ b/tools/binman/test/65_entry_args_unknown_datatype.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		_testing {
+			test-str-fdt = "test0";
+			test-int-fdt = <123>;
+			force-bad-datatype;
+		};
+	};
+};