| Message ID | 20220223230040.159317-24-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | binman: rockchip: Migrate from rockchip SPL_FIT_GENERATOR script | expand |
On Wed, Feb 23, 2022 at 6:04 PM Simon Glass <sjg@chromium.org> wrote: > Good Evening, > Add the required binman images to replace the Makefile rules which are > currently used. This includes subsuming: > > - tpl/u-boot-tpl-rockchip.bin if TPL is enabled > - idbloader.img if either or both of SPL and TPL are enabled > - u-boot.itb if SPL_FIT is enabled > - u-boot-rockchip.bin if SPL is used, either using u-boot.itb when > SPL_FIT is enabled or u-boot.img when it isn't > > Note that the intermediate files are dropped with binman, since it > producing everything in one pass. This means that > tpl/u-boot-tpl-rockchip.bin is not created, for example. I love this series, and look forward to it working. I've tested this with rk3399, unfortunately it does not produce a functional u-boot.itb. I originally thought it was, but then I realized it was placing it at an incorrect location. > > Note that for some 32-bit rk3288 boards, rockchip-optee.dtsi is included. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v2: > - Rename op-tee to tee-os > - Drop use of .itb2 > > arch/arm/dts/rockchip-u-boot.dtsi | 84 ++++++++++++++++++++++++++++++- > 1 file changed, 82 insertions(+), 2 deletions(-) > > 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. > + images { > + u-boot { > + description = "U-Boot (64-bit)"; > + type = "standalone"; > + os = "U-Boot"; > + arch = "arm64"; > + compression = "none"; > + load = <CONFIG_SYS_TEXT_BASE>; > + u-boot-nodtb { > + }; > + }; > > + @atf-SEQ { > + fit,operation = "split-elf"; > + description = "ARM Trusted Firmware"; > + type = "firmware"; > + arch = "arm64"; > + os = "arm-trusted-firmware"; > + compression = "none"; > + fit,load; > + fit,entry; > + fit,data; > + > + atf-bl31 { > + }; > + }; > + @tee-SEQ { > + fit,operation = "split-elf"; > + description = "TEE"; > + type = "tee"; > + arch = "arm64"; > + os = "tee"; > + compression = "none"; > + fit,load; > + fit,entry; > + fit,data; > + > + tee-os { > + }; > + }; > + > + @fdt-SEQ { > + description = "fdt-NAME"; > + compression = "none"; > + type = "flat_dt"; > + }; > + }; > + > + configurations { > + default = "@config-DEFAULT-SEQ"; > + @config-SEQ { > + description = "NAME.dtb"; > + fdt = "fdt-SEQ"; > + firmware = "u-boot"; > + fit,loadables; > + }; > + }; > + }; > +#else > u-boot-img { > offset = <CONFIG_SPL_PAD_TO>; > }; > +#endif /* CONFIG_ARM64 */ > }; > }; > #endif > -- > 2.35.1.574.g5d30c73bfb-goog > 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 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. 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
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/ > 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. > 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
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
Hi Peter, Alper, On Thu, 3 Mar 2022 at 15:34, Peter Geis <pgwipeout@gmail.com> wrote: > > 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 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
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>; + images { + u-boot { + description = "U-Boot (64-bit)"; + type = "standalone"; + os = "U-Boot"; + arch = "arm64"; + compression = "none"; + load = <CONFIG_SYS_TEXT_BASE>; + u-boot-nodtb { + }; + }; + @atf-SEQ { + fit,operation = "split-elf"; + description = "ARM Trusted Firmware"; + type = "firmware"; + arch = "arm64"; + os = "arm-trusted-firmware"; + compression = "none"; + fit,load; + fit,entry; + fit,data; + + atf-bl31 { + }; + }; + @tee-SEQ { + fit,operation = "split-elf"; + description = "TEE"; + type = "tee"; + arch = "arm64"; + os = "tee"; + compression = "none"; + fit,load; + fit,entry; + fit,data; + + tee-os { + }; + }; + + @fdt-SEQ { + description = "fdt-NAME"; + compression = "none"; + type = "flat_dt"; + }; + }; + + configurations { + default = "@config-DEFAULT-SEQ"; + @config-SEQ { + description = "NAME.dtb"; + fdt = "fdt-SEQ"; + firmware = "u-boot"; + fit,loadables; + }; + }; + }; +#else u-boot-img { offset = <CONFIG_SPL_PAD_TO>; }; +#endif /* CONFIG_ARM64 */ }; }; #endif
Add the required binman images to replace the Makefile rules which are currently used. This includes subsuming: - tpl/u-boot-tpl-rockchip.bin if TPL is enabled - idbloader.img if either or both of SPL and TPL are enabled - u-boot.itb if SPL_FIT is enabled - u-boot-rockchip.bin if SPL is used, either using u-boot.itb when SPL_FIT is enabled or u-boot.img when it isn't Note that the intermediate files are dropped with binman, since it producing everything in one pass. This means that tpl/u-boot-tpl-rockchip.bin is not created, for example. Note that for some 32-bit rk3288 boards, rockchip-optee.dtsi is included. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: - Rename op-tee to tee-os - Drop use of .itb2 arch/arm/dts/rockchip-u-boot.dtsi | 84 ++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 2 deletions(-)