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

[Sascha Hauer <s.hauer@pengutronix.de>]

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 |