[U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and "fel" boot target
This patch series builds upon http://lists.denx.de/pipermail/u-boot/2015-September/226515.html http://lists.denx.de/pipermail/u-boot/2015-September/226688.html
v2 combines the previous submissions, and adds some suggested fixes/changes.
The sunxi-tool side of things is discussed here: https://www.mail-archive.com/linux-sunxi@googlegroups.com/msg13071.html
* Hans de Goede already pointed out that it might be preferable to make use of the spl_boot_device() function even from the main (non-SPL) U-Boot binary (e.g. for the "NAND" case). Currently this function is only available with CONFIG_SPL_BUILD set, and implemented in arch/arm/cpu/armv7/sunxi/board.c Would it be safe to enable it for non-SPL builds and use something like "(spl_boot_device == BOOT_DEVICE_BOARD)" for misc_init_r()?
* Side note: the sunxi spl_boot_device() will probably require some future refinement anyway, to account for the 'oddball' A80. That SoC requires the SPL at a different address (0x10000 instead of 0x0).
* The test for FEL boot mode ("eGON.BT0") in spl_boot_device() explicitly uses the "io" function readl() to access the signature. Is there a specific reason for this? Marex pointed out (on IRC) that the SRAM might as well be read directly. For now, I have followed the existing code, and used readl() and readb() to work with the SPL header information.
* The check_signature() function somewhat duplicates very similar code from arch/arm/include/asm/io.h. That particular piece of code depends on the presence of a "__mem_pci" symbol. However, it seems 'generic' enough - so it may be worth to factor out and reuse it?
Regards, B. Nortmann
Changes in v2: - Rename field to fel_script_address, discard fel_data_size - Clearing header fields is no longer needed, as mksunxiboot.c zeroes entire image first - renamed fel_data_addr to fel_script_addr, discarded fel_data_size - make sure that FEL-related environment vars are always cleared first - support minimum and maximum SPL (header) version, more verbose error messages - renamed fel_data_addr to fel_scriptaddr - combined both tests into one as suggested by Hans de Goede
Bernhard Nortmann (3): sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant sunxi: retrieve FEL-provided values to environment variables sunxi: add "fel" boot target
board/sunxi/board.c | 56 ++++++++++++++++++++++++++++++++++++++++++ include/configs/sunxi-common.h | 11 +++++++++ tools/mksunxiboot.c | 23 ++++++++++++++++- 3 files changed, 89 insertions(+), 1 deletion(-)
This patch follows up on a discussion of ways to improve support for the sunxi FEL ("USB boot") mechanism, especially with regard to boot scripts, see: https://groups.google.com/d/msg/linux-sunxi/wBEGUoLNRro/rHGq6nSYCQAJ
The idea is to convert the (currently unused) "pad" bytes in the SPL header into an area where data can be passed to U-Boot. To do this safely, we have to make sure that we're actually using our "sunxi" flavor of the SPL, and not the Allwinner boot0.
The modified mksunxiboot introduces a special signature to the SPL header in place of the "pub_head_size" field. This can be used to reliably distinguish between compatible versions of sunxi SPL and anything else (older variants or Allwinner's boot0).
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de ---
Changes in v2: - Rename field to fel_script_address, discard fel_data_size - Clearing header fields is no longer needed, as mksunxiboot.c zeroes entire image first
tools/mksunxiboot.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c index 676d392..cb0865c 100644 --- a/tools/mksunxiboot.c +++ b/tools/mksunxiboot.c @@ -27,12 +27,29 @@ struct boot_file_head { * by the boot ROM. To be compatible with Allwinner tools we * would need to implement the proper fields here instead of * padding. + * + * Actually we want the ability to recognize our "sunxi" variant + * of the SPL. To do so, let's place a special signature into the + * "pub_head_size" field. We can reasonably expect Allwinner's + * boot0 to always have the upper 16 bits of this set to 0 (after + * all the value shouldn't be larger than the limit imposed by + * SRAM size). + * If the signature is present (at 0x14), then we know it's safe + * to use the remaining 8 bytes (at 0x18) for our own purposes. + * (E.g. sunxi-tools "fel" utility can pass information there.) */ - uint8_t pad[12]; /* align to 32 bytes */ + union { + uint32_t pub_head_size; + uint8_t spl_signature[4]; + }; + uint32_t fel_script_address; + uint32_t reserved; /* padding, align to 32 bytes */ };
#define BOOT0_MAGIC "eGON.BT0" #define STAMP_VALUE 0x5F0A6C39 +#define SPL_SIGNATURE "SPL" /* marks "sunxi" header */ +#define SPL_HEADER_VERSION 1
/* check sum functon from sun4i boot code */ int gen_check_sum(struct boot_file_head *head_p) @@ -133,6 +150,10 @@ int main(int argc, char *argv[]) ALIGN(file_size + sizeof(struct boot_file_head), BLOCK_SIZE); img.header.b_instruction = cpu_to_le32(img.header.b_instruction); img.header.length = cpu_to_le32(img.header.length); + + memcpy(img.header.spl_signature, SPL_SIGNATURE, 3); /* "sunxi" marker */ + img.header.spl_signature[3] = SPL_HEADER_VERSION; + gen_check_sum(&img.header);
count = write(fd_out, &img, le32_to_cpu(img.header.length));
On Mon, 14 Sep 2015 15:15:28 +0200 Bernhard Nortmann bernhard.nortmann@web.de wrote:
This patch follows up on a discussion of ways to improve support for the sunxi FEL ("USB boot") mechanism, especially with regard to boot scripts, see: https://groups.google.com/d/msg/linux-sunxi/wBEGUoLNRro/rHGq6nSYCQAJ
The idea is to convert the (currently unused) "pad" bytes in the SPL header into an area where data can be passed to U-Boot. To do this safely, we have to make sure that we're actually using our "sunxi" flavor of the SPL, and not the Allwinner boot0.
The modified mksunxiboot introduces a special signature to the SPL header in place of the "pub_head_size" field. This can be used to reliably distinguish between compatible versions of sunxi SPL and anything else (older variants or Allwinner's boot0).
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de
Changes in v2:
- Rename field to fel_script_address, discard fel_data_size
- Clearing header fields is no longer needed, as mksunxiboot.c zeroes entire image first
tools/mksunxiboot.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c index 676d392..cb0865c 100644 --- a/tools/mksunxiboot.c +++ b/tools/mksunxiboot.c @@ -27,12 +27,29 @@ struct boot_file_head { * by the boot ROM. To be compatible with Allwinner tools we * would need to implement the proper fields here instead of * padding.
** Actually we want the ability to recognize our "sunxi" variant* of the SPL. To do so, let's place a special signature into the* "pub_head_size" field. We can reasonably expect Allwinner's* boot0 to always have the upper 16 bits of this set to 0 (after* all the value shouldn't be larger than the limit imposed by* SRAM size).* If the signature is present (at 0x14), then we know it's safe* to use the remaining 8 bytes (at 0x18) for our own purposes.* (E.g. sunxi-tools "fel" utility can pass information there.)
- uint8_t pad[12]; /* align to 32 bytes */
- union {
uint32_t pub_head_size;uint8_t spl_signature[4];- };
- uint32_t fel_script_address;
- uint32_t reserved; /* padding, align to 32 bytes */
};
#define BOOT0_MAGIC "eGON.BT0" #define STAMP_VALUE 0x5F0A6C39 +#define SPL_SIGNATURE "SPL" /* marks "sunxi" header */ +#define SPL_HEADER_VERSION 1
/* check sum functon from sun4i boot code */ int gen_check_sum(struct boot_file_head *head_p) @@ -133,6 +150,10 @@ int main(int argc, char *argv[]) ALIGN(file_size + sizeof(struct boot_file_head), BLOCK_SIZE); img.header.b_instruction = cpu_to_le32(img.header.b_instruction); img.header.length = cpu_to_le32(img.header.length);
memcpy(img.header.spl_signature, SPL_SIGNATURE, 3); /* "sunxi" marker */
img.header.spl_signature[3] = SPL_HEADER_VERSION;
gen_check_sum(&img.header);
count = write(fd_out, &img, le32_to_cpu(img.header.length));
Acked-by: Siarhei Siamashka siarhei.siamashka@gmail.com
This patch extends the misc_init_r() function on sunxi boards to test for the presence of a suitable "sunxi" SPL header. If found, and the loader ("fel" utility) provided a non-zero value for the boot.scr address, then the corresponding environment variable fel_scriptaddr gets set.
misc_init_r() also sets (or clears) the "fel_booted" variable depending on the active boot device, using the same logic as spl_boot_device().
The goal is to provide sufficient information (within the U-Boot environment) to make intelligent decisions on how to continue the boot process, allowing specific customizations for the "FEL boot" case.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de ---
Changes in v2: - renamed fel_data_addr to fel_script_addr, discarded fel_data_size - make sure that FEL-related environment vars are always cleared first - support minimum and maximum SPL (header) version, more verbose error messages
board/sunxi/board.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 9c855f6..aa26e57 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -516,6 +516,52 @@ void get_board_serial(struct tag_serialnr *serialnr) } #endif
+#if !defined(CONFIG_SPL_BUILD) +static int check_signature(unsigned long io_addr, const char *signature, + int length) +{ + do { + if (readb(io_addr) != *signature) + return 0; + io_addr++; + signature++; + } while (--length > 0); + return 1; +} + +#define SPL_SIGNATURE "SPL" /* marks "sunxi" header */ +#define SPL_MIN_VERSION 1 +#define SPL_MAX_VERSION 1 + +/* + * Check the SPL header for the "sunxi" variant. If found: parse values + * that might have been passed by the loader ("fel" utility), and update + * the environment accordingly. + */ +static void parse_spl_header(void) +{ + uint8_t spl_header_version; + uint32_t fel_script_addr; + + if (check_signature(0x14, SPL_SIGNATURE, 3)) { + spl_header_version = readb(0x17); + if (spl_header_version < SPL_MIN_VERSION) { + printf("sunxi SPL version mismatch: found 0x%02X < required minimum 0x%02X\n", + spl_header_version, SPL_MIN_VERSION); + return; + } + if (spl_header_version > SPL_MAX_VERSION) { + printf("sunxi SPL version mismatch: found 0x%02X > maximum supported 0x%02X\n", + spl_header_version, SPL_MAX_VERSION); + return; + } + fel_script_addr = readl(0x18); + if (fel_script_addr) + setenv_hex("fel_scriptaddr", fel_script_addr); + } +} +#endif /* !defined(CONFIG_SPL_BUILD) */ + #ifdef CONFIG_MISC_INIT_R int misc_init_r(void) { @@ -524,6 +570,16 @@ int misc_init_r(void) uint8_t mac_addr[6]; int ret;
+#if !defined(CONFIG_SPL_BUILD) + setenv("fel_booted", NULL); + setenv("fel_scriptaddr", NULL); + /* determine if we are running in FEL mode */ + if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) { /* eGON.BT0 */ + setenv("fel_booted", "1"); + parse_spl_header(); + } +#endif + ret = sunxi_get_sid(sid); if (ret == 0 && sid[0] != 0 && sid[3] != 0) { if (!getenv("ethaddr")) {
On Mon, 14 Sep 2015 15:15:29 +0200 Bernhard Nortmann bernhard.nortmann@web.de wrote:
This patch extends the misc_init_r() function on sunxi boards to test for the presence of a suitable "sunxi" SPL header. If found, and the loader ("fel" utility) provided a non-zero value for the boot.scr address, then the corresponding environment variable fel_scriptaddr gets set.
misc_init_r() also sets (or clears) the "fel_booted" variable depending on the active boot device, using the same logic as spl_boot_device().
The goal is to provide sufficient information (within the U-Boot environment) to make intelligent decisions on how to continue the boot process, allowing specific customizations for the "FEL boot" case.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de
Changes in v2:
- renamed fel_data_addr to fel_script_addr, discarded fel_data_size
- make sure that FEL-related environment vars are always cleared first
- support minimum and maximum SPL (header) version, more verbose error messages
board/sunxi/board.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 9c855f6..aa26e57 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -516,6 +516,52 @@ void get_board_serial(struct tag_serialnr *serialnr) } #endif
+#if !defined(CONFIG_SPL_BUILD) +static int check_signature(unsigned long io_addr, const char *signature,
int length)+{
- do {
if (readb(io_addr) != *signature)return 0;io_addr++;signature++;- } while (--length > 0);
It is probably better to just use memcmp() instead of this loop.
Admittedly, I'm responsible for abusing readl() in the old code, because it allowed to fit the FEL mode check into just a single line (the pointer casts would make it exceed the 80 characters limit):
if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) { /* eGON.BT0 */
However now this readb() usage seems to do exactly the opposite and only inflates the code unnecessarily.
- return 1;
+}
+#define SPL_SIGNATURE "SPL" /* marks "sunxi" header */ +#define SPL_MIN_VERSION 1 +#define SPL_MAX_VERSION 1
Can we have a way to share these defines between "board/sunxi/board.c" and "tools/mksunxiboot.c"?
+/*
- Check the SPL header for the "sunxi" variant. If found: parse values
- that might have been passed by the loader ("fel" utility), and update
- the environment accordingly.
- */
+static void parse_spl_header(void) +{
- uint8_t spl_header_version;
- uint32_t fel_script_addr;
- if (check_signature(0x14, SPL_SIGNATURE, 3)) {
spl_header_version = readb(0x17);if (spl_header_version < SPL_MIN_VERSION) {printf("sunxi SPL version mismatch: found 0x%02X < required minimum 0x%02X\n",spl_header_version, SPL_MIN_VERSION);return;}if (spl_header_version > SPL_MAX_VERSION) {printf("sunxi SPL version mismatch: found 0x%02X > maximum supported 0x%02X\n",spl_header_version, SPL_MAX_VERSION);return;}
Yes, having this signature check before extracting the information from the SPL header is a good idea. Because we can have Allwinner's boot0 used together with the main U-Boot binary (for the SoC variants, which do not have SPL support yet).
But here it is probably better to just expect the exact SPL header version match? The SPL and the main U-Boot binary are usually both built together from the same sources and combined into a single u-boot-sunxi-with-spl.bin file.
fel_script_addr = readl(0x18);if (fel_script_addr)setenv_hex("fel_scriptaddr", fel_script_addr);- }
+} +#endif /* !defined(CONFIG_SPL_BUILD) */
#ifdef CONFIG_MISC_INIT_R int misc_init_r(void) { @@ -524,6 +570,16 @@ int misc_init_r(void) uint8_t mac_addr[6]; int ret;
+#if !defined(CONFIG_SPL_BUILD)
- setenv("fel_booted", NULL);
- setenv("fel_scriptaddr", NULL);
- /* determine if we are running in FEL mode */
- if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) { /* eGON.BT0 */
setenv("fel_booted", "1");parse_spl_header();- }
+#endif
- ret = sunxi_get_sid(sid); if (ret == 0 && sid[0] != 0 && sid[3] != 0) { if (!getenv("ethaddr")) {
Hi!
Am 16.09.2015 um 03:00 schrieb Siarhei Siamashka:
On Mon, 14 Sep 2015 15:15:29 +0200 Bernhard Nortmann bernhard.nortmann@web.de wrote:
This patch extends the misc_init_r() function on sunxi boards to test for the presence of a suitable "sunxi" SPL header. If found, and the loader ("fel" utility) provided a non-zero value for the boot.scr address, then the corresponding environment variable fel_scriptaddr gets set.
misc_init_r() also sets (or clears) the "fel_booted" variable depending on the active boot device, using the same logic as spl_boot_device().
The goal is to provide sufficient information (within the U-Boot environment) to make intelligent decisions on how to continue the boot process, allowing specific customizations for the "FEL boot" case.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de
Changes in v2:
renamed fel_data_addr to fel_script_addr, discarded fel_data_size
make sure that FEL-related environment vars are always cleared first
support minimum and maximum SPL (header) version, more verbose error messages
board/sunxi/board.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 9c855f6..aa26e57 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -516,6 +516,52 @@ void get_board_serial(struct tag_serialnr *serialnr) } #endif
+#if !defined(CONFIG_SPL_BUILD) +static int check_signature(unsigned long io_addr, const char *signature,
int length)+{
- do {
if (readb(io_addr) != *signature)return 0;io_addr++;signature++;- } while (--length > 0);
It is probably better to just use memcmp() instead of this loop.
Admittedly, I'm responsible for abusing readl() in the old code, because it allowed to fit the FEL mode check into just a single line (the pointer casts would make it exceed the 80 characters limit):
if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) { /* eGON.BT0 */However now this readb() usage seems to do exactly the opposite and only inflates the code unnecessarily.
That answers my questions from the introductory message. I'm glad we can do away with that function, as it really was just introduced to 'squeeze in' that readb() logic there. I think a nice way of dealing with the signature checks (and their line length) is to introduce appropriate macro wrappers in a reworked asm/arch/spl.h - e.g. have_boot0_magic() and have_sunxi_spl().
- return 1;
+}
+#define SPL_SIGNATURE "SPL" /* marks "sunxi" header */ +#define SPL_MIN_VERSION 1 +#define SPL_MAX_VERSION 1
Can we have a way to share these defines between "board/sunxi/board.c" and "tools/mksunxiboot.c"?
Yes, a sane(r) way of dealing with these definitions and the boot_file_head structure shared by that code is to place them in a common spl.h include file. As that is sunxi platform-specific I've targetted asm/arch/spl.h - that file's content seems superseded / currently unused anyway.
+/*
- Check the SPL header for the "sunxi" variant. If found: parse values
- that might have been passed by the loader ("fel" utility), and update
- the environment accordingly.
- */
+static void parse_spl_header(void) +{
- uint8_t spl_header_version;
- uint32_t fel_script_addr;
- if (check_signature(0x14, SPL_SIGNATURE, 3)) {
spl_header_version = readb(0x17);if (spl_header_version < SPL_MIN_VERSION) {printf("sunxi SPL version mismatch: found 0x%02X < required minimum 0x%02X\n",spl_header_version, SPL_MIN_VERSION);return;}if (spl_header_version > SPL_MAX_VERSION) {printf("sunxi SPL version mismatch: found 0x%02X > maximum supported 0x%02X\n",spl_header_version, SPL_MAX_VERSION);return;}Yes, having this signature check before extracting the information from the SPL header is a good idea. Because we can have Allwinner's boot0 used together with the main U-Boot binary (for the SoC variants, which do not have SPL support yet).
But here it is probably better to just expect the exact SPL header version match? The SPL and the main U-Boot binary are usually both built together from the same sources and combined into a single u-boot-sunxi-with-spl.bin file.
Agreed. I was following the fel.c logic from sunxi-tools, but here it's quite reasonable to expect an exact SPL_HEADER_VERSION originating from the same U-Boot version / build.
fel_script_addr = readl(0x18);if (fel_script_addr)setenv_hex("fel_scriptaddr", fel_script_addr);- }
+} +#endif /* !defined(CONFIG_SPL_BUILD) */
- #ifdef CONFIG_MISC_INIT_R int misc_init_r(void) {
@@ -524,6 +570,16 @@ int misc_init_r(void) uint8_t mac_addr[6]; int ret;
+#if !defined(CONFIG_SPL_BUILD)
- setenv("fel_booted", NULL);
- setenv("fel_scriptaddr", NULL);
- /* determine if we are running in FEL mode */
- if (readl(4) != 0x4E4F4765 || readl(8) != 0x3054422E) { /* eGON.BT0 */
setenv("fel_booted", "1");parse_spl_header();- }
+#endif
- ret = sunxi_get_sid(sid); if (ret == 0 && sid[0] != 0 && sid[3] != 0) { if (!getenv("ethaddr")) {
I think following your suggestions improves quite a bit on the readability of this patch series. I'll thus prepare and submit a v3 soon.
Regards, B. Nortmann
This patch makes use of the previous changes to add a new "fel" boot target for sunxi boards.
When booting via FEL, it's often desirable to work around the absence of other (usable) boot devices - or to be able to override them, deviating from the standard boot sequence. To achieve this, the "fel" boot target gets the highest priority, but won't actually do anything unless certain criteria are met.
The "bootcmd_fel" implementation proposed here first tests if an actual FEL boot takes place (using the "fel_booted" env var), and secondly checks that "fel_scriptaddr" was set (originating from the 'loader', i.e. the sunxi-tools fel utility). If both checks pass, then it will try to execute the boot script (boot.scr) at the given address. In case of an error (e.g. an invalid image), the source command might return "false", causing "distro_bootcmd" to proceed with the next boot target.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de
---
Changes in v2: - renamed fel_data_addr to fel_scriptaddr - combined both tests into one as suggested by Hans de Goede
include/configs/sunxi-common.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 48cc4ed..889146b 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -423,7 +423,18 @@ extern int soft_i2c_gpio_scl; #define BOOT_TARGET_DEVICES_USB(func) #endif
+/* FEL boot support, auto-execute boot.scr if a script address was provided */ +#define BOOTENV_DEV_FEL(devtypeu, devtypel, instance) \ + "bootcmd_fel=" \ + "if test -n ${fel_booted} && test -n ${fel_scriptaddr}; then " \ + "echo '(FEL boot)'; " \ + "source ${fel_scriptaddr}; " \ + "fi\0" +#define BOOTENV_DEV_NAME_FEL(devtypeu, devtypel, instance) \ + "fel " + #define BOOT_TARGET_DEVICES(func) \ + func(FEL, fel, na) \ BOOT_TARGET_DEVICES_MMC(func) \ BOOT_TARGET_DEVICES_SCSI(func) \ BOOT_TARGET_DEVICES_USB(func) \
On Mon, 14 Sep 2015 15:15:30 +0200 Bernhard Nortmann bernhard.nortmann@web.de wrote:
This patch makes use of the previous changes to add a new "fel" boot target for sunxi boards.
When booting via FEL, it's often desirable to work around the absence of other (usable) boot devices - or to be able to override them, deviating from the standard boot sequence. To achieve this, the "fel" boot target gets the highest priority, but won't actually do anything unless certain criteria are met.
The "bootcmd_fel" implementation proposed here first tests if an actual FEL boot takes place (using the "fel_booted" env var), and secondly checks that "fel_scriptaddr" was set (originating from the 'loader', i.e. the sunxi-tools fel utility). If both checks pass, then it will try to execute the boot script (boot.scr) at the given address. In case of an error (e.g. an invalid image), the source command might return "false", causing "distro_bootcmd" to proceed with the next boot target.
Signed-off-by: Bernhard Nortmann bernhard.nortmann@web.de
Changes in v2:
- renamed fel_data_addr to fel_scriptaddr
- combined both tests into one as suggested by Hans de Goede
include/configs/sunxi-common.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 48cc4ed..889146b 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -423,7 +423,18 @@ extern int soft_i2c_gpio_scl; #define BOOT_TARGET_DEVICES_USB(func) #endif
+/* FEL boot support, auto-execute boot.scr if a script address was provided */ +#define BOOTENV_DEV_FEL(devtypeu, devtypel, instance) \
- "bootcmd_fel=" \
"if test -n ${fel_booted} && test -n ${fel_scriptaddr}; then " \"echo '(FEL boot)'; " \"source ${fel_scriptaddr}; " \"fi\0"+#define BOOTENV_DEV_NAME_FEL(devtypeu, devtypel, instance) \
- "fel "
#define BOOT_TARGET_DEVICES(func) \
- func(FEL, fel, na) \ BOOT_TARGET_DEVICES_MMC(func) \ BOOT_TARGET_DEVICES_SCSI(func) \ BOOT_TARGET_DEVICES_USB(func) \
Acked-by: Siarhei Siamashka siarhei.siamashka@gmail.com
Hi,
On 14-09-15 15:15, Bernhard Nortmann wrote:
This patch series builds upon http://lists.denx.de/pipermail/u-boot/2015-September/226515.html http://lists.denx.de/pipermail/u-boot/2015-September/226688.html
v2 combines the previous submissions, and adds some suggested fixes/changes.
The sunxi-tool side of things is discussed here: https://www.mail-archive.com/linux-sunxi@googlegroups.com/msg13071.html
- Hans de Goede already pointed out that it might be preferable to
make use of the spl_boot_device() function even from the main (non-SPL) U-Boot binary (e.g. for the "NAND" case). Currently this function is only available with CONFIG_SPL_BUILD set, and implemented in arch/arm/cpu/armv7/sunxi/board.c Would it be safe to enable it for non-SPL builds and use something like "(spl_boot_device == BOOT_DEVICE_BOARD)" for misc_init_r()?
- Side note: the sunxi spl_boot_device() will probably require
some future refinement anyway, to account for the 'oddball' A80. That SoC requires the SPL at a different address (0x10000 instead of 0x0).
- The test for FEL boot mode ("eGON.BT0") in spl_boot_device()
explicitly uses the "io" function readl() to access the signature. Is there a specific reason for this? Marex pointed out (on IRC) that the SRAM might as well be read directly. For now, I have followed the existing code, and used readl() and readb() to work with the SPL header information.
- The check_signature() function somewhat duplicates very similar
code from arch/arm/include/asm/io.h. That particular piece of code depends on the presence of a "__mem_pci" symbol. However, it seems 'generic' enough - so it may be worth to factor out and reuse it?
Regards, B. Nortmann
Changes in v2:
- Rename field to fel_script_address, discard fel_data_size
- Clearing header fields is no longer needed, as mksunxiboot.c zeroes entire image first
- renamed fel_data_addr to fel_script_addr, discarded fel_data_size
- make sure that FEL-related environment vars are always cleared first
- support minimum and maximum SPL (header) version, more verbose error messages
- renamed fel_data_addr to fel_scriptaddr
- combined both tests into one as suggested by Hans de Goede
Bernhard Nortmann (3): sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant sunxi: retrieve FEL-provided values to environment variables sunxi: add "fel" boot target
Thanks, series looks good to me. Siarhei, can I have your Acked-by for this series (indicating that you are ok with them from having matching changes in sunxi-tools pov) ? Once I've your acked by I'll queue this up for merging.
Should we target this for v2015.10, or for the next cycle ?
The changes look quite safe to me, so I'm ok with putting this on v2015.10, but I wonder what others think.
Regards,
Hans
On Mon, 14 Sep 2015 16:12:25 +0200 Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 14-09-15 15:15, Bernhard Nortmann wrote:
This patch series builds upon http://lists.denx.de/pipermail/u-boot/2015-September/226515.html http://lists.denx.de/pipermail/u-boot/2015-September/226688.html
v2 combines the previous submissions, and adds some suggested fixes/changes.
The sunxi-tool side of things is discussed here: https://www.mail-archive.com/linux-sunxi@googlegroups.com/msg13071.html
- Hans de Goede already pointed out that it might be preferable to
make use of the spl_boot_device() function even from the main (non-SPL) U-Boot binary (e.g. for the "NAND" case). Currently this function is only available with CONFIG_SPL_BUILD set, and implemented in arch/arm/cpu/armv7/sunxi/board.c Would it be safe to enable it for non-SPL builds and use something like "(spl_boot_device == BOOT_DEVICE_BOARD)" for misc_init_r()?
- Side note: the sunxi spl_boot_device() will probably require
some future refinement anyway, to account for the 'oddball' A80. That SoC requires the SPL at a different address (0x10000 instead of 0x0).
- The test for FEL boot mode ("eGON.BT0") in spl_boot_device()
explicitly uses the "io" function readl() to access the signature. Is there a specific reason for this? Marex pointed out (on IRC) that the SRAM might as well be read directly. For now, I have followed the existing code, and used readl() and readb() to work with the SPL header information.
- The check_signature() function somewhat duplicates very similar
code from arch/arm/include/asm/io.h. That particular piece of code depends on the presence of a "__mem_pci" symbol. However, it seems 'generic' enough - so it may be worth to factor out and reuse it?
Regards, B. Nortmann
Changes in v2:
- Rename field to fel_script_address, discard fel_data_size
- Clearing header fields is no longer needed, as mksunxiboot.c zeroes entire image first
- renamed fel_data_addr to fel_script_addr, discarded fel_data_size
- make sure that FEL-related environment vars are always cleared first
- support minimum and maximum SPL (header) version, more verbose error messages
- renamed fel_data_addr to fel_scriptaddr
- combined both tests into one as suggested by Hans de Goede
Bernhard Nortmann (3): sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant sunxi: retrieve FEL-provided values to environment variables sunxi: add "fel" boot target
Thanks, series looks good to me. Siarhei, can I have your Acked-by for this series (indicating that you are ok with them from having matching changes in sunxi-tools pov) ? Once I've your acked by I'll queue this up for merging.
Yes, it also looks good to me after the 'fel_data_size' field has been dropped.
Should we target this for v2015.10, or for the next cycle ?
The changes look quite safe to me, so I'm ok with putting this on v2015.10, but I wonder what others think.
I don't have any strong opinion about this.
participants (3)
-
Bernhard Nortmann -
Hans de Goede -
Siarhei Siamashka