[PATCH v2] binman: support mkimage separate files

[Simon Glass]

Hi Peter,

On Fri, 4 Mar 2022 at 12:56, Peter Geis <pgwipeout at gmail.com> wrote:
>
> mkimage has the ability to process two files at the same time.
> This is necessary for rk356x support as both TPL and SPL need to be
> hashed individually in the resulting header.
> It also eases support for rkspi as mkimage handles everything for making
> the requisite file for maskrom loading.

This makes me wonder if we should move that functoinality out of
mkimage and into binman?

>
> Add a new flag "separate_files" for mkimage handling to gather the files
> separately rather than combining them before passing them to mkimage.
>
> Signed-off-by: Peter Geis <pgwipeout at gmail.com>
> ---
> Changelog:
> v2:
> I've managed to move this all into mkimage.py as per Alper's suggestion.
> I've added an example to the readme portion of the function.
> mkimage,separate_files is now separate-files.
>
>  tools/binman/etype/mkimage.py | 41 ++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
> index 5f6def2287f6..0a86f904a2b7 100644
> --- a/tools/binman/etype/mkimage.py
> +++ b/tools/binman/etype/mkimage.py
> @@ -17,6 +17,7 @@ class Entry_mkimage(Entry):
>      Properties / Entry arguments:
>          - datafile: Filename for -d argument
>          - args: Other arguments to pass
> +        - separate-files: Boolean flag for passing files individually.
>
>      The data passed to mkimage is collected from subnodes of the mkimage node,
>      e.g.::
> @@ -42,20 +43,54 @@ class Entry_mkimage(Entry):
>              };
>          };
>
> +    This calls mkimage to create a rksd image with a standalone ram init
> +    binary file and u-boot-spl.bin as individual input files. The output from
> +    mkimage then becomes part of the image produced by binman.
> +
> +        mkimage {
> +            args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> +            separate-files;
> +
> +            ddrl {
> +                type = "blob-ext";
> +                filename = "rk3568_ddr_1560MHz_v1.12.bin";
> +            };
> +
> +            u-boot-spl {
> +            };
> +        };
> +
>      """
>      def __init__(self, section, etype, node):
>          super().__init__(section, etype, node)
>          self._args = fdt_util.GetArgs(self._node, 'args')
> +        self._separate = fdt_util.GetBool(self._node, 'separate-files')
>          self._mkimage_entries = OrderedDict()
>          self.align_default = None
>          self.ReadEntries()
> +        self.index = 0
> +        self.fname_tmp = str()
>
>      def ObtainContents(self):
>          # Use a non-zero size for any fake files to keep mkimage happy
> -        data, input_fname, uniq = self.collect_contents_to_file(
> -            self._mkimage_entries.values(), 'mkimage', 1024)
> -        if data is None:
> +        if self._separate is False:
> +          data, input_fname, uniq = self.collect_contents_to_file(
> +              self._mkimage_entries.values(), 'mkimage', 1024)
> +          if data is None:
>              return False
> +        else:
> +          for entry in self._mkimage_entries.values():

We can do:

for index, entry in enumerate(self._mkimage_entries.values()):

then you don't need self.index

> +            self.index = (self.index + 1)
> +            if (self.index > 2):
> +              print('BINMAN Warn: mkimage only supports a maximum of two separate files')

Can we use self.Raise() here instead? It seems like a fatal error.
Also this check should go in ReadNode() since we don't want to die
this late in the picture if we know it is wrong upfront. (BTW I am
moving the node-reading code to ReadNode() in my v3 series as
suggested by Alper).

> +              return False
> +            input_fname = ['mkimage', str(self.index)]
> +            data, input_fname, uniq = self.collect_contents_to_file(
> +                    [entry], ".".join(input_fname), 1024)

I suppose we can just use

   data = entry.GetData()

here?

> +            if data is None:
> +              return False
> +            self.fname_tmp = [''.join(self.fname_tmp),input_fname]
> +          input_fname = ":".join(self.fname_tmp)
>          output_fname = tools.get_output_filename('mkimage-out.%s' % uniq)
>          if self.mkimage.run_cmd('-d', input_fname, *self._args,
>                                  output_fname) is not None:
> --
> 2.25.1
>

Looks OK to me, needs a test or two.

Regards,
Simon