diff mbox series

[RFC,7/8] binman: add support for skipping file concatenation for mkimage

Message ID 20220715153655.955874-8-foss+uboot@0leil.net
State RFC
Delegated to: Kever Yang
Headers show
Series migrate u-boot-rockchip.bin to binman and generate an image for SPI | expand

Commit Message

Quentin Schulz July 15, 2022, 3:36 p.m. UTC
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 rksd and rkspi for example, which require page
alignment (plus some weird hack for rkspi) plus size data of each stage
to be embedded in the mkimage header.

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>
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(-)

Comments

Simon Glass July 16, 2022, 11:58 a.m. UTC | #1
On Fri, 15 Jul 2022 at 09:37, 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 rksd and rkspi for example, which require page
> alignment (plus some weird hack for rkspi) plus size data of each stage
> to be embedded in the mkimage header.
>
> 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>
> 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(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

I wonder if we should move this logic to binman (from mkimage) when
all the boards are converted to binman?
Quentin Schulz July 18, 2022, 9:39 a.m. UTC | #2
Hi Simon,

On 7/16/22 13:58, Simon Glass wrote:
> On Fri, 15 Jul 2022 at 09:37, 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 rksd and rkspi for example, which require page
>> alignment (plus some weird hack for rkspi) plus size data of each stage
>> to be embedded in the mkimage header.
>>

I forgot to rephrase the commit log. As seen in patch 1/8, there 
actually doesn't seem to be a need for rksd for passing multiple data 
files to mkimage, only the TPL could be passed to mkimage and SPL 
appended right after that blob generated by mkimage. Maybe it's luck but 
it seems to work for me the few tries I did on SD card and eMMC.

>> 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>
>> 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(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I wonder if we should move this logic to binman (from mkimage) when
> all the boards are converted to binman?

Not sure this fits the "Relationship to mkimage" section in binman.rst? 
What part of the logic exactly would you like to move from mkimage to 
binman?

Cheers,
Quentin
Simon Glass July 20, 2022, 3:01 p.m. UTC | #3
Hi Quentin,

On Mon, 18 Jul 2022 at 03:39, Quentin Schulz
<quentin.schulz@theobroma-systems.com> wrote:
>
> Hi Simon,
>
> On 7/16/22 13:58, Simon Glass wrote:
> > On Fri, 15 Jul 2022 at 09:37, 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 rksd and rkspi for example, which require page
> >> alignment (plus some weird hack for rkspi) plus size data of each stage
> >> to be embedded in the mkimage header.
> >>
>
> I forgot to rephrase the commit log. As seen in patch 1/8, there
> actually doesn't seem to be a need for rksd for passing multiple data
> files to mkimage, only the TPL could be passed to mkimage and SPL
> appended right after that blob generated by mkimage. Maybe it's luck but
> it seems to work for me the few tries I did on SD card and eMMC.

OK I see.

>
> >> 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>
> >> 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(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > I wonder if we should move this logic to binman (from mkimage) when
> > all the boards are converted to binman?
>
> Not sure this fits the "Relationship to mkimage" section in binman.rst?
> What part of the logic exactly would you like to move from mkimage to
> binman?

The concatenation of the files and the strange SPI processing, for
example. It's just an idea and it is OK to keep it in mkimage.

Regards,
Simon
diff mbox series

Patch

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: