| Message ID | 20220722113505.3875669-1-foss+uboot@0leil.net |
|---|---|
| Headers | show |
| Series | migrate u-boot-rockchip.bin to binman and generate an image for SPI | expand |
El Fri, Jul 22, 2022 at 01:34:58PM +0200, Quentin Schulz deia: > From: Quentin Schulz <quentin.schulz@theobroma-systems.com> > > This migrates the generation of u-boot-rockchip.bin from Makefile to binman > completely. There is therefore no idbloader.img anymore as it is created on > the fly by binman. > Thanks a lot. I've tested this series and it worked for me and my Rock Pi 4B. That's a similar board, RK3399 too. Maybe it needs more refinements for other rockchip boards. I haven't spent a lot of time testing yet, and I've tested it with two or three dozen patches on top of master, so not quite the real thing. One thing that comes to mind is that master has CONFIG_ENV_IS_IN_MMC=y and I have it disabled and CONFIG_ENV_IS_NOWHERE=y. So maybe we should check in dtsi whether we need binman to call mkenvimage (and scripts/get_default_envs.sh or from what source?) and put the output at CONFIG_ENV_OFFSET=0x3F8000 for the SD image. Or maybe we should just disable CONFIG_ENV_IS_IN_MMC in the board ? (I think it conflicts with trust settings anyway). Or maybe just leave as it is and let env load non-fatally fail unless the user provides their own custom environment ? Now that we have Quentin's patches maybe we can remove the Makefile warning about CONFIG_USE_SPL_FIT_GENERATOR and move the task that arch/arm/mach-rockchip/make_fit_atf.py is doing to binman. But I don't seem to find any dts using "split-elf" I even wonder, am I right to assume make_fit_atf.py is deprecated and split-elf prefered ? Because split-elf seems to be unused and make_fit_atf.py seems to be default ???
Hi Xavier, On 7/24/22 09:46, Xavier Drudis Ferran wrote: > El Fri, Jul 22, 2022 at 01:34:58PM +0200, Quentin Schulz deia: >> From: Quentin Schulz <quentin.schulz@theobroma-systems.com> >> >> This migrates the generation of u-boot-rockchip.bin from Makefile to binman >> completely. There is therefore no idbloader.img anymore as it is created on >> the fly by binman. >> > > Thanks a lot. > > I've tested this series and it worked for me and my Rock Pi 4B. That's > a similar board, RK3399 too. Maybe it needs more refinements for other > rockchip boards. > > I haven't spent a lot of time testing yet, and I've tested it with > two or three dozen patches on top of master, so not quite the real thing. > > One thing that comes to mind is that master has CONFIG_ENV_IS_IN_MMC=y > and I have it disabled and CONFIG_ENV_IS_NOWHERE=y. So maybe we should > check in dtsi whether we need binman to call mkenvimage (and > scripts/get_default_envs.sh or from what source?) and put the output > at CONFIG_ENV_OFFSET=0x3F8000 for the SD image. > It's not currently done for u-boot-rockchip.bin in upstream master, so it's not part of this patch series. You'd need a new binman entry I assume for calling mkenvimage. It's not a super safe assumption that CONFIG_ENV_OFFSET will be used for declaring where the environment is stored. E.g., CONFIG_ENV_OFFSET for Puma declares where the env is located on SPI-NOR, however for MMC devices, it is specified with u-boot,mmc-env-offset DT property (though, if it is not defined, it defaults to CONFIG_ENV_OFFSET, c.f. env/mmc.c). But maybe the same comment as I did for CONFIG_SYS_SPI_U_BOOT_OFFS would be enough? e.g. states that this should be in sync with the DT property if there's one for the board in question. See: https://lore.kernel.org/u-boot/20220722160655.3904213-13-foss+uboot@0leil.net/ for what I needed to do for Puma to get u-boot-rockchip-spi.bin support. But that overall seems like a reasonable feature to add. > Or maybe we should just disable CONFIG_ENV_IS_IN_MMC in the board ? (I > think it conflicts with trust settings anyway). > Yes, anything but ENV_IS_NOWHERE depends on !CHAIN_OF_TRUST. So I guess a check on #ifndef CONFIG_CHAIN_OF_TRUST for that section in binman would do the trick? > Or maybe just leave as it is and let env load non-fatally fail unless > the user provides their own custom environment ? > If there's no environment where U-Boot is looking for it, it'll not fatally fail right now. The next time you save the environment (saveenv), it'll be written in the correct location and read during next boot. Again, this patch series does not modify u-boot-rockchip.bin current behavior :) > Now that we have Quentin's patches maybe we can remove the Makefile > warning about CONFIG_USE_SPL_FIT_GENERATOR and move the task that > arch/arm/mach-rockchip/make_fit_atf.py is doing to binman. I wasn't aware of this entry, maybe it is actually possible. That'd be nice to have :) > But I don't seem to find any dts using "split-elf" There are some test dts files in tools/binman directory for split-elf though. > I even wonder, am I right to assume make_fit_atf.py is deprecated and > split-elf prefered ? Because split-elf seems to be unused and make_fit_atf.py > seems to be default ??? > My understanding is that there's a will to replace make_fit_atf.py output generated by binman instead, if binman supports everything needed to do this migration, I don't know. Cheers, Quentin
El Mon, Jul 25, 2022 at 12:54:04PM +0200, Quentin Schulz deia: > You'd need a new binman entry I assume for calling mkenvimage. > > It's not a super safe assumption that CONFIG_ENV_OFFSET will be used for > declaring where the environment is stored. E.g., CONFIG_ENV_OFFSET for Puma > declares where the env is located on SPI-NOR, however for MMC devices, it is > specified with u-boot,mmc-env-offset DT property (though, if it is not > defined, it defaults to CONFIG_ENV_OFFSET, c.f. env/mmc.c). But maybe the > same comment as I did for CONFIG_SYS_SPI_U_BOOT_OFFS would be enough? e.g. > states that this should be in sync with the DT property if there's one for > the board in question. See: https://lore.kernel.org/u-boot/20220722160655.3904213-13-foss+uboot@0leil.net/ > for what I needed to do for Puma to get u-boot-rockchip-spi.bin support. > > But that overall seems like a reasonable feature to add. > But it is interesting enough ? I mean once you could call mkenvimage you'd still need to give it some environment (variables values). And what would you give it that would be different from the default environment ? Maybe what's wanted is just to fail to read the environment the first time. And the user can save it from hush (or run mkenvimage themselves). > > Or maybe we should just disable CONFIG_ENV_IS_IN_MMC in the board ? (I > > think it conflicts with trust settings anyway). > > > > Yes, anything but ENV_IS_NOWHERE depends on !CHAIN_OF_TRUST. So I guess a > check on #ifndef CONFIG_CHAIN_OF_TRUST for that section in binman would do > the trick? > If we need to add the environment to binman we could just check ENV_IS_IN_... because those will already be false if CHAIN_OF_TRUST is false, I think. But mayeb we don't need to do that, or we just want to leave some empty space somewhere (yeah, difficult to know where if the offset can be in dt or Kconfig). > > If there's no environment where U-Boot is looking for it, it'll not fatally > fail right now. The next time you save the environment (saveenv), it'll be > written in the correct location and read during next boot. Again, this patch > series does not modify u-boot-rockchip.bin current behavior :) > You're right, so maybe we don't really need to run mkenvimage to build the image, we leave the environment empty, and let the user do their thing. We may want to make sure we don't put something else there, though. > > Now that we have Quentin's patches maybe we can remove the Makefile > > warning about CONFIG_USE_SPL_FIT_GENERATOR and move the task that > > arch/arm/mach-rockchip/make_fit_atf.py is doing to binman. > > I wasn't aware of this entry, maybe it is actually possible. That'd be nice > to have :) > I'm playing with this right now. I don't have anything that boots. First attempt complains that SPL can't allocate so much RAM to fit the internal image (not really feeling like increasing space a lot for many boards) and 2nd attempt at having an external image to make it closer to what make_fit_atf.py used to do and have less breakage, then it doesn't even complain, just stops short of running ATF. I'll play some more... > > My understanding is that there's a will to replace make_fit_atf.py output > generated by binman instead, if binman supports everything needed to do this > migration, I don't know. > Thanks for your confirmation.
On 7/25/22 13:20, Xavier Drudis Ferran wrote: > El Mon, Jul 25, 2022 at 12:54:04PM +0200, Quentin Schulz deia: > >> You'd need a new binman entry I assume for calling mkenvimage. >> > > >> It's not a super safe assumption that CONFIG_ENV_OFFSET will be used for >> declaring where the environment is stored. E.g., CONFIG_ENV_OFFSET for Puma >> declares where the env is located on SPI-NOR, however for MMC devices, it is >> specified with u-boot,mmc-env-offset DT property (though, if it is not >> defined, it defaults to CONFIG_ENV_OFFSET, c.f. env/mmc.c). But maybe the >> same comment as I did for CONFIG_SYS_SPI_U_BOOT_OFFS would be enough? e.g. >> states that this should be in sync with the DT property if there's one for >> the board in question. See: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_u-2Dboot_20220722160655.3904213-2D13-2Dfoss-2Buboot-400leil.net_&d=DwIBaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=vRiCOa3kkluAD8H-j_n6Ah5cU-awcHa_QffxMmj8DZv5-_BNT1IcyP3RMiDj_ZWl&s=ptlh1k3KoLVMT-Zk2f-cUxoHrQeWZTVCLIP83D3NVuQ&e= >> for what I needed to do for Puma to get u-boot-rockchip-spi.bin support. >> >> But that overall seems like a reasonable feature to add. >> > > But it is interesting enough ? > I mean once you could call mkenvimage you'd still need to give it > some environment (variables values). And what would you give it > that would be different from the default environment ? Nothing, but that's the point? So you don't need to run a saveenv first to save the environment on the storage medium? Which is useful when you have userspace modifying the environment (e.g. A/B partition choice). But that's off-topic anyways and I'm not entirely sure this is actually easily doable for the simple reason that I think sections are ordered in DTB and we don't know where the user wants the environment on their storage medium and that might change the order of the sections. > Maybe what's wanted is just to fail to read the environment > the first time. And the user can save it from hush (or run mkenvimage > themselves). > >>> Or maybe we should just disable CONFIG_ENV_IS_IN_MMC in the board ? (I >>> think it conflicts with trust settings anyway). >>> >> >> Yes, anything but ENV_IS_NOWHERE depends on !CHAIN_OF_TRUST. So I guess a >> check on #ifndef CONFIG_CHAIN_OF_TRUST for that section in binman would do >> the trick? >> > > If we need to add the environment to binman we could just check ENV_IS_IN_... > because those will already be false if CHAIN_OF_TRUST is false, I think. > > But mayeb we don't need to do that, or we just want to leave some > empty space somewhere (yeah, difficult to know where if the offset can be in dt or Kconfig). > >> >> If there's no environment where U-Boot is looking for it, it'll not fatally >> fail right now. The next time you save the environment (saveenv), it'll be >> written in the correct location and read during next boot. Again, this patch >> series does not modify u-boot-rockchip.bin current behavior :) >> > > You're right, so maybe we don't really need to run mkenvimage to build the image, > we leave the environment empty, and let the user do their thing. We may want > to make sure we don't put something else there, though. > >>> Now that we have Quentin's patches maybe we can remove the Makefile >>> warning about CONFIG_USE_SPL_FIT_GENERATOR and move the task that >>> arch/arm/mach-rockchip/make_fit_atf.py is doing to binman. >> >> I wasn't aware of this entry, maybe it is actually possible. That'd be nice >> to have :) >> > > I'm playing with this right now. I don't have anything that boots. > First attempt complains that SPL can't allocate so much RAM to fit the > internal image (not really feeling like increasing space a lot for > many boards) and 2nd attempt at having an external image to make it closer > to what make_fit_atf.py used to do and have less breakage, then it doesn't even > complain, just stops short of running ATF. > > I'll play some more... > Don't really want to hijack the thread with something slightly unrelated but posting this here for posterity: diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi index daa5dea3d5..0af11341a1 100644 --- a/arch/arm/dts/rockchip-u-boot.dtsi +++ b/arch/arm/dts/rockchip-u-boot.dtsi @@ -11,8 +11,8 @@ }; }; -#ifdef CONFIG_SPL &binman { +#ifdef CONFIG_SPL simple-bin { filename = "u-boot-rockchip.bin"; pad-byte = <0xff>; @@ -31,13 +31,66 @@ }; #ifdef CONFIG_ARM64 - blob { - filename = "u-boot.itb"; + fit { + offset = <((CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR - 64) * 512)>; + + description = "FIT image for U-Boot with bl31 (TF-A)"; + #address-cells = <1>; + fit,fdt-list = "of-list"; + + images { + uboot { + 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 { + }; + }; + + @fdt-SEQ { + description = "NAME"; + type = "flat_dt"; + compression = "none"; + + u-boot-dtb { + }; + }; + }; + + configurations { + default = "@config-DEFAULT-SEQ"; + @config-SEQ { + description = "NAME"; + firmware = "atf-1"; + loadables = "uboot","atf-2","atf-3"; + fdt = "fdt-SEQ"; + }; + }; + }; #else u-boot-img { -#endif offset = <((CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR - 64) * 512)>; }; +#endif }; #ifdef CONFIG_ROCKCHIP_SPI_IMAGE @@ -69,5 +122,5 @@ }; }; #endif -}; #endif +}; is what I have currently done and the outcome of this is: U-Boot TPL 2022.07-00811-gf6815f93eb-dirty (Jul 25 2022 - 18:24:06) Channel 0: DDR3, 666MHz BW=32 Col=10 Bk=8 CS0 Row=15 CS=1 Die BW=16 Size=1024MB Channel 1: DDR3, 666MHz BW=32 Col=10 Bk=8 CS0 Row=15 CS=1 Die BW=16 Size=1024MB 256B stride Trying to boot from BOOTROM Returning to boot ROM... U-Boot SPL 2022.07-00811-gf6815f93eb-dirty (Jul 25 2022 - 18:24:06 +0200) Trying to boot from MMC2 alloc space exhausted FIT buffer of 1018880 bytes Could not get FIT buffer of 1018880 bytes check CONFIG_SYS_SPL_MALLOC_SIZE No valid device tree binary found at 00000000002c0e88 initcall sequence 0000000000286bd0 failed at call 0000000000279604 (err=-1) ### ERROR ### Please RESET the board ### The new u-boot-rockchip.bin is only about 2KB bigger. The name and addresses are correctly returned by mkimage -l when comparing the current implementation and with the patch above applied. Cheers, Quentin
From: Quentin Schulz <quentin.schulz@theobroma-systems.com> This migrates the generation of u-boot-rockchip.bin from Makefile to binman completely. There is therefore no idbloader.img anymore as it is created on the fly by binman. This also adds support for generating the same kind of image than u-boot-rockchip.bin but for SPI flashes (specifically, a different image type generated by mkimage is necessary, in addition to a different offset in the storage medium). This has been tested on Puma RK3399 with patches soon to be sent to the ML. This also helped removing the hardcoded value for the u-boot.itb offset in u-boot-rockchip.bin which prevented Puma SoM to be migrated to it. Cheers, Quentin v2: - removed patch 4/8 rockchip: pad u-boot-rockchip.bin correctly because it would break partitions table, - rebased on top of master, changes to patch 3/7 rockchip: remove unneeded CONFIG_SPL_PAD_TO compared to the RFC 3/8 rockchip: remove unneeded CONFIG_SPL_PAD_TO, Quentin Schulz (7): rockchip: generate idbloader.img content for u-boot-rockchip.bin with binman for ARM rockchip: generate u-boot-rockchip.bin with binman for ARM64 boards rockchip: remove unneeded CONFIG_SPL_PAD_TO rockchip: simplify binman image dependencies addition to INPUTS rockchip: allow to build SPI images even without HAS_ROM option binman: add support for skipping file concatenation for mkimage rockchip: add u-boot-rockchip-spi.bin image for booting from SPI-NOR flash Makefile | 39 ++------------- arch/arm/Kconfig | 2 +- arch/arm/dts/rk3288-u-boot.dtsi | 2 +- arch/arm/dts/rk3399-u-boot.dtsi | 2 +- arch/arm/dts/rockchip-u-boot.dtsi | 49 ++++++++++++++++++- arch/arm/mach-rockchip/Kconfig | 6 +-- configs/chromebit_mickey_defconfig | 1 - configs/chromebook_bob_defconfig | 1 - configs/chromebook_jerry_defconfig | 1 - configs/chromebook_kevin_defconfig | 1 - configs/chromebook_minnie_defconfig | 1 - configs/chromebook_speedy_defconfig | 1 - configs/evb-px30_defconfig | 1 - configs/evb-px5_defconfig | 1 - configs/evb-rk3036_defconfig | 1 - configs/evb-rk3229_defconfig | 1 - configs/evb-rk3288_defconfig | 1 - configs/evb-rk3308_defconfig | 1 - configs/evb-rk3328_defconfig | 1 - configs/evb-rk3399_defconfig | 1 - configs/evb-rk3568_defconfig | 1 - configs/ficus-rk3399_defconfig | 1 - configs/firefly-px30_defconfig | 1 - configs/firefly-rk3288_defconfig | 1 - configs/firefly-rk3399_defconfig | 1 - configs/khadas-edge-captain-rk3399_defconfig | 1 - configs/khadas-edge-rk3399_defconfig | 1 - configs/khadas-edge-v-rk3399_defconfig | 1 - configs/kylin-rk3036_defconfig | 1 - configs/leez-rk3399_defconfig | 1 - configs/lion-rk3368_defconfig | 1 - configs/miqi-rk3288_defconfig | 1 - configs/mk808_defconfig | 1 - configs/nanopc-t4-rk3399_defconfig | 1 - configs/nanopi-m4-2gb-rk3399_defconfig | 1 - configs/nanopi-m4-rk3399_defconfig | 1 - configs/nanopi-m4b-rk3399_defconfig | 1 - configs/nanopi-neo4-rk3399_defconfig | 1 - configs/nanopi-r2s-rk3328_defconfig | 1 - configs/nanopi-r4s-rk3399_defconfig | 1 - configs/odroid-go2_defconfig | 1 - configs/orangepi-rk3399_defconfig | 1 - configs/phycore-rk3288_defconfig | 1 - configs/pinebook-pro-rk3399_defconfig | 1 - configs/popmetal-rk3288_defconfig | 1 - configs/puma-rk3399_defconfig | 1 - configs/px30-core-ctouch2-of10-px30_defconfig | 1 - configs/px30-core-ctouch2-px30_defconfig | 1 - configs/px30-core-edimm2.2-px30_defconfig | 1 - configs/roc-cc-rk3308_defconfig | 1 - configs/roc-cc-rk3328_defconfig | 1 - configs/roc-pc-mezzanine-rk3399_defconfig | 1 - configs/roc-pc-rk3399_defconfig | 1 - configs/rock-pi-4-rk3399_defconfig | 1 - configs/rock-pi-4c-rk3399_defconfig | 1 - configs/rock-pi-e-rk3328_defconfig | 1 - configs/rock-pi-n10-rk3399pro_defconfig | 1 - configs/rock-pi-n8-rk3288_defconfig | 1 - configs/rock2_defconfig | 1 - configs/rock64-rk3328_defconfig | 1 - configs/rock960-rk3399_defconfig | 1 - configs/rock_defconfig | 1 - configs/rockpro64-rk3399_defconfig | 1 - configs/tinker-rk3288_defconfig | 1 - configs/tinker-s-rk3288_defconfig | 1 - configs/vyasa-rk3288_defconfig | 1 - include/configs/rockchip-common.h | 2 - tools/binman/entries.rst | 22 +++++++++ tools/binman/etype/mkimage.py | 41 ++++++++++++++-- 69 files changed, 116 insertions(+), 109 deletions(-)