[U-Boot] [PATCH v4 1/2] spl: Add spl_early_init()

[Simon Glass]

Hi Tom,

On 20 March 2017 at 08:27, Tom Rini <trini at konsulko.com> wrote:
> On Mon, Mar 20, 2017 at 10:58:57AM +0530, Lokesh Vutla wrote:
>> Hi Simon,
>>
>> On Wednesday 15 March 2017 08:13 PM, Simon Glass wrote:
>> > From: Eddie Cai <eddie.cai.linux at gmail.com>
>> >
>> > At present malloc_base/_limit/_ptr are not initialised in spl_init() when
>> > we call spl_init() in board_init_f(). This is due to a recent change aimed
>> > at avoiding overwriting the malloc area set up on some boards by
>> > spl_relocate_stack_gd().
>> >
>> > However if CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN is not defined, we now
>> > skip setting up the memory area in spl_init() which is obviously wrong.
>> >
>> > To fix this, add a new function spl_early_init() which can be called in
>> > board_init_f().
>> >
>> > Fixes: b3d2861e (spl: Remove overwrite of relocated malloc limit)
>> > Signed-off-by: Eddie Cai <eddie.cai.linux at gmail.com>
>> > Rewrote spl_{,early_}init() to avoid duplicate code:
>> > Rewrite/expand commit message:
>> > Signed-off-by: Simon Glass <sjg at chromium.org>
>> > ---
>> >
>> > Changes in v4: None
>> > Changes in v3: None
>> > Changes in v2:
>> > - Rewrote spl_{,early_}init() to avoid duplicate code
>> > - Rewrite commit message
>> >
>> >  common/spl/spl.c                  | 46 +++++++++++++++++++++++++++++----------
>> >  include/asm-generic/global_data.h |  1 +
>> >  include/spl.h                     | 24 +++++++++++++++++---
>> >  3 files changed, 57 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/common/spl/spl.c b/common/spl/spl.c
>> > index 766fb3d6f4..2bc8b42027 100644
>> > --- a/common/spl/spl.c
>> > +++ b/common/spl/spl.c
>> > @@ -170,22 +170,20 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>> >     image_entry();
>> >  }
>> >
>> > -int spl_init(void)
>> > +static int spl_common_init(bool setup_malloc)
>> >  {
>> >     int ret;
>> >
>> > -   debug("spl_init()\n");
>> > -/*
>> > - * with CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN we set malloc_base and
>> > - * malloc_limit in spl_relocate_stack_gd
>> > - */
>> > -#if defined(CONFIG_SYS_MALLOC_F_LEN) && \
>> > -   !defined(CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN)
>> > +   debug("spl_early_init()\n");
>> > +
>> > +#if defined(CONFIG_SYS_MALLOC_F_LEN)
>> > +   if (setup_malloc) {
>> >  #ifdef CONFIG_MALLOC_F_ADDR
>> > -   gd->malloc_base = CONFIG_MALLOC_F_ADDR;
>> > +           gd->malloc_base = CONFIG_MALLOC_F_ADDR;
>> >  #endif
>> > -   gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
>> > -   gd->malloc_ptr = 0;
>> > +           gd->malloc_limit = CONFIG_SYS_MALLOC_F_LEN;
>> > +           gd->malloc_ptr = 0;
>> > +   }
>> >  #endif
>> >     if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
>> >             ret = fdtdec_setup();
>> > @@ -202,6 +200,32 @@ int spl_init(void)
>> >                     return ret;
>> >             }
>> >     }
>> > +
>> > +   return 0;
>> > +}
>> > +
>> > +int spl_early_init(void)
>> > +{
>> > +   int ret;
>> > +
>> > +   ret = spl_common_init(true);
>> > +   if (ret)
>> > +           return ret;
>> > +   gd->flags |= GD_FLG_SPL_EARLY_INIT;
>> > +
>> > +   return 0;
>> > +}
>> > +
>> > +int spl_init(void)
>> > +{
>> > +   int ret;
>> > +
>> > +   if (!(gd->flags & GD_FLG_SPL_EARLY_INIT)) {
>> > +           ret = spl_common_init(
>> > +                   !IS_ENABLED(CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN));
>>
>> IS_ENABLED(CONFIG defined as hex) is always false. So this will always
>> return true and overriding the malloc_limit, which is causing boot
>> failures on DRA7xx based boards.:wq
>
> Maybe we should use a bool that we set based on (SPL_STACK_R &&
> SPL_SYS_MALLOC_SIMPLE) being true or not as that's the gating condition
> for CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN being set.

And it looks like you fixed this with:

cf334edfbb    spl: Correct call to spl_common_init() with
SPL_STACK_R_MALLOC_SIMPLE_LEN

Thanks!
- Simon