Message ID | 1418259698-7847-1-git-send-email-yamada.m@jp.panasonic.com |
---|---|
State | Changes Requested |
Delegated to: | Tom Rini |
Headers | show |
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 :)
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
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
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().
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 --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(); }
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(-)