mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [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