All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 1/2] spl: Add spl_early_init()
@ 2017-03-14 19:54 Simon Glass
  2017-03-14 19:54 ` [U-Boot] [PATCH v2 2/2] rockchip: use spl_early_init instead of spl_init Simon Glass
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Glass @ 2017-03-14 19:54 UTC (permalink / raw)
  To: u-boot

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

Andrew F. Davis's below patch will make malloc_base, limit, ptr not
initialised in spl_init() when we call spl_init() in board_init_f(). Add
spl_early_init() which can be called in board_init_f() to fix this issue.

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:
Updated commit message to fix typo and add ():
Signed-off-by: Simon Glass <sjg@chromium.org>
---

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

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v2 2/2] rockchip: use spl_early_init instead of spl_init
  2017-03-14 19:54 [U-Boot] [PATCH v2 1/2] spl: Add spl_early_init() Simon Glass
@ 2017-03-14 19:54 ` Simon Glass
  2017-03-14 22:13   ` Liviu Dudau
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Glass @ 2017-03-14 19:54 UTC (permalink / raw)
  To: u-boot

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

use spl_early_init to avoid malloc_base, limit, ptr not initualized.

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

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

diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
index e51e19bb2d..f494843663 100644
--- a/arch/arm/mach-rockchip/rk3288-board-spl.c
+++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
@@ -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();
-- 
2.12.0.367.g23dc2f6d3c-goog

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v2 2/2] rockchip: use spl_early_init instead of spl_init
  2017-03-14 19:54 ` [U-Boot] [PATCH v2 2/2] rockchip: use spl_early_init instead of spl_init Simon Glass
@ 2017-03-14 22:13   ` Liviu Dudau
  2017-03-14 22:18     ` Simon Glass
  0 siblings, 1 reply; 5+ messages in thread
From: Liviu Dudau @ 2017-03-14 22:13 UTC (permalink / raw)
  To: u-boot

On Tue, Mar 14, 2017 at 01:54:41PM -0600, Simon Glass wrote:
> From: Eddie Cai <eddie.cai.linux@gmail.com>
> 
> use spl_early_init to avoid malloc_base, limit, ptr not initualized.
> 
> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2:
> - Add v2 to the series since this is a new version

Hi Simon,

If you are doing this and spending the time to respin, can I suggest
that you improve the readability of this commit message and
s/initualized/initialized/ as a spell fix?

Also, now that I'm bikeshedding: the first commit message in the
series makes reference to "below patch" and one needs to read the
Fixes line to figure out which patch is referred. Linux kernel's 
convention is to name the patch's sha1 directly.

I'm trying to boot Firefly RK3288 Plus with v2017.03 without success.
I've applied this series and it made no difference. I have also
removed CONFIG_SPL_OF_PLATDATA from firefly-rk3288_defconfig (otherwise
doc/README.rockchip makes no sense because there is no spl/u-boot-spl-dtb.bin
and spl/u-boot-spl.bin is the same as spl/u-boot-spl-nodtb.bin), without
any success.

So, for what is worth, you can also add my Tested-by: Liviu Dudau <liviu@dudau.co.uk>

Best regards,
Liviu

> 
>  arch/arm/mach-rockchip/rk3288-board-spl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
> index e51e19bb2d..f494843663 100644
> --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
> +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
> @@ -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();
> -- 
> 2.12.0.367.g23dc2f6d3c-goog
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

-- 
   _
 _|_|_
 ('_')
 (⊃  )⊃
 |_|_|

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v2 2/2] rockchip: use spl_early_init instead of spl_init
  2017-03-14 22:13   ` Liviu Dudau
@ 2017-03-14 22:18     ` Simon Glass
  2017-03-16 11:16       ` Liviu Dudau
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Glass @ 2017-03-14 22:18 UTC (permalink / raw)
  To: u-boot

Hi,

On 14 March 2017 at 16:13, Liviu Dudau <liviu@dudau.co.uk> wrote:
> On Tue, Mar 14, 2017 at 01:54:41PM -0600, Simon Glass wrote:
>> From: Eddie Cai <eddie.cai.linux@gmail.com>
>>
>> use spl_early_init to avoid malloc_base, limit, ptr not initualized.
>>
>> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v2:
>> - Add v2 to the series since this is a new version
>
> Hi Simon,
>
> If you are doing this and spending the time to respin, can I suggest
> that you improve the readability of this commit message and
> s/initualized/initialized/ as a spell fix?
>
> Also, now that I'm bikeshedding: the first commit message in the
> series makes reference to "below patch" and one needs to read the
> Fixes line to figure out which patch is referred. Linux kernel's
> convention is to name the patch's sha1 directly.

OK I'll tidy these two commit messages up a little more.

>
> I'm trying to boot Firefly RK3288 Plus with v2017.03 without success.
> I've applied this series and it made no difference. I have also
> removed CONFIG_SPL_OF_PLATDATA from firefly-rk3288_defconfig (otherwise
> doc/README.rockchip makes no sense because there is no spl/u-boot-spl-dtb.bin
> and spl/u-boot-spl.bin is the same as spl/u-boot-spl-nodtb.bin), without
> any success.
>
> So, for what is worth, you can also add my Tested-by: Liviu Dudau <liviu@dudau.co.uk>

For my testing I enable CONFIG_SPL_OF_PLATDATA and disable
CONFIG_ROCKCHIP_SPL_BACK_TO_BROM.

How are you creating the SD card (or whatever you boot from)?

Regards,
Simon

>
> Best regards,
> Liviu
>
>>
>>  arch/arm/mach-rockchip/rk3288-board-spl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
>> index e51e19bb2d..f494843663 100644
>> --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
>> +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
>> @@ -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();
>> --
>> 2.12.0.367.g23dc2f6d3c-goog
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>
> --
>    _
>  _|_|_
>  ('_')
>  (⊃  )⊃
>  |_|_|

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH v2 2/2] rockchip: use spl_early_init instead of spl_init
  2017-03-14 22:18     ` Simon Glass
