From 25980791b1848df7df6c273dc5578837cc84f853 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 30 Jul 2022 20:57:05 -0600 Subject: [PATCH 1/7] dtoc: Tidy up fdt_tests RunTestCoverage() args Pass the options args in rather than using the global various. Use snake case and fix up comments to make pylint happy. Signed-off-by: Simon Glass --- tools/dtoc/test_fdt.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 3baf4437cd..ec257552ef 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -768,10 +768,14 @@ class TestFdtUtil(unittest.TestCase): tools.outdir= old_outdir -def RunTestCoverage(): - """Run the tests and check that we get 100% coverage""" +def run_test_coverage(build_dir): + """Run the tests and check that we get 100% coverage + + Args: + build_dir (str): Directory containing the build output + """ test_util.run_test_coverage('tools/dtoc/test_fdt.py', None, - ['tools/patman/*.py', '*test_fdt.py'], options.build_dir) + ['tools/patman/*.py', '*test_fdt.py'], build_dir) def RunTests(args): @@ -811,4 +815,4 @@ if options.test: ret_code = RunTests(args) sys.exit(ret_code) elif options.test_coverage: - RunTestCoverage() + run_test_coverage(options.build_dir) From b26dd9648c48d41c1486a623012363167c7a5fc7 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 30 Jul 2022 20:57:06 -0600 Subject: [PATCH 2/7] dtoc: Tidy up fdt_tests RunTests() Pass the options args in rather than using the global variables. Use snake case, fix up comments and use a ternary operator to make pylint happy. Signed-off-by: Simon Glass --- tools/dtoc/test_fdt.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index ec257552ef..b48819831d 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -778,17 +778,20 @@ def run_test_coverage(build_dir): ['tools/patman/*.py', '*test_fdt.py'], build_dir) -def RunTests(args): +def run_tests(args, processes): """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) + args (list or str): List of positional args provided. This can hold a + test name to execute (as in 'test_fdt -t testFdt', for example) + processes (int): Number of processes to use (None means as many as there + are CPUs on the system. This must be set to 1 when running under + the python3-coverage tool Returns: - Return code, 0 on success + int: Return code, 0 on success """ - test_name = args and args[0] or None + test_name = args[0] if args else None result = test_util.run_test_suites( 'test_fdt', False, False, False, None, test_name, None, [TestFdt, TestNode, TestProp, TestFdtUtil]) @@ -812,7 +815,7 @@ parser.add_option('-T', '--test-coverage', action='store_true', # Run our meagre tests if options.test: - ret_code = RunTests(args) + ret_code = run_tests(args, options.processes) sys.exit(ret_code) elif options.test_coverage: run_test_coverage(options.build_dir) From ad744222f59e765ef9d6ee7efa2ee670a4ebd06d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 30 Jul 2022 20:57:07 -0600 Subject: [PATCH 3/7] dtoc: Fix fdt test coverage Fix a bug that the --processes option was ignored, thus resulting in no test coverage information being generated. Signed-off-by: Simon Glass Fixes: 42ae363ddd9 ("dtoc: Update fdt tests to use test_util") --- tools/dtoc/test_fdt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index b48819831d..afa0bb5885 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -793,7 +793,7 @@ def run_tests(args, processes): """ test_name = args[0] if args else None result = test_util.run_test_suites( - 'test_fdt', False, False, False, None, test_name, None, + 'test_fdt', False, False, False, processes, test_name, None, [TestFdt, TestNode, TestProp, TestFdtUtil]) return (0 if result.wasSuccessful() else 1) From a8ad9aacd34cc1fd1dc7c0c8703c9467d76676b9 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 30 Jul 2022 20:57:08 -0600 Subject: [PATCH 4/7] dtoc: Move main program into its own function Use a function for the main program so everything there doesn't look like a global variable to pylint. Signed-off-by: Simon Glass --- tools/dtoc/test_fdt.py | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index afa0bb5885..e10fb528e8 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -799,23 +799,27 @@ def run_tests(args, processes): return (0 if result.wasSuccessful() else 1) -if __name__ != '__main__': - sys.exit(1) +def main(): + """Main program for this tool""" + parser = OptionParser() + parser.add_option('-B', '--build-dir', type='string', default='b', + help='Directory containing the build output') + parser.add_option('-P', '--processes', type=int, + help='set number of processes to use for running tests') + 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() -parser = OptionParser() -parser.add_option('-B', '--build-dir', type='string', default='b', - help='Directory containing the build output') -parser.add_option('-P', '--processes', type=int, - help='set number of processes to use for running tests') -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: + ret_code = run_tests(args, options.processes) + return ret_code + if options.test_coverage: + run_test_coverage(options.build_dir) + return 0 -# Run our meagre tests -if options.test: - ret_code = run_tests(args, options.processes) - sys.exit(ret_code) -elif options.test_coverage: - run_test_coverage(options.build_dir) +if __name__ == '__main__': + sys.exit(main()) +sys.exit(1) From 7640b166604e04ac19ed5a370441a6e5539c1420 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 30 Jul 2022 20:57:09 -0600 Subject: [PATCH 5/7] test_fdt: Convert to use argparse Drop the deprecated OptionParser. Signed-off-by: Simon Glass --- tools/dtoc/test_fdt.py | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index e10fb528e8..c38158edf3 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -4,7 +4,7 @@ # Written by Simon Glass # -from optparse import OptionParser +from argparse import ArgumentParser import glob import os import shutil @@ -778,12 +778,11 @@ def run_test_coverage(build_dir): ['tools/patman/*.py', '*test_fdt.py'], build_dir) -def run_tests(args, processes): +def run_tests(names, processes): """Run all the test we have for the fdt model Args: - args (list or str): List of positional args provided. This can hold a - test name to execute (as in 'test_fdt -t testFdt', for example) + names (list of str): List of test names provided. Only the first is used processes (int): Number of processes to use (None means as many as there are CPUs on the system. This must be set to 1 when running under the python3-coverage tool @@ -791,7 +790,7 @@ def run_tests(args, processes): Returns: int: Return code, 0 on success """ - test_name = args[0] if args else None + test_name = names[0] if names else None result = test_util.run_test_suites( 'test_fdt', False, False, False, processes, test_name, None, [TestFdt, TestNode, TestProp, TestFdtUtil]) @@ -801,23 +800,25 @@ def run_tests(args, processes): def main(): """Main program for this tool""" - parser = OptionParser() - parser.add_option('-B', '--build-dir', type='string', default='b', - help='Directory containing the build output') - parser.add_option('-P', '--processes', type=int, - help='set number of processes to use for running tests') - 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() + parser = ArgumentParser() + parser.add_argument('-B', '--build-dir', type=str, default='b', + help='Directory containing the build output') + parser.add_argument('-P', '--processes', type=int, + help='set number of processes to use for running tests') + parser.add_argument('-t', '--test', action='store_true', dest='test', + default=False, help='run tests') + parser.add_argument('-T', '--test-coverage', action='store_true', + default=False, + help='run tests and check for 100% coverage') + parser.add_argument('name', nargs='*') + args = parser.parse_args() # Run our meagre tests - if options.test: - ret_code = run_tests(args, options.processes) + if args.test: + ret_code = run_tests(args.name, args.processes) return ret_code - if options.test_coverage: - run_test_coverage(options.build_dir) + if args.test_coverage: + run_test_coverage(args.build_dir) return 0 if __name__ == '__main__': From 5d1637a40cd1c30f62ebee6e1e1611d5f5613003 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 30 Jul 2022 20:57:10 -0600 Subject: [PATCH 6/7] dtoc: Correct remaining pylint problems in test_fdt Fix various camel-case and other naming problems. Update the pylint base file to avoid regressions. Signed-off-by: Simon Glass --- scripts/pylint.base | 2 +- tools/dtoc/test_fdt.py | 254 +++++++++++++++++++++++------------------ 2 files changed, 142 insertions(+), 114 deletions(-) diff --git a/scripts/pylint.base b/scripts/pylint.base index dd6360e625..c7d141ed39 100644 --- a/scripts/pylint.base +++ b/scripts/pylint.base @@ -167,7 +167,7 @@ tools_binman_etype_x86_reset16_tpl -15.71 tools_binman_etype_x86_start16 -15.71 tools_binman_etype_x86_start16_spl -15.71 tools_binman_etype_x86_start16_tpl -15.71 -tools_binman_fdt_test 3.23 +tools_binman_fdt_test 10.00 tools_binman_fip_util 9.85 tools_binman_fip_util_test 10.00 tools_binman_fmap_util 6.88 diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index c38158edf3..8a990b8bd7 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -1,11 +1,13 @@ #!/usr/bin/env python3 # SPDX-License-Identifier: GPL-2.0+ -# Copyright (c) 2018 Google, Inc -# Written by Simon Glass -# + +""" +Tests for the Fdt module +Copyright (c) 2018 Google, Inc +Written by Simon Glass +""" from argparse import ArgumentParser -import glob import os import shutil import sys @@ -22,16 +24,18 @@ sys.path.insert(2, os.path.join(our_path, '../../scripts/dtc/pylibfdt')) sys.path.insert(2, os.path.join(our_path, '../../build-sandbox_spl/scripts/dtc/pylibfdt')) +#pylint: disable=wrong-import-position from dtoc import fdt from dtoc import fdt_util from dtoc.fdt_util import fdt32_to_cpu, fdt64_to_cpu from dtoc.fdt import Type, BytesToValue import libfdt -from patman import command from patman import test_util from patman import tools -def _GetPropertyValue(dtb, node, prop_name): +#pylint: disable=protected-access + +def _get_property_value(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 @@ -83,13 +87,13 @@ class TestFdt(unittest.TestCase): def setUp(self): self.dtb = fdt.FdtScan(find_dtb_file('dtoc_test_simple.dts')) - def testFdt(self): + def test_fdt(self): """Test that we can open an Fdt""" self.dtb.Scan() root = self.dtb.GetRoot() self.assertTrue(isinstance(root, fdt.Node)) - def testGetNode(self): + def test_get_node(self): """Test the GetNode() method""" node = self.dtb.GetNode('/spl-test') self.assertTrue(isinstance(node, fdt.Node)) @@ -103,27 +107,28 @@ class TestFdt(unittest.TestCase): self.assertTrue(isinstance(node, fdt.Node)) self.assertEqual(0, node.Offset()) - def testFlush(self): + def test_flush(self): """Check that we can flush the device tree out to its file""" fname = self.dtb._fname - with open(fname, 'rb') as fd: - data = fd.read() + with open(fname, 'rb') as inf: + inf.read() os.remove(fname) with self.assertRaises(IOError): - open(fname, 'rb') + with open(fname, 'rb'): + pass self.dtb.Flush() - with open(fname, 'rb') as fd: - data = fd.read() + with open(fname, 'rb') as inf: + inf.read() - def testPack(self): + def test_pack(self): """Test that packing a device tree works""" self.dtb.Pack() - def testGetFdtRaw(self): + def test_get_fdt_raw(self): """Tetst that we can access the raw device-tree data""" self.assertTrue(isinstance(self.dtb.GetContents(), bytes)) - def testGetProps(self): + def test_get_props(self): """Tests obtaining a list of properties""" node = self.dtb.GetNode('/spl-test') props = self.dtb.GetProps(node) @@ -133,17 +138,19 @@ class TestFdt(unittest.TestCase): 'stringval', 'u-boot,dm-pre-reloc'], sorted(props.keys())) - def testCheckError(self): + def test_check_error(self): """Tests the ChecKError() function""" - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: fdt.CheckErr(-libfdt.NOTFOUND, 'hello') - self.assertIn('FDT_ERR_NOTFOUND: hello', str(e.exception)) + self.assertIn('FDT_ERR_NOTFOUND: hello', str(exc.exception)) - def testGetFdt(self): + def test_get_fdt(self): + """Test getting an Fdt object from a node""" node = self.dtb.GetNode('/spl-test') self.assertEqual(self.dtb, node.GetFdt()) - def testBytesToValue(self): + def test_bytes_to_value(self): + """Test converting a string list into Python""" self.assertEqual(BytesToValue(b'this\0is\0'), (Type.STRING, ['this', 'is'])) @@ -163,11 +170,11 @@ class TestNode(unittest.TestCase): self.node = self.dtb.GetNode('/spl-test') self.fdt = self.dtb.GetFdtObj() - def testOffset(self): + def test_offset(self): """Tests that we can obtain the offset of a node""" self.assertTrue(self.node.Offset() > 0) - def testDelete(self): + def test_delete(self): """Tests that we can delete a property""" node2 = self.dtb.GetNode('/spl-test2') offset1 = node2.Offset() @@ -180,13 +187,13 @@ class TestNode(unittest.TestCase): with self.assertRaises(libfdt.FdtException): self.node.DeleteProp('missing') - def testDeleteGetOffset(self): + def test_delete_get_offset(self): """Test that property offset update when properties are deleted""" self.node.DeleteProp('intval') - prop, value = _GetPropertyValue(self.dtb, self.node, 'longbytearray') + prop, value = _get_property_value(self.dtb, self.node, 'longbytearray') self.assertEqual(prop.value, value) - def testFindNode(self): + def test_find_node(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) @@ -194,33 +201,33 @@ class TestNode(unittest.TestCase): self.assertEqual('pmic@9', subnode.name) self.assertEqual(None, node.FindNode('missing')) - def testRefreshMissingNode(self): + def test_refresh_missing_node(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: + with self.assertRaises(ValueError) as exc: self.dtb.Refresh() - self.assertIn('Internal error, offset', str(e.exception)) + self.assertIn('Internal error, offset', str(exc.exception)) - def testRefreshExtraNode(self): + def test_refresh_extra_node(self): """Test refreshing offsets when an expected node is missing""" # Delete it from the device tre, not our tables self.fdt.del_node(self.node.Offset()) - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: self.dtb.Refresh() self.assertIn('Internal error, node name mismatch ' - 'spl-test != spl-test2', str(e.exception)) + 'spl-test != spl-test2', str(exc.exception)) - def testRefreshMissingProp(self): + def test_refresh_missing_prop(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: + with self.assertRaises(ValueError) as exc: self.dtb.Refresh() self.assertIn("Internal error, node '/spl-test' property 'notstring' missing, offset ", - str(e.exception)) + str(exc.exception)) - def testLookupPhandle(self): + def test_lookup_phandle(self): """Test looking up a single phandle""" dtb = fdt.FdtScan(find_dtb_file('dtoc_test_phandle.dts')) node = dtb.GetNode('/phandle-source2') @@ -228,19 +235,19 @@ class TestNode(unittest.TestCase): target = dtb.GetNode('/phandle-target') self.assertEqual(target, dtb.LookupPhandle(fdt32_to_cpu(prop.value))) - def testAddNodeSpace(self): + def test_add_node_space(self): """Test adding a single node when out of space""" self.fdt.pack() self.node.AddSubnode('subnode') - with self.assertRaises(libfdt.FdtException) as e: + with self.assertRaises(libfdt.FdtException) as exc: self.dtb.Sync(auto_resize=False) - self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + self.assertIn('FDT_ERR_NOSPACE', str(exc.exception)) self.dtb.Sync(auto_resize=True) offset = self.fdt.path_offset('/spl-test/subnode') self.assertTrue(offset > 0) - def testAddNodes(self): + def test_add_nodes(self): """Test adding various subnode and properies""" node = self.dtb.GetNode('/i2c@0') @@ -272,7 +279,7 @@ class TestNode(unittest.TestCase): self.dtb.Sync(auto_resize=True) - def testAddOneNode(self): + def test_add_one_node(self): """Testing deleting and adding a subnode before syncing""" subnode = self.node.AddSubnode('subnode') self.node.AddSubnode('subnode2') @@ -283,21 +290,21 @@ class TestNode(unittest.TestCase): self.node.AddSubnode('subnode3') self.dtb.Sync() - def testRefreshNameMismatch(self): + def test_refresh_name_mismatch(self): """Test name mismatch when syncing nodes and properties""" - prop = self.node.AddInt('integer-a', 12) + self.node.AddInt('integer-a', 12) wrong_offset = self.dtb.GetNode('/i2c@0')._offset self.node._offset = wrong_offset - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: self.dtb.Sync() self.assertIn("Internal error, node '/spl-test' name mismatch 'i2c@0'", - str(e.exception)) + str(exc.exception)) - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: self.node.Refresh(wrong_offset) self.assertIn("Internal error, node '/spl-test' name mismatch 'i2c@0'", - str(e.exception)) + str(exc.exception)) class TestProp(unittest.TestCase): @@ -316,80 +323,83 @@ class TestProp(unittest.TestCase): self.node = self.dtb.GetNode('/spl-test') self.fdt = self.dtb.GetFdtObj() - def testMissingNode(self): + def test_missing_node(self): + """Test GetNode() when the node is missing""" self.assertEqual(None, self.dtb.GetNode('missing')) - def testPhandle(self): + def test_phandle(self): + """Test GetNode() on a phandle""" dtb = fdt.FdtScan(find_dtb_file('dtoc_test_phandle.dts')) node = dtb.GetNode('/phandle-source2') prop = node.props['clocks'] self.assertTrue(fdt32_to_cpu(prop.value) > 0) - def _ConvertProp(self, prop_name): + def _convert_prop(self, prop_name): """Helper function to look up a property in self.node and return it Args: - Property name to find + str: Property name to find - Return fdt.Prop object for this property + Returns: + fdt.Prop: object for this property """ - p = self.fdt.getprop(self.node.Offset(), prop_name) - return fdt.Prop(self.node, -1, prop_name, p) + prop = self.fdt.getprop(self.node.Offset(), prop_name) + return fdt.Prop(self.node, -1, prop_name, prop) - def testMakeProp(self): + def test_make_prop(self): """Test we can convert all the the types that are supported""" - prop = self._ConvertProp('boolval') + prop = self._convert_prop('boolval') self.assertEqual(Type.BOOL, prop.type) self.assertEqual(True, prop.value) - prop = self._ConvertProp('intval') + prop = self._convert_prop('intval') self.assertEqual(Type.INT, prop.type) self.assertEqual(1, fdt32_to_cpu(prop.value)) - prop = self._ConvertProp('int64val') + prop = self._convert_prop('int64val') self.assertEqual(Type.INT, prop.type) self.assertEqual(0x123456789abcdef0, fdt64_to_cpu(prop.value)) - prop = self._ConvertProp('intarray') + prop = self._convert_prop('intarray') self.assertEqual(Type.INT, prop.type) val = [fdt32_to_cpu(val) for val in prop.value] self.assertEqual([2, 3, 4], val) - prop = self._ConvertProp('byteval') + prop = self._convert_prop('byteval') self.assertEqual(Type.BYTE, prop.type) self.assertEqual(5, ord(prop.value)) - prop = self._ConvertProp('longbytearray') + prop = self._convert_prop('longbytearray') self.assertEqual(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') + prop = self._convert_prop('stringval') self.assertEqual(Type.STRING, prop.type) self.assertEqual('message', prop.value) - prop = self._ConvertProp('stringarray') + prop = self._convert_prop('stringarray') self.assertEqual(Type.STRING, prop.type) self.assertEqual(['multi-word', 'message'], prop.value) - prop = self._ConvertProp('notstring') + prop = self._convert_prop('notstring') self.assertEqual(Type.BYTE, prop.type) val = [ord(val) for val in prop.value] self.assertEqual([0x20, 0x21, 0x22, 0x10, 0], val) - def testGetEmpty(self): + def test_get_empty(self): """Tests the GetEmpty() function for the various supported types""" self.assertEqual(True, fdt.Prop.GetEmpty(Type.BOOL)) self.assertEqual(chr(0), fdt.Prop.GetEmpty(Type.BYTE)) self.assertEqual(tools.get_bytes(0, 4), fdt.Prop.GetEmpty(Type.INT)) self.assertEqual('', fdt.Prop.GetEmpty(Type.STRING)) - def testGetOffset(self): + def test_get_offset(self): """Test we can get the offset of a property""" - prop, value = _GetPropertyValue(self.dtb, self.node, 'longbytearray') + prop, value = _get_property_value(self.dtb, self.node, 'longbytearray') self.assertEqual(prop.value, value) - def testWiden(self): + def test_widen(self): """Test widening of values""" node2 = self.dtb.GetNode('/spl-test2') node3 = self.dtb.GetNode('/spl-test3') @@ -426,7 +436,13 @@ class TestProp(unittest.TestCase): self.assertEqual(['\x09', '\x0a', '\x0b', '\x0c', '\x0d', '\x0e', '\x0f', '\x10', '\0'], prop3.value) - # Similarly for a string array + def test_widen_more(self): + """More tests of widening values""" + node2 = self.dtb.GetNode('/spl-test2') + node3 = self.dtb.GetNode('/spl-test3') + prop = self.node.props['intval'] + + # Test widening a single string into a string array prop = self.node.props['stringval'] prop2 = node2.props['stringarray'] self.assertFalse(isinstance(prop.value, list)) @@ -466,7 +482,7 @@ class TestProp(unittest.TestCase): self.assertTrue(isinstance(prop.value, list)) self.assertEqual(1, len(prop.value)) - def testAdd(self): + def test_add(self): """Test adding properties""" self.fdt.pack() # This function should automatically expand the device tree @@ -485,12 +501,12 @@ class TestProp(unittest.TestCase): # This should fail since it would need to increase the device-tree size self.node.AddZeroProp('four') - with self.assertRaises(libfdt.FdtException) as e: + with self.assertRaises(libfdt.FdtException) as exc: self.dtb.Sync(auto_resize=False) - self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + self.assertIn('FDT_ERR_NOSPACE', str(exc.exception)) self.dtb.Sync(auto_resize=True) - def testAddMore(self): + def test_add_more(self): """Test various other methods for adding and setting properties""" self.node.AddZeroProp('one') self.dtb.Sync(auto_resize=True) @@ -516,9 +532,9 @@ class TestProp(unittest.TestCase): self.fdt.pack() self.node.SetString('string', val + 'x') - with self.assertRaises(libfdt.FdtException) as e: + with self.assertRaises(libfdt.FdtException) as exc: self.dtb.Sync(auto_resize=False) - self.assertIn('FDT_ERR_NOSPACE', str(e.exception)) + self.assertIn('FDT_ERR_NOSPACE', str(exc.exception)) self.node.SetString('string', val[:-1]) prop = self.node.props['string'] @@ -565,7 +581,8 @@ class TestProp(unittest.TestCase): new_offset = self.fdt.path_offset('/spl-test', libfdt.QUIET_NOTFOUND) self.assertEqual(-libfdt.NOTFOUND, new_offset) - def testFromData(self): + def test_from_data(self): + """Test creating an FDT from data""" dtb2 = fdt.Fdt.FromData(self.dtb.GetContents()) self.assertEqual(dtb2.GetContents(), self.dtb.GetContents()) @@ -573,28 +590,28 @@ class TestProp(unittest.TestCase): self.dtb.Sync(auto_resize=True) self.assertTrue(dtb2.GetContents() != self.dtb.GetContents()) - def testMissingSetInt(self): + def test_missing_set_int(self): """Test handling of a missing property with SetInt""" - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: self.node.SetInt('one', 1) self.assertIn("node '/spl-test': Missing property 'one'", - str(e.exception)) + str(exc.exception)) - def testMissingSetData(self): + def test_missing_set_data(self): """Test handling of a missing property with SetData""" - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: self.node.SetData('one', b'data') self.assertIn("node '/spl-test': Missing property 'one'", - str(e.exception)) + str(exc.exception)) - def testMissingSetString(self): + def test_missing_set_string(self): """Test handling of a missing property with SetString""" - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: self.node.SetString('one', 1) self.assertIn("node '/spl-test': Missing property 'one'", - str(e.exception)) + str(exc.exception)) - def testGetFilename(self): + def test_get_filename(self): """Test the dtb filename can be provided""" self.assertEqual(tools.get_output_filename('source.dtb'), self.dtb.GetFilename()) @@ -619,38 +636,42 @@ class TestFdtUtil(unittest.TestCase): self.dtb = fdt.FdtScan(find_dtb_file('dtoc_test_simple.dts')) self.node = self.dtb.GetNode('/spl-test') - def testGetInt(self): + def test_get_int(self): + """Test getting an int from a node""" 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: + with self.assertRaises(ValueError) as exc: fdt_util.GetInt(self.node, 'intarray') self.assertIn("property 'intarray' has list value: expecting a single " - 'integer', str(e.exception)) + 'integer', str(exc.exception)) - def testGetInt64(self): + def test_get_int64(self): + """Test getting a 64-bit int from a node""" self.assertEqual(0x123456789abcdef0, fdt_util.GetInt64(self.node, 'int64val')) self.assertEqual(3, fdt_util.GetInt64(self.node, 'missing', 3)) - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: fdt_util.GetInt64(self.node, 'intarray') self.assertIn( "property 'intarray' should be a list with 2 items for 64-bit values", - str(e.exception)) + str(exc.exception)) - def testGetString(self): + def test_get_string(self): + """Test getting a string from a node""" self.assertEqual('message', fdt_util.GetString(self.node, 'stringval')) self.assertEqual('test', fdt_util.GetString(self.node, 'missing', 'test')) self.assertEqual('', fdt_util.GetString(self.node, 'boolval')) - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: self.assertEqual(3, fdt_util.GetString(self.node, 'stringarray')) self.assertIn("property 'stringarray' has list value: expecting a " - 'single string', str(e.exception)) + 'single string', str(exc.exception)) - def testGetStringList(self): + def test_get_string_list(self): + """Test getting a string list from a node""" self.assertEqual(['message'], fdt_util.GetStringList(self.node, 'stringval')) self.assertEqual( @@ -660,7 +681,8 @@ class TestFdtUtil(unittest.TestCase): fdt_util.GetStringList(self.node, 'missing', ['test'])) self.assertEqual([], fdt_util.GetStringList(self.node, 'boolval')) - def testGetArgs(self): + def test_get_args(self): + """Test getting arguments from a node""" node = self.dtb.GetNode('/orig-node') self.assertEqual(['message'], fdt_util.GetArgs(self.node, 'stringval')) self.assertEqual( @@ -679,44 +701,48 @@ class TestFdtUtil(unittest.TestCase): "Node '/spl-test': Expected property 'missing'", str(exc.exception)) - def testGetBool(self): + def test_get_bool(self): + """Test getting a bool from a node""" 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 testGetByte(self): + def test_get_byte(self): + """Test getting a byte from a node""" self.assertEqual(5, fdt_util.GetByte(self.node, 'byteval')) self.assertEqual(3, fdt_util.GetByte(self.node, 'missing', 3)) - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: fdt_util.GetByte(self.node, 'longbytearray') self.assertIn("property 'longbytearray' has list value: expecting a " - 'single byte', str(e.exception)) + 'single byte', str(exc.exception)) - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: fdt_util.GetByte(self.node, 'intval') self.assertIn("property 'intval' has length 4, expecting 1", - str(e.exception)) + str(exc.exception)) - def testGetBytes(self): + def test_get_bytes(self): + """Test getting multiple bytes from a node""" self.assertEqual(bytes([5]), fdt_util.GetBytes(self.node, 'byteval', 1)) self.assertEqual(None, fdt_util.GetBytes(self.node, 'missing', 3)) self.assertEqual( bytes([3]), fdt_util.GetBytes(self.node, 'missing', 3, bytes([3]))) - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError) as exc: fdt_util.GetBytes(self.node, 'longbytearray', 7) self.assertIn( "Node 'spl-test' property 'longbytearray' has length 9, expecting 7", - str(e.exception)) + str(exc.exception)) self.assertEqual( bytes([0, 0, 0, 1]), fdt_util.GetBytes(self.node, 'intval', 4)) self.assertEqual( bytes([3]), fdt_util.GetBytes(self.node, 'missing', 3, bytes([3]))) - def testGetPhandleList(self): + def test_get_phandle_list(self): + """Test getting a list of phandles from a node""" dtb = fdt.FdtScan(find_dtb_file('dtoc_test_phandle.dts')) node = dtb.GetNode('/phandle-source2') self.assertEqual([1], fdt_util.GetPhandleList(node, 'clocks')) @@ -725,14 +751,16 @@ class TestFdtUtil(unittest.TestCase): fdt_util.GetPhandleList(node, 'clocks')) self.assertEqual(None, fdt_util.GetPhandleList(node, 'missing')) - def testGetDataType(self): + def test_get_data_type(self): + """Test getting a value of a particular type from a node""" self.assertEqual(1, fdt_util.GetDatatype(self.node, 'intval', int)) self.assertEqual('message', fdt_util.GetDatatype(self.node, 'stringval', str)) - with self.assertRaises(ValueError) as e: + with self.assertRaises(ValueError): self.assertEqual(3, fdt_util.GetDatatype(self.node, 'boolval', bool)) - def testFdtCellsToCpu(self): + def test_fdt_cells_to_cpu(self): + """Test getting cells with the correct endianness""" 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)) @@ -749,12 +777,12 @@ class TestFdtUtil(unittest.TestCase): 2)) self.assertEqual(0x12345678, fdt_util.fdt_cells_to_cpu(val, 1)) - def testEnsureCompiled(self): + def test_ensure_compiled(self): """Test a degenerate case of this function (file already compiled)""" dtb = fdt_util.EnsureCompiled(find_dtb_file('dtoc_test_simple.dts')) self.assertEqual(dtb, fdt_util.EnsureCompiled(dtb)) - def testEnsureCompiledTmpdir(self): + def test_ensure_compiled_tmpdir(self): """Test providing a temporary directory""" try: old_outdir = tools.outdir From be43a35bff17550fa707795a06eaed6114eb1742 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Thu, 28 Jul 2022 11:19:15 +0100 Subject: [PATCH 7/7] boot: allow bootmeth-distro without CONFIG_NET Remove the dependency on CMD_PXE from BOOTMETH_DISTRO by introducing a new hidden kconfig symbol to control whether pxe_utils is compiled, allowing bootstd's distro method to be compiled without needing networking support enabled. Signed-off-by: John Keeping Reviewed-by: Tom Rini Reviewed-by: Simon Glass Correct build errors when CMD_BOOTM is not enabled: Signed-off-by: Simon Glass --- boot/Kconfig | 8 +++++++- boot/Makefile | 3 +-- boot/pxe_utils.c | 3 ++- cmd/Kconfig | 4 ++-- include/command.h | 2 +- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/boot/Kconfig b/boot/Kconfig index eddb0c6b31..a294ad769e 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -294,6 +294,12 @@ endif # SPL endif # FIT +config PXE_UTILS + bool + select MENU + help + Utilities for parsing PXE file formats. + config BOOTSTD bool "Standard boot support" default y @@ -345,7 +351,7 @@ config BOOTSTD_BOOTCOMMAND config BOOTMETH_DISTRO bool "Bootdev support for distro boot" - depends on CMD_PXE + select PXE_UTILS default y help Enables support for distro boot using bootdevs. This makes the diff --git a/boot/Makefile b/boot/Makefile index a70674259c..124065a03f 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -10,8 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o bootm_os.o obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o -obj-$(CONFIG_CMD_PXE) += pxe_utils.o -obj-$(CONFIG_CMD_SYSBOOT) += pxe_utils.o +obj-$(CONFIG_PXE_UTILS) += pxe_utils.o endif diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index defbe465e4..a364fa8bb5 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -736,7 +736,8 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) kernel_addr_r = genimg_get_kernel_addr(kernel_addr); buf = map_sysmem(kernel_addr_r, 0); /* Try bootm for legacy and FIT format image */ - if (genimg_get_format(buf) != IMAGE_FORMAT_INVALID) + if (genimg_get_format(buf) != IMAGE_FORMAT_INVALID && + IS_ENABLED(CONFIG_CMD_BOOTM)) do_bootm(ctx->cmdtp, 0, bootm_argc, bootm_argv); /* Try booting an AArch64 Linux kernel image */ else if (IS_ENABLED(CONFIG_CMD_BOOTI)) diff --git a/cmd/Kconfig b/cmd/Kconfig index 3625ff2a50..7d19706a8e 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1825,7 +1825,7 @@ config CMD_ETHSW config CMD_PXE bool "pxe" - select MENU + select PXE_UTILS help Boot image via network using PXE protocol @@ -2007,7 +2007,7 @@ config CMD_SOUND config CMD_SYSBOOT bool "sysboot" - select MENU + select PXE_UTILS help Boot image via local extlinux.conf file diff --git a/include/command.h b/include/command.h index 44c91f655d..7fac7e02d4 100644 --- a/include/command.h +++ b/include/command.h @@ -148,9 +148,9 @@ int cmd_get_data_size(char *arg, int default_size); int do_bootd(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); #endif -#ifdef CONFIG_CMD_BOOTM int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); +#ifdef CONFIG_CMD_BOOTM int bootm_maybe_autostart(struct cmd_tbl *cmdtp, const char *cmd); #else static inline int bootm_maybe_autostart(struct cmd_tbl *cmdtp, const char *cmd)