[U-Boot] [PATCH] sunxi/spl: Detect at runtime where SPL was read from
Hans de Goede
hdegoede at redhat.com
Thu May 28 11:52:33 CEST 2015
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 at 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 at turtle-solutions.eu>
Daniel, thanks for working on this. I've added my comments to Roy's
comments.
>> CC: Roy Spliet <r.spliet at ultimaker.com>
>> Cc: Ian Campbell <ijc at hellion.org.uk>
>> Cc: Hans De Goede <hdegoede at 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.
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).
>
>> /*
>> - * 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?
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 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.
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.
>
>> + 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 */
Regards,
Hans
>> #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
>>
>>
>
More information about the U-Boot
mailing list