* [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and "fel" boot target
@ 2015-09-14 13:15 Bernhard Nortmann
2015-09-14 13:15 ` [U-Boot] [PATCH v2 1/3] sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant Bernhard Nortmann
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Bernhard Nortmann @ 2015-09-14 13:15 UTC (permalink / raw)
To: u-boot
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 at 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(-)
--
2.4.6
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 1/3] sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant
2015-09-14 13:15 [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and "fel" boot target Bernhard Nortmann
@ 2015-09-14 13:15 ` Bernhard Nortmann
2015-09-16 1:03 ` Siarhei Siamashka
2015-09-14 13:15 ` [U-Boot] [PATCH v2 2/3] sunxi: retrieve FEL-provided values to environment variables Bernhard Nortmann
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Bernhard Nortmann @ 2015-09-14 13:15 UTC (permalink / raw)
To: u-boot
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));
--
2.4.6
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 2/3] sunxi: retrieve FEL-provided values to environment variables
2015-09-14 13:15 [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and "fel" boot target Bernhard Nortmann
2015-09-14 13:15 ` [U-Boot] [PATCH v2 1/3] sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant Bernhard Nortmann
@ 2015-09-14 13:15 ` Bernhard Nortmann
2015-09-16 1:00 ` Siarhei Siamashka
2015-09-14 13:15 ` [U-Boot] [PATCH v2 3/3] sunxi: add "fel" boot target Bernhard Nortmann
2015-09-14 14:12 ` [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and " Hans de Goede
3 siblings, 1 reply; 10+ messages in thread
From: Bernhard Nortmann @ 2015-09-14 13:15 UTC (permalink / raw)
To: u-boot
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")) {
--
2.4.6
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 3/3] sunxi: add "fel" boot target
2015-09-14 13:15 [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and "fel" boot target Bernhard Nortmann
2015-09-14 13:15 ` [U-Boot] [PATCH v2 1/3] sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant Bernhard Nortmann
2015-09-14 13:15 ` [U-Boot] [PATCH v2 2/3] sunxi: retrieve FEL-provided values to environment variables Bernhard Nortmann
@ 2015-09-14 13:15 ` Bernhard Nortmann
2015-09-16 1:04 ` Siarhei Siamashka
2015-09-14 14:12 ` [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and " Hans de Goede
3 siblings, 1 reply; 10+ messages in thread
From: Bernhard Nortmann @ 2015-09-14 13:15 UTC (permalink / raw)
To: u-boot
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) \
--
2.4.6
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and "fel" boot target
2015-09-14 13:15 [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and "fel" boot target Bernhard Nortmann
` (2 preceding siblings ...)
2015-09-14 13:15 ` [U-Boot] [PATCH v2 3/3] sunxi: add "fel" boot target Bernhard Nortmann
@ 2015-09-14 14:12 ` Hans de Goede
2015-09-16 1:34 ` Siarhei Siamashka
3 siblings, 1 reply; 10+ messages in thread
From: Hans de Goede @ 2015-09-14 14:12 UTC (permalink / raw)
To: u-boot
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 at 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 2/3] sunxi: retrieve FEL-provided values to environment variables
2015-09-14 13:15 ` [U-Boot] [PATCH v2 2/3] sunxi: retrieve FEL-provided values to environment variables Bernhard Nortmann
@ 2015-09-16 1:00 ` Siarhei Siamashka
2015-09-17 14:05 ` Bernhard Nortmann
0 siblings, 1 reply; 10+ messages in thread
From: Siarhei Siamashka @ 2015-09-16 1:00 UTC (permalink / raw)
To: u-boot
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")) {
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 1/3] sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant
2015-09-14 13:15 ` [U-Boot] [PATCH v2 1/3] sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant Bernhard Nortmann
@ 2015-09-16 1:03 ` Siarhei Siamashka
0 siblings, 0 replies; 10+ messages in thread
From: Siarhei Siamashka @ 2015-09-16 1:03 UTC (permalink / raw)
To: u-boot
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>
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 3/3] sunxi: add "fel" boot target
2015-09-14 13:15 ` [U-Boot] [PATCH v2 3/3] sunxi: add "fel" boot target Bernhard Nortmann
@ 2015-09-16 1:04 ` Siarhei Siamashka
0 siblings, 0 replies; 10+ messages in thread
From: Siarhei Siamashka @ 2015-09-16 1:04 UTC (permalink / raw)
To: u-boot
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>
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and "fel" boot target
2015-09-14 14:12 ` [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and " Hans de Goede
@ 2015-09-16 1:34 ` Siarhei Siamashka
0 siblings, 0 replies; 10+ messages in thread
From: Siarhei Siamashka @ 2015-09-16 1:34 UTC (permalink / raw)
To: u-boot
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 at 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.
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v2 2/3] sunxi: retrieve FEL-provided values to environment variables
2015-09-16 1:00 ` Siarhei Siamashka
@ 2015-09-17 14:05 ` Bernhard Nortmann
0 siblings, 0 replies; 10+ messages in thread
From: Bernhard Nortmann @ 2015-09-17 14:05 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-09-17 14:05 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14 13:15 [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and "fel" boot target Bernhard Nortmann
2015-09-14 13:15 ` [U-Boot] [PATCH v2 1/3] sunxi: (mksunxiboot) signature to indicate "sunxi" SPL variant Bernhard Nortmann
2015-09-16 1:03 ` Siarhei Siamashka
2015-09-14 13:15 ` [U-Boot] [PATCH v2 2/3] sunxi: retrieve FEL-provided values to environment variables Bernhard Nortmann
2015-09-16 1:00 ` Siarhei Siamashka
2015-09-17 14:05 ` Bernhard Nortmann
2015-09-14 13:15 ` [U-Boot] [PATCH v2 3/3] sunxi: add "fel" boot target Bernhard Nortmann
2015-09-16 1:04 ` Siarhei Siamashka
2015-09-14 14:12 ` [U-Boot] [PATCH v2 0/3] sunxi: support FEL-provided environment vars and " Hans de Goede
2015-09-16 1:34 ` Siarhei Siamashka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.