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

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

[Simon Glass]

From: Eddie Cai <eddie.cai.linux@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@gmail.com>
Rewrote spl_{,early_}init() to avoid duplicate code:
Rewrite/expand commit message:
Signed-off-by: Simon Glass <sjg@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));
+		if (ret)
+			return ret;
+	}
 	gd->flags |= GD_FLG_SPL_INIT;
 
 	return 0;
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index e02863dc03..5b356dd231 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -127,5 +127,6 @@ typedef struct global_data {
 #define GD_FLG_SKIP_RELOC	0x00800	/* Don't relocate		   */
 #define GD_FLG_RECORD		0x01000	/* Record console		   */
 #define GD_FLG_ENV_DEFAULT	0x02000 /* Default variable flag	   */
+#define GD_FLG_SPL_EARLY_INIT	0x04000 /* Early SPL init is done	   */
 
 #endif /* __ASM_GENERIC_GBL_DATA_H */
diff --git a/include/spl.h b/include/spl.h
index bde44374ea..cdd196d187 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -213,11 +213,29 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image,
 			  struct blk_desc *block_dev, int partition);
 
 /**
- * spl_init() - Set up device tree and driver model in SPL if enabled
+ * spl_early_init() - Set up device tree and driver model in SPL if enabled
  *
  * Call this function in board_init_f() if you want to use device tree and
- * driver model early, before board_init_r() is called. This function will
- * be called from board_init_r() if not called earlier.
+ * driver model early, before board_init_r() is called.
+ *
+ * If this is not called, then driver model will be inactive in SPL's
+ * board_init_f(), and no device tree will be available.
+ */
+int spl_early_init(void);
+
+/**
+ * spl_init() - Set up device tree and driver model in SPL if enabled
+ *
+ * You can optionally call spl_early_init(), then optionally call spl_init().
+ * This function will be called from board_init_r() if not called earlier.
+ *
+ * Both spl_early_init() and spl_init() perform a similar function except that
+ * the latter will not set up the malloc() area if
+ * CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN is enabled, since it is assumed to
+ * already be done by a calll to spl_relocate_stack_gd() before board_init_r()
+ * is reached.
+ *
+ * This function will be called from board_init_r() if not called earlier.
  *
  * If this is not called, then driver model will be inactive in SPL's
  * board_init_f(), and no device tree will be available.
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH v4 2/2] rockchip: rk3288: use spl_early_init() instead of spl_init()

[Simon Glass]

From: Eddie Cai <eddie.cai.linux@gmail.com>

Use spl_early_init() to make sure that early malloc() is initialised. This
fixes booting on firefly-rk3288, for example.

Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v4:
- Change debug() message to match new function name

Changes in v3:
- Rewrite both commit messages

Changes in v2:
- Add v2 to the series since this is a new version

 arch/arm/mach-rockchip/rk3288-board-spl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
index e51e19bb2d..74f3379194 100644
--- a/arch/arm/mach-rockchip/rk3288-board-spl.c
+++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
@@ -185,9 +185,9 @@ void board_init_f(ulong dummy)
 	debug_uart_init();
 #endif
 
-	ret = spl_init();
+	ret = spl_early_init();
 	if (ret) {
-		debug("spl_init() failed: %d\n", ret);
+		debug("spl_early_init() failed: %d\n", ret);
 		hang();
 	}
 
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH v4 2/2] rockchip: rk3288: use spl_early_init() instead of spl_init()

[Kever Yang]

Hi Eddie, Simon,

Could you help to correct rk3188, rk3399 spl at the same time?

Thanks,
- Kever
On 03/15/2017 10:43 PM, Simon Glass wrote:
> From: Eddie Cai <eddie.cai.linux@gmail.com>
>
> Use spl_early_init() to make sure that early malloc() is initialised. This
> fixes booting on firefly-rk3288, for example.
>
> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v4:
> - Change debug() message to match new function name
>
> Changes in v3:
> - Rewrite both commit messages
>
> Changes in v2:
> - Add v2 to the series since this is a new version
>
>   arch/arm/mach-rockchip/rk3288-board-spl.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
> index e51e19bb2d..74f3379194 100644
> --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
> @@ -185,9 +185,9 @@ void board_init_f(ulong dummy)
>   	debug_uart_init();
>   #endif
>   
> -	ret = spl_init();
> +	ret = spl_early_init();
>   	if (ret) {
> -		debug("spl_init() failed: %d\n", ret);
> +		debug("spl_early_init() failed: %d\n", ret);
>   		hang();
>   	}
>   

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

[Eddie Cai]

2017-03-15 22:43 GMT+08:00 Simon Glass <sjg@chromium.org>:

> From: Eddie Cai <eddie.cai.linux@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@gmail.com>
> Rewrote spl_{,early_}init() to avoid duplicate code:
> Rewrite/expand commit message:
> Signed-off-by: Simon Glass <sjg@chromium.org>
>
Reviewed-by: Eddie Cai <eddie.cai.linux@gmail.com>

> ---
>
> 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));
> +               if (ret)
> +                       return ret;
> +       }
>         gd->flags |= GD_FLG_SPL_INIT;
>
>         return 0;
> diff --git a/include/asm-generic/global_data.h
> b/include/asm-generic/global_data.h
> index e02863dc03..5b356dd231 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -127,5 +127,6 @@ typedef struct global_data {
>  #define GD_FLG_SKIP_RELOC      0x00800 /* Don't relocate
> */
>  #define GD_FLG_RECORD          0x01000 /* Record console
> */
>  #define GD_FLG_ENV_DEFAULT     0x02000 /* Default variable flag
>  */
> +#define GD_FLG_SPL_EARLY_INIT  0x04000 /* Early SPL init is done
> */
>
>  #endif /* __ASM_GENERIC_GBL_DATA_H */
> diff --git a/include/spl.h b/include/spl.h
> index bde44374ea..cdd196d187 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -213,11 +213,29 @@ int spl_load_image_ext_os(struct spl_image_info
> *spl_image,
>                           struct blk_desc *block_dev, int partition);
>
>  /**
> - * spl_init() - Set up device tree and driver model in SPL if enabled
> + * spl_early_init() - Set up device tree and driver model in SPL if
> enabled
>   *
>   * Call this function in board_init_f() if you want to use device tree and
> - * driver model early, before board_init_r() is called. This function will
> - * be called from board_init_r() if not called earlier.
> + * driver model early, before board_init_r() is called.
> + *
> + * If this is not called, then driver model will be inactive in SPL's
> + * board_init_f(), and no device tree will be available.
> + */
> +int spl_early_init(void);
> +
> +/**
> + * spl_init() - Set up device tree and driver model in SPL if enabled
> + *
> + * You can optionally call spl_early_init(), then optionally call
> spl_init().
> + * This function will be called from board_init_r() if not called earlier.
> + *
> + * Both spl_early_init() and spl_init() perform a similar function except
> that
> + * the latter will not set up the malloc() area if
> + * CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN is enabled, since it is assumed
> to
> + * already be done by a calll to spl_relocate_stack_gd() before
> board_init_r()
> + * is reached.
> + *
> + * This function will be called from board_init_r() if not called earlier.
>   *
>   * If this is not called, then driver model will be inactive in SPL's
>   * board_init_f(), and no device tree will be available.
> --
> 2.12.0.367.g23dc2f6d3c-goog
>
>

[U-Boot] [PATCH v4 2/2] rockchip: rk3288: use spl_early_init() instead of spl_init()

[Eddie Cai]

2017-03-15 22:43 GMT+08:00 Simon Glass <sjg@chromium.org>:

> From: Eddie Cai <eddie.cai.linux@gmail.com>
>
> Use spl_early_init() to make sure that early malloc() is initialised. This
> fixes booting on firefly-rk3288, for example.
>
> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
> Signed-off-by: Simon Glass <sjg@chromium.org>
>
Reviewed-by: Eddie Cai <eddie.cai.linux@gmail.com>

> ---
>
> Changes in v4:
> - Change debug() message to match new function name
>
> Changes in v3:
> - Rewrite both commit messages
>
> Changes in v2:
> - Add v2 to the series since this is a new version
>
>  arch/arm/mach-rockchip/rk3288-board-spl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c
> b/arch/arm/mach-rockchip/rk3288-board-spl.c
> index e51e19bb2d..74f3379194 100644
> --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
> @@ -185,9 +185,9 @@ void board_init_f(ulong dummy)
>         debug_uart_init();
>  #endif
>
> -       ret = spl_init();
> +       ret = spl_early_init();
>         if (ret) {
> -               debug("spl_init() failed: %d\n", ret);
> +               debug("spl_early_init() failed: %d\n", ret);
>                 hang();
>         }
>
> --
> 2.12.0.367.g23dc2f6d3c-goog
>
>

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

[Simon Glass]

On 16 March 2017 at 09:02, Eddie Cai <eddie.cai.linux@gmail.com> wrote:
>
>
> 2017-03-15 22:43 GMT+08:00 Simon Glass <sjg@chromium.org>:
>>
>> From: Eddie Cai <eddie.cai.linux@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@gmail.com>
>> Rewrote spl_{,early_}init() to avoid duplicate code:
>> Rewrite/expand commit message:
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Reviewed-by: Eddie Cai <eddie.cai.linux@gmail.com>

Applied to u-boot-rockchip.

[U-Boot] [PATCH v4 2/2] rockchip: rk3288: use spl_early_init() instead of spl_init()

[Simon Glass]

Hi Kever,

On 16 March 2017 at 01:08, Kever Yang <kever.yang@rock-chips.com> wrote:
> Hi Eddie, Simon,
>
> Could you help to correct rk3188, rk3399 spl at the same time?

I don't have boards to test with for these, sorry.

Regards,
Simon

[U-Boot] [PATCH v4 2/2] rockchip: rk3288: use spl_early_init() instead of spl_init()

[Simon Glass]

On 16 March 2017 at 09:04, Eddie Cai <eddie.cai.linux@gmail.com> wrote:
>
>
> 2017-03-15 22:43 GMT+08:00 Simon Glass <sjg@chromium.org>:
>>
>> From: Eddie Cai <eddie.cai.linux@gmail.com>
>>
>> Use spl_early_init() to make sure that early malloc() is initialised. This
>> fixes booting on firefly-rk3288, for example.
>>
>> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Reviewed-by: Eddie Cai <eddie.cai.linux@gmail.com>

Applied to u-boot-rockchip.

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

[Tom Rini]

On Wed, Mar 15, 2017 at 08:43:28AM -0600, Simon Glass wrote:

> From: Eddie Cai <eddie.cai.linux@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@gmail.com>
> Rewrote spl_{,early_}init() to avoid duplicate code:
> Rewrite/expand commit message:
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Tom Rini <trini@konsulko.com>

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

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

[Lokesh Vutla]

Hi Simon,

On Wednesday 15 March 2017 08:13 PM, Simon Glass wrote:
> From: Eddie Cai <eddie.cai.linux@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@gmail.com>
> Rewrote spl_{,early_}init() to avoid duplicate code:
> Rewrite/expand commit message:
> Signed-off-by: Simon Glass <sjg@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

Thanks and regards,
Lokesh

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

[Tom Rini]

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@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@gmail.com>
> > Rewrote spl_{,early_}init() to avoid duplicate code:
> > Rewrite/expand commit message:
> > Signed-off-by: Simon Glass <sjg@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.

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

[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@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@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@gmail.com>
>> > Rewrote spl_{,early_}init() to avoid duplicate code:
>> > Rewrite/expand commit message:
>> > Signed-off-by: Simon Glass <sjg@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