From: Simon Glass <sjg@chromium.org>
To: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Cc: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>,
Tom Rini <trini@konsulko.com>,
Philippe Reynes <philippe.reynes@softathome.com>,
huang lin <hl@rock-chips.com>,
Jeffy Chen <jeffy.chen@rock-chips.com>,
Kever Yang <kever.yang@rock-chips.com>,
U-Boot Mailing List <u-boot@lists.denx.de>
Subject: Re: [PATCH v2 19/25] binman: Keep a separate list of entries for fit
Date: Sat, 5 Mar 2022 20:08:12 -0700 [thread overview]
Message-ID: <CAPnjgZ2P4dDNxoZ=7QYanF=gZ_hYOKqi47-37q-=6ztMyVJb7Q@mail.gmail.com> (raw)
In-Reply-To: <6d32aa01-9940-c112-af3e-b0fd9c39652d@gmail.com>
Hi Alper,
On Thu, 3 Mar 2022 at 14:17, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>
> On 24/02/2022 02:00, Simon Glass wrote:
> > The current implementation sets up the FIT entries but then deletes the
> > 'generator' ones so they don't appear in the final image.
>
> They still show up in the fdtmap if I add one to rockchip-u-boot.dtsi:
>
> $ binman ls -i u-boot-rockchip.bin
> Name Image-pos Size Entry-type Offset
> -------------------------------------------------------------
> main-section 0 103520 section 0
> mkimage 0 1a000 mkimage 0
> fit 1a000 e8d74 fit 1a000
> u-boot 1a644 b1898 section 644
> u-boot-nodtb 1a644 b1898 u-boot-nodtb 0
> @atf-SEQ 0 0 section 0
> atf-bl31 0 0 atf-bl31 0
> @tee-SEQ 0 0 section 0
> tee-os 0 0 tee-os 0
> @fdt-SEQ 0 0 section 0
> fdtmap 102d74 79c fdtmap 102d74
>
> But not simple-bin.map:
>
> ImagePos Offset Size Name
> 00000000 00000000 00103520 main-section
> 00000000 00000000 0001a000 mkimage
> 0001a000 0001a000 000e8d74 fit
> 0001a644 00000644 000b1898 u-boot
> 0001a644 00000000 000b1898 u-boot-nodtb
> 00102d74 00102d74 0000079c fdtmap
>
> This happens in v1 as well, but I hadn't checked it then.
>
> (The "fit" size and "fdtmap" offset are off by 0x10 too, but I'm not
> sure why yet.)
Well let's fix this after the series. We need tests for the map and so on.
>
> > This is a bit clumsy. We cannot build the image more than once, since the
> > generator entries are lost during the first build. Binman requires that
> > calling BuildSectionData() multiple times returns a valid result each
> > time.
>
> I think the generator entries should be turned into concrete entries and
> be removed in _gen_entries(), so BuildSectionData() should work on the
> entries that were generated. This way the individual entries would show
> up in fdtmap and could be binman-extracted/replaced as well.
That makes sense, but I'm not sure how to implement it. The split-elf
thing needs the contents of the entries which is not available at the
start. It is likely available before BuildSectionData() is called, but
not necessarily. So the template needs to hang around at least as long
as that.
I think there is something we could do here, but it isn't quite clear
to me. Perhaps we need a expand-nodes-based-on-contents phase in
control.py ? Eek...
>
> For FDTs we can generate blob entries for each file, for ELFs maybe we
> can split them into files like the script used to do (bl31_0x<addr>.bin)
> and do the same?
I'm not a big fan of adding files. Binman should be able to hold
everything in memory. It does generate files for later inspection
though, so yes we could add this feature.
>
> Maybe some new entry types, "data" for arbitrary data unrelated to a
> file whose contents we can set, or "elf-segment" that can extract a
> segment from an ELF file (but I don't like the information loss there).
Yes I quite like the idea of new entry types. In fact I was hoping to
turn split-elf into an entry type (one that generated multiple
entries). But I decided to stop before doing that since we really need
to gets the code in there, fix the bugs /move forward with some of the
ideas you and others have.
>
> > Keep a separate, private list which includes the generator nodes and use
> > that where needed, to correct this problem. Ensure that the missing list
> > includes removed generator entries too.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > (no changes since v1)
>
> I'm more and more convinced that generator nodes needs a thorough
> redesign, but the patch is consistent with status quo, so:
>
> Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>
> > tools/binman/etype/fit.py | 33 ++++++++++++++++++++++++++++-----
> > 1 file changed, 28 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> > index e1b056f56e..30b20a07a2 100644
> > --- a/tools/binman/etype/fit.py
> > +++ b/tools/binman/etype/fit.py
> > @@ -163,10 +163,15 @@ class Entry_fit(Entry_section):
> > key: relative path to entry Node (from the base of the FIT)
> > value: Entry_section object comprising the contents of this
> > node
> > + _priv_entries: Internal copy of _entries which includes 'generator'
> > + entries which are used to create the FIT, but should not be
> > + processed as real entries. This is set up once we have the
> > + entries
>
> Maybe this could be "_templates" or "_entry_generators" and only keep
> track of the generator entries.
Again I think we can revisit that. I feel it is a bit messy right now, too.
>
> > """
> > super().__init__(section, etype, node)
> > self._fit = None
> > self._fit_props = {}
> > + self._priv_entries = {}
> >
> > for pname, prop in self._node.props.items():
> > if pname.startswith('fit,'):
> >
> > [...]
Regards,
Simon
next prev parent reply other threads:[~2022-03-06 3:09 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-23 23:00 [PATCH v2 00/25] binman: rockchip: Migrate from rockchip SPL_FIT_GENERATOR script Simon Glass
2022-02-23 23:00 ` [PATCH v2 01/25] dtoc: Make GetArgs() more flexible Simon Glass
2022-03-03 21:07 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 02/25] moveconfig: Remove remove_defconfig() Simon Glass
2022-03-03 21:07 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 03/25] moveconfig: Use re.fullmatch() to avoid extra check Simon Glass
2022-03-03 21:07 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 04/25] spl: Correct Kconfig help for TPL_BINMAN_SYMBOLS Simon Glass
2022-03-03 21:08 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 05/25] dtoc: Tidy up implementation of AddStringList() Simon Glass
2022-03-03 21:08 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 06/25] elf: Rename load_segments() and module failure Simon Glass
2022-03-03 21:08 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 07/25] binman: Tweak collect_contents_to_file() and docs Simon Glass
2022-03-03 21:08 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 08/25] binman: Rename ExpandToLimit to extend_to_limit Simon Glass
2022-03-03 21:08 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 09/25] binman: Rename ExpandEntries to gen_entries Simon Glass
2022-03-03 21:09 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 10/25] binman: Refactor fit to generate output at the end Simon Glass
2022-03-03 21:09 ` Alper Nebi Yasak
2022-03-06 3:08 ` Simon Glass
2022-02-23 23:00 ` [PATCH v2 11/25] binman: Rename tools parameter to btools Simon Glass
2022-03-03 21:09 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 12/25] binman: Change how faked blobs are created Simon Glass
2022-03-03 21:09 ` Alper Nebi Yasak
2022-03-06 3:08 ` Simon Glass
2022-03-10 19:29 ` Alper Nebi Yasak
2022-03-12 5:02 ` Simon Glass
2022-03-14 21:29 ` Alper Nebi Yasak
2022-03-14 22:20 ` Simon Glass
2022-02-23 23:00 ` [PATCH v2 13/25] binman: Make fake blobs zero-sized by default Simon Glass
2022-03-03 21:09 ` Alper Nebi Yasak
2022-03-06 3:08 ` Simon Glass
2022-02-23 23:00 ` [PATCH v2 14/25] binman: Allow mkimage to use a non-zero fake-blob size Simon Glass
2022-03-03 21:09 ` Alper Nebi Yasak
2022-03-06 3:08 ` Simon Glass
2022-03-10 19:30 ` Alper Nebi Yasak
2022-03-12 5:02 ` Simon Glass
2022-02-23 23:00 ` [PATCH v2 15/25] binman: Read the fit entries only once Simon Glass
2022-03-03 21:10 ` Alper Nebi Yasak
2022-03-06 3:08 ` Simon Glass
2022-02-23 23:00 ` [PATCH v2 16/25] binman: Fix some pylint warnings in fit Simon Glass
2022-03-03 21:10 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 17/25] binman: Add a consistent way to report errors with fit Simon Glass
2022-03-03 21:10 ` Alper Nebi Yasak
2022-03-06 3:08 ` Simon Glass
2022-02-23 23:00 ` [PATCH v2 18/25] binman: Update fit to use node instead of subnode Simon Glass
2022-03-03 21:10 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 19/25] binman: Keep a separate list of entries for fit Simon Glass
2022-03-03 21:10 ` Alper Nebi Yasak
2022-03-06 3:08 ` Simon Glass [this message]
2022-03-10 19:30 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 20/25] binman: Support splitting an ELF file into multiple nodes Simon Glass
2022-03-03 21:10 ` Alper Nebi Yasak
2022-03-06 3:08 ` Simon Glass
2022-02-23 23:00 ` [PATCH v2 21/25] rockchip: evb-rk3288: Drop raw-image support Simon Glass
2022-02-23 23:00 ` [PATCH v2 22/25] rockchip: Include binman script in 64-bit boards Simon Glass
2022-03-03 21:10 ` Alper Nebi Yasak
2022-02-23 23:00 ` [PATCH v2 23/25] rockchip: Support building the all output files in binman Simon Glass
2022-03-02 22:16 ` Peter Geis
2022-03-03 21:11 ` Alper Nebi Yasak
2022-03-03 22:34 ` Peter Geis
2022-03-06 3:08 ` Simon Glass
2022-02-23 23:00 ` [PATCH v2 24/25] rockchip: Convert all boards to use binman Simon Glass
2022-02-23 23:00 ` [PATCH v2 25/25] rockchip: Drop the FIT generator script Simon Glass
2022-03-01 2:48 ` [RFC] [PATCH] binman: support mkimage split files Peter Geis
2022-03-03 21:11 ` Alper Nebi Yasak
2022-03-04 17:47 ` Peter Geis
2022-03-04 19:56 ` [PATCH v2] binman: support mkimage separate files Peter Geis
2022-03-06 3:08 ` Simon Glass
2022-03-06 14:44 ` Peter Geis
2022-03-10 19:29 ` Alper Nebi Yasak
2022-03-10 23:19 ` Peter Geis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPnjgZ2P4dDNxoZ=7QYanF=gZ_hYOKqi47-37q-=6ztMyVJb7Q@mail.gmail.com' \
--to=sjg@chromium.org \
--cc=alpernebiyasak@gmail.com \
--cc=hl@rock-chips.com \
--cc=ivan.mikhaylov@siemens.com \
--cc=jeffy.chen@rock-chips.com \
--cc=kever.yang@rock-chips.com \
--cc=philippe.reynes@softathome.com \
--cc=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.