Hi Peter,
On Fri, 4 Mar 2022 at 12:56, Peter Geis <pgwipe...@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 <pgwipe...@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
