Skip to contentSkip to conversation

sunxi/spl: Detect at runtime where SPL was read from

PATCH New Daniel Kochmański 3 participants 5 messages May 28, 2015

Subsystems (2)
Arm (32-bit) Arm/Allwinner sunXi SoC support
lore
Share & tools
lore

Daniel Kochmański

author
#0 lore raw

PATCH 1/1 sunxi/spl: Detect at runtime where SPL was read from

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 <[email protected]>
CC Roy Spliet <[email protected]>
Cc Ian Campbell <[email protected]>
Cc Hans De Goede <[email protected]>
---
 arch/arm/cpu/armv7/sunxi/board.c | 53 ++++++++++++++++++++++++++--------------
 include/configs/sunxi-common.h   |  2 --
 2 files changed, 35 insertions(+), 20 deletions(-)
arch/arm/cpu/armv7/sunxi/board.c +35 -18
index 70f413f..5e441ba 100644
@@ -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];
 	/*
-	 * 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);
+		/*
+		 * 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;
+			if (strcmp(&buf[4], "eGON.BT0") == 0)
+				return BOOT_DEVICE_MMC1;
+		}
+		return BOOT_DEVICE_NAND;
+#endif
+	}
 	else
 		return BOOT_DEVICE_BOARD;
 #endif
include/configs/sunxi-common.h +0 -2
index cce0441..e4db777 100644
@@ -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 */
Signature · 1 line
2.4.1
#1 lore raw

PATCH 1/1 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 <
[email protected]>:

Quoted 31 lines

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 <[email protected]>
CC Roy Spliet <[email protected]>
Cc Ian Campbell <[email protected]>
Cc Hans De Goede <[email protected]>
---
 arch/arm/cpu/armv7/sunxi/board.c | 53
++++++++++++++++++++++++++--------------
 include/configs/sunxi-common.h   |  2 --
 2 files changed, 35 insertions(+), 20 deletions(-)
arch/arm/cpu/armv7/sunxi/board.c +1 -0
index 70f413f..5e441ba 100644
@@ -11,6 +11,7 @@
  */
 
 #include <common.h>
+#include <mmc.h>
 #include <i2c.h>
 #include <serial.h>
 #ifdef CONFIG_SPL_BUILD

arch/arm/cpu/armv7/sunxi/board.c void s_init(void) · line 114

Quoted 5 lines
arch/arm/cpu/armv7/sunxi/board.c +1 -0
@@ -109,12 +110,10 @@ void s_init(void)
 }
 
 #ifdef CONFIG_SPL_BUILD
+DECLARE_GLOBAL_DATA_PTR;

+

