Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice

[David Picard <david.picard@clermont.in2p3.fr>]

I applied the patch on 36b7e6f4245cb29e3e95cd9e50bc49ad61ba9c2b.

The boot log: https://paste.debian.net/hidden/7fa6ec69

David


Le 17/03/2026 à 16:56, Ahmad Fatoum a écrit :
> Hello David,
>
> On 3/17/26 1:43 PM, David Picard wrote:
>> I tested from the last but one commit on master, but I don't think it's
>> a big deal.
>>
>> Anyway, my board boots. Congrats!
> I think you missed Sascha's patch at the end of his mail.
>
> The change that has been dropped from next is a building block for a
> boot time optimization and it would be very helpful to understand what
> exactly broke at your side, so a revised patch can be applied.
>
> To help with that, Sascha included a patch with some extra debug output.
>
> Cheers,
> Ahmad
>
>> David
>>
>> Le 17/03/2026 à 10:06, Sascha Hauer a écrit :
>>> Hi David,
>>>
>>> On Mon, Mar 16, 2026 at 02:21:37PM +0100, David Picard wrote:
>>>> Hi,
>>>>
>>>> I would like this patches be reverted, since, despite the proposed
>>>> fixes, it
>>>> prevents the board arch/arm/boards/enclustra-sa2 to boot.
>>> I reverted this patch for now, but nevertheless I'd like to understand
>>> what's wrong with it.
>>>
>>>> With the option CONFIG_DEBUG_LL enabled, the output is:
>>>>
>>>> lowlevel init done
>>>> SDRAM setup...
>>>> SDRAM calibration...
>>>> done
>>>>
>>>> Then, it hangs.
>>> Could you apply the following patch to current master and see what the
>>> output is?
>>>
>>> Thanks
>>>    Sascha
>>>
>>> ----------------------------------8<-----------------------------------
>>>
>>>   From b784bc80756d127ae4a824ad235d4e113d5dcc1f Mon Sep 17 00:00:00 2001
>>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>> Date: Wed, 4 Mar 2026 08:53:22 +0100
>>> Subject: [PATCH] ARM: setup_c: avoid clearing BSS twice
>>>
>>> Add a bss_cleared variable that is set after the first call to
>>> setup_c(). On subsequent calls, the BSS clear is skipped to avoid
>>> wiping already-initialized data.
>>>
>>> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
>>> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> Link: https://lore.barebox.org/20260304-arm-setup-c-v2-2-
>>> d26af520ab03@pengutronix.de
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>    arch/arm/cpu/setupc_32.S  | 16 ++++++++++++++--
>>>    arch/arm/cpu/setupc_64.S  | 15 +++++++++++++--
>>>    arch/arm/cpu/uncompress.c | 15 +++++++++++++++
>>>    3 files changed, 42 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/setupc_32.S b/arch/arm/cpu/setupc_32.S
>>> index fa31f94442..fadfc28ae1 100644
>>> --- a/arch/arm/cpu/setupc_32.S
>>> +++ b/arch/arm/cpu/setupc_32.S
>>> @@ -7,18 +7,30 @@
>>>    .section .text.setupc
>>>      /*
>>> - * setup_c: clear bss
>>> + * setup_c: clear bss if not yet done
>>>     */
>>>    ENTRY(setup_c)
>>>        push    {r4, lr}
>>> +    ldr    r0, =bss_cleared
>>> +    ldr    r1, [r0]
>>> +    cmp    r1, #0
>>> +    bne    1f            /* skip if already done */
>>>        ldr    r0, =__bss_start
>>>        mov    r1, #0
>>>        ldr    r2, =__bss_stop
>>>        sub    r2, r2, r0
>>>        bl    __memset        /* clear bss */
>>> -    pop    {r4, pc}
>>> +    ldr    r0, =bss_cleared
>>> +    mov    r1, #1
>>> +    str    r1, [r0]        /* mark bss cleared */
>>> +1:    pop    {r4, pc}
>>>    ENDPROC(setup_c)
>>>    +.section .data.bss_cleared
>>> +.balign 4
>>> +bss_cleared:
>>> +    .word     0
>>> +
>>>    /*
>>>     * void relocate_to_adr(unsigned long targetadr)
>>>     *
>>> diff --git a/arch/arm/cpu/setupc_64.S b/arch/arm/cpu/setupc_64.S
>>> index 18ab7ff3fe..a0767c5136 100644
>>> --- a/arch/arm/cpu/setupc_64.S
>>> +++ b/arch/arm/cpu/setupc_64.S
>>> @@ -7,19 +7,30 @@
>>>    .section .text.setupc
>>>      /*
>>> - * setup_c: clear bss
>>> + * setup_c: clear bss if not yet done
>>>     */
>>>    ENTRY(setup_c)
>>> +    adr_l    x0, bss_cleared
>>> +    ldr    w1, [x0]
>>> +    cbnz    w1, 1f            /* skip if already done */
>>>        mov    x15, x30
>>>        adr_l    x0, __bss_start
>>>        mov    x1, #0
>>>        adr_l    x2, __bss_stop
>>>        sub    x2, x2, x0
>>>        bl    __memset        /* clear bss */
>>> +    adr_l    x0, bss_cleared
>>> +    mov    w1, #1
>>> +    str    w1, [x0]        /* mark bss cleared */
>>>        mov    x30, x15
>>> -    ret
>>> +1:    ret
>>>    ENDPROC(setup_c)
>>>    +.section .data.bss_cleared
>>> +.balign 4
>>> +bss_cleared:
>>> +    .word     0
>>> +
>>>    /*
>>>     * void relocate_to_adr(unsigned long targetadr)
>>>     *
>>> diff --git a/arch/arm/cpu/uncompress.c b/arch/arm/cpu/uncompress.c
>>> index 38f7dbc113..bde0f2a0a5 100644
>>> --- a/arch/arm/cpu/uncompress.c
>>> +++ b/arch/arm/cpu/uncompress.c
>>> @@ -50,6 +50,15 @@ void __noreturn barebox_pbl_start(unsigned long
>>> membase, unsigned long memsize,
>>>        pg_start = runtime_address(input_data);
>>>        pg_end = runtime_address(input_data_end);
>>>    +    puthex_ll((unsigned long)pg_start); putc_ll('\r'); putc_ll('\n');
>>> +    puthex_ll((unsigned long)pg_end); putc_ll('\r'); putc_ll('\n');
>>> +    puthex_ll((unsigned long)pc); putc_ll('\r'); putc_ll('\n');
>>> +    puthex_ll(membase); putc_ll('\r'); putc_ll('\n');
>>> +    puthex_ll(memsize); putc_ll('\r'); putc_ll('\n');
>>> +    puthex_ll((unsigned long)&__bss_start); putc_ll('\r');
>>> putc_ll('\n');
>>> +    puthex_ll((unsigned long)&__bss_stop); putc_ll('\r'); putc_ll('\n');
>>> +    puthex_ll((unsigned long)&__image_end); putc_ll('\r');
>>> putc_ll('\n');
>>> +
>>>        /*
>>>         * If we run from inside the memory just relocate the binary
>>>         * to the current address. Otherwise it may be a readonly location.
>>> @@ -60,11 +69,17 @@ void __noreturn barebox_pbl_start(unsigned long
>>> membase, unsigned long memsize,
>>>        else
>>>            relocate_to_adr(membase);
>>>    +    putc_ll('A');
>>> +
>>>        pg_len = pg_end - pg_start;
>>>        uncompressed_len = get_unaligned((const u32 *)(pg_start + pg_len
>>> - 4));
>>>    +    putc_ll('B');
>>> +
>>>        setup_c();
>>>    +    putc_ll('C');
>>> +
>>>        pr_debug("memory at 0x%08lx, size 0x%08lx\n", membase, memsize);
>>>          arm_pbl_init_exceptions();
>>
>>