[PATCH v5 2/8] binman: allow user-defined filenames for mkimage entry

[Simon Glass]

On Fri, 26 Aug 2022 at 09:37, Quentin Schulz <foss+uboot at 0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz at theobroma-systems.com>
>
> mkimage entry currently creates a file whose name is derived from the
> section name containing said entry.
>
> Let's allow the user to define a filename for the mkimage-generated
> binary by using the 'filename' DT property.
>
> Cc: Quentin Schulz <foss+uboot at 0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz at theobroma-systems.com>
> ---
>
> v5:
>  - updated unit test filename,
>  - removed _testing section in unit test,
>  - removed -n arg in unit test,
>  - removed size property in binman node,
>
> v4:
>  - added binman test,
>  - fixed >80 chars-long line,
>
> added in v3
>
>  tools/binman/etype/mkimage.py              | 11 ++++++++---
>  tools/binman/ftest.py                      |  7 +++++++
>  tools/binman/test/242_mkimage_filename.dts | 18 ++++++++++++++++++
>  3 files changed, 33 insertions(+), 3 deletions(-)
>  create mode 100644 tools/binman/test/242_mkimage_filename.dts

Reviewed-by: Simon Glass <sjg at chromium.org>

nit below

>
> diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
> index 5f4bc6fa3c..c2288c48ee 100644
> --- a/tools/binman/etype/mkimage.py
> +++ b/tools/binman/etype/mkimage.py
> @@ -21,11 +21,13 @@ class Entry_mkimage(Entry):
>          - multiple-data-files: boolean to tell binman to pass all files as
>            datafiles to mkimage instead of creating a temporary file the result
>            of datafiles concatenation
> +        - filename: filename of output binary generated by mkimage
>
>      The data passed to mkimage via the -d flag is collected from subnodes of the
>      mkimage node, e.g.::
>
>          mkimage {
> +            filename = "imximage.bin";
>              args = "-n test -T imximage";
>
>              u-boot-spl {
> @@ -38,8 +40,9 @@ class Entry_mkimage(Entry):
>          mkimage -d <data_file> -n test -T imximage <output_file>
>
>      The output from mkimage then becomes part of the image produced by
> -    binman. If you need to put multiple things in the data file, you can use
> -    a section, or just multiple subnodes like this::
> +    binman but also is written into `imximage.bin` file. If you need to put
> +    multiple things in the data file, you can use a section, or just multiple
> +    subnodes like this::
>
>          mkimage {
>              args = "-n test -T imximage";
> @@ -121,6 +124,7 @@ class Entry_mkimage(Entry):
>          self._multiple_data_files = fdt_util.GetBool(self._node, 'multiple-data-files')
>          self._mkimage_entries = OrderedDict()
>          self._imagename = None
> +        self._filename = fdt_util.GetString(self._node, 'filename')
>          self.align_default = None
>
>      def ReadNode(self):
> @@ -164,7 +168,8 @@ class Entry_mkimage(Entry):
>                  [self._imagename], 'mkimage-n', 1024)
>              if image_data is None:
>                  return False
> -        output_fname = tools.get_output_filename('mkimage-out.%s' % uniq)
> +        outfile = self._filename if self._filename else 'mkimage-out.%s' % uniq
> +        output_fname = tools.get_output_filename(outfile)
>
>          args = ['-d', input_fname]
>          if self._data_to_imagename:
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 091692ef93..da0c7299ac 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -5834,6 +5834,13 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
>          expect += U_BOOT_SPL_DATA
>          self.assertEqual(expect, data[-len(expect):])
>
> +    def testMkimageFilename(self):
> +        """Test using mkimage to build a binary with a filename"""
> +        retcode = self._DoTestFile('242_mkimage_filename.dts')
> +        self.assertEqual(0, retcode)
> +        fname = tools.get_output_filename('mkimage-test.bin')
> +        self.assertTrue(os.path.exists(fname))
> +

Please put the new test at the end.

>      def testCompressDtbPrependInvalid(self):
>          """Test that invalid header is detected"""
>          with self.assertRaises(ValueError) as e:
> diff --git a/tools/binman/test/242_mkimage_filename.dts b/tools/binman/test/242_mkimage_filename.dts
> new file mode 100644
> index 0000000000..4483790ae8
> --- /dev/null
> +++ b/tools/binman/test/242_mkimage_filename.dts
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               mkimage {
> +                       filename = "mkimage-test.bin";
> +                       args = "-T script";
> +
> +                       u-boot-spl {
> +                       };
> +               };
> +       };
> +};
> --
> 2.37.2
>

Regards,
Simon