Quoted 15 lines
arch/arm/cpu/armv7/sunxi/board.c +0 -4
 /* 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;

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 128

Quoted 9 lines
arch/arm/cpu/armv7/sunxi/board.c +2 -6
-#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?

Quoted 51 lines
        /*

- * When booting from the SD card, the "eGON.BT0" signature is
expected
- * to be found in memory at the address 0x0004 (see the
"mksunxiboot"

arch/arm/cpu/armv7/sunxi/board.c +2 -1
-        * 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

arch/arm/cpu/armv7/sunxi/board.c +0 -2
@@ -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

arch/arm/cpu/armv7/sunxi/board.c +1 -1
-        * 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,

arch/arm/cpu/armv7/sunxi/board.c +2 -0
+        * 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

arch/arm/cpu/armv7/sunxi/board.c +1 -1
+        * 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;
arch/arm/cpu/armv7/sunxi/board.c +3 -0
+#else
+               mmc_initialize(gd->bd);
+
Quoted 4 lines

??
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?

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 162

Quoted 13 lines

?

arch/arm/cpu/armv7/sunxi/board.c +2 -0
+               /*
+                * Figure out where we're booting from. Try mmc0 first,

just
+ * like the brom does. If it doesn't contain valid
signature,

arch/arm/cpu/armv7/sunxi/board.c +5 -0
+                * 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.?

Quoted 30 lines
arch/arm/cpu/armv7/sunxi/board.c +6 -0
+                       if (strcmp(&buf[4], "eGON.BT0") == 0)
+                               return BOOT_DEVICE_MMC1;
+               }
+               return BOOT_DEVICE_NAND;
+#endif
+       }
        else
                return BOOT_DEVICE_BOARD;
 #endif
include/configs/sunxi-common.h +0 -1
index cce0441..e4db777 100644
@@ -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

Signature · 6 lines
IMAGINE IT >> MAKE IT

Meet us online at Twitter <http://twitter.com/ultimaker>, Facebook 
<http://facebook.com/ultimaker>, Google+ <http://google.com/+Ultimaker>

www.ultimaker.com
#2 lore raw

PATCH 1/1 sunxi/spl: Detect at runtime where SPL was read from

Hi,

Quoted 15 lines · Roy Spliet

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>:

Quoted 8 lines

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 <[email protected]>

Daniel, thanks for working on this. I've added my comments to Roy's comments.

Quoted 47 lines
Quoted 23 lines
CC Roy Spliet <[email protected]>
Cc Ian Campbell <[email protected]>
Cc Hans De Goede <[email protected]>
---
  arch/arm/cpu/armv7/sunxi/board.c | 53
++++++++++++++++++++++++++--------------
  include/configs/sunxi-common.h   |  2 --
  2 files changed, 35 insertions(+), 20 deletions(-)
arch/arm/cpu/armv7/sunxi/board.c +1 -0
index 70f413f..5e441ba 100644
@@ -11,6 +11,7 @@
   */
 
  #include <common.h>
+#include <mmc.h>
  #include <i2c.h>
  #include <serial.h>
  #ifdef CONFIG_SPL_BUILD

arch/arm/cpu/armv7/sunxi/board.c void s_init(void) · line 114

Quoted 5 lines
arch/arm/cpu/armv7/sunxi/board.c +1 -0
@@ -109,12 +110,10 @@ void s_init(void)
  }
 
  #ifdef CONFIG_SPL_BUILD
+DECLARE_GLOBAL_DATA_PTR;

+

Quoted 18 lines
arch/arm/cpu/armv7/sunxi/board.c +0 -4
  /* 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;
arch/arm/cpu/armv7/sunxi/board.c +0 -4
-#elif defined(CONFIG_SPL_NAND_SUPPORT)
-       /*
-        * This is compile time configuration informing SPL, that it
-        * was loaded from nand flash.
Quoted 8 lines

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 128

Quoted 6 lines
arch/arm/cpu/armv7/sunxi/board.c +2 -2
-        */
-       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.

Quoted 2 lines

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.

Quoted 3 lines

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).

