diff mbox

[U-Boot] spl: spl_nor: surround Linux-load code with #ifdef CONFIG_SPL_OS_BOOT

Message ID 1418259698-7847-1-git-send-email-yamada.m@jp.panasonic.com
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Masahiro Yamada Dec. 11, 2014, 1:01 a.m. UTC
If CONFIG_SPL_NOR_SUPPORT is defined, spl_nor_load_image() requires
spl_start_uboot(), CONFIG_SYS_OS_BASE, CONFIG_SYS_SPL_ARGS_ADDR,
CONFIG_SYS_FDT_BASE to be defined even if users just want to run
U-Boot, not Linux.  This is inconvenient.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

 common/spl/spl_nor.c | 74 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 41 insertions(+), 33 deletions(-)

Comments

Tom Rini Dec. 11, 2014, 1:34 a.m. UTC | #1
On Thu, Dec 11, 2014 at 10:01:38AM +0900, Masahiro Yamada wrote:

> If CONFIG_SPL_NOR_SUPPORT is defined, spl_nor_load_image() requires
> spl_start_uboot(), CONFIG_SYS_OS_BASE, CONFIG_SYS_SPL_ARGS_ADDR,
> CONFIG_SYS_FDT_BASE to be defined even if users just want to run
> U-Boot, not Linux.  This is inconvenient.
> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>

Good idea, but the function to check on U-Boot or Linux should be called
spl_start_uboot to match the other load methods :)
Masahiro Yamada Dec. 18, 2014, 7:13 a.m. UTC | #2
Hi Tom,

On Wed, 10 Dec 2014 20:34:03 -0500
Tom Rini <trini@ti.com> wrote:

> On Thu, Dec 11, 2014 at 10:01:38AM +0900, Masahiro Yamada wrote:
> 
> > If CONFIG_SPL_NOR_SUPPORT is defined, spl_nor_load_image() requires
> > spl_start_uboot(), CONFIG_SYS_OS_BASE, CONFIG_SYS_SPL_ARGS_ADDR,
> > CONFIG_SYS_FDT_BASE to be defined even if users just want to run
> > U-Boot, not Linux.  This is inconvenient.
> > 
> > Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> 
> Good idea, but the function to check on U-Boot or Linux should be called
> spl_start_uboot to match the other load methods :)
> 

I think I am following this way.



