SPL: sunxi: don't force .BSS into DRAM

PATCH New Andre Przywara 2 participants 3 messages Jun 30, 2016

Subsystems (3)

Arm (32-bit) Library code Arm/Allwinner sunXi SoC support

lore

Share & tools

lore

Andre Przywara

author

2016-06-30 00:29 UTC #0 lore raw

PATCH 1/1 SPL: sunxi: don't force .BSS into DRAM

Probably due to some (ill-founded) fear of a large BSS all sunxi boards forced their SPL BSS section into DRAM.
This only works if there is no usage of a .BSS variable before the DRAM is initialised.
The recent inclusion of tiny-printf breaks this assumption (it has two variables in .BSS), so any early printf (printing a number) hangs a board. This in particular breaks the (WIP) Pine64 SPL, which at the moment links Allwinner's libdram library, trying to print debug information:
DRAM:DRAM driver version: V1.0
DRAM Type = <hangs>

As it turns out the normal BSS size for sunxi is about 256 Bytes, so we can happily remove the symbols and the linker script part that was forcing the section into DRAM and let the linker naturally put it into SRAM.
Tested on BananaPi M1 and Pine64(-SPL), also buildman sunxi was happy.

Thanks to Siarhei for providing helpful hints!

Signed-off-by Andre Przywara <[email protected]>

---

(and now with the list in CC: as well) ...

 arch/arm/cpu/armv7/sunxi/u-boot-spl.lds | 4 +---
 include/configs/sunxi-common.h          | 4 ----
 2 files changed, 1 insertion(+), 7 deletions(-)

arch/arm/cpu/armv7/sunxi/u-boot-spl.lds +1 -3

index 53f0cbd..a90404f 100644
@@ -16,8 +16,6 @@
  */
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
 		LENGTH = CONFIG_SPL_MAX_SIZE }
-MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR, \
-		LENGTH = CONFIG_SPL_BSS_MAX_SIZE }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)

@@ -54,5 +52,5 @@ SECTIONS
 		*(.bss*)
 		. = ALIGN(4);
 		__bss_end = .;
-	} > .sdram
+	} > .sram
 }

include/configs/sunxi-common.h +0 -4

index 94275a7..e3fe965 100644
@@ -75,7 +75,6 @@
  * since it needs to fit in with the other values. By also #defining it
  * we get warnings if the Kconfig value mismatches. */
 #define CONFIG_SPL_STACK_R_ADDR		0x2fe00000
-#define CONFIG_SPL_BSS_START_ADDR	0x2ff80000
 #else
 #define SDRAM_OFFSET(x) 0x4##x
 #define CONFIG_SYS_SDRAM_BASE		0x40000000

@@ -86,11 +85,8 @@
  * since it needs to fit in with the other values. By also #defining it
  * we get warnings if the Kconfig value mismatches. */
 #define CONFIG_SPL_STACK_R_ADDR		0x4fe00000
-#define CONFIG_SPL_BSS_START_ADDR	0x4ff80000
 #endif
 
-#define CONFIG_SPL_BSS_MAX_SIZE		0x00080000 /* 512 KiB */
-
 #if defined(CONFIG_MACH_SUN9I) || defined(CONFIG_MACH_SUN50I)
 /*
  * The A80's A1 sram starts at 0x00010000 rather then at 0x00000000 and is

Signature · 1 line

2.9.0

Marek Vasut

2016-06-30 01:54 UTC #1 lore raw

PATCH 1/1 SPL: sunxi: don't force .BSS into DRAM

Quoted 6 lines · Andre Przywara

Probably due to some (ill-founded) fear of a large BSS all sunxi boards forced their SPL BSS section into DRAM.
This only works if there is no usage of a .BSS variable before the DRAM is initialised.
The recent inclusion of tiny-printf breaks this assumption (it has two variables in .BSS), so any early printf (printing a number) hangs a board.

I believe you should fix tiny-printf instead, try this patch:

lib/tiny-printf.c +1 -1

index 451f4f7..5b9b0dc 100644
@@ -17,7 +17,7 @@ static char *bf;
 static char zs;
 
 /* Current position in sprintf() output string */
