| Message ID | 20220803121906.2629478-2-foss+uboot@0leil.net |
|---|---|
| State | Superseded |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | migrate u-boot-rockchip.bin to binman and generate an image for SPI | expand |
Hi Quentin, On Wed, 3 Aug 2022 at 06:19, Quentin Schulz <foss+uboot@0leil.net> wrote: > > From: Quentin Schulz <quentin.schulz@theobroma-systems.com> > > Some image types handled by mkimage require the datafiles to be passed > independently (-d data1:data2) for specific handling of each. A > concatenation of datafiles prior to passing them to mkimage wouldn't > work. > > That is the case for rkspi for example which requires page alignment > and only writing 2KB every 4KB. > > This adds the ability to tell binman to pass the datafiles without > prior concatenation to mkimage, by adding the multiple-data-files > boolean property to the mkimage node. > > Cc: Quentin Schulz <foss+uboot@0leil.net> > Reviewed-by: Simon Glass <sjg@chromium.org> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> > --- > tools/binman/entries.rst | 22 +++++++++++++++++++ > tools/binman/etype/mkimage.py | 41 +++++++++++++++++++++++++++++++---- > 2 files changed, 59 insertions(+), 4 deletions(-) Somehow I missed this one, sorry. It needs a test (try 'binman test -T' to see that it breaks code coverage). Regards, Simon
Hi Simon, On 8/18/22 10:17, Simon Glass wrote: > Hi Quentin, > > On Wed, 3 Aug 2022 at 06:19, Quentin Schulz <foss+uboot@0leil.net> wrote: >> >> From: Quentin Schulz <quentin.schulz@theobroma-systems.com> >> >> Some image types handled by mkimage require the datafiles to be passed >> independently (-d data1:data2) for specific handling of each. A >> concatenation of datafiles prior to passing them to mkimage wouldn't >> work. >> >> That is the case for rkspi for example which requires page alignment >> and only writing 2KB every 4KB. >> >> This adds the ability to tell binman to pass the datafiles without >> prior concatenation to mkimage, by adding the multiple-data-files >> boolean property to the mkimage node. >> >> Cc: Quentin Schulz <foss+uboot@0leil.net> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> >> --- >> tools/binman/entries.rst | 22 +++++++++++++++++++ >> tools/binman/etype/mkimage.py | 41 +++++++++++++++++++++++++++++++---- >> 2 files changed, 59 insertions(+), 4 deletions(-) > > Somehow I missed this one, sorry. > > It needs a test (try 'binman test -T' to see that it breaks code coverage). > How do you see this test being implemented? I think it'd be best to mock calls to mkimage (or self.mkimage.run_cmd()) and check that the -d parameter has two entries separated by a colon but my limited knowledge of unittest.mock does not seem to guide me to the proper implementation. Another way would be to use a specific mkimage type that handles that kind of multiple files -d parameter and outputs something more or less reproducible, e.g. rkspi type. I don't like this too much even though it's much easier because we're bound to this type and the image type could change (I recall some endianness fixes a few months ago). Cheers, Quentin
Hi Quentin, On Wed, 24 Aug 2022 at 09:29, Quentin Schulz <quentin.schulz@theobroma-systems.com> wrote: > > Hi Simon, > > On 8/18/22 10:17, Simon Glass wrote: > > Hi Quentin, > > > > On Wed, 3 Aug 2022 at 06:19, Quentin Schulz <foss+uboot@0leil.net> wrote: > >> > >> From: Quentin Schulz <quentin.schulz@theobroma-systems.com> > >> > >> Some image types handled by mkimage require the datafiles to be passed > >> independently (-d data1:data2) for specific handling of each. A > >> concatenation of datafiles prior to passing them to mkimage wouldn't > >> work. > >> > >> That is the case for rkspi for example which requires page alignment > >> and only writing 2KB every 4KB. > >> > >> This adds the ability to tell binman to pass the datafiles without > >> prior concatenation to mkimage, by adding the multiple-data-files > >> boolean property to the mkimage node. > >> > >> Cc: Quentin Schulz <foss+uboot@0leil.net> > >> Reviewed-by: Simon Glass <sjg@chromium.org> > >> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> > >> --- > >> tools/binman/entries.rst | 22 +++++++++++++++++++ > >> tools/binman/etype/mkimage.py | 41 +++++++++++++++++++++++++++++++---- > >> 2 files changed, 59 insertions(+), 4 deletions(-) > > > > Somehow I missed this one, sorry. > > > > It needs a test (try 'binman test -T' to see that it breaks code coverage). > > > > How do you see this test being implemented? > > I think it'd be best to mock calls to mkimage (or > self.mkimage.run_cmd()) and check that the -d parameter has two entries > separated by a colon but my limited knowledge of unittest.mock does not > seem to guide me to the proper implementation. > > Another way would be to use a specific mkimage type that handles that > kind of multiple files -d parameter and outputs something more or less > reproducible, e.g. rkspi type. I don't like this too much even though > it's much easier because we're bound to this type and the image type > could change (I recall some endianness fixes a few months ago). > You should be able to call mkimage for real and check that it does the right thing by looking at the output, in a simple way. See for example testMkimage() and let me know if you need any other hints. Regards, Simon
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index ae4305c99e..60c89aec59 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -1101,6 +1101,9 @@ Entry: mkimage: Binary produced by mkimage Properties / Entry arguments: - datafile: Filename for -d argument + - multiple-data-files: boolean to tell binman to pass all files as + datafiles to mkimage instead of creating a temporary file the result + of datafiles concatenation - args: Other arguments to pass The data passed to mkimage is collected from subnodes of the mkimage node, @@ -1117,6 +1120,25 @@ This calls mkimage to create an imximage with u-boot-spl.bin as the input file. The output from mkimage then becomes part of the image produced by binman. +To pass all datafiles untouched to mkimage:: + + mkimage { + args = "-n rk3399 -T rkspi"; + multiple-data-files; + + u-boot-tpl { + }; + + u-boot-spl { + }; + }; + +This calls mkimage to create a Rockchip RK3399-specific first stage +bootloader, made of TPL+SPL. Since this first stage bootloader requires to +align the TPL and SPL but also some weird hacks that is handled by mkimage +directly, binman is told to not perform the concatenation of datafiles prior +to passing the data to mkimage. + To use CONFIG options in the arguments, use a string list instead, as in this example which also produces four arguments:: diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py index 5f6def2287..52297c23ea 100644 --- a/tools/binman/etype/mkimage.py +++ b/tools/binman/etype/mkimage.py @@ -16,6 +16,9 @@ class Entry_mkimage(Entry): Properties / Entry arguments: - datafile: Filename for -d argument + - multiple-data-files: boolean to tell binman to pass all files as + datafiles to mkimage instead of creating a temporary file the result + of datafiles concatenation - args: Other arguments to pass The data passed to mkimage is collected from subnodes of the mkimage node, @@ -32,6 +35,25 @@ class Entry_mkimage(Entry): file. The output from mkimage then becomes part of the image produced by binman. + To pass all datafiles untouched to mkimage:: + + mkimage { + args = "-n rk3399 -T rkspi"; + multiple-data-files; + + u-boot-tpl { + }; + + u-boot-spl { + }; + }; + + This calls mkimage to create a Rockchip RK3399-specific first stage + bootloader, made of TPL+SPL. Since this first stage bootloader requires to + align the TPL and SPL but also some weird hacks that is handled by mkimage + directly, binman is told to not perform the concatenation of datafiles prior + to passing the data to mkimage. + To use CONFIG options in the arguments, use a string list instead, as in this example which also produces four arguments:: @@ -46,16 +68,27 @@ class Entry_mkimage(Entry): def __init__(self, section, etype, node): super().__init__(section, etype, node) self._args = fdt_util.GetArgs(self._node, 'args') + self._multiple_data_files = fdt_util.GetBool(self._node, 'multiple-data-files') self._mkimage_entries = OrderedDict() self.align_default = None self.ReadEntries() 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: - return False + fake_size = 1024 + if self._multiple_data_files: + fnames = [] + uniq = self.GetUniqueName() + for entry in self._mkimage_entries.values(): + if not entry.ObtainContents(fake_size=fake_size): + return False + fnames.append(entry.GetDefaultFilename()) + input_fname = ":".join(fnames) + else: + data, input_fname, uniq = self.collect_contents_to_file( + self._mkimage_entries.values(), 'mkimage', fake_size) + if data is None: + return False 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: