Skip to contentSkip to conversation

malloc_simple: Allow malloc_simple to be used with non stack RAM

U-Boot New Hans de Goede 3 participants 11 messages Aug 17, 2015 – Aug 19, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hi Tom and Simon,

Here is an old patch of mine, which at the time I send it got reviewed by Simon, but never got merged.

I've just brushed this patch of, because it shrinks the SPL by about 3k, and with the spl nand support we've recently added we are going over the size limit with the SPL on some sunxi boards.

So I would like to see this merged. Simon, does your ack still stand for this patch ? Tom, if so can you merge this please ?

Note this patch has close to 0 chance of causing regressions as it is a nop unless the new CONFIG_SYS_MALLOC_F_BASE define is defined.

Patch 2 / 3 are merely added as example / for Ian to review :) I will merge these through the sunxi tree once patch 1 is merged.

Thanks & Regards,

Hans

#1 lore raw

PATCH 1/3 malloc_simple: Allow malloc_simple to be used with non stack RAM

Before this patch malloc_simple would always allocate a chunk of RAM from the stack. This commit adds a CONFIG_SYS_MALLOC_F_BASE define, which when set directly specifies the memory address to use for the heap with malloc_simple.

---
 arch/arm/lib/crt0.S | 2 +-
 common/board_f.c    | 4 ++++
 common/dlmalloc.c   | 4 ++++
 common/spl/spl.c    | 3 +++
 4 files changed, 12 insertions(+), 1 deletion(-)
arch/arm/lib/crt0.S +1 -1
index afd4f10..5e6619f 100644
@@ -96,7 +96,7 @@ clr_gd:
 	strlo	r0, [r1]		/* clear 32-bit GD word */
 	addlo	r1, r1, #4		/* move to next */
 	blo	clr_gd
-#if defined(CONFIG_SYS_MALLOC_F_LEN)
+#if defined(CONFIG_SYS_MALLOC_F_LEN) && !defined(CONFIG_SYS_MALLOC_F_BASE)
 	sub	sp, sp, #CONFIG_SYS_MALLOC_F_LEN
 	str	sp, [r9, #GD_MALLOC_BASE]
 #endif
common/board_f.c +4 -0
index c959774..7f3b96f 100644
@@ -1050,9 +1050,13 @@ ulong board_init_f_mem(ulong top)
 	arch_setup_gd(gd_ptr);
 
 #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+	gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
 	top -= CONFIG_SYS_MALLOC_F_LEN;
 	gd->malloc_base = top;
 #endif
+#endif
 
 	return top;
 }
