Hi Simon, On Thu, Oct 29, 2015 at 11:19:38AM -0600, Simon Glass wrote:
Hi Nikita,
On 28 October 2015 at 03:23, Nikita Kiryanov nikita@compulab.co.il wrote:
Make spl_*_load_image() functions return a value instead of hanging if a problem is encountered. This enables main spl code to make the decision whether to hang or not, thus preparing it to support alternative boot devices.
Some boot devices (namely nand and spi) do not hang on error. Instead, they return normally and SPL proceeds to boot the contents of the load address. This is considered a bug and is rectified by hanging on error for these devices as well.
Signed-off-by: Nikita Kiryanov nikita@compulab.co.il Cc: Igor Grinberg grinberg@compulab.co.il Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Ian Campbell ijc@hellion.org.uk Cc: Hans De Goede hdegoede@redhat.com Cc: Albert Aribaud albert.u.boot@aribaud.net Cc: Jagan Teki jteki@openedev.com
Changes in V2: - Minor collateral adjustments from changes in patch 2 (only one return statement at the end of spl_mmc_load_image).
arch/arm/cpu/armv7/sunxi/board.c | 4 +++- arch/arm/include/asm/spl.h | 2 +- common/spl/spl.c | 43 +++++++++++++++++++++++++++------------- common/spl/spl_mmc.c | 26 ++++++++++++++---------- common/spl/spl_nand.c | 18 +++++++++++------ common/spl/spl_net.c | 9 ++++++--- common/spl/spl_nor.c | 6 ++++-- common/spl/spl_onenand.c | 4 +++- common/spl/spl_sata.c | 11 +++++++--- common/spl/spl_usb.c | 17 ++++++++++------ common/spl/spl_ymodem.c | 5 +++-- drivers/mtd/spi/spi_spl_load.c | 17 +++++++++++----- include/spl.h | 18 ++++++++--------- 13 files changed, 116 insertions(+), 64 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index 4785ac6..9b5c46b 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -95,10 +95,12 @@ static int gpio_init(void) return 0; }
-void spl_board_load_image(void) +int spl_board_load_image(void) { debug("Returning to FEL sp=%x, lr=%x\n", fel_stash.sp, fel_stash.lr); return_to_fel(fel_stash.sp, fel_stash.lr);
return 0;}
void s_init(void) diff --git a/arch/arm/include/asm/spl.h b/arch/arm/include/asm/spl.h index 6db405d..f8092c7 100644 --- a/arch/arm/include/asm/spl.h +++ b/arch/arm/include/asm/spl.h @@ -32,7 +32,7 @@ enum { #endif
/* Board-specific load method */ -void spl_board_load_image(void); +int spl_board_load_image(void);
Please add a comment about what this does, and the return value.
OK
/* Linker symbols. */ extern char __bss_start[], __bss_end[]; diff --git a/common/spl/spl.c b/common/spl/spl.c index 4b319d6..ff1bad2 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -132,7 +132,7 @@ __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) }
#ifdef CONFIG_SPL_RAM_DEVICE -static void spl_ram_load_image(void) +static int spl_ram_load_image(void) { const struct image_header *header;
@@ -145,6 +145,8 @@ static void spl_ram_load_image(void) (CONFIG_SYS_TEXT_BASE - sizeof(struct image_header));
spl_parse_image_header(header);
return 0;} #endif
@@ -208,68 +210,81 @@ void board_init_r(gd_t *dummy1, ulong dummy2) switch (boot_device) { #ifdef CONFIG_SPL_RAM_DEVICE case BOOT_DEVICE_RAM:
spl_ram_load_image();
if (spl_ram_load_image())hang(); break;#endif #ifdef CONFIG_SPL_MMC_SUPPORT case BOOT_DEVICE_MMC1: case BOOT_DEVICE_MMC2: case BOOT_DEVICE_MMC2_2:
spl_mmc_load_image();
if (spl_mmc_load_image())hang();Can you use something like:
ret = spl_mmc_load_image()
and put the hang() after the switch()?
The next patch takes care of that.
break;#endif #ifdef CONFIG_SPL_NAND_SUPPORT case BOOT_DEVICE_NAND:
spl_nand_load_image();
if (spl_nand_load_image())hang(); break;#endif #ifdef CONFIG_SPL_ONENAND_SUPPORT case BOOT_DEVICE_ONENAND:
spl_onenand_load_image();
if (spl_onenand_load_image())hang(); break;#endif #ifdef CONFIG_SPL_NOR_SUPPORT case BOOT_DEVICE_NOR:
spl_nor_load_image();
if (spl_nor_load_image())hang(); break;#endif #ifdef CONFIG_SPL_YMODEM_SUPPORT case BOOT_DEVICE_UART:
spl_ymodem_load_image();
if (spl_ymodem_load_image())hang(); break;#endif #ifdef CONFIG_SPL_SPI_SUPPORT case BOOT_DEVICE_SPI:
spl_spi_load_image();
if (spl_spi_load_image())hang(); break;#endif #ifdef CONFIG_SPL_ETH_SUPPORT case BOOT_DEVICE_CPGMAC: #ifdef CONFIG_SPL_ETH_DEVICE
spl_net_load_image(CONFIG_SPL_ETH_DEVICE);
if (spl_net_load_image(CONFIG_SPL_ETH_DEVICE))hang();#else
spl_net_load_image(NULL);
if (spl_net_load_image(NULL))hang();#endif break; #endif
[snip]
Regards, Simon