| Message ID | 1465410226-21187-1-git-send-email-bernhard.nortmann@web.de |
|---|---|
| State | Superseded |
| Delegated to: | Hans de Goede |
| Headers | show |
Hi, On 08-06-16 20:23, Bernhard Nortmann wrote: > The patch converts one of the "reserved" fields in the sunxi SPL > header to a fel_uEnv_length entry. When booting over USB ("FEL > mode"), this enables the sunxi-fel utility to pass the string > length of uEnv.txt compatible data; at the same time requesting > that this data be imported into the U-Boot environment. > > If parse_spl_header() in the sunxi board.c encounters a non-zero > value in this header field, it will therefore call himport_r() to > merge the string (lines) passed via FEL into the default settings. > Environment vars can be changed this way even before U-Boot will > attempt to autoboot - specifically, this also allows overriding > "bootcmd". > > With fel_script_addr set and a zero fel_uEnv_length, U-Boot is > safe to assume that data in .scr format (a mkimage-type script) > was passed at fel_script_addr, and will handle it using the > existing mechanism ("bootcmd_fel"). > > Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> This patch looks good to me. Siarhei any comments from your side ? If not then I'll add this to u-boot-sunxi/next. Regards, Hans > > --- > A corresponding proof-of-concept version of sunxi-fel is available > from my https://github.com/n1tehawk/sunxi-tools/tree/20160608_uEnv-magic > branch. I've picked up the suggestion to use a "#=uEnv" magic string > to request that a file be treated as uEnv.txt-style data. > > For example, use your text editor to save a my.env file with > > #=uEnv > myvar=world > bootcmd=echo "Hello $myvar." > > and then test it with > ./sunxi-fel uboot u-boot-sunxi-with-spl.bin write 0x43100000 my.env > > You should see U-Boot's autoboot print the corresponding message > and drop you to the prompt, proving that you have successfully > overwritten the "bootcmd". > > Changes in v2: > - Patch renamed to something more suitable (was "sunxi: Add the > ability to pass (script) filesize in the SPL header") > - The data field is now named fel_uEnv_length, and comes with > a corresponding description in spl.h > - Instead of simply passing file size, fel_uEnv_length is now > associated with uEnv.txt format. The patch modifies U-Boot's > sunxi parse_spl_header() to auto-import such data. > > arch/arm/include/asm/arch-sunxi/spl.h | 9 ++++++++- > board/sunxi/board.c | 30 ++++++++++++++++++++++-------- > 2 files changed, 30 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h > index a0f33b0..a966a88 100644 > --- a/arch/arm/include/asm/arch-sunxi/spl.h > +++ b/arch/arm/include/asm/arch-sunxi/spl.h > @@ -49,7 +49,14 @@ struct boot_file_head { > uint8_t spl_signature[4]; > }; > uint32_t fel_script_address; > - uint32_t reserved1[3]; > + /* > + * If the fel_uEnv_length member below is set to a non-zero value, > + * it specifies the size (byte count) of data at fel_script_address. > + * At the same time this indicates that the data is in uEnv.txt > + * compatible format, ready to be imported via "env import -t". > + */ > + uint32_t fel_uEnv_length; > + uint32_t reserved1[2]; > uint32_t boot_media; /* written here by the boot ROM */ > uint32_t reserved2[5]; /* padding, align to 64 bytes */ > }; > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > index d09cf6d..fc57e60 100644 > --- a/board/sunxi/board.c > +++ b/board/sunxi/board.c > @@ -573,6 +573,7 @@ void get_board_serial(struct tag_serialnr *serialnr) > > #if !defined(CONFIG_SPL_BUILD) > #include <asm/arch/spl.h> > +#include <environment.h> > > /* > * Check the SPL header for the "sunxi" variant. If found: parse values > @@ -582,17 +583,30 @@ void get_board_serial(struct tag_serialnr *serialnr) > static void parse_spl_header(const uint32_t spl_addr) > { > struct boot_file_head *spl = (void *)(ulong)spl_addr; > - if (memcmp(spl->spl_signature, SPL_SIGNATURE, 3) == 0) { > - uint8_t spl_header_version = spl->spl_signature[3]; > - if (spl_header_version == SPL_HEADER_VERSION) { > - if (spl->fel_script_address) > - setenv_hex("fel_scriptaddr", > - spl->fel_script_address); > - return; > - } > + if (memcmp(spl->spl_signature, SPL_SIGNATURE, 3) != 0) > + return; /* signature mismatch, no usable header */ > + > + uint8_t spl_header_version = spl->spl_signature[3]; > + if (spl_header_version != SPL_HEADER_VERSION) { > printf("sunxi SPL version mismatch: expected %u, got %u\n", > SPL_HEADER_VERSION, spl_header_version); > + return; > + } > + if (!spl->fel_script_address) > + return; > + > + if (spl->fel_uEnv_length != 0) { > + /* > + * data is expected in uEnv.txt compatible format, so "env > + * import -t" the string(s) at fel_script_address right away. > + */ > + himport_r(&env_htab, (char *)spl->fel_script_address, > + spl->fel_uEnv_length, '\n', H_NOCLEAR, 0, 0, NULL); > + return; > } > + /* otherwise assume .scr format (mkimage-type script) */ > + setenv_hex("fel_scriptaddr", spl->fel_script_address); > + return; > } > #endif > >
Hi Hans! Am 08.06.2016 um 22:13 schrieb Hans de Goede: > Hi, > [...] > > This patch looks good to me. > > Siarhei any comments from your side ? If not then I'll add this to > u-boot-sunxi/next. > > Regards, > > Hans Thanks for looking into it. One small thing I only noticed after posting the patch: The last "return;" at the end of parse_spl_header() is unneeded and may safely be dropped. Regards, B. Nortmann
Hi, On Wed, 8 Jun 2016 22:13:50 +0200 Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 08-06-16 20:23, Bernhard Nortmann wrote: > > The patch converts one of the "reserved" fields in the sunxi SPL > > header to a fel_uEnv_length entry. When booting over USB ("FEL > > mode"), this enables the sunxi-fel utility to pass the string > > length of uEnv.txt compatible data; at the same time requesting > > that this data be imported into the U-Boot environment. > > > > If parse_spl_header() in the sunxi board.c encounters a non-zero > > value in this header field, it will therefore call himport_r() to > > merge the string (lines) passed via FEL into the default settings. > > Environment vars can be changed this way even before U-Boot will > > attempt to autoboot - specifically, this also allows overriding > > "bootcmd". > > > > With fel_script_addr set and a zero fel_uEnv_length, U-Boot is > > safe to assume that data in .scr format (a mkimage-type script) > > was passed at fel_script_addr, and will handle it using the > > existing mechanism ("bootcmd_fel"). > > > > Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> > > This patch looks good to me. > > Siarhei any comments from your side ? If not then I'll add this to > u-boot-sunxi/next. Yes, it also looks good to me: Acked-by: Siarhei Siamashka <siarhei.siamashka@gmail.com> The only nitpick is about the subject line. It does not mention FEL and may be a bit misleading. Regarding the uEnv.txt support in general, I tried to grep U-Boot sources and found that it's use is very much inconsistent on different platforms. For example, there seems to be some sort of leftover junk in sunxi-common.h: http://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sunxi-common.h;h=b33cfb86f82e0831f5d19b1e473205f65efb5a96;hb=b104b3dc1dd90cdbf67ccf3c51b06e4f1592fe91#l481 Hardcoding the partition number and the file system type is hardly something that the users may reasonably expect. It also might be a good idea to do more or less uniform handling of the uEnv.txt for both FEL boot and normal SD card boot. If Bernhard wants to improve the uEnv.txt support in general, then could this be possibly addressed later (not in this patch)? Still the "doc/README.distro" mentions boot.scr but has no references to uEnv.txt (which seems to imply that the uEnv.txt is a second class citizen). > > > > --- > > A corresponding proof-of-concept version of sunxi-fel is available > > from my https://github.com/n1tehawk/sunxi-tools/tree/20160608_uEnv-magic > > branch. I've picked up the suggestion to use a "#=uEnv" magic string > > to request that a file be treated as uEnv.txt-style data. > > > > For example, use your text editor to save a my.env file with > > > > #=uEnv > > myvar=world > > bootcmd=echo "Hello $myvar." > > > > and then test it with > > ./sunxi-fel uboot u-boot-sunxi-with-spl.bin write 0x43100000 my.env > > > > You should see U-Boot's autoboot print the corresponding message > > and drop you to the prompt, proving that you have successfully > > overwritten the "bootcmd". > > > > Changes in v2: > > - Patch renamed to something more suitable (was "sunxi: Add the > > ability to pass (script) filesize in the SPL header") > > - The data field is now named fel_uEnv_length, and comes with > > a corresponding description in spl.h > > - Instead of simply passing file size, fel_uEnv_length is now > > associated with uEnv.txt format. The patch modifies U-Boot's > > sunxi parse_spl_header() to auto-import such data. > > > > arch/arm/include/asm/arch-sunxi/spl.h | 9 ++++++++- > > board/sunxi/board.c | 30 ++++++++++++++++++++++-------- > > 2 files changed, 30 insertions(+), 9 deletions(-) > > > > diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h > > index a0f33b0..a966a88 100644 > > --- a/arch/arm/include/asm/arch-sunxi/spl.h > > +++ b/arch/arm/include/asm/arch-sunxi/spl.h > > @@ -49,7 +49,14 @@ struct boot_file_head { > > uint8_t spl_signature[4]; > > }; > > uint32_t fel_script_address; > > - uint32_t reserved1[3]; > > + /* > > + * If the fel_uEnv_length member below is set to a non-zero value, > > + * it specifies the size (byte count) of data at fel_script_address. > > + * At the same time this indicates that the data is in uEnv.txt > > + * compatible format, ready to be imported via "env import -t". > > + */ > > + uint32_t fel_uEnv_length; > > + uint32_t reserved1[2]; > > uint32_t boot_media; /* written here by the boot ROM */ > > uint32_t reserved2[5]; /* padding, align to 64 bytes */ > > }; > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > > index d09cf6d..fc57e60 100644 > > --- a/board/sunxi/board.c > > +++ b/board/sunxi/board.c > > @@ -573,6 +573,7 @@ void get_board_serial(struct tag_serialnr *serialnr) > > > > #if !defined(CONFIG_SPL_BUILD) > > #include <asm/arch/spl.h> > > +#include <environment.h> > > > > /* > > * Check the SPL header for the "sunxi" variant. If found: parse values > > @@ -582,17 +583,30 @@ void get_board_serial(struct tag_serialnr *serialnr) > > static void parse_spl_header(const uint32_t spl_addr) > > { > > struct boot_file_head *spl = (void *)(ulong)spl_addr; > > - if (memcmp(spl->spl_signature, SPL_SIGNATURE, 3) == 0) { > > - uint8_t spl_header_version = spl->spl_signature[3]; > > - if (spl_header_version == SPL_HEADER_VERSION) { > > - if (spl->fel_script_address) > > - setenv_hex("fel_scriptaddr", > > - spl->fel_script_address); > > - return; > > - } > > + if (memcmp(spl->spl_signature, SPL_SIGNATURE, 3) != 0) > > + return; /* signature mismatch, no usable header */ > > + > > + uint8_t spl_header_version = spl->spl_signature[3]; > > + if (spl_header_version != SPL_HEADER_VERSION) { > > printf("sunxi SPL version mismatch: expected %u, got %u\n", > > SPL_HEADER_VERSION, spl_header_version); > > + return; > > + } > > + if (!spl->fel_script_address) > > + return; > > + > > + if (spl->fel_uEnv_length != 0) { > > + /* > > + * data is expected in uEnv.txt compatible format, so "env > > + * import -t" the string(s) at fel_script_address right away. > > + */ > > + himport_r(&env_htab, (char *)spl->fel_script_address, > > + spl->fel_uEnv_length, '\n', H_NOCLEAR, 0, 0, NULL); > > + return; > > } > > + /* otherwise assume .scr format (mkimage-type script) */ > > + setenv_hex("fel_scriptaddr", spl->fel_script_address); > > + return; > > } > > #endif
diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h index a0f33b0..a966a88 100644 --- a/arch/arm/include/asm/arch-sunxi/spl.h +++ b/arch/arm/include/asm/arch-sunxi/spl.h @@ -49,7 +49,14 @@ struct boot_file_head { uint8_t spl_signature[4]; }; uint32_t fel_script_address; - uint32_t reserved1[3]; + /* + * If the fel_uEnv_length member below is set to a non-zero value, + * it specifies the size (byte count) of data at fel_script_address. + * At the same time this indicates that the data is in uEnv.txt + * compatible format, ready to be imported via "env import -t". + */ + uint32_t fel_uEnv_length; + uint32_t reserved1[2]; uint32_t boot_media; /* written here by the boot ROM */ uint32_t reserved2[5]; /* padding, align to 64 bytes */ }; diff --git a/board/sunxi/board.c b/board/sunxi/board.c index d09cf6d..fc57e60 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -573,6 +573,7 @@ void get_board_serial(struct tag_serialnr *serialnr) #if !defined(CONFIG_SPL_BUILD) #include <asm/arch/spl.h> +#include <environment.h> /* * Check the SPL header for the "sunxi" variant. If found: parse values @@ -582,17 +583,30 @@ void get_board_serial(struct tag_serialnr *serialnr) static void parse_spl_header(const uint32_t spl_addr) { struct boot_file_head *spl = (void *)(ulong)spl_addr; - if (memcmp(spl->spl_signature, SPL_SIGNATURE, 3) == 0) { - uint8_t spl_header_version = spl->spl_signature[3]; - if (spl_header_version == SPL_HEADER_VERSION) { - if (spl->fel_script_address) - setenv_hex("fel_scriptaddr", - spl->fel_script_address); - return; - } + if (memcmp(spl->spl_signature, SPL_SIGNATURE, 3) != 0) + return; /* signature mismatch, no usable header */ + + uint8_t spl_header_version = spl->spl_signature[3]; + if (spl_header_version != SPL_HEADER_VERSION) { printf("sunxi SPL version mismatch: expected %u, got %u\n", SPL_HEADER_VERSION, spl_header_version); + return; + } + if (!spl->fel_script_address) + return; + + if (spl->fel_uEnv_length != 0) { + /* + * data is expected in uEnv.txt compatible format, so "env + * import -t" the string(s) at fel_script_address right away. + */ + himport_r(&env_htab, (char *)spl->fel_script_address, + spl->fel_uEnv_length, '\n', H_NOCLEAR, 0, 0, NULL); + return; } + /* otherwise assume .scr format (mkimage-type script) */ + setenv_hex("fel_scriptaddr", spl->fel_script_address); + return; } #endif
The patch converts one of the "reserved" fields in the sunxi SPL header to a fel_uEnv_length entry. When booting over USB ("FEL mode"), this enables the sunxi-fel utility to pass the string length of uEnv.txt compatible data; at the same time requesting that this data be imported into the U-Boot environment. If parse_spl_header() in the sunxi board.c encounters a non-zero value in this header field, it will therefore call himport_r() to merge the string (lines) passed via FEL into the default settings. Environment vars can be changed this way even before U-Boot will attempt to autoboot - specifically, this also allows overriding "bootcmd". With fel_script_addr set and a zero fel_uEnv_length, U-Boot is safe to assume that data in .scr format (a mkimage-type script) was passed at fel_script_addr, and will handle it using the existing mechanism ("bootcmd_fel"). Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> --- A corresponding proof-of-concept version of sunxi-fel is available from my https://github.com/n1tehawk/sunxi-tools/tree/20160608_uEnv-magic branch. I've picked up the suggestion to use a "#=uEnv" magic string to request that a file be treated as uEnv.txt-style data. For example, use your text editor to save a my.env file with #=uEnv myvar=world bootcmd=echo "Hello $myvar." and then test it with ./sunxi-fel uboot u-boot-sunxi-with-spl.bin write 0x43100000 my.env You should see U-Boot's autoboot print the corresponding message and drop you to the prompt, proving that you have successfully overwritten the "bootcmd". Changes in v2: - Patch renamed to something more suitable (was "sunxi: Add the ability to pass (script) filesize in the SPL header") - The data field is now named fel_uEnv_length, and comes with a corresponding description in spl.h - Instead of simply passing file size, fel_uEnv_length is now associated with uEnv.txt format. The patch modifies U-Boot's sunxi parse_spl_header() to auto-import such data. arch/arm/include/asm/arch-sunxi/spl.h | 9 ++++++++- board/sunxi/board.c | 30 ++++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 9 deletions(-)