Re: [U-Boot] [PATCH v2 04/29] binman: Correct operation of ObtainContents()

2018-07-09 Thread Simon Glass
On 6 July 2018 at 10:27, Simon Glass  wrote:
> 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 
> ---
>
> Changes in v2: None
>
>  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 +
>  .../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

Applied to u-boot-dm
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v2 04/29] binman: Correct operation of ObtainContents()

2018-07-06 Thread Simon Glass
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 
---

Changes in v2: None

 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 +
 .../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

diff --git a/tools/binman/bsection.py b/tools/binman/bsection.py
index 3f30f6e4fe..06a6711350 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 0b1eaefc3c..c075c3ff0d 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 139fcad51a..36b31a849f 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 3d2dea2e0d..6c397957e3 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 3a0cff7c3a..8cae7deed3 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 80eadeffab..ca9d158eef 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