On Fri, Jun 20, 2025 at 02:21:59PM -0700, Tony Dinh wrote: > Hi Jerome, > > On Fri, Jun 20, 2025 at 4:03 AM Jerome Forissier > <[email protected]> wrote: > > > > On 6/20/25 12:53, Jerome Forissier wrote: > > > Hi Tony, > > > > > > On 6/19/25 22:37, Tony Dinh wrote: > > >> Hi Jerome, > > >> > > >> While fixing some ext4 file system problems on 2025.07-rc4, I ran into > > >> a problem booting a few Kirkwood boards. kwboot loaded the u-boot > > >> image, > > >> and then u-boot execution was frozen without anything going to the > > >> serial console. I did a bisect and found the commit that caused it. > > >> > > >> https://github.com/trini/u-boot/commit/6fe50e39508043f386fc1bd40bbc02b8a75c1940 > > >> > > >> When I tried to reverse this commit, I had a build error like this > > >> "{standard input}:24246: Error: selected processor does not support > > >> `mrc p15,1,r3,c15,c1,0' in Thumb mode" > > >> > > >> Currently, the only way to boot Kirkwood boards is to disable either > > >> CONFIG_LTO or CONFIG_SYS_THUMB_BUILD or both. > > >> > > >> I'm using the pogo_v4 and nsa325 boards as test beds. Please let me > > >> know if I can help with testing to troubleshoot this regression. > > >> > > >> All the best, > > >> Tony > > > > > > The problem seems to be that some arm32 instructions are inlined > > > inside thumb code and that's not allowed on this CPU without some > > > interworking instruction (bx, blx etc.). For example, consider > > > board_init_r(), is is mainly 16-bit instructions (Thumb): > > > > > > 006186a0 <board_init_r>: > > > board_init_r(): > > > /home/jerome/work/u-boot/common/board_r.c:799 > > > 6186a0: b5f0 push {r4, r5, r6, r7, lr} > > > 6186a2: b0ab sub sp, #172 @ 0xac > > > get_gd(): > > > /home/jerome/work/u-boot/./arch/arm/include/asm/global_data.h:127 > > > 6186a4: 464a mov r2, r9 > > > ... > > > > > > but then suddenly: > > > > > > /home/jerome/work/u-boot/arch/arm/mach-kirkwood/cpu.c:242 > > > 619aae: 9b15 ldr r3, [sp, #84] @ 0x54 > > > writefr_extra_feature_reg(): > > > /home/jerome/work/u-boot/./arch/arm/include/asm/arch/cpu.h:100 > > > 619ab0: ee2f3f11 mcr 15, 1, r3, cr15, cr1, {0} > > > ^^^^^^^^ > > > 32-bit arm instruction > > > > > > I would expect some call to a *_from_thumb() veneer, such as in: > > > > > > 006009dc <do_bootm_qnxelf>: > > > [...] > > > /home/jerome/work/u-boot/boot/bootm_os.c:379 > > > 600a02: 9501 str r5, [sp, #4] > > > /home/jerome/work/u-boot/boot/bootm_os.c:384 > > > 600a04: f04a fc7c bl 64b300 > > > <__dcache_status_from_thumb> > > > > > > > > > 0064b130 <__dcache_enable_from_thumb>: > > > __dcache_enable_from_thumb(): > > > 64b130: 4778 bx pc > > > 64b132: e7fd b.n 64b130 > > > <__dcache_enable_from_thumb> > > > 64b134: e59fc000 ldr ip, [pc] @ 64b13c > > > <__dcache_enable_from_thumb+0xc> > > > 64b138: e08cf00f add pc, ip, pc > > > 64b13c: fffffb28 .word 0xfffffb28 > > > > > > We may need to move the inline functions with mrc/mcr to a .S file, > > > making sure it's built with -marm on CPUs that do not support Thumb2. > > > Then hopefully the proper interworking sequence would be inserted by the > > > linker as is the case when Thumb is calling into object files resulting > > > from .c files built with -marm (for example: arch/arm/mach-kirkwood/cpu.c > > > which has CFLAGS_cpu.o := -marm). > > > > > > In the meantime, could you try the following patch? > > > > > > diff --git a/common/Makefile b/common/Makefile > > > index 35991562a12..a509675335a 100644 > > > --- a/common/Makefile > > > +++ b/common/Makefile > > > @@ -19,6 +19,8 @@ obj-y += version.o > > > # # boards > > > obj-y += board_f.o > > > obj-y += board_r.o > > > +CFLAGS_board_f.o += -marm > > > +CFLAGS_board_r.o += -marm > > > obj-$(CONFIG_DISPLAY_BOARDINFO) += board_info.o > > > obj-$(CONFIG_DISPLAY_BOARDINFO_LATE) += board_info.o > > > > > > I think it should resolve the issue, but is does also increase the > > > size of the binary (+4 KiB). So the .S solution would definitely be > > > better if it works IMO. I'll try to propose something ASAP, will > > > likely be on Monday, feel free to give it a try by yourself though :) > > > > Or maybe this which is a bit less radical: > > > > diff --git a/common/Makefile b/common/Makefile > > index 35991562a12..f0b7eadf7d2 100644 > > --- a/common/Makefile > > +++ b/common/Makefile > > @@ -19,6 +19,8 @@ obj-y += version.o > > # # boards > > obj-y += board_f.o > > obj-y += board_r.o > > +CFLAGS_REMOVE_board_f.o := $(LTO_CFLAGS) > > +CFLAGS_REMOVE_board_r.o := $(LTO_CFLAGS) > > obj-$(CONFIG_DISPLAY_BOARDINFO) += board_info.o > > obj-$(CONFIG_DISPLAY_BOARDINFO_LATE) += board_info.o > > > > Still results in +1 KiB. > > The above patch works great! I've tested it on 3 Kirkwood boards: the > Pogo V4 (88F6192), Zyxel NSA325 (88F6282), and GoFlex Home (88F6281, > same SoC as the Sheevaplug). > > The Sheevaplug is now 498K, which is well under limit. All these > boards see only 1K increase in binary size.
Can we please get a patch of the latter option now for the release and look at doing the .S file for the future? Thanks! -- Tom
signature.asc
Description: PGP signature
