Re: [PATCH 3/5] binman: Support adding a U-Boot environment

2020-09-04 Thread Andy Shevchenko
On Thu, Sep 03, 2020 at 07:28:54PM -0600, Simon Glass wrote:
> In some cases it is useful to include a U-Boot environment region in an
> image. This allows the board to start up with an environment ready to go.
> 
> Add a new entry type for this. The input is a text file containing the
> environment entries, one per line, in the format:
> 
>var=value

Acked-by: Andy Shevchenko 

> Signed-off-by: Simon Glass 
> ---
> 
>  tools/binman/etype/u_boot_env.py| 42 +
>  tools/binman/ftest.py   | 31 ++
>  tools/binman/test/174_env.dts   | 20 
>  tools/binman/test/175_env_no_size.dts   | 19 +++
>  tools/binman/test/176_env_too_small.dts | 20 
>  5 files changed, 132 insertions(+)
>  create mode 100644 tools/binman/etype/u_boot_env.py
>  create mode 100644 tools/binman/test/174_env.dts
>  create mode 100644 tools/binman/test/175_env_no_size.dts
>  create mode 100644 tools/binman/test/176_env_too_small.dts
> 
> diff --git a/tools/binman/etype/u_boot_env.py 
> b/tools/binman/etype/u_boot_env.py
> new file mode 100644
> index 000..1694c2a6eef
> --- /dev/null
> +++ b/tools/binman/etype/u_boot_env.py
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2018 Google, Inc
> +# Written by Simon Glass 
> +#
> +
> +import struct
> +import zlib
> +
> +from binman.etype.blob import Entry_blob
> +from dtoc import fdt_util
> +from patman import tools
> +
> +class Entry_u_boot_env(Entry_blob):
> +"""An entry which contains a U-Boot environment
> +
> +Properties / Entry arguments:
> +- filename: File containing the environment text, with each line in 
> the
> +form var=value
> +"""
> +def __init__(self, section, etype, node):
> +super().__init__(section, etype, node)
> +
> +def ReadNode(self):
> +super().ReadNode()
> +if self.size is None:
> +self.Raise("'u-boot-env' entry must have a size property")
> +self.fill_value = fdt_util.GetByte(self._node, 'fill-byte', 0)
> +
> +def ReadBlobContents(self):
> +indata = tools.ReadFile(self._pathname)
> +data = b''
> +for line in indata.splitlines():
> +data += line + b'\0'
> +data += b'\0';
> +pad = self.size - len(data) - 5
> +if pad < 0:
> +self.Raise("'u-boot-env' entry too small to hold data (need %#x 
> more bytes)" % -pad)
> +data += tools.GetBytes(self.fill_value, pad)
> +crc = zlib.crc32(data)
> +buf = struct.pack(' +self.SetContents(buf)
> +return True
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 91225459162..b771b9d5df7 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -77,6 +77,7 @@ FSP_T_DATA= b'fsp_t'
>  ATF_BL31_DATA = b'bl31'
>  TEST_FDT1_DATA= b'fdt1'
>  TEST_FDT2_DATA= b'test-fdt2'
> +ENV_DATA  = b'var1=1\nvar2="2"'
>  
>  # Subdirectory of the input dir to use to put test FDTs
>  TEST_FDT_SUBDIR   = 'fdts'
> @@ -181,6 +182,8 @@ class TestFunctional(unittest.TestCase):
>  TestFunctional._MakeInputFile('%s/test-fdt2.dtb' % TEST_FDT_SUBDIR,
>TEST_FDT2_DATA)
>  
> +TestFunctional._MakeInputFile('env.txt', ENV_DATA)
> +
>  # Travis-CI may have an old lz4
>  cls.have_lz4 = True
>  try:
> @@ -3715,5 +3718,33 @@ class TestFunctional(unittest.TestCase):
>  self.assertIn("Filename 'missing' not found in input path",
>str(e.exception))
>  
> +def testEnvironment(self):
> +"""Test adding a U-Boot environment"""
> +data = self._DoReadFile('174_env.dts')
> +self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)])
> +self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):])
> +env = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)]
> +self.assertEqual(b'\x1b\x97\x22\x7c\x01var1=1\0var2="2"\0\0\xff\xff',
> + env)
> +
> +def testEnvironmentNoSize(self):
> +"""Test that a missing 'size' property is detected"""
> +with self.assertRaises(ValueError) as e:
> +data = self._DoTestFile('175_env_no_size.dts')
> +self.assertIn("'u-boot-env' entry must have a size property",
> +  str(e.exception))
> +
> +def testEnvironmentTooSmall(self):
> +"""Test handling of an environment that does not fit"""
> +with self.assertRaises(ValueError) as e:
> +data = self._DoTestFile('176_env_too_small.dts')
> +
> +# checksum, start byte, environment with \0 terminator, final \0
> +need = 4 + 1 + len(ENV_DATA) + 1 + 1
> +short = need - 0x8
> +self.assertIn("too small to hold data (need %#x more bytes)" % short,
> +  str(e.exception))
> +
> +
>  if __name__ == 

