Hi,
On 18-08-15 14:45, Simon Glass wrote:
Hi Hans,
On 18 August 2015 at 03:23, Hans de Goede <hdego...@redhat.com> wrote:
Hi,
On 18-08-15 03:59, Simon Glass wrote:
Hi Hans,
On 17 August 2015 at 10:08, Hans de Goede <hdego...@redhat.com> wrote:
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.
Signed-off-by: Hans de Goede <hdego...@redhat.com>
Reviewed-by: Simon Glass <s...@chromium.org>
---
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(-)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index afd4f10..5e6619f 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -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
diff --git a/common/board_f.c b/common/board_f.c
index c959774..7f3b96f 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -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;
}
diff --git a/common/dlmalloc.c b/common/dlmalloc.c
index b5bb051..9b14033 100644
--- a/common/dlmalloc.c
+++ b/common/dlmalloc.c
@@ -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
diff --git a/common/spl/spl.c b/common/spl/spl.c
index 94b01da..811452b 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -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)) {
--
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).
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.
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.
Sounds like a plan, I'll give this a try, may be a while before I get
around to doing this though.
Regards,
Hans
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
