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

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

[Tim Harvey]

Allocating space for temporary env on the stack makes env_relocate_spec()
unsuitable for SPL environments which have very little stack.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 common/env_mmc.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/common/env_mmc.c b/common/env_mmc.c
index 6c4ce2f..19a28da 100644
--- a/common/env_mmc.c
+++ b/common/env_mmc.c
@@ -213,13 +213,19 @@ void env_relocate_spec(void)
 	u32 offset1, offset2;
 	int read1_fail = 0, read2_fail = 0;
 	int crc1_ok = 0, crc2_ok = 0;
-	env_t *ep;
+	env_t *ep, *tmp_env1, *tmp_env2;
 	int ret;
 	int dev = CONFIG_SYS_MMC_ENV_DEV;
 	const char *errmsg = NULL;
 
-	ALLOC_CACHE_ALIGN_BUFFER(env_t, tmp_env1, 1);
-	ALLOC_CACHE_ALIGN_BUFFER(env_t, tmp_env2, 1);
+	tmp_env1 = memalign(CONFIG_SYS_CACHELINE_SIZE, CONFIG_ENV_SIZE);
+	tmp_env2 = memalign(CONFIG_SYS_CACHELINE_SIZE, CONFIG_ENV_SIZE);
+	if (tmp_env1 == NULL || tmp_env2 == NULL) {
+		puts("Can't allocate buffers for environment\n");
+		ret = 1;
+		errmsg = "!malloc() failed";
+		goto err;
+	}
 
 #ifdef CONFIG_SPL_BUILD
 	dev = 0;
@@ -287,6 +293,8 @@ void env_relocate_spec(void)
 	ret = 0;
 
 fini:
+	free(tmp_env1);
+	free(tmp_env2);
 	fini_mmc_for_env(mmc);
 err:
 	if (ret)
-- 
1.9.1

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

[Marek Vasut]

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.

Best regards,
Marek Vasut

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

[Tom Rini]

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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150508/f62c641b/attachment.sig>

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

[Tim Harvey]

On Fri, May 8, 2015 at 4:14 PM, Tom Rini <trini@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.
>
> --
> Tom

Tom,

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.

This sounds like there could be some boards using nand that rely on
malloc (that's where I got the inspiration from for this) and others
using mmc that rely on a decent size stack. I wonder if there is a
#define that we can base off of to provide the best of both? Its not
that great having two env drivers that have opposite requirements but
I certainly don't want to change one to make them consistent it it
will break boards.

Tim

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

[Tom Rini]

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@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).

