[PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
David Picard
david.picard at clermont.in2p3.fr
Wed Mar 18 00:49:02 PDT 2026
Hi,
The boot log with CONFIG_DEBUG_LL enabled for 1st and 2nd stage:
https://paste.debian.net/hidden/d82f6a59
David
Le 17/03/2026 à 17:11, Ahmad Fatoum a écrit :
> Hello David,
>
> On 3/17/26 5:03 PM, David Picard wrote:
>> I applied the patch on 36b7e6f4245cb29e3e95cd9e50bc49ad61ba9c2b.
>>
>> The boot log: https://paste.debian.net/hidden/7fa6ec69
> Is it possible to enable CONFIG_DEBUG_LL also in the first stage?
>
> I assume that's what was hanging with the patch applied, so debug output
> from there would be valuable.
>
> Thanks,
> Ahmad
>
>> 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 at 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 at anthropic.com>
>>>>> Reviewed-by: Ahmad Fatoum <a.fatoum at pengutronix.de>
>>>>> Link: https://lore.barebox.org/20260304-arm-setup-c-v2-2-
>>>>> d26af520ab03 at pengutronix.de
>>>>> Signed-off-by: Sascha Hauer <s.hauer at 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();
>>>>
>>
More information about the barebox
mailing list