arm: mvebu: Kirkwood boards are broken by commit 6fe50e3

[Tom Rini]

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
> <jerome.forissier at linaro.org> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250620/1f25dee0/attachment.sig>