[U-Boot] [PATCH] rockchip: Reconfigure the malloc based to point to system memory

Hans de Goede
2015-10-01T10:08:41Z

Hi Sjoerd,

On 01-10-15 11:10, Sjoerd Simons wrote:
> When malloc_base initially gets setup in the SPL it is based on the
> current (early) stack pointer, which for rockchip is pointing into SRAM.
> This means simple memory allocations happen in SRAM space, which is
> somewhat unfortunate. Specifically a bounce buffer for the mmc allocated
> in SRAM space seems to cause the mmc engine to stall/fail causing
> timeouts and a failure to load the main u-boot image.
>
> To resolve this, reconfigure the malloc_base to start at the relocated
> stack pointer after DRAM has been setup.
>
> For reference, things did work fine on rockchip before 596380db was
> merged to fix memalign_simple due to a combination of rockchip SDRAM
> starting at address 0 and the dw_mmc driver not checking errors from
> bounce_buffer_start. As a result, when a bounce buffer needed to be
> allocated mem_align simple would fail and return NULL. The mmc driver
> ignored the error and happily continued with the bounce buffer address
> being set to 0, which just happened to work fine..
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>
> ---
> A potentially better fix for this issue would be to reconfigure the
> malloc_base in spl_relocate_stack_gd following the same steps as is done
> for the initial setup.

I actually have a patch series pending for this:

http://patchwork.ozlabs.org/patch/517191/
http://patchwork.ozlabs.org/patch/517194/
http://patchwork.ozlabs.org/patch/517193/
http://patchwork.ozlabs.org/patch/517195/
http://patchwork.ozlabs.org/patch/517196/

(I've omitted 2 uninteresting patches)

Your review of / input on this series would be appreciated.

> However at this point in the release cycle i
> preferred to do a minimal rockchip only fix (so those boards become
> bootable again) for this issue to minimize the potential impact on other
> boards.

I agree that a minimal rockchip only fix likely is best at this time,
however your fix seems wrong:

> arch/arm/mach-rockchip/board-spl.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/mach-rockchip/board-spl.c b/arch/arm/mach-rockchip/board-spl.c
> index a241d96..5daced7 100644
> --- a/arch/arm/mach-rockchip/board-spl.c
> +++ b/arch/arm/mach-rockchip/board-spl.c
> @@ -217,6 +217,10 @@ void board_init_f(ulong dummy)
> debug("DRAM init failed: %d\n", ret);
> return;
> }
> +
> + /* Now that DRAM is initialized setup base pointer for simple malloc
> + * into RAM */
> + gd->malloc_base = CONFIG_SPL_STACK_R_ADDR;
> }
>
> static int setup_led(void)

SPL_STACK_R_ADDR is where the stack will be put by spl_relocate_stack_gd
so now you've the stack and the heap overlapping.

You likely will want to make CONFIG_SPL_STACK_R_ADDR be something like 0x80000
and then do:

gd->malloc_base = 0x00000000;
gd->malloc_limit = CONFIG_SPL_STACK_R_ADDR;
gd->malloc_ptr = 0;

Here to stop the 2 from overlapping, things are likely working with
your patch since you're not resetting gd->malloc_ptr, so you keep space
for the stack for whatever that is set to at that point.

Regards,

Hans