Hello,
Hans de Goede writes:
Hi,
On 28-05-15 11:08, Roy Spliet wrote:
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
Daniel, thanks for working on this. I've added my comments to Roy's comments.
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
*/ u32 spl_boot_device(void) {
- stated that SPL is on nand flash.
@@ -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;
__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.
No, __maybe_unused is appropriate here in u-boot (and AFAIK the kernel too) this is preferred over sprinkling #ifdef's everywhere.
Also, why are you reserving half a K if you are only going to use 12 bytes?
Because mmc-s emulate disks and we are reading a sector from the mmc here, which requires a sector-sized buffer.
And if you need to round up, can't you just malloc it from SDRAM instead of reserving this space in SRAM like this?
I think that we're post dram init here, so yes malloc should work, and given the stack size limitations it is probably a good idea to switch to using malloc (and panic on malloc failure).
Ack
/*
* When booting from the SD card, the "eGON.BT0" signature isexpected
* 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 address0x0004
* (see the "mksunxiboot" tool, which generates this header). * * When booting in the FEL mode over USB, this signature ispatched 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 andreceive
* the main u-boot binary over USB.
* This branch is just making a decision at runtime whether toload 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 towait and
* receive the main u-boot binary over USB. If signature ispresent,
* decision where to boot from (SD card or NAND memory) depends on* compile options (if SPL_NAND_SUPPORT isn't defined, we assumewe boot
* from SD card), and runtime check - if SD card doesn't containvalid
* 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?
We really want mmc0 here since that is what the RROM uses so 0 is correct.
/** Figure out where we're booting from. Try mmc0 first,just
* like the brom does. If it doesn't contain validsignature,
* 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.
True that would work too, actually this is code I wrote, which Daniel has copied from board/sunxi/board.c .
Daniel for the next revision of this patch set please factor the code to check for a bootable sdcard in mmc0 out of sunxi/board/board.c into a helper function, and use that helper function in both code paths.
Also please do the factoring out in a separate preparation patch.
Ok - I'm not sure however, where I should put function declaration - in `arch/arm/include/asm/arch-sunxi/mmc.h`? Definition could be in `board/sunxi/board.c` as separate function. If not, where does such helper functions belong?
if (strcmp(&buf[4], "eGON.BT0") == 0)return BOOT_DEVICE_MMC1;}return BOOT_DEVICE_NAND;+#endif
} else return BOOT_DEVICE_BOARD;I'm not thrilled about the nesting here, IMHO it would be better to have something like this:
/* * 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 checks for the signature and if it is not found returns to * the FEL code in the BROM to wait and receive the main u-boot * binary over USB. */ if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) /* eGON.BT0 */ return BOOT_DEVICE_BOARD;
/* The BROM will try to boot from mmc0 first, so try that first */ if (sunxi_mmc0_has_egon_boot_signature()) return BOOT_DEVICE_MMC1;
/* Fallback to booting NAND if enabled */ if (IS_ENABLED(SPL_NAND_SUPPORT)) return BOOT_DEVICE_NAND;
panic("Could not determine boot source\n"); return -1; /* Never reached */
Ack
Regards,
Hans
Best regards, Daniel
#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