Hi Nikita,
On 28 October 2015 at 03:23, Nikita Kiryanov <nik...@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 <nik...@compulab.co.il>
> Cc: Igor Grinberg <grinb...@compulab.co.il>
> Cc: Tom Rini <tr...@konsulko.com>
> Cc: Simon Glass <s...@chromium.org>
> Cc: Ian Campbell <i...@hellion.org.uk>
> Cc: Hans De Goede <hdego...@redhat.com>
> Cc: Albert Aribaud <albert.u.b...@aribaud.net>
> Cc: Jagan Teki <jt...@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.
>
> /* 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()?
> 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
