From 16d836cd6fa0d22e24fa2340df08e6d6d80c5107 Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Fri, 6 Jul 2018 10:27:14 -0600 Subject: binman: Switch to 'python-coverage' The most portable way to get access to coverage is to invoke it as 'python-coverage'. Cc: Simon Glass Signed-off-by: Tom Rini Signed-off-by: Simon Glass --- tools/binman/README | 3 +-- tools/binman/binman.py | 9 +++++---- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index 22f21bc5b44..f74e39242f6 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -583,8 +583,7 @@ implementations target 100% test coverage. Run 'binman -T' to check this. To enable Python test coverage on Debian-type distributions (e.g. Ubuntu): - $ sudo apt-get install python-pip python-pytest - $ sudo pip install coverage + $ sudo apt-get install python-coverage python-pytest Advanced Features / Technical docs diff --git a/tools/binman/binman.py b/tools/binman/binman.py index 31b045337d2..944fd5d7ba3 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -81,24 +81,25 @@ def RunTests(debug, args): def RunTestCoverage(): """Run the tests and check that we get 100% coverage""" # This uses the build output from sandbox_spl to get _libfdt.so - cmd = ('PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools coverage run ' + cmd = ('PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools python-coverage run ' '--include "tools/binman/*.py" --omit "*test*,*binman.py" ' 'tools/binman/binman.py -t' % options.build_dir) os.system(cmd) - stdout = command.Output('coverage', 'report') + stdout = command.Output('python-coverage', 'report') lines = stdout.splitlines() test_set= set([os.path.basename(line.split()[0]) for line in lines if '/etype/' in line]) glob_list = glob.glob(os.path.join(our_path, 'etype/*.py')) - all_set = set([os.path.basename(item) for item in glob_list]) + all_set = set([os.path.splitext(os.path.basename(item))[0] + for item in glob_list if '_testing' not in item]) missing_list = all_set missing_list.difference_update(test_set) - missing_list.remove('_testing.py') coverage = lines[-1].split(' ')[-1] ok = True if missing_list: print 'Missing tests for %s' % (', '.join(missing_list)) + print stdout ok = False if coverage != '100%': print stdout -- cgit v1.2.3 From 1835cd2264ebd8814624b738d6e25ee772682cbe Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:15 -0600 Subject: binman: Make the operation of Entry__testing explicit This fake entry is used for testing. At present it only has one behaviour which is to return an invalid set of entry positions, to cause an error. The fake entry will need to be used for other things too. Allow the test .dts file to specify the behaviour of the fake entry, so we can control its behaviour easily. While we are here, drop the ReadContents() method, since this only applies to subclasses of Entry_blob, which Entry__testing is not. Signed-off-by: Simon Glass --- tools/binman/etype/_testing.py | 14 ++++++++++---- tools/binman/test/41_unknown_pos_size.dts | 1 + 2 files changed, 11 insertions(+), 4 deletions(-) (limited to 'tools') diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index c376dd5c9ca..0b1eaefc3ce 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -10,16 +10,22 @@ import fdt_util import tools class Entry__testing(Entry): + """A fake entry used for testing + + Properties: + return_invalid_entry: Return an invalid entry from GetPositions() + """ def __init__(self, section, etype, node): Entry.__init__(self, section, etype, node) + self.return_invalid_entry = fdt_util.GetBool(self._node, + 'return-invalid-entry') def ObtainContents(self): self.data = 'a' self.contents_size = len(self.data) return True - def ReadContents(self): - return True - def GetPositions(self): - return {'invalid-entry': [1, 2]} + if self.return_invalid_entry : + return {'invalid-entry': [1, 2]} + return {} diff --git a/tools/binman/test/41_unknown_pos_size.dts b/tools/binman/test/41_unknown_pos_size.dts index a8e7d8aa227..94fe821c470 100644 --- a/tools/binman/test/41_unknown_pos_size.dts +++ b/tools/binman/test/41_unknown_pos_size.dts @@ -6,6 +6,7 @@ binman { _testing { + return-invalid-entry; }; }; }; -- cgit v1.2.3 From adc5701134f7041d7f767b672b66afb8b64ba5ce Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:16 -0600 Subject: binman: Tidy up variables in _RunMicrocodeTest() At present we call the three entries first, second and third. Rename them to reflect their contents instead, for clarity. Signed-off-by: Simon Glass --- tools/binman/ftest.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) (limited to 'tools') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index eb8a0793cbe..80eadeffab8 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -689,29 +689,40 @@ class TestFunctional(unittest.TestCase): self.assertEqual(X86_START16_DATA, data[:len(X86_START16_DATA)]) def _RunMicrocodeTest(self, dts_fname, nodtb_data): + """Handle running a test for insertion of microcode + + Args: + dts_fname: Name of test .dts file + nodtb_data: Data that we expect in the first section + + Returns: + Tuple: + Contents of first region (U-Boot or SPL) + Position and size components of microcode pointer, as inserted + in the above (two 4-byte words) + """ data = self._DoReadFile(dts_fname, True) # Now check the device tree has no microcode - second = data[len(nodtb_data):] + dtb_with_ucode = data[len(nodtb_data):] + fdt_len = self.GetFdtLen(dtb_with_ucode) + ucode_content = dtb_with_ucode[fdt_len:] + ucode_pos = len(nodtb_data) + fdt_len fname = tools.GetOutputFilename('test.dtb') with open(fname, 'wb') as fd: - fd.write(second) + fd.write(dtb_with_ucode) dtb = fdt.FdtScan(fname) ucode = dtb.GetNode('/microcode') self.assertTrue(ucode) for node in ucode.subnodes: self.assertFalse(node.props.get('data')) - fdt_len = self.GetFdtLen(second) - third = second[fdt_len:] - # Check that the microcode appears immediately after the Fdt # This matches the concatenation of the data properties in # the /microcode/update@xxx nodes in 34_x86_ucode.dts. ucode_data = struct.pack('>4L', 0x12345678, 0x12345679, 0xabcd0000, 0x78235609) - self.assertEqual(ucode_data, third[:len(ucode_data)]) - ucode_pos = len(nodtb_data) + fdt_len + self.assertEqual(ucode_data, ucode_content[:len(ucode_data)]) # Check that the microcode pointer was inserted. It should match the # expected position and size -- cgit v1.2.3 From 736bb0aec80891891f3f86004e1afa7976585918 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:17 -0600 Subject: binman: Correct operation of ObtainContents() This method is supposed to return the contents of an entry. However at present there is no check that it actually does. Also some implementations do not return 'True' to indicate success, as required. Add a check for things working as expected, and correct the implementations. This requires some additional test cases to cover things which were missed originally. Add these at the same time. Signed-off-by: Simon Glass --- tools/binman/bsection.py | 4 ++ tools/binman/etype/_testing.py | 5 ++ tools/binman/etype/section.py | 2 +- tools/binman/etype/u_boot_spl_bss_pad.py | 1 + tools/binman/etype/u_boot_ucode.py | 9 +++- tools/binman/ftest.py | 57 +++++++++++++++++----- tools/binman/test/57_unknown_contents.dts | 14 ++++++ tools/binman/test/58_x86_ucode_spl_needs_retry.dts | 36 ++++++++++++++ 8 files changed, 114 insertions(+), 14 deletions(-) create mode 100644 tools/binman/test/57_unknown_contents.dts create mode 100644 tools/binman/test/58_x86_ucode_spl_needs_retry.dts (limited to 'tools') diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 3f30f6e4fe7..06a6711350a 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -162,6 +162,10 @@ class Section(object): todo = next_todo if not todo: break + if todo: + self._Raise('Internal error: Could not complete processing of ' + 'contents: remaining %s' % todo) + return True def _SetEntryPosSize(self, name, pos, size): """Set the position and size of an entry diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index 0b1eaefc3ce..c075c3ff0d9 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -9,6 +9,7 @@ from entry import Entry import fdt_util import tools + class Entry__testing(Entry): """A fake entry used for testing @@ -19,8 +20,12 @@ class Entry__testing(Entry): Entry.__init__(self, section, etype, node) self.return_invalid_entry = fdt_util.GetBool(self._node, 'return-invalid-entry') + self.return_unknown_contents = fdt_util.GetBool(self._node, + 'return-unknown-contents') def ObtainContents(self): + if self.return_unknown_contents: + return False self.data = 'a' self.contents_size = len(self.data) return True diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 139fcad51af..36b31a849f8 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -18,7 +18,7 @@ class Entry_section(Entry): self._section = bsection.Section(node.name, node) def ObtainContents(self): - self._section.GetEntryContents() + return self._section.GetEntryContents() def GetData(self): return self._section.GetData() diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index 3d2dea2e0d8..6c397957e30 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -24,3 +24,4 @@ class Entry_u_boot_spl_bss_pad(Entry_blob): self.Raise('Expected __bss_size symbol in spl/u-boot-spl') self.data = chr(0) * bss_size self.contents_size = bss_size + return True diff --git a/tools/binman/etype/u_boot_ucode.py b/tools/binman/etype/u_boot_ucode.py index 3a0cff7c3a3..8cae7deed3c 100644 --- a/tools/binman/etype/u_boot_ucode.py +++ b/tools/binman/etype/u_boot_ucode.py @@ -64,9 +64,14 @@ class Entry_u_boot_ucode(Entry_blob): self.data = '' return True - # Get the microcode from the device tree entry + # 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') - if not fdt_entry or not fdt_entry.ucode_data: + if not fdt_entry: + return True + if not fdt_entry.ucode_data: return False if not fdt_entry.collate: diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 80eadeffab8..ca9d158eef3 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -688,12 +688,14 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('33_x86-start16.dts') self.assertEqual(X86_START16_DATA, data[:len(X86_START16_DATA)]) - def _RunMicrocodeTest(self, dts_fname, nodtb_data): + def _RunMicrocodeTest(self, dts_fname, nodtb_data, ucode_second=False): """Handle running a test for insertion of microcode Args: dts_fname: Name of test .dts file nodtb_data: Data that we expect in the first section + ucode_second: True if the microsecond entry is second instead of + third Returns: Tuple: @@ -704,10 +706,16 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile(dts_fname, True) # Now check the device tree has no microcode - dtb_with_ucode = data[len(nodtb_data):] - fdt_len = self.GetFdtLen(dtb_with_ucode) - ucode_content = dtb_with_ucode[fdt_len:] - ucode_pos = len(nodtb_data) + fdt_len + if ucode_second: + ucode_content = data[len(nodtb_data):] + ucode_pos = len(nodtb_data) + dtb_with_ucode = ucode_content[16:] + fdt_len = self.GetFdtLen(dtb_with_ucode) + else: + dtb_with_ucode = data[len(nodtb_data):] + fdt_len = self.GetFdtLen(dtb_with_ucode) + ucode_content = dtb_with_ucode[fdt_len:] + ucode_pos = len(nodtb_data) + fdt_len fname = tools.GetOutputFilename('test.dtb') with open(fname, 'wb') as fd: fd.write(dtb_with_ucode) @@ -728,8 +736,8 @@ class TestFunctional(unittest.TestCase): # expected position and size pos_and_size = struct.pack('<2L', 0xfffffe00 + ucode_pos, len(ucode_data)) - first = data[:len(nodtb_data)] - return first, pos_and_size + u_boot = data[:len(nodtb_data)] + return u_boot, pos_and_size def testPackUbootMicrocode(self): """Test that x86 microcode can be handled correctly @@ -892,23 +900,42 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('48_x86-start16-spl.dts') self.assertEqual(X86_START16_SPL_DATA, data[:len(X86_START16_SPL_DATA)]) - def testPackUbootSplMicrocode(self): - """Test that x86 microcode can be handled correctly in SPL + def _PackUbootSplMicrocode(self, dts, ucode_second=False): + """Helper function for microcode tests We expect to see the following in the image, in order: u-boot-spl-nodtb.bin with a microcode pointer inserted at the correct place u-boot.dtb with the microcode removed the microcode + + Args: + dts: Device tree file to use for test + ucode_second: True if the microsecond entry is second instead of + third """ # ELF file with a '_dt_ucode_base_size' symbol with open(self.TestFile('u_boot_ucode_ptr')) as fd: TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) - first, pos_and_size = self._RunMicrocodeTest('49_x86_ucode_spl.dts', - U_BOOT_SPL_NODTB_DATA) + first, pos_and_size = self._RunMicrocodeTest(dts, U_BOOT_SPL_NODTB_DATA, + ucode_second=ucode_second) self.assertEqual('splnodtb with microc' + pos_and_size + 'ter somewhere in here', first) + def testPackUbootSplMicrocode(self): + """Test that x86 microcode can be handled correctly in SPL""" + self._PackUbootSplMicrocode('49_x86_ucode_spl.dts') + + def testPackUbootSplMicrocodeReorder(self): + """Test that order doesn't matter for microcode entries + + This is the same as testPackUbootSplMicrocode but when we process the + u-boot-ucode entry we have not yet seen the u-boot-dtb-with-ucode + entry, so we reply on binman to try later. + """ + self._PackUbootSplMicrocode('58_x86_ucode_spl_needs_retry.dts', + ucode_second=True) + def testPackMrc(self): """Test that an image with an MRC binary can be created""" data = self._DoReadFile('50_intel_mrc.dts') @@ -971,5 +998,13 @@ class TestFunctional(unittest.TestCase): 00000000 00000004 rw-u-boot ''', map_data) + def testUnknownContents(self): + """Test that obtaining the contents works as expected""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('57_unknown_contents.dts', True) + self.assertIn("Section '/binman': Internal error: Could not complete " + "processing of contents: remaining [<_testing.Entry__testing ", + str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/57_unknown_contents.dts b/tools/binman/test/57_unknown_contents.dts new file mode 100644 index 00000000000..6ea98d7cab6 --- /dev/null +++ b/tools/binman/test/57_unknown_contents.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + return-unknown-contents; + }; + }; +}; diff --git a/tools/binman/test/58_x86_ucode_spl_needs_retry.dts b/tools/binman/test/58_x86_ucode_spl_needs_retry.dts new file mode 100644 index 00000000000..e2cb80cf6e2 --- /dev/null +++ b/tools/binman/test/58_x86_ucode_spl_needs_retry.dts @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + sort-by-pos; + end-at-4gb; + size = <0x200>; + u-boot-spl-with-ucode-ptr { + }; + + /* + * Microcode goes before the DTB which contains it, so binman + * will need to obtain the contents of the next section before + * obtaining the contents of this one. + */ + u-boot-ucode { + }; + + u-boot-dtb-with-ucode { + }; + }; + + microcode { + update@0 { + data = <0x12345678 0x12345679>; + }; + update@1 { + data = <0xabcd0000 0x78235609>; + }; + }; +}; -- cgit v1.2.3 From 2cd01285b53f376e439e4cbdbce808ca8231ef84 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:18 -0600 Subject: binman: Tidy up execution of tests Move all the test execution into the same mechanism so that we can request a particular test (from any suite) by passing it as an argument to 'binman -t'. Signed-off-by: Simon Glass --- tools/binman/binman.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/binman/binman.py b/tools/binman/binman.py index 944fd5d7ba3..74862b1146e 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -54,14 +54,12 @@ def RunTests(debug, args): # Run the entry tests first ,since these need to be the first to import the # 'entry' module. - suite = unittest.TestLoader().loadTestsFromTestCase(entry_test.TestEntry) - suite.run(result) test_name = args and args[0] or None - for module in (ftest.TestFunctional, fdt_test.TestFdt, elf_test.TestElf, - image_test.TestImage): + for module in (entry_test.TestEntry, ftest.TestFunctional, fdt_test.TestFdt, + elf_test.TestElf, image_test.TestImage): if test_name: try: - suite = unittest.TestLoader().loadTestsFromName(args[0], module) + suite = unittest.TestLoader().loadTestsFromName(test_name, module) except AttributeError: continue else: -- cgit v1.2.3 From 5c890238c480a96d4d0b06c92199e21867170c31 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:19 -0600 Subject: binman: Tidy up setting of entry contents At present the contents of an entry are set in subclasses simply by assigning to the data and content_size properties. Add some methods to do this, so that we have more control. In particular, add a method to set the contents without changing its size, so we can validate that case. Add a test case for trying to change the size when this is not allowed. Signed-off-by: Simon Glass --- tools/binman/entry.py | 28 ++++++++++++++++++++++++++++ tools/binman/etype/_testing.py | 8 ++++++++ tools/binman/etype/blob.py | 3 +-- tools/binman/etype/u_boot_spl_bss_pad.py | 3 +-- tools/binman/etype/u_boot_with_ucode_ptr.py | 4 ++-- tools/binman/ftest.py | 8 ++++++++ tools/binman/test/59_change_size.dts | 14 ++++++++++++++ 7 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 tools/binman/test/59_change_size.dts (limited to 'tools') diff --git a/tools/binman/entry.py b/tools/binman/entry.py index e4d688c91f9..303c992e375 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -55,6 +55,7 @@ class Entry(object): self.name = node and (name_prefix + node.name) or 'none' self.pos = None self.size = None + self.data = '' self.contents_size = 0 self.align = None self.align_size = None @@ -138,6 +139,33 @@ class Entry(object): if prefix: self.name = prefix + self.name + def SetContents(self, data): + """Set the contents of an entry + + This sets both the data and content_size properties + + Args: + data: Data to set to the contents (string) + """ + self.data = data + self.contents_size = len(self.data) + + def ProcessContentsUpdate(self, data): + """Update the contens of an entry, after the size is fixed + + This checks that the new data is the same size as the old. + + Args: + data: Data to set to the contents (string) + + Raises: + ValueError if the new data size is not the same as the old + """ + if len(data) != self.contents_size: + self.Raise('Cannot update entry size from %d to %d' % + (len(data), self.contents_size)) + self.SetContents(data) + def ObtainContents(self): """Figure out the contents of an entry. diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index c075c3ff0d9..04bdc6c532b 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -22,6 +22,8 @@ class Entry__testing(Entry): 'return-invalid-entry') self.return_unknown_contents = fdt_util.GetBool(self._node, 'return-unknown-contents') + self.bad_update_contents = fdt_util.GetBool(self._node, + 'bad-update-contents') def ObtainContents(self): if self.return_unknown_contents: @@ -34,3 +36,9 @@ class Entry__testing(Entry): if self.return_invalid_entry : return {'invalid-entry': [1, 2]} return {} + + def ProcessContents(self): + if self.bad_update_contents: + # Request to update the conents with something larger, to cause a + # failure. + self.ProcessContentsUpdate('aa') diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 16b1e5f64d9..28e6651a935 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -28,8 +28,7 @@ class Entry_blob(Entry): # 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.data = fd.read() - self.contents_size = len(self.data) + self.SetContents(fd.read()) return True def GetDefaultFilename(self): diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index 6c397957e30..65f631d3c5e 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -22,6 +22,5 @@ class Entry_u_boot_spl_bss_pad(Entry_blob): bss_size = elf.GetSymbolAddress(fname, '__bss_size') if not bss_size: self.Raise('Expected __bss_size symbol in spl/u-boot-spl') - self.data = chr(0) * bss_size - self.contents_size = bss_size + self.SetContents(chr(0) * bss_size) 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 41c2ded2fe8..86945f33184 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -81,5 +81,5 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): # Write the microcode position and size into the entry pos_and_size = struct.pack('<2L', pos, size) self.target_pos -= self.pos - self.data = (self.data[:self.target_pos] + pos_and_size + - self.data[self.target_pos + 8:]) + self.ProcessContentsUpdate(self.data[:self.target_pos] + pos_and_size + + self.data[self.target_pos + 8:]) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index ca9d158eef3..af3b4dc3e56 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -1006,5 +1006,13 @@ class TestFunctional(unittest.TestCase): "processing of contents: remaining [<_testing.Entry__testing ", str(e.exception)) + def testBadChangeSize(self): + """Test that trying to change the size of an entry fails""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('59_change_size.dts', True) + self.assertIn("Node '/binman/_testing': Cannot update entry size from " + '2 to 1', str(e.exception)) + + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/59_change_size.dts b/tools/binman/test/59_change_size.dts new file mode 100644 index 00000000000..1a69026a64c --- /dev/null +++ b/tools/binman/test/59_change_size.dts @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + _testing { + bad-update-contents; + }; + }; +}; -- cgit v1.2.3 From 3def0cf238e0df9736a4ce8fb54c1eb561a56ddd Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:20 -0600 Subject: libfdt: Bring in proposed pylibfdt changes This provides various patches sent to the devicetree-compiler mailing list to enhance the Python bindings. A final version of this patch may be created once upstreaming is complete, but if it takes too long, this can act as a placeholder. New pylibfdt features: - Support for most remaining, relevant libfdt functions - Support for sequential-write functions Changes are applied to existing U-Boot tools as needed. Signed-off-by: Simon Glass --- tools/dtoc/dtoc.py | 20 ++++++++++++++++---- tools/dtoc/fdt.py | 3 +-- tools/dtoc/test_dtoc.py | 3 ++- 3 files changed, 19 insertions(+), 7 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/dtoc.py b/tools/dtoc/dtoc.py index 008c0f4d693..c891b063800 100755 --- a/tools/dtoc/dtoc.py +++ b/tools/dtoc/dtoc.py @@ -36,14 +36,26 @@ sys.path.append(os.path.join(our_path, '../patman')) import dtb_platdata -def run_tests(): - """Run all the test we have for dtoc""" +def run_tests(args): + """Run all the test we have for dtoc + + Args: + args: List of positional args provided to binman. This can hold a test + name to execute (as in 'binman -t testSections', for example) + """ import test_dtoc result = unittest.TestResult() sys.argv = [sys.argv[0]] + test_name = args and args[0] or None for module in (test_dtoc.TestDtoc,): - suite = unittest.TestLoader().loadTestsFromTestCase(module) + if test_name: + try: + suite = unittest.TestLoader().loadTestsFromName(test_name, module) + except AttributeError: + continue + else: + suite = unittest.TestLoader().loadTestsFromTestCase(module) suite.run(result) print result @@ -68,7 +80,7 @@ parser.add_option('-t', '--test', action='store_true', dest='test', # Run our meagre tests if options.test: - run_tests() + run_tests(args) else: dtb_platdata.run_steps(args, options.dtb_file, options.include_disabled, diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 7fab0cd8e90..d08b0b53e61 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -234,7 +234,6 @@ class Node: be updated. """ if self._offset != my_offset: - #print '%s: %d -> %d\n' % (self.path, self._offset, my_offset) self._offset = my_offset offset = libfdt.fdt_first_subnode(self._fdt.GetFdt(), self._offset) for subnode in self.subnodes: @@ -359,7 +358,7 @@ class Fdt: poffset = libfdt.fdt_first_property_offset(self._fdt, node._offset) while poffset >= 0: p = self._fdt_obj.get_property_by_offset(poffset) - prop = Prop(node, poffset, p.name, p.value) + prop = Prop(node, poffset, p.name, p) props_dict[prop.name] = prop poffset = libfdt.fdt_next_property_offset(self._fdt, poffset) diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 99f4e1a13a4..e475a7eb14e 100644 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -4,7 +4,8 @@ """Tests for the dtb_platdata module -This includes unit tests for some functions and functional tests for +This includes unit tests for some functions and functional tests for the dtoc +tool. """ import collections -- cgit v1.2.3 From ff1fd6ccde3d166213d1277fa6b6b9685d45044f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:23 -0600 Subject: binman: Move coverage logic into a new test_util file At present only binman has the logic for determining Python test coverage but this is useful for other tools also. Move it out into a separate file so it can be used by other tools. Signed-off-by: Simon Glass --- tools/binman/binman.py | 29 +++------------------ tools/patman/test_util.py | 64 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 25 deletions(-) create mode 100644 tools/patman/test_util.py (limited to 'tools') diff --git a/tools/binman/binman.py b/tools/binman/binman.py index 74862b1146e..52e02ed91b4 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -26,6 +26,7 @@ sys.path.insert(0, 'scripts/dtc/pylibfdt') import cmdline import command import control +import test_util def RunTests(debug, args): """Run the functional tests and any embedded doctests @@ -78,34 +79,12 @@ def RunTests(debug, args): def RunTestCoverage(): """Run the tests and check that we get 100% coverage""" - # This uses the build output from sandbox_spl to get _libfdt.so - cmd = ('PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools python-coverage run ' - '--include "tools/binman/*.py" --omit "*test*,*binman.py" ' - 'tools/binman/binman.py -t' % options.build_dir) - os.system(cmd) - stdout = command.Output('python-coverage', 'report') - lines = stdout.splitlines() - - test_set= set([os.path.basename(line.split()[0]) - for line in lines if '/etype/' in line]) glob_list = glob.glob(os.path.join(our_path, 'etype/*.py')) all_set = set([os.path.splitext(os.path.basename(item))[0] for item in glob_list if '_testing' not in item]) - missing_list = all_set - missing_list.difference_update(test_set) - coverage = lines[-1].split(' ')[-1] - ok = True - if missing_list: - print 'Missing tests for %s' % (', '.join(missing_list)) - print stdout - ok = False - if coverage != '100%': - print stdout - print "Type 'coverage html' to get a report in htmlcov/index.html" - print 'Coverage error: %s, but should be 100%%' % coverage - ok = False - if not ok: - raise ValueError('Test coverage failure') + test_util.RunTestCoverage('tools/binman/binman.py', None, + ['*test*', '*binman.py', 'tools/patman/*', 'tools/dtoc/*'], + options.build_dir, all_set) def RunBinman(options, args): """Main entry point to binman once arguments are parsed diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py new file mode 100644 index 00000000000..1a33c997c4c --- /dev/null +++ b/tools/patman/test_util.py @@ -0,0 +1,64 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (c) 2016 Google, Inc +# + +import glob +import os +import sys + +import command + +def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): + """Run tests and check that we get 100% coverage + + Args: + prog: Program to run (with be passed a '-t' argument to run tests + filter_fname: Normally all *.py files in the program's directory will + be included. If this is not None, then it is used to filter the + list so that only filenames that don't contain filter_fname are + included. + exclude_list: List of file patterns to exclude from the coverage + calculation + build_dir: Build directory, used to locate libfdt.py + required: List of modules which must be in the coverage report + + Raises: + ValueError if the code coverage is not 100% + """ + # This uses the build output from sandbox_spl to get _libfdt.so + path = os.path.dirname(prog) + if filter_fname: + glob_list = glob.glob(os.path.join(path, '*.py')) + glob_list = [fname for fname in glob_list if filter_fname in fname] + else: + glob_list = [] + glob_list += exclude_list + glob_list += ['*libfdt.py', '*site-packages*'] + cmd = ('PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools python-coverage run ' + '--omit "%s" %s -t' % (build_dir, ','.join(glob_list), prog)) + os.system(cmd) + stdout = command.Output('python-coverage', 'report') + lines = stdout.splitlines() + if required: + # Convert '/path/to/name.py' just the module name 'name' + test_set = set([os.path.splitext(os.path.basename(line.split()[0]))[0] + for line in lines if '/etype/' in line]) + missing_list = required + missing_list.difference_update(test_set) + if missing_list: + print 'Missing tests for %s' % (', '.join(missing_list)) + print stdout + ok = False + + coverage = lines[-1].split(' ')[-1] + ok = True + print coverage + if coverage != '100%': + print stdout + print ("Type 'python-coverage html' to get a report in " + 'htmlcov/index.html') + print 'Coverage error: %s, but should be 100%%' % coverage + ok = False + if not ok: + raise ValueError('Test coverage failure') -- cgit v1.2.3 From 2ba987531254dfacb299e88406b3da73ea2d7915 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:24 -0600 Subject: dtoc: Add some tests for the fdt module At present this module is tested via the dtoc tests. This is a bit painful since the tests are at a higher level and so failures are more difficult to diagnose. Add some tests that exercise the fdt module directly. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 12 ++- tools/dtoc/test_fdt | 1 + tools/dtoc/test_fdt.py | 246 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 257 insertions(+), 2 deletions(-) create mode 120000 tools/dtoc/test_fdt create mode 100755 tools/dtoc/test_fdt.py (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index d08b0b53e61..5cde8c125ae 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -138,6 +138,7 @@ class Prop: else: return TYPE_INT, val + @classmethod def GetEmpty(self, type): """Get an empty / zero value of the given type @@ -335,12 +336,19 @@ class Fdt: """ return self._fdt - def CheckErr(errnum, msg): + def GetFdtObj(self): + """Get the contents of the FDT + + Returns: + The FDT contents as a libfdt.Fdt object + """ + return self._fdt_obj + + def CheckErr(self, errnum, msg): if errnum: raise ValueError('Error %d: %s: %s' % (errnum, libfdt.fdt_strerror(errnum), msg)) - def GetProps(self, node): """Get all properties from a node. diff --git a/tools/dtoc/test_fdt b/tools/dtoc/test_fdt new file mode 120000 index 00000000000..7c3b23031f0 --- /dev/null +++ b/tools/dtoc/test_fdt @@ -0,0 +1 @@ +test_fdt.py \ No newline at end of file diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py new file mode 100755 index 00000000000..ba660ca9b75 --- /dev/null +++ b/tools/dtoc/test_fdt.py @@ -0,0 +1,246 @@ +#!/usr/bin/python +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2018 Google, Inc +# Written by Simon Glass +# + +from optparse import OptionParser +import glob +import os +import sys +import unittest + +# Bring in the patman libraries +our_path = os.path.dirname(os.path.realpath(__file__)) +for dirname in ['../patman', '..']: + sys.path.insert(0, os.path.join(our_path, dirname)) + +import command +import fdt +from fdt import TYPE_BYTE, TYPE_INT, TYPE_STRING, TYPE_BOOL +from fdt_util import fdt32_to_cpu +import libfdt +import test_util +import tools + +class TestFdt(unittest.TestCase): + """Tests for the Fdt module + + This includes unit tests for some functions and functional tests for the fdt + module. + """ + @classmethod + def setUpClass(cls): + tools.PrepareOutputDir(None) + + @classmethod + def tearDownClass(cls): + tools._FinaliseForTest() + + def setUp(self): + self.dtb = fdt.FdtScan('tools/dtoc/dtoc_test_simple.dts') + + def testFdt(self): + """Test that we can open an Fdt""" + self.dtb.Scan() + root = self.dtb.GetRoot() + self.assertTrue(isinstance(root, fdt.Node)) + + def testGetNode(self): + """Test the GetNode() method""" + node = self.dtb.GetNode('/spl-test') + self.assertTrue(isinstance(node, fdt.Node)) + node = self.dtb.GetNode('/i2c@0/pmic@9') + self.assertTrue(isinstance(node, fdt.Node)) + self.assertEqual('pmic@9', node.name) + + def testFlush(self): + """Check that we can flush the device tree out to its file""" + fname = self.dtb._fname + with open(fname) as fd: + data = fd.read() + os.remove(fname) + with self.assertRaises(IOError): + open(fname) + self.dtb.Flush() + with open(fname) as fd: + data = fd.read() + + def testPack(self): + """Test that packing a device tree works""" + self.dtb.Pack() + + def testGetFdt(self): + """Tetst that we can access the raw device-tree data""" + self.assertTrue(isinstance(self.dtb.GetFdt(), bytearray)) + + def testGetProps(self): + """Tests obtaining a list of properties""" + node = self.dtb.GetNode('/spl-test') + props = self.dtb.GetProps(node) + self.assertEqual(['boolval', 'bytearray', 'byteval', 'compatible', + 'intarray', 'intval', 'longbytearray', + 'stringarray', 'stringval', 'u-boot,dm-pre-reloc'], + sorted(props.keys())) + + def testCheckError(self): + """Tests the ChecKError() function""" + with self.assertRaises(ValueError) as e: + self.dtb.CheckErr(-libfdt.NOTFOUND, 'hello') + self.assertIn('FDT_ERR_NOTFOUND: hello', str(e.exception)) + + +class TestNode(unittest.TestCase): + """Test operation of the Node class""" + + @classmethod + def setUpClass(cls): + tools.PrepareOutputDir(None) + + @classmethod + def tearDownClass(cls): + tools._FinaliseForTest() + + def setUp(self): + self.dtb = fdt.FdtScan('tools/dtoc/dtoc_test_simple.dts') + self.node = self.dtb.GetNode('/spl-test') + + def testOffset(self): + """Tests that we can obtain the offset of a node""" + self.assertTrue(self.node.Offset() > 0) + + def testDelete(self): + """Tests that we can delete a property""" + node2 = self.dtb.GetNode('/spl-test2') + offset1 = node2.Offset() + self.node.DeleteProp('intval') + offset2 = node2.Offset() + self.assertTrue(offset2 < offset1) + self.node.DeleteProp('intarray') + offset3 = node2.Offset() + self.assertTrue(offset3 < offset2) + + def testFindNode(self): + """Tests that we can find a node using the _FindNode() functoin""" + node = self.dtb.GetRoot()._FindNode('i2c@0') + self.assertEqual('i2c@0', node.name) + subnode = node._FindNode('pmic@9') + self.assertEqual('pmic@9', subnode.name) + + +class TestProp(unittest.TestCase): + """Test operation of the Prop class""" + + @classmethod + def setUpClass(cls): + tools.PrepareOutputDir(None) + + @classmethod + def tearDownClass(cls): + tools._FinaliseForTest() + + def setUp(self): + self.dtb = fdt.FdtScan('tools/dtoc/dtoc_test_simple.dts') + self.node = self.dtb.GetNode('/spl-test') + self.fdt = self.dtb.GetFdtObj() + + def testGetEmpty(self): + """Tests the GetEmpty() function for the various supported types""" + self.assertEqual(True, fdt.Prop.GetEmpty(fdt.TYPE_BOOL)) + self.assertEqual(chr(0), fdt.Prop.GetEmpty(fdt.TYPE_BYTE)) + self.assertEqual(chr(0) * 4, fdt.Prop.GetEmpty(fdt.TYPE_INT)) + self.assertEqual('', fdt.Prop.GetEmpty(fdt.TYPE_STRING)) + + def testGetOffset(self): + """Test we can get the offset of a property""" + prop = self.node.props['longbytearray'] + + # Add 12, which is sizeof(struct fdt_property), to get to start of data + offset = prop.GetOffset() + 12 + data = self.dtb._fdt[offset:offset + len(prop.value)] + bytes = [chr(x) for x in data] + self.assertEqual(bytes, prop.value) + + def testWiden(self): + """Test widening of values""" + node2 = self.dtb.GetNode('/spl-test2') + prop = self.node.props['intval'] + + # No action + prop2 = node2.props['intval'] + prop.Widen(prop2) + self.assertEqual(fdt.TYPE_INT, prop.type) + self.assertEqual(1, fdt32_to_cpu(prop.value)) + + # Convert singla value to array + prop2 = self.node.props['intarray'] + prop.Widen(prop2) + self.assertEqual(fdt.TYPE_INT, prop.type) + self.assertTrue(isinstance(prop.value, list)) + + # A 4-byte array looks like a single integer. When widened by a longer + # byte array, it should turn into an array. + prop = self.node.props['longbytearray'] + prop2 = node2.props['longbytearray'] + self.assertFalse(isinstance(prop2.value, list)) + self.assertEqual(4, len(prop2.value)) + prop2.Widen(prop) + self.assertTrue(isinstance(prop2.value, list)) + self.assertEqual(9, len(prop2.value)) + + # Similarly for a string array + prop = self.node.props['stringval'] + prop2 = node2.props['stringarray'] + self.assertFalse(isinstance(prop.value, list)) + self.assertEqual(7, len(prop.value)) + prop.Widen(prop2) + self.assertTrue(isinstance(prop.value, list)) + self.assertEqual(3, len(prop.value)) + + # Enlarging an existing array + prop = self.node.props['stringarray'] + prop2 = node2.props['stringarray'] + self.assertTrue(isinstance(prop.value, list)) + self.assertEqual(2, len(prop.value)) + prop.Widen(prop2) + self.assertTrue(isinstance(prop.value, list)) + self.assertEqual(3, len(prop.value)) + + +def RunTests(args): + """Run all the test we have for the fdt model + + Args: + args: List of positional args provided to fdt. This can hold a test + name to execute (as in 'fdt -t testFdt', for example) + """ + result = unittest.TestResult() + sys.argv = [sys.argv[0]] + test_name = args and args[0] or None + for module in (TestFdt, TestNode, TestProp): + if test_name: + try: + suite = unittest.TestLoader().loadTestsFromName(test_name, module) + except AttributeError: + continue + else: + suite = unittest.TestLoader().loadTestsFromTestCase(module) + suite.run(result) + + print result + for _, err in result.errors: + print err + for _, err in result.failures: + print err + +if __name__ != '__main__': + sys.exit(1) + +parser = OptionParser() +parser.add_option('-t', '--test', action='store_true', dest='test', + default=False, help='run tests') +(options, args) = parser.parse_args() + +# Run our meagre tests +if options.test: + RunTests(args) -- cgit v1.2.3 From 57f0bc428aeecbc30a89a10be05fa69fd3e64aef Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:25 -0600 Subject: dtoc: Update tests to write failures to /tmp When a test fails due to an output mismatch (e.g. due to a new property being adding to a test file) it is currently hard to update the test to the new output. In particular the tabs in the file are written as \t in the Python tests. To make this easier, write both the expected and actual results to /tmp to allow use of meld, and copying into the test. Signed-off-by: Simon Glass --- tools/dtoc/test_dtoc.py | 56 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 14 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index e475a7eb14e..ce6d2585a40 100644 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -68,6 +68,34 @@ class TestDtoc(unittest.TestCase): def tearDownClass(cls): tools._RemoveOutputDir() + def _WritePythonString(self, fname, data): + """Write a string with tabs expanded as done in this Python file + + Args: + fname: Filename to write to + data: Raw string to convert + """ + data = data.replace('\t', '\\t') + with open(fname, 'w') as fd: + fd.write(data) + + def _CheckStrings(self, expected, actual): + """Check that a string matches its expected value + + If the strings do not match, they are written to the /tmp directory in + the same Python format as is used here in the test. This allows for + easy comparison and update of the tests. + + Args: + expected: Expected string + actual: Actual string + """ + if expected != actual: + self._WritePythonString('/tmp/binman.expected', expected) + self._WritePythonString('/tmp/binman.actual', actual) + print 'Failures written to /tmp/binman.{expected,actual}' + self.assertEquals(expected, actual) + def test_name(self): """Test conversion of device tree names to C identifiers""" self.assertEqual('serial_at_0x12', conv_name_to_c('serial@0x12')) @@ -138,7 +166,7 @@ class TestDtoc(unittest.TestCase): dtb_platdata.run_steps(['struct'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(HEADER + ''' + self._CheckStrings(HEADER + ''' struct dtd_sandbox_i2c_test { }; struct dtd_sandbox_pmic_test { @@ -162,7 +190,7 @@ struct dtd_sandbox_spl_test_2 { dtb_platdata.run_steps(['platdata'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(C_HEADER + ''' + self._CheckStrings(C_HEADER + ''' static struct dtd_sandbox_spl_test dtv_spl_test = { \t.bytearray\t\t= {0x6, 0x0, 0x0}, \t.byteval\t\t= 0x5, @@ -240,7 +268,7 @@ U_BOOT_DEVICE(pmic_at_9) = { dtb_platdata.run_steps(['struct'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(HEADER + ''' + self._CheckStrings(HEADER + ''' struct dtd_source { \tstruct phandle_2_arg clocks[4]; }; @@ -252,7 +280,7 @@ struct dtd_target { dtb_platdata.run_steps(['platdata'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(C_HEADER + ''' + self._CheckStrings(C_HEADER + ''' static struct dtd_target dtv_phandle_target = { \t.intval\t\t\t= 0x0, }; @@ -302,7 +330,7 @@ U_BOOT_DEVICE(phandle_source) = { dtb_platdata.run_steps(['struct'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(HEADER + ''' + self._CheckStrings(HEADER + ''' struct dtd_compat1 { \tfdt32_t\t\tintval; }; @@ -313,7 +341,7 @@ struct dtd_compat1 { dtb_platdata.run_steps(['platdata'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(C_HEADER + ''' + self._CheckStrings(C_HEADER + ''' static struct dtd_compat1 dtv_spl_test = { \t.intval\t\t\t= 0x1, }; @@ -332,7 +360,7 @@ U_BOOT_DEVICE(spl_test) = { dtb_platdata.run_steps(['struct'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(HEADER + ''' + self._CheckStrings(HEADER + ''' struct dtd_test1 { \tfdt64_t\t\treg[2]; }; @@ -347,7 +375,7 @@ struct dtd_test3 { dtb_platdata.run_steps(['platdata'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(C_HEADER + ''' + self._CheckStrings(C_HEADER + ''' static struct dtd_test1 dtv_test1 = { \t.reg\t\t\t= {0x1234, 0x5678}, }; @@ -384,7 +412,7 @@ U_BOOT_DEVICE(test3) = { dtb_platdata.run_steps(['struct'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(HEADER + ''' + self._CheckStrings(HEADER + ''' struct dtd_test1 { \tfdt32_t\t\treg[2]; }; @@ -396,7 +424,7 @@ struct dtd_test2 { dtb_platdata.run_steps(['platdata'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(C_HEADER + ''' + self._CheckStrings(C_HEADER + ''' static struct dtd_test1 dtv_test1 = { \t.reg\t\t\t= {0x1234, 0x5678}, }; @@ -424,7 +452,7 @@ U_BOOT_DEVICE(test2) = { dtb_platdata.run_steps(['struct'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(HEADER + ''' + self._CheckStrings(HEADER + ''' struct dtd_test1 { \tfdt64_t\t\treg[2]; }; @@ -439,7 +467,7 @@ struct dtd_test3 { dtb_platdata.run_steps(['platdata'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(C_HEADER + ''' + self._CheckStrings(C_HEADER + ''' static struct dtd_test1 dtv_test1 = { \t.reg\t\t\t= {0x123400000000, 0x5678}, }; @@ -476,7 +504,7 @@ U_BOOT_DEVICE(test3) = { dtb_platdata.run_steps(['struct'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(HEADER + ''' + self._CheckStrings(HEADER + ''' struct dtd_test1 { \tfdt64_t\t\treg[2]; }; @@ -491,7 +519,7 @@ struct dtd_test3 { dtb_platdata.run_steps(['platdata'], dtb_file, False, output) with open(output) as infile: data = infile.read() - self.assertEqual(C_HEADER + ''' + self._CheckStrings(C_HEADER + ''' static struct dtd_test1 dtv_test1 = { \t.reg\t\t\t= {0x1234, 0x567800000000}, }; -- cgit v1.2.3 From 117f57b75aee2e145d2d23107f600d869b04231a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:26 -0600 Subject: dtoc: Make use of the new pylibfdt methods Now that pylibfdt supports a fuller API we don't need to directly call the libfdt stubs. Update the code to use the Fdt methods instead. Some other cases remain which will be tidied up in a later commit, since they need larger changes. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 5cde8c125ae..18cde2604ff 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -10,6 +10,7 @@ import sys import fdt_util import libfdt +from libfdt import QUIET_NOTFOUND # This deals with a device tree, presenting it as an assortment of Node and # Prop objects, representing nodes and properties, respectively. This file @@ -211,22 +212,22 @@ class Node: This fills in the props and subnodes properties, recursively searching into subnodes so that the entire tree is built. """ + fdt_obj = self._fdt._fdt_obj self.props = self._fdt.GetProps(self) - phandle = self.props.get('phandle') + phandle = fdt_obj.get_phandle(self.Offset()) if phandle: - val = fdt_util.fdt32_to_cpu(phandle.value) - self._fdt.phandle_to_node[val] = self + self._fdt.phandle_to_node[phandle] = self - offset = libfdt.fdt_first_subnode(self._fdt.GetFdt(), self.Offset()) + offset = fdt_obj.first_subnode(self.Offset(), QUIET_NOTFOUND) while offset >= 0: sep = '' if self.path[-1] == '/' else '/' - name = self._fdt._fdt_obj.get_name(offset) + name = fdt_obj.get_name(offset) path = self.path + sep + name node = Node(self._fdt, self, offset, name, path) self.subnodes.append(node) node.Scan() - offset = libfdt.fdt_next_subnode(self._fdt.GetFdt(), offset) + offset = fdt_obj.next_subnode(offset, QUIET_NOTFOUND) def Refresh(self, my_offset): """Fix up the _offset for each node, recursively @@ -324,9 +325,8 @@ class Fdt: When nodes and properties shrink or are deleted, wasted space can build up in the device tree binary. """ - CheckErr(libfdt.fdt_pack(self._fdt), 'pack') - fdt_len = libfdt.fdt_totalsize(self._fdt) - del self._fdt[fdt_len:] + CheckErr(self._fdt_obj.pack(), 'pack') + self.Invalidate() def GetFdt(self): """Get the contents of the FDT @@ -363,13 +363,15 @@ class Fdt: ValueError: if the node does not exist. """ props_dict = {} - poffset = libfdt.fdt_first_property_offset(self._fdt, node._offset) + poffset = self._fdt_obj.first_property_offset(node._offset, + QUIET_NOTFOUND) while poffset >= 0: p = self._fdt_obj.get_property_by_offset(poffset) prop = Prop(node, poffset, p.name, p) props_dict[prop.name] = prop - poffset = libfdt.fdt_next_property_offset(self._fdt, poffset) + poffset = self._fdt_obj.next_property_offset(poffset, + QUIET_NOTFOUND) return props_dict def Invalidate(self): @@ -395,7 +397,7 @@ class Fdt: Returns: Position of @offset within the device tree binary """ - return libfdt.fdt_off_dt_struct(self._fdt) + offset + return self._fdt_obj.off_dt_struct() + offset @classmethod def Node(self, fdt, parent, offset, name, path): -- cgit v1.2.3 From 960662404f492e7088e5a91833dfb0c19ef5450e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:27 -0600 Subject: dtoc: Drop use of a local dtb buffer At present the Fdt class has its own copy of the device tree. This is confusing an unnecessary now that pylibfdt has its own. Drop it and provide access functions to the buffer. This allows us to move the rest of the implementation to use pylibfdt methods instead of directly calling libfdt stubs. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 16 ++++++++-------- tools/dtoc/test_fdt.py | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 18cde2604ff..e24acf12804 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -235,12 +235,13 @@ class Node: Note: This does not take account of property offsets - these will not be updated. """ + fdt_obj = self._fdt._fdt_obj if self._offset != my_offset: self._offset = my_offset - offset = libfdt.fdt_first_subnode(self._fdt.GetFdt(), self._offset) + offset = fdt_obj.first_subnode(self._offset, QUIET_NOTFOUND) for subnode in self.subnodes: subnode.Refresh(offset) - offset = libfdt.fdt_next_subnode(self._fdt.GetFdt(), offset) + offset = fdt_obj.next_subnode(offset, QUIET_NOTFOUND) def DeleteProp(self, prop_name): """Delete a property of a node @@ -252,7 +253,7 @@ class Node: Raises: ValueError if the property does not exist """ - CheckErr(libfdt.fdt_delprop(self._fdt.GetFdt(), self.Offset(), prop_name), + CheckErr(self._fdt._fdt_obj.delprop(self.Offset(), prop_name), "Node '%s': delete property: '%s'" % (self.path, prop_name)) del self.props[prop_name] self._fdt.Invalidate() @@ -272,8 +273,7 @@ class Fdt: self._fname = fdt_util.EnsureCompiled(self._fname) with open(self._fname) as fd: - self._fdt = bytearray(fd.read()) - self._fdt_obj = libfdt.Fdt(self._fdt) + self._fdt_obj = libfdt.Fdt(fd.read()) def Scan(self, root='/'): """Scan a device tree, building up a tree of Node objects @@ -317,7 +317,7 @@ class Fdt: If the device tree has changed in memory, write it back to the file. """ with open(self._fname, 'wb') as fd: - fd.write(self._fdt) + fd.write(self._fdt_obj.as_bytearray()) def Pack(self): """Pack the device tree down to its minimum size @@ -328,13 +328,13 @@ class Fdt: CheckErr(self._fdt_obj.pack(), 'pack') self.Invalidate() - def GetFdt(self): + def GetContents(self): """Get the contents of the FDT Returns: The FDT contents as a string of bytes """ - return self._fdt + return self._fdt_obj.as_bytearray() def GetFdtObj(self): """Get the contents of the FDT diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index ba660ca9b75..daa9d128b5f 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -72,7 +72,7 @@ class TestFdt(unittest.TestCase): def testGetFdt(self): """Tetst that we can access the raw device-tree data""" - self.assertTrue(isinstance(self.dtb.GetFdt(), bytearray)) + self.assertTrue(isinstance(self.dtb.GetContents(), bytearray)) def testGetProps(self): """Tests obtaining a list of properties""" @@ -157,7 +157,7 @@ class TestProp(unittest.TestCase): # Add 12, which is sizeof(struct fdt_property), to get to start of data offset = prop.GetOffset() + 12 - data = self.dtb._fdt[offset:offset + len(prop.value)] + data = self.dtb.GetContents()[offset:offset + len(prop.value)] bytes = [chr(x) for x in data] self.assertEqual(bytes, prop.value) -- cgit v1.2.3 From 2a2d91d0d6fbe4f5c0f336a75ff42dcf7b49a774 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:28 -0600 Subject: dtoc: Update fdt tests to increase code coverage At present only some of the fdt functionality is tested. Add more tests to cover the rest of it. Also turn on test coverage, which is now 100% with a small exclusion for a Python 3 feature. Signed-off-by: Simon Glass --- tools/dtoc/dtoc_test_simple.dts | 1 + tools/dtoc/fdt.py | 12 ---- tools/dtoc/fdt_util.py | 25 +++++--- tools/dtoc/test_dtoc.py | 2 + tools/dtoc/test_fdt.py | 134 +++++++++++++++++++++++++++++++++++++++- 5 files changed, 150 insertions(+), 24 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/dtoc_test_simple.dts b/tools/dtoc/dtoc_test_simple.dts index 895cc1fea28..165680bd4bf 100644 --- a/tools/dtoc/dtoc_test_simple.dts +++ b/tools/dtoc/dtoc_test_simple.dts @@ -21,6 +21,7 @@ longbytearray = [09 0a 0b 0c 0d 0e 0f 10 11]; stringval = "message"; stringarray = "multi-word", "message"; + notstring = [20 21 22 10 00]; }; spl-test2 { diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index e24acf12804..fd016bb8ce1 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -49,13 +49,6 @@ class Prop: return self.type, self.value = self.BytesToValue(bytes) - def GetPhandle(self): - """Get a (single) phandle value from a property - - Gets the phandle valuie from a property and returns it as an integer - """ - return fdt_util.fdt32_to_cpu(self.value[:4]) - def Widen(self, newprop): """Figure out which property type is more general @@ -344,11 +337,6 @@ class Fdt: """ return self._fdt_obj - def CheckErr(self, errnum, msg): - if errnum: - raise ValueError('Error %d: %s: %s' % - (errnum, libfdt.fdt_strerror(errnum), msg)) - def GetProps(self, node): """Get all properties from a node. diff --git a/tools/dtoc/fdt_util.py b/tools/dtoc/fdt_util.py index 2d09649f720..88fc3183833 100644 --- a/tools/dtoc/fdt_util.py +++ b/tools/dtoc/fdt_util.py @@ -13,6 +13,14 @@ import tempfile import command import tools +VERSION3 = sys.version_info > (3, 0) + +def get_plain_bytes(val): + """Handle Python 3 strings""" + if isinstance(val, bytes): + val = val.decode('utf-8') + return val.encode('raw_unicode_escape') + def fdt32_to_cpu(val): """Convert a device tree cell to an integer @@ -22,10 +30,9 @@ def fdt32_to_cpu(val): Return: A native-endian integer value """ - if sys.version_info > (3, 0): - if isinstance(val, bytes): - val = val.decode('utf-8') - val = val.encode('raw_unicode_escape') + if VERSION3: + # This code is not reached in Python 2 + val = get_plain_bytes(val) # pragma: no cover return struct.unpack('>I', val)[0] def fdt_cells_to_cpu(val, cells): @@ -86,10 +93,10 @@ def GetInt(node, propname, default=None): prop = node.props.get(propname) if not prop: return default - value = fdt32_to_cpu(prop.value) - if type(value) == type(list): - raise ValueError("Node '%s' property '%' has list value: expecting" + if isinstance(prop.value, list): + raise ValueError("Node '%s' property '%s' has list value: expecting " "a single integer" % (node.name, propname)) + value = fdt32_to_cpu(prop.value) return value def GetString(node, propname, default=None): @@ -97,8 +104,8 @@ def GetString(node, propname, default=None): if not prop: return default value = prop.value - if type(value) == type(list): - raise ValueError("Node '%s' property '%' has list value: expecting" + if isinstance(value, list): + raise ValueError("Node '%s' property '%s' has list value: expecting " "a single string" % (node.name, propname)) return value diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index ce6d2585a40..20fea522c4f 100644 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -180,6 +180,7 @@ struct dtd_sandbox_spl_test { \tfdt32_t\t\tintarray[4]; \tfdt32_t\t\tintval; \tunsigned char\tlongbytearray[9]; +\tunsigned char\tnotstring[5]; \tconst char *\tstringarray[3]; \tconst char *\tstringval; }; @@ -195,6 +196,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test = { \t.bytearray\t\t= {0x6, 0x0, 0x0}, \t.byteval\t\t= 0x5, \t.intval\t\t\t= 0x1, +\t.notstring\t\t= {0x20, 0x21, 0x22, 0x10, 0x0}, \t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10, \t\t0x11}, \t.stringval\t\t= "message", diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index daa9d128b5f..d44e4dd8423 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -18,6 +18,7 @@ for dirname in ['../patman', '..']: import command import fdt from fdt import TYPE_BYTE, TYPE_INT, TYPE_STRING, TYPE_BOOL +import fdt_util from fdt_util import fdt32_to_cpu import libfdt import test_util @@ -53,6 +54,7 @@ class TestFdt(unittest.TestCase): node = self.dtb.GetNode('/i2c@0/pmic@9') self.assertTrue(isinstance(node, fdt.Node)) self.assertEqual('pmic@9', node.name) + self.assertIsNone(self.dtb.GetNode('/i2c@0/pmic@9/missing')) def testFlush(self): """Check that we can flush the device tree out to its file""" @@ -79,14 +81,14 @@ class TestFdt(unittest.TestCase): node = self.dtb.GetNode('/spl-test') props = self.dtb.GetProps(node) self.assertEqual(['boolval', 'bytearray', 'byteval', 'compatible', - 'intarray', 'intval', 'longbytearray', + 'intarray', 'intval', 'longbytearray', 'notstring', 'stringarray', 'stringval', 'u-boot,dm-pre-reloc'], sorted(props.keys())) def testCheckError(self): """Tests the ChecKError() function""" with self.assertRaises(ValueError) as e: - self.dtb.CheckErr(-libfdt.NOTFOUND, 'hello') + fdt.CheckErr(-libfdt.NOTFOUND, 'hello') self.assertIn('FDT_ERR_NOTFOUND: hello', str(e.exception)) @@ -119,6 +121,8 @@ class TestNode(unittest.TestCase): self.node.DeleteProp('intarray') offset3 = node2.Offset() self.assertTrue(offset3 < offset2) + with self.assertRaises(libfdt.FdtException): + self.node.DeleteProp('missing') def testFindNode(self): """Tests that we can find a node using the _FindNode() functoin""" @@ -126,6 +130,7 @@ class TestNode(unittest.TestCase): self.assertEqual('i2c@0', node.name) subnode = node._FindNode('pmic@9') self.assertEqual('pmic@9', subnode.name) + self.assertEqual(None, node._FindNode('missing')) class TestProp(unittest.TestCase): @@ -144,6 +149,58 @@ class TestProp(unittest.TestCase): self.node = self.dtb.GetNode('/spl-test') self.fdt = self.dtb.GetFdtObj() + def testPhandle(self): + dtb = fdt.FdtScan('tools/dtoc/dtoc_test_phandle.dts') + node = dtb.GetNode('/phandle-source') + + def _ConvertProp(self, prop_name): + """Helper function to look up a property in self.node and return it + + Args: + Property name to find + + Return fdt.Prop object for this property + """ + p = self.fdt.get_property(self.node.Offset(), prop_name) + return fdt.Prop(self.node, -1, prop_name, p) + + def testMakeProp(self): + """Test we can convert all the the types that are supported""" + prop = self._ConvertProp('boolval') + self.assertEqual(fdt.TYPE_BOOL, prop.type) + self.assertEqual(True, prop.value) + + prop = self._ConvertProp('intval') + self.assertEqual(fdt.TYPE_INT, prop.type) + self.assertEqual(1, fdt32_to_cpu(prop.value)) + + prop = self._ConvertProp('intarray') + self.assertEqual(fdt.TYPE_INT, prop.type) + val = [fdt32_to_cpu(val) for val in prop.value] + self.assertEqual([2, 3, 4], val) + + prop = self._ConvertProp('byteval') + self.assertEqual(fdt.TYPE_BYTE, prop.type) + self.assertEqual(5, ord(prop.value)) + + prop = self._ConvertProp('longbytearray') + self.assertEqual(fdt.TYPE_BYTE, prop.type) + val = [ord(val) for val in prop.value] + self.assertEqual([9, 10, 11, 12, 13, 14, 15, 16, 17], val) + + prop = self._ConvertProp('stringval') + self.assertEqual(fdt.TYPE_STRING, prop.type) + self.assertEqual('message', prop.value) + + prop = self._ConvertProp('stringarray') + self.assertEqual(fdt.TYPE_STRING, prop.type) + self.assertEqual(['multi-word', 'message'], prop.value) + + prop = self._ConvertProp('notstring') + self.assertEqual(fdt.TYPE_BYTE, prop.type) + val = [ord(val) for val in prop.value] + self.assertEqual([0x20, 0x21, 0x22, 0x10, 0], val) + def testGetEmpty(self): """Tests the GetEmpty() function for the various supported types""" self.assertEqual(True, fdt.Prop.GetEmpty(fdt.TYPE_BOOL)) @@ -207,6 +264,71 @@ class TestProp(unittest.TestCase): self.assertEqual(3, len(prop.value)) +class TestFdtUtil(unittest.TestCase): + """Tests for the fdt_util module + + This module will likely be mostly replaced at some point, once upstream + libfdt has better Python support. For now, this provides tests for current + functionality. + """ + @classmethod + def setUpClass(cls): + tools.PrepareOutputDir(None) + + def setUp(self): + self.dtb = fdt.FdtScan('tools/dtoc/dtoc_test_simple.dts') + self.node = self.dtb.GetNode('/spl-test') + + def testGetInt(self): + self.assertEqual(1, fdt_util.GetInt(self.node, 'intval')) + self.assertEqual(3, fdt_util.GetInt(self.node, 'missing', 3)) + + with self.assertRaises(ValueError) as e: + self.assertEqual(3, fdt_util.GetInt(self.node, 'intarray')) + self.assertIn("property 'intarray' has list value: expecting a single " + 'integer', str(e.exception)) + + def testGetString(self): + self.assertEqual('message', fdt_util.GetString(self.node, 'stringval')) + self.assertEqual('test', fdt_util.GetString(self.node, 'missing', + 'test')) + + with self.assertRaises(ValueError) as e: + self.assertEqual(3, fdt_util.GetString(self.node, 'stringarray')) + self.assertIn("property 'stringarray' has list value: expecting a " + 'single string', str(e.exception)) + + def testGetBool(self): + self.assertEqual(True, fdt_util.GetBool(self.node, 'boolval')) + self.assertEqual(False, fdt_util.GetBool(self.node, 'missing')) + self.assertEqual(True, fdt_util.GetBool(self.node, 'missing', True)) + self.assertEqual(False, fdt_util.GetBool(self.node, 'missing', False)) + + def testFdtCellsToCpu(self): + val = self.node.props['intarray'].value + self.assertEqual(0, fdt_util.fdt_cells_to_cpu(val, 0)) + self.assertEqual(2, fdt_util.fdt_cells_to_cpu(val, 1)) + + dtb2 = fdt.FdtScan('tools/dtoc/dtoc_test_addr64.dts') + node2 = dtb2.GetNode('/test1') + val = node2.props['reg'].value + self.assertEqual(0x1234, fdt_util.fdt_cells_to_cpu(val, 2)) + + def testEnsureCompiled(self): + """Test a degenerate case of this function""" + dtb = fdt_util.EnsureCompiled('tools/dtoc/dtoc_test_simple.dts') + self.assertEqual(dtb, fdt_util.EnsureCompiled(dtb)) + + def testGetPlainBytes(self): + self.assertEqual('fred', fdt_util.get_plain_bytes('fred')) + + +def RunTestCoverage(): + """Run the tests and check that we get 100% coverage""" + test_util.RunTestCoverage('tools/dtoc/test_fdt.py', None, + ['tools/patman/*.py', '*test_fdt.py'], options.build_dir) + + def RunTests(args): """Run all the test we have for the fdt model @@ -217,7 +339,7 @@ def RunTests(args): result = unittest.TestResult() sys.argv = [sys.argv[0]] test_name = args and args[0] or None - for module in (TestFdt, TestNode, TestProp): + for module in (TestFdt, TestNode, TestProp, TestFdtUtil): if test_name: try: suite = unittest.TestLoader().loadTestsFromName(test_name, module) @@ -237,10 +359,16 @@ if __name__ != '__main__': sys.exit(1) parser = OptionParser() +parser.add_option('-B', '--build-dir', type='string', default='b', + help='Directory containing the build output') parser.add_option('-t', '--test', action='store_true', dest='test', default=False, help='run tests') +parser.add_option('-T', '--test-coverage', action='store_true', + default=False, help='run tests and check for 100% coverage') (options, args) = parser.parse_args() # Run our meagre tests if options.test: RunTests(args) +elif options.test_coverage: + RunTestCoverage() -- cgit v1.2.3 From f9b88b3a5d7ff18760c2c5a0cb596ec8c577706e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:29 -0600 Subject: dtoc: Keep track of property offsets At present the Fdt class does not keep track of property offsets if they change due to removal of properties. Update the code to handle this, and add a test. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 20 ++++++++++++++++ tools/dtoc/test_fdt.py | 65 ++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 78 insertions(+), 7 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index fd016bb8ce1..274c142e7ae 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -49,6 +49,9 @@ class Prop: return self.type, self.value = self.BytesToValue(bytes) + def RefreshOffset(self, poffset): + self._offset = poffset + def Widen(self, newprop): """Figure out which property type is more general @@ -154,6 +157,7 @@ class Prop: Returns: The offset of the property (struct fdt_property) within the file """ + self._node._fdt.CheckCache() return self._node._fdt.GetStructOffset(self._offset) class Node: @@ -233,8 +237,23 @@ class Node: self._offset = my_offset offset = fdt_obj.first_subnode(self._offset, QUIET_NOTFOUND) for subnode in self.subnodes: + if subnode.name != fdt_obj.get_name(offset): + raise ValueError('Internal error, node name mismatch %s != %s' % + (subnode.name, fdt_obj.get_name(offset))) subnode.Refresh(offset) offset = fdt_obj.next_subnode(offset, QUIET_NOTFOUND) + if offset != -libfdt.FDT_ERR_NOTFOUND: + raise ValueError('Internal error, offset == %d' % offset) + + poffset = fdt_obj.first_property_offset(self._offset, QUIET_NOTFOUND) + while poffset >= 0: + p = fdt_obj.get_property_by_offset(poffset) + prop = self.props.get(p.name) + if not prop: + raise ValueError("Internal error, property '%s' missing, " + 'offset %d' % (p.name, poffset)) + prop.RefreshOffset(poffset) + poffset = fdt_obj.next_property_offset(poffset, QUIET_NOTFOUND) def DeleteProp(self, prop_name): """Delete a property of a node @@ -278,6 +297,7 @@ class Fdt: TODO(sjg@chromium.org): Implement the 'root' parameter """ + self._cached_offsets = True self._root = self.Node(self, None, 0, '/', '/') self._root.Scan() diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index d44e4dd8423..e57298dbe7c 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -24,6 +24,30 @@ import libfdt import test_util import tools +def _GetPropertyValue(dtb, node, prop_name): + """Low-level function to get the property value based on its offset + + This looks directly in the device tree at the property's offset to find + its value. It is useful as a check that the property is in the correct + place. + + Args: + node: Node to look in + prop_name: Property name to find + + Returns: + Tuple: + Prop object found + Value of property as a string (found using property offset) + """ + prop = node.props[prop_name] + + # Add 12, which is sizeof(struct fdt_property), to get to start of data + offset = prop.GetOffset() + 12 + data = dtb.GetContents()[offset:offset + len(prop.value)] + return prop, [chr(x) for x in data] + + class TestFdt(unittest.TestCase): """Tests for the Fdt module @@ -124,6 +148,12 @@ class TestNode(unittest.TestCase): with self.assertRaises(libfdt.FdtException): self.node.DeleteProp('missing') + def testDeleteGetOffset(self): + """Test that property offset update when properties are deleted""" + self.node.DeleteProp('intval') + prop, value = _GetPropertyValue(self.dtb, self.node, 'longbytearray') + self.assertEqual(prop.value, value) + def testFindNode(self): """Tests that we can find a node using the _FindNode() functoin""" node = self.dtb.GetRoot()._FindNode('i2c@0') @@ -132,6 +162,32 @@ class TestNode(unittest.TestCase): self.assertEqual('pmic@9', subnode.name) self.assertEqual(None, node._FindNode('missing')) + def testRefreshMissingNode(self): + """Test refreshing offsets when an extra node is present in dtb""" + # Delete it from our tables, not the device tree + del self.dtb._root.subnodes[-1] + with self.assertRaises(ValueError) as e: + self.dtb.Refresh() + self.assertIn('Internal error, offset', str(e.exception)) + + def testRefreshExtraNode(self): + """Test refreshing offsets when an expected node is missing""" + # Delete it from the device tre, not our tables + self.dtb.GetFdtObj().del_node(self.node.Offset()) + with self.assertRaises(ValueError) as e: + self.dtb.Refresh() + self.assertIn('Internal error, node name mismatch ' + 'spl-test != spl-test2', str(e.exception)) + + def testRefreshMissingProp(self): + """Test refreshing offsets when an extra property is present in dtb""" + # Delete it from our tables, not the device tree + del self.node.props['notstring'] + with self.assertRaises(ValueError) as e: + self.dtb.Refresh() + self.assertIn("Internal error, property 'notstring' missing, offset ", + str(e.exception)) + class TestProp(unittest.TestCase): """Test operation of the Prop class""" @@ -210,13 +266,8 @@ class TestProp(unittest.TestCase): def testGetOffset(self): """Test we can get the offset of a property""" - prop = self.node.props['longbytearray'] - - # Add 12, which is sizeof(struct fdt_property), to get to start of data - offset = prop.GetOffset() + 12 - data = self.dtb.GetContents()[offset:offset + len(prop.value)] - bytes = [chr(x) for x in data] - self.assertEqual(bytes, prop.value) + prop, value = _GetPropertyValue(self.dtb, self.node, 'longbytearray') + self.assertEqual(prop.value, value) def testWiden(self): """Test widening of values""" -- cgit v1.2.3 From b9066ffc136afd2e46e8d033c4edce98f5557afc Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:30 -0600 Subject: dtoc: Fix Fdt.GetNode() to handle a missing node At present the algortihm is not correct since it will return the root node if the requested node is not found and there are no slashes in the requested node name. Fix this and add a test. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 5 ++++- tools/dtoc/test_fdt.py | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index 274c142e7ae..c1d04d48e88 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -318,7 +318,10 @@ class Fdt: Node object, or None if not found """ node = self._root - for part in path.split('/')[1:]: + parts = path.split('/') + if len(parts) < 2: + return None + for part in parts[1:]: node = node._FindNode(part) if not node: return None diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index e57298dbe7c..9fef8ed5496 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -205,6 +205,9 @@ class TestProp(unittest.TestCase): self.node = self.dtb.GetNode('/spl-test') self.fdt = self.dtb.GetFdtObj() + def testMissingNode(self): + self.assertEqual(None, self.dtb.GetNode('missing')) + def testPhandle(self): dtb = fdt.FdtScan('tools/dtoc/dtoc_test_phandle.dts') node = dtb.GetNode('/phandle-source') -- cgit v1.2.3 From 760b7170c5e44c771397eaf57b7197b621373809 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:31 -0600 Subject: dtoc: Fix properties with a single zero-arg phandle At present a property with a single phandle looks like an integer value to dtoc. Correct this by adjusting it in the phandle-processing code. Add a test for this. Signed-off-by: Simon Glass --- tools/dtoc/dtb_platdata.py | 12 ++++++++---- tools/dtoc/dtoc_test_phandle.dts | 6 ++++++ tools/dtoc/test_dtoc.py | 10 ++++++++++ tools/dtoc/test_fdt.py | 4 +++- 4 files changed, 27 insertions(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 2f7302e5295..b1323aef198 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -211,15 +211,21 @@ class DtbPlatdata(object): Number of argument cells is this is a phandle, else None """ if prop.name in ['clocks']: + if not isinstance(prop.value, list): + prop.value = [prop.value] val = prop.value - if not isinstance(val, list): - val = [val] i = 0 max_args = 0 args = [] while i < len(val): phandle = fdt_util.fdt32_to_cpu(val[i]) + # If we get to the end of the list, stop. This can happen + # since some nodes have more phandles in the list than others, + # but we allocate enough space for the largest list. So those + # nodes with shorter lists end up with zeroes at the end. + if not phandle: + break target = self._fdt.phandle_to_node.get(phandle) if not target: raise ValueError("Cannot parse '%s' in node '%s'" % @@ -400,8 +406,6 @@ class DtbPlatdata(object): continue info = self.get_phandle_argc(prop, node.name) if info: - if not isinstance(prop.value, list): - prop.value = [prop.value] # Process the list as pairs of (phandle, id) pos = 0 for args in info.args: diff --git a/tools/dtoc/dtoc_test_phandle.dts b/tools/dtoc/dtoc_test_phandle.dts index 91dfec5c63f..a71acffc698 100644 --- a/tools/dtoc/dtoc_test_phandle.dts +++ b/tools/dtoc/dtoc_test_phandle.dts @@ -33,4 +33,10 @@ compatible = "source"; clocks = <&phandle &phandle_1 11 &phandle_2 12 13 &phandle>; }; + + phandle-source2 { + u-boot,dm-pre-reloc; + compatible = "source"; + clocks = <&phandle>; + }; }; diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 20fea522c4f..11cac3fc7af 100644 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -323,6 +323,16 @@ U_BOOT_DEVICE(phandle_source) = { \t.platdata_size\t= sizeof(dtv_phandle_source), }; +static struct dtd_source dtv_phandle_source2 = { +\t.clocks\t\t\t= { +\t\t\t{&dtv_phandle_target, {}},}, +}; +U_BOOT_DEVICE(phandle_source2) = { +\t.name\t\t= "source", +\t.platdata\t= &dtv_phandle_source2, +\t.platdata_size\t= sizeof(dtv_phandle_source2), +}; + ''', data) def test_aliases(self): diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 9fef8ed5496..49d188b1c12 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -210,7 +210,9 @@ class TestProp(unittest.TestCase): def testPhandle(self): dtb = fdt.FdtScan('tools/dtoc/dtoc_test_phandle.dts') - node = dtb.GetNode('/phandle-source') + node = dtb.GetNode('/phandle-source2') + prop = node.props['clocks'] + self.assertTrue(fdt32_to_cpu(prop.value) > 0) def _ConvertProp(self, prop_name): """Helper function to look up a property in self.node and return it -- cgit v1.2.3 From dfe5f5b97d369b0fcf24c792b0fc83661aa24afa Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:32 -0600 Subject: dtoc: Fix some minor errors Fix some comments and a printf string which is incorrect. Signed-off-by: Simon Glass --- tools/dtoc/dtb_platdata.py | 3 ++- tools/dtoc/dtoc.py | 4 ++-- tools/dtoc/fdt.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index b1323aef198..6cb12594466 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -316,7 +316,8 @@ class DtbPlatdata(object): total = na + ns if reg.type != fdt.TYPE_INT: - raise ValueError("Node '%s' reg property is not an int") + raise ValueError("Node '%s' reg property is not an int" % + node.name) if len(reg.value) % total: raise ValueError("Node '%s' reg property has %d cells " 'which is not a multiple of na + ns = %d + %d)' % diff --git a/tools/dtoc/dtoc.py b/tools/dtoc/dtoc.py index c891b063800..2e6a4db8bc1 100755 --- a/tools/dtoc/dtoc.py +++ b/tools/dtoc/dtoc.py @@ -40,8 +40,8 @@ def run_tests(args): """Run all the test we have for dtoc Args: - args: List of positional args provided to binman. This can hold a test - name to execute (as in 'binman -t testSections', for example) + args: List of positional args provided to dtoc. This can hold a test + name to execute (as in 'dtoc -t test_empty_file', for example) """ import test_dtoc diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index c1d04d48e88..e7703c1c753 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -428,7 +428,7 @@ class Fdt: return node def FdtScan(fname): - """Returns a new Fdt object from the implementation we are using""" + """Returns a new Fdt object""" dtb = Fdt(fname) dtb.Scan() return dtb -- cgit v1.2.3 From ba765217ed0258fa0ecd5cb77c6c4c171624c4ad Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:33 -0600 Subject: dtoc: Add a test for code coverage Add a -T option to run a code-coverage test on dtoc. At present this is about 96%. Future work will increase it to 100%. Signed-off-by: Simon Glass --- tools/dtoc/dtoc.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'tools') diff --git a/tools/dtoc/dtoc.py b/tools/dtoc/dtoc.py index 2e6a4db8bc1..827094e72ab 100755 --- a/tools/dtoc/dtoc.py +++ b/tools/dtoc/dtoc.py @@ -35,6 +35,7 @@ our_path = os.path.dirname(os.path.realpath(__file__)) sys.path.append(os.path.join(our_path, '../patman')) import dtb_platdata +import test_util def run_tests(args): """Run all the test we have for dtoc @@ -64,10 +65,19 @@ def run_tests(args): for _, err in result.failures: print err +def RunTestCoverage(): + """Run the tests and check that we get 100% coverage""" + sys.argv = [sys.argv[0]] + test_util.RunTestCoverage('tools/dtoc/dtoc.py', '/dtoc.py', + ['tools/patman/*.py', '*/fdt*', '*test*'], options.build_dir) + + if __name__ != '__main__': sys.exit(1) parser = OptionParser() +parser.add_option('-B', '--build-dir', type='string', default='b', + help='Directory containing the build output') parser.add_option('-d', '--dtb-file', action='store', help='Specify the .dtb input file') parser.add_option('--include-disabled', action='store_true', @@ -76,12 +86,17 @@ parser.add_option('-o', '--output', action='store', default='-', help='Select output filename') parser.add_option('-t', '--test', action='store_true', dest='test', default=False, help='run tests') +parser.add_option('-T', '--test-coverage', action='store_true', + default=False, help='run tests and check for 100% coverage') (options, args) = parser.parse_args() # Run our meagre tests if options.test: run_tests(args) +elif options.test_coverage: + RunTestCoverage() + else: dtb_platdata.run_steps(args, options.dtb_file, options.include_disabled, options.output) -- cgit v1.2.3 From c3f9454103462e2cfcc5a9120b893f4ea250650a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:34 -0600 Subject: binman: Move capture_sys_output() to test_util This function is useful in various tests. Move it into the common test utility module. Signed-off-by: Simon Glass --- tools/binman/elf_test.py | 22 ++-------------------- tools/binman/image_test.py | 2 +- tools/patman/test_util.py | 21 +++++++++++++++++++++ 3 files changed, 24 insertions(+), 21 deletions(-) (limited to 'tools') diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index fb6e451cf0e..9c8f1feca85 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -4,33 +4,15 @@ # # Test for the elf module -from contextlib import contextmanager import os import sys import unittest -try: - from StringIO import StringIO -except ImportError: - from io import StringIO - import elf +import test_util binman_dir = os.path.dirname(os.path.realpath(sys.argv[0])) -# Use this to suppress stdout/stderr output: -# with capture_sys_output() as (stdout, stderr) -# ...do something... -@contextmanager -def capture_sys_output(): - capture_out, capture_err = StringIO(), StringIO() - old_out, old_err = sys.stdout, sys.stderr - try: - sys.stdout, sys.stderr = capture_out, capture_err - yield capture_out, capture_err - finally: - sys.stdout, sys.stderr = old_out, old_err - class FakeEntry: def __init__(self, contents_size): @@ -110,7 +92,7 @@ class TestElf(unittest.TestCase): entry = FakeEntry(20) section = FakeSection() elf_fname = os.path.join(binman_dir, 'test', 'u_boot_binman_syms') - with capture_sys_output() as (stdout, stderr): + with test_util.capture_sys_output() as (stdout, stderr): syms = elf.LookupAndWriteSymbols(elf_fname, entry, section) elf.debug = False self.assertTrue(len(stdout.getvalue()) > 0) diff --git a/tools/binman/image_test.py b/tools/binman/image_test.py index 45dd2378c8a..3775e1afb07 100644 --- a/tools/binman/image_test.py +++ b/tools/binman/image_test.py @@ -7,7 +7,7 @@ import unittest from image import Image -from elf_test import capture_sys_output +from test_util import capture_sys_output class TestImage(unittest.TestCase): def testInvalidFormat(self): diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index 1a33c997c4c..0e79af871ab 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -3,12 +3,19 @@ # Copyright (c) 2016 Google, Inc # +from contextlib import contextmanager import glob import os import sys import command +try: + from StringIO import StringIO +except ImportError: + from io import StringIO + + def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): """Run tests and check that we get 100% coverage @@ -62,3 +69,17 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): ok = False if not ok: raise ValueError('Test coverage failure') + + +# Use this to suppress stdout/stderr output: +# with capture_sys_output() as (stdout, stderr) +# ...do something... +@contextmanager +def capture_sys_output(): + capture_out, capture_err = StringIO(), StringIO() + old_out, old_err = sys.stdout, sys.stderr + try: + sys.stdout, sys.stderr = capture_out, capture_err + yield capture_out, capture_err + finally: + sys.stdout, sys.stderr = old_out, old_err -- cgit v1.2.3 From 8512ea2e359cf5217183fc87326a015caab1994d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:35 -0600 Subject: dtoc: Increase code coverage to 100% Add more tests to increase dtoc code coverage to 100%. Correct a whitespace error in some test .dts files at the same time. Signed-off-by: Simon Glass --- tools/dtoc/dtoc_test_add_prop.dts | 24 ++++++ tools/dtoc/dtoc_test_addr32_64.dts | 2 +- tools/dtoc/dtoc_test_addr64_32.dts | 2 +- tools/dtoc/dtoc_test_bad_reg.dts | 17 ++++ tools/dtoc/dtoc_test_bad_reg2.dts | 17 ++++ tools/dtoc/dtoc_test_phandle_bad.dts | 16 ++++ tools/dtoc/dtoc_test_phandle_bad2.dts | 22 +++++ tools/dtoc/dtoc_test_phandle_reorder.dts | 23 +++++ tools/dtoc/dtoc_test_phandle_single.dts | 23 +++++ tools/dtoc/test_dtoc.py | 141 +++++++++++++++++++++++++++++++ 10 files changed, 285 insertions(+), 2 deletions(-) create mode 100644 tools/dtoc/dtoc_test_add_prop.dts create mode 100644 tools/dtoc/dtoc_test_bad_reg.dts create mode 100644 tools/dtoc/dtoc_test_bad_reg2.dts create mode 100644 tools/dtoc/dtoc_test_phandle_bad.dts create mode 100644 tools/dtoc/dtoc_test_phandle_bad2.dts create mode 100644 tools/dtoc/dtoc_test_phandle_reorder.dts create mode 100644 tools/dtoc/dtoc_test_phandle_single.dts (limited to 'tools') diff --git a/tools/dtoc/dtoc_test_add_prop.dts b/tools/dtoc/dtoc_test_add_prop.dts new file mode 100644 index 00000000000..fa296e55527 --- /dev/null +++ b/tools/dtoc/dtoc_test_add_prop.dts @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2018 Google, Inc + */ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + spl-test { + u-boot,dm-pre-reloc; + compatible = "sandbox,spl-test"; + intval = <1>; + }; + + spl-test2 { + u-boot,dm-pre-reloc; + compatible = "sandbox,spl-test"; + intarray = <5>; + }; +}; diff --git a/tools/dtoc/dtoc_test_addr32_64.dts b/tools/dtoc/dtoc_test_addr32_64.dts index 7891ee59fa2..7599d5b0a59 100644 --- a/tools/dtoc/dtoc_test_addr32_64.dts +++ b/tools/dtoc/dtoc_test_addr32_64.dts @@ -5,7 +5,7 @@ * Copyright 2017 Google, Inc */ - /dts-v1/; +/dts-v1/; / { #address-cells = <1>; diff --git a/tools/dtoc/dtoc_test_addr64_32.dts b/tools/dtoc/dtoc_test_addr64_32.dts index 759a7e8e265..85e4f5fdaeb 100644 --- a/tools/dtoc/dtoc_test_addr64_32.dts +++ b/tools/dtoc/dtoc_test_addr64_32.dts @@ -5,7 +5,7 @@ * Copyright 2017 Google, Inc */ - /dts-v1/; +/dts-v1/; / { #address-cells = <2>; diff --git a/tools/dtoc/dtoc_test_bad_reg.dts b/tools/dtoc/dtoc_test_bad_reg.dts new file mode 100644 index 00000000000..1312acb619b --- /dev/null +++ b/tools/dtoc/dtoc_test_bad_reg.dts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2018 Google, Inc + */ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + spl-test { + compatible = "test"; + reg = "fre"; + }; +}; diff --git a/tools/dtoc/dtoc_test_bad_reg2.dts b/tools/dtoc/dtoc_test_bad_reg2.dts new file mode 100644 index 00000000000..3e9efa43af1 --- /dev/null +++ b/tools/dtoc/dtoc_test_bad_reg2.dts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2018 Google, Inc + */ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + spl-test { + compatible = "test"; + reg = <1 2 3>; + }; +}; diff --git a/tools/dtoc/dtoc_test_phandle_bad.dts b/tools/dtoc/dtoc_test_phandle_bad.dts new file mode 100644 index 00000000000..a3ddc595851 --- /dev/null +++ b/tools/dtoc/dtoc_test_phandle_bad.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2018 Google, Inc + */ + +/dts-v1/; + +/ { + phandle-source { + u-boot,dm-pre-reloc; + compatible = "source"; + clocks = <20>; /* Invalid phandle */ + }; +}; diff --git a/tools/dtoc/dtoc_test_phandle_bad2.dts b/tools/dtoc/dtoc_test_phandle_bad2.dts new file mode 100644 index 00000000000..fe25f565fbb --- /dev/null +++ b/tools/dtoc/dtoc_test_phandle_bad2.dts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2018 Google, Inc + */ + +/dts-v1/; + +/ { + phandle: phandle-target { + u-boot,dm-pre-reloc; + compatible = "target"; + intval = <0>; + }; + + phandle-source2 { + u-boot,dm-pre-reloc; + compatible = "source"; + clocks = <&phandle>; + }; +}; diff --git a/tools/dtoc/dtoc_test_phandle_reorder.dts b/tools/dtoc/dtoc_test_phandle_reorder.dts new file mode 100644 index 00000000000..aa71d56f27c --- /dev/null +++ b/tools/dtoc/dtoc_test_phandle_reorder.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2018 Google, Inc + */ + +/dts-v1/; + +/ { + + phandle-source2 { + u-boot,dm-pre-reloc; + compatible = "source"; + clocks = <&phandle>; + }; + + phandle: phandle-target { + u-boot,dm-pre-reloc; + compatible = "target"; + #clock-cells = <0>; + }; +}; diff --git a/tools/dtoc/dtoc_test_phandle_single.dts b/tools/dtoc/dtoc_test_phandle_single.dts new file mode 100644 index 00000000000..aacd0b15fa1 --- /dev/null +++ b/tools/dtoc/dtoc_test_phandle_single.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Test device tree file for dtoc + * + * Copyright 2018 Google, Inc + */ + +/dts-v1/; + +/ { + phandle: phandle-target { + u-boot,dm-pre-reloc; + compatible = "target"; + intval = <0>; + #clock-cells = <0>; + }; + + phandle-source2 { + u-boot,dm-pre-reloc; + compatible = "source"; + clocks = <&phandle>; + }; +}; diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 11cac3fc7af..622b39df395 100644 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -20,6 +20,7 @@ from dtb_platdata import get_value from dtb_platdata import tab_to import fdt import fdt_util +import test_util import tools our_path = os.path.dirname(os.path.realpath(__file__)) @@ -335,6 +336,68 @@ U_BOOT_DEVICE(phandle_source2) = { ''', data) + def test_phandle_single(self): + """Test output from a node containing a phandle reference""" + dtb_file = get_dtb_file('dtoc_test_phandle_single.dts') + output = tools.GetOutputFilename('output') + dtb_platdata.run_steps(['struct'], dtb_file, False, output) + with open(output) as infile: + data = infile.read() + self._CheckStrings(HEADER + ''' +struct dtd_source { +\tstruct phandle_0_arg clocks[1]; +}; +struct dtd_target { +\tfdt32_t\t\tintval; +}; +''', data) + + def test_phandle_reorder(self): + """Test that phandle targets are generated before their references""" + dtb_file = get_dtb_file('dtoc_test_phandle_reorder.dts') + output = tools.GetOutputFilename('output') + dtb_platdata.run_steps(['platdata'], dtb_file, False, output) + with open(output) as infile: + data = infile.read() + self._CheckStrings(C_HEADER + ''' +static struct dtd_target dtv_phandle_target = { +}; +U_BOOT_DEVICE(phandle_target) = { +\t.name\t\t= "target", +\t.platdata\t= &dtv_phandle_target, +\t.platdata_size\t= sizeof(dtv_phandle_target), +}; + +static struct dtd_source dtv_phandle_source2 = { +\t.clocks\t\t\t= { +\t\t\t{&dtv_phandle_target, {}},}, +}; +U_BOOT_DEVICE(phandle_source2) = { +\t.name\t\t= "source", +\t.platdata\t= &dtv_phandle_source2, +\t.platdata_size\t= sizeof(dtv_phandle_source2), +}; + +''', data) + + def test_phandle_bad(self): + """Test a node containing an invalid phandle fails""" + dtb_file = get_dtb_file('dtoc_test_phandle_bad.dts') + output = tools.GetOutputFilename('output') + with self.assertRaises(ValueError) as e: + dtb_platdata.run_steps(['struct'], dtb_file, False, output) + self.assertIn("Cannot parse 'clocks' in node 'phandle-source'", + str(e.exception)) + + def test_phandle_bad2(self): + """Test a phandle target missing its #*-cells property""" + dtb_file = get_dtb_file('dtoc_test_phandle_bad2.dts') + output = tools.GetOutputFilename('output') + with self.assertRaises(ValueError) as e: + dtb_platdata.run_steps(['struct'], dtb_file, False, output) + self.assertIn("Node 'phandle-target' has no '#clock-cells' property", + str(e.exception)) + def test_aliases(self): """Test output from a node with multiple compatible strings""" dtb_file = get_dtb_file('dtoc_test_aliases.dts') @@ -560,3 +623,81 @@ U_BOOT_DEVICE(test3) = { }; ''', data) + + def test_bad_reg(self): + """Test that a reg property with an invalid type generates an error""" + dtb_file = get_dtb_file('dtoc_test_bad_reg.dts') + output = tools.GetOutputFilename('output') + with self.assertRaises(ValueError) as e: + dtb_platdata.run_steps(['struct'], dtb_file, False, output) + self.assertIn("Node 'spl-test' reg property is not an int", + str(e.exception)) + + def test_bad_reg2(self): + """Test that a reg property with an invalid cell count is detected""" + dtb_file = get_dtb_file('dtoc_test_bad_reg2.dts') + output = tools.GetOutputFilename('output') + with self.assertRaises(ValueError) as e: + dtb_platdata.run_steps(['struct'], dtb_file, False, output) + self.assertIn("Node 'spl-test' reg property has 3 cells which is not a multiple of na + ns = 1 + 1)", + str(e.exception)) + + def test_add_prop(self): + """Test that a subequent node can add a new property to a struct""" + dtb_file = get_dtb_file('dtoc_test_add_prop.dts') + output = tools.GetOutputFilename('output') + dtb_platdata.run_steps(['struct'], dtb_file, False, output) + with open(output) as infile: + data = infile.read() + self._CheckStrings(HEADER + ''' +struct dtd_sandbox_spl_test { +\tfdt32_t\t\tintarray; +\tfdt32_t\t\tintval; +}; +''', data) + + dtb_platdata.run_steps(['platdata'], dtb_file, False, output) + with open(output) as infile: + data = infile.read() + self._CheckStrings(C_HEADER + ''' +static struct dtd_sandbox_spl_test dtv_spl_test = { +\t.intval\t\t\t= 0x1, +}; +U_BOOT_DEVICE(spl_test) = { +\t.name\t\t= "sandbox_spl_test", +\t.platdata\t= &dtv_spl_test, +\t.platdata_size\t= sizeof(dtv_spl_test), +}; + +static struct dtd_sandbox_spl_test dtv_spl_test2 = { +\t.intarray\t\t= 0x5, +}; +U_BOOT_DEVICE(spl_test2) = { +\t.name\t\t= "sandbox_spl_test", +\t.platdata\t= &dtv_spl_test2, +\t.platdata_size\t= sizeof(dtv_spl_test2), +}; + +''', data) + + def testStdout(self): + """Test output to stdout""" + dtb_file = get_dtb_file('dtoc_test_simple.dts') + with test_util.capture_sys_output() as (stdout, stderr): + dtb_platdata.run_steps(['struct'], dtb_file, False, '-') + + def testNoCommand(self): + """Test running dtoc without a command""" + with self.assertRaises(ValueError) as e: + dtb_platdata.run_steps([], '', False, '') + self.assertIn("Please specify a command: struct, platdata", + str(e.exception)) + + def testBadCommand(self): + """Test running dtoc with an invalid command""" + dtb_file = get_dtb_file('dtoc_test_simple.dts') + output = tools.GetOutputFilename('output') + with self.assertRaises(ValueError) as e: + dtb_platdata.run_steps(['invalid-cmd'], dtb_file, False, output) + self.assertIn("Unknown command 'invalid-cmd': (use: struct, platdata)", + str(e.exception)) -- cgit v1.2.3 From fe57c784ad7187318a3aa6923d82095555bb50be Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:37 -0600 Subject: dtoc: Avoid unwanted output during tests At present some warnings are printed to indicate failures which are a known part of running the tests. Suppress these. Signed-off-by: Simon Glass --- tools/dtoc/fdt_util.py | 4 ++-- tools/dtoc/test_dtoc.py | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) (limited to 'tools') diff --git a/tools/dtoc/fdt_util.py b/tools/dtoc/fdt_util.py index 88fc3183833..5b631419a92 100644 --- a/tools/dtoc/fdt_util.py +++ b/tools/dtoc/fdt_util.py @@ -51,7 +51,7 @@ def fdt_cells_to_cpu(val, cells): out = out << 32 | fdt32_to_cpu(val[1]) return out -def EnsureCompiled(fname): +def EnsureCompiled(fname, capture_stderr=False): """Compile an fdt .dts source file into a .dtb binary blob if needed. Args: @@ -86,7 +86,7 @@ def EnsureCompiled(fname): args.extend(search_list) args.append(dts_input) dtc = os.environ.get('DTC') or 'dtc' - command.Run(dtc, *args) + command.Run(dtc, *args, capture_stderr=capture_stderr) return dtb_output def GetInt(node, propname, default=None): diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 622b39df395..72bcb37244e 100644 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -47,16 +47,19 @@ C_HEADER = '''/* ''' -def get_dtb_file(dts_fname): + +def get_dtb_file(dts_fname, capture_stderr=False): """Compile a .dts file to a .dtb Args: dts_fname: Filename of .dts file in the current directory + capture_stderr: True to capture and discard stderr output Returns: Filename of compiled file in output directory """ - return fdt_util.EnsureCompiled(os.path.join(our_path, dts_fname)) + return fdt_util.EnsureCompiled(os.path.join(our_path, dts_fname), + capture_stderr=capture_stderr) class TestDtoc(unittest.TestCase): @@ -626,7 +629,8 @@ U_BOOT_DEVICE(test3) = { def test_bad_reg(self): """Test that a reg property with an invalid type generates an error""" - dtb_file = get_dtb_file('dtoc_test_bad_reg.dts') + # Capture stderr since dtc will emit warnings for this file + dtb_file = get_dtb_file('dtoc_test_bad_reg.dts', capture_stderr=True) output = tools.GetOutputFilename('output') with self.assertRaises(ValueError) as e: dtb_platdata.run_steps(['struct'], dtb_file, False, output) @@ -635,7 +639,8 @@ U_BOOT_DEVICE(test3) = { def test_bad_reg2(self): """Test that a reg property with an invalid cell count is detected""" - dtb_file = get_dtb_file('dtoc_test_bad_reg2.dts') + # Capture stderr since dtc will emit warnings for this file + dtb_file = get_dtb_file('dtoc_test_bad_reg2.dts', capture_stderr=True) output = tools.GetOutputFilename('output') with self.assertRaises(ValueError) as e: dtb_platdata.run_steps(['struct'], dtb_file, False, output) -- cgit v1.2.3 From 116adecb5e8e343cfc73a55f25a5d3d8463f4512 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:38 -0600 Subject: dtoc: Add functions to add integer properties Add a few simple functions to add a placeholder integer property, and set its value. Signed-off-by: Simon Glass --- tools/dtoc/fdt.py | 27 +++++++++++++++++++++++++++ tools/dtoc/test_fdt.py | 20 ++++++++++++++++++++ 2 files changed, 47 insertions(+) (limited to 'tools') diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py index e7703c1c753..9d69b426c14 100644 --- a/tools/dtoc/fdt.py +++ b/tools/dtoc/fdt.py @@ -270,6 +270,33 @@ class Node: del self.props[prop_name] self._fdt.Invalidate() + def AddZeroProp(self, prop_name): + """Add a new property to the device tree with an integer value of 0. + + 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.open_into(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() + + 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. + + Args: + prop_name: Name of property + val: Value to set + """ + fdt_obj = self._fdt._fdt_obj + fdt_obj.setprop_u32(self.Offset(), prop_name, val) + + class Fdt: """Provides simple access to a flat device tree blob using libfdts. diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 49d188b1c12..f085b1dd1a9 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -319,6 +319,26 @@ class TestProp(unittest.TestCase): self.assertTrue(isinstance(prop.value, list)) self.assertEqual(3, len(prop.value)) + def testAdd(self): + """Test adding properties""" + self.fdt.pack() + # This function should automatically expand the device tree + self.node.AddZeroProp('one') + self.node.AddZeroProp('two') + self.node.AddZeroProp('three') + + # Updating existing properties should be OK, since the device-tree size + # does not change + self.fdt.pack() + self.node.SetInt('one', 1) + self.node.SetInt('two', 2) + self.node.SetInt('three', 3) + + # This should fail since it would need to increase the device-tree size + with self.assertRaises(libfdt.FdtException) as e: + self.node.SetInt('four', 4) + self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + class TestFdtUtil(unittest.TestCase): """Tests for the fdt_util module -- cgit v1.2.3 From 0a4357c4c2b29d787d70775dd6ad64b50e23082a Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:39 -0600 Subject: binman: Complete documentation of stages At present one of the stages is badly numbered and not described. Fix this. Signed-off-by: Simon Glass --- tools/binman/README | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index f74e39242f6..3cfcf84d92c 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -495,9 +495,11 @@ contents of an entry in some way. For example, it would be possible to create an entry containing a hash of the contents of some other entries. At this stage the position and size of entries should not be adjusted. -6. WriteEntryInfo() +7. WriteSymbols() - write the value of symbols into the U-Boot SPL binary. +See 'Access to binman entry positions at run time' below for a description of +what happens in this stage. -7. BuildImage() - builds the image and writes it to a file. This is the final +8. BuildImage() - builds the image and writes it to a file. This is the final step. -- cgit v1.2.3 From ecab89737a4eb58c043388b1ca1c0f1dfdaa3346 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:40 -0600 Subject: binman: Add a ProcessFdt() method Some entry types modify the device tree, e.g. to remove microcode or add a property. So far this just modifies their local copy and does not affect a 'shared' device tree. Rather than doing this modification in the ObtainContents() method, and a new ProcessFdt() method which is specifically designed to modify this shared device tree. Move the existing device-tree code over to use this method, reducing ObtainContents() to the goal of just obtaining the contents without any processing, even for device tree. Signed-off-by: Simon Glass --- tools/binman/README | 23 ++++++++----- tools/binman/bsection.py | 15 +++++++++ tools/binman/control.py | 52 ++++++++++++++++++++++++++++- tools/binman/entry.py | 3 ++ tools/binman/etype/section.py | 3 ++ tools/binman/etype/u_boot_dtb_with_ucode.py | 46 ++++++++++++------------- tools/binman/etype/u_boot_ucode.py | 2 +- tools/binman/etype/u_boot_with_ucode_ptr.py | 7 ++-- tools/binman/image.py | 3 ++ 9 files changed, 117 insertions(+), 37 deletions(-) (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index 3cfcf84d92c..008d5750528 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -462,7 +462,14 @@ Order of image creation Image creation proceeds in the following order, for each entry in the image. -1. GetEntryContents() - the contents of each entry are obtained, normally by +1. ProcessFdt() - process the device tree information as required by the +particular entry. This may involve adding or deleting properties. If the +processing is complete, this method should return True. If the processing +cannot complete because it needs the ProcessFdt() method of another entry to +run first, this method should return False, in which case it will be called +again later. + +2. GetEntryContents() - the contents of each entry are obtained, normally by reading from a file. This calls the Entry.ObtainContents() to read the contents. The default version of Entry.ObtainContents() calls Entry.GetDefaultFilename() and then reads that file. So a common mechanism @@ -471,35 +478,35 @@ functions must return True when they have read the contents. Binman will retry calling the functions a few times if False is returned, allowing dependencies between the contents of different entries. -2. GetEntryPositions() - calls Entry.GetPositions() for each entry. This can +3. GetEntryPositions() - calls Entry.GetPositions() for each entry. This can return a dict containing entries that need updating. The key should be the entry name and the value is a tuple (pos, size). This allows an entry to provide the position and size for other entries. The default implementation of GetEntryPositions() returns {}. -3. PackEntries() - calls Entry.Pack() which figures out the position and +4. PackEntries() - calls Entry.Pack() which figures out the position and size of an entry. The 'current' image position is passed in, and the function returns the position immediately after the entry being packed. The default implementation of Pack() is usually sufficient. -4. CheckSize() - checks that the contents of all the entries fits within +5. CheckSize() - checks that the contents of all the entries fits within the image size. If the image does not have a defined size, the size is set large enough to hold all the entries. -5. CheckEntries() - checks that the entries do not overlap, nor extend +6. CheckEntries() - checks that the entries do not overlap, nor extend outside the image. -6. ProcessEntryContents() - this calls Entry.ProcessContents() on each entry. +7. ProcessEntryContents() - this calls Entry.ProcessContents() on each entry. The default implementatoin does nothing. This can be overriden to adjust the contents of an entry in some way. For example, it would be possible to create an entry containing a hash of the contents of some other entries. At this stage the position and size of entries should not be adjusted. -7. WriteSymbols() - write the value of symbols into the U-Boot SPL binary. +8. WriteSymbols() - write the value of symbols into the U-Boot SPL binary. See 'Access to binman entry positions at run time' below for a description of what happens in this stage. -8. BuildImage() - builds the image and writes it to a file. This is the final +9. BuildImage() - builds the image and writes it to a file. This is the final step. diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 06a6711350a..3ed361d69a2 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -90,6 +90,21 @@ class Section(object): entry.SetPrefix(self._name_prefix) self._entries[node.name] = entry + def ProcessFdt(self, fdt): + todo = self._entries.values() + for passnum in range(3): + next_todo = [] + for entry in todo: + if not entry.ProcessFdt(fdt): + next_todo.append(entry) + todo = next_todo + if not todo: + break + if todo: + self._Raise('Internal error: Could not complete processing of Fdt: ' + 'remaining %s' % todo) + return True + def CheckSize(self): """Check that the section contents does not exceed its size, etc.""" contents_size = 0 diff --git a/tools/binman/control.py b/tools/binman/control.py index 92434729061..5325a6a0066 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -21,6 +21,11 @@ import tout # 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 = {} + + def _ReadImageDesc(binman_node): """Read the image descriptions from the /binman node @@ -53,6 +58,24 @@ 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. + + 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): + return fdt_files[fname]._fname + def Binman(options, args): """The main control code for binman @@ -93,12 +116,39 @@ def Binman(options, args): try: tools.SetInputDirs(options.indir) tools.PrepareOutputDir(options.outdir, options.preserve) - dtb = fdt.FdtScan(dtb_fname) + + # Get the device tree ready by compiling it and copying the compiled + # output into a file in our output directly. Then scan it for use + # in binman. + dtb_fname = fdt_util.EnsureCompiled(dtb_fname) + fname = tools.GetOutputFilename('u-boot-out.dtb') + with open(dtb_fname) as infd: + with open(fname, 'wb') as outfd: + outfd.write(infd.read()) + dtb = fdt.FdtScan(fname) + + # Note the file so that GetFdt() can find it + fdt_files['u-boot.dtb'] = dtb node = _FindBinmanNode(dtb) if not node: raise ValueError("Device tree '%s' does not have a 'binman' " "node" % dtb_fname) + images = _ReadImageDesc(node) + + # Prepare the device tree by making sure that any missing + # properties are added (e.g. 'pos' and 'size'). The values of these + # may not be correct yet, but we add placeholders so that the + # size of the device tree is correct. Later, in + # SetCalculatedProperties() we will insert the correct values + # without changing the device-tree size, thus ensuring that our + # entry positions remain the same. + for image in images.values(): + image.ProcessFdt(dtb) + + dtb.Pack() + dtb.Flush() + for image in images.values(): # Perform all steps for this image, including checking and # writing it. This means that errors found with a later diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 303c992e375..62e65c91266 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -130,6 +130,9 @@ class Entry(object): self.align_end = fdt_util.GetInt(self._node, 'align-end') self.pos_unset = fdt_util.GetBool(self._node, 'pos-unset') + def ProcessFdt(self, fdt): + return True + def SetPrefix(self, prefix): """Set the name prefix for a node diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 36b31a849f8..9b38738d38d 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -17,6 +17,9 @@ class Entry_section(Entry): Entry.__init__(self, image, etype, node) self._section = bsection.Section(node.name, node) + def ProcessFdt(self, fdt): + return self._section.ProcessFdt(fdt) + def ObtainContents(self): return self._section.GetEntryContents() diff --git a/tools/binman/etype/u_boot_dtb_with_ucode.py b/tools/binman/etype/u_boot_dtb_with_ucode.py index 1e530d6570a..3808d6d2780 100644 --- a/tools/binman/etype/u_boot_dtb_with_ucode.py +++ b/tools/binman/etype/u_boot_dtb_with_ucode.py @@ -5,6 +5,7 @@ # Entry-type module for U-Boot device tree with the microcode removed # +import control import fdt from entry import Entry from blob import Entry_blob @@ -22,13 +23,13 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): self.collate = False self.ucode_offset = None self.ucode_size = None + self.ucode = None + self.ready = False def GetDefaultFilename(self): return 'u-boot.dtb' - def ObtainContents(self): - Entry_blob.ObtainContents(self) - + def ProcessFdt(self, fdt): # If the section does not need microcode, there is nothing to do ucode_dest_entry = self.section.FindEntryType( 'u-boot-spl-with-ucode-ptr') @@ -38,36 +39,35 @@ class Entry_u_boot_dtb_with_ucode(Entry_blob): if not ucode_dest_entry or not ucode_dest_entry.target_pos: return True - # Create a new file to hold the copied device tree - dtb_name = 'u-boot-dtb-with-ucode.dtb' - fname = tools.GetOutputFilename(dtb_name) - with open(fname, 'wb') as fd: - fd.write(self.data) - # Remove the microcode - dtb = fdt.FdtScan(fname) - ucode = dtb.GetNode('/microcode') - if not ucode: + fname = self.GetDefaultFilename() + fdt = control.GetFdt(fname) + self.ucode = fdt.GetNode('/microcode') + if not self.ucode: raise self.Raise("No /microcode node found in '%s'" % fname) # There's no need to collate it (move all microcode into one place) # if we only have one chunk of microcode. - self.collate = len(ucode.subnodes) > 1 - for node in ucode.subnodes: + self.collate = len(self.ucode.subnodes) > 1 + for node in self.ucode.subnodes: data_prop = node.props.get('data') if data_prop: self.ucode_data += ''.join(data_prop.bytes) if self.collate: - prop = node.DeleteProp('data') - else: + node.DeleteProp('data') + return True + + 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.ReadContents() + if self.ucode: + for node in self.ucode.subnodes: + data_prop = node.props.get('data') + if data_prop and not self.collate: # 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) - if self.collate: - dtb.Pack() - dtb.Flush() - - # Make this file the contents of this entry - self._pathname = fname - self.ReadContents() + self.ready = True return True diff --git a/tools/binman/etype/u_boot_ucode.py b/tools/binman/etype/u_boot_ucode.py index 8cae7deed3c..ea0c85cc5c7 100644 --- a/tools/binman/etype/u_boot_ucode.py +++ b/tools/binman/etype/u_boot_ucode.py @@ -71,7 +71,7 @@ class Entry_u_boot_ucode(Entry_blob): fdt_entry = self.section.FindEntryType('u-boot-dtb-with-ucode') if not fdt_entry: return True - if not fdt_entry.ucode_data: + if not fdt_entry.ready: return False if not fdt_entry.collate: diff --git a/tools/binman/etype/u_boot_with_ucode_ptr.py b/tools/binman/etype/u_boot_with_ucode_ptr.py index 86945f33184..8b1e41152e3 100644 --- a/tools/binman/etype/u_boot_with_ucode_ptr.py +++ b/tools/binman/etype/u_boot_with_ucode_ptr.py @@ -28,7 +28,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): def GetDefaultFilename(self): return 'u-boot-nodtb.bin' - def ObtainContents(self): + def ProcessFdt(self, fdt): # Figure out where to put the microcode pointer fname = tools.GetInputFilename(self.elf_fname) sym = elf.GetSymbolAddress(fname, '_dt_ucode_base_size') @@ -36,8 +36,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): self.target_pos = sym elif not fdt_util.GetBool(self._node, 'optional-ucode'): self.Raise('Cannot locate _dt_ucode_base_size symbol in u-boot') - - return Entry_blob.ObtainContents(self) + return True def ProcessContents(self): # If the image does not need microcode, there is nothing to do @@ -73,7 +72,7 @@ class Entry_u_boot_with_ucode_ptr(Entry_blob): pos, size = ucode_entry.pos, ucode_entry.size else: dtb_entry = self.section.FindEntryType('u-boot-dtb-with-ucode') - if not dtb_entry: + if not dtb_entry or not dtb_entry.ready: self.Raise('Cannot find microcode region u-boot-dtb-with-ucode') pos = dtb_entry.pos + dtb_entry.ucode_offset size = dtb_entry.ucode_size diff --git a/tools/binman/image.py b/tools/binman/image.py index 835b66c99f5..9732493709c 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -54,6 +54,9 @@ class Image: self._filename = filename self._section = bsection.Section('main-section', self._node) + def ProcessFdt(self, fdt): + return self._section.ProcessFdt(fdt) + def GetEntryContents(self): """Call ObtainContents() for the section """ -- cgit v1.2.3 From 078ab1a2f5a9901f5f9983163d4f98b8d94bfda0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:41 -0600 Subject: binman: Add a SetCalculatedProperties() method Once binman has packed the image, the position and size of each entry is known. It is then possible for binman to update the device tree with these positions. Since placeholder values have been added, this does not affect the size of the device tree and therefore the packing does not need to be performed again. Add a new SetCalculatedProperties method to handle this. Signed-off-by: Simon Glass --- tools/binman/README | 29 ++++++++++++++++++++--------- tools/binman/bsection.py | 8 ++++++++ tools/binman/cmdline.py | 2 ++ tools/binman/control.py | 4 ++++ tools/binman/entry.py | 11 +++++++++++ tools/binman/etype/section.py | 8 ++++++++ tools/binman/image.py | 14 ++++++++++++++ 7 files changed, 67 insertions(+), 9 deletions(-) (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index 008d5750528..8b598a75c80 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -462,14 +462,22 @@ Order of image creation Image creation proceeds in the following order, for each entry in the image. -1. ProcessFdt() - process the device tree information as required by the +1. AddMissingProperties() - binman can add calculated values to the device +tree as part of its processing, for example the position and size of each +entry. This method adds any properties associated with this, expanding the +device tree as needed. These properties can have placeholder values which are +set later by SetCalculatedProperties(). By that stage the size of sections +cannot be changed (since it would cause the images to need to be repacked), +but the correct values can be inserted. + +2. ProcessFdt() - process the device tree information as required by the particular entry. This may involve adding or deleting properties. If the processing is complete, this method should return True. If the processing cannot complete because it needs the ProcessFdt() method of another entry to run first, this method should return False, in which case it will be called again later. -2. GetEntryContents() - the contents of each entry are obtained, normally by +3. GetEntryContents() - the contents of each entry are obtained, normally by reading from a file. This calls the Entry.ObtainContents() to read the contents. The default version of Entry.ObtainContents() calls Entry.GetDefaultFilename() and then reads that file. So a common mechanism @@ -478,35 +486,38 @@ functions must return True when they have read the contents. Binman will retry calling the functions a few times if False is returned, allowing dependencies between the contents of different entries. -3. GetEntryPositions() - calls Entry.GetPositions() for each entry. This can +4. GetEntryPositions() - calls Entry.GetPositions() for each entry. This can return a dict containing entries that need updating. The key should be the entry name and the value is a tuple (pos, size). This allows an entry to provide the position and size for other entries. The default implementation of GetEntryPositions() returns {}. -4. PackEntries() - calls Entry.Pack() which figures out the position and +5. PackEntries() - calls Entry.Pack() which figures out the position and size of an entry. The 'current' image position is passed in, and the function returns the position immediately after the entry being packed. The default implementation of Pack() is usually sufficient. -5. CheckSize() - checks that the contents of all the entries fits within +6. CheckSize() - checks that the contents of all the entries fits within the image size. If the image does not have a defined size, the size is set large enough to hold all the entries. -6. CheckEntries() - checks that the entries do not overlap, nor extend +7. CheckEntries() - checks that the entries do not overlap, nor extend outside the image. -7. ProcessEntryContents() - this calls Entry.ProcessContents() on each entry. +8. SetCalculatedProperties() - update any calculated properties in the device +tree. This sets the correct 'pos' and 'size' vaues, for example. + +9. ProcessEntryContents() - this calls Entry.ProcessContents() on each entry. The default implementatoin does nothing. This can be overriden to adjust the contents of an entry in some way. For example, it would be possible to create an entry containing a hash of the contents of some other entries. At this stage the position and size of entries should not be adjusted. -8. WriteSymbols() - write the value of symbols into the U-Boot SPL binary. +10. WriteSymbols() - write the value of symbols into the U-Boot SPL binary. See 'Access to binman entry positions at run time' below for a description of what happens in this stage. -9. BuildImage() - builds the image and writes it to a file. This is the final +11. BuildImage() - builds the image and writes it to a file. This is the final step. diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py index 3ed361d69a2..de439ef625f 100644 --- a/tools/binman/bsection.py +++ b/tools/binman/bsection.py @@ -90,6 +90,14 @@ class Section(object): entry.SetPrefix(self._name_prefix) self._entries[node.name] = entry + def AddMissingProperties(self): + for entry in self._entries.values(): + entry.AddMissingProperties() + + def SetCalculatedProperties(self): + for entry in self._entries.values(): + entry.SetCalculatedProperties() + def ProcessFdt(self, fdt): todo = self._entries.values() for passnum in range(3): diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index bf63919eb79..ae2d1670f9a 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -42,6 +42,8 @@ def ParseArgs(argv): default=False, help='run tests') parser.add_option('-T', '--test-coverage', action='store_true', default=False, help='run tests and check for 100% coverage') + parser.add_option('-u', '--update-fdt', action='store_true', + default=False, help='Update the binman node with position/size info') parser.add_option('-v', '--verbosity', default=1, type='int', help='Control verbosity: 0=silent, 1=progress, 3=full, ' '4=debug') diff --git a/tools/binman/control.py b/tools/binman/control.py index 5325a6a0066..eafabf05c7c 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -144,6 +144,8 @@ def Binman(options, args): # without changing the device-tree size, thus ensuring that our # entry positions remain the same. for image in images.values(): + if options.update_fdt: + image.AddMissingProperties() image.ProcessFdt(dtb) dtb.Pack() @@ -159,6 +161,8 @@ def Binman(options, args): image.PackEntries() image.CheckSize() image.CheckEntries() + if options.update_fdt: + image.SetCalculatedProperties() image.ProcessEntryContents() image.WriteSymbols() image.BuildImage() diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 62e65c91266..6a173e663d0 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -130,6 +130,17 @@ class Entry(object): self.align_end = fdt_util.GetInt(self._node, 'align-end') self.pos_unset = fdt_util.GetBool(self._node, 'pos-unset') + def AddMissingProperties(self): + """Add new properties to the device tree as needed for this entry""" + for prop in ['pos', 'size']: + if not prop in self._node.props: + self._node.AddZeroProp(prop) + + def SetCalculatedProperties(self): + """Set the value of device-tree properties calculated by binman""" + self._node.SetInt('pos', self.pos) + self._node.SetInt('size', self.size) + def ProcessFdt(self, fdt): return True diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 9b38738d38d..787257d3ec1 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -20,6 +20,10 @@ class Entry_section(Entry): def ProcessFdt(self, fdt): return self._section.ProcessFdt(fdt) + def AddMissingProperties(self): + Entry.AddMissingProperties(self) + self._section.AddMissingProperties() + def ObtainContents(self): return self._section.GetEntryContents() @@ -45,6 +49,10 @@ class Entry_section(Entry): """Write symbol values into binary files for access at run time""" self._section.WriteSymbols() + def SetCalculatedProperties(self): + Entry.SetCalculatedProperties(self) + self._section.SetCalculatedProperties() + def ProcessContents(self): self._section.ProcessEntryContents() super(Entry_section, self).ProcessContents() diff --git a/tools/binman/image.py b/tools/binman/image.py index 9732493709c..94028f19a51 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -54,6 +54,17 @@ class Image: self._filename = filename self._section = bsection.Section('main-section', self._node) + def AddMissingProperties(self): + """Add properties that are not present in the device tree + + When binman has completed packing the entries the position and size of + each entry are known. But before this the device tree may not specify + these. Add any missing properties, with a dummy value, so that the + size of the entry is correct. That way we can insert the correct values + later. + """ + self._section.AddMissingProperties() + def ProcessFdt(self, fdt): return self._section.ProcessFdt(fdt) @@ -82,6 +93,9 @@ class Image: """Check that entries do not overlap or extend outside the image""" self._section.CheckEntries() + def SetCalculatedProperties(self): + self._section.SetCalculatedProperties() + def ProcessEntryContents(self): """Call the ProcessContents() method for each entry -- cgit v1.2.3 From 16b8d6b76992690c65c58dc8b0591496cc5e46ef Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 6 Jul 2018 10:27:42 -0600 Subject: binman: Support updating the device tree with calc'd info It is useful to write the position and size of each entry back to the device tree so that U-Boot can access this at runtime. Add a feature to support this, along with associated tests. Signed-off-by: Simon Glass --- tools/binman/README | 6 +-- tools/binman/control.py | 2 + tools/binman/etype/_testing.py | 10 +++++ tools/binman/ftest.py | 77 ++++++++++++++++++++++++++++----- tools/binman/test/60_fdt_update.dts | 31 +++++++++++++ tools/binman/test/61_fdt_update_bad.dts | 32 ++++++++++++++ 6 files changed, 144 insertions(+), 14 deletions(-) create mode 100644 tools/binman/test/60_fdt_update.dts create mode 100644 tools/binman/test/61_fdt_update_bad.dts (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index 8b598a75c80..207928aa955 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -669,13 +669,11 @@ To do ----- Some ideas: -- Fill out the device tree to include the final position and size of each - entry (since the input file may not always specify these). See also - 'Access to binman entry positions at run time' above - Use of-platdata to make the information available to code that is unable to use device tree (such as a very small SPL image) - Allow easy building of images by specifying just the board name -- Produce a full Python binding for libfdt (for upstream) +- Produce a full Python binding for libfdt (for upstream). This is nearing + completion but some work remains - Add an option to decode an image into the constituent binaries - Support building an image for a board (-b) more completely, with a configurable build directory diff --git a/tools/binman/control.py b/tools/binman/control.py index eafabf05c7c..a40b300fdac 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -168,6 +168,8 @@ def Binman(options, args): image.BuildImage() if options.map: image.WriteMap() + with open(fname, 'wb') as outfd: + outfd.write(dtb.GetContents()) finally: tools.FinaliseOutputDir() finally: diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py index 04bdc6c532b..6a1af577988 100644 --- a/tools/binman/etype/_testing.py +++ b/tools/binman/etype/_testing.py @@ -24,6 +24,9 @@ class Entry__testing(Entry): 'return-unknown-contents') self.bad_update_contents = fdt_util.GetBool(self._node, 'bad-update-contents') + self.process_fdt_ready = False + self.never_complete_process_fdt = fdt_util.GetBool(self._node, + 'never-complete-process-fdt') def ObtainContents(self): if self.return_unknown_contents: @@ -42,3 +45,10 @@ class Entry__testing(Entry): # Request to update the conents with something larger, to cause a # failure. self.ProcessContentsUpdate('aa') + + def ProcessFdt(self, fdt): + """Force reprocessing the first time""" + ready = self.process_fdt_ready + if not self.never_complete_process_fdt: + self.process_fdt_ready = True + return ready diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index af3b4dc3e56..12164a85b4d 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -146,19 +146,23 @@ class TestFunctional(unittest.TestCase): # options.verbosity = tout.DEBUG return control.Binman(options, args) - def _DoTestFile(self, fname, debug=False, map=False): + def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False): """Run binman with a given test file Args: fname: Device-tree source filename to use (e.g. 05_simple.dts) debug: True to enable debugging output map: True to output map files for the images + update_dtb: Update the position and size of each entry in the device + tree before packing it into the image """ args = ['-p', '-I', self._indir, '-d', self.TestFile(fname)] if debug: args.append('-D') if map: args.append('-m') + if update_dtb: + args.append('-up') return self._DoBinman(*args) def _SetupDtb(self, fname, outfile='u-boot.dtb'): @@ -183,7 +187,8 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile(outfile, data) return data - def _DoReadFileDtb(self, fname, use_real_dtb=False, map=False): + def _DoReadFileDtb(self, fname, use_real_dtb=False, map=False, + update_dtb=False): """Run binman and return the resulting image This runs binman with a given test file and then reads the resulting @@ -199,6 +204,8 @@ class TestFunctional(unittest.TestCase): test contents (the U_BOOT_DTB_DATA string) can be used. But in some test we need the real contents. map: True to output map files for the images + update_dtb: Update the position and size of each entry in the device + tree before packing it into the image Returns: Tuple: @@ -212,21 +219,22 @@ class TestFunctional(unittest.TestCase): dtb_data = self._SetupDtb(fname) try: - retcode = self._DoTestFile(fname, map=map) + retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb) self.assertEqual(0, retcode) + out_dtb_fname = control.GetFdtPath('u-boot.dtb') # Find the (only) image, read it and return its contents image = control.images['image'] - fname = tools.GetOutputFilename('image.bin') - self.assertTrue(os.path.exists(fname)) + image_fname = tools.GetOutputFilename('image.bin') + self.assertTrue(os.path.exists(image_fname)) if map: map_fname = tools.GetOutputFilename('image.map') with open(map_fname) as fd: map_data = fd.read() else: map_data = None - with open(fname) as fd: - return fd.read(), dtb_data, map_data + with open(image_fname) as fd: + return fd.read(), dtb_data, map_data, out_dtb_fname finally: # Put the test file back if use_real_dtb: @@ -300,6 +308,26 @@ class TestFunctional(unittest.TestCase): """ return struct.unpack('>L', dtb[4:8])[0] + def _GetPropTree(self, dtb_data, node_names): + def AddNode(node, path): + if node.name != '/': + path += '/' + node.name + #print 'path', path + for subnode in node.subnodes: + for prop in subnode.props.values(): + if prop.name in node_names: + prop_path = path + '/' + subnode.name + ':' + prop.name + tree[prop_path[len('/binman/'):]] = fdt_util.fdt32_to_cpu( + prop.value) + #print ' ', prop.name + AddNode(subnode, path) + + tree = {} + dtb = fdt.Fdt(dtb_data) + dtb.Scan() + AddNode(dtb.GetRoot(), '') + return tree + def testRun(self): """Test a basic run with valid args""" result = self._RunBinman('-h') @@ -845,7 +873,7 @@ class TestFunctional(unittest.TestCase): """Test that we can cope with an image without microcode (e.g. qemu)""" with open(self.TestFile('u_boot_no_ucode_ptr')) as fd: TestFunctional._MakeInputFile('u-boot', fd.read()) - data, dtb, _ = self._DoReadFileDtb('44_x86_optional_ucode.dts', True) + data, dtb, _, _ = self._DoReadFileDtb('44_x86_optional_ucode.dts', True) # Now check the device tree has no microcode self.assertEqual(U_BOOT_NODTB_DATA, data[:len(U_BOOT_NODTB_DATA)]) @@ -980,7 +1008,7 @@ class TestFunctional(unittest.TestCase): def testMap(self): """Tests outputting a map of the images""" - _, _, map_data = self._DoReadFileDtb('55_sections.dts', map=True) + _, _, map_data, _ = self._DoReadFileDtb('55_sections.dts', map=True) self.assertEqual('''Position Size Name 00000000 00000010 section@0 00000000 00000004 u-boot @@ -990,7 +1018,7 @@ class TestFunctional(unittest.TestCase): def testNamePrefix(self): """Tests that name prefixes are used""" - _, _, map_data = self._DoReadFileDtb('56_name_prefix.dts', map=True) + _, _, map_data, _ = self._DoReadFileDtb('56_name_prefix.dts', map=True) self.assertEqual('''Position Size Name 00000000 00000010 section@0 00000000 00000004 ro-u-boot @@ -1013,6 +1041,35 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/_testing': Cannot update entry size from " '2 to 1', str(e.exception)) + def testUpdateFdt(self): + """Test that we can update the device tree with pos/size info""" + _, _, _, out_dtb_fname = self._DoReadFileDtb('60_fdt_update.dts', + update_dtb=True) + props = self._GetPropTree(out_dtb_fname, ['pos', 'size']) + with open('/tmp/x.dtb', 'wb') as outf: + with open(out_dtb_fname) as inf: + outf.write(inf.read()) + self.assertEqual({ + '_testing:pos': 32, + '_testing:size': 1, + 'section@0/u-boot:pos': 0, + 'section@0/u-boot:size': len(U_BOOT_DATA), + 'section@0:pos': 0, + 'section@0:size': 16, + + 'section@1/u-boot:pos': 0, + 'section@1/u-boot:size': len(U_BOOT_DATA), + 'section@1:pos': 16, + 'section@1:size': 16, + 'size': 40 + }, props) + + def testUpdateFdtBad(self): + """Test that we detect when ProcessFdt never completes""" + with self.assertRaises(ValueError) as e: + self._DoReadFileDtb('61_fdt_update_bad.dts', update_dtb=True) + self.assertIn('Could not complete processing of Fdt: remaining ' + '[<_testing.Entry__testing', str(e.exception)) if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/60_fdt_update.dts b/tools/binman/test/60_fdt_update.dts new file mode 100644 index 00000000000..f53c8a5053e --- /dev/null +++ b/tools/binman/test/60_fdt_update.dts @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0x26>; + size = <0x28>; + section@0 { + read-only; + name-prefix = "ro-"; + size = <0x10>; + pad-byte = <0x21>; + + u-boot { + }; + }; + section@1 { + name-prefix = "rw-"; + size = <0x10>; + pad-byte = <0x61>; + + u-boot { + }; + }; + _testing { + }; + }; +}; diff --git a/tools/binman/test/61_fdt_update_bad.dts b/tools/binman/test/61_fdt_update_bad.dts new file mode 100644 index 00000000000..e5abf31699c --- /dev/null +++ b/tools/binman/test/61_fdt_update_bad.dts @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0x26>; + size = <0x28>; + section@0 { + read-only; + name-prefix = "ro-"; + size = <0x10>; + pad-byte = <0x21>; + + u-boot { + }; + }; + section@1 { + name-prefix = "rw-"; + size = <0x10>; + pad-byte = <0x61>; + + u-boot { + }; + }; + _testing { + never-complete-process-fdt; + }; + }; +}; -- cgit v1.2.3