diff mbox series

[v2,16/28] binman: Make section padding consistent with other entries

Message ID 20201026234026.1903778-16-sjg@chromium.org
State Accepted
Commit 7d398bb1c71580da2182f0b820d91950bf4b3d24
Delegated to: Simon Glass
Headers show
Series binman: Support compression of sections | expand

Commit Message

Simon Glass Oct. 26, 2020, 11:40 p.m. UTC
At present padding of sections is inconsistent with other entry types, in
that different pad bytes are used.

When a normal entry is padded by its parent, the parent's pad byte is
used. But for sections, the section's pad byte is used.

Adjust logic to always do this the same way.

Note there is still a special case in entry_Section.GetPaddedData() where
an image is padded with the pad byte of the top-level section. This is
necessary since otherwise there would be no way to set the pad byte of
the image, without adding a top-level section to every image.

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

Changes in v2:
- Renumber the test .dts files to make space for three inserted earlier

 tools/binman/etype/section.py           |  4 +--
 tools/binman/ftest.py                   | 23 +++++++++++++++++
 tools/binman/test/180_section_pad.dts   | 27 ++++++++++++++++++++
 tools/binman/test/181_section_align.dts | 34 +++++++++++++++++++++++++
 4 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/test/180_section_pad.dts
 create mode 100644 tools/binman/test/181_section_align.dts

Comments

Simon Glass Oct. 30, 2020, 3:33 a.m. UTC | #1
At present padding of sections is inconsistent with other entry types, in
that different pad bytes are used.

When a normal entry is padded by its parent, the parent's pad byte is
used. But for sections, the section's pad byte is used.

Adjust logic to always do this the same way.

Note there is still a special case in entry_Section.GetPaddedData() where
an image is padded with the pad byte of the top-level section. This is
necessary since otherwise there would be no way to set the pad byte of
the image, without adding a top-level section to every image.

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

Changes in v2:
- Renumber the test .dts files to make space for three inserted earlier

 tools/binman/etype/section.py           |  4 +--
 tools/binman/ftest.py                   | 23 +++++++++++++++++
 tools/binman/test/180_section_pad.dts   | 27 ++++++++++++++++++++
 tools/binman/test/181_section_align.dts | 34 +++++++++++++++++++++++++
 4 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 tools/binman/test/180_section_pad.dts
 create mode 100644 tools/binman/test/181_section_align.dts

Applied to u-boot-dm, thanks!
diff mbox series

Patch

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 7cbb50057a3..c423a22c80f 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -165,14 +165,14 @@  class Entry_section(Entry):
         data = b''
         # Handle padding before the entry
         if entry.pad_before:
-            data += tools.GetBytes(pad_byte, entry.pad_before)
+            data += tools.GetBytes(self._pad_byte, entry.pad_before)
 
         # Add in the actual entry data
         data += entry.GetData()
 
         # Handle padding after the entry
         if entry.pad_after:
-            data += tools.GetBytes(pad_byte, entry.pad_after)
+            data += tools.GetBytes(self._pad_byte, entry.pad_after)
 
         if entry.size:
             data += tools.GetBytes(pad_byte, entry.size - len(data))
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 31e93c647fe..830b610890f 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -3886,5 +3886,28 @@  class TestFunctional(unittest.TestCase):
         self.assertEqual(len(U_BOOT_DATA), entry.size)
         self.assertEqual(U_BOOT_DATA, entry.data)
 
+    def testSectionPad(self):
+        """Testing padding with sections"""
+        data = self._DoReadFile('180_section_pad.dts')
+        expected = (tools.GetBytes(ord('&'), 3) +
+                    tools.GetBytes(ord('!'), 5) +
+                    U_BOOT_DATA +
+                    tools.GetBytes(ord('!'), 1) +
+                    tools.GetBytes(ord('&'), 2))
+        self.assertEqual(expected, data)
+
+    def testSectionAlign(self):
+        """Testing alignment with sections"""
+        data = self._DoReadFileDtb('181_section_align.dts', map=True)[0]
+        expected = (b'\0' +                         # fill section
+                    tools.GetBytes(ord('&'), 1) +   # padding to section align
+                    b'\0' +                         # fill section
+                    tools.GetBytes(ord('!'), 3) +   # padding to u-boot align
+                    U_BOOT_DATA +
+                    tools.GetBytes(ord('!'), 4) +   # padding to u-boot size
+                    tools.GetBytes(ord('!'), 4))    # padding to section size
+        self.assertEqual(expected, data)
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/180_section_pad.dts b/tools/binman/test/180_section_pad.dts
new file mode 100644
index 00000000000..7e4ebf257b8
--- /dev/null
+++ b/tools/binman/test/180_section_pad.dts
@@ -0,0 +1,27 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pad-byte = <0x26>;
+		section@0 {
+			read-only;
+
+			/* Padding for the section uses the 0x26 pad byte */
+			pad-before = <3>;
+			pad-after = <2>;
+
+			/* Set the padding byte for entries, i.e. u-boot */
+			pad-byte = <0x21>;
+
+			u-boot {
+				pad-before = <5>;
+				pad-after = <1>;
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/181_section_align.dts b/tools/binman/test/181_section_align.dts
new file mode 100644
index 00000000000..90795d131b0
--- /dev/null
+++ b/tools/binman/test/181_section_align.dts
@@ -0,0 +1,34 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		pad-byte = <0x26>;
+		fill {
+			size = <1>;
+		};
+		section@1 {
+			read-only;
+
+			/* Padding for the section uses the 0x26 pad byte */
+			align = <2>;
+			align-size = <0x10>;
+
+			/* Set the padding byte for entries, i.e. u-boot */
+			pad-byte = <0x21>;
+
+			fill {
+				size = <1>;
+			};
+
+			u-boot {
+				align = <4>;
+				align-size = <8>;
+			};
+		};
+	};
+};