[PATCH v2 23/25] rockchip: Support building the all output files in binman

[Simon Glass]

Hi Peter, Alper,

On Thu, 3 Mar 2022 at 15:34, Peter Geis <pgwipeout at gmail.com> wrote:
>
> On Thu, Mar 3, 2022 at 4:17 PM Alper Nebi Yasak
> <alpernebiyasak at gmail.com> wrote:
> >
> > On 03/03/2022 01:16, Peter Geis wrote:
> > > On Wed, Feb 23, 2022 at 6:04 PM Simon Glass <sjg at chromium.org> wrote:
> > >> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> > >> index eae3ee715d..64e4466489 100644
> > >> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> > >> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> > >> @@ -17,13 +17,93 @@
> > >>                 filename = "u-boot-rockchip.bin";
> > >>                 pad-byte = <0xff>;
> > >>
> > >> -               blob {
> > >> -                       filename = "idbloader.img";
> > >> +#ifdef CONFIG_TPL
> > >> +               mkimage {
> > >> +                       args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> > >> +
> > >> +                       u-boot-tpl {
> > >> +                       };
> > >> +               };
> > >> +
> > >> +               u-boot-spl {
> > >>                 };
> > >> +#elif defined(CONFIG_SPL) /* SPL only */
> > >> +               mkimage {
> > >> +                       args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> > >> +
> > >> +                       u-boot-spl {
> > >> +                       };
> > >> +               };
> > >> +#endif
> > >> +#if defined(CONFIG_SPL_FIT) && defined(CONFIG_ARM64)
> > >> +               fit: fit {
> > >> +                       description = "FIT image for U-Boot with bl31 (TF-A)";
> > >> +                       #address-cells = <1>;
> > >> +                       fit,fdt-list = "of-list";
> > >> +                       fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
> > >
> > > You need:
> > > offset = <CONFIG_SPL_PAD_TO>;
> > > here or the image is located at the wrong location.
> >
> > Thanks for confirming this.
> >
> > > The image produced is not functional however, because it swaps the
> > > firmware node with the loadable-1 node.
> > > Working image:
> > >  Configuration 0 (config_1)
> > >   Description:  rk3399-pinephone-pro.dtb
> > >   Kernel:       unavailable
> > >   Firmware:     atf_1
> > >   FDT:          fdt_1
> > >   Loadables:    uboot
> > >                 atf_2
> > >                 atf_3
> > >
> > > Non working image produced from this:
> > >  Configuration 0 (config-1)
> > >   Description:  rk3399-rockpro64.dtb
> > >   Kernel:       unavailable
> > >   Firmware:     u-boot
> > >   FDT:          fdt-1
> > >   Loadables:    atf-1
> > >                 atf-2
> > >                 atf-3
> >
> > And also this. I encountered the same things on rk3399-gru-kevin,
> > mentioned them in my reply [1] to v1 but I guess Simon missed it.
> >
> > [1] My reply to v1 of this patch
> > https://lore.kernel.org/u-boot/d50a7e13-7113-8c95-1861-cbc6c1000755@gmail.com/
>
> Yes, that's the solution I ended up with as well.
> Funnily enough, it seems loadables has a limit of five items, meaning
> the newest rk356x ATF images which have four binaries, u-boot, and
> optee trigger an error.
>
> >
> > > Also, it still doesn't support passing two separate files to mkimage
> > > at the same time, required for proper support of rk33xx_SPI generation
> > > and for rk35xx in general.
> >
> > I got v1 of this booting from SPI, but on a board that doesn't use TPL.
> > I see in doc/boards/rockchip.rst that one would indeed pass TPL+SPL to
> > mkimage as two different files. I don't know how they're processed or
> > anything about the rksd/rkspi formats though.
>
> RKSPI mode splits the image up into 2048 chunks, and pads each chunk
> with 2048 of blank space.
> For some reason the rk32/rk33 bootrom SPI code reads only 2048 out of
> each 4096 chunk.
> Delightfully this seems to have been fixed in rk35xx.
>
> >
> > > Please see my standalone patch for what I hacked together to get that working:
> > > http://patchwork.ozlabs.org/project/uboot/patch/20220301024826.1228290-1-pgwipeout@gmail.com/
> > > Note: those images are not functional either due to this issue.
> > >
> > > A wishlist item would be for it to produce the original idbloader.img
> > > and u-boot.itb files, at least for now.

Thanks for all the info. I look forward to kevin being supported...but
I should try jerry as well.

I hope to get this series applied soon and review the various
follow-on patches. It would be great to sort things out this we as
there are quite a few deficiencies in all of this, as you have found.
But for now I will hold off on the rockchip-specific patches, until
binman FIT is a bit better.

Regards,
Simon