> +#if defined(CONFIG_SPL_OS_BOOT)
> +int load_linux(void)
> +{
> +	if (spl_start_uboot())
> +		return -1;


Here.
Any problem?



Best Regards
Masahiro Yamada
Masahiro Yamada Jan. 7, 2015, 7:20 p.m. UTC | #3
ping?



2014-12-18 16:13 GMT+09:00 Masahiro Yamada <yamada.m@jp.panasonic.com>:
> Hi Tom,
>
> On Wed, 10 Dec 2014 20:34:03 -0500
> Tom Rini <trini@ti.com> wrote:
>
>> On Thu, Dec 11, 2014 at 10:01:38AM +0900, Masahiro Yamada wrote:
>>
>> > If CONFIG_SPL_NOR_SUPPORT is defined, spl_nor_load_image() requires
>> > spl_start_uboot(), CONFIG_SYS_OS_BASE, CONFIG_SYS_SPL_ARGS_ADDR,
>> > CONFIG_SYS_FDT_BASE to be defined even if users just want to run
>> > U-Boot, not Linux.  This is inconvenient.
>> >
>> > Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
>>
>> Good idea, but the function to check on U-Boot or Linux should be called
>> spl_start_uboot to match the other load methods :)
>>
>
> I think I am following this way.
>
>
>
>> +#if defined(CONFIG_SPL_OS_BOOT)
>> +int load_linux(void)
>> +{
>> +     if (spl_start_uboot())
>> +             return -1;
>
>
> Here.
> Any problem?
>
>
>
> Best Regards
> Masahiro Yamada
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Tom Rini Jan. 7, 2015, 7:47 p.m. UTC | #4
On Thu, Dec 18, 2014 at 04:13:48PM +0900, Masahiro Yamada wrote:
> Hi Tom,
> 
> On Wed, 10 Dec 2014 20:34:03 -0500
> Tom Rini <trini@ti.com> wrote:
> 
> > On Thu, Dec 11, 2014 at 10:01:38AM +0900, Masahiro Yamada wrote:
> > 
> > > If CONFIG_SPL_NOR_SUPPORT is defined, spl_nor_load_image() requires
> > > spl_start_uboot(), CONFIG_SYS_OS_BASE, CONFIG_SYS_SPL_ARGS_ADDR,
> > > CONFIG_SYS_FDT_BASE to be defined even if users just want to run
> > > U-Boot, not Linux.  This is inconvenient.
> > > 
> > > Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> > 
> > Good idea, but the function to check on U-Boot or Linux should be called
> > spl_start_uboot to match the other load methods :)
> > 
> 
> I think I am following this way.
> 
> 
> 
> > +#if defined(CONFIG_SPL_OS_BOOT)
> > +int load_linux(void)
> > +{
> > +	if (spl_start_uboot())
> > +		return -1;
> 
> 
> Here.
> Any problem?

Yes, it should look like spl_nand_load_image().
Masahiro Yamada Jan. 8, 2015, 10:27 a.m. UTC | #5
On Wed, 7 Jan 2015 14:47:36 -0500
Tom Rini <trini@ti.com> wrote:

> On Thu, Dec 18, 2014 at 04:13:48PM +0900, Masahiro Yamada wrote:
> > Hi Tom,
> > 
> > On Wed, 10 Dec 2014 20:34:03 -0500
> > Tom Rini <trini@ti.com> wrote:
> > 
> > > On Thu, Dec 11, 2014 at 10:01:38AM +0900, Masahiro Yamada wrote:
> > > 
> > > > If CONFIG_SPL_NOR_SUPPORT is defined, spl_nor_load_image() requires
> > > > spl_start_uboot(), CONFIG_SYS_OS_BASE, CONFIG_SYS_SPL_ARGS_ADDR,
> > > > CONFIG_SYS_FDT_BASE to be defined even if users just want to run
> > > > U-Boot, not Linux.  This is inconvenient.
> > > > 
> > > > Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> > > 
> > > Good idea, but the function to check on U-Boot or Linux should be called
> > > spl_start_uboot to match the other load methods :)
> > > 
> > 
> > I think I am following this way.
> > 
> > 
> > 
> > > +#if defined(CONFIG_SPL_OS_BOOT)
> > > +int load_linux(void)
> > > +{
> > > +	if (spl_start_uboot())
> > > +		return -1;
> > 
> > 
> > Here.
> > Any problem?
> 
> Yes, it should look like spl_nand_load_image().
> 


OK.

I generally prefer to dividing shorter helper functions and shaller nests,
but common/spl/spl_nand.c is what you want, v2 is here:

http://patchwork.ozlabs.org/patch/426558/
diff mbox

Patch

diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c
index b444a3e..6f91176 100644
--- a/common/spl/spl_nor.c
+++ b/common/spl/spl_nor.c
@@ -7,6 +7,45 @@ 
 #include <common.h>
 #include <spl.h>
 
+#if defined(CONFIG_SPL_OS_BOOT)
+int load_linux(void)
+{
+	if (spl_start_uboot())
+		return -1;
+
+	spl_parse_image_header(
+		       (const struct image_header *)CONFIG_SYS_OS_BASE);
+
+	memcpy((void *)spl_image.load_addr,
+	       (void *)(CONFIG_SYS_OS_BASE + sizeof(struct image_header)),
+	       spl_image.size);
+
+	/*
+	 * Copy DT blob (fdt) to SDRAM. Passing pointer to flash
+	 * doesn't work (16 KiB should be enough for DT)
+	 */
+	memcpy((void *)CONFIG_SYS_SPL_ARGS_ADDR, (void *)(CONFIG_SYS_FDT_BASE),
+	       16 << 10);
+
+	return 0;
+}
+#else
+int load_linux(void)
+{
+	return -1;
+}
+#endif
+
+void load_uboot(void)
+{
+	spl_parse_image_header(
+			(const struct image_header *)CONFIG_SYS_UBOOT_BASE);
+
+	memcpy((void *)spl_image.load_addr,
+	       (void *)(CONFIG_SYS_UBOOT_BASE + sizeof(struct image_header)),
+	       spl_image.size);
+}
+
 void spl_nor_load_image(void)
 {
 	/*
@@ -15,37 +54,6 @@  void spl_nor_load_image(void)
 	 */
 	spl_image.flags |= SPL_COPY_PAYLOAD_ONLY;
 
-	if (spl_start_uboot()) {
-		/*
-		 * Load real U-Boot from its location in NOR flash to its
-		 * defined location in SDRAM
-		 */
-		spl_parse_image_header(
-			(const struct image_header *)CONFIG_SYS_UBOOT_BASE);
-
-		memcpy((void *)spl_image.load_addr,
-		       (void *)(CONFIG_SYS_UBOOT_BASE +
-				sizeof(struct image_header)),
-		       spl_image.size);
-	} else {
-		/*
-		 * Load Linux from its location in NOR flash to its defined
-		 * location in SDRAM
-		 */
-		spl_parse_image_header(
-			(const struct image_header *)CONFIG_SYS_OS_BASE);
-
-		memcpy((void *)spl_image.load_addr,
-		       (void *)(CONFIG_SYS_OS_BASE +
-				sizeof(struct image_header)),
-		       spl_image.size);
-
-		/*
-		 * Copy DT blob (fdt) to SDRAM. Passing pointer to flash
-		 * doesn't work (16 KiB should be enough for DT)
-		 */
-		memcpy((void *)CONFIG_SYS_SPL_ARGS_ADDR,
-		       (void *)(CONFIG_SYS_FDT_BASE),
-		       (16 << 10));
-	}
+	if (load_linux() < 0)
+		load_uboot();
 }