From 34967c2618439ef80cc76b4c43c3da4f66f12ffe Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:06 -0600 Subject: [PATCH 01/46] fdt: Add Python support for adding/removing nodes Pull this support from these upstream commits: bfbfab0 pylibfdt: Add a means to add and delete notes 9005f41 pylibfdt: Allow delprop() to return errors Signed-off-by: Simon Glass --- scripts/dtc/pylibfdt/libfdt.i_shipped | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/scripts/dtc/pylibfdt/libfdt.i_shipped b/scripts/dtc/pylibfdt/libfdt.i_shipped index 0f17c58..76e61e9 100644 --- a/scripts/dtc/pylibfdt/libfdt.i_shipped +++ b/scripts/dtc/pylibfdt/libfdt.i_shipped @@ -628,28 +628,50 @@ class Fdt(FdtRo): return check_err(fdt_setprop(self._fdt, nodeoffset, prop_name, val, len(val)), quiet) - def delprop(self, nodeoffset, prop_name): + def delprop(self, nodeoffset, prop_name, quiet=()): """Delete a property from a node Args: nodeoffset: Node offset containing property to delete prop_name: Name of property to delete + quiet: Errors to ignore (empty to raise on all errors) + + Returns: + Error code, or 0 if OK Raises: FdtError if the property does not exist, or another error occurs """ - return check_err(fdt_delprop(self._fdt, nodeoffset, prop_name)) + return check_err(fdt_delprop(self._fdt, nodeoffset, prop_name), quiet) + + def add_subnode(self, parentoffset, name, quiet=()): + """Add a new subnode to a node - def del_node(self, nodeoffset): + Args: + parentoffset: Parent offset to add the subnode to + name: Name of node to add + + Returns: + offset of the node created, or negative error code on failure + + Raises: + FdtError if there is not enough space, or another error occurs + """ + return check_err(fdt_add_subnode(self._fdt, parentoffset, name), quiet) + + def del_node(self, nodeoffset, quiet=()): """Delete a node Args: - nodeoffset: Node offset containing property to delete + nodeoffset: Offset of node to delete + + Returns: + Error code, or 0 if OK Raises: - FdtError if the node does not exist, or another error occurs + FdtError if an error occurs """ - return check_err(fdt_del_node(self._fdt, nodeoffset)) + return check_err(fdt_del_node(self._fdt, nodeoffset), quiet) class Property(bytearray): From f069303852bdfa1303cd5787667bd7924f0482d9 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:07 -0600 Subject: [PATCH 02/46] binman: Move 'special properties' docs to README.entries This information should be in the entry it relates to, not in the main README. Move it. Signed-off-by: Simon Glass --- tools/binman/README | 11 ----------- tools/binman/README.entries | 5 ++++- tools/binman/etype/u_boot_with_ucode_ptr.py | 3 +++ 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/tools/binman/README b/tools/binman/README index 9d9d183..10dfe57 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -462,17 +462,6 @@ see README.entries. This is generated from the source code using: binman -E >tools/binman/README.entries -Special properties ------------------- - -Some entries support special properties, documented here: - -u-boot-with-ucode-ptr: - optional-ucode: boolean property to make microcode optional. If the - u-boot.bin image does not include microcode, no error will - be generated. - - Order of image creation ----------------------- diff --git a/tools/binman/README.entries b/tools/binman/README.entries index c6e7b22..041e777 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -524,6 +524,9 @@ Entry: u-boot-with-ucode-ptr: U-Boot with embedded microcode pointer Properties / Entry arguments: - filename: Filename of u-boot-nodtb.dtb (default 'u-boot-nodtb.dtb') + - optional-ucode: boolean property to make microcode optional. If the + u-boot.bin image does not include microcode, no error will + be generated. See Entry_u_boot_ucode for full details of the three entries involved in this process. This entry updates U-Boot with the offset and size of the @@ -543,7 +546,7 @@ Properties / Entry arguments: - kernelkey: Name of the kernel key to use (inside keydir) - preamble-flags: Value of the vboot preamble flags (typically 0) -Chromium OS signs the read-write firmware and kernel, writing the signature +Chromium OS signs the read-write firmware and kernel, writing the signature in this block. This allows U-Boot to verify that the next firmware stage and kernel are genuine. diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 51e7ba4..c850b59 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -19,6 +19,9 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): Properties / Entry arguments: - filename: Filename of u-boot-nodtb.dtb (default 'u-boot-nodtb.dtb') + - optional-ucode: boolean property to make microcode optional. If the + u-boot.bin image does not include microcode, no error will + be generated. See Entry_u_boot_ucode for full details of the three entries involved in this process. This entry updates U-Boot with the offset and size of the From d178eab8f92cb2d67288e01d1ae5b0eb8d0f8db1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:08 -0600 Subject: [PATCH 03/46] binman: Allow 'fill' entry to have a size of 0 The check for this should be for None, not 0. Fix it and add a test. Signed-off-by: Simon Glass --- tools/binman/etype/fill.py | 2 +- tools/binman/ftest.py | 5 +++++ tools/binman/test/80_fill_empty.dts | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 tools/binman/test/80_fill_empty.dts diff --git a/tools/binman/etype/fill.py b/tools/binman/etype/fill.py index 7210a83..dcfe978 100644 --- a/tools/binman/etype/fill.py +++ b/tools/binman/etype/fill.py @@ -23,7 +23,7 @@ class Entry_fill(Entry): """ def __init__(self, section, etype, node): Entry.__init__(self, section, etype, node) - if not self.size: + 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/ftest.py b/tools/binman/ftest.py index a8456c2..7f82264 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1364,6 +1364,11 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/u-boot': Please use 'offset' instead of " "'pos'", str(e.exception)) + def testFillZero(self): + """Test for an fill entry type with a size of 0""" + data = self._DoReadFile('80_fill_empty.dts') + self.assertEqual(chr(0) * 16, data) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/80_fill_empty.dts b/tools/binman/test/80_fill_empty.dts new file mode 100644 index 0000000..2b78d3a --- /dev/null +++ b/tools/binman/test/80_fill_empty.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + fill { + size = <0>; + fill-byte = [ff]; + }; + }; +}; From 0b489364f90d5cd70b594268442b14e0143c89b5 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:09 -0600 Subject: [PATCH 04/46] binman: Generate an error when text is not provided When the value of a text entry is not provided an execption is generated talking about a None type. This is confusing. Add a more explanatory error and a test for this case. Signed-off-by: Simon Glass --- tools/binman/etype/text.py | 3 +++ tools/binman/ftest.py | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/tools/binman/etype/text.py b/tools/binman/etype/text.py index 7a1cddf..6e99819 100644 --- a/tools/binman/etype/text.py +++ b/tools/binman/etype/text.py @@ -51,6 +51,9 @@ class Entry_text(Entry): self.text_label, = self.GetEntryArgsOrProps( [EntryArg('text-label', str)]) self.value, = self.GetEntryArgsOrProps([EntryArg(self.text_label, str)]) + if not self.value: + self.Raise("No value provided for text label '%s'" % + self.text_label) def ObtainContents(self): self.SetContents(self.value) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 7f82264..d956bd4 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1369,6 +1369,13 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('80_fill_empty.dts') self.assertEqual(chr(0) * 16, data) + def testTextMissing(self): + """Test for a text entry type where there is no text""" + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('66_text.dts',) + self.assertIn("Node '/binman/text': No value provided for text label " + "'test-id'", str(e.exception)) + if __name__ == "__main__": unittest.main() From 35b384cbe5d40e618391cc076409e89cedf9c863 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:10 -0600 Subject: [PATCH 05/46] binman: Add x86 support for starting TPL Sometimes we want to include TPL for x86 platforms, such as when we want to select between different SPL images (e.g. for Chrome OS verified boot). Add support for this. Signed-off-by: Simon Glass --- tools/binman/README.entries | 17 +++++++++++++++++ tools/binman/etype/x86_start16_tpl.py | 30 ++++++++++++++++++++++++++++++ tools/binman/ftest.py | 10 +++++++++- tools/binman/test/81_x86-start16-tpl.dts | 14 ++++++++++++++ 4 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 tools/binman/etype/x86_start16_tpl.py create mode 100644 tools/binman/test/81_x86-start16-tpl.dts diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 041e777..31bc725 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -586,3 +586,20 @@ For 32-bit U-Boot, the 'x86_start16' entry type is used instead. +Entry: x86-start16-tpl: x86 16-bit start-up code for TPL +-------------------------------------------------------- + +Properties / Entry arguments: + - filename: Filename of tpl/u-boot-x86-16bit-tpl.bin (default + 'tpl/u-boot-x86-16bit-tpl.bin') + +x86 CPUs start up in 16-bit mode, even if they are 64-bit CPUs. This code +must be placed at a particular address. This entry holds that code. It is +typically placed at offset CONFIG_SYS_X86_START16. The code is responsible +for changing to 32-bit mode and starting TPL, which in turn jumps to SPL. + +If TPL is not being used, the 'x86_start16_spl or 'x86_start16' entry types +may be used instead. + + + diff --git a/tools/binman/etype/x86_start16_tpl.py b/tools/binman/etype/x86_start16_tpl.py new file mode 100644 index 0000000..46ce169 --- /dev/null +++ b/tools/binman/etype/x86_start16_tpl.py @@ -0,0 +1,30 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass +# +# Entry-type module for the 16-bit x86 start-up code for U-Boot TPL +# + +from entry import Entry +from blob import Entry_blob + +class Entry_x86_start16_tpl(Entry_blob): + """x86 16-bit start-up code for TPL + + Properties / Entry arguments: + - filename: Filename of tpl/u-boot-x86-16bit-tpl.bin (default + 'tpl/u-boot-x86-16bit-tpl.bin') + + x86 CPUs start up in 16-bit mode, even if they are 64-bit CPUs. This code + must be placed at a particular address. This entry holds that code. It is + typically placed at offset CONFIG_SYS_X86_START16. The code is responsible + for changing to 32-bit mode and starting TPL, which in turn jumps to SPL. + + If TPL is not being used, the 'x86_start16_spl or 'x86_start16' entry types + may be used instead. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def GetDefaultFilename(self): + return 'tpl/u-boot-x86-16bit-tpl.bin' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index d956bd4..3f4f5f3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -39,6 +39,7 @@ U_BOOT_SPL_DTB_DATA = 'spldtb' U_BOOT_TPL_DTB_DATA = 'tpldtb' X86_START16_DATA = 'start16' X86_START16_SPL_DATA = 'start16spl' +X86_START16_TPL_DATA = 'start16tpl' U_BOOT_NODTB_DATA = 'nodtb with microcode pointer somewhere in here' U_BOOT_SPL_NODTB_DATA = 'splnodtb with microcode pointer somewhere in here' FSP_DATA = 'fsp' @@ -92,6 +93,8 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('u-boot-x86-16bit.bin', X86_START16_DATA) TestFunctional._MakeInputFile('spl/u-boot-x86-16bit-spl.bin', X86_START16_SPL_DATA) + TestFunctional._MakeInputFile('tpl/u-boot-x86-16bit-tpl.bin', + X86_START16_TPL_DATA) TestFunctional._MakeInputFile('u-boot-nodtb.bin', U_BOOT_NODTB_DATA) TestFunctional._MakeInputFile('spl/u-boot-spl-nodtb.bin', U_BOOT_SPL_NODTB_DATA) @@ -964,7 +967,7 @@ class TestFunctional(unittest.TestCase): str(e.exception)) def testPackStart16Spl(self): - """Test that an image with an x86 start16 region can be created""" + """Test that an image with an x86 start16 SPL region can be created""" data = self._DoReadFile('48_x86-start16-spl.dts') self.assertEqual(X86_START16_SPL_DATA, data[:len(X86_START16_SPL_DATA)]) @@ -1376,6 +1379,11 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/text': No value provided for text label " "'test-id'", str(e.exception)) + def testPackStart16Tpl(self): + """Test that an image with an x86 start16 TPL region can be created""" + data = self._DoReadFile('81_x86-start16-tpl.dts') + self.assertEqual(X86_START16_TPL_DATA, data[:len(X86_START16_TPL_DATA)]) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/81_x86-start16-tpl.dts b/tools/binman/test/81_x86-start16-tpl.dts new file mode 100644 index 0000000..68e6bbd --- /dev/null +++ b/tools/binman/test/81_x86-start16-tpl.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + size = <16>; + + x86-start16-tpl { + }; + }; +}; From a326b495cdcfd56507841e38158683e6e4d5894c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:11 -0600 Subject: [PATCH 06/46] binman: Tidy up the vblock entry At present if there are two vblock entries an image their contents are written to the same file in the output directory. This prevents checking the contents of each separately. Fix this by adding part of the entry path to the filename, and add some missing comments. Signed-off-by: Simon Glass --- tools/binman/README.entries | 5 +++++ tools/binman/entry.py | 18 ++++++++++++++++++ tools/binman/entry_test.py | 11 +++++++++++ tools/binman/etype/vblock.py | 11 ++++++++--- tools/binman/ftest.py | 2 +- 5 files changed, 43 insertions(+), 4 deletions(-) diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 31bc725..5cb52a9 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -546,6 +546,11 @@ Properties / Entry arguments: - kernelkey: Name of the kernel key to use (inside keydir) - preamble-flags: Value of the vboot preamble flags (typically 0) +Output files: + - input. - input file passed to futility + - vblock. - output file generated by futility (which is + used as the entry contents) + Chromium OS signs the read-write firmware and kernel, writing the signature in this block. This allows U-Boot to verify that the next firmware stage and kernel are genuine. diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 77cfab9..e671a2e 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -456,3 +456,21 @@ features to produce new behaviours. if missing: raise ValueError('Documentation is missing for modules: %s' % ', '.join(missing)) + + def GetUniqueName(self): + """Get a unique name for a node + + Returns: + String containing a unique name for a node, consisting of the name + of all ancestors (starting from within the 'binman' node) separated + by a dot ('.'). This can be useful for generating unique filesnames + in the output directory. + """ + name = self.name + node = self._node + while node.parent: + node = node.parent + if node.name == 'binman': + break + name = '%s.%s' % (node.name, name) + return name diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index 6fa735e..4100bcc 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -54,6 +54,17 @@ class TestEntry(unittest.TestCase): self.assertIn("Unknown entry type 'invalid-name' in node " "'invalid-path'", str(e.exception)) + def testUniqueName(self): + """Test Entry.GetUniqueName""" + import entry + Node = collections.namedtuple('Node', ['name', 'parent']) + base_node = Node('root', None) + base_entry = entry.Entry(None, None, base_node, read_node=False) + self.assertEqual('root', base_entry.GetUniqueName()) + sub_node = Node('subnode', base_node) + sub_entry = entry.Entry(None, None, sub_node, read_node=False) + self.assertEqual('root.subnode', sub_entry.GetUniqueName()) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/etype/vblock.py b/tools/binman/etype/vblock.py index 595af54..c4d970e 100644 --- a/tools/binman/etype/vblock.py +++ b/tools/binman/etype/vblock.py @@ -25,6 +25,11 @@ class Entry_vblock(Entry): - kernelkey: Name of the kernel key to use (inside keydir) - preamble-flags: Value of the vboot preamble flags (typically 0) + Output files: + - input. - input file passed to futility + - vblock. - output file generated by futility (which is + used as the entry contents) + Chromium OS signs the read-write firmware and kernel, writing the signature in this block. This allows U-Boot to verify that the next firmware stage and kernel are genuine. @@ -53,8 +58,9 @@ class Entry_vblock(Entry): return False input_data += data - output_fname = tools.GetOutputFilename('vblock.%s' % self.name) - input_fname = tools.GetOutputFilename('input.%s' % self.name) + uniq = self.GetUniqueName() + output_fname = tools.GetOutputFilename('vblock.%s' % uniq) + input_fname = tools.GetOutputFilename('input.%s' % uniq) tools.WriteFile(input_fname, input_data) prefix = self.keydir + '/' args = [ @@ -69,6 +75,5 @@ class Entry_vblock(Entry): ] #out.Notice("Sign '%s' into %s" % (', '.join(self.value), self.label)) stdout = tools.Run('futility', *args) - #out.Debug(stdout) self.SetContents(tools.ReadFile(output_fname)) return True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 3f4f5f3..c406555 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1316,7 +1316,7 @@ class TestFunctional(unittest.TestCase): """Fake calls to the futility utility""" if pipe_list[0][0] == 'futility': fname = pipe_list[0][3] - with open(fname, 'w') as fd: + with open(fname, 'wb') as fd: fd.write(VBLOCK_DATA) return command.CommandResult() From 0bfa7b09ba16f4ffaf0cfc9315336aaa708dcd26 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:12 -0600 Subject: [PATCH 07/46] binman: Support building a selection of images Sometimes it is useful to build only a subset of the images provided by the binman configuration. Add a -i option for this. It can be given multiple times to build several images. If the option is not given, all images are built. Signed-off-by: Simon Glass --- tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 9 +++++++++ tools/binman/ftest.py | 19 ++++++++++++++++++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index f0de4de..4ce8bc6 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -30,6 +30,8 @@ def ParseArgs(argv): help='Enabling debugging (provides a full traceback on error)') parser.add_option('-E', '--entry-docs', action='store_true', help='Write out entry documentation (see README.entries)') + parser.add_option('-i', '--image', type='string', action='append', + help='Image filename to build (if not specified, build all)') parser.add_option('-I', '--indir', action='append', help='Add a path to a directory to use for input files') parser.add_option('-H', '--full-help', action='store_true', diff --git a/tools/binman/control.py b/tools/binman/control.py index 2de1c86..8c48008 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -162,6 +162,15 @@ def Binman(options, args): images = _ReadImageDesc(node) + if options.image: + skip = [] + for name, image in images.iteritems(): + if name not in options.image: + del images[name] + skip.append(name) + if skip: + print 'Skipping images: %s\n' % ', '.join(skip) + # 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 diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index c406555..290e9ae 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -171,7 +171,7 @@ class TestFunctional(unittest.TestCase): return control.Binman(options, args) def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False, - entry_args=None): + entry_args=None, images=None): """Run binman with a given test file Args: @@ -180,6 +180,10 @@ class TestFunctional(unittest.TestCase): map: True to output map files for the images update_dtb: Update the offset and size of each entry in the device tree before packing it into the image + entry_args: Dict of entry args to supply to binman + key: arg name + value: value of that arg + images: List of image names to build """ args = ['-p', '-I', self._indir, '-d', self.TestFile(fname)] if debug: @@ -191,6 +195,9 @@ class TestFunctional(unittest.TestCase): if entry_args: for arg, value in entry_args.iteritems(): args.append('-a%s=%s' % (arg, value)) + if images: + for image in images: + args += ['-i', image] return self._DoBinman(*args) def _SetupDtb(self, fname, outfile='u-boot.dtb'): @@ -1384,6 +1391,16 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('81_x86-start16-tpl.dts') self.assertEqual(X86_START16_TPL_DATA, data[:len(X86_START16_TPL_DATA)]) + def testSelectImage(self): + """Test that we can select which images to build""" + with test_util.capture_sys_output() as (stdout, stderr): + retcode = self._DoTestFile('06_dual_image.dts', images=['image2']) + self.assertEqual(0, retcode) + self.assertIn('Skipping images: image1', stdout.getvalue()) + + self.assertFalse(os.path.exists(tools.GetOutputFilename('image1.bin'))) + self.assertTrue(os.path.exists(tools.GetOutputFilename('image2.bin'))) + if __name__ == "__main__": unittest.main() From fa80c25c09a6c59be4df9c42b4a02538d8a07382 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:13 -0600 Subject: [PATCH 08/46] dtoc: Allow syncing of the device tree back to a file At present we require the caller to manually update the device tree using individual calls to libfdt functions. This is not ideal. It would be better if we could make changes using the Python structure and then call a Sync() function to write them back. Add this feature to the Fdt class. Update binman and the tests to match. Signed-off-by: Simon Glass --- tools/binman/control.py | 2 ++ tools/dtoc/fdt.py | 91 ++++++++++++++++++++++++++++++++++++++++++++----- tools/dtoc/test_fdt.py | 8 ++++- 3 files changed, 91 insertions(+), 10 deletions(-) diff --git a/tools/binman/control.py b/tools/binman/control.py index 8c48008..ded1b71 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -183,6 +183,7 @@ def Binman(options, args): image.AddMissingProperties() image.ProcessFdt(dtb) + dtb.Sync(auto_resize=True) dtb.Pack() dtb.Flush() @@ -199,6 +200,7 @@ def Binman(options, args): image.SetImagePos() if options.update_fdt: image.SetCalculatedProperties() + dtb.Sync() image.ProcessEntryContents() image.WriteSymbols() image.BuildImage() diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 55baa38..76cd66f 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -43,6 +43,7 @@ class Prop: self.name = name self.value = None self.bytes = str(bytes) + self.dirty = False if not bytes: self.type = TYPE_BOOL self.value = True @@ -160,6 +161,45 @@ class Prop: self._node._fdt.CheckCache() return self._node._fdt.GetStructOffset(self._offset) + def SetInt(self, val): + """Set the integer value of the property + + The device tree is marked dirty so that the value will be written to + the block on the next sync. + + Args: + val: Integer value (32-bit, single cell) + """ + self.bytes = struct.pack('>I', val); + self.value = val + self.type = TYPE_INT + self.dirty = True + + def Sync(self, auto_resize=False): + """Sync property changes back to the device tree + + This updates the device tree blob with any changes to this property + since the last sync. + + Args: + auto_resize: Resize the device tree automatically if it does not + have enough space for the update + + Raises: + FdtException if auto_resize is False and there is not enough space + """ + if self._offset is None or self.dirty: + node = self._node + fdt_obj = node._fdt._fdt_obj + if auto_resize: + while fdt_obj.setprop(node.Offset(), self.name, self.bytes, + (libfdt.NOSPACE,)) == -libfdt.NOSPACE: + fdt_obj.resize(fdt_obj.totalsize() + 1024) + fdt_obj.setprop(node.Offset(), self.name, self.bytes) + else: + fdt_obj.setprop(node.Offset(), self.name, self.bytes) + + class Node: """A device tree node @@ -284,13 +324,7 @@ class Node: Args: prop_name: Name of property """ - fdt_obj = self._fdt._fdt_obj - if fdt_obj.setprop_u32(self.Offset(), prop_name, 0, - (libfdt.NOSPACE,)) == -libfdt.NOSPACE: - fdt_obj.resize(fdt_obj.totalsize() + 1024) - fdt_obj.setprop_u32(self.Offset(), prop_name, 0) - self.props[prop_name] = Prop(self, -1, prop_name, '\0' * 4) - self._fdt.Invalidate() + self.props[prop_name] = Prop(self, None, prop_name, '\0' * 4) def SetInt(self, prop_name, val): """Update an integer property int the device tree. @@ -301,8 +335,34 @@ class Node: prop_name: Name of property val: Value to set """ - fdt_obj = self._fdt._fdt_obj - fdt_obj.setprop_u32(self.Offset(), prop_name, val) + self.props[prop_name].SetInt(val) + + def Sync(self, auto_resize=False): + """Sync node changes back to the device tree + + This updates the device tree blob with any changes to this node and its + subnodes since the last sync. + + Args: + auto_resize: Resize the device tree automatically if it does not + have enough space for the update + + Raises: + FdtException if auto_resize is False and there is not enough space + """ + # Sync subnodes in reverse so that we don't disturb node offsets for + # nodes that are earlier in the DT. This avoids an O(n^2) rescan of + # node offsets. + for node in reversed(self.subnodes): + node.Sync(auto_resize) + + # Sync properties now, whose offsets should not have been disturbed. + # We do this after subnodes, since this disturbs the offsets of these + # properties. + prop_list = sorted(self.props.values(), key=lambda prop: prop._offset, + reverse=True) + for prop in prop_list: + prop.Sync(auto_resize) class Fdt: @@ -381,6 +441,19 @@ class Fdt: with open(self._fname, 'wb') as fd: fd.write(self._fdt_obj.as_bytearray()) + def Sync(self, auto_resize=False): + """Make sure any DT changes are written to the blob + + Args: + auto_resize: Resize the device tree automatically if it does not + have enough space for the update + + Raises: + FdtException if auto_resize is False and there is not enough space + """ + self._root.Sync(auto_resize) + self.Invalidate() + def Pack(self): """Pack the device tree down to its minimum size diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index e88d19f..4a67f89 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -337,6 +337,7 @@ class TestProp(unittest.TestCase): self.node.AddZeroProp('one') self.node.AddZeroProp('two') self.node.AddZeroProp('three') + self.dtb.Sync(auto_resize=True) # Updating existing properties should be OK, since the device-tree size # does not change @@ -344,12 +345,17 @@ class TestProp(unittest.TestCase): self.node.SetInt('one', 1) self.node.SetInt('two', 2) self.node.SetInt('three', 3) + self.dtb.Sync(auto_resize=False) # This should fail since it would need to increase the device-tree size + self.node.AddZeroProp('four') with self.assertRaises(libfdt.FdtException) as e: - self.node.SetInt('four', 4) + self.dtb.Sync(auto_resize=False) self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + def testAddNode(self): + self.fdt.pack() + class TestFdtUtil(unittest.TestCase): """Tests for the fdt_util module From af53f5aafcbbe1ba97264a0262067657f3dc8c34 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:14 -0600 Subject: [PATCH 09/46] dtoc: Fixed endianness in Prop.GetEmpty() This should be big endian, since that is what device tree uses. Fix it. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 76cd66f..ccf3b23 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -146,7 +146,7 @@ class Prop: if type == TYPE_BYTE: return chr(0) elif type == TYPE_INT: - return struct.pack('I', 0); elif type == TYPE_STRING: return '' else: From e21c27af47183619c7ec7097abb1dec34410e775 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:15 -0600 Subject: [PATCH 10/46] dtoc: Support adding new nodes Add a way to add new nodes and sync them back to the blob. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 20 ++++++++++++++++++++ tools/dtoc/test_fdt.py | 8 ++++++++ 2 files changed, 28 insertions(+) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index ccf3b23..26f4a4e 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -337,6 +337,12 @@ class Node: """ self.props[prop_name].SetInt(val) + def AddSubnode(self, name): + path = self.path + '/' + name + subnode = Node(self._fdt, self, None, name, path) + self.subnodes.append(subnode) + return subnode + def Sync(self, auto_resize=False): """Sync node changes back to the device tree @@ -350,6 +356,20 @@ class Node: Raises: FdtException if auto_resize is False and there is not enough space """ + if self._offset is None: + # The subnode doesn't exist yet, so add it + fdt_obj = self._fdt._fdt_obj + if auto_resize: + while True: + offset = fdt_obj.add_subnode(self.parent._offset, self.name, + (libfdt.NOSPACE,)) + if offset != -libfdt.NOSPACE: + break + fdt_obj.resize(fdt_obj.totalsize() + 1024) + else: + offset = fdt_obj.add_subnode(self.parent._offset, self.name) + self._offset = offset + # Sync subnodes in reverse so that we don't disturb node offsets for # nodes that are earlier in the DT. This avoids an O(n^2) rescan of # node offsets. diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 4a67f89..c94e455 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -355,6 +355,14 @@ class TestProp(unittest.TestCase): def testAddNode(self): self.fdt.pack() + self.node.AddSubnode('subnode') + with self.assertRaises(libfdt.FdtException) as e: + self.dtb.Sync(auto_resize=False) + self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + + self.dtb.Sync(auto_resize=True) + offset = self.fdt.path_offset('/spl-test/subnode') + self.assertTrue(offset > 0) class TestFdtUtil(unittest.TestCase): From 6434961b2b2099b458e7dc9dcced5d450b45cbb4 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:16 -0600 Subject: [PATCH 11/46] dtoc: Add methods for adding and updating properties Add a few more functions which allow creating and modifying property values. If only we could do this so easily in the real world. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/dtoc/test_fdt.py | 43 +++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 26f4a4e..c5054e8 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -175,6 +175,16 @@ class Prop: self.type = TYPE_INT self.dirty = True + def SetData(self, bytes): + """Set the value of a property as bytes + + Args: + bytes: New property value to set + """ + self.bytes = str(bytes) + self.type, self.value = self.BytesToValue(bytes) + self.dirty = True + def Sync(self, auto_resize=False): """Sync property changes back to the device tree @@ -326,18 +336,78 @@ class Node: """ self.props[prop_name] = Prop(self, None, prop_name, '\0' * 4) + def AddEmptyProp(self, prop_name, len): + """Add a property with a fixed data size, for filling in later + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property + len: Length of data in property + """ + value = chr(0) * len + self.props[prop_name] = Prop(self, None, prop_name, value) + def SetInt(self, prop_name, val): """Update an integer property int the device tree. This is not allowed to change the size of the FDT. + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + Args: prop_name: Name of property val: Value to set """ self.props[prop_name].SetInt(val) + def SetData(self, prop_name, val): + """Set the data value of a property + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property to set + val: Data value to set + """ + self.props[prop_name].SetData(val) + + def SetString(self, prop_name, val): + """Set the string value of a property + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property to set + val: String value to set (will be \0-terminated in DT) + """ + self.props[prop_name].SetData(val + chr(0)) + + def AddString(self, prop_name, val): + """Add a new string property to a node + + The device tree is marked dirty so that the value will be written to + the blob on the next sync. + + Args: + prop_name: Name of property to add + val: String value of property + """ + self.props[prop_name] = Prop(self, None, prop_name, val + chr(0)) + def AddSubnode(self, name): + """Add a new subnode to the node + + Args: + name: name of node to add + + Returns: + New subnode that was created + """ path = self.path + '/' + name subnode = Node(self._fdt, self, None, name, path) self.subnodes.append(subnode) diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index c94e455..22a075d 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -352,6 +352,7 @@ class TestProp(unittest.TestCase): with self.assertRaises(libfdt.FdtException) as e: self.dtb.Sync(auto_resize=False) self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + self.dtb.Sync(auto_resize=True) def testAddNode(self): self.fdt.pack() @@ -364,6 +365,48 @@ class TestProp(unittest.TestCase): offset = self.fdt.path_offset('/spl-test/subnode') self.assertTrue(offset > 0) + def testAddMore(self): + """Test various other methods for adding and setting properties""" + self.node.AddZeroProp('one') + self.dtb.Sync(auto_resize=True) + data = self.fdt.getprop(self.node.Offset(), 'one') + self.assertEqual(0, fdt32_to_cpu(data)) + + self.node.SetInt('one', 1) + self.dtb.Sync(auto_resize=False) + data = self.fdt.getprop(self.node.Offset(), 'one') + self.assertEqual(1, fdt32_to_cpu(data)) + + val = '123' + chr(0) + '456' + self.node.AddString('string', val) + self.dtb.Sync(auto_resize=True) + data = self.fdt.getprop(self.node.Offset(), 'string') + self.assertEqual(val + '\0', data) + + self.fdt.pack() + self.node.SetString('string', val + 'x') + with self.assertRaises(libfdt.FdtException) as e: + self.dtb.Sync(auto_resize=False) + self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + self.node.SetString('string', val[:-1]) + + prop = self.node.props['string'] + prop.SetData(val) + self.dtb.Sync(auto_resize=False) + data = self.fdt.getprop(self.node.Offset(), 'string') + self.assertEqual(val, data) + + self.node.AddEmptyProp('empty', 5) + self.dtb.Sync(auto_resize=True) + prop = self.node.props['empty'] + prop.SetData(val) + self.dtb.Sync(auto_resize=False) + data = self.fdt.getprop(self.node.Offset(), 'empty') + self.assertEqual(val, data) + + self.node.SetData('empty', '123') + self.assertEqual('123', prop.bytes) + class TestFdtUtil(unittest.TestCase): """Tests for the fdt_util module From 746aee3f2f8c0c7534ad7ac7d438ccec35c6c99c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:17 -0600 Subject: [PATCH 12/46] dtoc: Add a way to create an Fdt object from a data block Support creating an Fdt object without having to write the data to a file first. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 14 ++++++++++++++ tools/dtoc/test_fdt.py | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index c5054e8..2df2d4b 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -472,6 +472,20 @@ class Fdt: with open(self._fname) as fd: self._fdt_obj = libfdt.Fdt(fd.read()) + @staticmethod + def FromData(data): + """Create a new Fdt object from the given data + + Args: + data: Device-tree data blob + + Returns: + Fdt object containing the data + """ + fdt = Fdt(None) + fdt._fdt_obj = libfdt.Fdt(bytearray(data)) + return fdt + def LookupPhandle(self, phandle): """Look up a phandle diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 22a075d..d259702 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -407,6 +407,14 @@ class TestProp(unittest.TestCase): self.node.SetData('empty', '123') self.assertEqual('123', prop.bytes) + def testFromData(self): + dtb2 = fdt.Fdt.FromData(self.dtb.GetContents()) + self.assertEqual(dtb2.GetContents(), self.dtb.GetContents()) + + self.node.AddEmptyProp('empty', 5) + self.dtb.Sync(auto_resize=True) + self.assertTrue(dtb2.GetContents() != self.dtb.GetContents()) + class TestFdtUtil(unittest.TestCase): """Tests for the fdt_util module From 6c234bfbf7a9c5b33c3bea92e037c45d37e94f35 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:18 -0600 Subject: [PATCH 13/46] binman: Add an entry method for getting the default filename Various entry implementations provide a way to obtain the default filename for an entry. But at present there is no base-class implementation for this function. Add one so that the API is defined. Signed-off-by: Simon Glass --- tools/binman/entry.py | 3 +++ tools/binman/entry_test.py | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e671a2e..ec3b22e 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -160,6 +160,9 @@ class Entry(object): self.align_end = fdt_util.GetInt(self._node, 'align-end') self.offset_unset = fdt_util.GetBool(self._node, 'offset-unset') + def GetDefaultFilename(self): + return None + def AddMissingProperties(self): """Add new properties to the device tree as needed for this entry""" for prop in ['offset', 'size', 'image-pos']: diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index 4100bcc..69d85b4 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -65,6 +65,11 @@ class TestEntry(unittest.TestCase): sub_entry = entry.Entry(None, None, sub_node, read_node=False) self.assertEqual('root.subnode', sub_entry.GetUniqueName()) + def testGetDefaultFilename(self): + """Trivial test for this base class function""" + import entry + base_entry = entry.Entry(None, None, None, read_node=False) + self.assertIsNone(base_entry.GetDefaultFilename()) if __name__ == "__main__": unittest.main() From c55a50f558f13c6c018c0e5cc0f0d765711a3828 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:19 -0600 Subject: [PATCH 14/46] binman: Move state information into a new module At present the control module has state information in it, since it is the primary user of this. But it is a bit odd to have entries and other modules importing control to obtain this information. It seems better to have a dedicated state module, which control can use as well. Create a new module using code from control and update other modules to use it. Signed-off-by: Simon Glass --- tools/binman/control.py | 50 ++++--------------- tools/binman/entry.py | 7 +-- tools/binman/etype/u_boot_dtb_with_ucode.py | 6 +-- tools/binman/ftest.py | 3 +- tools/binman/state.py | 77 +++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 48 deletions(-) create mode 100644 tools/binman/state.py diff --git a/tools/binman/control.py b/tools/binman/control.py index ded1b71..00dcb8a 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -7,27 +7,19 @@ from collections import OrderedDict import os -import re import sys import tools import command import elf from image import Image +import state import tout # List of images we plan to create # Make this global so that it can be referenced from tests images = OrderedDict() -# Records the device-tree files known to binman, keyed by filename (e.g. -# '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 @@ -60,39 +52,15 @@ def _FindBinmanNode(dtb): return node return None -def GetFdt(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 - can also have others (e.g. for SPL). This function looks up the given - filename and returns the associated Fdt object. +def WriteEntryDocs(modules, test_missing=None): + """Write out documentation for all entries Args: - fname: Filename to look up (e.g. 'u-boot.dtb'). - - Returns: - Fdt object associated with the filename + modules: List of Module objects to get docs for + test_missing: Used for testing only, to force an entry's documeentation + to show as missing even if it is present. Should be set to None in + normal use. """ - return fdt_files[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 WriteEntryDocs(modules, test_missing=None): from entry import Entry Entry.WriteDocs(modules, test_missing) @@ -141,7 +109,7 @@ def Binman(options, args): try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) - SetEntryArgs(options.entry_arg) + state.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 @@ -154,7 +122,7 @@ def Binman(options, args): dtb = fdt.FdtScan(fname) # Note the file so that GetFdt() can find it - fdt_files['u-boot.dtb'] = dtb + state.fdt_files['u-boot.dtb'] = dtb node = _FindBinmanNode(dtb) if not node: raise ValueError("Device tree '%s' does not have a 'binman' " diff --git a/tools/binman/entry.py b/tools/binman/entry.py index ec3b22e..1d6299a 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -17,10 +17,11 @@ try: except: have_importlib = False -import fdt_util -import control import os import sys + +import fdt_util +import state import tools modules = {} @@ -393,7 +394,7 @@ class Entry(object): Raises: ValueError if the argument cannot be converted to in """ - value = control.GetEntryArg(name) + value = state.GetEntryArg(name) if value is not None: if datatype == int: try: diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py index 285a28d..3fab766 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -5,9 +5,9 @@ # Entry-type module for U-Boot device tree with the microcode removed # -import control from entry import Entry from blob import Entry_blob +import state import tools class Entry_u_boot_dtb_with_ucode(Entry_blob): @@ -51,7 +51,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): # Remove the microcode fname = self.GetDefaultFilename() - fdt = control.GetFdt(fname) + fdt = state.GetFdt(fname) self.ucode = fdt.GetNode('/microcode') if not self.ucode: raise self.Raise("No /microcode node found in '%s'" % fname) @@ -70,7 +70,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): def ObtainContents(self): # Call the base class just in case it does something important. Entry_blob.ObtainContents(self) - self._pathname = control.GetFdtPath(self._filename) + self._pathname = state.GetFdtPath(self._filename) self.ReadBlobContents() if self.ucode: for node in self.ucode.subnodes: diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 290e9ae..8671797 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -23,6 +23,7 @@ import fdt import fdt_util import fmap_util import test_util +import state import tools import tout @@ -258,7 +259,7 @@ class TestFunctional(unittest.TestCase): 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') + out_dtb_fname = state.GetFdtPath('u-boot.dtb') # Find the (only) image, read it and return its contents image = control.images['image'] diff --git a/tools/binman/state.py b/tools/binman/state.py new file mode 100644 index 0000000..6bc24dd --- /dev/null +++ b/tools/binman/state.py @@ -0,0 +1,77 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2018 Google, Inc +# Written by Simon Glass +# +# Holds and modifies the state information held by binman +# + +import re +from sets import Set + +import os +import tools + +# Records the device-tree files known to binman, keyed by filename (e.g. +# 'u-boot-spl.dtb') +fdt_files = {} + +# Arguments passed to binman to provide arguments to entries +entry_args = {} + +def GetFdt(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 + can also have others (e.g. for SPL). This function looks up the given + filename and returns the associated Fdt object. + + Args: + fname: Filename to look up (e.g. 'u-boot.dtb'). + + Returns: + Fdt object associated with the filename + """ + return fdt_files[fname] + +def GetFdtPath(fname): + """Get the full pathname of a particular Fdt object + + Similar to GetFdt() but returns the pathname associated with the Fdt. + + Args: + fname: Filename to look up (e.g. 'u-boot.dtb'). + + Returns: + Full path name to the associated Fdt + """ + return fdt_files[fname]._fname + +def SetEntryArgs(args): + """Set the value of the entry args + + This sets up the entry_args dict which is used to supply entry arguments to + entries. + + Args: + args: List of entry arguments, each in the format "name=value" + """ + 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): + """Get the value of an entry argument + + Args: + name: Name of argument to retrieve + + Returns: + String value of argument + """ + return entry_args.get(name) From 2a72cc72ca29fb14a61dd50a60ffcd096a0be317 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:20 -0600 Subject: [PATCH 15/46] binman: Move state logic into the state module Rather than reaching into this module from control, move the code that needs this info into state. Signed-off-by: Simon Glass --- tools/binman/control.py | 21 +++++++++++++-------- tools/binman/state.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/tools/binman/control.py b/tools/binman/control.py index 00dcb8a..fd8b779 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -121,8 +121,6 @@ def Binman(options, args): outfd.write(infd.read()) dtb = fdt.FdtScan(fname) - # Note the file so that GetFdt() can find it - state.fdt_files['u-boot.dtb'] = dtb node = _FindBinmanNode(dtb) if not node: raise ValueError("Device tree '%s' does not have a 'binman' " @@ -139,6 +137,8 @@ def Binman(options, args): if skip: print 'Skipping images: %s\n' % ', '.join(skip) + state.Prepare(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 @@ -151,9 +151,10 @@ def Binman(options, args): image.AddMissingProperties() image.ProcessFdt(dtb) - dtb.Sync(auto_resize=True) - dtb.Pack() - dtb.Flush() + for dtb_item in state.GetFdts(): + dtb_item.Sync(auto_resize=True) + dtb_item.Pack() + dtb_item.Flush() for image in images.values(): # Perform all steps for this image, including checking and @@ -168,14 +169,18 @@ def Binman(options, args): image.SetImagePos() if options.update_fdt: image.SetCalculatedProperties() - dtb.Sync() + for dtb_item in state.GetFdts(): + dtb_item.Sync() image.ProcessEntryContents() image.WriteSymbols() image.BuildImage() if options.map: image.WriteMap() - with open(fname, 'wb') as outfd: - outfd.write(dtb.GetContents()) + + # Write the updated FDTs to our output files + for dtb_item in state.GetFdts(): + tools.WriteFile(dtb_item._fname, dtb_item.GetContents()) + finally: tools.FinaliseOutputDir() finally: diff --git a/tools/binman/state.py b/tools/binman/state.py index 6bc24dd..9583b3f 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -18,6 +18,15 @@ fdt_files = {} # Arguments passed to binman to provide arguments to entries entry_args = {} +# Set of all device tree files references by images +fdt_set = Set() + +# Same as above, but excluding the main one +fdt_subset = Set() + +# The DTB which contains the full image information +main_dtb = None + def GetFdt(fname): """Get the Fdt object for a particular device-tree filename @@ -75,3 +84,37 @@ def GetEntryArg(name): String value of argument """ return entry_args.get(name) + +def Prepare(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. + + Args: + dtb: Main dtb + """ + global fdt_set, fdt_subset, 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 + import fdt_util + + # If we are updating the DTBs we need to put these updated versions + # where Entry_blob_dtb can find them. We can ignore 'u-boot.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 + fdt_set = Set() + fdt_subset = Set() + +def GetFdts(): + """Yield all device tree files being used by binman + + Yields: + Device trees being used (U-Boot proper, SPL, TPL) + """ + yield main_dtb + From f46621d255181bd8d1e8092945ffc66147b88531 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:21 -0600 Subject: [PATCH 16/46] binman: Centralise device-tree updates within binman At present we have a few calls to device-tree functions in binman and plan to add more as we add new entry types which need to report their results. It makes sense to put this code in a central place so that we can make sure all device trees are updated. At present we only have U-Boot proper, but plan to add SPL and TPL too. Signed-off-by: Simon Glass --- tools/binman/bsection.py | 9 +++++---- tools/binman/entry.py | 8 ++++---- tools/binman/state.py | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index a0bd1b6..4f5c33f 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -12,6 +12,7 @@ import sys import fdt_util import re +import state import tools class Section(object): @@ -98,14 +99,14 @@ class Section(object): """Add new properties to the device tree as needed for this entry""" for prop in ['offset', 'size', 'image-pos']: if not prop in self._node.props: - self._node.AddZeroProp(prop) + state.AddZeroProp(self._node, prop) for entry in self._entries.values(): entry.AddMissingProperties() def SetCalculatedProperties(self): - self._node.SetInt('offset', self._offset) - self._node.SetInt('size', self._size) - self._node.SetInt('image-pos', self._image_pos) + state.SetInt(self._node, 'offset', self._offset) + state.SetInt(self._node, 'size', self._size) + state.SetInt(self._node, 'image-pos', self._image_pos) for entry in self._entries.values(): entry.SetCalculatedProperties() diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 1d6299a..4b87156 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -168,13 +168,13 @@ class Entry(object): """Add new properties to the device tree as needed for this entry""" for prop in ['offset', 'size', 'image-pos']: if not prop in self._node.props: - self._node.AddZeroProp(prop) + state.AddZeroProp(self._node, prop) def SetCalculatedProperties(self): """Set the value of device-tree properties calculated by binman""" - self._node.SetInt('offset', self.offset) - self._node.SetInt('size', self.size) - self._node.SetInt('image-pos', self.image_pos) + state.SetInt(self._node, 'offset', self.offset) + state.SetInt(self._node, 'size', self.size) + state.SetInt(self._node, 'image-pos', self.image_pos) def ProcessFdt(self, fdt): return True diff --git a/tools/binman/state.py b/tools/binman/state.py index 9583b3f..5f25b90 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -118,3 +118,38 @@ def GetFdts(): """ yield main_dtb +def GetUpdateNodes(node): + """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 + have the given node. Due to removable of unwanted notes, SPL and TPL may + not have this node. + + Args: + node: Node object in the main device tree to look up + + Yields: + Node objects in each device tree that is in use (U-Boot proper, which + is node, SPL and TPL) + """ + yield node + +def AddZeroProp(node, prop): + """Add a new property to affected device trees with an integer value of 0. + + Args: + prop_name: Name of property + """ + for n in GetUpdateNodes(node): + n.AddZeroProp(prop) + +def SetInt(node, prop, value): + """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 n in GetUpdateNodes(node): + n.SetInt(prop, value) From 539aece516d87084437a38d879a1b91c661209f8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:22 -0600 Subject: [PATCH 17/46] binman: Obtain the list of device trees from the config We always have a device tree for U-Boot proper. But we may also have one for SPL and TPL. Add a new Entry method to find out what DTs an entry has, and use that list when updating DTs. Signed-off-by: Simon Glass --- tools/binman/bsection.py | 8 ++++++++ tools/binman/control.py | 2 +- tools/binman/entry.py | 15 +++++++++++++++ tools/binman/image.py | 4 ++++ tools/binman/state.py | 20 ++++++++++++++++++-- 5 files changed, 46 insertions(+), 3 deletions(-) diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 4f5c33f..44adb82 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -8,6 +8,7 @@ from __future__ import print_function from collections import OrderedDict +from sets import Set import sys import fdt_util @@ -92,6 +93,13 @@ class Section(object): entry.SetPrefix(self._name_prefix) self._entries[node.name] = entry + def GetFdtSet(self): + """Get the set of device tree files used by this image""" + fdt_set = Set() + for entry in self._entries.values(): + fdt_set.update(entry.GetFdtSet()) + return fdt_set + def SetOffset(self, offset): self._offset = offset diff --git a/tools/binman/control.py b/tools/binman/control.py index fd8b779..49d49a0 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -137,7 +137,7 @@ def Binman(options, args): if skip: print 'Skipping images: %s\n' % ', '.join(skip) - state.Prepare(dtb) + 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 diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 4b87156..e5f5577 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -18,6 +18,7 @@ except: have_importlib = False import os +from sets import Set import sys import fdt_util @@ -164,6 +165,20 @@ class Entry(object): def GetDefaultFilename(self): return None + def GetFdtSet(self): + """Get the set of device trees used by this entry + + Returns: + 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 AddMissingProperties(self): """Add new properties to the device tree as needed for this entry""" for prop in ['offset', 'size', 'image-pos']: diff --git a/tools/binman/image.py b/tools/binman/image.py index 68126bc..1fb5eb6 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -54,6 +54,10 @@ class Image: self._filename = filename self._section = bsection.Section('main-section', self._node) + def GetFdtSet(self): + """Get the set of device tree files used by this image""" + return self._section.GetFdtSet() + def AddMissingProperties(self): """Add properties that are not present in the device tree diff --git a/tools/binman/state.py b/tools/binman/state.py index 5f25b90..600eb86 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -18,6 +18,10 @@ fdt_files = {} # Arguments passed to binman to provide arguments to entries entry_args = {} +# True to use fake device-tree files for testing (see U_BOOT_DTB_DATA in +# ftest.py) +use_fake_dtb = True + # Set of all device tree files references by images fdt_set = Set() @@ -85,13 +89,14 @@ def GetEntryArg(name): """ return entry_args.get(name) -def Prepare(dtb): +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. Args: + images: List of images being used dtb: Main dtb """ global fdt_set, fdt_subset, fdt_files, main_dtb @@ -107,8 +112,19 @@ def Prepare(dtb): main_dtb = dtb fdt_files.clear() fdt_files['u-boot.dtb'] = dtb - fdt_set = Set() fdt_subset = Set() + if not use_fake_dtb: + for image in images.values(): + fdt_subset.update(image.GetFdtSet()) + fdt_subset.discard('u-boot.dtb') + for other_fname in fdt_subset: + 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) + fdt_files[other_fname] = other_dtb def GetFdts(): """Yield all device tree files being used by binman From 93d174135ac44cbbe81c87ea564488309949e6d4 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:23 -0600 Subject: [PATCH 18/46] binman: Allow control of whether a fake DT is used We use a fake device tree in tests most of the time since tests don't normally care about the actual data. For example, for U-Boot proper we use U_BOOT_DTB_DATA which is just a four-character string. This makes testing the image output against an expected value very easy. However in some cases, such as when the test wants to check that the DT output containing particular nodes, we do actually need the real DT. Add support for this, along with a command-line option to select 'test mode'. Signed-off-by: Simon Glass --- tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 1 + tools/binman/ftest.py | 4 +++- tools/binman/state.py | 2 +- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 4ce8bc6..f8caa7d 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -30,6 +30,8 @@ def ParseArgs(argv): help='Enabling debugging (provides a full traceback on error)') parser.add_option('-E', '--entry-docs', action='store_true', help='Write out entry documentation (see README.entries)') + parser.add_option('--fake-dtb', action='store_true', + help='Use fake device tree contents (for testing only)') parser.add_option('-i', '--image', type='string', action='append', help='Image filename to build (if not specified, build all)') parser.add_option('-I', '--indir', action='append', diff --git a/tools/binman/control.py b/tools/binman/control.py index 49d49a0..34ec74b 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -106,6 +106,7 @@ def Binman(options, args): tout.Init(options.verbosity) elf.debug = options.debug + state.use_fake_dtb = options.fake_dtb try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 8671797..75e9a21 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -172,7 +172,7 @@ class TestFunctional(unittest.TestCase): return control.Binman(options, args) def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False, - entry_args=None, images=None): + entry_args=None, images=None, use_real_dtb=False): """Run binman with a given test file Args: @@ -193,6 +193,8 @@ class TestFunctional(unittest.TestCase): args.append('-m') if update_dtb: args.append('-up') + if not use_real_dtb: + args.append('--fake-dtb') if entry_args: for arg, value in entry_args.iteritems(): args.append('-a%s=%s' % (arg, value)) diff --git a/tools/binman/state.py b/tools/binman/state.py index 600eb86..b27eb07 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -20,7 +20,7 @@ entry_args = {} # True to use fake device-tree files for testing (see U_BOOT_DTB_DATA in # ftest.py) -use_fake_dtb = True +use_fake_dtb = False # Set of all device tree files references by images fdt_set = Set() From 6ed45ba0a831393ab6c5d3355384238d2b4f47da Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:24 -0600 Subject: [PATCH 19/46] binman: Support updating all device tree files Binman currently supports updating the main device tree with things like the position of each entry. Extend this support to SPL and TPL as well, since they may need (a subset of) this information. Also adjust DTB output files to have a .out extension since this seems clearer than having a .dtb extension with 'out' in the name somwhere. Also add a few missing comments and update the DT setup code to use ReadFile and WriteFile(). Signed-off-by: Simon Glass --- tools/binman/README.entries | 12 +++ tools/binman/control.py | 6 +- tools/binman/entry.py | 11 +++ tools/binman/etype/blob_dtb.py | 33 +++++++++ tools/binman/etype/section.py | 3 + tools/binman/etype/u_boot_dtb.py | 9 ++- tools/binman/etype/u_boot_dtb_with_ucode.py | 8 +- tools/binman/etype/u_boot_spl_dtb.py | 6 +- tools/binman/etype/u_boot_tpl_dtb.py | 6 +- tools/binman/ftest.py | 109 +++++++++++++++++++++++++++- tools/binman/image.py | 5 ++ tools/binman/state.py | 30 ++++++++ tools/binman/test/82_fdt_update_all.dts | 18 +++++ 13 files changed, 235 insertions(+), 21 deletions(-) create mode 100644 tools/binman/etype/blob_dtb.py create mode 100644 tools/binman/test/82_fdt_update_all.dts diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 5cb52a9..091fb5c 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -26,6 +26,15 @@ example the 'u_boot' entry which provides the filename 'u-boot.bin'. +Entry: blob-dtb: A blob that holds a device tree +------------------------------------------------ + +This is a blob containing a device tree. The contents of the blob are +obtained from the list of available device-tree files, managed by the +'state' module. + + + Entry: blob-named-by-arg: A blob entry which gets its filename property from its subclass ----------------------------------------------------------------------------------------- @@ -309,6 +318,9 @@ This is the U-Boot device tree, containing configuration information for U-Boot. U-Boot needs this to know what devices are present and which drivers to activate. +Note: This is mostly an internal entry type, used by others. This allows +binman to know which entries contain a device tree. + Entry: u-boot-dtb-with-ucode: A U-Boot device tree file, with the microcode removed diff --git a/tools/binman/control.py b/tools/binman/control.py index 34ec74b..e326456 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -116,10 +116,8 @@ def Binman(options, args): # 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-out.dtb') - with open(dtb_fname) as infd: - with open(fname, 'wb') as outfd: - outfd.write(infd.read()) + fname = tools.GetOutputFilename('u-boot.dtb.out') + tools.WriteFile(fname, tools.ReadFile(dtb_fname)) dtb = fdt.FdtScan(fname) node = _FindBinmanNode(dtb) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e5f5577..f922107 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -192,6 +192,17 @@ class Entry(object): state.SetInt(self._node, 'image-pos', self.image_pos) def ProcessFdt(self, fdt): + """Allow entries to adjust the device tree + + Some entries need to adjust the device tree for their purposes. This + may involve adding or deleting properties. + + Returns: + True if processing is complete + False if processing could not be completed due to a dependency. + This will cause the entry to be retried after others have been + called + """ return True def SetPrefix(self, prefix): diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py new file mode 100644 index 0000000..cc5b4a3 --- /dev/null +++ b/tools/binman/etype/blob_dtb.py @@ -0,0 +1,33 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass +# +# Entry-type module for U-Boot device tree files +# + +import state + +from entry import Entry +from blob import Entry_blob + +class Entry_blob_dtb(Entry_blob): + """A blob that holds a device tree + + This is a blob containing a device tree. The contents of the blob are + obtained from the list of available device-tree files, managed by the + 'state' module. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def ObtainContents(self): + """Get the device-tree from the list held by the 'state' module""" + self._filename = self.GetDefaultFilename() + self._pathname, data = state.GetFdtContents(self._filename) + self.SetContents(data) + return True + + def ProcessContents(self): + """Re-read the DTB contents so that we get any calculated properties""" + _, data = state.GetFdtContents(self._filename) + self.SetContents(data) diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index f5b2ed6..a30cc91 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -34,6 +34,9 @@ class Entry_section(Entry): Entry.__init__(self, section, etype, node) self._section = bsection.Section(node.name, node) + def GetFdtSet(self): + return self._section.GetFdtSet() + def ProcessFdt(self, fdt): return self._section.ProcessFdt(fdt) diff --git a/tools/binman/etype/u_boot_dtb.py b/tools/binman/etype/u_boot_dtb.py index fb3dd1c..6263c4e 100644 --- a/tools/binman/etype/u_boot_dtb.py +++ b/tools/binman/etype/u_boot_dtb.py @@ -6,9 +6,9 @@ # from entry import Entry -from blob import Entry_blob +from blob_dtb import Entry_blob_dtb -class Entry_u_boot_dtb(Entry_blob): +class Entry_u_boot_dtb(Entry_blob_dtb): """U-Boot device tree Properties / Entry arguments: @@ -17,9 +17,12 @@ class Entry_u_boot_dtb(Entry_blob): This is the U-Boot device tree, containing configuration information for U-Boot. U-Boot needs this to know what devices are present and which drivers to activate. + + Note: This is mostly an internal entry type, used by others. This allows + binman to know which entries contain a device tree. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + Entry_blob_dtb.__init__(self, section, etype, node) def GetDefaultFilename(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 3fab766..73f5fbf 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -6,11 +6,11 @@ # from entry import Entry -from blob import Entry_blob +from blob_dtb import Entry_blob_dtb import state import tools -class Entry_u_boot_dtb_with_ucode(Entry_blob): +class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): """A U-Boot device tree file, with the microcode removed Properties / Entry arguments: @@ -25,7 +25,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): it available to u_boot_ucode. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + Entry_blob_dtb.__init__(self, section, etype, node) self.ucode_data = '' self.collate = False self.ucode_offset = None @@ -69,7 +69,7 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): def ObtainContents(self): # Call the base class just in case it does something important. - Entry_blob.ObtainContents(self) + Entry_blob_dtb.ObtainContents(self) self._pathname = state.GetFdtPath(self._filename) self.ReadBlobContents() if self.ucode: diff --git a/tools/binman/etype/u_boot_spl_dtb.py b/tools/binman/etype/u_boot_spl_dtb.py index cb29ba3..e735464 100644 --- a/tools/binman/etype/u_boot_spl_dtb.py +++ b/tools/binman/etype/u_boot_spl_dtb.py @@ -6,9 +6,9 @@ # from entry import Entry -from blob import Entry_blob +from blob_dtb import Entry_blob_dtb -class Entry_u_boot_spl_dtb(Entry_blob): +class Entry_u_boot_spl_dtb(Entry_blob_dtb): """U-Boot SPL device tree Properties / Entry arguments: @@ -19,7 +19,7 @@ class Entry_u_boot_spl_dtb(Entry_blob): to activate. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + Entry_blob_dtb.__init__(self, section, etype, node) def GetDefaultFilename(self): return 'spl/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 9c4e668..bdeb0f7 100644 --- a/tools/binman/etype/u_boot_tpl_dtb.py +++ b/tools/binman/etype/u_boot_tpl_dtb.py @@ -6,9 +6,9 @@ # from entry import Entry -from blob import Entry_blob +from blob_dtb import Entry_blob_dtb -class Entry_u_boot_tpl_dtb(Entry_blob): +class Entry_u_boot_tpl_dtb(Entry_blob_dtb): """U-Boot TPL device tree Properties / Entry arguments: @@ -19,7 +19,7 @@ class Entry_u_boot_tpl_dtb(Entry_blob): to activate. """ def __init__(self, section, etype, node): - Entry_blob.__init__(self, section, etype, node) + Entry_blob_dtb.__init__(self, section, etype, node) def GetDefaultFilename(self): return 'tpl/u-boot-tpl.dtb' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 75e9a21..6bfef7b 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -225,8 +225,26 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile(outfile, data) return data + def _GetDtbContentsForSplTpl(self, dtb_data, name): + """Create a version of the main DTB for SPL or SPL + + For testing we don't actually have different versions of the DTB. With + U-Boot we normally run fdtgrep to remove unwanted nodes, but for tests + we don't normally have any unwanted nodes. + + We still want the DTBs for SPL and TPL to be different though, since + otherwise it is confusing to know which one we are looking at. So add + an 'spl' or 'tpl' property to the top-level node. + """ + dtb = fdt.Fdt.FromData(dtb_data) + dtb.Scan() + dtb.GetNode('/binman').AddZeroProp(name) + dtb.Sync(auto_resize=True) + dtb.Pack() + return dtb.GetContents() + def _DoReadFileDtb(self, fname, use_real_dtb=False, map=False, - update_dtb=False, entry_args=None): + update_dtb=False, entry_args=None, reset_dtbs=True): """Run binman and return the resulting image This runs binman with a given test file and then reads the resulting @@ -256,12 +274,21 @@ class TestFunctional(unittest.TestCase): # Use the compiled test file as the u-boot-dtb input if use_real_dtb: dtb_data = self._SetupDtb(fname) + infile = os.path.join(self._indir, 'u-boot.dtb') + + # For testing purposes, make a copy of the DT for SPL and TPL. Add + # a node indicating which it is, so aid verification. + for name in ['spl', 'tpl']: + dtb_fname = '%s/u-boot-%s.dtb' % (name, name) + outfile = os.path.join(self._indir, dtb_fname) + TestFunctional._MakeInputFile(dtb_fname, + self._GetDtbContentsForSplTpl(dtb_data, name)) try: retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb, - entry_args=entry_args) + entry_args=entry_args, use_real_dtb=use_real_dtb) self.assertEqual(0, retcode) - out_dtb_fname = state.GetFdtPath('u-boot.dtb') + out_dtb_fname = tools.GetOutputFilename('u-boot.dtb.out') # Find the (only) image, read it and return its contents image = control.images['image'] @@ -277,7 +304,7 @@ class TestFunctional(unittest.TestCase): return fd.read(), dtb_data, map_data, out_dtb_fname finally: # Put the test file back - if use_real_dtb: + if reset_dtbs and use_real_dtb: self._ResetDtbs() def _DoReadFile(self, fname, use_real_dtb=False): @@ -1404,6 +1431,80 @@ class TestFunctional(unittest.TestCase): self.assertFalse(os.path.exists(tools.GetOutputFilename('image1.bin'))) self.assertTrue(os.path.exists(tools.GetOutputFilename('image2.bin'))) + def testUpdateFdtAll(self): + """Test that all device trees are updated with offset/size info""" + data, _, _, _ = self._DoReadFileDtb('82_fdt_update_all.dts', + use_real_dtb=True, update_dtb=True) + + base_expected = { + 'section:image-pos': 0, + 'u-boot-tpl-dtb:size': 513, + 'u-boot-spl-dtb:size': 513, + 'u-boot-spl-dtb:offset': 493, + 'image-pos': 0, + 'section/u-boot-dtb:image-pos': 0, + 'u-boot-spl-dtb:image-pos': 493, + 'section/u-boot-dtb:size': 493, + 'u-boot-tpl-dtb:image-pos': 1006, + 'section/u-boot-dtb:offset': 0, + 'section:size': 493, + 'offset': 0, + 'section:offset': 0, + 'u-boot-tpl-dtb:offset': 1006, + 'size': 1519 + } + + # We expect three device-tree files in the output, one after the other. + # 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 postions and offset. + start = 0 + for item in ['', 'spl', 'tpl']: + dtb = fdt.Fdt.FromData(data[start:]) + dtb.Scan() + props = self._GetPropTree(dtb, ['offset', 'size', 'image-pos', + 'spl', 'tpl']) + expected = dict(base_expected) + if item: + expected[item] = 0 + self.assertEqual(expected, props) + start += dtb._fdt_obj.totalsize() + + def testUpdateFdtOutput(self): + """Test that output DTB files are updated""" + try: + data, dtb_data, _, _ = self._DoReadFileDtb('82_fdt_update_all.dts', + use_real_dtb=True, update_dtb=True, reset_dtbs=False) + + # Unfortunately, compiling a source file always results in a file + # called source.dtb (see fdt_util.EnsureCompiled()). The test + # source file (e.g. test/75_fdt_update_all.dts) thus does not enter + # binman as a file called u-boot.dtb. To fix this, copy the file + # over to the expected place. + #tools.WriteFile(os.path.join(self._indir, 'u-boot.dtb'), + #tools.ReadFile(tools.GetOutputFilename('source.dtb'))) + start = 0 + for fname in ['u-boot.dtb.out', 'spl/u-boot-spl.dtb.out', + 'tpl/u-boot-tpl.dtb.out']: + dtb = fdt.Fdt.FromData(data[start:]) + size = dtb._fdt_obj.totalsize() + pathname = tools.GetOutputFilename(os.path.split(fname)[1]) + outdata = tools.ReadFile(pathname) + name = os.path.split(fname)[0] + + if name: + orig_indata = self._GetDtbContentsForSplTpl(dtb_data, name) + else: + orig_indata = dtb_data + self.assertNotEqual(outdata, orig_indata, + "Expected output file '%s' be updated" % pathname) + self.assertEqual(outdata, data[start:start + size], + "Expected output file '%s' to match output image" % + pathname) + start += size + finally: + self._ResetDtbs() + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index 1fb5eb6..bfb2229 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -70,6 +70,11 @@ class Image: self._section.AddMissingProperties() def ProcessFdt(self, fdt): + """Allow entries to adjust the device tree + + Some entries need to adjust the device tree for their purposes. This + may involve adding or deleting properties. + """ return self._section.ProcessFdt(fdt) def GetEntryContents(self): diff --git a/tools/binman/state.py b/tools/binman/state.py index b27eb07..09ead44 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -59,6 +59,29 @@ def GetFdtPath(fname): """ return fdt_files[fname]._fname +def GetFdtContents(fname): + """Looks up the FDT pathname and contents + + This is used to obtain the Fdt pathname and contents when needed by an + entry. It supports a 'fake' dtb, allowing tests to substitute test data for + the real dtb. + + Args: + fname: Filename 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 = GetFdt(fname).GetContents() + else: + pathname = tools.GetInputFilename(fname) + data = tools.ReadFile(pathname) + return pathname, data + def SetEntryArgs(args): """Set the value of the entry args @@ -133,6 +156,8 @@ def GetFdts(): Device trees being used (U-Boot proper, SPL, TPL) """ yield main_dtb + for other_fname in fdt_subset: + yield fdt_files[other_fname] def GetUpdateNodes(node): """Yield all the nodes that need to be updated in all device trees @@ -149,6 +174,11 @@ def GetUpdateNodes(node): is node, SPL and TPL) """ yield node + for dtb in fdt_files.values(): + if dtb != node.GetFdt(): + other_node = dtb.GetNode(node.path) + if other_node: + yield other_node def AddZeroProp(node, prop): """Add a new property to affected device trees with an integer value of 0. diff --git a/tools/binman/test/82_fdt_update_all.dts b/tools/binman/test/82_fdt_update_all.dts new file mode 100644 index 0000000..284975c --- /dev/null +++ b/tools/binman/test/82_fdt_update_all.dts @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + section { + u-boot-dtb { + }; + }; + u-boot-spl-dtb { + }; + u-boot-tpl-dtb { + }; + }; +}; From 04187a845c86215da0e3ad680cdcf2fc7515b99a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:25 -0600 Subject: [PATCH 20/46] patman: Detect missing tools and report them When tools are needed but not present, at present we just get an error which can be confusing for the user. Try to be helpful by reporting the tool as missing and suggesting a possible remedy. Also update the Run() method to support this. Signed-off-by: Simon Glass --- tools/patman/tools.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tools/patman/tools.py b/tools/patman/tools.py index e804814..0870bcc 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -22,6 +22,10 @@ chroot_path = None # Search paths to use for Filename(), used to find files search_paths = [] +# Tools and the packages that contain them, on debian +packages = { + 'lz4': 'liblz4-tool', + } def PrepareOutputDir(dirname, preserve=False): """Select an output directory, ensuring it exists. @@ -128,8 +132,31 @@ def Align(pos, align): def NotPowerOfTwo(num): return num and (num & (num - 1)) +def PathHasFile(fname): + """Check if a given filename is in the PATH + + Args: + fname: Filename to check + + Returns: + True if found, False if not + """ + for dir in os.environ['PATH'].split(':'): + if os.path.exists(os.path.join(dir, fname)): + return True + return False + def Run(name, *args): - command.Run(name, *args, cwd=outdir) + try: + return command.Run(name, *args, cwd=outdir, capture=True) + except: + if not PathHasFile(name): + msg = "Plesae install tool '%s'" % name + package = packages.get(name) + if package: + msg += " (e.g. from package '%s')" % package + raise ValueError(msg) + raise def Filename(fname): """Resolve a file path to an absolute path. From 83d73c2f7c471c1a7e5a9a2bf0de287491408b2d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:26 -0600 Subject: [PATCH 21/46] binman: Support compressed entries Add support for compressing blob entries. This can help reduce image sizes for many types of data. It requires that the firmware be able to decompress the data at run-time. Signed-off-by: Simon Glass --- .travis.yml | 1 + tools/binman/README | 16 +++++++++++++ tools/binman/README.entries | 7 ++++++ tools/binman/etype/blob.py | 49 ++++++++++++++++++++++++++++++++------- tools/binman/ftest.py | 31 +++++++++++++++++++++++++ tools/binman/test/83_compress.dts | 11 +++++++++ 6 files changed, 106 insertions(+), 9 deletions(-) create mode 100644 tools/binman/test/83_compress.dts diff --git a/.travis.yml b/.travis.yml index ea3b20e..2b759c9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,6 +27,7 @@ addons: - wget - device-tree-compiler - lzop + - liblz4-tool before_install: - sudo add-apt-repository ppa:ubuntu-toolchain-r/test -y diff --git a/tools/binman/README b/tools/binman/README index 10dfe57..d687194 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -593,6 +593,22 @@ the device tree. These can be used by U-Boot at run-time to find the location of each entry. +Compression +----------- + +Binman support compression for 'blob' entries (those of type 'blob' and +derivatives). To enable this for an entry, add a 'compression' property: + + blob { + filename = "datafile"; + compression = "lz4"; + }; + +The entry will then contain the compressed data, using the 'lz4' compression +algorithm. Currently this is the only one that is supported. + + + Map files --------- diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 091fb5c..2cf7dc0 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -19,11 +19,18 @@ class by other entry types. Properties / Entry arguments: - filename: Filename of file to read into entry + - compress: Compression algorithm to use: + none: No compression + lz4: Use lz4 compression (via 'lz4' command-line utility) This entry reads data from a file and places it in the entry. The default filename is often specified specified by the subclass. See for example the 'u_boot' entry which provides the filename 'u-boot.bin'. +If compression is enabled, an extra 'uncomp-size' property is written to +the node (if enabled with -u) which provides the uncompressed size of the +data. + Entry: blob-dtb: A blob that holds a device tree diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 3f46eec..642a0e4 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -7,6 +7,7 @@ from entry import Entry import fdt_util +import state import tools class Entry_blob(Entry): @@ -17,14 +18,23 @@ class Entry_blob(Entry): Properties / Entry arguments: - filename: Filename of file to read into entry + - compress: Compression algorithm to use: + none: No compression + lz4: Use lz4 compression (via 'lz4' command-line utility) This entry reads data from a file and places it in the entry. The default filename is often specified specified by the subclass. See for example the 'u_boot' entry which provides the filename 'u-boot.bin'. + + If compression is enabled, an extra 'uncomp-size' property is written to + the node (if enabled with -u) which provides the uncompressed size of the + data. """ def __init__(self, section, etype, node): Entry.__init__(self, section, etype, node) - self._filename = fdt_util.GetString(self._node, "filename", self.etype) + self._filename = fdt_util.GetString(self._node, 'filename', self.etype) + self._compress = fdt_util.GetString(self._node, 'compress', 'none') + self._uncompressed_size = None def ObtainContents(self): self._filename = self.GetDefaultFilename() @@ -33,15 +43,36 @@ class Entry_blob(Entry): return True def ReadBlobContents(self): - with open(self._pathname) as fd: - # We assume the data is small enough to fit into memory. If this - # is used for large filesystem image that might not be true. - # In that case, Image.BuildImage() could be adjusted to use a - # new Entry method which can read in chunks. Then we could copy - # the data in chunks and avoid reading it all at once. For now - # this seems like an unnecessary complication. - self.SetContents(fd.read()) + # We assume the data is small enough to fit into memory. If this + # is used for large filesystem image that might not be true. + # In that case, Image.BuildImage() could be adjusted to use a + # new Entry method which can read in chunks. Then we could copy + # the data in chunks and avoid reading it all at once. For now + # this seems like an unnecessary complication. + data = tools.ReadFile(self._pathname) + if self._compress == 'lz4': + self._uncompressed_size = len(data) + ''' + import lz4 # Import this only if needed (python-lz4 dependency) + + try: + data = lz4.frame.compress(data) + except AttributeError: + data = lz4.compress(data) + ''' + data = tools.Run('lz4', '-c', self._pathname, ) + self.SetContents(data) return True def GetDefaultFilename(self): return self._filename + + def AddMissingProperties(self): + Entry.AddMissingProperties(self) + if self._compress != 'none': + state.AddZeroProp(self._node, 'uncomp-size') + + def SetCalculatedProperties(self): + Entry.SetCalculatedProperties(self) + if self._uncompressed_size is not None: + state.SetInt(self._node, 'uncomp-size', self._uncompressed_size) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 6bfef7b..1c3c46f 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -54,6 +54,7 @@ CROS_EC_RW_DATA = 'ecrw' GBB_DATA = 'gbbd' BMPBLK_DATA = 'bmp' VBLOCK_DATA = 'vblk' +COMPRESS_DATA = 'data to compress' class TestFunctional(unittest.TestCase): @@ -116,6 +117,8 @@ class TestFunctional(unittest.TestCase): with open(self.TestFile('descriptor.bin')) as fd: TestFunctional._MakeInputFile('descriptor.bin', fd.read()) + TestFunctional._MakeInputFile('compress', COMPRESS_DATA) + @classmethod def tearDownClass(self): """Remove the temporary input directory and its contents""" @@ -1505,6 +1508,34 @@ class TestFunctional(unittest.TestCase): finally: self._ResetDtbs() + def _decompress(self, data): + out = os.path.join(self._indir, 'lz4.tmp') + with open(out, 'wb') as fd: + fd.write(data) + return tools.Run('lz4', '-dc', out) + ''' + try: + orig = lz4.frame.decompress(data) + except AttributeError: + orig = lz4.decompress(data) + ''' + + def testCompress(self): + """Test compression of blobs""" + data, _, _, out_dtb_fname = self._DoReadFileDtb('83_compress.dts', + use_real_dtb=True, update_dtb=True) + dtb = fdt.Fdt(out_dtb_fname) + dtb.Scan() + props = self._GetPropTree(dtb, ['size', 'uncomp-size']) + orig = self._decompress(data) + self.assertEquals(COMPRESS_DATA, orig) + expected = { + 'blob:uncomp-size': len(COMPRESS_DATA), + 'blob:size': len(data), + 'size': len(data), + } + self.assertEqual(expected, props) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/83_compress.dts b/tools/binman/test/83_compress.dts new file mode 100644 index 0000000..07813bd --- /dev/null +++ b/tools/binman/test/83_compress.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + blob { + filename = "compress"; + compress = "lz4"; + }; + }; +}; From b4e1a38c294f56708d1a82717c850da359401d7e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:27 -0600 Subject: [PATCH 22/46] binman: Allow zero-size sections At present if there is only a zero-size entry in a section this is reported as an error, e.g.: Offset 0x0 (0) is outside the section starting at 0x0 (0) Adjust the logic in CheckEntries() to avoid this. Signed-off-by: Simon Glass --- tools/binman/bsection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 44adb82..1c37d84 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -258,7 +258,7 @@ class Section(object): for entry in self._entries.values(): entry.CheckOffset() if (entry.offset < self._skip_at_start or - entry.offset >= self._skip_at_start + self._size): + entry.offset + entry.size > self._skip_at_start + self._size): entry.Raise("Offset %#x (%d) is outside the section starting " "at %#x (%d)" % (entry.offset, entry.offset, self._skip_at_start, From 0a98b28b06800da48f006069fe14e47dd399d2ff Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:28 -0600 Subject: [PATCH 23/46] binman: Support adding files In some cases it is useful to add a group of files to the image and be able to access them at run-time. Of course it is possible to generate the binman config file with a set of blobs each with a filename. But for convenience, add an entry type which can do this. Add required support (for adding nodes and string properties) into the state module. Signed-off-by: Simon Glass --- tools/binman/README.entries | 15 +++++++ tools/binman/bsection.py | 4 ++ tools/binman/control.py | 1 + tools/binman/entry.py | 3 ++ tools/binman/etype/files.py | 57 ++++++++++++++++++++++++++ tools/binman/ftest.py | 43 +++++++++++++++++++ tools/binman/image.py | 9 ++++ tools/binman/state.py | 27 ++++++++++++ tools/binman/test/84_files.dts | 11 +++++ tools/binman/test/85_files_compress.dts | 11 +++++ tools/binman/test/86_files_none.dts | 12 ++++++ tools/binman/test/87_files_no_pattern.dts | 11 +++++ tools/binman/test/files/1.dat | 1 + tools/binman/test/files/2.dat | 1 + tools/binman/test/files/ignored_dir.dat/ignore | 0 tools/binman/test/files/not-this-one | 1 + tools/patman/tools.py | 18 ++++++++ 17 files changed, 225 insertions(+) create mode 100644 tools/binman/etype/files.py create mode 100644 tools/binman/test/84_files.dts create mode 100644 tools/binman/test/85_files_compress.dts create mode 100644 tools/binman/test/86_files_none.dts create mode 100644 tools/binman/test/87_files_no_pattern.dts create mode 100644 tools/binman/test/files/1.dat create mode 100644 tools/binman/test/files/2.dat create mode 100644 tools/binman/test/files/ignored_dir.dat/ignore create mode 100644 tools/binman/test/files/not-this-one diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 2cf7dc0..3afc560 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -71,6 +71,21 @@ updating the EC on startup via software sync. +Entry: files: Entry containing a set of files +--------------------------------------------- + +Properties / Entry arguments: + - pattern: Filename pattern to match the files to include + - compress: Compression algorithm to use: + none: No compression + lz4: Use lz4 compression (via 'lz4' command-line utility) + +This entry reads a number of files and places each in a separate sub-entry +within this entry. To access these you need to enable device-tree updates +at run-time so you can obtain the file positions. + + + Entry: fill: An entry which is filled to a particular byte value ---------------------------------------------------------------- diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 1c37d84..4bf2068 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -103,6 +103,10 @@ class Section(object): def SetOffset(self, offset): self._offset = offset + def ExpandEntries(self): + for entry in self._entries.values(): + entry.ExpandEntries() + def AddMissingProperties(self): """Add new properties to the device tree as needed for this entry""" for prop in ['offset', 'size', 'image-pos']: diff --git a/tools/binman/control.py b/tools/binman/control.py index e326456..caa194c 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -146,6 +146,7 @@ def Binman(options, args): # without changing the device-tree size, thus ensuring that our # entry offsets remain the same. for image in images.values(): + image.ExpandEntries() if options.update_fdt: image.AddMissingProperties() image.ProcessFdt(dtb) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index f922107..7316ad4 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -179,6 +179,9 @@ class Entry(object): return Set([fname]) return Set() + def ExpandEntries(self): + pass + def AddMissingProperties(self): """Add new properties to the device tree as needed for this entry""" for prop in ['offset', 'size', 'image-pos']: diff --git a/tools/binman/etype/files.py b/tools/binman/etype/files.py new file mode 100644 index 0000000..99f2f2f --- /dev/null +++ b/tools/binman/etype/files.py @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass +# +# Entry-type module for a set of files which are placed in individual +# sub-entries +# + +import glob +import os + +from section import Entry_section +import fdt_util +import state +import tools + +import bsection + +class Entry_files(Entry_section): + """Entry containing a set of files + + Properties / Entry arguments: + - pattern: Filename pattern to match the files to include + - compress: Compression algorithm to use: + none: No compression + lz4: Use lz4 compression (via 'lz4' command-line utility) + + This entry reads a number of files and places each in a separate sub-entry + within this entry. To access these you need to enable device-tree updates + at run-time so you can obtain the file positions. + """ + def __init__(self, section, etype, node): + Entry_section.__init__(self, section, etype, node) + self._pattern = fdt_util.GetString(self._node, 'pattern') + if not self._pattern: + self.Raise("Missing 'pattern' property") + self._compress = fdt_util.GetString(self._node, 'compress', 'none') + self._require_matches = fdt_util.GetBool(self._node, + 'require-matches') + + def ExpandEntries(self): + files = tools.GetInputFilenameGlob(self._pattern) + if self._require_matches and not files: + self.Raise("Pattern '%s' matched no files" % self._pattern) + for fname in files: + if not os.path.isfile(fname): + continue + name = os.path.basename(fname) + subnode = self._node.FindNode(name) + if not subnode: + subnode = state.AddSubnode(self._node, name) + state.AddString(subnode, 'type', 'blob') + state.AddString(subnode, 'filename', fname) + state.AddString(subnode, 'compress', self._compress) + + # Read entries again, now that we have some + self._section._ReadEntries() diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1c3c46f..e919e70 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -54,6 +54,8 @@ CROS_EC_RW_DATA = 'ecrw' GBB_DATA = 'gbbd' BMPBLK_DATA = 'bmp' VBLOCK_DATA = 'vblk' +FILES_DATA = ("sorry I'm late\nOh, don't bother apologising, I'm " + + "sorry you're alive\n") COMPRESS_DATA = 'data to compress' @@ -117,6 +119,9 @@ class TestFunctional(unittest.TestCase): with open(self.TestFile('descriptor.bin')) as fd: TestFunctional._MakeInputFile('descriptor.bin', fd.read()) + shutil.copytree(self.TestFile('files'), + os.path.join(self._indir, 'files')) + TestFunctional._MakeInputFile('compress', COMPRESS_DATA) @classmethod @@ -1536,6 +1541,44 @@ class TestFunctional(unittest.TestCase): } self.assertEqual(expected, props) + def testFiles(self): + """Test bringing in multiple files""" + data = self._DoReadFile('84_files.dts') + self.assertEqual(FILES_DATA, data) + + def testFilesCompress(self): + """Test bringing in multiple files and compressing them""" + data = self._DoReadFile('85_files_compress.dts') + + image = control.images['image'] + entries = image.GetEntries() + files = entries['files'] + entries = files._section._entries + + orig = '' + for i in range(1, 3): + key = '%d.dat' % i + start = entries[key].image_pos + len = entries[key].size + chunk = data[start:start + len] + orig += self._decompress(chunk) + + self.assertEqual(FILES_DATA, orig) + + def testFilesMissing(self): + """Test missing files""" + with self.assertRaises(ValueError) as e: + data = self._DoReadFile('86_files_none.dts') + self.assertIn("Node '/binman/files': Pattern \'files/*.none\' matched " + 'no files', str(e.exception)) + + def testFilesNoPattern(self): + """Test missing files""" + with self.assertRaises(ValueError) as e: + data = self._DoReadFile('87_files_no_pattern.dts') + self.assertIn("Node '/binman/files': Missing 'pattern' property", + str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index bfb2229..4b922b5 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -58,6 +58,15 @@ class Image: """Get the set of device tree files used by this image""" return self._section.GetFdtSet() + def ExpandEntries(self): + """Expand out any entries which have calculated sub-entries + + Some entries are expanded out at runtime, e.g. 'files', which produces + a section containing a list of files. Process these entries so that + this information is added to the device tree. + """ + self._section.ExpandEntries() + def AddMissingProperties(self): """Add properties that are not present in the device tree diff --git a/tools/binman/state.py b/tools/binman/state.py index 09ead44..2f8c086 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -189,6 +189,33 @@ def AddZeroProp(node, prop): for n in GetUpdateNodes(node): n.AddZeroProp(prop) +def AddSubnode(node, name): + """Add a new subnode to a node in affected device trees + + Args: + node: Node to add to + name: name of node to add + + Returns: + New subnode that was created in main tree + """ + first = None + for n in GetUpdateNodes(node): + subnode = n.AddSubnode(name) + if not first: + first = subnode + return first + +def AddString(node, prop, value): + """Add a new string property to affected device trees + + Args: + prop_name: Name of property + value: String value (which will be \0-terminated in the DT) + """ + for n in GetUpdateNodes(node): + n.AddString(prop, value) + def SetInt(node, prop, value): """Update an integer property in affected device trees with an integer value diff --git a/tools/binman/test/84_files.dts b/tools/binman/test/84_files.dts new file mode 100644 index 0000000..83ddb78 --- /dev/null +++ b/tools/binman/test/84_files.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + files { + pattern = "files/*.dat"; + compress = "none"; + }; + }; +}; diff --git a/tools/binman/test/85_files_compress.dts b/tools/binman/test/85_files_compress.dts new file mode 100644 index 0000000..847b398 --- /dev/null +++ b/tools/binman/test/85_files_compress.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + files { + pattern = "files/*.dat"; + compress = "lz4"; + }; + }; +}; diff --git a/tools/binman/test/86_files_none.dts b/tools/binman/test/86_files_none.dts new file mode 100644 index 0000000..34bd92f --- /dev/null +++ b/tools/binman/test/86_files_none.dts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + files { + pattern = "files/*.none"; + compress = "none"; + require-matches; + }; + }; +}; diff --git a/tools/binman/test/87_files_no_pattern.dts b/tools/binman/test/87_files_no_pattern.dts new file mode 100644 index 0000000..0cb5b46 --- /dev/null +++ b/tools/binman/test/87_files_no_pattern.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + files { + compress = "none"; + require-matches; + }; + }; +}; diff --git a/tools/binman/test/files/1.dat b/tools/binman/test/files/1.dat new file mode 100644 index 0000000..a952470 --- /dev/null +++ b/tools/binman/test/files/1.dat @@ -0,0 +1 @@ +sorry I'm late diff --git a/tools/binman/test/files/2.dat b/tools/binman/test/files/2.dat new file mode 100644 index 0000000..687ea52 --- /dev/null +++ b/tools/binman/test/files/2.dat @@ -0,0 +1 @@ +Oh, don't bother apologising, I'm sorry you're alive diff --git a/tools/binman/test/files/ignored_dir.dat/ignore b/tools/binman/test/files/ignored_dir.dat/ignore new file mode 100644 index 0000000..e69de29 diff --git a/tools/binman/test/files/not-this-one b/tools/binman/test/files/not-this-one new file mode 100644 index 0000000..e71c225 --- /dev/null +++ b/tools/binman/test/files/not-this-one @@ -0,0 +1 @@ +this does not have a .dat extenion diff --git a/tools/patman/tools.py b/tools/patman/tools.py index 0870bcc..1c9bf4e 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -4,6 +4,7 @@ # import command +import glob import os import shutil import tempfile @@ -123,6 +124,23 @@ def GetInputFilename(fname): raise ValueError("Filename '%s' not found in input path (%s) (cwd='%s')" % (fname, ','.join(indir), os.getcwd())) +def GetInputFilenameGlob(pattern): + """Return a list of filenames for use as input. + + Args: + pattern: Filename pattern to search for + + Returns: + A list of matching files in all input directories + """ + if not indir: + return glob.glob(fname) + files = [] + for dirname in indir: + pathname = os.path.join(dirname, pattern) + files += glob.glob(pathname) + return sorted(files) + def Align(pos, align): if align: mask = align - 1 From ba64a0bbb7b7128479a97fdf58baa9ddfbfe4db6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:29 -0600 Subject: [PATCH 24/46] binman: Support expanding entries It is useful to have entries which can grow automatically to fill available space. Add support for this. Signed-off-by: Simon Glass --- tools/binman/README | 4 +++ tools/binman/bsection.py | 22 +++++++++++++++- tools/binman/entry.py | 11 ++++++++ tools/binman/etype/_testing.py | 7 +++++- tools/binman/etype/section.py | 8 ++++++ tools/binman/ftest.py | 29 +++++++++++++++++++++ tools/binman/test/88_expand_size.dts | 43 ++++++++++++++++++++++++++++++++ tools/binman/test/89_expand_size_bad.dts | 14 +++++++++++ 8 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 tools/binman/test/88_expand_size.dts create mode 100644 tools/binman/test/89_expand_size_bad.dts diff --git a/tools/binman/README b/tools/binman/README index d687194..6aa5b38 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -330,6 +330,10 @@ image-pos: for each entry. This makes it easy to find out exactly where the entry ended up in the image, regardless of parent sections, etc. +expand-size: + Expand the size of this entry to fit available space. This space is only + limited by the size of the image/section and the position of the next + entry. The attributes supported for images are described below. Several are similar to those for entries. diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 4bf2068..52ac31a 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -253,10 +253,26 @@ class Section(object): for entry in entries: self._entries[entry._node.name] = entry + def _ExpandEntries(self): + """Expand any entries that are permitted to""" + exp_entry = None + for entry in self._entries.values(): + if exp_entry: + exp_entry.ExpandToLimit(entry.offset) + exp_entry = None + if entry.expand_size: + exp_entry = entry + if exp_entry: + exp_entry.ExpandToLimit(self._size) + def CheckEntries(self): - """Check that entries do not overlap or extend outside the section""" + """Check that entries do not overlap or extend outside the section + + This also sorts entries, if needed and expands + """ if self._sort: self._SortEntries() + self._ExpandEntries() offset = 0 prev_name = 'None' for entry in self._entries.values(): @@ -419,3 +435,7 @@ class Section(object): return None return entry.data source_entry.Raise("Cannot find entry for node '%s'" % node.name) + + def ExpandSize(self, size): + if size != self._size: + self._size = size diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 7316ad4..0915b47 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -76,6 +76,7 @@ class Entry(object): self.pad_after = 0 self.offset_unset = False self.image_pos = None + self._expand_size = False if read_node: self.ReadNode() @@ -161,6 +162,7 @@ class Entry(object): "of two" % (self._node.path, self.align_size)) self.align_end = fdt_util.GetInt(self._node, 'align-end') self.offset_unset = fdt_util.GetBool(self._node, 'offset-unset') + self.expand_size = fdt_util.GetBool(self._node, 'expand-size') def GetDefaultFilename(self): return None @@ -507,3 +509,12 @@ features to produce new behaviours. break name = '%s.%s' % (node.name, name) return name + + def ExpandToLimit(self, limit): + """Expand an entry so that it ends at the given offset limit""" + if self.offset + self.size < limit: + self.size = limit - self.offset + # Request the contents again, since changing the size requires that + # the data grows. This should not fail, but check it to be sure. + if not self.ObtainContents(): + self.Raise('Cannot obtain contents when expanding entry') diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index 02c165c..3e345bd 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -48,6 +48,8 @@ class Entry__testing(Entry): 'return-unknown-contents') self.bad_update_contents = fdt_util.GetBool(self._node, 'bad-update-contents') + self.return_contents_once = fdt_util.GetBool(self._node, + 'return-contents-once') # Set to True when the entry is ready to process the FDT. self.process_fdt_ready = False @@ -68,12 +70,15 @@ class Entry__testing(Entry): EntryArg('test-existing-prop', str)], self.require_args) if self.force_bad_datatype: self.GetEntryArgsOrProps([EntryArg('test-bad-datatype-arg', bool)]) + self.return_contents = True def ObtainContents(self): - if self.return_unknown_contents: + if self.return_unknown_contents or not self.return_contents: return False self.data = 'a' self.contents_size = len(self.data) + if self.return_contents_once: + self.return_contents = False return True def GetOffsets(self): diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index a30cc91..005a9f9 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -40,6 +40,10 @@ class Entry_section(Entry): def ProcessFdt(self, fdt): return self._section.ProcessFdt(fdt) + def ExpandEntries(self): + Entry.ExpandEntries(self) + self._section.ExpandEntries() + def AddMissingProperties(self): Entry.AddMissingProperties(self) self._section.AddMissingProperties() @@ -95,3 +99,7 @@ class Entry_section(Entry): def GetEntries(self): return self._section.GetEntries() + + def ExpandToLimit(self, limit): + super(Entry_section, self).ExpandToLimit(limit) + self._section.ExpandSize(self.size) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index e919e70..b156943 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1579,6 +1579,35 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/files': Missing 'pattern' property", str(e.exception)) + def testExpandSize(self): + """Test an expanding entry""" + data, _, map_data, _ = self._DoReadFileDtb('88_expand_size.dts', + map=True) + expect = ('a' * 8 + U_BOOT_DATA + + MRC_DATA + 'b' * 1 + U_BOOT_DATA + + 'c' * 8 + U_BOOT_DATA + + 'd' * 8) + self.assertEqual(expect, data) + self.assertEqual('''ImagePos Offset Size Name +00000000 00000000 00000028 main-section +00000000 00000000 00000008 fill +00000008 00000008 00000004 u-boot +0000000c 0000000c 00000004 section +0000000c 00000000 00000003 intel-mrc +00000010 00000010 00000004 u-boot2 +00000014 00000014 0000000c section2 +00000014 00000000 00000008 fill +0000001c 00000008 00000004 u-boot +00000020 00000020 00000008 fill2 +''', map_data) + + def testExpandSizeBad(self): + """Test an expanding entry which fails to provide contents""" + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('89_expand_size_bad.dts', map=True) + self.assertIn("Node '/binman/_testing': Cannot obtain contents when " + 'expanding entry', str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/88_expand_size.dts b/tools/binman/test/88_expand_size.dts new file mode 100644 index 0000000..c8a0130 --- /dev/null +++ b/tools/binman/test/88_expand_size.dts @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + size = <40>; + fill { + expand-size; + fill-byte = [61]; + size = <0>; + }; + u-boot { + offset = <8>; + }; + section { + expand-size; + pad-byte = <0x62>; + intel-mrc { + }; + }; + u-boot2 { + type = "u-boot"; + offset = <16>; + }; + section2 { + type = "section"; + fill { + expand-size; + fill-byte = [63]; + size = <0>; + }; + u-boot { + offset = <8>; + }; + }; + fill2 { + type = "fill"; + expand-size; + fill-byte = [64]; + size = <0>; + }; + }; +}; diff --git a/tools/binman/test/89_expand_size_bad.dts b/tools/binman/test/89_expand_size_bad.dts new file mode 100644 index 0000000..edc0e5c --- /dev/null +++ b/tools/binman/test/89_expand_size_bad.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + _testing { + expand-size; + return-contents-once; + }; + u-boot { + offset = <8>; + }; + }; +}; From 9c888cca5e87e28e9addcffae9810fee481428a8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:30 -0600 Subject: [PATCH 25/46] binman: Mention section attributes in docs Images and sections have the same attributes, since an image is mostly just a top-level section. Update the docs to explain this. Signed-off-by: Simon Glass --- tools/binman/README | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/binman/README b/tools/binman/README index 6aa5b38..cf1a06d 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -335,8 +335,8 @@ expand-size: limited by the size of the image/section and the position of the next entry. -The attributes supported for images are described below. Several are similar -to those for entries. +The attributes supported for images and sections are described below. Several +are similar to those for entries. size: Sets the image size in bytes, for example 'size = <0x100000>' for a From e0e5df9310d3a0e1fc0eda86ff43fd3e782e61f1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:31 -0600 Subject: [PATCH 26/46] binman: Support hashing entries Sometimesi it us useful to be able to verify the content of entries with a hash. Add an easy way to do this in binman. The hash information can be retrieved from the device tree at run time. Signed-off-by: Simon Glass --- tools/binman/README | 22 +++++++++++++++++++++ tools/binman/bsection.py | 3 +++ tools/binman/entry.py | 4 ++++ tools/binman/ftest.py | 36 ++++++++++++++++++++++++++++++++++ tools/binman/state.py | 25 +++++++++++++++++++++++ tools/binman/test/90_hash.dts | 12 ++++++++++++ tools/binman/test/91_hash_no_algo.dts | 11 +++++++++++ tools/binman/test/92_hash_bad_algo.dts | 12 ++++++++++++ tools/binman/test/99_hash_section.dts | 18 +++++++++++++++++ 9 files changed, 143 insertions(+) create mode 100644 tools/binman/test/90_hash.dts create mode 100644 tools/binman/test/91_hash_no_algo.dts create mode 100644 tools/binman/test/92_hash_bad_algo.dts create mode 100644 tools/binman/test/99_hash_section.dts diff --git a/tools/binman/README b/tools/binman/README index cf1a06d..088f3a6 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -466,6 +466,28 @@ see README.entries. This is generated from the source code using: binman -E >tools/binman/README.entries +Hashing Entries +--------------- + +It is possible to ask binman to hash the contents of an entry and write that +value back to the device-tree node. For example: + + binman { + u-boot { + hash { + algo = "sha256"; + }; + }; + }; + +Here, a new 'value' property will be written to the 'hash' node containing +the hash of the 'u-boot' entry. Only SHA256 is supported at present. Whole +sections can be hased if desired, by adding the 'hash' node to the section. + +The has value can be chcked at runtime by hashing the data actually read and +comparing this has to the value in the device tree. + + Order of image creation ----------------------- diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 52ac31a..650e9ba 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -89,6 +89,8 @@ class Section(object): def _ReadEntries(self): for node in self._node.subnodes: + if node.name == 'hash': + continue entry = Entry.Create(self, node) entry.SetPrefix(self._name_prefix) self._entries[node.name] = entry @@ -112,6 +114,7 @@ class Section(object): for prop in ['offset', 'size', 'image-pos']: if not prop in self._node.props: state.AddZeroProp(self._node, prop) + state.CheckAddHashProp(self._node) for entry in self._entries.values(): entry.AddMissingProperties() diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 0915b47..fd72234 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -189,12 +189,16 @@ class Entry(object): for prop in ['offset', 'size', 'image-pos']: if not prop in self._node.props: state.AddZeroProp(self._node, prop) + err = state.CheckAddHashProp(self._node) + if err: + self.Raise(err) def SetCalculatedProperties(self): """Set the value of device-tree properties calculated by binman""" state.SetInt(self._node, 'offset', self.offset) state.SetInt(self._node, 'size', self.size) state.SetInt(self._node, 'image-pos', self.image_pos) + state.CheckSetHashValue(self._node, self.GetData) def ProcessFdt(self, fdt): """Allow entries to adjust the device tree diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index b156943..c46a065 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -6,6 +6,7 @@ # # python -m unittest func_test.TestFunctional.testHelp +import hashlib from optparse import OptionParser import os import shutil @@ -1608,6 +1609,41 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/_testing': Cannot obtain contents when " 'expanding entry', str(e.exception)) + def testHash(self): + """Test hashing of the contents of an entry""" + _, _, _, out_dtb_fname = self._DoReadFileDtb('90_hash.dts', + use_real_dtb=True, update_dtb=True) + dtb = fdt.Fdt(out_dtb_fname) + dtb.Scan() + hash_node = dtb.GetNode('/binman/u-boot/hash').props['value'] + m = hashlib.sha256() + m.update(U_BOOT_DATA) + self.assertEqual(m.digest(), ''.join(hash_node.value)) + + def testHashNoAlgo(self): + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('91_hash_no_algo.dts', update_dtb=True) + self.assertIn("Node \'/binman/u-boot\': Missing \'algo\' property for " + 'hash node', str(e.exception)) + + def testHashBadAlgo(self): + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('92_hash_bad_algo.dts', update_dtb=True) + self.assertIn("Node '/binman/u-boot': Unknown hash algorithm", + str(e.exception)) + + def testHashSection(self): + """Test hashing of the contents of an entry""" + _, _, _, out_dtb_fname = self._DoReadFileDtb('99_hash_section.dts', + use_real_dtb=True, update_dtb=True) + dtb = fdt.Fdt(out_dtb_fname) + dtb.Scan() + hash_node = dtb.GetNode('/binman/section/hash').props['value'] + m = hashlib.sha256() + m.update(U_BOOT_DATA) + m.update(16 * 'a') + self.assertEqual(m.digest(), ''.join(hash_node.value)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/state.py b/tools/binman/state.py index 2f8c086..d945e4b 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -5,6 +5,7 @@ # Holds and modifies the state information held by binman # +import hashlib import re from sets import Set @@ -226,3 +227,27 @@ def SetInt(node, prop, value): """ for n in GetUpdateNodes(node): n.SetInt(prop, value) + +def CheckAddHashProp(node): + hash_node = node.FindNode('hash') + if hash_node: + algo = hash_node.props.get('algo') + if not algo: + return "Missing 'algo' property for hash node" + if algo.value == 'sha256': + size = 32 + else: + return "Unknown hash algorithm '%s'" % algo + for n in GetUpdateNodes(hash_node): + n.AddEmptyProp('value', size) + +def CheckSetHashValue(node, get_data_func): + hash_node = node.FindNode('hash') + if hash_node: + algo = hash_node.props.get('algo').value + if algo == 'sha256': + m = hashlib.sha256() + m.update(get_data_func()) + data = m.digest() + for n in GetUpdateNodes(hash_node): + n.SetData('value', data) diff --git a/tools/binman/test/90_hash.dts b/tools/binman/test/90_hash.dts new file mode 100644 index 0000000..2003045 --- /dev/null +++ b/tools/binman/test/90_hash.dts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + u-boot { + hash { + algo = "sha256"; + }; + }; + }; +}; diff --git a/tools/binman/test/91_hash_no_algo.dts b/tools/binman/test/91_hash_no_algo.dts new file mode 100644 index 0000000..b64df20 --- /dev/null +++ b/tools/binman/test/91_hash_no_algo.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + u-boot { + hash { + }; + }; + }; +}; diff --git a/tools/binman/test/92_hash_bad_algo.dts b/tools/binman/test/92_hash_bad_algo.dts new file mode 100644 index 0000000..d240200 --- /dev/null +++ b/tools/binman/test/92_hash_bad_algo.dts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + u-boot { + hash { + algo = "invalid"; + }; + }; + }; +}; diff --git a/tools/binman/test/99_hash_section.dts b/tools/binman/test/99_hash_section.dts new file mode 100644 index 0000000..dcd8683 --- /dev/null +++ b/tools/binman/test/99_hash_section.dts @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + section { + u-boot { + }; + fill { + size = <0x10>; + fill-byte = [61]; + }; + hash { + algo = "sha256"; + }; + }; + }; +}; From f025363543636191cfc6d277733317cb0198189f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:32 -0600 Subject: [PATCH 27/46] binman: Support x86 microcode in TPL When TPL is used on x86 we may want to program the microcode (at least for the first CPU) early in boot. Add support for this by refactoring the existing code to be more generic. Signed-off-by: Simon Glass --- tools/binman/README.entries | 20 +++++++++++++++++ tools/binman/etype/u_boot_dtb_with_ucode.py | 15 ++++++++----- tools/binman/etype/u_boot_spl_with_ucode_ptr.py | 2 ++ tools/binman/etype/u_boot_tpl_dtb_with_ucode.py | 25 +++++++++++++++++++++ tools/binman/etype/u_boot_tpl_with_ucode_ptr.py | 27 +++++++++++++++++++++++ tools/binman/etype/u_boot_ucode.py | 26 ++++++++++++---------- tools/binman/etype/u_boot_with_ucode_ptr.py | 9 +++++--- tools/binman/ftest.py | 19 ++++++++++++++++ tools/binman/test/93_x86_tpl_ucode.dts | 29 +++++++++++++++++++++++++ 9 files changed, 151 insertions(+), 21 deletions(-) create mode 100644 tools/binman/etype/u_boot_tpl_dtb_with_ucode.py create mode 100644 tools/binman/etype/u_boot_tpl_with_ucode_ptr.py create mode 100644 tools/binman/test/93_x86_tpl_ucode.dts diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 3afc560..4dd67d6 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -462,6 +462,8 @@ both SPL and the device tree). Entry: u-boot-spl-with-ucode-ptr: U-Boot SPL with embedded microcode pointer ---------------------------------------------------------------------------- +This is used when SPL must set up the microcode for U-Boot. + See Entry_u_boot_ucode for full details of the entries involved in this process. @@ -503,6 +505,24 @@ to activate. +Entry: u-boot-tpl-dtb-with-ucode: U-Boot TPL with embedded microcode pointer +---------------------------------------------------------------------------- + +This is used when TPL must set up the microcode for U-Boot. + +See Entry_u_boot_ucode for full details of the entries involved in this +process. + + + +Entry: u-boot-tpl-with-ucode-ptr: U-Boot TPL with embedded microcode pointer +---------------------------------------------------------------------------- + +See Entry_u_boot_ucode for full details of the entries involved in this +process. + + + Entry: u-boot-ucode: U-Boot microcode block ------------------------------------------- diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py index 73f5fbf..444c51b 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -45,6 +45,9 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): 'u-boot-spl-with-ucode-ptr') if not ucode_dest_entry or not ucode_dest_entry.target_offset: ucode_dest_entry = self.section.FindEntryType( + 'u-boot-tpl-with-ucode-ptr') + if not ucode_dest_entry or not ucode_dest_entry.target_offset: + ucode_dest_entry = self.section.FindEntryType( 'u-boot-with-ucode-ptr') if not ucode_dest_entry or not ucode_dest_entry.target_offset: return True @@ -70,14 +73,14 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob_dtb): def ObtainContents(self): # Call the base class just in case it does something important. Entry_blob_dtb.ObtainContents(self) - self._pathname = state.GetFdtPath(self._filename) - self.ReadBlobContents() - if self.ucode: + if self.ucode and not self.collate: for node in self.ucode.subnodes: data_prop = node.props.get('data') - if data_prop and not self.collate: + if data_prop: # Find the offset in the device tree of the ucode data self.ucode_offset = data_prop.GetOffset() + 12 self.ucode_size = len(data_prop.bytes) - self.ready = True - return True + self.ready = True + else: + self.ready = True + return self.ready diff --git a/tools/binman/etype/u_boot_spl_with_ucode_ptr.py b/tools/binman/etype/u_boot_spl_with_ucode_ptr.py index 0dfe268..b650cf0 100644 --- a/tools/binman/etype/u_boot_spl_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_spl_with_ucode_ptr.py @@ -16,6 +16,8 @@ import tools class Entry_u_boot_spl_with_ucode_ptr(Entry_u_boot_with_ucode_ptr): """U-Boot SPL with embedded microcode pointer + This is used when SPL must set up the microcode for U-Boot. + See Entry_u_boot_ucode for full details of the entries involved in this process. """ diff --git a/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py b/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py new file mode 100644 index 0000000..71e04fc --- /dev/null +++ b/tools/binman/etype/u_boot_tpl_dtb_with_ucode.py @@ -0,0 +1,25 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass +# +# Entry-type module for U-Boot device tree with the microcode removed +# + +import control +from entry import Entry +from u_boot_dtb_with_ucode import Entry_u_boot_dtb_with_ucode +import tools + +class Entry_u_boot_tpl_dtb_with_ucode(Entry_u_boot_dtb_with_ucode): + """U-Boot TPL with embedded microcode pointer + + This is used when TPL must set up the microcode for U-Boot. + + See Entry_u_boot_ucode for full details of the entries involved in this + process. + """ + def __init__(self, section, etype, node): + Entry_u_boot_dtb_with_ucode.__init__(self, section, etype, node) + + def GetDefaultFilename(self): + return 'tpl/u-boot-tpl.dtb' diff --git a/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py b/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py new file mode 100644 index 0000000..8d94dde --- /dev/null +++ b/tools/binman/etype/u_boot_tpl_with_ucode_ptr.py @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2016 Google, Inc +# Written by Simon Glass +# +# Entry-type module for an TPL binary with an embedded microcode pointer +# + +import struct + +import command +from entry import Entry +from blob import Entry_blob +from u_boot_with_ucode_ptr import Entry_u_boot_with_ucode_ptr +import tools + +class Entry_u_boot_tpl_with_ucode_ptr(Entry_u_boot_with_ucode_ptr): + """U-Boot TPL with embedded microcode pointer + + See Entry_u_boot_ucode for full details of the entries involved in this + process. + """ + def __init__(self, section, etype, node): + Entry_u_boot_with_ucode_ptr.__init__(self, section, etype, node) + self.elf_fname = 'tpl/u-boot-tpl' + + def GetDefaultFilename(self): + return 'tpl/u-boot-tpl-nodtb.bin' diff --git a/tools/binman/etype/u_boot_ucode.py b/tools/binman/etype/u_boot_ucode.py index 6acf94d..a00e530 100644 --- a/tools/binman/etype/u_boot_ucode.py +++ b/tools/binman/etype/u_boot_ucode.py @@ -62,19 +62,24 @@ class Entry_u_boot_ucode(Entry_blob): def ObtainContents(self): # If the section does not need microcode, there is nothing to do - ucode_dest_entry = self.section.FindEntryType('u-boot-with-ucode-ptr') - ucode_dest_entry_spl = self.section.FindEntryType( - 'u-boot-spl-with-ucode-ptr') - if ((not ucode_dest_entry or not ucode_dest_entry.target_offset) and - (not ucode_dest_entry_spl or not ucode_dest_entry_spl.target_offset)): + found = False + for suffix in ['', '-spl', '-tpl']: + name = 'u-boot%s-with-ucode-ptr' % suffix + entry = self.section.FindEntryType(name) + if entry and entry.target_offset: + found = True + if not found: self.data = '' return True - # Get the microcode from the device tree entry. If it is not available # yet, return False so we will be called later. If the section simply # doesn't exist, then we may as well return True, since we are going to # get an error anyway. - fdt_entry = self.section.FindEntryType('u-boot-dtb-with-ucode') + for suffix in ['', '-spl', '-tpl']: + name = 'u-boot%s-dtb-with-ucode' % suffix + fdt_entry = self.section.FindEntryType(name) + if fdt_entry: + break if not fdt_entry: return True if not fdt_entry.ready: @@ -86,12 +91,9 @@ class Entry_u_boot_ucode(Entry_blob): return True # Write it out to a file - dtb_name = 'u-boot-ucode.bin' - fname = tools.GetOutputFilename(dtb_name) - with open(fname, 'wb') as fd: - fd.write(fdt_entry.ucode_data) + self._pathname = tools.GetOutputFilename('u-boot-ucode.bin') + tools.WriteFile(self._pathname, fdt_entry.ucode_data) - self._pathname = fname self.ReadBlobContents() return True diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index c850b59..01f3513 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -74,13 +74,16 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): # Get the microcode, either from u-boot-ucode or u-boot-dtb-with-ucode. # If we have left the microcode in the device tree, then it will be - # in the former. If we extracted the microcode from the device tree - # and collated it in one place, it will be in the latter. + # in the latter. If we extracted the microcode from the device tree + # and collated it in one place, it will be in the former. if ucode_entry.size: offset, size = ucode_entry.offset, ucode_entry.size else: dtb_entry = self.section.FindEntryType('u-boot-dtb-with-ucode') - if not dtb_entry or not dtb_entry.ready: + if not dtb_entry: + dtb_entry = self.section.FindEntryType( + 'u-boot-tpl-dtb-with-ucode') + if not dtb_entry: self.Raise('Cannot find microcode region u-boot-dtb-with-ucode') offset = dtb_entry.offset + dtb_entry.ucode_offset size = dtb_entry.ucode_size diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index c46a065..f8faef1 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -44,6 +44,7 @@ X86_START16_SPL_DATA = 'start16spl' X86_START16_TPL_DATA = 'start16tpl' U_BOOT_NODTB_DATA = 'nodtb with microcode pointer somewhere in here' U_BOOT_SPL_NODTB_DATA = 'splnodtb with microcode pointer somewhere in here' +U_BOOT_TPL_NODTB_DATA = 'tplnodtb with microcode pointer somewhere in here' FSP_DATA = 'fsp' CMC_DATA = 'cmc' VBT_DATA = 'vbt' @@ -103,6 +104,8 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('u-boot-nodtb.bin', U_BOOT_NODTB_DATA) TestFunctional._MakeInputFile('spl/u-boot-spl-nodtb.bin', U_BOOT_SPL_NODTB_DATA) + TestFunctional._MakeInputFile('tpl/u-boot-tpl-nodtb.bin', + U_BOOT_TPL_NODTB_DATA) TestFunctional._MakeInputFile('fsp.bin', FSP_DATA) TestFunctional._MakeInputFile('cmc.bin', CMC_DATA) TestFunctional._MakeInputFile('vbt.bin', VBT_DATA) @@ -1644,6 +1647,22 @@ class TestFunctional(unittest.TestCase): m.update(16 * 'a') self.assertEqual(m.digest(), ''.join(hash_node.value)) + def testPackUBootTplMicrocode(self): + """Test that x86 microcode can be handled correctly in TPL + + We expect to see the following in the image, in order: + u-boot-tpl-nodtb.bin with a microcode pointer inserted at the correct + place + u-boot-tpl.dtb with the microcode removed + the microcode + """ + with open(self.TestFile('u_boot_ucode_ptr')) as fd: + TestFunctional._MakeInputFile('tpl/u-boot-tpl', fd.read()) + first, pos_and_size = self._RunMicrocodeTest('93_x86_tpl_ucode.dts', + U_BOOT_TPL_NODTB_DATA) + self.assertEqual('tplnodtb with microc' + pos_and_size + + 'ter somewhere in here', first) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/93_x86_tpl_ucode.dts b/tools/binman/test/93_x86_tpl_ucode.dts new file mode 100644 index 0000000..d7ed9fc --- /dev/null +++ b/tools/binman/test/93_x86_tpl_ucode.dts @@ -0,0 +1,29 @@ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-offset; + end-at-4gb; + size = <0x200>; + u-boot-tpl-with-ucode-ptr { + }; + + u-boot-tpl-dtb-with-ucode { + }; + + u-boot-ucode { + }; + }; + + microcode { + update@0 { + data = <0x12345678 0x12345679>; + }; + update@1 { + data = <0xabcd0000 0x78235609>; + }; + }; +}; From 08723a7abbc7e28b22d18684faf5142fc6f155e8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:33 -0600 Subject: [PATCH 28/46] binman: Record the parent section of each section At present sections have no record of their parent so it is not possible to traverse up the tree to the root and figure out the position of a section within the image. Change the constructor to record this information. Signed-off-by: Simon Glass --- tools/binman/bsection.py | 5 ++++- tools/binman/etype/section.py | 3 ++- tools/binman/image.py | 5 +++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 650e9ba..e4c1900 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -24,6 +24,7 @@ class Section(object): Attributes: _node: Node object that contains the section definition in device tree + _parent_section: Parent Section object which created this Section _size: Section size in bytes, or None if not known yet _align_size: Section size alignment, or None _pad_before: Number of bytes before the first entry starts. This @@ -46,14 +47,16 @@ class Section(object): section _entries: OrderedDict() of entries """ - def __init__(self, name, node, test=False): + def __init__(self, name, parent_section, node, image, test=False): global entry global Entry import entry from entry import Entry + self._parent_section = parent_section self._name = name self._node = node + self._image = image self._offset = 0 self._size = None self._align_size = None diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 005a9f9..7f1b413 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -32,7 +32,8 @@ class Entry_section(Entry): """ def __init__(self, section, etype, node): Entry.__init__(self, section, etype, node) - self._section = bsection.Section(node.name, node) + self._section = bsection.Section(node.name, section, node, + section._image) def GetFdtSet(self): return self._section.GetFdtSet() diff --git a/tools/binman/image.py b/tools/binman/image.py index 4b922b5..e113a60 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -42,7 +42,8 @@ class Image: self._size = None self._filename = '%s.bin' % self._name if test: - self._section = bsection.Section('main-section', self._node, True) + self._section = bsection.Section('main-section', None, self._node, + self, True) else: self._ReadNode() @@ -52,7 +53,7 @@ class Image: filename = fdt_util.GetString(self._node, 'filename') if filename: self._filename = filename - self._section = bsection.Section('main-section', self._node) + self._section = bsection.Section('main-section', None, self._node, self) def GetFdtSet(self): """Get the set of device tree files used by this image""" From f8f8df6eb870b53e025aa447f8d40cd2ce2a77f6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:34 -0600 Subject: [PATCH 29/46] binman: Correct fmap output on x86 Normally x86 platforms use the end-at-4gb option. This currently produces an FMAP with positions which have a large offset. The use of end-at-4gb is a useful convenience within binman, but we don't really want to export a map with these offsets. Fix this by subtracting the 'skip at start' parameter. Also put the code which convers names to fmap format, for clarity. Signed-off-by: Simon Glass --- tools/binman/bsection.py | 18 +++++++++--- tools/binman/entry.py | 3 +- tools/binman/etype/fmap.py | 11 ++++--- tools/binman/etype/u_boot_with_ucode_ptr.py | 12 ++++---- tools/binman/fmap_util.py | 6 +++- tools/binman/ftest.py | 45 +++++++++++++++++++++++++++++ tools/binman/test/94_fmap_x86.dts | 20 +++++++++++++ tools/binman/test/95_fmap_x86_section.dts | 22 ++++++++++++++ 8 files changed, 121 insertions(+), 16 deletions(-) create mode 100644 tools/binman/test/94_fmap_x86.dts create mode 100644 tools/binman/test/95_fmap_x86_section.dts diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index e4c1900..c208029 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -68,6 +68,7 @@ class Section(object): self._end_4gb = False self._name_prefix = '' self._entries = OrderedDict() + self._image_pos = None if not test: self._ReadNode() self._ReadEntries() @@ -124,7 +125,10 @@ class Section(object): def SetCalculatedProperties(self): state.SetInt(self._node, 'offset', self._offset) state.SetInt(self._node, 'size', self._size) - state.SetInt(self._node, 'image-pos', self._image_pos) + image_pos = self._image_pos + if self._parent_section: + image_pos -= self._parent_section.GetRootSkipAtStart() + state.SetInt(self._node, 'image-pos', image_pos) for entry in self._entries.values(): entry.SetCalculatedProperties() @@ -437,11 +441,17 @@ class Section(object): source_entry.Raise("Cannot find node for phandle %d" % phandle) for entry in self._entries.values(): if entry._node == node: - if entry.data is None: - return None - return entry.data + return entry.GetData() source_entry.Raise("Cannot find entry for node '%s'" % node.name) def ExpandSize(self, size): if size != self._size: self._size = size + + def GetRootSkipAtStart(self): + if self._parent_section: + return self._parent_section.GetRootSkipAtStart() + return self._skip_at_start + + def GetImageSize(self): + return self._image._size diff --git a/tools/binman/entry.py b/tools/binman/entry.py index fd72234..01be291 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -197,7 +197,8 @@ class Entry(object): """Set the value of device-tree properties calculated by binman""" state.SetInt(self._node, 'offset', self.offset) state.SetInt(self._node, 'size', self.size) - state.SetInt(self._node, 'image-pos', self.image_pos) + state.SetInt(self._node, 'image-pos', + self.image_pos - self.section.GetRootSkipAtStart()) state.CheckSetHashValue(self._node, self.GetData) def ProcessFdt(self, fdt): diff --git a/tools/binman/etype/fmap.py b/tools/binman/etype/fmap.py index f1dd81e..bf35a5b 100644 --- a/tools/binman/etype/fmap.py +++ b/tools/binman/etype/fmap.py @@ -42,14 +42,17 @@ class Entry_fmap(Entry): for subentry in entries.values(): _AddEntries(areas, subentry) else: - areas.append(fmap_util.FmapArea(entry.image_pos or 0, - entry.size or 0, entry.name, 0)) + pos = entry.image_pos + if pos is not None: + pos -= entry.section.GetRootSkipAtStart() + areas.append(fmap_util.FmapArea(pos or 0, entry.size or 0, + entry.name, 0)) - entries = self.section.GetEntries() + entries = self.section._image.GetEntries() areas = [] for entry in entries.values(): _AddEntries(areas, entry) - return fmap_util.EncodeFmap(self.section.GetSize() or 0, self.name, + return fmap_util.EncodeFmap(self.section.GetImageSize() or 0, self.name, areas) def ObtainContents(self): diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 01f3513..da0e124 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -66,11 +66,11 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): # the U-Boot region must start at offset 7MB in the section. In this # case the ROM starts at 0xff800000, so the offset of the first # entry in the section corresponds to that. - if (self.target_offset < self.offset or - self.target_offset >= self.offset + self.size): - self.Raise('Microcode pointer _dt_ucode_base_size at %08x is ' - 'outside the section ranging from %08x to %08x' % - (self.target_offset, self.offset, self.offset + self.size)) + if (self.target_offset < self.image_pos or + self.target_offset >= self.image_pos + self.size): + self.Raise('Microcode pointer _dt_ucode_base_size at %08x is outside the section ranging from %08x to %08x' % + (self.target_offset, self.image_pos, + self.image_pos + self.size)) # Get the microcode, either from u-boot-ucode or u-boot-dtb-with-ucode. # If we have left the microcode in the device tree, then it will be @@ -90,7 +90,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): # Write the microcode offset and size into the entry offset_and_size = struct.pack('<2L', offset, size) - self.target_offset -= self.offset + self.target_offset -= self.image_pos self.ProcessContentsUpdate(self.data[:self.target_offset] + offset_and_size + self.data[self.target_offset + 8:]) diff --git a/tools/binman/fmap_util.py b/tools/binman/fmap_util.py index 7d520e3..be3cbee 100644 --- a/tools/binman/fmap_util.py +++ b/tools/binman/fmap_util.py @@ -49,6 +49,9 @@ FmapHeader = collections.namedtuple('FmapHeader', FMAP_HEADER_NAMES) FmapArea = collections.namedtuple('FmapArea', FMAP_AREA_NAMES) +def NameToFmap(name): + return name.replace('\0', '').replace('-', '_').upper() + def ConvertName(field_names, fields): """Convert a name to something flashrom likes @@ -62,7 +65,7 @@ def ConvertName(field_names, fields): value: value of that field (string for the ones we support) """ name_index = field_names.index('name') - fields[name_index] = fields[name_index].replace('\0', '').replace('-', '_').upper() + fields[name_index] = NameToFmap(fields[name_index]) def DecodeFmap(data): """Decode a flashmap into a header and list of areas @@ -100,6 +103,7 @@ def EncodeFmap(image_size, name, areas): """ def _FormatBlob(fmt, names, obj): params = [getattr(obj, name) for name in names] + ConvertName(names, params) return struct.pack(fmt, *params) values = FmapHeader(FMAP_SIGNATURE, 1, 0, 0, image_size, name, len(areas)) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index f8faef1..1abb768 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1663,6 +1663,51 @@ class TestFunctional(unittest.TestCase): self.assertEqual('tplnodtb with microc' + pos_and_size + 'ter somewhere in here', first) + def testFmapX86(self): + """Basic test of generation of a flashrom fmap""" + data = self._DoReadFile('94_fmap_x86.dts') + fhdr, fentries = fmap_util.DecodeFmap(data[32:]) + expected = U_BOOT_DATA + MRC_DATA + 'a' * (32 - 7) + self.assertEqual(expected, data[:32]) + fhdr, fentries = fmap_util.DecodeFmap(data[32:]) + + self.assertEqual(0x100, fhdr.image_size) + + self.assertEqual(0, fentries[0].offset) + self.assertEqual(4, fentries[0].size) + self.assertEqual('U_BOOT', fentries[0].name) + + self.assertEqual(4, fentries[1].offset) + self.assertEqual(3, fentries[1].size) + self.assertEqual('INTEL_MRC', fentries[1].name) + + self.assertEqual(32, fentries[2].offset) + self.assertEqual(fmap_util.FMAP_HEADER_LEN + + fmap_util.FMAP_AREA_LEN * 3, fentries[2].size) + self.assertEqual('FMAP', fentries[2].name) + + def testFmapX86Section(self): + """Basic test of generation of a flashrom fmap""" + data = self._DoReadFile('95_fmap_x86_section.dts') + expected = U_BOOT_DATA + MRC_DATA + 'b' * (32 - 7) + self.assertEqual(expected, data[:32]) + fhdr, fentries = fmap_util.DecodeFmap(data[36:]) + + self.assertEqual(0x100, fhdr.image_size) + + self.assertEqual(0, fentries[0].offset) + self.assertEqual(4, fentries[0].size) + self.assertEqual('U_BOOT', fentries[0].name) + + self.assertEqual(4, fentries[1].offset) + self.assertEqual(3, fentries[1].size) + self.assertEqual('INTEL_MRC', fentries[1].name) + + self.assertEqual(36, fentries[2].offset) + self.assertEqual(fmap_util.FMAP_HEADER_LEN + + fmap_util.FMAP_AREA_LEN * 3, fentries[2].size) + self.assertEqual('FMAP', fentries[2].name) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/94_fmap_x86.dts b/tools/binman/test/94_fmap_x86.dts new file mode 100644 index 0000000..613c5da --- /dev/null +++ b/tools/binman/test/94_fmap_x86.dts @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + end-at-4gb; + size = <0x100>; + pad-byte = <0x61>; + u-boot { + }; + intel-mrc { + }; + fmap { + offset = <0xffffff20>; + }; + }; +}; diff --git a/tools/binman/test/95_fmap_x86_section.dts b/tools/binman/test/95_fmap_x86_section.dts new file mode 100644 index 0000000..4cfce45 --- /dev/null +++ b/tools/binman/test/95_fmap_x86_section.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + end-at-4gb; + size = <0x100>; + u-boot { + }; + section { + pad-byte = <0x62>; + intel-mrc { + }; + fmap { + offset = <0x20>; + }; + }; + }; +}; From fe1ae3ecc3a2203babd7837bd2d5cf514a374c1f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:35 -0600 Subject: [PATCH 30/46] binman: Support ELF files for U-Boot and SPL For sandbox we want to put ELF files in the image since that is what we need to execute. Add support for this. Signed-off-by: Simon Glass --- tools/binman/README.entries | 22 ++++++++++++++++++++ tools/binman/etype/u_boot_elf.py | 39 ++++++++++++++++++++++++++++++++++++ tools/binman/etype/u_boot_spl_elf.py | 24 ++++++++++++++++++++++ tools/binman/ftest.py | 16 +++++++++++++++ tools/binman/test/96_elf.dts | 14 +++++++++++++ tools/binman/test/97_elf_strip.dts | 15 ++++++++++++++ 6 files changed, 130 insertions(+) create mode 100644 tools/binman/etype/u_boot_elf.py create mode 100644 tools/binman/etype/u_boot_spl_elf.py create mode 100644 tools/binman/test/96_elf.dts create mode 100644 tools/binman/test/97_elf_strip.dts diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 4dd67d6..69b435f 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -361,6 +361,17 @@ it available to u_boot_ucode. +Entry: u-boot-elf: U-Boot ELF image +----------------------------------- + +Properties / Entry arguments: + - filename: Filename of u-boot (default 'u-boot') + +This is the U-Boot ELF image. It does not include a device tree but can be +relocated to any address for execution. + + + Entry: u-boot-img: U-Boot legacy image -------------------------------------- @@ -444,6 +455,17 @@ to activate. +Entry: u-boot-spl-elf: U-Boot SPL ELF image +------------------------------------------- + +Properties / Entry arguments: + - filename: Filename of SPL u-boot (default 'spl/u-boot') + +This is the U-Boot SPL ELF image. It does not include a device tree but can +be relocated to any address for execution. + + + Entry: u-boot-spl-nodtb: SPL binary without device tree appended ---------------------------------------------------------------- diff --git a/tools/binman/etype/u_boot_elf.py b/tools/binman/etype/u_boot_elf.py new file mode 100644 index 0000000..134b6cc --- /dev/null +++ b/tools/binman/etype/u_boot_elf.py @@ -0,0 +1,39 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass +# +# Entry-type module for U-Boot ELF image +# + +from entry import Entry +from blob import Entry_blob + +import fdt_util +import tools + +class Entry_u_boot_elf(Entry_blob): + """U-Boot ELF image + + Properties / Entry arguments: + - filename: Filename of u-boot (default 'u-boot') + + This is the U-Boot ELF image. It does not include a device tree but can be + relocated to any address for execution. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + self._strip = fdt_util.GetBool(self._node, 'strip') + + def ReadBlobContents(self): + if self._strip: + uniq = self.GetUniqueName() + out_fname = tools.GetOutputFilename('%s.stripped' % uniq) + tools.WriteFile(out_fname, tools.ReadFile(self._pathname)) + tools.Run('strip', out_fname) + self.SetContents(tools.ReadFile(out_fname)) + else: + self.SetContents(tools.ReadFile(self._pathname)) + return True + + def GetDefaultFilename(self): + return 'u-boot' diff --git a/tools/binman/etype/u_boot_spl_elf.py b/tools/binman/etype/u_boot_spl_elf.py new file mode 100644 index 0000000..da328ae --- /dev/null +++ b/tools/binman/etype/u_boot_spl_elf.py @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass +# +# Entry-type module for U-Boot SPL ELF image +# + +from entry import Entry +from blob import Entry_blob + +class Entry_u_boot_spl_elf(Entry_blob): + """U-Boot SPL ELF image + + Properties / Entry arguments: + - filename: Filename of SPL u-boot (default 'spl/u-boot') + + This is the U-Boot SPL ELF image. It does not include a device tree but can + be relocated to any address for execution. + """ + def __init__(self, section, etype, node): + Entry_blob.__init__(self, section, etype, node) + + def GetDefaultFilename(self): + return 'spl/u-boot-spl' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 1abb768..27dca3a 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1708,6 +1708,22 @@ class TestFunctional(unittest.TestCase): fmap_util.FMAP_AREA_LEN * 3, fentries[2].size) self.assertEqual('FMAP', fentries[2].name) + def testElf(self): + """Basic test of ELF entries""" + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('-boot', fd.read()) + data = self._DoReadFile('96_elf.dts') + + def testElfStripg(self): + """Basic test of ELF entries""" + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + with open(self.TestFile('bss_data')) as fd: + TestFunctional._MakeInputFile('-boot', fd.read()) + data = self._DoReadFile('97_elf_strip.dts') + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/96_elf.dts b/tools/binman/test/96_elf.dts new file mode 100644 index 0000000..df3440c --- /dev/null +++ b/tools/binman/test/96_elf.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-elf { + }; + u-boot-spl-elf { + }; + }; +}; diff --git a/tools/binman/test/97_elf_strip.dts b/tools/binman/test/97_elf_strip.dts new file mode 100644 index 0000000..6f3c66f --- /dev/null +++ b/tools/binman/test/97_elf_strip.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-elf { + strip; + }; + u-boot-spl-elf { + }; + }; +}; From 163ed6c342cfd15b623a46f3755203c712374a9a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 14 Sep 2018 04:57:36 -0600 Subject: [PATCH 31/46] binman: Allow writing a map file when something goes wrong When we get a problem like overlapping regions it is sometimes hard to figure what what is going on. At present we don't write the map file in this case. However the file does provide useful information. Catch any packing errors and write a map file (if enabled with -m) to aid debugging. Signed-off-by: Simon Glass --- tools/binman/control.py | 12 +++++++++--- tools/binman/entry.py | 11 +++++++++-- tools/binman/ftest.py | 24 ++++++++++++++++++++++-- tools/binman/image.py | 7 ++++++- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/tools/binman/control.py b/tools/binman/control.py index caa194c..3446e2e 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -163,9 +163,15 @@ def Binman(options, args): # completed and written, but that does not seem important. image.GetEntryContents() image.GetEntryOffsets() - image.PackEntries() - image.CheckSize() - image.CheckEntries() + try: + image.PackEntries() + image.CheckSize() + image.CheckEntries() + except Exception as e: + if options.map: + fname = image.WriteMap() + print "Wrote map file '%s' to show errors" % fname + raise image.SetImagePos() if options.update_fdt: image.SetCalculatedProperties() diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 01be291..648cfd2 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -391,9 +391,16 @@ class Entry(object): pass @staticmethod + def GetStr(value): + if value is None: + return ' ' + return '%08x' % value + + @staticmethod def WriteMapLine(fd, indent, name, offset, size, image_pos): - print('%08x %s%08x %08x %s' % (image_pos, ' ' * indent, offset, - size, name), file=fd) + print('%s %s%s %s %s' % (Entry.GetStr(image_pos), ' ' * indent, + Entry.GetStr(offset), Entry.GetStr(size), + name), file=fd) def WriteMap(self, fd, indent): """Write a map of the entry to a .map file diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 27dca3a..abf02b6 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1607,8 +1607,9 @@ class TestFunctional(unittest.TestCase): def testExpandSizeBad(self): """Test an expanding entry which fails to provide contents""" - with self.assertRaises(ValueError) as e: - self._DoReadFileDtb('89_expand_size_bad.dts', map=True) + with test_util.capture_sys_output() as (stdout, stderr): + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('89_expand_size_bad.dts', map=True) self.assertIn("Node '/binman/_testing': Cannot obtain contents when " 'expanding entry', str(e.exception)) @@ -1724,6 +1725,25 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('-boot', fd.read()) data = self._DoReadFile('97_elf_strip.dts') + def testPackOverlapMap(self): + """Test that overlapping regions are detected""" + with test_util.capture_sys_output() as (stdout, stderr): + with self.assertRaises(ValueError) as e: + self._DoTestFile('14_pack_overlap.dts', map=True) + map_fname = tools.GetOutputFilename('image.map') + self.assertEqual("Wrote map file '%s' to show errors\n" % map_fname, + stdout.getvalue()) + + # We should not get an inmage, but there should be a map file + self.assertFalse(os.path.exists(tools.GetOutputFilename('image.bin'))) + self.assertTrue(os.path.exists(map_fname)) + map_data = tools.ReadFile(map_fname) + self.assertEqual('''ImagePos Offset Size Name + 00000000 00000007 main-section + 00000000 00000004 u-boot + 00000003 00000004 u-boot-align +''', map_data) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index e113a60..f237ae3 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -139,10 +139,15 @@ class Image: return self._section.GetEntries() def WriteMap(self): - """Write a map of the image to a .map file""" + """Write a map of the image to a .map file + + Returns: + Filename of map file written + """ filename = '%s.map' % self._name fname = tools.GetOutputFilename(filename) with open(fname, 'w') as fd: print('%8s %8s %8s %s' % ('ImagePos', 'Offset', 'Size', 'Name'), file=fd) self._section.WriteMap(fd, 0) + return fname From e369e58df79cf152c299b898e31c42f08167082a Mon Sep 17 00:00:00 2001 From: Mario Six Date: Tue, 26 Jun 2018 08:46:48 +0200 Subject: [PATCH 32/46] core: Add functions to set properties in live-tree Implement a set of functions to manipulate properties in a live device tree: * ofnode_write_prop() to set generic properties of a node * ofnode_write_string() to set string properties of a node * ofnode_set_enabled() to either enable or disable a node Signed-off-by: Mario Six Reviewed-by: Simon Glass --- drivers/core/ofnode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/ofnode.h | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index a7e1927..1e35480 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -791,3 +791,73 @@ ofnode ofnode_by_prop_value(ofnode from, const char *propname, propname, propval, proplen)); } } + +int ofnode_write_prop(ofnode node, const char *propname, int len, + const void *value) +{ + const struct device_node *np = ofnode_to_np(node); + struct property *pp; + struct property *pp_last = NULL; + struct property *new; + + if (!of_live_active()) + return -ENOSYS; + + if (!np) + return -EINVAL; + + for (pp = np->properties; pp; pp = pp->next) { + if (strcmp(pp->name, propname) == 0) { + /* Property exists -> change value */ + pp->value = (void *)value; + pp->length = len; + return 0; + } + pp_last = pp; + } + + if (!pp_last) + return -ENOENT; + + /* Property does not exist -> append new property */ + new = malloc(sizeof(struct property)); + if (!new) + return -ENOMEM; + + new->name = strdup(propname); + if (!new->name) + return -ENOMEM; + + new->value = (void *)value; + new->length = len; + new->next = NULL; + + pp_last->next = new; + + return 0; +} + +int ofnode_write_string(ofnode node, const char *propname, const char *value) +{ + if (!of_live_active()) + return -ENOSYS; + + assert(ofnode_valid(node)); + + debug("%s: %s = %s", __func__, propname, value); + + return ofnode_write_prop(node, propname, strlen(value) + 1, value); +} + +int ofnode_set_enabled(ofnode node, bool value) +{ + if (!of_live_active()) + return -ENOSYS; + + assert(ofnode_valid(node)); + + if (value) + return ofnode_write_string(node, "status", "okay"); + else + return ofnode_write_string(node, "status", "disable"); +} diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index c06d778..2fc9fa3 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -764,4 +764,50 @@ u64 ofnode_translate_address(ofnode node, const fdt32_t *in_addr); * @return true if OK, false if the compatible is not found */ int ofnode_device_is_compatible(ofnode node, const char *compat); + +/** + * ofnode_write_prop() - Set a property of a ofnode + * + * Note that the value passed to the function is *not* allocated by the + * function itself, but must be allocated by the caller if necessary. + * + * @node: The node for whose property should be set + * @propname: The name of the property to set + * @len: The length of the new value of the property + * @value: The new value of the property (must be valid prior to calling + * the function) + * @return 0 if successful, -ve on error + */ +int ofnode_write_prop(ofnode node, const char *propname, int len, + const void *value); + +/** + * ofnode_write_string() - Set a string property of a ofnode + * + * Note that the value passed to the function is *not* allocated by the + * function itself, but must be allocated by the caller if necessary. + * + * @node: The node for whose string property should be set + * @propname: The name of the string property to set + * @value: The new value of the string property (must be valid prior to + * calling the function) + * @return 0 if successful, -ve on error + */ +int ofnode_write_string(ofnode node, const char *propname, const char *value); + +/** + * ofnode_set_enabled() - Enable or disable a device tree node given by its + * ofnode + * + * This function effectively sets the node's "status" property to either "okay" + * or "disable", hence making it available for driver model initialization or + * not. + * + * @node: The node to enable + * @value: Flag that tells the function to either disable or enable the + * node + * @return 0 if successful, -ve on error + */ +int ofnode_set_enabled(ofnode node, bool value); + #endif From 2ea4d0db738f69f14a08406763ffca332f5c0446 Mon Sep 17 00:00:00 2001 From: Mario Six Date: Tue, 26 Jun 2018 08:46:49 +0200 Subject: [PATCH 33/46] test: Add tests for DT-manipulation functions Add tests for the ofnode_set_enabled, ofnode_write_string, and ofnode_write_property functions. Reviewed-by: Simon Glass Signed-off-by: Mario Six --- test/dm/test-fdt.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 8b72fe4..efe5342 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -14,6 +14,8 @@ #include #include #include +#include +#include #include DECLARE_GLOBAL_DATA_PTR; @@ -503,3 +505,55 @@ static int dm_test_fdt_remap_addr_live(struct unit_test_state *uts) } DM_TEST(dm_test_fdt_remap_addr_live, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_fdt_livetree_writing(struct unit_test_state *uts) +{ + struct udevice *dev; + ofnode node; + + if (!of_live_active()) { + printf("Live tree not active; ignore test\n"); + return 0; + } + + /* Test enabling devices */ + + node = ofnode_path("/usb@2"); + + ut_assert(!of_device_is_available(ofnode_to_np(node))); + ofnode_set_enabled(node, true); + ut_assert(of_device_is_available(ofnode_to_np(node))); + + device_bind_driver_to_node(dm_root(), "usb_sandbox", "usb@2", node, + &dev); + ut_assertok(uclass_find_device_by_seq(UCLASS_USB, 2, true, &dev)); + + /* Test string property setting */ + + ut_assert(device_is_compatible(dev, "sandbox,usb")); + ofnode_write_string(node, "compatible", "gdsys,super-usb"); + ut_assert(device_is_compatible(dev, "gdsys,super-usb")); + ofnode_write_string(node, "compatible", "sandbox,usb"); + ut_assert(device_is_compatible(dev, "sandbox,usb")); + + /* Test setting generic properties */ + + /* Non-existent in DTB */ + ut_asserteq(FDT_ADDR_T_NONE, dev_read_addr(dev)); + /* reg = 0x42, size = 0x100 */ + ut_assertok(ofnode_write_prop(node, "reg", 8, + "\x00\x00\x00\x42\x00\x00\x01\x00")); + ut_asserteq(0x42, dev_read_addr(dev)); + + /* Test disabling devices */ + + device_remove(dev, DM_REMOVE_NORMAL); + device_unbind(dev); + + ut_assert(of_device_is_available(ofnode_to_np(node))); + ofnode_set_enabled(node, false); + ut_assert(!of_device_is_available(ofnode_to_np(node))); + + return 0; +} +DM_TEST(dm_test_fdt_livetree_writing, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); From e4c98a59db99e6bfba74d27cc571d59213acb64e Mon Sep 17 00:00:00 2001 From: Mario Six Date: Tue, 26 Jun 2018 08:46:50 +0200 Subject: [PATCH 34/46] core: Add dev_{disable,enable}_by_path We cannot use device structures to disable devices, since getting them with the API functions would bind and activate the device, which would fail if the underlying device does not exist. Reviewed-by: Simon Glass --- drivers/core/device.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/device.h | 16 +++++++++++ 2 files changed, 94 insertions(+) diff --git a/drivers/core/device.c b/drivers/core/device.c index fd59fe1..feed43c 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -516,6 +516,33 @@ static int device_get_device_tail(struct udevice *dev, int ret, return 0; } +/** + * device_find_by_ofnode() - Return device associated with given ofnode + * + * The returned device is *not* activated. + * + * @node: The ofnode for which a associated device should be looked up + * @devp: Pointer to structure to hold the found device + * Return: 0 if OK, -ve on error + */ +static int device_find_by_ofnode(ofnode node, struct udevice **devp) +{ + struct uclass *uc; + struct udevice *dev; + int ret; + + list_for_each_entry(uc, &gd->uclass_root, sibling_node) { + ret = uclass_find_device_by_ofnode(uc->uc_drv->id, node, + &dev); + if (!ret || dev) { + *devp = dev; + return 0; + } + } + + return -ENODEV; +} + int device_get_child(struct udevice *parent, int index, struct udevice **devp) { struct udevice *dev; @@ -739,3 +766,54 @@ bool of_machine_is_compatible(const char *compat) return !fdt_node_check_compatible(fdt, 0, compat); } + +int dev_disable_by_path(const char *path) +{ + struct uclass *uc; + ofnode node = ofnode_path(path); + struct udevice *dev; + int ret = 1; + + if (!of_live_active()) + return -ENOSYS; + + list_for_each_entry(uc, &gd->uclass_root, sibling_node) { + ret = uclass_find_device_by_ofnode(uc->uc_drv->id, node, &dev); + if (!ret) + break; + } + + if (ret) + return ret; + + ret = device_remove(dev, DM_REMOVE_NORMAL); + if (ret) + return ret; + + ret = device_unbind(dev); + if (ret) + return ret; + + return ofnode_set_enabled(node, false); +} + +int dev_enable_by_path(const char *path) +{ + ofnode node = ofnode_path(path); + ofnode pnode = ofnode_get_parent(node); + struct udevice *parent; + int ret = 1; + + if (!of_live_active()) + return -ENOSYS; + + ret = device_find_by_ofnode(pnode, &parent); + if (ret) + return ret; + + ret = ofnode_set_enabled(node, true); + if (ret) + return ret; + + return lists_bind_fdt(parent, node, NULL); +} diff --git a/include/dm/device.h b/include/dm/device.h index 3120b68..9812d86 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -601,6 +601,22 @@ bool device_is_compatible(struct udevice *dev, const char *compat); bool of_machine_is_compatible(const char *compat); /** + * dev_disable_by_path() - Disable a device given its device tree path + * + * @path: The device tree path identifying the device to be disabled + * @return 0 on success, -ve on error + */ +int dev_disable_by_path(const char *path); + +/** + * dev_enable_by_path() - Enable a device given its device tree path + * + * @path: The device tree path identifying the device to be enabled + * @return 0 on success, -ve on error + */ +int dev_enable_by_path(const char *path); + +/** * device_is_on_pci_bus - Test if a device is on a PCI bus * * @dev: device to test From 172942a4a0afe12a603470db7728f3871a81de01 Mon Sep 17 00:00:00 2001 From: Mario Six Date: Tue, 26 Jun 2018 08:46:51 +0200 Subject: [PATCH 35/46] test: Add tests for dev_{enable, disable}_by_path Add tests for the dev_{enable,disable}_by_path functions. Reviewed-by: Simon Glass Signed-off-by: Mario Six --- test/dm/test-fdt.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index efe5342..79b1f1d 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -557,3 +557,31 @@ static int dm_test_fdt_livetree_writing(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_fdt_livetree_writing, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_fdt_disable_enable_by_path(struct unit_test_state *uts) +{ + ofnode node; + + if (!of_live_active()) { + printf("Live tree not active; ignore test\n"); + return 0; + } + + node = ofnode_path("/usb@2"); + + /* Test enabling devices */ + + ut_assert(!of_device_is_available(ofnode_to_np(node))); + dev_enable_by_path("/usb@2"); + ut_assert(of_device_is_available(ofnode_to_np(node))); + + /* Test disabling devices */ + + ut_assert(of_device_is_available(ofnode_to_np(node))); + dev_disable_by_path("/usb@2"); + ut_assert(!of_device_is_available(ofnode_to_np(node))); + + return 0; +} +DM_TEST(dm_test_fdt_disable_enable_by_path, DM_TESTF_SCAN_PDATA | + DM_TESTF_SCAN_FDT); From 5381c2856de3ed0191135b435cff39789e5f17ad Mon Sep 17 00:00:00 2001 From: Mario Six Date: Tue, 31 Jul 2018 11:44:11 +0200 Subject: [PATCH 36/46] drivers: Add board uclass Since there is no canonical "board device" that can be used in board files, it is difficult to use DM function for board initialization in these cases. Hence, add a uclass that implements a simple "board device", which can hold devices not suitable anywhere else in the device tree, and is also able to read encoded information, e.g. hard-wired GPIOs on a GPIO expander, read-only memory ICs, etc. that carry information about the hardware. The devices of this uclass expose methods to read generic data types (integers, strings, booleans) to encode the information provided by the hardware. Reviewed-by: Simon Glass Signed-off-by: Mario Six --- drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/board/Kconfig | 7 +++ drivers/board/Makefile | 6 ++ drivers/board/board-uclass.c | 60 +++++++++++++++++++ include/board.h | 139 +++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + 7 files changed, 216 insertions(+) create mode 100644 drivers/board/Kconfig create mode 100644 drivers/board/Makefile create mode 100644 drivers/board/board-uclass.c create mode 100644 include/board.h diff --git a/drivers/Kconfig b/drivers/Kconfig index 56536c4..64ba5bf 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -24,6 +24,8 @@ source "drivers/ddr/Kconfig" source "drivers/demo/Kconfig" +source "drivers/board/Kconfig" + source "drivers/ddr/fsl/Kconfig" source "drivers/dfu/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 23ea609..764f51d 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -71,6 +71,7 @@ obj-y += ata/ obj-$(CONFIG_DM_DEMO) += demo/ obj-$(CONFIG_BIOSEMU) += bios_emulator/ obj-y += block/ +obj-y += board/ obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/ obj-$(CONFIG_CPU) += cpu/ obj-y += crypto/ diff --git a/drivers/board/Kconfig b/drivers/board/Kconfig new file mode 100644 index 0000000..c9a52dc --- /dev/null +++ b/drivers/board/Kconfig @@ -0,0 +1,7 @@ +menuconfig BOARD + bool "Device Information" + help + Support methods to query hardware configurations from internal + mechanisms (e.g. reading GPIO values, determining the presence of + devices on busses, etc.). This enables the usage of U-Boot with + modular board architectures. diff --git a/drivers/board/Makefile b/drivers/board/Makefile new file mode 100644 index 0000000..12dd203 --- /dev/null +++ b/drivers/board/Makefile @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# (C) Copyright 2017 +# Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc +obj-$(CONFIG_BOARD) += board-uclass.o +obj-$(CONFIG_BOARD_GAZERBEAM) += gazerbeam.o diff --git a/drivers/board/board-uclass.c b/drivers/board/board-uclass.c new file mode 100644 index 0000000..a516ba4 --- /dev/null +++ b/drivers/board/board-uclass.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * (C) Copyright 2017 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + */ + +#include +#include +#include + +int board_get(struct udevice **devp) +{ + return uclass_first_device_err(UCLASS_BOARD, devp); +} + +int board_detect(struct udevice *dev) +{ + struct board_ops *ops = board_get_ops(dev); + + if (!ops->detect) + return -ENOSYS; + + return ops->detect(dev); +} + +int board_get_bool(struct udevice *dev, int id, bool *val) +{ + struct board_ops *ops = board_get_ops(dev); + + if (!ops->get_bool) + return -ENOSYS; + + return ops->get_bool(dev, id, val); +} + +int board_get_int(struct udevice *dev, int id, int *val) +{ + struct board_ops *ops = board_get_ops(dev); + + if (!ops->get_int) + return -ENOSYS; + + return ops->get_int(dev, id, val); +} + +int board_get_str(struct udevice *dev, int id, size_t size, char *val) +{ + struct board_ops *ops = board_get_ops(dev); + + if (!ops->get_str) + return -ENOSYS; + + return ops->get_str(dev, id, size, val); +} + +UCLASS_DRIVER(board) = { + .id = UCLASS_BOARD, + .name = "board", + .post_bind = dm_scan_fdt_dev, +}; diff --git a/include/board.h b/include/board.h new file mode 100644 index 0000000..9dc7868 --- /dev/null +++ b/include/board.h @@ -0,0 +1,139 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * (C) Copyright 2017 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + */ + +/* + * This uclass encapsulates hardware methods to gather information about a + * board or a specific device such as hard-wired GPIOs on GPIO expanders, + * read-only data in flash ICs, or similar. + * + * The interface offers functions to read the usual standard data types (bool, + * int, string) from the device, each of which is identified by a static + * numeric ID (which will usually be defined as a enum in a header file). + * + * If for example the board had a read-only serial number flash IC, we could + * call + * + * ret = board_detect(dev); + * if (ret) { + * debug("board device not found."); + * return ret; + * } + * + * ret = board_get_int(dev, ID_SERIAL_NUMBER, &serial); + * if (ret) { + * debug("Error when reading serial number from device."); + * return ret; + * } + * + * to read the serial number. + */ + +struct board_ops { + /** + * detect() - Run the hardware info detection procedure for this + * device. + * @dev: The device containing the information + * + * This operation might take a long time (e.g. read from EEPROM, + * check the presence of a device on a bus etc.), hence this is not + * done in the probe() method, but later during operation in this + * dedicated method. + * + * Return: 0 if OK, -ve on error. + */ + int (*detect)(struct udevice *dev); + + /** + * get_bool() - Read a specific bool data value that describes the + * hardware setup. + * @dev: The board instance to gather the data. + * @id: A unique identifier for the bool value to be read. + * @val: Pointer to a buffer that receives the value read. + * + * Return: 0 if OK, -ve on error. + */ + int (*get_bool)(struct udevice *dev, int id, bool *val); + + /** + * get_int() - Read a specific int data value that describes the + * hardware setup. + * @dev: The board instance to gather the data. + * @id: A unique identifier for the int value to be read. + * @val: Pointer to a buffer that receives the value read. + * + * Return: 0 if OK, -ve on error. + */ + int (*get_int)(struct udevice *dev, int id, int *val); + + /** + * get_str() - Read a specific string data value that describes the + * hardware setup. + * @dev: The board instance to gather the data. + * @id: A unique identifier for the string value to be read. + * @size: The size of the buffer to receive the string data. + * @val: Pointer to a buffer that receives the value read. + * + * Return: 0 if OK, -ve on error. + */ + int (*get_str)(struct udevice *dev, int id, size_t size, char *val); +}; + +#define board_get_ops(dev) ((struct board_ops *)(dev)->driver->ops) + +/** + * board_detect() - Run the hardware info detection procedure for this device. + * + * @dev: The device containing the information + * + * Return: 0 if OK, -ve on error. + */ +int board_detect(struct udevice *dev); + +/** + * board_get_bool() - Read a specific bool data value that describes the + * hardware setup. + * @dev: The board instance to gather the data. + * @id: A unique identifier for the bool value to be read. + * @val: Pointer to a buffer that receives the value read. + * + * Return: 0 if OK, -ve on error. + */ +int board_get_bool(struct udevice *dev, int id, bool *val); + +/** + * board_get_int() - Read a specific int data value that describes the + * hardware setup. + * @dev: The board instance to gather the data. + * @id: A unique identifier for the int value to be read. + * @val: Pointer to a buffer that receives the value read. + * + * Return: 0 if OK, -ve on error. + */ +int board_get_int(struct udevice *dev, int id, int *val); + +/** + * board_get_str() - Read a specific string data value that describes the + * hardware setup. + * @dev: The board instance to gather the data. + * @id: A unique identifier for the string value to be read. + * @size: The size of the buffer to receive the string data. + * @val: Pointer to a buffer that receives the value read. + * + * Return: 0 if OK, -ve on error. + */ +int board_get_str(struct udevice *dev, int id, size_t size, char *val); + +/** + * board_get() - Return the board device for the board in question. + * @devp: Pointer to structure to receive the board device. + * + * Since there can only be at most one board instance, the API can supply a + * function that returns the unique device. This is especially useful for use + * in board files. + * + * Return: 0 if OK, -ve on error. + */ +int board_get(struct udevice **devp); diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index 7027ea0..5e19465 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -30,6 +30,7 @@ enum uclass_id { UCLASS_ADC, /* Analog-to-digital converter */ UCLASS_AHCI, /* SATA disk controller */ UCLASS_BLK, /* Block device */ + UCLASS_BOARD, /* Device information from hardware */ UCLASS_CLK, /* Clock source, e.g. used by peripherals */ UCLASS_CPU, /* CPU, typically part of an SoC */ UCLASS_CROS_EC, /* Chrome OS EC */ From 6238ae4d60476dd7535b781ef3f255f676851283 Mon Sep 17 00:00:00 2001 From: Mario Six Date: Tue, 31 Jul 2018 11:44:12 +0200 Subject: [PATCH 37/46] board: Add gazerbeam driver Add a board driver for the upcoming gdsys Gazerbeam board. Signed-off-by: Mario Six Reviewed-by: Simon Glass --- .../bindings/board/gdsys,board_gazerbeam.txt | 46 ++++ drivers/board/Kconfig | 10 + drivers/board/gazerbeam.c | 262 +++++++++++++++++++++ drivers/board/gazerbeam.h | 18 ++ 4 files changed, 336 insertions(+) create mode 100644 Documentation/devicetree/bindings/board/gdsys,board_gazerbeam.txt create mode 100644 drivers/board/gazerbeam.c create mode 100644 drivers/board/gazerbeam.h diff --git a/Documentation/devicetree/bindings/board/gdsys,board_gazerbeam.txt b/Documentation/devicetree/bindings/board/gdsys,board_gazerbeam.txt new file mode 100644 index 0000000..28c1080 --- /dev/null +++ b/Documentation/devicetree/bindings/board/gdsys,board_gazerbeam.txt @@ -0,0 +1,46 @@ +gdsys Gazerbeam board driver + +This driver provides capabilities to access the gdsys Gazerbeam board's device +information. Furthermore, phandles to some internal devices are provided for +the board files. + +Required properties: +- compatible: should be "gdsys,board_gazerbeam" +- csb: phandle to the board's coherent system bus (CSB) device node +- rxaui[0-3]: phandles to the rxaui control device nodes +- fpga[0-1]: phandles to the board's gdsys FPGA device nodes +- ioep[0-1]: phandles to the board's IO endpoint device nodes +- ver-gpios: GPIO list to read the hardware version from +- var-gpios: GPIO list to read the hardware variant information from +- reset-gpios: GPIO list for the board's reset GPIOs + +Example: + + +board { + compatible = "gdsys,board_gazerbeam"; + csb = <&board_soc>; + serdes = <&SERDES>; + rxaui0 = <&RXAUI0>; + rxaui1 = <&RXAUI1>; + rxaui2 = <&RXAUI2>; + rxaui3 = <&RXAUI3>; + fpga0 = <&FPGA0>; + fpga1 = <&FPGA1>; + ioep0 = <&IOEP0>; + ioep1 = <&IOEP1>; + + ver-gpios = <&PPCPCA 12 0 + &PPCPCA 13 0 + &PPCPCA 14 0 + &PPCPCA 15 0>; + + /* MC2/SC-Board */ + var-gpios-mc2 = <&GPIO_VB0 0 0 /* VAR-MC_SC */ + &GPIO_VB0 11 0>; /* VAR-CON */ + /* MC4-Board */ + var-gpios-mc4 = <&GPIO_VB1 0 0 /* VAR-MC_SC */ + &GPIO_VB1 11 0>; /* VAR-CON */ + + reset-gpios = <&gpio0 1 0 &gpio0 2 1>; +}; diff --git a/drivers/board/Kconfig b/drivers/board/Kconfig index c9a52dc..cc1cf27 100644 --- a/drivers/board/Kconfig +++ b/drivers/board/Kconfig @@ -5,3 +5,13 @@ menuconfig BOARD mechanisms (e.g. reading GPIO values, determining the presence of devices on busses, etc.). This enables the usage of U-Boot with modular board architectures. + +if BOARD + + +config BOARD_GAZERBEAM + bool "Enable device information for the Gazerbeam board" + help + Support querying device information for the gdsys Gazerbeam board. + +endif diff --git a/drivers/board/gazerbeam.c b/drivers/board/gazerbeam.c new file mode 100644 index 0000000..481cce8 --- /dev/null +++ b/drivers/board/gazerbeam.c @@ -0,0 +1,262 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * (C) Copyright 2017 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + */ + +#include +#include +#include +#include +#include + +#include "gazerbeam.h" + +/* Sequence number of I2C bus that holds the GPIO expanders */ +static const int I2C_BUS_SEQ_NO = 1; + +/* I2C address of SC/MC2 expander */ +static const int MC2_EXPANDER_ADDR = 0x20; +/* I2C address of MC4 expander */ +static const int MC4_EXPANDER_ADDR = 0x22; + +/* Number of the GPIO to read the SC data from */ +static const int SC_GPIO_NO; +/* Number of the GPIO to read the CON data from */ +static const int CON_GPIO_NO = 1; + +/** + * struct board_gazerbeam_priv - Private data structure for the gazerbeam board + * driver. + * @reset_gpios: GPIOs for the board's reset GPIOs. + * @var_gpios: GPIOs for the board's hardware variant GPIOs + * @ver_gpios: GPIOs for the board's hardware version GPIOs + * @variant: Container for the board's hardware variant (CON/CPU) + * @multichannel: Container for the board's multichannel variant (MC4/MC2/SC) + * @hwversion: Container for the board's hardware version + */ +struct board_gazerbeam_priv { + struct gpio_desc reset_gpios[2]; + struct gpio_desc var_gpios[2]; + struct gpio_desc ver_gpios[4]; + int variant; + int multichannel; + int hwversion; +}; + +/** + * _read_board_variant_data() - Read variant information from the hardware. + * @dev: The board device for which to determine the multichannel and device + * type information. + * + * The data read from the board's hardware (mostly hard-wired GPIOs) is stored + * in the private data structure of the driver to be used by other driver + * methods. + * + * Return: 0 if OK, -ve on error. + */ +static int _read_board_variant_data(struct udevice *dev) +{ + struct board_gazerbeam_priv *priv = dev_get_priv(dev); + struct udevice *i2c_bus; + struct udevice *dummy; + char *listname; + int mc4, mc2, sc, con; + int gpio_num; + int res; + + res = uclass_get_device_by_seq(UCLASS_I2C, I2C_BUS_SEQ_NO, &i2c_bus); + if (res) { + debug("%s: Could not get I2C bus %d (err = %d)\n", + dev->name, I2C_BUS_SEQ_NO, res); + return res; + } + + if (!i2c_bus) { + debug("%s: Could not get I2C bus %d\n", + dev->name, I2C_BUS_SEQ_NO); + return -EIO; + } + + mc2 = !dm_i2c_probe(i2c_bus, MC2_EXPANDER_ADDR, 0, &dummy); + mc4 = !dm_i2c_probe(i2c_bus, MC4_EXPANDER_ADDR, 0, &dummy); + + if (mc2 && mc4) { + debug("%s: Board hardware configuration inconsistent.\n", + dev->name); + return -EINVAL; + } + + listname = mc2 ? "var-gpios-mc2" : "var-gpios-mc4"; + + gpio_num = gpio_request_list_by_name(dev, listname, priv->var_gpios, + ARRAY_SIZE(priv->var_gpios), + GPIOD_IS_IN); + if (gpio_num < 0) { + debug("%s: Requesting gpio list %s failed (err = %d).\n", + dev->name, listname, gpio_num); + return gpio_num; + } + + sc = dm_gpio_get_value(&priv->var_gpios[SC_GPIO_NO]); + if (sc < 0) { + debug("%s: Error while reading 'sc' GPIO (err = %d)", + dev->name, sc); + return sc; + } + + con = dm_gpio_get_value(&priv->var_gpios[CON_GPIO_NO]); + if (con < 0) { + debug("%s: Error while reading 'con' GPIO (err = %d)", + dev->name, con); + return con; + } + + if ((sc && mc2) || (sc && mc4) || (!sc && !mc2 && !mc4)) { + debug("%s: Board hardware configuration inconsistent.\n", + dev->name); + return -EINVAL; + } + + priv->variant = con ? VAR_CON : VAR_CPU; + + priv->multichannel = mc4 ? 4 : (mc2 ? 2 : (sc ? 1 : 0)); + + return 0; +} + +/** + * _read_hwversion() - Read the hardware version from the board. + * @dev: The board device for which to read the hardware version. + * + * The hardware version read from the board (from hard-wired GPIOs) is stored + * in the private data structure of the driver to be used by other driver + * methods. + * + * Return: 0 if OK, -ve on error. + */ +static int _read_hwversion(struct udevice *dev) +{ + struct board_gazerbeam_priv *priv = dev_get_priv(dev); + int res; + + res = gpio_request_list_by_name(dev, "ver-gpios", priv->ver_gpios, + ARRAY_SIZE(priv->ver_gpios), + GPIOD_IS_IN); + if (res < 0) { + debug("%s: Error getting GPIO list 'ver-gpios' (err = %d)\n", + dev->name, res); + return -ENODEV; + } + + res = dm_gpio_get_values_as_int(priv->ver_gpios, + ARRAY_SIZE(priv->ver_gpios)); + if (res < 0) { + debug("%s: Error reading HW version from expander (err = %d)\n", + dev->name, res); + return res; + } + + priv->hwversion = res; + + res = gpio_free_list(dev, priv->ver_gpios, ARRAY_SIZE(priv->ver_gpios)); + if (res < 0) { + debug("%s: Error freeing HW version GPIO list (err = %d)\n", + dev->name, res); + return res; + } + + return 0; +} + +static int board_gazerbeam_detect(struct udevice *dev) +{ + int res; + + res = _read_board_variant_data(dev); + if (res) { + debug("%s: Error reading multichannel variant (err = %d)\n", + dev->name, res); + return res; + } + + res = _read_hwversion(dev); + if (res) { + debug("%s: Error reading hardware version (err = %d)\n", + dev->name, res); + return res; + } + + return 0; +} + +static int board_gazerbeam_get_int(struct udevice *dev, int id, int *val) +{ + struct board_gazerbeam_priv *priv = dev_get_priv(dev); + + switch (id) { + case BOARD_MULTICHANNEL: + *val = priv->multichannel; + break; + case BOARD_VARIANT: + *val = priv->variant; + break; + case BOARD_HWVERSION: + *val = priv->hwversion; + break; + default: + debug("%s: Integer value %d unknown\n", dev->name, id); + return -EINVAL; + } + + return 0; +} + +static const struct udevice_id board_gazerbeam_ids[] = { + { .compatible = "gdsys,board_gazerbeam" }, + { /* sentinel */ } +}; + +static const struct board_ops board_gazerbeam_ops = { + .detect = board_gazerbeam_detect, + .get_int = board_gazerbeam_get_int, +}; + +static int board_gazerbeam_probe(struct udevice *dev) +{ + struct board_gazerbeam_priv *priv = dev_get_priv(dev); + int gpio_num, i; + + gpio_num = gpio_request_list_by_name(dev, "reset-gpios", + priv->reset_gpios, + ARRAY_SIZE(priv->reset_gpios), + GPIOD_IS_OUT); + + if (gpio_num < 0) { + debug("%s: Error getting GPIO list 'reset-gpios' (err = %d)\n", + dev->name, gpio_num); + return gpio_num; + } + + /* Set startup-finished GPIOs */ + for (i = 0; i < ARRAY_SIZE(priv->reset_gpios); i++) { + int res = dm_gpio_set_value(&priv->reset_gpios[i], 0); + + if (res) { + debug("%s: Error while setting GPIO %d (err = %d)\n", + dev->name, i, res); + return res; + } + } + + return 0; +} + +U_BOOT_DRIVER(board_gazerbeam) = { + .name = "board_gazerbeam", + .id = UCLASS_BOARD, + .of_match = board_gazerbeam_ids, + .ops = &board_gazerbeam_ops, + .priv_auto_alloc_size = sizeof(struct board_gazerbeam_priv), + .probe = board_gazerbeam_probe, +}; diff --git a/drivers/board/gazerbeam.h b/drivers/board/gazerbeam.h new file mode 100644 index 0000000..0ca003a --- /dev/null +++ b/drivers/board/gazerbeam.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * (C) Copyright 2017 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +enum { + BOARD_MULTICHANNEL, + BOARD_VARIANT, + BOARD_HWVERSION, +}; + +enum { + VAR_CON, + VAR_CPU, +}; From e6fd0181082a04e743a07ebd9f6fdd0e06dc1399 Mon Sep 17 00:00:00 2001 From: Mario Six Date: Tue, 31 Jul 2018 11:44:13 +0200 Subject: [PATCH 38/46] test: Add tests for board uclass Add tests for the new board uclass. Reviewed-by: Simon Glass Signed-off-by: Mario Six --- arch/sandbox/dts/test.dts | 4 ++ configs/sandbox64_defconfig | 2 + configs/sandbox_defconfig | 2 + configs/sandbox_flattree_defconfig | 2 + configs/sandbox_noblk_defconfig | 2 + configs/sandbox_spl_defconfig | 2 + drivers/board/Kconfig | 7 ++- drivers/board/Makefile | 1 + drivers/board/sandbox.c | 107 +++++++++++++++++++++++++++++++++++++ drivers/board/sandbox.h | 12 +++++ test/dm/Makefile | 1 + test/dm/board.c | 57 ++++++++++++++++++++ 12 files changed, 198 insertions(+), 1 deletion(-) create mode 100644 drivers/board/sandbox.c create mode 100644 drivers/board/sandbox.h create mode 100644 test/dm/board.c diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index b8524e3..751c13b 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -642,6 +642,10 @@ }; }; }; + + board { + compatible = "sandbox,board_sandbox"; + }; }; #include "sandbox_pmic.dtsi" diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 27797c6..b80e2eb 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -86,6 +86,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 0b20968..356c48b 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -91,6 +91,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 618d646..1d3dd9f 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -70,6 +70,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/configs/sandbox_noblk_defconfig b/configs/sandbox_noblk_defconfig index a7691da..5789cf3 100644 --- a/configs/sandbox_noblk_defconfig +++ b/configs/sandbox_noblk_defconfig @@ -77,6 +77,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index dad5e1c..3ee276f 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -91,6 +91,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/drivers/board/Kconfig b/drivers/board/Kconfig index cc1cf27..2a3fc9c 100644 --- a/drivers/board/Kconfig +++ b/drivers/board/Kconfig @@ -10,8 +10,13 @@ if BOARD config BOARD_GAZERBEAM - bool "Enable device information for the Gazerbeam board" + bool "Enable board driver for the Gazerbeam board" help Support querying device information for the gdsys Gazerbeam board. +config BOARD_SANDBOX + bool "Enable board driver for the Sandbox board" + help + Support querying device information for the Sandbox boards. + endif diff --git a/drivers/board/Makefile b/drivers/board/Makefile index 12dd203..2224338 100644 --- a/drivers/board/Makefile +++ b/drivers/board/Makefile @@ -4,3 +4,4 @@ # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-$(CONFIG_BOARD) += board-uclass.o obj-$(CONFIG_BOARD_GAZERBEAM) += gazerbeam.o +obj-$(CONFIG_BOARD_SANDBOX) += sandbox.o diff --git a/drivers/board/sandbox.c b/drivers/board/sandbox.c new file mode 100644 index 0000000..50621e4 --- /dev/null +++ b/drivers/board/sandbox.c @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * (C) Copyright 2018 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + */ + +#include +#include +#include + +#include "sandbox.h" + +struct board_sandbox_priv { + bool called_detect; + int test_i1; + int test_i2; +}; + +char vacation_spots[][64] = {"R'lyeh", "Dreamlands", "Plateau of Leng", + "Carcosa", "Yuggoth", "The Nameless City"}; + +int board_sandbox_detect(struct udevice *dev) +{ + struct board_sandbox_priv *priv = dev_get_priv(dev); + + priv->called_detect = true; + priv->test_i2 = 100; + + return 0; +} + +int board_sandbox_get_bool(struct udevice *dev, int id, bool *val) +{ + struct board_sandbox_priv *priv = dev_get_priv(dev); + + switch (id) { + case BOOL_CALLED_DETECT: + /* Checks if the dectect method has been called */ + *val = priv->called_detect; + return 0; + } + + return -ENOENT; +} + +int board_sandbox_get_int(struct udevice *dev, int id, int *val) +{ + struct board_sandbox_priv *priv = dev_get_priv(dev); + + switch (id) { + case INT_TEST1: + *val = priv->test_i1; + /* Increments with every call */ + priv->test_i1++; + return 0; + case INT_TEST2: + *val = priv->test_i2; + /* Decrements with every call */ + priv->test_i2--; + return 0; + } + + return -ENOENT; +} + +int board_sandbox_get_str(struct udevice *dev, int id, size_t size, char *val) +{ + struct board_sandbox_priv *priv = dev_get_priv(dev); + int i1 = priv->test_i1; + int i2 = priv->test_i2; + int index = (i1 * i2) % ARRAY_SIZE(vacation_spots); + + switch (id) { + case STR_VACATIONSPOT: + /* Picks a vacation spot depending on i1 and i2 */ + snprintf(val, size, vacation_spots[index]); + return 0; + } + + return -ENOENT; +} + +static const struct udevice_id board_sandbox_ids[] = { + { .compatible = "sandbox,board_sandbox" }, + { /* sentinel */ } +}; + +static const struct board_ops board_sandbox_ops = { + .detect = board_sandbox_detect, + .get_bool = board_sandbox_get_bool, + .get_int = board_sandbox_get_int, + .get_str = board_sandbox_get_str, +}; + +int board_sandbox_probe(struct udevice *dev) +{ + return 0; +} + +U_BOOT_DRIVER(board_sandbox) = { + .name = "board_sandbox", + .id = UCLASS_BOARD, + .of_match = board_sandbox_ids, + .ops = &board_sandbox_ops, + .priv_auto_alloc_size = sizeof(struct board_sandbox_priv), + .probe = board_sandbox_probe, +}; diff --git a/drivers/board/sandbox.h b/drivers/board/sandbox.h new file mode 100644 index 0000000..2cff494 --- /dev/null +++ b/drivers/board/sandbox.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * (C) Copyright 2018 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + */ + +enum { + BOOL_CALLED_DETECT, + INT_TEST1, + INT_TEST2, + STR_VACATIONSPOT, +}; diff --git a/test/dm/Makefile b/test/dm/Makefile index 8b1ba91..d7f5d6b 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_UT_DM) += test-uclass.o obj-$(CONFIG_UT_DM) += core.o ifneq ($(CONFIG_SANDBOX),) obj-$(CONFIG_BLK) += blk.o +obj-$(CONFIG_BOARD) += board.o obj-$(CONFIG_CLK) += clk.o obj-$(CONFIG_DM_ETH) += eth.o obj-$(CONFIG_DM_GPIO) += gpio.o diff --git a/test/dm/board.c b/test/dm/board.c new file mode 100644 index 0000000..0f267a1 --- /dev/null +++ b/test/dm/board.c @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * (C) Copyright 2018 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + */ + +#include +#include +#include +#include +#include + +#include "../../drivers/board/sandbox.h" + +static int dm_test_board(struct unit_test_state *uts) +{ + struct udevice *board; + bool called_detect; + char str[64]; + int i; + + board_get(&board); + ut_assert(board); + + board_get_bool(board, BOOL_CALLED_DETECT, &called_detect); + ut_assert(!called_detect); + + board_detect(board); + + board_get_bool(board, BOOL_CALLED_DETECT, &called_detect); + ut_assert(called_detect); + + board_get_str(board, STR_VACATIONSPOT, sizeof(str), str); + ut_assertok(strcmp(str, "R'lyeh")); + + board_get_int(board, INT_TEST1, &i); + ut_asserteq(0, i); + + board_get_int(board, INT_TEST2, &i); + ut_asserteq(100, i); + + board_get_str(board, STR_VACATIONSPOT, sizeof(str), str); + ut_assertok(strcmp(str, "Carcosa")); + + board_get_int(board, INT_TEST1, &i); + ut_asserteq(1, i); + + board_get_int(board, INT_TEST2, &i); + ut_asserteq(99, i); + + board_get_str(board, STR_VACATIONSPOT, sizeof(str), str); + ut_assertok(strcmp(str, "Yuggoth")); + + return 0; +} + +DM_TEST(dm_test_board, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); From ff8eee0330a68ee3fbb3e15ed8e315043784869f Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Wed, 8 Aug 2018 22:10:44 +0200 Subject: [PATCH 39/46] cmd: clk: Add trivial implementation of clock dump for DM Add trivial implementation of the clk dump in case DM is enabled. This implementation just iterates over all the clock registered with the CLK uclass and prints their rate. Signed-off-by: Marek Vasut Cc: Chin Liang See Cc: Dinh Nguyen Cc: Ley Foon Tan Cc: Simon Glass Cc: Tom Rini Reviewed-by: Ley Foon Tan --- cmd/clk.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/cmd/clk.c b/cmd/clk.c index 73fb250..fd42315 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -5,11 +5,48 @@ #include #include #include +#if defined(CONFIG_DM) && defined(CONFIG_CLK) +#include +#include +#endif int __weak soc_clk_dump(void) { +#if defined(CONFIG_DM) && defined(CONFIG_CLK) + struct udevice *dev; + struct uclass *uc; + struct clk clk; + int ret; + + /* Device addresses start at 1 */ + ret = uclass_get(UCLASS_CLK, &uc); + if (ret) + return ret; + + uclass_foreach_dev(dev, uc) { + memset(&clk, 0, sizeof(clk)); + ret = device_probe(dev); + if (ret) { + printf("%-30.30s : ? Hz\n", dev->name); + continue; + } + + ret = clk_request(dev, &clk); + if (ret) { + printf("%-30.30s : ? Hz\n", dev->name); + continue; + } + + printf("%-30.30s : %lu Hz\n", dev->name, clk_get_rate(&clk)); + + clk_free(&clk); + } + + return 0; +#else puts("Not implemented\n"); return 1; +#endif } static int do_clk_dump(cmd_tbl_t *cmdtp, int flag, int argc, From c35a7d375ec8f0a8ee343ae4868be3242172632e Mon Sep 17 00:00:00 2001 From: Jens Wiklander Date: Mon, 20 Aug 2018 11:09:59 +0200 Subject: [PATCH 40/46] fdt: fdtdec_setup_memory_banksize() use livetree Converts fdtdec_setup_memory_banksize() to use ofnode functions instead. Reviewed-by: Simon Glass Signed-off-by: Jens Wiklander --- lib/fdtdec.c | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/lib/fdtdec.c b/lib/fdtdec.c index bf5e0f6..74196ce 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -15,6 +15,7 @@ #include #include #include +#include #include DECLARE_GLOBAL_DATA_PTR; @@ -1181,41 +1182,34 @@ int fdtdec_setup_mem_size_base(void) #if defined(CONFIG_NR_DRAM_BANKS) -static int get_next_memory_node(const void *blob, int mem) +static ofnode get_next_memory_node(ofnode mem) { do { - mem = fdt_node_offset_by_prop_value(gd->fdt_blob, mem, - "device_type", "memory", 7); - } while (!fdtdec_get_is_enabled(blob, mem)); + mem = ofnode_by_prop_value(mem, "device_type", "memory", 7); + } while (ofnode_valid(mem) && !ofnode_is_available(mem)); return mem; } int fdtdec_setup_memory_banksize(void) { - int bank, ret, mem, reg = 0; - struct fdt_resource res; + int bank, reg = 0; + struct resource res; + ofnode mem; - mem = get_next_memory_node(gd->fdt_blob, -1); - if (mem < 0) { - debug("%s: Missing /memory node\n", __func__); - return -EINVAL; - } + mem = get_next_memory_node(ofnode_null()); + if (!ofnode_valid(mem)) + goto missing_node; for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { - ret = fdt_get_resource(gd->fdt_blob, mem, "reg", reg++, &res); - if (ret == -FDT_ERR_NOTFOUND) { + while (ofnode_read_resource(mem, reg++, &res)) { reg = 0; - mem = get_next_memory_node(gd->fdt_blob, mem); - if (mem == -FDT_ERR_NOTFOUND) - break; - - ret = fdt_get_resource(gd->fdt_blob, mem, "reg", reg++, &res); - if (ret == -FDT_ERR_NOTFOUND) - break; - } - if (ret != 0) { - return -EINVAL; + mem = get_next_memory_node(mem); + if (!ofnode_valid(mem)) { + if (bank) + return 0; + goto missing_node; + } } gd->bd->bi_dram[bank].start = (phys_addr_t)res.start; @@ -1229,6 +1223,10 @@ int fdtdec_setup_memory_banksize(void) } return 0; + +missing_node: + debug("%s: Missing /memory node\n", __func__); + return -EINVAL; } #endif From 9bf86506777d02141b76249413534856162aad6a Mon Sep 17 00:00:00 2001 From: Liviu Dudau Date: Mon, 17 Sep 2018 17:43:08 +0100 Subject: [PATCH 41/46] include/clk.h: Fix the name of the clock uclass in comment The comment references a structure name that doesn't exist. Use the name of the actual uclass. Signed-off-by: Liviu Dudau Reviewed-by: Simon Glass Reviewed-by: Heiko Schocher Drop period at end of commit subject: Signed-off-by: Simon Glass --- include/clk.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/clk.h b/include/clk.h index c0a20cd..8e36616 100644 --- a/include/clk.h +++ b/include/clk.h @@ -21,7 +21,7 @@ * * A driver that implements UCLASS_CLOCK is a clock provider. A provider will * often implement multiple separate clocks, since the hardware it manages - * often has this capability. clock_uclass.h describes the interface which + * often has this capability. clk-uclass.h describes the interface which * clock providers must implement. * * Clock consumers/clients are the HW modules driven by the clock signals. This From 3eb992a481157a954e7c436faa7eeae757767621 Mon Sep 17 00:00:00 2001 From: Liviu Dudau Date: Mon, 17 Sep 2018 17:46:07 +0100 Subject: [PATCH 42/46] include/dm.h: Remove duplicated include directive Remove duplicated inclusion of dm/ofnode.h Signed-off-by: Liviu Dudau Reviewed-by: Simon Glass Reviewed-by: Heiko Schocher Drop period at end of commit subject: Signed-off-by: Simon Glass --- include/dm.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/dm.h b/include/dm.h index bf4b07d..2e1afda 100644 --- a/include/dm.h +++ b/include/dm.h @@ -6,7 +6,6 @@ #ifndef _DM_H_ #define _DM_H_ -#include #include #include #include From e62a24ce27ab86efc1b37d14112c29d3f2010238 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 17 Sep 2018 23:55:42 -0600 Subject: [PATCH 43/46] buildman: Avoid hanging when the config changes Something has changed in the last several month such that when buildman builds U-Boot incrementally and a new CONFIG option has been added to the Kconfig, the build hanges waiting for input: Test new config (NEW_CONFIG) [N/y/?] (NEW) Since binamn does not connect the build's stdin to anything this waits on stdin to the build thread, which never comes. Eventually I suspect all the threads end up in this state and the build does not progress. Fix this by passing /dev/null as input to the build. That way, if there is a new CONFIG, the build will stop (and fail): Test new config (NEW_CONFIG) [N/y/?] (NEW) Error in reading or end of file. Signed-off-by: Simon Glass --- tools/buildman/builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index a5a2ffd..05f8299 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -408,7 +408,7 @@ class Builder: """ cmd = [self.gnu_make] + list(args) result = command.RunPipe([cmd], capture=True, capture_stderr=True, - cwd=cwd, raise_on_error=False, **kwargs) + cwd=cwd, raise_on_error=False, infile='/dev/null', **kwargs) if self.verbose_build: result.stdout = '%s\n' % (' '.join(cmd)) + result.stdout result.combined = '%s\n' % (' '.join(cmd)) + result.combined From 969c8f4d5a341ee5ba47dbbe8cf382a250eb8cb7 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 18 Sep 2018 18:43:28 -0600 Subject: [PATCH 44/46] sandbox: Add an explanation of the sandbox variants There are quite a few builds of sandbox now. Add information about these to the README. Signed-off-by: Simon Glass --- board/sandbox/README.sandbox | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/board/sandbox/README.sandbox b/board/sandbox/README.sandbox index b49042d..a28fc9f 100644 --- a/board/sandbox/README.sandbox +++ b/board/sandbox/README.sandbox @@ -193,6 +193,30 @@ device are supported. Also sandbox supports driver model (CONFIG_DM) and associated commands. +Sandbox Variants +---------------- + +There are unfortunately quite a few variants at present: + +sandbox - should be used for most tests +sandbox64 - special build that forces a 64-bit host +sandbox_flattree - builds with dev_read_...() functions defined as inline. + We need this build so that we can test those inline functions, and we + cannot build with both the inline functions and the non-inline functions + since they are named the same. +sandbox_noblk - builds without CONFIG_BLK, which means the legacy block + drivers are used. We cannot use both the legacy and driver-model block + drivers since they implement the same functions +sandbox_spl - builds sandbox with SPL support, so you can run spl/u-boot-spl + and it will start up and then load ./u-boot. It is also possible to + run ./u-boot directly. + +Of these sandbox_noblk can be removed once CONFIG_BLK is used everwhere, and +sandbox_spl can probably be removed since it is a superset of sandbox. + +Most of the config options should be identical between these variants. + + Linux RAW Networking Bridge --------------------------- From e7a52ba65be227f4fa06b851d1d92af1bef694e0 Mon Sep 17 00:00:00 2001 From: Rajan Vaja Date: Wed, 19 Sep 2018 03:43:43 -0700 Subject: [PATCH 45/46] firmware: Add FIRMWARE config prompt string There is no prompt string for FIRMWARE config. Without this, FIRMWARE config cannot be enabled through menuconfing or config file. Fix this by adding prompt summary. Signed-off-by: Rajan Vaja Reviewed-by: Simon Glass --- drivers/firmware/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index cb73b70..feaea81 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -1,5 +1,5 @@ config FIRMWARE - bool + bool "Enable Firmware driver support" config ARM_PSCI_FW bool From 31b8217e83a63d1c8c70edcdcdf5aff3b1791640 Mon Sep 17 00:00:00 2001 From: Rajan Vaja Date: Wed, 19 Sep 2018 03:43:46 -0700 Subject: [PATCH 46/46] dm: test: Add "/firmware" node scan test Add a test which verifies that all subnodes under "/firmware" nodes are scanned. Signed-off-by: Rajan Vaja Reviewed-by: Simon Glass Added 'imply FIRMWARE' to sandbox Kconfig to fix test failures, fixed ordering of lines in arch/sandbox/dts/test.dts and test/dm/Makefile, updated #if condition in drivers/firmware/firmware-uclass.c: Signed-off-by: Simon Glass --- arch/Kconfig | 1 + arch/sandbox/dts/test.dts | 6 ++++++ drivers/firmware/Makefile | 1 + drivers/firmware/firmware-sandbox.c | 20 ++++++++++++++++++++ drivers/firmware/firmware-uclass.c | 2 +- test/dm/Makefile | 1 + test/dm/firmware.c | 22 ++++++++++++++++++++++ 7 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/firmware-sandbox.c create mode 100644 test/dm/firmware.c diff --git a/arch/Kconfig b/arch/Kconfig index 11900b0..9b4bcbf 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -89,6 +89,7 @@ config SANDBOX imply CMD_SF_TEST imply CRC32_VERIFY imply FAT_WRITE + imply FIRMWARE imply HASH_VERIFY imply LZMA imply SCSI diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 751c13b..42ceeb9 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -190,6 +190,12 @@ fake-host-hwaddr = [00 00 66 44 22 22]; }; + firmware { + sandbox_firmware: sandbox-firmware { + compatible = "sandbox,firmware"; + }; + }; + gpio_a: base-gpios { compatible = "sandbox,gpio"; gpio-controller; diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile index 1cdda14..6cb8358 100644 --- a/drivers/firmware/Makefile +++ b/drivers/firmware/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_FIRMWARE) += firmware-uclass.o obj-$(CONFIG_ARM_PSCI_FW) += psci.o obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o +obj-$(CONFIG_SANDBOX) += firmware-sandbox.o diff --git a/drivers/firmware/firmware-sandbox.c b/drivers/firmware/firmware-sandbox.c new file mode 100644 index 0000000..d970d75 --- /dev/null +++ b/drivers/firmware/firmware-sandbox.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * sandbox firmware driver + * + * Copyright (C) 2018 Xilinx, Inc. + */ + +#include +#include + +static const struct udevice_id generic_sandbox_firmware_ids[] = { + { .compatible = "sandbox,firmware" }, + { } +}; + +U_BOOT_DRIVER(sandbox_firmware) = { + .name = "sandbox_firmware", + .id = UCLASS_FIRMWARE, + .of_match = generic_sandbox_firmware_ids, +}; diff --git a/drivers/firmware/firmware-uclass.c b/drivers/firmware/firmware-uclass.c index 3d33b6d..7fcd7fb 100644 --- a/drivers/firmware/firmware-uclass.c +++ b/drivers/firmware/firmware-uclass.c @@ -7,7 +7,7 @@ UCLASS_DRIVER(firmware) = { .id = UCLASS_FIRMWARE, .name = "firmware", -#if CONFIG_IS_ENABLED(OF_CONTROL) +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) .post_bind = dm_scan_fdt_dev, #endif }; diff --git a/test/dm/Makefile b/test/dm/Makefile index d7f5d6b..00acc7f 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_BLK) += blk.o obj-$(CONFIG_BOARD) += board.o obj-$(CONFIG_CLK) += clk.o obj-$(CONFIG_DM_ETH) += eth.o +obj-$(CONFIG_FIRMWARE) += firmware.o obj-$(CONFIG_DM_GPIO) += gpio.o obj-$(CONFIG_DM_I2C) += i2c.o obj-$(CONFIG_LED) += led.o diff --git a/test/dm/firmware.c b/test/dm/firmware.c new file mode 100644 index 0000000..60fdcbb --- /dev/null +++ b/test/dm/firmware.c @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2018 Xilinx, Inc. + */ + +#include +#include +#include +#include +#include +#include + +/* Base test of firmware probe */ +static int dm_test_firmware_probe(struct unit_test_state *uts) +{ + struct udevice *dev; + + ut_assertok(uclass_get_device_by_name(UCLASS_FIRMWARE, + "sandbox-firmware", &dev)); + return 0; +} +DM_TEST(dm_test_firmware_probe, DM_TESTF_SCAN_FDT);