[PATCH 3/5] binman: Support adding a U-Boot environment

2020-09-03 Thread Simon Glass
In some cases it is useful to include a U-Boot environment region in an
image. This allows the board to start up with an environment ready to go.

Add a new entry type for this. The input is a text file containing the
environment entries, one per line, in the format:

   var=value

Signed-off-by: Simon Glass 
---

 tools/binman/etype/u_boot_env.py| 42 +
 tools/binman/ftest.py   | 31 ++
 tools/binman/test/174_env.dts   | 20 
 tools/binman/test/175_env_no_size.dts   | 19 +++
 tools/binman/test/176_env_too_small.dts | 20 
 5 files changed, 132 insertions(+)
 create mode 100644 tools/binman/etype/u_boot_env.py
 create mode 100644 tools/binman/test/174_env.dts
 create mode 100644 tools/binman/test/175_env_no_size.dts
 create mode 100644 tools/binman/test/176_env_too_small.dts

diff --git a/tools/binman/etype/u_boot_env.py b/tools/binman/etype/u_boot_env.py
new file mode 100644
index 000..1694c2a6eef
--- /dev/null
+++ b/tools/binman/etype/u_boot_env.py
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (c) 2018 Google, Inc
+# Written by Simon Glass 
+#
+
+import struct
+import zlib
+
+from binman.etype.blob import Entry_blob
+from dtoc import fdt_util
+from patman import tools
+
+class Entry_u_boot_env(Entry_blob):
+"""An entry which contains a U-Boot environment
+
+Properties / Entry arguments:
+- filename: File containing the environment text, with each line in the
+form var=value
+"""
+def __init__(self, section, etype, node):
+super().__init__(section, etype, node)
+
+def ReadNode(self):
+super().ReadNode()
+if self.size is None:
+self.Raise("'u-boot-env' entry must have a size property")
+self.fill_value = fdt_util.GetByte(self._node, 'fill-byte', 0)
+
+def ReadBlobContents(self):
+indata = tools.ReadFile(self._pathname)
+data = b''
+for line in indata.splitlines():
+data += line + b'\0'
+data += b'\0';
+pad = self.size - len(data) - 5
+if pad < 0:
+self.Raise("'u-boot-env' entry too small to hold data (need %#x 
more bytes)" % -pad)
+data += tools.GetBytes(self.fill_value, pad)
+crc = zlib.crc32(data)
+buf = struct.pack(';
+   #size-cells = <1>;
+
+   binman {
+   u-boot {
+   };
+   u-boot-env {
+   filename = "env.txt";
+   size = <0x18>;
+   fill-byte = [ff];
+   };
+   u-boot-nodtb {
+   };
+   };
+};
diff --git a/tools/binman/test/175_env_no_size.dts 
b/tools/binman/test/175_env_no_size.dts
new file mode 100644
index 000..267acd15491
--- /dev/null
+++ b/tools/binman/test/175_env_no_size.dts
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   u-boot {
+   };
+   u-boot-env {
+   filename = "env.txt";
+   fill-byte = [ff];
+   };
+   u-boot-nodtb {
+   };
+   };
+};
diff --git a/tools/binman/test/176_env_too_small.dts 
b/tools/binman/test/176_env_too_small.dts
new file mode 100644
index 000..2db8d054639
--- /dev/null
+++ b/tools/binman/test/176_env_too_small.dts
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   u-boot {
+   };
+   u-boot-env {
+   filename = "env.txt";
+   size = <0x8>;
+   fill-byte = [ff];
+   };
+   u-boot-nodtb {
+   };
+   };
+};
-- 
2.28.0.526.ge36021eeef-goog