Hi,
On 28-05-15 12:05, Daniel Kochmański wrote:
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`?
Yes that is a good place for it.
Definition could be in `board/sunxi/board.c` as separate function. If not, where does such helper functions belong?
Putting the function in board/sunxi/board.c is fine.
Regards,
Hans