On Tue, Jun 13, 2023 at 7:48 PM Jonas Karlman <jo...@kwiboo.se> wrote: > > Hi Peter, > > On 2023-06-13 09:58, Peter Robinson wrote: > > On Mon, Jun 12, 2023 at 11:20 PM Jonas Karlman <jo...@kwiboo.se> wrote: > >> > >> Hi Peter, > >> > >> On 2023-06-11 22:22, Peter Robinson wrote: > >>> Hi Jonas, > >>> > >>> This regresses the Rockpro64 build for me when building with gcc 12/13 > >>> with the following error, if I remove CONFIG_LTO it builds, but > >>> overlaps. > >>> > >>> /usr/bin/aarch64-linux-gnu-ld: > >>> /usr/lib/gcc/aarch64-linux-gnu/13/libgcc.a(lse-init.o): in function > >>> `init_have_lse_atomics': > >>> /builddir/build/BUILD/gcc-13.1.1-20230519/aarch64-linux-gnu/aarch64-linux-gnu/libgcc/../../../gcc-13.1.1-20230519/libgcc/config/aarch64/lse-init.c:46: > >>> undefined reference to `__getauxval' > >>> collect2: fatal error: ld terminated with signal 11 [Segmentation > >>> fault], core dumped > >>> compilation terminated. > >> > >> Interesting, my test build using ubuntu-22.04 and gcc-12 did not seem > >> to fail, see [1]. Compiling with -mno-outline-atomics flag may work, not > >> sure that is a good workaround. > > > > I would prefer a fix. > > Not sure what such fix could be, cross compiling in CI and under ubuntu > 22.04 does not trigger this linking issue, not sure what is different in > your build environment to not allow build with CONFIG_LTO=y. > > Searching for similar linking issue suggest it could be related to > outline-atomics, do you use any special C/LDFLAGS or similar? > > > > >> An alternative could be to move the payload to @ 512KB instead of @ 384KB. > > > > What impact will moving this around have on upgrades? I am not sure we > > should be randomly moving stuff without knowing the impact TBH. Will > > this break existing users vs what you feel is appropriate for your > > usecase? > > I did a re-calculation and using @ 512 KB offset may not be enough > for a worst-case scenario. > > mkimage will enforce a 184 KB limit for the TPL on RK3399. With SPL > loaded at 0x0 and TF-A loaded to 0x40000, we have a 256 KB size limit of > SPL. BootRom that loads both TPL and SPL only read first 2 KB of every > 4 KB page of SPI flash, so idbloader.img (TPL+SPL) could grow up to > ~880 KB in a worst-case scenario. (184+256) x 2 = 880 > > Using a payload offset of @ 896 KB (0xE0000) is probably the safest > default option if we need to revert the CONFIG_LTO=y change. > > As long as u-boot-rockchip-spi.bin is used to update SPI flash changing > offset should not have an impact. However, there are guides and scripts > that write idbloader.img and u-boot.itb separately, those could break.
Well I believe your patch only enabled that option by default so most users I suspect write them separately up until now, does the new offsets affect existing env variables at all? > Such guides and scripts can already lead to users overwriting part of > SPL when the produced idbloader.img is over 384 KB in size. > With CONFIG_ROCKCHIP_SPI_IMAGE=y the size of idbloader.img is instead > enforced to not cause an overlap of the u-boot.itb at build time. Ultimately I would prefer to not actively break existing users if they're not aware of changes, and that is hard to communicate. People with broken devices tend to scream a lot. > Regards, > Jonas > > > > >> configs/rockpro64-rk3399_defconfig: > >> CONFIG_SYS_SPI_U_BOOT_OFFS=0x80000 > >> > >> arch/arm/dts/rk3399-rockpro64-u-boot.dtsi: > >> u-boot,spl-payload-offset = <0x80000>; > >> > >> > >> [1] > >> https://github.com/Kwiboo/u-boot-build/actions/runs/5248398191/jobs/9479827969 > >> > >> Regards, > >> Jonas > >> > >>> > >>> Peter > >>> > >>> On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jo...@kwiboo.se> wrote: > >>>> > >>>> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin. > >>>> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected > >>>> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage > >>>> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin. > >>>> > >>>> => sf probe > >>>> SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, > >>>> total 16 MiB > >>>> => load mmc 1:1 10000000 u-boot-rockchip-spi.bin > >>>> 1442304 bytes read in 27 ms (50.9 MiB/s) > >>>> => sf update $fileaddr 0 $filesize > >>>> device 0 offset 0x0, size 0x160200 > >>>> 1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s > >>>> > >>>> Signed-off-by: Jonas Karlman <jo...@kwiboo.se> > >>>> Reviewed-by: Kever Yang <kever.y...@rock-chips.com> > >>>> --- > >>>> v2: > >>>> - Collect r-b tag > >>>> > >>>> configs/rockpro64-rk3399_defconfig | 3 +++ > >>>> 1 file changed, 3 insertions(+) > >>>> > >>>> diff --git a/configs/rockpro64-rk3399_defconfig > >>>> b/configs/rockpro64-rk3399_defconfig > >>>> index 0ca2cecade25..f41c03067903 100644 > >>>> --- a/configs/rockpro64-rk3399_defconfig > >>>> +++ b/configs/rockpro64-rk3399_defconfig > >>>> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000 > >>>> CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64" > >>>> CONFIG_DM_RESET=y > >>>> CONFIG_ROCKCHIP_RK3399=y > >>>> +CONFIG_ROCKCHIP_SPI_IMAGE=y > >>>> CONFIG_TARGET_ROCKPRO64_RK3399=y > >>>> CONFIG_SPL_STACK=0x400000 > >>>> CONFIG_DEBUG_UART_BASE=0xFF1A0000 > >>>> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y > >>>> CONFIG_SYS_LOAD_ADDR=0x800800 > >>>> CONFIG_PCI=y > >>>> CONFIG_DEBUG_UART=y > >>>> +CONFIG_LTO=y > >>>> CONFIG_SPL_FIT_SIGNATURE=y > >>>> CONFIG_BOOTSTAGE=y > >>>> CONFIG_BOOTSTAGE_REPORT=y > >>>> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000 > >>>> CONFIG_SPL_STACK_R=y > >>>> CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000 > >>>> CONFIG_SPL_SPI_LOAD=y > >>>> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000 > >>>> CONFIG_TPL=y > >>>> CONFIG_CMD_BOOTZ=y > >>>> CONFIG_CMD_GPT=y > >>>> -- > >>>> 2.40.1 > >>>> > >> >