From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikita Kiryanov Date: Sat, 31 Oct 2015 17:44:33 +0200 Subject: [U-Boot] [PATCH V2 08/13] spl: change return values of spl_*_load_image() In-Reply-To: References: <1446024210-16517-1-git-send-email-nikita@compulab.co.il> <1446024210-16517-9-git-send-email-nikita@compulab.co.il> Message-ID: <20151031154433.GC27702@skynet> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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 > > Cc: Igor Grinberg > > Cc: Tom Rini > > Cc: Simon Glass > > Cc: Ian Campbell > > Cc: Hans De Goede > > Cc: Albert Aribaud > > Cc: Jagan Teki > > --- > > 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 >