* [PATCH v2 0/2] ARM: Avoid to clear bss multiple times
@ 2026-03-04 7:53 Sascha Hauer
2026-03-04 7:53 ` [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c Sascha Hauer
` (3 more replies)
0 siblings, 4 replies; 24+ messages in thread
From: Sascha Hauer @ 2026-03-04 7:53 UTC (permalink / raw)
To: BAREBOX; +Cc: Claude Opus 4.6
Currently we clear the bss everytime we call setup_c(). We can do better
and clear it only once which makes for example alloc_pte() in the PBL
MMU setup preserve its state and we can call mmu_early_enable() multiple
times without harm.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Changes in v2:
- Avoid to put the information if BSS is cleared into the BSS (Ahmad)
- Link to v1: https://lore.barebox.org/20260226-arm-setup-c-v1-0-f52fe8ee81ba@pengutronix.de
---
Sascha Hauer (2):
ARM: setupc_32: remove relocation code from setup_c
ARM: setup_c: avoid clearing BSS twice
arch/arm/cpu/setupc_32.S | 38 ++++++++++++++++----------------------
arch/arm/cpu/setupc_64.S | 14 ++++++++++++--
2 files changed, 28 insertions(+), 24 deletions(-)
---
base-commit: bd5518cd7e34861706f87c8ca67b640d6257d5b8
change-id: 20260226-arm-setup-c-30f0dccdde71
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c
2026-03-04 7:53 [PATCH v2 0/2] ARM: Avoid to clear bss multiple times Sascha Hauer
@ 2026-03-04 7:53 ` Sascha Hauer
2026-03-04 9:02 ` Ahmad Fatoum
2026-03-11 14:41 ` Ahmad Fatoum
2026-03-04 7:53 ` [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice Sascha Hauer
` (2 subsequent siblings)
3 siblings, 2 replies; 24+ messages in thread
From: Sascha Hauer @ 2026-03-04 7:53 UTC (permalink / raw)
To: BAREBOX; +Cc: Claude Opus 4.6
All callers of setup_c() already call relocate_to_current_adr() or
relocate_to_adr() before setup_c(), so the relocation (memcpy) code
in setup_c is dead and also the sync_caches_for_execution() is unnecessary
Remove it and reduce setup_c to just clearing BSS.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
arch/arm/cpu/setupc_32.S | 27 +++++----------------------
1 file changed, 5 insertions(+), 22 deletions(-)
diff --git a/arch/arm/cpu/setupc_32.S b/arch/arm/cpu/setupc_32.S
index 0134637f62..c2c3f97528 100644
--- a/arch/arm/cpu/setupc_32.S
+++ b/arch/arm/cpu/setupc_32.S
@@ -7,33 +7,16 @@
.section .text.setupc
/*
- * setup_c: copy binary to link address, clear bss and
- * continue executing at new address.
- *
- * This function does not return to the address it is
- * called from, but to the same location in the copied
- * binary.
+ * setup_c: clear bss
*/
ENTRY(setup_c)
- push {r4, r5}
- mov r5, lr
- bl get_runtime_offset
- subs r4, r0, #0
- beq 1f /* skip memcpy if already at correct address */
- ldr r0,=_text
- ldr r2,=__bss_start
- sub r2, r2, r0
- add r1, r0, r4
- bl __memcpy /* memcpy(_text, _text + offset, __bss_start - _text) */
-1: ldr r0, =__bss_start
+ mov r4, lr
+ ldr r0, =__bss_start
mov r1, #0
ldr r2, =__bss_stop
sub r2, r2, r0
- bl __memset /* clear bss */
- bl sync_caches_for_execution
- sub lr, r5, r4 /* adjust return address to new location */
- pop {r4, r5}
- ret lr
+ bl __memset /* clear bss */
+ ret r4
ENDPROC(setup_c)
/*
--
2.47.3
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-04 7:53 [PATCH v2 0/2] ARM: Avoid to clear bss multiple times Sascha Hauer
2026-03-04 7:53 ` [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c Sascha Hauer
@ 2026-03-04 7:53 ` Sascha Hauer
2026-03-04 9:05 ` Ahmad Fatoum
2026-03-10 10:42 ` [PATCH v2 0/2] ARM: Avoid to clear bss multiple times Sascha Hauer
2026-03-17 10:21 ` Sascha Hauer
3 siblings, 1 reply; 24+ messages in thread
From: Sascha Hauer @ 2026-03-04 7:53 UTC (permalink / raw)
To: BAREBOX; +Cc: Claude Opus 4.6
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>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
arch/arm/cpu/setupc_32.S | 15 +++++++++++++--
arch/arm/cpu/setupc_64.S | 14 ++++++++++++--
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/setupc_32.S b/arch/arm/cpu/setupc_32.S
index c2c3f97528..4a5e2f9e96 100644
--- a/arch/arm/cpu/setupc_32.S
+++ b/arch/arm/cpu/setupc_32.S
@@ -7,18 +7,29 @@
.section .text.setupc
/*
- * setup_c: clear bss
+ * setup_c: clear bss if not yet done
*/
ENTRY(setup_c)
mov 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 */
- ret r4
+ ldr r0, =bss_cleared
+ mov r1, #1
+ str r1, [r0] /* mark bss cleared */
+1: ret r4
ENDPROC(setup_c)
+.section .data
+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 fd95187a04..caa16d3582 100644
--- a/arch/arm/cpu/setupc_64.S
+++ b/arch/arm/cpu/setupc_64.S
@@ -7,19 +7,29 @@
.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:
+ .word 0
+
/*
* void relocate_to_adr(unsigned long targetadr)
*
--
2.47.3
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c
2026-03-04 7:53 ` [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c Sascha Hauer
@ 2026-03-04 9:02 ` Ahmad Fatoum
2026-03-11 14:41 ` Ahmad Fatoum
1 sibling, 0 replies; 24+ messages in thread
From: Ahmad Fatoum @ 2026-03-04 9:02 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX; +Cc: Claude Opus 4.6
On 3/4/26 8:53 AM, Sascha Hauer wrote:
> All callers of setup_c() already call relocate_to_current_adr() or
> relocate_to_adr() before setup_c(), so the relocation (memcpy) code
> in setup_c is dead and also the sync_caches_for_execution() is unnecessary
> Remove it and reduce setup_c to just clearing BSS.
>
> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> arch/arm/cpu/setupc_32.S | 27 +++++----------------------
> 1 file changed, 5 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/cpu/setupc_32.S b/arch/arm/cpu/setupc_32.S
> index 0134637f62..c2c3f97528 100644
> --- a/arch/arm/cpu/setupc_32.S
> +++ b/arch/arm/cpu/setupc_32.S
> @@ -7,33 +7,16 @@
> .section .text.setupc
>
> /*
> - * setup_c: copy binary to link address, clear bss and
> - * continue executing at new address.
> - *
> - * This function does not return to the address it is
> - * called from, but to the same location in the copied
> - * binary.
> + * setup_c: clear bss
> */
> ENTRY(setup_c)
> - push {r4, r5}
> - mov r5, lr
> - bl get_runtime_offset
> - subs r4, r0, #0
> - beq 1f /* skip memcpy if already at correct address */
> - ldr r0,=_text
> - ldr r2,=__bss_start
> - sub r2, r2, r0
> - add r1, r0, r4
> - bl __memcpy /* memcpy(_text, _text + offset, __bss_start - _text) */
> -1: ldr r0, =__bss_start
> + mov r4, lr
> + ldr r0, =__bss_start
> mov r1, #0
> ldr r2, =__bss_stop
> sub r2, r2, r0
> - bl __memset /* clear bss */
> - bl sync_caches_for_execution
> - sub lr, r5, r4 /* adjust return address to new location */
> - pop {r4, r5}
> - ret lr
> + bl __memset /* clear bss */
> + ret r4
> ENDPROC(setup_c)
>
> /*
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-04 7:53 ` [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice Sascha Hauer
@ 2026-03-04 9:05 ` Ahmad Fatoum
2026-03-10 10:44 ` Sascha Hauer
0 siblings, 1 reply; 24+ messages in thread
From: Ahmad Fatoum @ 2026-03-04 9:05 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX; +Cc: Claude Opus 4.6
Hi,
On 3/4/26 8:53 AM, Sascha Hauer wrote:
> 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>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Minor point below:
> +.section .data
.data.bss_cleared or is it already in its own section?
> +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 fd95187a04..caa16d3582 100644
> --- a/arch/arm/cpu/setupc_64.S
> +++ b/arch/arm/cpu/setupc_64.S
> @@ -7,19 +7,29 @@
> .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
Same question
> +bss_cleared:
> + .word 0
> +
> /*
> * void relocate_to_adr(unsigned long targetadr)
> *
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/2] ARM: Avoid to clear bss multiple times
2026-03-04 7:53 [PATCH v2 0/2] ARM: Avoid to clear bss multiple times Sascha Hauer
2026-03-04 7:53 ` [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c Sascha Hauer
2026-03-04 7:53 ` [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice Sascha Hauer
@ 2026-03-10 10:42 ` Sascha Hauer
2026-03-17 10:21 ` Sascha Hauer
3 siblings, 0 replies; 24+ messages in thread
From: Sascha Hauer @ 2026-03-10 10:42 UTC (permalink / raw)
To: BAREBOX, Sascha Hauer; +Cc: Claude Opus 4.6
On Wed, 04 Mar 2026 08:53:20 +0100, Sascha Hauer wrote:
> Currently we clear the bss everytime we call setup_c(). We can do better
> and clear it only once which makes for example alloc_pte() in the PBL
> MMU setup preserve its state and we can call mmu_early_enable() multiple
> times without harm.
>
>
Applied, thanks!
[1/2] ARM: setupc_32: remove relocation code from setup_c
https://git.pengutronix.de/cgit/barebox/commit/?id=ccbeb0b5d78c (link may not be stable)
[2/2] ARM: setup_c: avoid clearing BSS twice
https://git.pengutronix.de/cgit/barebox/commit/?id=e3a134753029 (link may not be stable)
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-04 9:05 ` Ahmad Fatoum
@ 2026-03-10 10:44 ` Sascha Hauer
2026-03-16 13:21 ` David Picard
0 siblings, 1 reply; 24+ messages in thread
From: Sascha Hauer @ 2026-03-10 10:44 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: BAREBOX, Claude Opus 4.6
On Wed, Mar 04, 2026 at 10:05:38AM +0100, Ahmad Fatoum wrote:
> Hi,
>
> On 3/4/26 8:53 AM, Sascha Hauer wrote:
> > 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>
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>
> Minor point below:
>
> > +.section .data
>
> .data.bss_cleared or is it already in its own section?
Fixed while applying.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c
2026-03-04 7:53 ` [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c Sascha Hauer
2026-03-04 9:02 ` Ahmad Fatoum
@ 2026-03-11 14:41 ` Ahmad Fatoum
2026-03-11 19:08 ` Sascha Hauer
1 sibling, 1 reply; 24+ messages in thread
From: Ahmad Fatoum @ 2026-03-11 14:41 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX, David Picard; +Cc: Claude Opus 4.6
Hello Sascha,
On 3/4/26 8:53 AM, Sascha Hauer wrote:
> All callers of setup_c() already call relocate_to_current_adr() or
> relocate_to_adr() before setup_c(), so the relocation (memcpy) code
> in setup_c is dead and also the sync_caches_for_execution() is unnecessary
> Remove it and reduce setup_c to just clearing BSS.
David reports this breaks his enclustra-sa2 and I observe also a hang on
Qemu Virt:
$ make multi_v7_defconfig
$ pytest --interactive
Using -bios device tree at 40000000
<hang>
Please drop from next, it's likely also what's breaking CI.
Cheers,
Ahmad
>
> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> arch/arm/cpu/setupc_32.S | 27 +++++----------------------
> 1 file changed, 5 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/cpu/setupc_32.S b/arch/arm/cpu/setupc_32.S
> index 0134637f62..c2c3f97528 100644
> --- a/arch/arm/cpu/setupc_32.S
> +++ b/arch/arm/cpu/setupc_32.S
> @@ -7,33 +7,16 @@
> .section .text.setupc
>
> /*
> - * setup_c: copy binary to link address, clear bss and
> - * continue executing at new address.
> - *
> - * This function does not return to the address it is
> - * called from, but to the same location in the copied
> - * binary.
> + * setup_c: clear bss
> */
> ENTRY(setup_c)
> - push {r4, r5}
> - mov r5, lr
> - bl get_runtime_offset
> - subs r4, r0, #0
> - beq 1f /* skip memcpy if already at correct address */
> - ldr r0,=_text
> - ldr r2,=__bss_start
> - sub r2, r2, r0
> - add r1, r0, r4
> - bl __memcpy /* memcpy(_text, _text + offset, __bss_start - _text) */
> -1: ldr r0, =__bss_start
> + mov r4, lr
> + ldr r0, =__bss_start
> mov r1, #0
> ldr r2, =__bss_stop
> sub r2, r2, r0
> - bl __memset /* clear bss */
> - bl sync_caches_for_execution
> - sub lr, r5, r4 /* adjust return address to new location */
> - pop {r4, r5}
> - ret lr
> + bl __memset /* clear bss */
> + ret r4
> ENDPROC(setup_c)
>
> /*
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c
2026-03-11 14:41 ` Ahmad Fatoum
@ 2026-03-11 19:08 ` Sascha Hauer
0 siblings, 0 replies; 24+ messages in thread
From: Sascha Hauer @ 2026-03-11 19:08 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: BAREBOX, David Picard, Claude Opus 4.6
On Wed, Mar 11, 2026 at 03:41:04PM +0100, Ahmad Fatoum wrote:
> Hello Sascha,
>
> On 3/4/26 8:53 AM, Sascha Hauer wrote:
> > All callers of setup_c() already call relocate_to_current_adr() or
> > relocate_to_adr() before setup_c(), so the relocation (memcpy) code
> > in setup_c is dead and also the sync_caches_for_execution() is unnecessary
> > Remove it and reduce setup_c to just clearing BSS.
>
> David reports this breaks his enclustra-sa2 and I observe also a hang on
> Qemu Virt:
>
> $ make multi_v7_defconfig
> $ pytest --interactive
> Using -bios device tree at 40000000
> <hang>
>
> Please drop from next, it's likely also what's breaking CI.
r4 should be restored before returning. Let's try again with the fix
below.
Sascha
---------------------------8<------------------------------
>From 139e75d06150896add0555f2164dac0c7f90e644 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 11 Mar 2026 20:02:58 +0100
Subject: [PATCH] fixup! ARM: setupc_32: remove relocation code from setup_c
---
arch/arm/cpu/setupc_32.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/setupc_32.S b/arch/arm/cpu/setupc_32.S
index c2c3f97528..c3e53b220e 100644
--- a/arch/arm/cpu/setupc_32.S
+++ b/arch/arm/cpu/setupc_32.S
@@ -10,13 +10,13 @@
* setup_c: clear bss
*/
ENTRY(setup_c)
- mov r4, lr
+ push {r4, lr}
ldr r0, =__bss_start
mov r1, #0
ldr r2, =__bss_stop
sub r2, r2, r0
bl __memset /* clear bss */
- ret r4
+ pop {r4, pc}
ENDPROC(setup_c)
/*
--
2.47.3
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-10 10:44 ` Sascha Hauer
@ 2026-03-16 13:21 ` David Picard
2026-03-17 9:06 ` Sascha Hauer
0 siblings, 1 reply; 24+ messages in thread
From: David Picard @ 2026-03-16 13:21 UTC (permalink / raw)
To: barebox
Hi,
I would like this patches be reverted, since, despite the proposed
fixes, it prevents the board arch/arm/boards/enclustra-sa2 to boot.
With the option CONFIG_DEBUG_LL enabled, the output is:
lowlevel init done
SDRAM setup...
SDRAM calibration...
done
Then, it hangs.
David
Le 10/03/2026 à 11:44, Sascha Hauer a écrit :
> On Wed, Mar 04, 2026 at 10:05:38AM +0100, Ahmad Fatoum wrote:
>> Hi,
>>
>> On 3/4/26 8:53 AM, Sascha Hauer wrote:
>>> 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>
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>
>> Minor point below:
>>
>>> +.section .data
>> .data.bss_cleared or is it already in its own section?
> Fixed while applying.
>
> Sascha
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-16 13:21 ` David Picard
@ 2026-03-17 9:06 ` Sascha Hauer
2026-03-17 9:50 ` David Picard
2026-03-17 12:43 ` David Picard
0 siblings, 2 replies; 24+ messages in thread
From: Sascha Hauer @ 2026-03-17 9:06 UTC (permalink / raw)
To: David Picard; +Cc: barebox
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();
--
2.47.3
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-17 9:06 ` Sascha Hauer
@ 2026-03-17 9:50 ` David Picard
2026-03-17 12:43 ` David Picard
1 sibling, 0 replies; 24+ messages in thread
From: David Picard @ 2026-03-17 9:50 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
Hi,
Sure, I'll do it later today. I need to fix something right now.
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();
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/2] ARM: Avoid to clear bss multiple times
2026-03-04 7:53 [PATCH v2 0/2] ARM: Avoid to clear bss multiple times Sascha Hauer
` (2 preceding siblings ...)
2026-03-10 10:42 ` [PATCH v2 0/2] ARM: Avoid to clear bss multiple times Sascha Hauer
@ 2026-03-17 10:21 ` Sascha Hauer
3 siblings, 0 replies; 24+ messages in thread
From: Sascha Hauer @ 2026-03-17 10:21 UTC (permalink / raw)
To: BAREBOX, Sascha Hauer; +Cc: Claude Opus 4.6
On Wed, 04 Mar 2026 08:53:20 +0100, Sascha Hauer wrote:
> Currently we clear the bss everytime we call setup_c(). We can do better
> and clear it only once which makes for example alloc_pte() in the PBL
> MMU setup preserve its state and we can call mmu_early_enable() multiple
> times without harm.
>
>
Applied, thanks!
[1/2] ARM: setupc_32: remove relocation code from setup_c
https://git.pengutronix.de/cgit/barebox/commit/?id=dd85266b1fa3 (link may not be stable)
[2/2] ARM: setup_c: avoid clearing BSS twice
(no commit info)
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-17 9:06 ` Sascha Hauer
2026-03-17 9:50 ` David Picard
@ 2026-03-17 12:43 ` David Picard
2026-03-17 15:56 ` Ahmad Fatoum
1 sibling, 1 reply; 24+ messages in thread
From: David Picard @ 2026-03-17 12:43 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
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!
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();
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-17 12:43 ` David Picard
@ 2026-03-17 15:56 ` Ahmad Fatoum
2026-03-17 16:03 ` David Picard
0 siblings, 1 reply; 24+ messages in thread
From: Ahmad Fatoum @ 2026-03-17 15:56 UTC (permalink / raw)
To: David Picard, Sascha Hauer; +Cc: barebox
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();
>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-17 15:56 ` Ahmad Fatoum
@ 2026-03-17 16:03 ` David Picard
2026-03-17 16:11 ` Ahmad Fatoum
0 siblings, 1 reply; 24+ messages in thread
From: David Picard @ 2026-03-17 16:03 UTC (permalink / raw)
To: Ahmad Fatoum, Sascha Hauer; +Cc: barebox
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();
>>
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-17 16:03 ` David Picard
@ 2026-03-17 16:11 ` Ahmad Fatoum
2026-03-18 7:49 ` David Picard
0 siblings, 1 reply; 24+ messages in thread
From: Ahmad Fatoum @ 2026-03-17 16:11 UTC (permalink / raw)
To: David Picard, Sascha Hauer; +Cc: barebox
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@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();
>>>
>>>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-17 16:11 ` Ahmad Fatoum
@ 2026-03-18 7:49 ` David Picard
2026-03-18 8:00 ` Sascha Hauer
0 siblings, 1 reply; 24+ messages in thread
From: David Picard @ 2026-03-18 7:49 UTC (permalink / raw)
To: Ahmad Fatoum, Sascha Hauer; +Cc: barebox
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@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();
>>>>
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-18 7:49 ` David Picard
@ 2026-03-18 8:00 ` Sascha Hauer
2026-03-18 10:06 ` David Picard
0 siblings, 1 reply; 24+ messages in thread
From: Sascha Hauer @ 2026-03-18 8:00 UTC (permalink / raw)
To: David Picard; +Cc: Ahmad Fatoum, barebox
Hi David,
On Wed, Mar 18, 2026 at 08:49:02AM +0100, David Picard wrote:
> Hi,
>
> The boot log with CONFIG_DEBUG_LL enabled for 1st and 2nd stage:
> https://paste.debian.net/hidden/d82f6a59
This starts with:
> lowlevel init done
> SDRAM setup...
> SDRAM calibration...
> done
> Switch to console [cs0]
>
>
> barebox 2026.03.0 #1 Tue Mar 17 14:17:44 CET 2026
The binary has the same timestamp as the one you posted yesterday and it
doesn't have my patch applied, otherwise it should print the 8 hex
numbers followed by ABC, just like the 2nd stage shows.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-18 8:00 ` Sascha Hauer
@ 2026-03-18 10:06 ` David Picard
2026-03-18 14:12 ` David Picard
0 siblings, 1 reply; 24+ messages in thread
From: David Picard @ 2026-03-18 10:06 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Ahmad Fatoum, barebox
https://paste.debian.net/hidden/5de2d5d1
Le 18/03/2026 à 09:00, Sascha Hauer a écrit :
> Hi David,
>
> On Wed, Mar 18, 2026 at 08:49:02AM +0100, David Picard wrote:
>> Hi,
>>
>> The boot log with CONFIG_DEBUG_LL enabled for 1st and 2nd stage:
>> https://paste.debian.net/hidden/d82f6a59
> This starts with:
>
>> lowlevel init done
>> SDRAM setup...
>> SDRAM calibration...
>> done
>> Switch to console [cs0]
>>
>>
>> barebox 2026.03.0 #1 Tue Mar 17 14:17:44 CET 2026
> The binary has the same timestamp as the one you posted yesterday and it
> doesn't have my patch applied, otherwise it should print the 8 hex
> numbers followed by ABC, just like the 2nd stage shows.
>
> Sascha
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-18 10:06 ` David Picard
@ 2026-03-18 14:12 ` David Picard
2026-03-18 14:55 ` Sascha Hauer
0 siblings, 1 reply; 24+ messages in thread
From: David Picard @ 2026-03-18 14:12 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Ahmad Fatoum, barebox
Did it help? I can make more tests.
David
Le 18/03/2026 à 11:06, David Picard a écrit :
> https://paste.debian.net/hidden/5de2d5d1
>
> Le 18/03/2026 à 09:00, Sascha Hauer a écrit :
>> Hi David,
>>
>> On Wed, Mar 18, 2026 at 08:49:02AM +0100, David Picard wrote:
>>> Hi,
>>>
>>> The boot log with CONFIG_DEBUG_LL enabled for 1st and 2nd stage:
>>> https://paste.debian.net/hidden/d82f6a59
>> This starts with:
>>
>>> lowlevel init done
>>> SDRAM setup...
>>> SDRAM calibration...
>>> done
>>> Switch to console [cs0]
>>>
>>>
>>> barebox 2026.03.0 #1 Tue Mar 17 14:17:44 CET 2026
>> The binary has the same timestamp as the one you posted yesterday and it
>> doesn't have my patch applied, otherwise it should print the 8 hex
>> numbers followed by ABC, just like the 2nd stage shows.
>>
>> Sascha
>>
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-18 14:12 ` David Picard
@ 2026-03-18 14:55 ` Sascha Hauer
2026-03-18 15:43 ` David Picard
2026-03-23 13:22 ` David Picard
0 siblings, 2 replies; 24+ messages in thread
From: Sascha Hauer @ 2026-03-18 14:55 UTC (permalink / raw)
To: David Picard; +Cc: Ahmad Fatoum, barebox
On Wed, Mar 18, 2026 at 03:12:23PM +0100, David Picard wrote:
> Did it help? I can make more tests.
Thanks, yes it helps.
The patch you tested basically is the same as I reverted, modulo the
debug prints. So indeed the original patch seems to work. My assumption
is that you didn't have Ahmads fix applied which is on current master
now.
So I am going to re-apply this patch. Please let me know if something
still breaks.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-18 14:55 ` Sascha Hauer
@ 2026-03-18 15:43 ` David Picard
2026-03-23 13:22 ` David Picard
1 sibling, 0 replies; 24+ messages in thread
From: David Picard @ 2026-03-18 15:43 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Ahmad Fatoum, barebox
Oh, I'm sorry about that...
David
Le 18/03/2026 à 15:55, Sascha Hauer a écrit :
> On Wed, Mar 18, 2026 at 03:12:23PM +0100, David Picard wrote:
>> Did it help? I can make more tests.
> Thanks, yes it helps.
>
> The patch you tested basically is the same as I reverted, modulo the
> debug prints. So indeed the original patch seems to work. My assumption
> is that you didn't have Ahmads fix applied which is on current master
> now.
>
> So I am going to re-apply this patch. Please let me know if something
> still breaks.
>
> Sascha
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice
2026-03-18 14:55 ` Sascha Hauer
2026-03-18 15:43 ` David Picard
@ 2026-03-23 13:22 ` David Picard
1 sibling, 0 replies; 24+ messages in thread
From: David Picard @ 2026-03-23 13:22 UTC (permalink / raw)
To: Sascha Hauer; +Cc: Ahmad Fatoum, barebox
Hi,
I just tested today's master (0933e8f2ebf0d91dfcf177a4e4292b02921a53f1)
with your patch applied, and it works.
I did checked the timestamp, this time...
barebox 2026.03.0 #1 Mon Mar 23 14:07:47 CET 2026
David
Le 18/03/2026 à 15:55, Sascha Hauer a écrit :
> On Wed, Mar 18, 2026 at 03:12:23PM +0100, David Picard wrote:
>> Did it help? I can make more tests.
> Thanks, yes it helps.
>
> The patch you tested basically is the same as I reverted, modulo the
> debug prints. So indeed the original patch seems to work. My assumption
> is that you didn't have Ahmads fix applied which is on current master
> now.
>
> So I am going to re-apply this patch. Please let me know if something
> still breaks.
>
> Sascha
>
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2026-03-23 13:23 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-04 7:53 [PATCH v2 0/2] ARM: Avoid to clear bss multiple times Sascha Hauer
2026-03-04 7:53 ` [PATCH v2 1/2] ARM: setupc_32: remove relocation code from setup_c Sascha Hauer
2026-03-04 9:02 ` Ahmad Fatoum
2026-03-11 14:41 ` Ahmad Fatoum
2026-03-11 19:08 ` Sascha Hauer
2026-03-04 7:53 ` [PATCH v2 2/2] ARM: setup_c: avoid clearing BSS twice Sascha Hauer
2026-03-04 9:05 ` Ahmad Fatoum
2026-03-10 10:44 ` Sascha Hauer
2026-03-16 13:21 ` David Picard
2026-03-17 9:06 ` Sascha Hauer
2026-03-17 9:50 ` David Picard
2026-03-17 12:43 ` David Picard
2026-03-17 15:56 ` Ahmad Fatoum
2026-03-17 16:03 ` David Picard
2026-03-17 16:11 ` Ahmad Fatoum
2026-03-18 7:49 ` David Picard
2026-03-18 8:00 ` Sascha Hauer
2026-03-18 10:06 ` David Picard
2026-03-18 14:12 ` David Picard
2026-03-18 14:55 ` Sascha Hauer
2026-03-18 15:43 ` David Picard
2026-03-23 13:22 ` David Picard
2026-03-10 10:42 ` [PATCH v2 0/2] ARM: Avoid to clear bss multiple times Sascha Hauer
2026-03-17 10:21 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox