Skip to contentSkip to conversation

spl: Add spl_early_init()

PATCH Superseded Simon Glass 3 participants 10 messages Mar 15, 2017 – Mar 16, 2017

Subsystems (4)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/2 spl: Add spl_early_init()

From: Eddie Cai <[email protected]>

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)

Rewrote spl_{,early_}init() to avoid duplicate code:
Rewrite/expand commit message:

---

Changes in v3: None
Changes in v2: None

 common/spl/spl.c                  | 46 +++++++++++++++++++++++++++++----------
 include/asm-generic/global_data.h |  1 +
 include/spl.h                     | 24 +++++++++++++++++---
 3 files changed, 57 insertions(+), 14 deletions(-)
common/spl/spl.c +35 -11
index 766fb3d6f4..2bc8b42027 100644
@@ -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;
include/asm-generic/global_data.h +1 -0
index e02863dc03..5b356dd231 100644
@@ -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 */
include/spl.h +21 -3
index bde44374ea..cdd196d187 100644
@@ -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.
Signature · 1 line
2.12.0.367.g23dc2f6d3c-goog
#1 lore raw

PATCH v3 2/2 rockchip: rk3288: use spl_early_init() instead of spl_init()

From: Eddie Cai <[email protected]>

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

---

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-rockchip/rk3288-board-spl.c +1 -1
index e51e19bb2d..f494843663 100644
@@ -185,7 +185,7 @@ 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);
 		hang();
Signature · 1 line
2.12.0.367.g23dc2f6d3c-goog
#2 lore raw

PATCH v3 2/2 rockchip: rk3288: use spl_early_init() instead of spl_init()

Simon wrote...

arch/arm/mach-rockchip/rk3288-board-spl.c void board_init_f(ulong dummy) · line 190

Quoted 13 lines
arch/arm/mach- renamed from arch/arm/mach-rockchip/rk3288-board-spl.c +0 -0

rockchip/rk3288-board-spl.c
index e51e19bb2d..f494843663 100644

arch/arm/mach-rockchip/rk3288-board-spl.c +1 -1
@@ -185,7 +185,7 @@ 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);

Shouldn't the function name used in the debug statement be changed to spl_early_init()?

-Andy.

#3 lore raw

PATCH v3 1/2 spl: Add spl_early_init()

In reply to Simon Glass
Quoted 4 lines · Simon Glass

From: Eddie Cai <[email protected]>

At present malloc_base/_limit/_ptr are not initialised in spl_init() when we call spl_init() in board_init_f().

Are you even supposed to call spl_init() from board_init_f() ? I was under the impression that's supposed to be called by board_init_r() only.

Quoted 143 lines

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)

Rewrote spl_{,early_}init() to avoid duplicate code:
Rewrite/expand commit message:

---

Changes in v3: None
Changes in v2: None

 common/spl/spl.c                  | 46 +++++++++++++++++++++++++++++----------
 include/asm-generic/global_data.h |  1 +
 include/spl.h                     | 24 +++++++++++++++++---
 3 files changed, 57 insertions(+), 14 deletions(-)
common/spl/spl.c +35 -11
index 766fb3d6f4..2bc8b42027 100644
@@ -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;
include/asm-generic/global_data.h +1 -0
index e02863dc03..5b356dd231 100644
@@ -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 */
include/spl.h +21 -3
index bde44374ea..cdd196d187 100644
@@ -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.
Signature · 2 lines
Best regards,
Marek Vasut
#4 lore raw

PATCH v3 1/2 spl: Add spl_early_init()

Hi Marek,

Quoted 10 lines · Marek Vasut
Quoted 4 lines · Simon Glass

From: Eddie Cai <[email protected]>

At present malloc_base/_limit/_ptr are not initialised in spl_init() when we call spl_init() in board_init_f().

Are you even supposed to call spl_init() from board_init_f() ? I was under the impression that's supposed to be called by board_init_r() only.

If you need to use DM then yes you must call it. At present with SPL it is not normally available until board_init_r(). At some point we will have all the boards consistent and we might be able to change this by default.

[..]

Regards,
Simon

#5 lore raw

PATCH v3 1/2 spl: Add spl_early_init()

Quoted 9 lines · Simon Glass

Hi Marek,

Quoted 6 lines · Marek Vasut
Quoted 4 lines · Simon Glass

From: Eddie Cai <[email protected]>

At present malloc_base/_limit/_ptr are not initialised in spl_init() when we call spl_init() in board_init_f().

Quoted 6 lines
Quoted 4 lines

Are you even supposed to call spl_init() from board_init_f() ? I was under the impression that's supposed to be called by board_init_r() only.

If you need to use DM then yes you must call it.

SoCFPGA uses DM and doesn't call it until board_init_r() . I think there's some significant confusion about how this whole malloc stuff should work.

Quoted 10 lines

At present with SPL
it is not normally available until board_init_r(). At some point we will have all the boards consistent and we might be able to change this by default.

[..]

Regards,
Simon

Signature · 2 lines
Best regards,
Marek Vasut
#6 lore raw

PATCH v3 1/2 spl: Add spl_early_init()

Hi Marek,

Quoted 15 lines · Marek Vasut
Quoted 14 lines · Simon Glass

Hi Marek,

Quoted 10 lines · Marek Vasut
Quoted 4 lines · Simon Glass

From: Eddie Cai <[email protected]>

At present malloc_base/_limit/_ptr are not initialised in spl_init() when we call spl_init() in board_init_f().

Are you even supposed to call spl_init() from board_init_f() ? I was under the impression that's supposed to be called by board_init_r() only.

Quoted 5 lines
Quoted 1 line

If you need to use DM then yes you must call it.

SoCFPGA uses DM and doesn't call it until board_init_r() . I think there's some significant confusion about how this whole malloc stuff should work.

OK, well if you want to use DM within board_init_f() you must call this function If not, then board_init_r() will do it for you. When does SoCFPGA start using DM?

Quoted 7 lines
Quoted 7 lines

At present with SPL
it is not normally available until board_init_r(). At some point we will have all the boards consistent and we might be able to change this by default.

[..]

Regards,
Simon

#7 lore raw

PATCH v3 1/2 spl: Add spl_early_init()

Quoted 20 lines · Simon Glass

Hi Marek,

Quoted 17 lines · Marek Vasut
Quoted 15 lines · Simon Glass

Hi Marek,

Quoted 10 lines · Marek Vasut
Quoted 4 lines · Simon Glass

From: Eddie Cai <[email protected]>

At present malloc_base/_limit/_ptr are not initialised in spl_init() when we call spl_init() in board_init_f().

Are you even supposed to call spl_init() from board_init_f() ? I was under the impression that's supposed to be called by board_init_r() only.

If you need to use DM then yes you must call it.

Quoted 7 lines
Quoted 3 lines

SoCFPGA uses DM and doesn't call it until board_init_r() . I think there's some significant confusion about how this whole malloc stuff should work.

OK, well if you want to use DM within board_init_f() you must call this function If not, then board_init_r() will do it for you. When does SoCFPGA start using DM?

In _r . I don't see anyone using DM in _f though. What is that needed for , serial ?

Signature · 2 lines
Best regards,
Marek Vasut
#8 lore raw

PATCH v3 1/2 spl: Add spl_early_init()

Hi Marek,

Quoted 25 lines · Marek Vasut
Quoted 24 lines · Simon Glass

Hi Marek,

Quoted 20 lines · Marek Vasut
Quoted 9 lines · Simon Glass

Hi Marek,

Quoted 6 lines · Marek Vasut
Quoted 4 lines · Simon Glass

From: Eddie Cai <[email protected]>

At present malloc_base/_limit/_ptr are not initialised in spl_init() when we call spl_init() in board_init_f().

Quoted 6 lines
Quoted 4 lines

Are you even supposed to call spl_init() from board_init_f() ? I was under the impression that's supposed to be called by board_init_r() only.

If you need to use DM then yes you must call it.

SoCFPGA uses DM and doesn't call it until board_init_r() . I think there's some significant confusion about how this whole malloc stuff should work.

Quoted 6 lines
Quoted 3 lines

OK, well if you want to use DM within board_init_f() you must call this function If not, then board_init_r() will do it for you. When does SoCFPGA start using DM?

In _r . I don't see anyone using DM in _f though. What is that needed for , serial ?

See for example board_init_f() in arch/arm/mach-rockchip/rk3288-board-spl.c

It include sclock, pintctrl, SDRAM at least.

Regards,
Simon

#9 lore raw

PATCH v3 1/2 spl: Add spl_early_init()

Quoted 32 lines · Simon Glass

Hi Marek,

Quoted 29 lines · Marek Vasut
Quoted 27 lines · Simon Glass

Hi Marek,

Quoted 15 lines · Marek Vasut
Quoted 14 lines · Simon Glass

Hi Marek,

Quoted 10 lines · Marek Vasut
Quoted 4 lines · Simon Glass

From: Eddie Cai <[email protected]>

At present malloc_base/_limit/_ptr are not initialised in spl_init() when we call spl_init() in board_init_f().

Are you even supposed to call spl_init() from board_init_f() ? I was under the impression that's supposed to be called by board_init_r() only.

Quoted 5 lines
Quoted 1 line

If you need to use DM then yes you must call it.

SoCFPGA uses DM and doesn't call it until board_init_r() . I think there's some significant confusion about how this whole malloc stuff should work.

OK, well if you want to use DM within board_init_f() you must call this function If not, then board_init_r() will do it for you. When does SoCFPGA start using DM?

Quoted 6 lines
Quoted 2 lines

In _r . I don't see anyone using DM in _f though. What is that needed for , serial ?

See for example board_init_f() in arch/arm/mach-rockchip/rk3288-board-spl.c

It include sclock, pintctrl, SDRAM at least.

Haaa, OK

Signature · 2 lines
Best regards,
Marek Vasut