@ 2017-03-16 11:16       ` Liviu Dudau
  0 siblings, 0 replies; 5+ messages in thread
From: Liviu Dudau @ 2017-03-16 11:16 UTC (permalink / raw)
  To: u-boot

On Tue, Mar 14, 2017 at 04:18:45PM -0600, Simon Glass wrote:
> Hi,
> 
> On 14 March 2017 at 16:13, Liviu Dudau <liviu@dudau.co.uk> wrote:
> > On Tue, Mar 14, 2017 at 01:54:41PM -0600, Simon Glass wrote:
> >> From: Eddie Cai <eddie.cai.linux@gmail.com>
> >>
> >> use spl_early_init to avoid malloc_base, limit, ptr not initualized.
> >>
> >> Signed-off-by: Eddie Cai <eddie.cai.linux@gmail.com>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> ---
> >>
> >> Changes in v2:
> >> - Add v2 to the series since this is a new version
> >
> > Hi Simon,
> >
> > If you are doing this and spending the time to respin, can I suggest
> > that you improve the readability of this commit message and
> > s/initualized/initialized/ as a spell fix?
> >
> > Also, now that I'm bikeshedding: the first commit message in the
> > series makes reference to "below patch" and one needs to read the
> > Fixes line to figure out which patch is referred. Linux kernel's
> > convention is to name the patch's sha1 directly.
> 
> OK I'll tidy these two commit messages up a little more.

Cheers, I'll look at v4 tonight when I can get access to my Firefly and give my feedback.

> 
> >
> > I'm trying to boot Firefly RK3288 Plus with v2017.03 without success.
> > I've applied this series and it made no difference. I have also
> > removed CONFIG_SPL_OF_PLATDATA from firefly-rk3288_defconfig (otherwise
> > doc/README.rockchip makes no sense because there is no spl/u-boot-spl-dtb.bin
> > and spl/u-boot-spl.bin is the same as spl/u-boot-spl-nodtb.bin), without
> > any success.
> >
> > So, for what is worth, you can also add my Tested-by: Liviu Dudau <liviu@dudau.co.uk>
> 
> For my testing I enable CONFIG_SPL_OF_PLATDATA and disable
> CONFIG_ROCKCHIP_SPL_BACK_TO_BROM.
> 
> How are you creating the SD card (or whatever you boot from)?

Yeah, that was something that I screwed up, but found the solution. In the end I went with
CONFIG_ROCKCHIP_SPL_BACK_TO_BROM=y and CONFIG_SPL_OF_PLATDATA=n, but that also needs
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=n otherwise (I guess) SPL still wants to load u-boot.

Now I have an RK3288 SD card that boots on both Firefly and (surprisingly) some TV box that I have
(Kingnovel K-R68 or just R6, both names been used by the sellers). I have this patch applied though,
so I need to go back and test without it to see if it makes any difference.

Best regards,
Liviu

> 
> Regards,
> Simon
> 
> >
> > Best regards,
> > Liviu
> >
> >>
> >>  arch/arm/mach-rockchip/rk3288-board-spl.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
> >> index e51e19bb2d..f494843663 100644
> >> --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
> >> +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
> >> @@ -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();
> >> --
> >> 2.12.0.367.g23dc2f6d3c-goog
> >>
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot at lists.denx.de
> >> https://lists.denx.de/listinfo/u-boot
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-16 11:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-14 19:54 [U-Boot] [PATCH v2 1/2] spl: Add spl_early_init() Simon Glass
2017-03-14 19:54 ` [U-Boot] [PATCH v2 2/2] rockchip: use spl_early_init instead of spl_init Simon Glass
2017-03-14 22:13   ` Liviu Dudau
2017-03-14 22:18     ` Simon Glass
2017-03-16 11:16       ` Liviu Dudau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.