On Thu, Mar 3, 2022 at 4:17 PM Alper Nebi Yasak
<alpernebiya...@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/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.
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 <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.
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)