-static char *outstr;
+static char *outstr __section(".data");
 
 static void out(char c)
 {

Quoted 9 lines

This in particular breaks the (WIP) Pine64 SPL, which at the moment links Allwinner's libdram library, trying to print debug information:
DRAM:DRAM driver version: V1.0
DRAM Type = <hangs>
As it turns out the normal BSS size for sunxi is about 256 Bytes, so we can happily remove the symbols and the linker script part that was forcing the section into DRAM and let the linker naturally put it into SRAM.

Except SRAM is limited, which is why bss was in DRAM.

Quoted 58 lines

Tested on BananaPi M1 and Pine64(-SPL), also buildman sunxi was happy.

Thanks to Siarhei for providing helpful hints!

Signed-off-by Andre Przywara <[email protected]>

---

(and now with the list in CC: as well) ...

 arch/arm/cpu/armv7/sunxi/u-boot-spl.lds | 4 +---
 include/configs/sunxi-common.h          | 4 ----
 2 files changed, 1 insertion(+), 7 deletions(-)

arch/arm/cpu/armv7/sunxi/u-boot-spl.lds +1 -3

index 53f0cbd..a90404f 100644
@@ -16,8 +16,6 @@
  */
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
 		LENGTH = CONFIG_SPL_MAX_SIZE }
-MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR, \
-		LENGTH = CONFIG_SPL_BSS_MAX_SIZE }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)

@@ -54,5 +52,5 @@ SECTIONS
 		*(.bss*)
 		. = ALIGN(4);
 		__bss_end = .;
-	} > .sdram
+	} > .sram
 }

include/configs/sunxi-common.h +0 -4

index 94275a7..e3fe965 100644
@@ -75,7 +75,6 @@
  * since it needs to fit in with the other values. By also #defining it
  * we get warnings if the Kconfig value mismatches. */
 #define CONFIG_SPL_STACK_R_ADDR		0x2fe00000
-#define CONFIG_SPL_BSS_START_ADDR	0x2ff80000
 #else
 #define SDRAM_OFFSET(x) 0x4##x
 #define CONFIG_SYS_SDRAM_BASE		0x40000000

@@ -86,11 +85,8 @@
  * since it needs to fit in with the other values. By also #defining it
  * we get warnings if the Kconfig value mismatches. */
 #define CONFIG_SPL_STACK_R_ADDR		0x4fe00000
-#define CONFIG_SPL_BSS_START_ADDR	0x4ff80000
 #endif
 
-#define CONFIG_SPL_BSS_MAX_SIZE		0x00080000 /* 512 KiB */
-
 #if defined(CONFIG_MACH_SUN9I) || defined(CONFIG_MACH_SUN50I)
 /*
  * The A80's A1 sram starts at 0x00010000 rather then at 0x00000000 and is

Signature · 2 lines

Best regards,
Marek Vasut

Andre Przywara

author

2016-06-30 08:50 UTC #2 lore raw

PATCH 1/1 SPL: sunxi: don't force .BSS into DRAM

Hi Marek,

Quoted 9 lines · Marek Vasut

Quoted 6 lines · Andre Przywara
Probably due to some (ill-founded) fear of a large BSS all sunxi boards forced their SPL BSS section into DRAM.
This only works if there is no usage of a .BSS variable before the DRAM is initialised.
The recent inclusion of tiny-printf breaks this assumption (it has two variables in .BSS), so any early printf (printing a number) hangs a board.
I believe you should fix tiny-printf instead, try this patch:

Mmmh, that looks like a hack to paper over another hack for me. I don't think we should start to annotate seemingly random variables (at least in the code) just to cover up for some linker hack.
And frankly, having bss in initially inaccessible memory without documenting this or enforcing sanity checks to catch those cases like tiny-printf seems quite hacky to me.
Also if I am not mistaken we don't actually clear BSS for the SPL?

I could live with some compiler switch or linker script snippet to avoid using BSS variables for the SPL (or parts of it, at least).

Or we provide a per SoC BSS offset to utilise other SRAM locations and use that for BSS.

lib/tiny-printf.c static char *bf; · line 20

Quoted 10 lines

lib/tiny-printf.c +1 -1

index 451f4f7..5b9b0dc 100644
@@ -17,7 +17,7 @@ static char *bf;
 static char zs;
 
 /* Current position in sprintf() output string */