common/dlmalloc.c +4 -0
index b5bb051..9b14033 100644
@@ -3264,7 +3264,11 @@ int mALLOPt(param_number, value) int param_number; int value;
 int initf_malloc(void)
 {
 #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+	gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
 	assert(gd->malloc_base);	/* Set up by crt0.S */
+#endif
 	gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
 	gd->malloc_ptr = 0;
 #endif
common/spl/spl.c +3 -0
index 94b01da..811452b 100644
@@ -156,6 +156,9 @@ int spl_init(void)
 #if defined(CONFIG_SYS_MALLOC_F_LEN)
 	gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
 	gd->malloc_ptr = 0;
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+	gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#endif
 #endif
 	if (IS_ENABLED(CONFIG_OF_CONTROL) &&
 			!IS_ENABLED(CONFIG_SPL_DISABLE_OF_CONTROL)) {
Signature · 1 line
2.4.3
#2 lore raw

PATCH 1/3 malloc_simple: Allow malloc_simple to be used with non stack RAM

Hi Hans,

Quoted 76 lines · Hans de Goede

Before this patch malloc_simple would always allocate a chunk of RAM from the stack. This commit adds a CONFIG_SYS_MALLOC_F_BASE define, which when set directly specifies the memory address to use for the heap with malloc_simple.

---
 arch/arm/lib/crt0.S | 2 +-
 common/board_f.c    | 4 ++++
 common/dlmalloc.c   | 4 ++++
 common/spl/spl.c    | 3 +++
 4 files changed, 12 insertions(+), 1 deletion(-)
arch/arm/lib/crt0.S +1 -1
index afd4f10..5e6619f 100644
@@ -96,7 +96,7 @@ clr_gd:
        strlo   r0, [r1]                /* clear 32-bit GD word */
        addlo   r1, r1, #4              /* move to next */
        blo     clr_gd
-#if defined(CONFIG_SYS_MALLOC_F_LEN)
+#if defined(CONFIG_SYS_MALLOC_F_LEN) && !defined(CONFIG_SYS_MALLOC_F_BASE)
        sub     sp, sp, #CONFIG_SYS_MALLOC_F_LEN
        str     sp, [r9, #GD_MALLOC_BASE]
 #endif
common/board_f.c +4 -0
index c959774..7f3b96f 100644
@@ -1050,9 +1050,13 @@ ulong board_init_f_mem(ulong top)
        arch_setup_gd(gd_ptr);
 
 #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
        top -= CONFIG_SYS_MALLOC_F_LEN;
        gd->malloc_base = top;
 #endif
+#endif
 
        return top;
 }
common/dlmalloc.c +4 -0
index b5bb051..9b14033 100644
@@ -3264,7 +3264,11 @@ int mALLOPt(param_number, value) int param_number; int value;
 int initf_malloc(void)
 {
 #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
        assert(gd->malloc_base);        /* Set up by crt0.S */
+#endif
        gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
        gd->malloc_ptr = 0;
 #endif
common/spl/spl.c +3 -1
index 94b01da..811452b 100644
@@ -156,6 +156,9 @@ int spl_init(void)
 #if defined(CONFIG_SYS_MALLOC_F_LEN)
        gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
        gd->malloc_ptr = 0;
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#endif
 #endif
        if (IS_ENABLED(CONFIG_OF_CONTROL) &&
                        !IS_ENABLED(CONFIG_SPL_DISABLE_OF_CONTROL)) {
--
Quoted 2 lines

2.4.3

Why does this save memory?

In general we should move away from hard-coding specific addresses I think, and just work out the memory from a single address, subtracting space for each area we need.

Regards,
Simon

#3 lore raw

PATCH 1/3 malloc_simple: Allow malloc_simple to be used with non stack RAM

Hi,

Quoted 75 lines · Simon Glass

Hi Hans,

Quoted 72 lines · Hans de Goede

Before this patch malloc_simple would always allocate a chunk of RAM from the stack. This commit adds a CONFIG_SYS_MALLOC_F_BASE define, which when set directly specifies the memory address to use for the heap with malloc_simple.

---
  arch/arm/lib/crt0.S | 2 +-
  common/board_f.c    | 4 ++++
  common/dlmalloc.c   | 4 ++++
  common/spl/spl.c    | 3 +++
  4 files changed, 12 insertions(+), 1 deletion(-)
arch/arm/lib/crt0.S +1 -1
index afd4f10..5e6619f 100644
@@ -96,7 +96,7 @@ clr_gd:
         strlo   r0, [r1]                /* clear 32-bit GD word */
         addlo   r1, r1, #4              /* move to next */
         blo     clr_gd
-#if defined(CONFIG_SYS_MALLOC_F_LEN)
+#if defined(CONFIG_SYS_MALLOC_F_LEN) && !defined(CONFIG_SYS_MALLOC_F_BASE)
         sub     sp, sp, #CONFIG_SYS_MALLOC_F_LEN
         str     sp, [r9, #GD_MALLOC_BASE]
  #endif
common/board_f.c +4 -0
index c959774..7f3b96f 100644
@@ -1050,9 +1050,13 @@ ulong board_init_f_mem(ulong top)
         arch_setup_gd(gd_ptr);
 
  #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
         top -= CONFIG_SYS_MALLOC_F_LEN;
         gd->malloc_base = top;
  #endif
+#endif
 
         return top;
  }
common/dlmalloc.c +4 -0
index b5bb051..9b14033 100644
@@ -3264,7 +3264,11 @@ int mALLOPt(param_number, value) int param_number; int value;
  int initf_malloc(void)
  {
  #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
         assert(gd->malloc_base);        /* Set up by crt0.S */
+#endif
         gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
         gd->malloc_ptr = 0;
  #endif
common/spl/spl.c +3 -0
index 94b01da..811452b 100644
@@ -156,6 +156,9 @@ int spl_init(void)
  #if defined(CONFIG_SYS_MALLOC_F_LEN)
         gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
         gd->malloc_ptr = 0;
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#endif
Quoted 8 lines
Quoted 6 lines
common/spl/spl.c +0 -1
  #endif
         if (IS_ENABLED(CONFIG_OF_CONTROL) &&
                         !IS_ENABLED(CONFIG_SPL_DISABLE_OF_CONTROL)) {
--

2.4.3

Why does this save memory?

See patch 2/3, which does #define CONFIG_SYS_MALLOC_SIMPLE when building the SPL, removing common/dlmalloc.c and only using common/malloc_simple.c both pre and post reloc.

We need this patch to do this because we do not have room on the stack (which sits in SRAM) and setting CONFIG_SYS_MALLOC_F_LEN normally puts the malloc_simple heap there.

We do however have room in DRAM and the SPL (which does not use device- model on sunxi) does not need malloc until after DRAM has been brought up, so we use this to point the malloc_simple.c heap at DRAM (far far away from where u-boot.bin will be loaded).

Quoted 3 lines

In general we should move away from hard-coding specific addresses I think, and just work out the memory from a single address, subtracting space for each area we need.

I understand and agree, but I've been unable to find another easy solution for this, and now that we are adding nand support we are really running out of space in the SPL on sunxi, and could really use the circa 3k (out of 24k total) dlmalloc is costing us.

Regards,

Hans

#4 lore raw

PATCH 1/3 malloc_simple: Allow malloc_simple to be used with non stack RAM

Hi Hans,

Quoted 111 lines · Hans de Goede

Hi,

Quoted 79 lines · Simon Glass

Hi Hans,

Quoted 75 lines · Hans de Goede

Before this patch malloc_simple would always allocate a chunk of RAM from the stack. This commit adds a CONFIG_SYS_MALLOC_F_BASE define, which when set directly specifies the memory address to use for the heap with malloc_simple.

---
  arch/arm/lib/crt0.S | 2 +-
  common/board_f.c    | 4 ++++
  common/dlmalloc.c   | 4 ++++
  common/spl/spl.c    | 3 +++
  4 files changed, 12 insertions(+), 1 deletion(-)
arch/arm/lib/crt0.S +1 -1
index afd4f10..5e6619f 100644
@@ -96,7 +96,7 @@ clr_gd:
         strlo   r0, [r1]                /* clear 32-bit GD word */
         addlo   r1, r1, #4              /* move to next */
         blo     clr_gd
-#if defined(CONFIG_SYS_MALLOC_F_LEN)
+#if defined(CONFIG_SYS_MALLOC_F_LEN) &&
 !defined(CONFIG_SYS_MALLOC_F_BASE)
         sub     sp, sp, #CONFIG_SYS_MALLOC_F_LEN
         str     sp, [r9, #GD_MALLOC_BASE]
  #endif
common/board_f.c +4 -0
index c959774..7f3b96f 100644
@@ -1050,9 +1050,13 @@ ulong board_init_f_mem(ulong top)
         arch_setup_gd(gd_ptr);
 
  #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
         top -= CONFIG_SYS_MALLOC_F_LEN;
         gd->malloc_base = top;
  #endif
+#endif
 
         return top;
  }
common/dlmalloc.c +4 -0
index b5bb051..9b14033 100644
@@ -3264,7 +3264,11 @@ int mALLOPt(param_number, value) int param_number;
 int value;
  int initf_malloc(void)
  {
  #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
         assert(gd->malloc_base);        /* Set up by crt0.S */
+#endif
         gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
         gd->malloc_ptr = 0;
  #endif
common/spl/spl.c +3 -0
index 94b01da..811452b 100644
@@ -156,6 +156,9 @@ int spl_init(void)
  #if defined(CONFIG_SYS_MALLOC_F_LEN)
         gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
         gd->malloc_ptr = 0;
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#endif
Quoted 8 lines
Quoted 6 lines
common/spl/spl.c +0 -1
  #endif
         if (IS_ENABLED(CONFIG_OF_CONTROL) &&
                         !IS_ENABLED(CONFIG_SPL_DISABLE_OF_CONTROL)) {
--

2.4.3

Why does this save memory?

See patch 2/3, which does #define CONFIG_SYS_MALLOC_SIMPLE when building the SPL, removing common/dlmalloc.c and only using common/malloc_simple.c both pre and post reloc.

We need this patch to do this because we do not have room on the stack (which sits in SRAM) and setting CONFIG_SYS_MALLOC_F_LEN normally puts the malloc_simple heap there.

We do however have room in DRAM and the SPL (which does not use device- model on sunxi) does not need malloc until after DRAM has been brought up, so we use this to point the malloc_simple.c heap at DRAM (far far away from where u-boot.bin will be loaded).

Quoted 3 lines

In general we should move away from hard-coding specific addresses I think, and just work out the memory from a single address, subtracting space for each area we need.

Quoted 4 lines

I understand and agree, but I've been unable to find another easy solution for this, and now that we are adding nand support we are really running out of space in the SPL on sunxi, and could really use the circa 3k (out of 24k total) dlmalloc is costing us.

OK thanks for the explanation.

You say that you are trying to change this in SPL but your patch changes U-Boot proper also. If it is just SPL you should not need to change board_init_f_mem() and initf_malloc().

For SPL we now have spl_relocate_stack_gd() which sets up the stack in SDRAM before calling board_init_r(). This implements the CONFIG_SPL_STACK_R option.

We should avoid hard-coding an address if we can. I wonder if we could have bool CONFIG_SPL_MALLOC_R and hex CONFIG_SPL_MALLOC_R_LEN (or hopefully you can think of better names). Then you change could go in spl_relocate_stack_gd(), something like:

if (IS_ENABLED(CONFIG_SPL_STACK_R)) {
ptr -= CONFIG_SPL_MALLOC_R_LEN;
gd->malloc_base = ptr;
}

You'll unfortunately need to add another conditional to the top of spl_init() since gd->malloc_limit will need to be set to a different value.

Regards,
Simon

#5 lore raw

PATCH 1/3 malloc_simple: Allow malloc_simple to be used with non stack RAM

Hi Hans,

Quoted 119 lines · Simon Glass

Hi Hans,

Quoted 111 lines · Hans de Goede

Hi,

Quoted 79 lines · Simon Glass

Hi Hans,

Quoted 75 lines · Hans de Goede

Before this patch malloc_simple would always allocate a chunk of RAM from the stack. This commit adds a CONFIG_SYS_MALLOC_F_BASE define, which when set directly specifies the memory address to use for the heap with malloc_simple.

---
  arch/arm/lib/crt0.S | 2 +-
  common/board_f.c    | 4 ++++
  common/dlmalloc.c   | 4 ++++
  common/spl/spl.c    | 3 +++
  4 files changed, 12 insertions(+), 1 deletion(-)
arch/arm/lib/crt0.S +1 -1
index afd4f10..5e6619f 100644
@@ -96,7 +96,7 @@ clr_gd:
         strlo   r0, [r1]                /* clear 32-bit GD word */
         addlo   r1, r1, #4              /* move to next */
         blo     clr_gd
-#if defined(CONFIG_SYS_MALLOC_F_LEN)
+#if defined(CONFIG_SYS_MALLOC_F_LEN) &&
 !defined(CONFIG_SYS_MALLOC_F_BASE)
         sub     sp, sp, #CONFIG_SYS_MALLOC_F_LEN
         str     sp, [r9, #GD_MALLOC_BASE]
  #endif
common/board_f.c +4 -0
index c959774..7f3b96f 100644
@@ -1050,9 +1050,13 @@ ulong board_init_f_mem(ulong top)
         arch_setup_gd(gd_ptr);
 
  #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
         top -= CONFIG_SYS_MALLOC_F_LEN;
         gd->malloc_base = top;
  #endif
+#endif
 
         return top;
  }
common/dlmalloc.c +4 -0
index b5bb051..9b14033 100644
@@ -3264,7 +3264,11 @@ int mALLOPt(param_number, value) int param_number;
 int value;
  int initf_malloc(void)
  {
  #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
         assert(gd->malloc_base);        /* Set up by crt0.S */
+#endif
         gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
         gd->malloc_ptr = 0;
  #endif
common/spl/spl.c +3 -0
index 94b01da..811452b 100644
@@ -156,6 +156,9 @@ int spl_init(void)
  #if defined(CONFIG_SYS_MALLOC_F_LEN)
         gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
         gd->malloc_ptr = 0;
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#endif
Quoted 8 lines
Quoted 6 lines
common/spl/spl.c +0 -1
  #endif
         if (IS_ENABLED(CONFIG_OF_CONTROL) &&
                         !IS_ENABLED(CONFIG_SPL_DISABLE_OF_CONTROL)) {
--

2.4.3

Why does this save memory?

See patch 2/3, which does #define CONFIG_SYS_MALLOC_SIMPLE when building the SPL, removing common/dlmalloc.c and only using common/malloc_simple.c both pre and post reloc.

We need this patch to do this because we do not have room on the stack (which sits in SRAM) and setting CONFIG_SYS_MALLOC_F_LEN normally puts the malloc_simple heap there.

We do however have room in DRAM and the SPL (which does not use device- model on sunxi) does not need malloc until after DRAM has been brought up, so we use this to point the malloc_simple.c heap at DRAM (far far away from where u-boot.bin will be loaded).

Quoted 3 lines

In general we should move away from hard-coding specific addresses I think, and just work out the memory from a single address, subtracting space for each area we need.

Quoted 4 lines

I understand and agree, but I've been unable to find another easy solution for this, and now that we are adding nand support we are really running out of space in the SPL on sunxi, and could really use the circa 3k (out of 24k total) dlmalloc is costing us.

Quoted 5 lines

OK thanks for the explanation.

You say that you are trying to change this in SPL but your patch changes U-Boot proper also. If it is just SPL you should not need to change board_init_f_mem() and initf_malloc().

(Little update - this is not strictly true - with your patch you needed to change board_init_f_mem(), but with my thoughts below you don't need to)

Quoted 21 lines

For SPL we now have spl_relocate_stack_gd() which sets up the stack in SDRAM before calling board_init_r(). This implements the CONFIG_SPL_STACK_R option.

We should avoid hard-coding an address if we can. I wonder if we could have bool CONFIG_SPL_MALLOC_R and hex CONFIG_SPL_MALLOC_R_LEN (or hopefully you can think of better names). Then you change could go in spl_relocate_stack_gd(), something like:

if (IS_ENABLED(CONFIG_SPL_STACK_R)) {
   ptr -= CONFIG_SPL_MALLOC_R_LEN;
   gd->malloc_base = ptr;
}

You'll unfortunately need to add another conditional to the top of spl_init() since gd->malloc_limit will need to be set to a different value.

Regards,
Simon

#6 lore raw

PATCH 1/3 malloc_simple: Allow malloc_simple to be used with non stack RAM

In reply to Simon Glass

Hi,

Quoted 134 lines · Simon Glass

Hi Hans,

Quoted 111 lines · Hans de Goede

Hi,

Quoted 79 lines · Simon Glass

Hi Hans,

Quoted 75 lines · Hans de Goede

Before this patch malloc_simple would always allocate a chunk of RAM from the stack. This commit adds a CONFIG_SYS_MALLOC_F_BASE define, which when set directly specifies the memory address to use for the heap with malloc_simple.

---
   arch/arm/lib/crt0.S | 2 +-
   common/board_f.c    | 4 ++++
   common/dlmalloc.c   | 4 ++++
   common/spl/spl.c    | 3 +++
   4 files changed, 12 insertions(+), 1 deletion(-)
arch/arm/lib/crt0.S +1 -1
index afd4f10..5e6619f 100644
@@ -96,7 +96,7 @@ clr_gd:
          strlo   r0, [r1]                /* clear 32-bit GD word */
          addlo   r1, r1, #4              /* move to next */
          blo     clr_gd
-#if defined(CONFIG_SYS_MALLOC_F_LEN)
+#if defined(CONFIG_SYS_MALLOC_F_LEN) &&
 !defined(CONFIG_SYS_MALLOC_F_BASE)
          sub     sp, sp, #CONFIG_SYS_MALLOC_F_LEN
          str     sp, [r9, #GD_MALLOC_BASE]
   #endif
common/board_f.c +4 -0
index c959774..7f3b96f 100644
@@ -1050,9 +1050,13 @@ ulong board_init_f_mem(ulong top)
          arch_setup_gd(gd_ptr);
 
   #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
          top -= CONFIG_SYS_MALLOC_F_LEN;
          gd->malloc_base = top;
   #endif
+#endif
 
          return top;
   }
common/dlmalloc.c +4 -0
index b5bb051..9b14033 100644
@@ -3264,7 +3264,11 @@ int mALLOPt(param_number, value) int param_number;
 int value;
   int initf_malloc(void)
   {
   #ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#else
          assert(gd->malloc_base);        /* Set up by crt0.S */
+#endif
          gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
          gd->malloc_ptr = 0;
   #endif
common/spl/spl.c +3 -0
index 94b01da..811452b 100644
@@ -156,6 +156,9 @@ int spl_init(void)
   #if defined(CONFIG_SYS_MALLOC_F_LEN)
          gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
          gd->malloc_ptr = 0;
+#if defined(CONFIG_SYS_MALLOC_F_BASE)
+       gd->malloc_base = CONFIG_SYS_MALLOC_F_BASE;
+#endif
Quoted 8 lines
Quoted 6 lines
common/spl/spl.c +0 -1
   #endif
          if (IS_ENABLED(CONFIG_OF_CONTROL) &&
                          !IS_ENABLED(CONFIG_SPL_DISABLE_OF_CONTROL)) {
--

2.4.3

Why does this save memory?

See patch 2/3, which does #define CONFIG_SYS_MALLOC_SIMPLE when building the SPL, removing common/dlmalloc.c and only using common/malloc_simple.c both pre and post reloc.

We need this patch to do this because we do not have room on the stack (which sits in SRAM) and setting CONFIG_SYS_MALLOC_F_LEN normally puts the malloc_simple heap there.

We do however have room in DRAM and the SPL (which does not use device- model on sunxi) does not need malloc until after DRAM has been brought up, so we use this to point the malloc_simple.c heap at DRAM (far far away from where u-boot.bin will be loaded).

Quoted 3 lines

In general we should move away from hard-coding specific addresses I think, and just work out the memory from a single address, subtracting space for each area we need.

Quoted 4 lines

I understand and agree, but I've been unable to find another easy solution for this, and now that we are adding nand support we are really running out of space in the SPL on sunxi, and could really use the circa 3k (out of 24k total) dlmalloc is costing us.

OK thanks for the explanation.

You say that you are trying to change this in SPL but your patch changes U-Boot proper also. If it is just SPL you should not need to change board_init_f_mem() and initf_malloc().

For SPL we now have spl_relocate_stack_gd() which sets up the stack in SDRAM before calling board_init_r(). This implements the CONFIG_SPL_STACK_R option.

We should avoid hard-coding an address if we can. I wonder if we could have bool CONFIG_SPL_MALLOC_R and hex CONFIG_SPL_MALLOC_R_LEN (or hopefully you can think of better names). Then you change could go in spl_relocate_stack_gd(), something like:

Quoted 8 lines
if (IS_ENABLED(CONFIG_SPL_STACK_R)) {
    ptr -= CONFIG_SPL_MALLOC_R_LEN;
    gd->malloc_base = ptr;
}

You'll unfortunately need to add another conditional to the top of spl_init() since gd->malloc_limit will need to be set to a different value.

Sounds like a plan, I'll give this a try, may be a while before I get around to doing this though.

Regards,

Hans

#7 lore raw

PATCH 2/3 sunxi: Switch to using malloc_simple for the spl

In reply to Hans de Goede

common/dlmalloc.c is quite big, both in .text and .data usage. E.g. for a Mele_M9 sun6i board build this reduces .text from 0x4214 to 0x3b94 bytes, and .data from 0x54c to 0x144 bytes.

---
 include/configs/sunxi-common.h | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
include/configs/sunxi-common.h +10 -3
index 837feaf..1776609 100644
@@ -80,7 +80,7 @@
 #define CONFIG_SYS_LOAD_ADDR		0x22000000 /* default load address */
 #define CONFIG_SYS_TEXT_BASE		0x2a000000
 #define CONFIG_PRE_CON_BUF_ADDR		0x2f000000
-#define CONFIG_SYS_SPL_MALLOC_START	0x2ff00000
+#define CONFIG_SPL_MALLOC_F_BASE	0x2ff00000
 #define CONFIG_SPL_BSS_START_ADDR	0x2ff80000
 #else
 #define SDRAM_OFFSET(x) 0x4##x
@@ -88,12 +88,19 @@
 #define CONFIG_SYS_LOAD_ADDR		0x42000000 /* default load address */
 #define CONFIG_SYS_TEXT_BASE		0x4a000000
 #define CONFIG_PRE_CON_BUF_ADDR		0x4f000000
-#define CONFIG_SYS_SPL_MALLOC_START	0x4ff00000
+#define CONFIG_SPL_MALLOC_F_BASE	0x4ff00000
 #define CONFIG_SPL_BSS_START_ADDR	0x4ff80000
 #endif
 
 #define CONFIG_SPL_BSS_MAX_SIZE		0x00080000 /* 512 KiB */
-#define CONFIG_SYS_SPL_MALLOC_SIZE	0x00080000 /* 512 KiB */
+#define CONFIG_SPL_MALLOC_F_LEN		0x00080000 /* 512 KiB */
+
+#ifdef CONFIG_SPL_BUILD
+#define CONFIG_SYS_MALLOC_SIMPLE	/* Use malloc_simple to save space */
+#undef CONFIG_SYS_MALLOC_F_LEN
+#define CONFIG_SYS_MALLOC_F_LEN		CONFIG_SPL_MALLOC_F_LEN
+#define CONFIG_SYS_MALLOC_F_BASE	CONFIG_SPL_MALLOC_F_BASE
+#endif
 
 #ifdef CONFIG_MACH_SUN9I
 /*
Signature · 1 line
2.4.3
#9 lore raw

PATCH 3/3 sunxi: sunxi-common.h cleanup

In reply to Hans de Goede

Move some #define-s around from one #ifdef block to another to reduce the number of #ifdef blocks (note this causes no functional changes even though the conditions are not always exactly the same) and move generic #include statements to the top.

---
 include/configs/sunxi-common.h | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)
include/configs/sunxi-common.h +6 -15
index 1776609..915b597 100644
@@ -13,6 +13,7 @@
 #ifndef _SUNXI_COMMON_CONFIG_H
 #define _SUNXI_COMMON_CONFIG_H
 
+#include <asm/arch/cpu.h>
 #include <linux/stringify.h>
 
 #ifdef CONFIG_OLD_SUNXI_KERNEL_COMPAT
@@ -35,27 +36,15 @@
  * High Level Configuration Options
  */
 #define CONFIG_SUNXI		/* sunxi family */
-#ifdef CONFIG_SPL_BUILD
-#define CONFIG_SYS_THUMB_BUILD	/* Thumbs mode to save space in SPL */
-#endif
-
-#include <asm/arch/cpu.h>	/* get chip and board defs */
-
-#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM_SERIAL)
-# define CONFIG_DW_SERIAL
-#endif
-
-/*
- * Display CPU information
- */
-#define CONFIG_DISPLAY_CPUINFO
 
 /* Serial & console */
 #define CONFIG_SYS_NS16550
 #define CONFIG_SYS_NS16550_SERIAL
 /* ns16550 reg in the low bits of cpu reg */
 #define CONFIG_SYS_NS16550_CLK		24000000
-#ifndef CONFIG_DM_SERIAL
+#ifdef CONFIG_DM_SERIAL
+# define CONFIG_DW_SERIAL
+#else
 # define CONFIG_SYS_NS16550_REG_SIZE	-4
 # define CONFIG_SYS_NS16550_COM1		SUNXI_UART0_BASE
 # define CONFIG_SYS_NS16550_COM2		SUNXI_UART1_BASE
@@ -65,6 +54,7 @@
 #endif
 
 /* CPU */
+#define CONFIG_DISPLAY_CPUINFO
 #define CONFIG_SYS_CACHELINE_SIZE	64
 
 /*
@@ -96,6 +86,7 @@
 #define CONFIG_SPL_MALLOC_F_LEN		0x00080000 /* 512 KiB */
 
 #ifdef CONFIG_SPL_BUILD
+#define CONFIG_SYS_THUMB_BUILD		/* Thumbs mode to save space in SPL */
 #define CONFIG_SYS_MALLOC_SIMPLE	/* Use malloc_simple to save space */
 #undef CONFIG_SYS_MALLOC_F_LEN
 #define CONFIG_SYS_MALLOC_F_LEN		CONFIG_SPL_MALLOC_F_LEN
Signature · 1 line
2.4.3
#10 lore raw

PATCH 3/3 sunxi: sunxi-common.h cleanup

Quoted 6 lines · Hans de Goede

Move some #define-s around from one #ifdef block to another to reduce the number of #ifdef blocks (note this causes no functional changes even though the conditions are not always exactly the same) and move generic #include statements to the top.