Re: [RFC] [PATCH] binman: support mkimage split files

On Thu, Mar 3, 2022 at 4:17 PM Alper Nebi Yasak alpernebiyasak@gmail.com wrote:

On 01/03/2022 05:48, Peter Geis wrote:

...

Good Evening,

I successfully tested your v2 patch series to create a bootable sdcard image out of the box for rockpro64-rk3399. Unfortunately, rk356x and rk3399-spi modes are broken, due to the inability to pass multiple images to mkimage at the same time. rk3399-spi mode is already supported manually, see: https://elixir.bootlin.com/u-boot/v2022.04-rc3/source/doc/board/rockchip/roc...

rk356x is currently only supported manually, the image built by the old Makefile method is non functional. (u-boot-rockchip.bin)

Knowing absolutely nothing about python, I've hacked together something that works for splitting the image in the way mkimage expects. The file name passed to mkimage with the -d flag is: ./mkimage.simple-bin.mkimage.1:./mkimage.simple-bin.mkimage.2

I definitely don't expect this to be accepted as is, I just use it as an example of what we need to fully support this in binman. Adding the following allows me to build images automatically for rk356x:

mkimage { args = "-n", CONFIG_SYS_SOC, "-T", "rksd"; mkimage,separate_files;

Adding a property to toggle this sounds reasonable to me. The prefix might not be necessary, and I think dashes are preferred to underscores in property names.

Thanks! I thought including mkimage as a prefix was necessary to make it clear what was happening, but on second thought it's in the mkimage node.

...

  ddrl {
          type = "blob-ext";
          filename = "rk3568_ddr_1560MHz_v1.12.bin";
  };

  u-boot-spl {
  };

};

This is my first attempt to use in-reply-to, so I hope this works.

FYI, I see it as a reply to 00/25 of the series.

I appreciate the confirmation.

...

Thanks, Peter Geis

Signed-off-by: Peter Geis pgwipeout@gmail.com

tools/binman/entry.py | 43 ++++++++++++++++++++++++++--------- tools/binman/etype/mkimage.py | 3 ++- 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 249f117ace56..48e552fc6af3 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -114,6 +114,8 @@ class Entry(object): self.bintools = {} self.missing_bintools = [] self.update_hash = True

•    self.fname_tmp = str()
  

•    self.index = 0
  
  

  @staticmethod def FindEntryClass(etype, expanded):

@@ -1134,7 +1136,7 @@ features to produce new behaviours. """ self.update_hash = update_hash

• def collect_contents_to_file(self, entries, prefix, fake_size=0):

• def collect_contents_to_file(self, entries, prefix, fake_size=0, separate=False): """Put the contents of a list of entries into a file

  Args:
  

@@ -1152,13 +1154,32 @@ features to produce new behaviours. str: Unique portion of filename (or None if no data) """ data = b''

•    for entry in entries:
  

•        # First get the input data and put it in a file. If not available,
  

•        # try later.
  

•        if not entry.ObtainContents(fake_size=fake_size):
  

•            return None, None, None
  

•        data += entry.GetData()
  

•    uniq = self.GetUniqueName()
  

•    fname = tools.get_output_filename(f'{prefix}.{uniq}')
  

•    tools.write_file(fname, data)
  

•    return data, fname, uniq
  

•    if separate is False:
  

•        for entry in entries:
  

•            # First get the input data and put it in a file. If not available,
  

•            # try later.
  

•            if not entry.ObtainContents(fake_size=fake_size):
  

•                return None, None, None
  

•            data += entry.GetData()
  

•        uniq = self.GetUniqueName()
  

•        fname = tools.get_output_filename(f'{prefix}.{uniq}')
  

•        tools.write_file(fname, data)
  

•        return data, fname, uniq
  

•    else:
  

•        for entry in entries:
  

•            self.index = (self.index + 1)
  

•            if (self.index > 2):
  

•                print('BINMAN Warn: mkimage only supports a maximum of two separate files')
  

•                break
  

•            # First get the input data and put it in a file. If not available,
  

•            # try later.
  

•            if not entry.ObtainContents(fake_size=fake_size):
  

•                return None, None, None
  

•            data = entry.GetData()
  

•            uniq = self.GetUniqueName()
  

•            fname = tools.get_output_filename(f'{prefix}.{uniq}.{self.index}')
  

•            tools.write_file(fname, data)
  

•            self.fname_tmp = [''.join(self.fname_tmp),fname]
  

•        fname = ':'.join(self.fname_tmp)
  

•        uniq = self.GetUniqueName()
  

•        return data, fname, uniq
  

I would keep this function as-is and call it multiple times in the mkimage etype code below (once per subentry), and also do the mkimage-specific checks and 'file1:file2' argument joining there as well.

Hmm, thinking about it I think I can pull this off. My first (non public) attempt was something similar to this, which just hardcoded the first file name.

I'm not kidding when I said all the python I know I learned to make this happen, so it's a learning event. Thanks for the ideas!

...

diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index 5f6def2287f6..ce5f6b6b543a 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -46,6 +46,7 @@ class Entry_mkimage(Entry): def __init__(self, section, etype, node): super().__init__(section, etype, node) self._args = fdt_util.GetArgs(self._node, 'args')

•    self._mkimage_separate = fdt_util.GetBool(self._node, 'mkimage,separate_files')
     self._mkimage_entries = OrderedDict()
     self.align_default = None
     self.ReadEntries()
  

@@ -53,7 +54,7 @@ class Entry_mkimage(Entry): 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)
  

•        self._mkimage_entries.values(), 'mkimage', 1024, self._mkimage_separate)
     if data is None:
         return False
     output_fname = tools.get_output_filename('mkimage-out.%s' % uniq)