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

[Jerome Forissier]

Hi Tom,

On 6/21/25 03:36, Tom Rini wrote:
> 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!

https://lists.denx.de/pipermail/u-boot/2025-June/592785.html

Regards,
-- 
Jerome