binman: Don't add compression attribute for uncompressed files

cbfsutil changed to skip adding a compression attribute if there is no
compression. Adjust the binman implementation to do the same.

This mirrors commit 105cdf5625 in coreboot.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Neha Malcom Francis <n-francis@ti.com>

3 files changed, 15 insertions, 12 deletions

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
index fc56b40..92d2add 100644
--- a/tools/binman/cbfs_util.py
+++ b/tools/binman/cbfs_util.py
@@ -333,7 +333,8 @@ class CbfsFile(object):
         if self.ftype == TYPE_STAGE:
             pass
         elif self.ftype == TYPE_RAW:
-            hdr_len += ATTR_COMPRESSION_LEN
+            if self.compress:
+                hdr_len += ATTR_COMPRESSION_LEN
         elif self.ftype == TYPE_EMPTY:
             pass
         else:
@@ -369,9 +370,11 @@ class CbfsFile(object):
                 data = self.comp_bintool.compress(orig_data)
             self.memlen = len(orig_data)
             self.data_len = len(data)
-            attr = struct.pack(ATTR_COMPRESSION_FORMAT,
-                               FILE_ATTR_TAG_COMPRESSION, ATTR_COMPRESSION_LEN,
-                               self.compress, self.memlen)
+            if self.compress:
+                attr = struct.pack(ATTR_COMPRESSION_FORMAT,
+                                   FILE_ATTR_TAG_COMPRESSION,
+                                   ATTR_COMPRESSION_LEN, self.compress,
+                                   self.memlen)
         elif self.ftype == TYPE_EMPTY:
             data = tools.get_bytes(self.erase_byte, self.size)
         else:
@@ -405,7 +408,7 @@ class CbfsFile(object):
         if expected_len != actual_len:  # pragma: no cover
             # Test coverage of this is not available since this should never
             # happen. It probably indicates that get_header_len() is broken.
-            raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#d" %
+            raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#x" %
                              (self.name, expected_len, actual_len))
         return hdr + name + attr + pad + content + data, hdr_len
 
diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
index ee951d1..c3babbc 100755
--- a/tools/binman/cbfs_util_test.py
+++ b/tools/binman/cbfs_util_test.py
@@ -96,7 +96,7 @@ class TestCbfs(unittest.TestCase):
         self.assertEqual(arch, cbfs.arch)
         return cbfs
 
-    def _check_uboot(self, cbfs, ftype=cbfs_util.TYPE_RAW, offset=0x38,
+    def _check_uboot(self, cbfs, ftype=cbfs_util.TYPE_RAW, offset=0x28,
                      data=U_BOOT_DATA, cbfs_offset=None):
         """Check that the U-Boot file is as expected
 
@@ -122,7 +122,7 @@ class TestCbfs(unittest.TestCase):
         self.assertEqual(len(data), cfile.memlen)
         return cfile
 
-    def _check_dtb(self, cbfs, offset=0x38, data=U_BOOT_DTB_DATA,
+    def _check_dtb(self, cbfs, offset=0x28, data=U_BOOT_DTB_DATA,
                    cbfs_offset=None):
         """Check that the U-Boot dtb file is as expected
 
@@ -598,8 +598,8 @@ class TestCbfs(unittest.TestCase):
         data = cbw.get_data()
 
         cbfs = cbfs_util.CbfsReader(data)
-        self.assertEqual(0x38, cbfs.files['u-boot'].cbfs_offset)
-        self.assertEqual(0x78, cbfs.files['u-boot-dtb'].cbfs_offset)
+        self.assertEqual(0x28, cbfs.files['u-boot'].cbfs_offset)
+        self.assertEqual(0x68, cbfs.files['u-boot-dtb'].cbfs_offset)
 
 
 if __name__ == '__main__':
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 16156b7..2875194 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2670,9 +2670,9 @@ class TestFunctional(unittest.TestCase):
             'cbfs/u-boot:offset': 0x38,
             'cbfs/u-boot:uncomp-size': len(U_BOOT_DATA),
             'cbfs/u-boot:image-pos': 0x38,
-            'cbfs/u-boot-dtb:offset': 0xb8,
+            'cbfs/u-boot-dtb:offset': 0xa8,
             'cbfs/u-boot-dtb:size': len(U_BOOT_DATA),
-            'cbfs/u-boot-dtb:image-pos': 0xb8,
+            'cbfs/u-boot-dtb:image-pos': 0xa8,
             }, props)
 
     def testCbfsBadType(self):
@@ -2854,7 +2854,7 @@ class TestFunctional(unittest.TestCase):
 '  u-boot                  0     4  u-boot             0',
 '  section               100   %x  section          100' % section_size,
 '    cbfs                100   400  cbfs               0',
-'      u-boot            138     4  u-boot            38',
+'      u-boot            128     4  u-boot            28',
 '      u-boot-dtb        180   105  u-boot-dtb        80          3c9',
 '    u-boot-dtb          500   %x  u-boot-dtb       400          3c9' % fdt_size,
 '  fdtmap                %x   3bd  fdtmap           %x' %