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.) > 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. 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? 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). > 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 <s...@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 <alpernebiya...@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. > """ > 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,'): > > [...]