Re: [U-Boot] [PATCH] sunxi/spl: Detect at runtime where SPL was read from

Hey Daniel,

The approach seems good. Some comments inline

2015-05-28 10:43 GMT+02:00 Daniel Kochmański < dkochmanski@turtle-solutions.eu>:

This patch makes possible using single `u-boot-sunxi-with-spl.bin` binary for both NAND memory and SD card. Detection where SPL was read from is implemented in `spl_boot_device`.

Detection is performed only if `CONFIG_SPL_NAND_SUPPORT` is defined. Unless SD card contains valid signature we assume, that SPL was read from NAND.

Signed-off-by: Daniel Kochmański dkochmanski@turtle-solutions.eu CC: Roy Spliet r.spliet@ultimaker.com Cc: Ian Campbell ijc@hellion.org.uk Cc: Hans De Goede hdegoede@redhat.com

---

arch/arm/cpu/armv7/sunxi/board.c | 53 ++++++++++++++++++++++++++-------------- include/configs/sunxi-common.h | 2 -- 2 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/board.c b/arch/arm/cpu/armv7/sunxi/board.c index 70f413f..5e441ba 100644 --- a/arch/arm/cpu/armv7/sunxi/board.c +++ b/arch/arm/cpu/armv7/sunxi/board.c @@ -11,6 +11,7 @@ */

#include <common.h> +#include <mmc.h> #include <i2c.h> #include <serial.h> #ifdef CONFIG_SPL_BUILD @@ -109,12 +110,10 @@ void s_init(void) }

#ifdef CONFIG_SPL_BUILD +DECLARE_GLOBAL_DATA_PTR;

+

/* The sunxi internal brom will try to loader external bootloader

• from mmc0, nand flash, mmc2.

• • Unfortunately we can't check how SPL was loaded so assume it's
• • always the first SD/MMC controller, unless it was explicitly
• • stated that SPL is on nand flash.
  */

u32 spl_boot_device(void) { @@ -124,17 +123,13 @@ u32 spl_boot_device(void) * enabled build. It has many restrictions and can only boot over USB. */ return BOOT_DEVICE_BOARD; -#elif defined(CONFIG_SPL_NAND_SUPPORT)

•   /*
  

•    * This is compile time configuration informing SPL, that it
  

•    * was loaded from nand flash.
  

•    */
  

•   return BOOT_DEVICE_NAND;
  

#else

•   __maybe_unused struct mmc *mmc0;
  

•   __maybe_unused char buf[512];
  
  

​Maybe_unused? Either reserve it or don't. If you only need them for ​​CONFIG_SPL_NAND_SUPPORT, then ifdef around it. Also, why are you reserving half a K if you are only going to use 12 bytes? And if you need to round up, can't you just malloc it from SDRAM instead of reserving this space in SRAM like this?

    /*

•    * When booting from the SD card, the "eGON.BT0" signature is
  

expected

•    * to be found in memory at the address 0x0004 (see the
  

"mksunxiboot"

•    * tool, which generates this header).
  

•    * When booting from the SD card or NAND memory, the "eGON.BT0"
  

•    * signature is expected to be found in memory at the address
  

0x0004

•    * (see the "mksunxiboot" tool, which generates this header).
     *
     * When booting in the FEL mode over USB, this signature is
  

patched in * memory and replaced with something else by the 'fel' tool. This other @@ -142,13 +137,35 @@ u32 spl_boot_device(void) * valid bootable SD card image (because the BROM would refuse to * execute the SPL in this case). *

•    * This branch is just making a decision at runtime whether to load
  

•    * the main u-boot binary from the SD card (if the "eGON.BT0"
  

signature

•    * is found) or return to the FEL code in the BROM to wait and
  

receive

•    * the main u-boot binary over USB.
  

•    * This branch is just making a decision at runtime whether to
  

load the

•    * main u-boot binary from the SD card or NAND memory (if the
  

"eGON.BT0"

•    * signature is found) or return to the FEL code in the BROM to
  

wait and

•    * receive the main u-boot binary over USB. If signature is
  

present,

•    * decision where to boot from (SD card or NAND memory) depends on
  

•    * compile options (if SPL_NAND_SUPPORT isn't defined, we assume
  

we boot

•    * from SD card), and runtime check - if SD card doesn't contain
  

valid

•    * signature we assume that SPL was loaded from NAND.
     */
  

•   if (readl(4) == 0x4E4F4765 && readl(8) == 0x3054422E) /* eGON.BT0
  

*/

•   if (readl(4) == 0x4E4F4765 && readl(8) == 0x3054422E) { /*
  

eGON.BT0 */ +#if !defined( ​​ CONFIG_SPL_NAND_SUPPORT) return BOOT_DEVICE_MMC1; +#else

•           mmc_initialize(gd->bd);
  

​​ mmc0 = find_mmc_device(0); ​

​Is this not supposed to be ​mmc0 = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV​); ? I'm not entirely sure, Hans, how should this constant be interpreted?

​

•           /*
  

•            * Figure out where we're booting from. Try mmc0 first,
  

just

•            * like the brom does. If it doesn't contain valid
  

signature,

•            * assume SPL was loaded from NAND memory.
  

•            */
  

•           if (mmc0 && mmc_getcd(mmc0) && mmc_init(mmc0) == 0 &&
  

•               mmc0->block_dev.block_read(0, 16, 1, buf) == 1) {
  

•                   buf[12] = 0;
  
  

​String terminator? I'd just use strncmp or try the same comparisons as above.​

•                   if (strcmp(&buf[4], "eGON.BT0") == 0)
  

•                           return BOOT_DEVICE_MMC1;
  

•           }
  

•           return BOOT_DEVICE_NAND;
  

+#endif

•   }
    else
            return BOOT_DEVICE_BOARD;
  

#endif diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index cce0441..e4db777 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -106,10 +106,8 @@ #define CONFIG_CMD_MMC #define CONFIG_MMC_SUNXI #define CONFIG_MMC_SUNXI_SLOT 0 -#if !defined(CONFIG_SPL_NAND_SUPPORT) #define CONFIG_ENV_IS_IN_MMC #define CONFIG_SYS_MMC_ENV_DEV 0 /* first detected MMC controller */ -#endif /* CONFIG_SPL_NAND_SUPPORT */ #endif

/* 4MB of malloc() pool */

2.4.1