Quoted 51 lines
Quoted 51 lines
         /*

- * When booting from the SD card, the "eGON.BT0" signature is
expected
- * to be found in memory at the address 0x0004 (see the
"mksunxiboot"

arch/arm/cpu/armv7/sunxi/board.c +2 -1
-        * 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

arch/arm/cpu/armv7/sunxi/board.c +0 -2
@@ -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

arch/arm/cpu/armv7/sunxi/board.c +1 -1
-        * 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,

arch/arm/cpu/armv7/sunxi/board.c +2 -0
+        * 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

arch/arm/cpu/armv7/sunxi/board.c +1 -1
+        * 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;
arch/arm/cpu/armv7/sunxi/board.c +2 -0
+#else
+               mmc_initialize(gd->bd);
Quoted 8 lines
Quoted 5 lines

+
??
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.

Quoted 13 lines

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 162

Quoted 13 lines

?

arch/arm/cpu/armv7/sunxi/board.c +2 -0
+               /*
+                * Figure out where we're booting from. Try mmc0 first,

just
+ * like the brom does. If it doesn't contain valid
signature,

arch/arm/cpu/armv7/sunxi/board.c +5 -0
+                * 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;
Quoted 2 lines

?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.

Quoted 10 lines

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 170

Quoted 10 lines
arch/arm/cpu/armv7/sunxi/board.c +6 -0
+                       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

Quoted 23 lines
Quoted 22 lines
  #endif
include/configs/sunxi-common.h +0 -1
index cce0441..e4db777 100644
@@ -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

author
#3 lore raw

PATCH 1/1 sunxi/spl: Detect at runtime where SPL was read from

Hello,

Quoted 87 lines · Hans de Goede

Hi,

Quoted 15 lines · Roy Spliet

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>:

Quoted 8 lines

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 <[email protected]>

Daniel, thanks for working on this. I've added my comments to Roy's comments.

Quoted 47 lines
Quoted 23 lines
CC Roy Spliet <[email protected]>
Cc Ian Campbell <[email protected]>
Cc Hans De Goede <[email protected]>
---
  arch/arm/cpu/armv7/sunxi/board.c | 53
++++++++++++++++++++++++++--------------
  include/configs/sunxi-common.h   |  2 --
  2 files changed, 35 insertions(+), 20 deletions(-)
arch/arm/cpu/armv7/sunxi/board.c +1 -0
index 70f413f..5e441ba 100644
@@ -11,6 +11,7 @@
   */
 
  #include <common.h>
+#include <mmc.h>
  #include <i2c.h>
  #include <serial.h>
  #ifdef CONFIG_SPL_BUILD

arch/arm/cpu/armv7/sunxi/board.c void s_init(void) · line 114

Quoted 5 lines
arch/arm/cpu/armv7/sunxi/board.c +1 -0
@@ -109,12 +110,10 @@ void s_init(void)
  }
 
  #ifdef CONFIG_SPL_BUILD
+DECLARE_GLOBAL_DATA_PTR;

+

Quoted 18 lines
arch/arm/cpu/armv7/sunxi/board.c +0 -4
  /* 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;
arch/arm/cpu/armv7/sunxi/board.c +0 -4
-#elif defined(CONFIG_SPL_NAND_SUPPORT)
-       /*
-        * This is compile time configuration informing SPL, that it
-        * was loaded from nand flash.
Quoted 8 lines

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 128

Quoted 6 lines
arch/arm/cpu/armv7/sunxi/board.c +2 -2
-        */
-       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.

Quoted 2 lines

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.

Quoted 7 lines
Quoted 3 lines

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

Quoted 79 lines
Quoted 52 lines
Quoted 52 lines

         /*

- * When booting from the SD card, the "eGON.BT0" signature is
expected
- * to be found in memory at the address 0x0004 (see the
"mksunxiboot"

arch/arm/cpu/armv7/sunxi/board.c +2 -1
-        * 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

arch/arm/cpu/armv7/sunxi/board.c +0 -2
@@ -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

arch/arm/cpu/armv7/sunxi/board.c +1 -1
-        * 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,

arch/arm/cpu/armv7/sunxi/board.c +2 -0
+        * 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

arch/arm/cpu/armv7/sunxi/board.c +1 -1
+        * 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;
arch/arm/cpu/armv7/sunxi/board.c +2 -0
+#else
+               mmc_initialize(gd->bd);
Quoted 8 lines
Quoted 5 lines

+
??
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.

Quoted 13 lines

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 162

Quoted 13 lines

?

arch/arm/cpu/armv7/sunxi/board.c +2 -0
+               /*
+                * Figure out where we're booting from. Try mmc0 first,

just
+ * like the brom does. If it doesn't contain valid
signature,

arch/arm/cpu/armv7/sunxi/board.c +5 -0
+                * 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;
Quoted 2 lines

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

Quoted 9 lines

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?

Quoted 34 lines
Quoted 4 lines
Quoted 8 lines

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 170

Quoted 8 lines
arch/arm/cpu/armv7/sunxi/board.c +6 -0
+                       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())
Quoted 9 lines
		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

Quoted 5 lines

Regards,

Hans

Best regards,
Daniel

Quoted 25 lines
Quoted 25 lines
Quoted 24 lines
	
	
  #endif
include/configs/sunxi-common.h +0 -1
index cce0441..e4db777 100644
@@ -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

Signature · 4 lines
Daniel Kochma?ski | Pozna?, Poland
;; aka jackdaniel

"Be the change that you wish to see in the world." - Mahatma Gandhi
#4 lore raw

PATCH 1/1 sunxi/spl: Detect at runtime where SPL was read from

Hi,

Quoted 186 lines · Daniel Kochma?ski

Hello,

Quoted 87 lines · Hans de Goede

Hi,

Quoted 15 lines · Roy Spliet

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>:

Quoted 8 lines

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 <[email protected]>

Daniel, thanks for working on this. I've added my comments to Roy's comments.

Quoted 47 lines
Quoted 23 lines
CC Roy Spliet <[email protected]>
Cc Ian Campbell <[email protected]>
Cc Hans De Goede <[email protected]>
---
   arch/arm/cpu/armv7/sunxi/board.c | 53
++++++++++++++++++++++++++--------------
   include/configs/sunxi-common.h   |  2 --
   2 files changed, 35 insertions(+), 20 deletions(-)
arch/arm/cpu/armv7/sunxi/board.c +1 -0
index 70f413f..5e441ba 100644
@@ -11,6 +11,7 @@
    */
 
   #include <common.h>
