Message ID | 20230517184028.1769172-6-jonas@kwiboo.se |
---|---|
State | Accepted |
Commit | 5713135ecc75660684ff3eb6cfba7bc11eb6433c |
Delegated to: | Kever Yang |
Headers | show |
Series | rockchip: Fix eMMC performance regression | expand |
On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@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 None of the other rk33* devices enable this offset yet my Pinebook Pro works fine booting from SPI flash, what does this fix/enable/change over the defaults? > 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. The enabling of LTO seems like a separate change TBH, the changes seem to be independent and there's no mention of it in the subject. > => 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 <jonas@kwiboo.se> > Reviewed-by: Kever Yang <kever.yang@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 >
Hi Peter, On 2023-05-17 21:14, Peter Robinson wrote: > On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@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 > > None of the other rk33* devices enable this offset yet my Pinebook Pro > works fine booting from SPI flash, what does this fix/enable/change > over the defaults? Most other RK3399 devices define the offset in the device tree, even for the rockpro64. However, the CONFIG_SYS_SPI_U_BOOT_OFFS is used as the fallback when device tree value is missing, and also as the offset used when generating the u-boot-rockchip-spi.bin using binman. puma-rk3399_defconfig is the only other RK3399 board that also generates a u-boot-rockchip-spi.bin, has CONFIG_ROCKCHIP_SPI_IMAGE=y. That board override the simple-bin-spi fit offset in rk3399-puma-haikou-u-boot.dtsi instead of using the Kconfig option. > >> 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. > > The enabling of LTO seems like a separate change TBH, the changes seem > to be independent and there's no mention of it in the subject. Without LTO enabled the idbloader.img grows too large that it does not fit before the u-boot.itb payload at 0x60000 and bulding of u-boot-rockchip-spi.bin fails the u-boot build. In order to generate a valid u-boot-rockchip-spi.bin, LTO was required to be enabled, there is also a short mention of it in the commit message. The alternative would be to move the payload offset to e.g. 0x80000 but that felt like a too big and risky change. Regards, Jonas > >> => 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 <jonas@kwiboo.se> >> Reviewed-by: Kever Yang <kever.yang@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 >>
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. Peter On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@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 <jonas@kwiboo.se> > Reviewed-by: Kever Yang <kever.yang@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 >
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. An alternative could be to move the payload to @ 512KB instead of @ 384KB. 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 <jonas@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 <jonas@kwiboo.se> >> Reviewed-by: Kever Yang <kever.yang@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 >>
On Mon, Jun 12, 2023 at 11:20 PM Jonas Karlman <jonas@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. > 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? > 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 <jonas@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 <jonas@kwiboo.se> > >> Reviewed-by: Kever Yang <kever.yang@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 > >> >
Hi Peter, On 2023-06-13 09:58, Peter Robinson wrote: > On Mon, Jun 12, 2023 at 11:20 PM Jonas Karlman <jonas@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. 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. 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 <jonas@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 <jonas@kwiboo.se> >>>> Reviewed-by: Kever Yang <kever.yang@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 >>>> >>
On Tue, Jun 13, 2023 at 7:48 PM Jonas Karlman <jonas@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 <jonas@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 <jonas@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 <jonas@kwiboo.se> > >>>> Reviewed-by: Kever Yang <kever.yang@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 > >>>> > >> >
On 2023-06-14 09:46, Peter Robinson wrote: > On Tue, Jun 13, 2023 at 7:48 PM Jonas Karlman <jonas@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 <jonas@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? It does not look like it will overwrite env offset. CONFIG_ENV_SIZE=0x8000 CONFIG_ENV_OFFSET=0x3F8000 There will be room for a 3168 KB u-boot.itb payload until it would overlap env in SPI flash with a 0xE0000 offset for u-boot.itb. Also look like many RK3399 defconfig have a SPL_MAX_SIZE=0x2e000, something that could be increased to SPL_MAX_SIZE=0x40000 with an updated offset for u-boot.itb. I will prepare a small fix series that address your LTO issue and use proper offsets and max size options. > >> 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. Such change will not actively break existing users, SPL look for a FIT header and will fall back to next media in u-boot,spl-boot-order. Will include an update to SPI flash section of rockchip.rst to mention how to properly flash to correct offset using u-boot-rockchip-spi.bin in the fix series. Regards, Jonas > >> 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 <jonas@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 <jonas@kwiboo.se> >>>>>> Reviewed-by: Kever Yang <kever.yang@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 >>>>>> >>>> >>
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