[PATCH] dtoc: Correct dtoc output when testing

* [PATCH] dtoc: Correct dtoc output when testing
@ 2021-04-26 20:19 Simon Glass
  2021-04-29 16:03 ` Simon Glass
  0 siblings, 1 reply; 2+ messages in thread
From: Simon Glass @ 2021-04-26 20:19 UTC (permalink / raw)
  To: u-boot

At present each invocation of run_steps() updates OUTPUT_FILES_COMMON,
since it does not make a copy of the dict. This is fine for a single
invocation, but for tests, run_steps() is invoked many times.

As a result it may include unwanted items from the previous run, if it
happens that a test runs twice on the same CPU. The problem has not been
noticied previously, as there are few enough tests and enough CPUs that
is is rare for the 'wrong' combination of tests to run together.

Fix this by making a copy of the dict, before updating it. Update the
tests to suit, taking account of the files that are no-longer generated.

With this fix, we no-longer generate files which are not needed for a
particular state of OF_PLATDATA_INST, so the check_instantiate() function
is not needed anymore. It has become dead code and so fails the
code-coverage test (dtoc -T). Remove it.

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

 tools/dtoc/dtb_platdata.py | 24 +-----------------
 tools/dtoc/test_dtoc.py    | 51 ++++++++++++++++----------------------
 2 files changed, 22 insertions(+), 53 deletions(-)

diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index 1374f01c707..2d42480a9a5 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -824,8 +824,6 @@ class DtbPlatdata():
         self.buf('\t},\n')
 
     def generate_uclasses(self):
-        if not self.check_instantiate(True):
-            return
         self.out('\n')
         self.out('#include <common.h>\n')
         self.out('#include <dm.h>\n')
@@ -1038,22 +1036,6 @@ class DtbPlatdata():
 
         self.out(''.join(self.get_buf()))
 
-    def check_instantiate(self, require):
-        """Check if self._instantiate is set to the required value
-
-        If not, this outputs a message into the current file
-
-        Args:
-            require: True to require --instantiate, False to require that it not
-                be enabled
-        """
-        if require != self._instantiate:
-            self.out(
-                '/* This file is not used: --instantiate was %senabled */\n' %
-                ('not ' if require else ''))
-            return False
-        return True
-
     def generate_plat(self):
         """Generate device defintions for the platform data
 
@@ -1064,8 +1046,6 @@ class DtbPlatdata():
         See the documentation in doc/driver-model/of-plat.rst for more
         information.
         """
-        if not self.check_instantiate(False):
-            return
         self.out('/* Allow use of U_BOOT_DRVINFO() in this file */\n')
         self.out('#define DT_PLAT_C\n')
         self.out('\n')
@@ -1102,8 +1082,6 @@ class DtbPlatdata():
         See the documentation in doc/driver-model/of-plat.rst for more
         information.
         """
-        if not self.check_instantiate(True):
-            return
         self.out('#include <common.h>\n')
         self.out('#include <dm.h>\n')
         self.out('#include <dt-structs.h>\n')
@@ -1216,7 +1194,7 @@ def run_steps(args, dtb_file, include_disabled, output, output_dirs, phase,
     plat.assign_seqs()
 
     # Figure out what output files we plan to generate
-    output_files = OUTPUT_FILES_COMMON
+    output_files = dict(OUTPUT_FILES_COMMON)
     if instantiate:
         output_files.update(OUTPUT_FILES_INST)
     else:
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index a05e3d9ed65..0b2805feed2 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -74,10 +74,6 @@ UCLASS_HEADER_COMMON = '''/*
  */
 '''
 
-UCLASS_HEADER = UCLASS_HEADER_COMMON + '''
-/* This file is not used: --instantiate was not enabled */
-'''
-
 # Scanner saved from a previous run of the tests (to speed things up)
 saved_scan = None
 
@@ -412,7 +408,6 @@ U_BOOT_DRVINFO(spl_test3) = {
 };
 
 '''
-    uclass_text = UCLASS_HEADER
     uclass_text_inst = '''
 
 #include <common.h>
@@ -511,15 +506,6 @@ DM_UCLASS_INST(testfdt) = {
 	},
 };
 
-'''
-    device_text = '''/*
- * DO NOT MODIFY
- *
- * Declares the DM_DEVICE_INST() records.
- * This was generated by dtoc from a .dtb (device tree binary) file.
- */
-
-/* This file is not used: --instantiate was not enabled */
 '''
     device_text_inst = '''/*
  * DO NOT MODIFY
@@ -833,8 +819,7 @@ DM_DEVICE_INST(test0) = {
         self.run_test(['all'], dtb_file, output)
         data = tools.ReadFile(output, binary=False)
         self._check_strings(
-            self.decl_text + self.device_text + self.platdata_text +
-            self.struct_text + self.uclass_text, data)
+            self.decl_text + self.platdata_text + self.struct_text, data)
 
     def test_driver_alias(self):
         """Test output from a device tree file with a driver alias"""
@@ -1537,8 +1522,7 @@ U_BOOT_DRVINFO(spl_test2) = {
         self.run_test(['all'], dtb_file, output)
         data = tools.ReadFile(output, binary=False)
         self._check_strings(
-            self.decl_text + self.device_text + self.platdata_text +
-            self.struct_text + self.uclass_text, data)
+            self.decl_text + self.platdata_text + self.struct_text, data)
 
     def test_no_command(self):
         """Test running dtoc without a command"""
@@ -1566,8 +1550,7 @@ U_BOOT_DRVINFO(spl_test2) = {
         self.assertIn("Must specify either output or output_dirs, not both",
                       str(exc.exception))
 
-    def test_output_dirs(self):
-        """Test outputting files to a directory"""
+    def check_output_dirs(self, instantiate):
         # Remove the directory so that files from other tests are not there
         tools._RemoveOutputDir()
         tools.PrepareOutputDir(None)
@@ -1579,14 +1562,30 @@ U_BOOT_DRVINFO(spl_test2) = {
         self.assertEqual(2, len(fnames))
 
         dtb_platdata.run_steps(
-            ['all'], dtb_file, False, None, [outdir], None, False,
+            ['all'], dtb_file, False, None, [outdir], None, instantiate,
             warning_disabled=True, scan=copy_scan())
         fnames = glob.glob(outdir + '/*')
-        self.assertEqual(7, len(fnames))
+        return fnames
+
+    def test_output_dirs(self):
+        """Test outputting files to a directory"""
+        fnames = self.check_output_dirs(False)
+        self.assertEqual(5, len(fnames))
 
         leafs = set(os.path.basename(fname) for fname in fnames)
         self.assertEqual(
             {'dt-structs-gen.h', 'source.dts', 'dt-plat.c', 'source.dtb',
+             'dt-decl.h'},
+            leafs)
+
+    def test_output_dirs_inst(self):
+        """Test outputting files to a directory with instantiation"""
+        fnames = self.check_output_dirs(True)
+        self.assertEqual(6, len(fnames))
+
+        leafs = set(os.path.basename(fname) for fname in fnames)
+        self.assertEqual(
+            {'dt-structs-gen.h', 'source.dts', 'source.dtb',
              'dt-uclass.c', 'dt-decl.h', 'dt-device.c'},
             leafs)
 
@@ -1785,14 +1784,6 @@ U_BOOT_DRVINFO(spl_test2) = {
 
         self._check_strings(self.decl_text_inst, data)
 
-        self.run_test(['platdata'], dtb_file, output, True)
-        with open(output) as infile:
-            data = infile.read()
-
-        self._check_strings(C_HEADER_PRE + '''
-/* This file is not used: --instantiate was enabled */
-''', data)
-
         self.run_test(['uclass'], dtb_file, output, True)
         with open(output) as infile:
             data = infile.read()
-- 
2.31.1.498.g6c1eba8ee3d-goog

^ permalink raw reply related	[flat|nested] 2+ messages in thread