Re: [PATCH] binman: Allow FIT binaries to have missing external blobs

On 01/09/2020 13:36, Simon Glass wrote:

Hi Alper,

I found that evb-rk3288 fails to build with the final patch from your series. See u-boot-dm/testing for the tree.

I had seen your patch and assumed that nothing was already using the allow-missing functionality in its FITs (since you're adding it now). Looks like the version in your tree (aa3f219e) succeeds in the pipeline, so I think there's nothing for me to do now. Thanks for fixing it.

At present if a FIT references a missing external blob, binman reports an error, even if the image is supposed to allow this.

Propagate this setting down to the child sections of the FIT, so that the behaviour is consistent.

This is a fix-up patch to:

binman: Build FIT image subentries with the section etype

and will be squashed into that.

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

tools/binman/etype/fit.py | 15 ++++++++ tools/binman/ftest.py | 8 +++++ tools/binman/test/168_fit_missing_blob.dts | 41 ++++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 tools/binman/test/168_fit_missing_blob.dts

diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index acba53aa92c..615b2fd8778 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -169,3 +169,18 @@ class Entry_fit(Entry): fdt.Sync(auto_resize=True) data = fdt.GetContents() return data

• def CheckMissing(self, missing_list):

•    """Check if any entries in this FIT have missing external blobs
  

•    If there are missing blobs, the entries are added to the list
  

•    Args:
  

•        missing_list: List of Entry objects to be added to
  

•    """
  

•    for path, section in self._fit_sections.items():
  

•        section.CheckMissing(missing_list)
  

• def SetAllowMissing(self, allow_missing):

•    for section in self._fit_sections.values():
  

•        section.SetAllowMissing(allow_missing)
  

I saw GetAllowMissing() in section.py, doesn't this also need it?

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 762535b5a74..e672967dbaa 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3515,5 +3515,13 @@ class TestFunctional(unittest.TestCase): U_BOOT_DTB_DATA) self.assertEqual(expected, data)

• def testFitExtblobMissingOk(self):

•    """Test a FIT with a missing external blob that is allowed"""
  

•    with test_util.capture_sys_output() as (stdout, stderr):
  

•        self._DoTestFile('168_fit_missing_blob.dts',
  

•                         allow_missing=True)
  

•    err = stderr.getvalue()
  

•    self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")
  

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/168_fit_missing_blob.dts b/tools/binman/test/168_fit_missing_blob.dts new file mode 100644 index 00000000000..e007aa41d8a --- /dev/null +++ b/tools/binman/test/168_fit_missing_blob.dts @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: GPL-2.0+

+/dts-v1/;

+/ {

• #address-cells = <1>;
• #size-cells = <1>;
• binman {

• u-boot {
  

• };
  

• fit {
  

• 	description = "test-desc";
  

• 	#address-cells = <1>;
  

• 	fit,fdt-list = "of-list";
  

• 	images {
  

• 		kernel {
  

• 			description = "ATF BL31";
  

• 			type = "kernel";
  

• 			arch = "ppc";
  

• 			os = "linux";
  

• 			compression = "gzip";
  

• 			load = <00000000>;
  

• 			entry = <00000000>;
  

• 			hash-1 {
  

• 				algo = "crc32";
  

• 			};
  

• 			hash-2 {
  

• 				algo = "sha1";
  

• 			};
  

• 			blob-ext {
  

• 				filename = "missing";
  

• 			};
  

• 		};
  

• 	};
  

• };
  

• u-boot-nodtb {
  

• };
  

• };

+};

Anyway, feel free to add if you need to:

Reviewed-by: Alper Nebi Yasak alpernebiyasak@gmail.com

(or Acked-by, or neither)