| Message ID | 1446024210-16517-9-git-send-email-nikita@compulab.co.il |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
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. > > /* 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
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 >
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); /* 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(); 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 #ifdef CONFIG_SPL_USBETH_SUPPORT case BOOT_DEVICE_USBETH: - spl_net_load_image("usb_ether"); + if (spl_net_load_image("usb_ether")) + hang(); break; #endif #ifdef CONFIG_SPL_USB_SUPPORT case BOOT_DEVICE_USB: - spl_usb_load_image(); + if (spl_usb_load_image()) + hang(); break; #endif #ifdef CONFIG_SPL_SATA_SUPPORT case BOOT_DEVICE_SATA: - spl_sata_load_image(); + if (spl_sata_load_image()) + hang(); break; #endif #ifdef CONFIG_SPL_BOARD_LOAD_IMAGE case BOOT_DEVICE_BOARD: - spl_board_load_image(); + if (spl_board_load_image()) + hang(); break; #endif default: diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index eaba29b..ab9ec88 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -10,6 +10,7 @@ #include <dm.h> #include <spl.h> #include <linux/compiler.h> +#include <errno.h> #include <asm/u-boot.h> #include <errno.h> #include <mmc.h> @@ -220,25 +221,27 @@ int spl_mmc_do_fs_boot(struct mmc *mmc) } #endif -void spl_mmc_load_image(void) +int spl_mmc_load_image(void) { struct mmc *mmc; u32 boot_mode; int err = 0; __maybe_unused int part; - if (spl_mmc_find_device(&mmc)) - hang(); + err = spl_mmc_find_device(&mmc); + if (err) + return err; err = mmc_init(mmc); if (err) { #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: mmc init failed with error: %d\n", err); #endif - hang(); + return err; } boot_mode = spl_boot_mode(); + err = -EINVAL; switch (boot_mode) { case MMCSD_MODE_EMMCBOOT: /* @@ -251,11 +254,12 @@ void spl_mmc_load_image(void) if (part == 7) part = 0; - if (mmc_switch_part(0, part)) { + err = mmc_switch_part(0, part); + if (err) { #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT puts("spl: mmc partition switch failed\n"); #endif - hang(); + return err; } /* Fall through */ case MMCSD_MODE_RAW: @@ -264,18 +268,18 @@ void spl_mmc_load_image(void) if (!spl_start_uboot()) { err = mmc_load_image_raw_os(mmc); if (!err) - return; + return err; } err = mmc_load_image_raw_partition(mmc, CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION); if (!err) - return; + return err; #if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR) err = mmc_load_image_raw_sector(mmc, CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR); if (!err) - return; + return err; #endif break; case MMCSD_MODE_FS: @@ -283,7 +287,7 @@ void spl_mmc_load_image(void) err = spl_mmc_do_fs_boot(mmc); if (!err) - return; + return err; break; case MMCSD_MODE_UNDEFINED: @@ -293,5 +297,5 @@ void spl_mmc_load_image(void) #endif } - hang(); + return err; } diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c index 6e4e641..ad69db7 100644 --- a/common/spl/spl_nand.c +++ b/common/spl/spl_nand.c @@ -11,7 +11,7 @@ #include <nand.h> #if defined(CONFIG_SPL_NAND_RAW_ONLY) -void spl_nand_load_image(void) +int spl_nand_load_image(void) { nand_init(); @@ -20,6 +20,8 @@ void spl_nand_load_image(void) (void *)CONFIG_SYS_NAND_U_BOOT_DST); spl_set_header_raw_uboot(); nand_deselect(); + + return 0; } #else static int spl_nand_load_element(int offset, struct image_header *header) @@ -35,8 +37,9 @@ static int spl_nand_load_element(int offset, struct image_header *header) (void *)spl_image.load_addr); } -void spl_nand_load_image(void) +int spl_nand_load_image(void) { + int err; struct image_header *header; int *src __attribute__((unused)); int *dst __attribute__((unused)); @@ -73,10 +76,12 @@ void spl_nand_load_image(void) spl_parse_image_header(header); if (header->ih_os == IH_OS_LINUX) { /* happy - was a linux */ - nand_spl_load_image(CONFIG_SYS_NAND_SPL_KERNEL_OFFS, - spl_image.size, (void *)spl_image.load_addr); + err = nand_spl_load_image( + CONFIG_SYS_NAND_SPL_KERNEL_OFFS, + spl_image.size, + (void *)spl_image.load_addr); nand_deselect(); - return; + return err; } else { puts("The Expected Linux image was not " "found. Please check your NAND " @@ -92,7 +97,8 @@ void spl_nand_load_image(void) #endif #endif /* Load u-boot */ - spl_nand_load_element(CONFIG_SYS_NAND_U_BOOT_OFFS, header); + err = spl_nand_load_element(CONFIG_SYS_NAND_U_BOOT_OFFS, header); nand_deselect(); + return err; } #endif diff --git a/common/spl/spl_net.c b/common/spl/spl_net.c index 217a435..63b20d8 100644 --- a/common/spl/spl_net.c +++ b/common/spl/spl_net.c @@ -8,12 +8,13 @@ * SPDX-License-Identifier: GPL-2.0+ */ #include <common.h> +#include <errno.h> #include <spl.h> #include <net.h> DECLARE_GLOBAL_DATA_PTR; -void spl_net_load_image(const char *device) +int spl_net_load_image(const char *device) { int rv; @@ -24,14 +25,16 @@ void spl_net_load_image(const char *device) rv = eth_initialize(); if (rv == 0) { printf("No Ethernet devices found\n"); - hang(); + return -ENODEV; } if (device) setenv("ethact", device); rv = net_loop(BOOTP); if (rv < 0) { printf("Problem booting with BOOTP\n"); - hang(); + return rv; } spl_parse_image_header((struct image_header *)load_addr); + + return 0; } diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c index c2fee01..e08afe2 100644 --- a/common/spl/spl_nor.c +++ b/common/spl/spl_nor.c @@ -7,7 +7,7 @@ #include <common.h> #include <spl.h> -void spl_nor_load_image(void) +int spl_nor_load_image(void) { /* * Loading of the payload to SDRAM is done with skipping of @@ -43,7 +43,7 @@ void spl_nor_load_image(void) (void *)(CONFIG_SYS_FDT_BASE), (16 << 10)); - return; + return 0; } else { puts("The Expected Linux image was not found.\n" "Please check your NOR configuration.\n" @@ -62,4 +62,6 @@ void spl_nor_load_image(void) memcpy((void *)spl_image.load_addr, (void *)(CONFIG_SYS_UBOOT_BASE + sizeof(struct image_header)), spl_image.size); + + return 0; } diff --git a/common/spl/spl_onenand.c b/common/spl/spl_onenand.c index d8d8097..af7d82e 100644 --- a/common/spl/spl_onenand.c +++ b/common/spl/spl_onenand.c @@ -14,7 +14,7 @@ #include <asm/io.h> #include <onenand_uboot.h> -void spl_onenand_load_image(void) +int spl_onenand_load_image(void) { struct image_header *header; @@ -28,4 +28,6 @@ void spl_onenand_load_image(void) spl_parse_image_header(header); onenand_spl_load_image(CONFIG_SYS_ONENAND_U_BOOT_OFFS, spl_image.size, (void *)spl_image.load_addr); + + return 0; } diff --git a/common/spl/spl_sata.c b/common/spl/spl_sata.c index 2a5eb29..3ba4c24 100644 --- a/common/spl/spl_sata.c +++ b/common/spl/spl_sata.c @@ -14,12 +14,13 @@ #include <asm/u-boot.h> #include <sata.h> #include <scsi.h> +#include <errno.h> #include <fat.h> #include <image.h> DECLARE_GLOBAL_DATA_PTR; -void spl_sata_load_image(void) +int spl_sata_load_image(void) { int err; block_dev_desc_t *stor_dev; @@ -29,11 +30,13 @@ void spl_sata_load_image(void) #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("spl: sata init failed: err - %d\n", err); #endif - hang(); + return err; } else { /* try to recognize storage devices immediately */ scsi_scan(0); stor_dev = scsi_get_dev(0); + if (!stor_dev) + return -ENODEV; } #ifdef CONFIG_SPL_OS_BOOT @@ -45,6 +48,8 @@ void spl_sata_load_image(void) CONFIG_SPL_FS_LOAD_PAYLOAD_NAME); if (err) { puts("Error loading sata device\n"); - hang(); + return err; } + + return 0; } diff --git a/common/spl/spl_usb.c b/common/spl/spl_usb.c index c81672b..588b85c 100644 --- a/common/spl/spl_usb.c +++ b/common/spl/spl_usb.c @@ -12,6 +12,7 @@ #include <common.h> #include <spl.h> #include <asm/u-boot.h> +#include <errno.h> #include <usb.h> #include <fat.h> @@ -21,7 +22,7 @@ DECLARE_GLOBAL_DATA_PTR; static int usb_stor_curr_dev = -1; /* current device */ #endif -void spl_usb_load_image(void) +int spl_usb_load_image(void) { int err; block_dev_desc_t *stor_dev; @@ -32,13 +33,15 @@ void spl_usb_load_image(void) #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT printf("%s: usb init failed: err - %d\n", __func__, err); #endif - hang(); + return err; } #ifdef CONFIG_USB_STORAGE /* try to recognize storage devices immediately */ usb_stor_curr_dev = usb_stor_scan(1); stor_dev = usb_stor_get_dev(usb_stor_curr_dev); + if (!stor_dev) + return -ENODEV; #endif debug("boot mode - FAT\n"); @@ -51,8 +54,10 @@ void spl_usb_load_image(void) CONFIG_SYS_USB_FAT_BOOT_PARTITION, CONFIG_SPL_FS_LOAD_PAYLOAD_NAME); - if (err) { - puts("Error loading USB device\n"); - hang(); - } + if (err) { + puts("Error loading from USB device\n"); + return err; + } + + return 0; } diff --git a/common/spl/spl_ymodem.c b/common/spl/spl_ymodem.c index 0f1e9970..380d8dd 100644 --- a/common/spl/spl_ymodem.c +++ b/common/spl/spl_ymodem.c @@ -23,7 +23,7 @@ static int getcymodem(void) { return -1; } -void spl_ymodem_load_image(void) +int spl_ymodem_load_image(void) { int size = 0; int err; @@ -49,11 +49,12 @@ void spl_ymodem_load_image(void) } } else { printf("spl: ymodem err - %s\n", xyzModem_error(err)); - hang(); + return ret; } xyzModem_stream_close(&err); xyzModem_stream_terminate(false, &getcymodem); printf("Loaded %d bytes\n", size); + return 0; } diff --git a/drivers/mtd/spi/spi_spl_load.c b/drivers/mtd/spi/spi_spl_load.c index 2e0c871..ca56fe9 100644 --- a/drivers/mtd/spi/spi_spl_load.c +++ b/drivers/mtd/spi/spi_spl_load.c @@ -12,6 +12,7 @@ #include <common.h> #include <spi.h> #include <spi_flash.h> +#include <errno.h> #include <spl.h> #ifdef CONFIG_SPL_OS_BOOT @@ -48,8 +49,9 @@ static int spi_load_image_os(struct spi_flash *flash, * configured and available since this code loads the main U-Boot image * from SPI into SDRAM and starts it from there. */ -void spl_spi_load_image(void) +int spl_spi_load_image(void) { + int err = 0; struct spi_flash *flash; struct image_header *header; @@ -63,7 +65,7 @@ void spl_spi_load_image(void) CONFIG_SF_DEFAULT_MODE); if (!flash) { puts("SPI probe failed.\n"); - hang(); + return -ENODEV; } /* use CONFIG_SYS_TEXT_BASE as temporary storage area */ @@ -74,10 +76,15 @@ void spl_spi_load_image(void) #endif { /* Load u-boot, mkimage header is 64 bytes. */ - spi_flash_read(flash, CONFIG_SYS_SPI_U_BOOT_OFFS, 0x40, - (void *)header); + err = spi_flash_read(flash, CONFIG_SYS_SPI_U_BOOT_OFFS, 0x40, + (void *)header); + if (err) + return err; + spl_parse_image_header(header); - spi_flash_read(flash, CONFIG_SYS_SPI_U_BOOT_OFFS, + err = spi_flash_read(flash, CONFIG_SYS_SPI_U_BOOT_OFFS, spl_image.size, (void *)spl_image.load_addr); } + + return err; } diff --git a/include/spl.h b/include/spl.h index 8e53426..46fc454 100644 --- a/include/spl.h +++ b/include/spl.h @@ -45,31 +45,31 @@ int spl_start_uboot(void); void spl_display_print(void); /* NAND SPL functions */ -void spl_nand_load_image(void); +int spl_nand_load_image(void); /* OneNAND SPL functions */ -void spl_onenand_load_image(void); +int spl_onenand_load_image(void); /* NOR SPL functions */ -void spl_nor_load_image(void); +int spl_nor_load_image(void); /* MMC SPL functions */ -void spl_mmc_load_image(void); +int spl_mmc_load_image(void); /* YMODEM SPL functions */ -void spl_ymodem_load_image(void); +int spl_ymodem_load_image(void); /* SPI SPL functions */ -void spl_spi_load_image(void); +int spl_spi_load_image(void); /* Ethernet SPL functions */ -void spl_net_load_image(const char *device); +int spl_net_load_image(const char *device); /* USB SPL functions */ -void spl_usb_load_image(void); +int spl_usb_load_image(void); /* SATA SPL functions */ -void spl_sata_load_image(void); +int spl_sata_load_image(void); /* SPL FAT image functions */ int spl_load_image_fat(block_dev_desc_t *block_dev, int partition, const char *filename);
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(-)