> This sounds like there could be some boards using nand that rely on
> malloc (that's where I got the inspiration from for this) and others
> using mmc that rely on a decent size stack. I wonder if there is a
> #define that we can base off of to provide the best of both? Its not
> that great having two env drivers that have opposite requirements but
> I certainly don't want to change one to make them consistent it it
> will break boards.

My first guess is that NAND should also be converted to DDR not stack (I
too found the NAND example, switched MMC to it, and then Wolfgang/others
pointed out that really stack is where this kind of allocation should
be, iirc, but then didn't get a chance to dig back at NAND).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150512/3a450df9/attachment.sig>

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

[Tim Harvey]

On Tue, May 12, 2015 at 8:14 AM, Tom Rini <trini@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@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

>
>> This sounds like there could be some boards using nand that rely on
>> malloc (that's where I got the inspiration from for this) and others
>> using mmc that rely on a decent size stack. I wonder if there is a
>> #define that we can base off of to provide the best of both? Its not
>> that great having two env drivers that have opposite requirements but
>> I certainly don't want to change one to make them consistent it it
>> will break boards.
>
> My first guess is that NAND should also be converted to DDR not stack (I
> too found the NAND example, switched MMC to it, and then Wolfgang/others
> pointed out that really stack is where this kind of allocation should
> be, iirc, but then didn't get a chance to dig back at NAND).

You said that backwards right? You mean nand env should be converted
to stack not dram.

How do we do that without risking the chance of breaking boards that
don't have CONFIG_SPL_STACK_R? Wouldn't you need to implement both
around a #define to not cause breakage?

Tim

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

[Tim Harvey]

On Wed, May 13, 2015 at 12:58 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> On Tue, May 12, 2015 at 8:14 AM, Tom Rini <trini@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@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?

Tim

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

[Simon Glass]

Hi,

On 13 May 2015 at 16:40, Tim Harvey <tharvey@gateworks.com> wrote:
> On Wed, May 13, 2015 at 12:58 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>> On Tue, May 12, 2015 at 8:14 AM, Tom Rini <trini@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@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.

Regards,
Simon

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

[Simon Glass]

Hi,

On 13 May 2015 at 20:02, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 13 May 2015 at 16:40, Tim Harvey <tharvey@gateworks.com> wrote:
>> On Wed, May 13, 2015 at 12:58 PM, Tim Harvey <tharvey@gateworks.com> wrote:
>>> On Tue, May 12, 2015 at 8:14 AM, Tom Rini <trini@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@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

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

[Tim Harvey]

On Wed, May 13, 2015 at 8:22 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On 13 May 2015 at 20:02, Simon Glass <sjg@chromium.org> wrote:
>> Hi,
>>
>> On 13 May 2015 at 16:40, Tim Harvey <tharvey@gateworks.com> wrote:
>>> On Wed, May 13, 2015 at 12:58 PM, Tim Harvey <tharvey@gateworks.com> wrote:
<snip
>>>
>>> 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).

ok - makes sense.... I thought I was going crazy.

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

probably just lucky address wrapping

>
> 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().

I completely missed your recent commit regarding the stack relocation
(db910353a126d84fe8dff7a694ea792f50fcfb6a) - I must admit I don't have
time to keep up-to-date on the maillist.

As its a new thing that spl board_init_f should 'not' call
board_init_r, there are a lot of spl files that still call it directly
(as well as the weak implementation of board_init_f in
arch/arm/lib/spl.c). Of course, as I found out you really don't need
to remove that call unless you want to use stack relocation. The
documentation in arch/arm/lib/crt0.S is now out of date and doesn't
mention the fact that for SPL you should not call board_init_f -
perhaps this documentation should be removed and the README referred
to (or visa versa).

Thanks for clearing this up.

Tim

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

[Simon Glass]

Hi Tim,

On 14 May 2015 at 07:11, Tim Harvey <tharvey@gateworks.com> wrote:
> On Wed, May 13, 2015 at 8:22 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi,
>>
>> On 13 May 2015 at 20:02, Simon Glass <sjg@chromium.org> wrote:
>>> Hi,
>>>
>>> On 13 May 2015 at 16:40, Tim Harvey <tharvey@gateworks.com> wrote:
>>>> On Wed, May 13, 2015 at 12:58 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> <snip
>>>>
>>>> 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).
>
> ok - makes sense.... I thought I was going crazy.
>
>>
>> It's a little bit of a mystery as to why this works on my beaglebone black.
>
> probably just lucky address wrapping
>
>>
>> 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().
>
> I completely missed your recent commit regarding the stack relocation
> (db910353a126d84fe8dff7a694ea792f50fcfb6a) - I must admit I don't have
> time to keep up-to-date on the maillist.
>
> As its a new thing that spl board_init_f should 'not' call
> board_init_r, there are a lot of spl files that still call it directly
> (as well as the weak implementation of board_init_f in
> arch/arm/lib/spl.c). Of course, as I found out you really don't need
> to remove that call unless you want to use stack relocation. The
> documentation in arch/arm/lib/crt0.S is now out of date and doesn't
> mention the fact that for SPL you should not call board_init_f -
> perhaps this documentation should be removed and the README referred
> to (or visa versa).
>
> Thanks for clearing this up.
>
> Tim

Just to update this thread, I sent a patch for this:

http://patchwork.ozlabs.org/patch/502840/

Regards,
Simon

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

[Tim Harvey]

On Fri, May 8, 2015 at 3:15 PM, Marek Vasut <marex@denx.de> 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.
>
> Best regards,
> Marek Vasut

Marek,

I suppose it depends on when in SPL you need env. I assume memory
controller is about the first thing done after things like iomux. If
this is a problem then at least nand env (env_nand.c) is broken too as
that's where I got my inspiration for this after I realized I was
hanging due to not having enough stack space.

It looks like Tom may have a solution that I'll look at.

Tim