diff mbox series

[v4,02/13] binman: Add length header attribute to dtb entry

Message ID 20220816084210.14972-2-stefan.herbrechtsmeier-oss@weidmueller.com
State Changes Requested
Delegated to: Simon Glass
Headers show
Series [v4,01/13] binman: Skip elf tests if python elftools is not available | expand

Commit Message

Stefan Herbrechtsmeier Aug. 16, 2022, 8:41 a.m. UTC
From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Add an optional length header attribute to the device tree blob entry
class based on the compressed data header from the utilities to compress
and decompress data.

If needed the header could be enabled with the following
attribute beside the compress attribute:
  prepend = "length";

The header was introduced as part of commit eb0f4a4cb402 ("binman:
Support replacing data in a cbfs") to allow device tree entries to be
larger than the compressed contents. Regarding the commit "this is
necessary to cope with a compressed device tree being updated in such a
way that it shrinks after the entry size is already set (an obscure
case)". This case need to be fixed without influence any compressed data
by itself.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

---

(no changes since v3)

Changes in v3:
- Move comp_util.py changes (drop with_header) into separate commits.

Changes in v2:
- Reworked to make the feature optional.

 tools/binman/entries.rst                      |  3 +++
 tools/binman/etype/blob_dtb.py                | 24 +++++++++++++++++++
 tools/binman/ftest.py                         | 22 +++++++++++++++++
 .../test/235_compress_prepend_length_dtb.dts  | 17 +++++++++++++
 4 files changed, 66 insertions(+)
 create mode 100644 tools/binman/test/235_compress_prepend_length_dtb.dts

Comments

Simon Glass Aug. 16, 2022, 11:48 a.m. UTC | #1
Hi Stefan,

On Tue, 16 Aug 2022 at 02:42, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Add an optional length header attribute to the device tree blob entry
> class based on the compressed data header from the utilities to compress
> and decompress data.
>
> If needed the header could be enabled with the following
> attribute beside the compress attribute:
>   prepend = "length";
>
> The header was introduced as part of commit eb0f4a4cb402 ("binman:
> Support replacing data in a cbfs") to allow device tree entries to be
> larger than the compressed contents. Regarding the commit "this is
> necessary to cope with a compressed device tree being updated in such a
> way that it shrinks after the entry size is already set (an obscure
> case)". This case need to be fixed without influence any compressed data
> by itself.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> ---
>
> (no changes since v3)

This looks fine except that it drops test coverage (binman test -T
should be 100%). Please can you check it?

Regards,
Simon
diff mbox series

Patch

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 782bff1f8d..c9336d1bb4 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -216,6 +216,9 @@  This is a blob containing a device tree. The contents of the blob are
 obtained from the list of available device-tree files, managed by the
 'state' module.
 
+Additional Properties / Entry arguments:
+    - prepend: Header type to use:
+        length: 32-bit length header
 
 
 .. _etype_blob_ext:
diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py
index 4159e3032a..4fd2ecda83 100644
--- a/tools/binman/etype/blob_dtb.py
+++ b/tools/binman/etype/blob_dtb.py
@@ -7,6 +7,8 @@ 
 
 from binman.entry import Entry
 from binman.etype.blob import Entry_blob
+from dtoc import fdt_util
+import struct
 
 # This is imported if needed
 state = None
@@ -17,6 +19,9 @@  class Entry_blob_dtb(Entry_blob):
     This is a blob containing a device tree. The contents of the blob are
     obtained from the list of available device-tree files, managed by the
     'state' module.
+
+    Additional attributes:
+        prepend: Header used (e.g. 'length')
     """
     def __init__(self, section, etype, node):
         # Put this here to allow entry-docs and help to work without libfdt
@@ -25,6 +30,15 @@  class Entry_blob_dtb(Entry_blob):
 
         super().__init__(section, etype, node)
 
+        self.prepend = None
+
+    def ReadNode(self):
+        super().ReadNode()
+        self.prepend = fdt_util.GetString(self._node, 'prepend')
+        if self.prepend and self.prepend not in ['length']:
+            self.Raise("Invalid prepend in '%s': '%s'" %
+                       (self._node.name, self.prepend))
+
     def ObtainContents(self):
         """Get the device-tree from the list held by the 'state' module"""
         self._filename = self.GetDefaultFilename()
@@ -35,6 +49,9 @@  class Entry_blob_dtb(Entry_blob):
         """Re-read the DTB contents so that we get any calculated properties"""
         _, indata = state.GetFdtContents(self.GetFdtEtype())
         data = self.CompressData(indata)
+        if self.prepend == 'length':
+            hdr = struct.pack('<I', len(data))
+            data = hdr + data
         return self.ProcessContentsUpdate(data)
 
     def GetFdtEtype(self):
@@ -50,6 +67,13 @@  class Entry_blob_dtb(Entry_blob):
         fname = self.GetDefaultFilename()
         return {self.GetFdtEtype(): [self, fname]}
 
+    def ReadData(self, decomp=True, alt_format=None):
+        data = super().ReadData(decomp, alt_format)
+        if self.prepend == 'length':
+            data_len = struct.unpack('<I', data[:4])[0]
+            data = data[4:4 + data_len]
+        return data
+
     def WriteData(self, data, decomp=True):
         ok = super().WriteData(data, decomp)
 
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index da9aa9e679..1b468d8e6d 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2536,6 +2536,28 @@  class TestFunctional(unittest.TestCase):
             }
         self.assertEqual(expected, props)
 
+    def testCompressPrependLengthDtb(self):
+        """Test that compress of device-tree files with length header is
+        supported
+        """
+        data = self.data = self._DoReadFileRealDtb('235_compress_prepend_length_dtb.dts')
+        self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)])
+        dtb_data = data[len(U_BOOT_DATA):]
+        comp_data_len = struct.unpack('<I', dtb_data[:4])[0]
+        comp_data = dtb_data[4:4 + comp_data_len]
+        orig = self._decompress(comp_data)
+        dtb = fdt.Fdt.FromData(orig)
+        dtb.Scan()
+        props = self._GetPropTree(dtb, ['size', 'uncomp-size'])
+        expected = {
+            'u-boot:size': len(U_BOOT_DATA),
+            'u-boot-dtb:uncomp-size': len(orig),
+            'u-boot-dtb:size': len(dtb_data),
+            'size': len(data),
+            }
+        self.assertEqual(expected, props)
+
+
     def testCbfsUpdateFdt(self):
         """Test that we can update the device tree with CBFS offset/size info"""
         self._CheckLz4()
diff --git a/tools/binman/test/235_compress_prepend_length_dtb.dts b/tools/binman/test/235_compress_prepend_length_dtb.dts
new file mode 100644
index 0000000000..a5abc60477
--- /dev/null
+++ b/tools/binman/test/235_compress_prepend_length_dtb.dts
@@ -0,0 +1,17 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		u-boot {
+		};
+		u-boot-dtb {
+			compress = "lz4";
+			prepend = "length";
+		};
+	};
+};