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/rockchip.rst#L182 > > 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. > > 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. > > Thanks, > Peter Geis > > Signed-off-by: Peter Geis <pgwipe...@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. > 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)