-static char *outstr;
+static char *outstr __section(".data");

To prove my debugging theory I initialised _both_ those variables to 1, that also worked. But definitely you need to cover "zs" as well.

Quoted 9 lines

lib/tiny-printf.c +0 -0
 static void out(char c)
 {
 
Quoted 5 lines
This in particular breaks the (WIP) Pine64 SPL, which at the moment links Allwinner's libdram library, trying to print debug information:
DRAM:DRAM driver version: V1.0
DRAM Type = <hangs>

Quoted 6 lines

Quoted 4 lines
As it turns out the normal BSS size for sunxi is about 256 Bytes, so we can happily remove the symbols and the linker script part that was forcing the section into DRAM and let the linker naturally put it into SRAM.
Except SRAM is limited, which is why bss was in DRAM.

Except that DRAM is not available initially ;-)

Cheers,
Andre.

Quoted 61 lines

Quoted 59 lines

Tested on BananaPi M1 and Pine64(-SPL), also buildman sunxi was happy.

Thanks to Siarhei for providing helpful hints!

Signed-off-by Andre Przywara <[email protected]>

---

(and now with the list in CC: as well) ...

 arch/arm/cpu/armv7/sunxi/u-boot-spl.lds | 4 +---
 include/configs/sunxi-common.h          | 4 ----
 2 files changed, 1 insertion(+), 7 deletions(-)

arch/arm/cpu/armv7/sunxi/u-boot-spl.lds +1 -3

index 53f0cbd..a90404f 100644
@@ -16,8 +16,6 @@
  */
 MEMORY { .sram : ORIGIN = CONFIG_SPL_TEXT_BASE,\
 		LENGTH = CONFIG_SPL_MAX_SIZE }
-MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR, \
-		LENGTH = CONFIG_SPL_BSS_MAX_SIZE }
 
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)

@@ -54,5 +52,5 @@ SECTIONS
 		*(.bss*)
 		. = ALIGN(4);
 		__bss_end = .;
-	} > .sdram
+	} > .sram
 }

include/configs/sunxi-common.h +0 -4

index 94275a7..e3fe965 100644
@@ -75,7 +75,6 @@
  * since it needs to fit in with the other values. By also #defining it
  * we get warnings if the Kconfig value mismatches. */
 #define CONFIG_SPL_STACK_R_ADDR		0x2fe00000
-#define CONFIG_SPL_BSS_START_ADDR	0x2ff80000
 #else
 #define SDRAM_OFFSET(x) 0x4##x
 #define CONFIG_SYS_SDRAM_BASE		0x40000000

@@ -86,11 +85,8 @@
  * since it needs to fit in with the other values. By also #defining it
  * we get warnings if the Kconfig value mismatches. */
 #define CONFIG_SPL_STACK_R_ADDR		0x4fe00000
-#define CONFIG_SPL_BSS_START_ADDR	0x4ff80000
 #endif
 
-#define CONFIG_SPL_BSS_MAX_SIZE		0x00080000 /* 512 KiB */
-
 #if defined(CONFIG_MACH_SUN9I) || defined(CONFIG_MACH_SUN50I)
 /*
  * The A80's A1 sram starts at 0x00010000 rather then at 0x00000000 and is