+#include <mmc.h>
   #include <i2c.h>
   #include <serial.h>
   #ifdef CONFIG_SPL_BUILD

arch/arm/cpu/armv7/sunxi/board.c void s_init(void) · line 114

Quoted 5 lines
arch/arm/cpu/armv7/sunxi/board.c +1 -0
@@ -109,12 +110,10 @@ void s_init(void)
   }
 
   #ifdef CONFIG_SPL_BUILD
+DECLARE_GLOBAL_DATA_PTR;

+

Quoted 18 lines
arch/arm/cpu/armv7/sunxi/board.c +0 -4
   /* 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;
arch/arm/cpu/armv7/sunxi/board.c +0 -4
-#elif defined(CONFIG_SPL_NAND_SUPPORT)
-       /*
-        * This is compile time configuration informing SPL, that it
-        * was loaded from nand flash.
Quoted 8 lines

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 128

Quoted 6 lines
arch/arm/cpu/armv7/sunxi/board.c +2 -2
-        */
-       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.

Quoted 2 lines

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.

Quoted 7 lines
Quoted 3 lines

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

Quoted 86 lines
Quoted 52 lines
Quoted 52 lines

          /*

- * When booting from the SD card, the "eGON.BT0" signature is
expected
- * to be found in memory at the address 0x0004 (see the
"mksunxiboot"

arch/arm/cpu/armv7/sunxi/board.c +2 -1
-        * 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

arch/arm/cpu/armv7/sunxi/board.c +0 -2
@@ -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

arch/arm/cpu/armv7/sunxi/board.c +1 -1
-        * 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,

arch/arm/cpu/armv7/sunxi/board.c +2 -0
+        * 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

arch/arm/cpu/armv7/sunxi/board.c +1 -1
+        * 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;
arch/arm/cpu/armv7/sunxi/board.c +2 -0
+#else
+               mmc_initialize(gd->bd);
Quoted 8 lines
Quoted 5 lines

+
??
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.

Quoted 13 lines

arch/arm/cpu/armv7/sunxi/board.c u32 spl_boot_device(void) · line 162

Quoted 13 lines

?

arch/arm/cpu/armv7/sunxi/board.c +2 -0
+               /*
+                * Figure out where we're booting from. Try mmc0 first,

just
+ * like the brom does. If it doesn't contain valid
signature,

arch/arm/cpu/armv7/sunxi/board.c +5 -0
+                * 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;
Quoted 2 lines

?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.

Quoted 5 lines
Quoted 2 lines

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.

Quoted 3 lines

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