[U-Boot] [PATCH] env_mmc: avoid stack allocation for env

[Simon Glass]

Hi,

On 13 May 2015 at 20:02, Simon Glass <sjg at chromium.org> wrote:
> Hi,
>
> On 13 May 2015 at 16:40, Tim Harvey <tharvey at gateworks.com> wrote:
>> On Wed, May 13, 2015 at 12:58 PM, Tim Harvey <tharvey at gateworks.com> wrote:
>>> On Tue, May 12, 2015 at 8:14 AM, Tom Rini <trini at konsulko.com> wrote:
>>>> On Mon, May 11, 2015 at 12:08:25PM -0700, Tim Harvey wrote:
>>>>> On Fri, May 8, 2015 at 4:14 PM, Tom Rini <trini at konsulko.com> wrote:
>>>>> > On Sat, May 09, 2015 at 12:15:37AM +0200, Marek Vasut wrote:
>>>>> >> On Friday, May 08, 2015 at 11:51:36 PM, Tim Harvey wrote:
>>>>> >> > Allocating space for temporary env on the stack makes env_relocate_spec()
>>>>> >> > unsuitable for SPL environments which have very little stack.
>>>>> >>
>>>>> >> Well yeah, but what if you don't have malloc area ? I'd expect that
>>>>> >> the be the case in SPL quite often.
>>>>> >
>>>>> > OK, hold up.  We went through this a while back which is why things are
>>>>> > how they are today.  First, we have things setup today such that you can
>>>>> > throw stack (at the point we do env in SPL) into DDR.  This means we can
>>>>> > keep doing things the way they are.  You can take a look at
>>>>> > include/configs/am335x_evm.h and where CONFIG_EMMC_BOOT is set, that's
>>>>> > how we do what it looks like you're trying to do on imx6 but on TI
>>>>> > am335x.
>>>>>
>>>>> Yes, I'm doing this on Gateworks Ventana (board/gateworks/gw_ventana)
>>>>> based on IMX6 which has a limmited stack size during SPL. I don't see
>>>>> anything magic in include/configs/am335x_evm.h that catches my
>>>>> attention. Do you know where the code is that lets you point the stack
>>>>> to DDR? It sounds like that's what I need.
>>>>
>>>> Ah oops.  Kconfig, we use Kconfig now :)
>>>> configs/am335x_evm_defconfig:CONFIG_SPL_STACK_R=y
>>>> configs/am335x_evm_defconfig:CONFIG_SPL_STACK_R_ADDR=0x82000000
>>>>
>>>> That's moving stack to DDR and then grabbing 0x82000000 for stack (base
>>>> + 0x02000000 which is not too high, not too low, not likely to stomp on
>>>> the kernel).
>>>
>>> thats strange... common/spl/spl.c uses CONFIG_SPL_STACK_R as the
>>> address not CONFIG_SPL_STACK_R_ADDR - how can that even compile?
>>>
>>> http://git.denx.de/?p=u-boot.git;a=blob;f=common/spl/spl.c;#l324
>>
>> Tom,
>>
>> I don't really understand the CONFIG_SPL_STACK_R usage at all. The
>> only thing that uses CONFIG_SPL_STACK_R is spl_relocate_stack_gd which
>> is called from arch/arm/lib/crt0.S 'after' board_init_f() is called,
>> which will never return for SPL because we loaded and jumped to
>> u-boot.img.
>>
>> How is this working for the am335x stuff?
>
> Tom just forwarded me this thread. There is definitely something
> missing upstream compared to my local branch. I'll take a look and see
> what I missed.

It should say CONFIG_SPL_STACK_R_ADDR instead of CONFIG_SPL_STACK_R

As to the question about why CONFIG_SPL_STACK_R does not produce a
compile error, it is defined to 1 in autoconf.h, which is a valid
number. With the current value it sets the stack to 0xffffff28 which
seems invalid to me (it is in reserved memory).

It's a little bit of a mystery as to why this works on my beaglebone black.

Anyway I have sent a patch to correct it.

There are instructions in the README for using CONFIG_SPL_STACK_R, and
for the new board init flow. Basically you need to return normally
from your SPL board_init_f(). The code in crt0.S will then change the
SPL stack to CONFIG_SPL_STACK_R_ADDR which should be in SDRAM and
everything should be good.

So perhaps what you are missing is that board_init_f() is now expected
to return. It must not load U-Boot at this stage. That is supposed to
happen in board_init_r().

Regards,
Simon