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

Peter Geis
2022-03-03T22:35:03Z

On Thu, Mar 3, 2022 at 4:17 PM Alper Nebi Yasak
<alpernebiyasak@gmail.com> wrote:
>
> On 03/03/2022 01:16, Peter Geis wrote:
> > On Wed, Feb 23, 2022 at 6:04 PM Simon Glass <sjg@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!
> > Very Respectfully,
> > Peter Geis