using binman fails boot

[Simon Glass]

Hi Tim,

On Fri, 23 Jul 2021 at 15:06, Tim Harvey <tharvey at gateworks.com> wrote:
>
> On Thu, Jul 22, 2021 at 8:07 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Tim,
> >
> > On Mon, 19 Jul 2021 at 17:23, Tim Harvey <tharvey at gateworks.com> wrote:
> > >
> > > On Sat, Jul 17, 2021 at 7:22 PM Simon Glass <sjg at chromium.org> wrote:
> > > >
> >
> > [..]
> >
> > > > > But isn't blob-ext at 4 a correct name? I can't use 'blob-ext-4' as
> > > > > that's an unknown entry type.
> > > >
> > > > Well you can use any name and specify the type:
> > > >
> > > > my-name {
> > > >    type = "blob-ext";
> > > > };
> > > >
> > >
> > > Ok - I understand.
> > >
> > > > >
> > > > > >
> > > > > > If you can push your tree somewhere (with this problem) I'll see if I
> > > > > > can figure out why.
> > > > > >
> > > > >
> > > > > Sure, I pushed it to
> > > > > https://github.com/Gateworks/uboot-venice/tree/WIP-venice-binman
> > > > > make imx8mm_venice_defconfig
> > > > > make
> > > >
> > > > OK
> > > >
> > > > >
> > > > > > >
> > > > > > > BINMAN_VERBOSE=4 indeed prints out a tone of stuff but I'm not seeing
> > > > > > > anything for 'blob' below that would seem to indicate one node name vs
> > > > > > > another:
> > > > > >
> > > > > > Oops you need BINMAN_VERBOSE=5 - see elf.py LookupAndWriteSymbols()
> > > > > > which has tout.Debug() which is level 5.
> > > > > >
> > > > >
> > > > > LookupAndWriteSymbols ends up doing nothing because
> > > > > syms.get('__image_copy_start') returns None.
> > > >
> > > > Well that is likely the problem.
> >
> > I sent a patch to make binman report this as an error.
> >
> > I pushed the resulting tree to:
> >
> > https://github.com/sjg20/u-boot/tree/try-tim
> >
> > Now the error is:
> >
> > binman: Section '/binman/u-boot-spl-ddr': Symbol
> > '_binman_u_boot_any_prop_image_pos'
> >
> >    in entry '/binman/u-boot-spl-ddr/u-boot-spl/u-boot-spl-nodtb':
> > Entry 'u-boot-any' not found in list
> > (u-boot-spl-nodtb,u-boot-spl-dtb,u-boot-spl,blob-ext at 1,blob-ext at 2,blob-ext at 3,blob-ext at 4,main-section)
> >
> > The problem seems to be that you are asking binman to generate three
> > independent images. U-Boot is in a FIT which is not in the same image
> > as SPL. So it is not possible to locate the flash offset of U-Boot
> > (with in the FIT).
> >
> > Can you give me a bit more info about your intent here? Is it to load
> > U-Boot from the FIT? I so, I suppose it is possible to make binman
> > access an independent image, if it is told where it starts.
> >
> > But why is everything not in one image?
> >
>
> Simon,
>
> I would rather have 1 image. I was going off of the imx8mm_evk switch
> to binman which creates the separate images.

Well at present you are loading a FIT into RAM, I think? Is it coming
from flash?

If you load a FIT containing U-Boot then you don't need the binman
symbol stuff, since SPL looks in the FIT for the location of U-Boot.
There isn't much benefit in having binman point to U-Boot within the
FIT, since we already have code to find it. It might save a few bytes
of code, but it would be confusing...I'm not sure if that is worth the
hassle.

If you want a single image, then you might not want FIT at all...just
use binman.

It really depends what you want.

>
> The whole point of what I'm investigating here has to do with the SPL.
> OCRAM is at a premium and the current way the IMX8M is handling DDR
> firmware is to tack it on after the code in the SPL image and it gets
> padded to make it easy to locate which is a huge waste of space. I
> figured we can use binman to locate the blobs without the padding.
>
> So, if you take 'just' the spl image here:
>         spl: u-boot-spl-ddr {
>                 filename = "u-boot-spl-ddr.bin";
>                 pad-byte = <0xff>;
>                 align-size = <4>;
>                 align = <4>;
>
>                 u-boot-spl {
>                         align-end = <4>;
>                 };
>
>                 blob_1: blob-ext at 1 {
>                         filename = "lpddr4_pmu_train_1d_imem.bin";
>                         size = <0x8000>;
>                 };
>
>                 blob_2: blob-ext at 2 {
>                         filename = "lpddr4_pmu_train_1d_dmem.bin";
>                         size = <0x4000>;
>                 };
>
>                 blob_3: blob-ext at 3 {
>                         filename = "lpddr4_pmu_train_2d_imem.bin";
>                         size = <0x8000>;
>                 };
>
>                 blob_4: blob-ext at 4 {
>                         filename = "lpddr4_pmu_train_2d_dmem.bin";
>                         size = <0x4000>;
>                 };
>         };
>
> My intention is to remove the size arguments above which are currently
> forcing wasted padding and locate the blobs at runtime with binman.

Well you can just remove them.

>
> Based on your other patch it it would seem I'm missing something from
> my lds to add __image_copy_start yet in
> arch/arm/cpu/armv8/u-boot-spl.lds I see:
>         .text : {
>                 . = ALIGN(8);
>                 *(.__image_copy_start)
>                 CPUDIR/start.o (.text*)
>                 *(.text*)
>         } >.sram
>
> My understanding of linker files is pretty slim so perhaps there's
> something missing above.

Yes you need to define the value of the __image_copy_start symbol, so:

.text: {
    __image_copy_start = .;

See for example arch/arm/cpu/u-boot-spl.lds

Regards,
SImon