于 2018年5月17日 GMT+08:00 下午7:05:15, Siarhei Siamashka <siarhei.siamas...@gmail.com> 写到: >On Thu, 17 May 2018 09:16:59 +0100 >Andre Przywara <andre.przyw...@arm.com> wrote: > >> On Allwinner SoCs we use some free bytes at the beginning of the SPL >image >> to store various information. We have a version byte to allow >updates, >> but changing this always requires all tools to be updated as well. > >The tools do not need to be updated together with U-Boot even now. > >U-Boot may freely increment the SPL version number as long as the new >header is a superset of the old one.
But now sunxi-fel will work when SPL ver not recognized. > >> Introduce the concept of semantic versioning [1] to the SPL header: >> The major part of the version number only changes on incompatible >> updates, a minor number bump indicates backward compatibility. >> This patch just documents the major/minor split, adds some comments >> to the header file and uses the versioning information for the >existing >> users. >> >> [1] https://semver.org > >So basically you are implementing the versioning scheme that I proposed >back in 2015: > https://lists.denx.de/pipermail/u-boot/2015-September/228727.html > >Hans de Goede thought that the major/minor versioning was too complex >and unnecessary (if I remember correctly, we had several discussion >threads which concluded in the same way), so we did not implement it >explicitly back then. But a potential non-compatible SPL header upgrade >still could be handled, albeit less gracefully: > > Yes, we can also always change the SPL header signature in the case > of introducing incompatible changes. But the down side is that the > "fel" tool would treat this situation as "unknown bootloader" > instead of "incompatible U-Boot". > >> Signed-off-by: Andre Przywara <andre.przyw...@arm.com> > >In general, improvements in this area are welcome. Just some >comments below. > >> --- >> arch/arm/include/asm/arch-sunxi/spl.h | 19 ++++++++++++++----- >> board/sunxi/board.c | 4 ++-- >> 2 files changed, 16 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm/include/asm/arch-sunxi/spl.h >b/arch/arm/include/asm/arch-sunxi/spl.h >> index 4277d836e5..7cf89c8db2 100644 >> --- a/arch/arm/include/asm/arch-sunxi/spl.h >> +++ b/arch/arm/include/asm/arch-sunxi/spl.h >> @@ -9,7 +9,16 @@ >> >> #define BOOT0_MAGIC "eGON.BT0" >> #define SPL_SIGNATURE "SPL" /* marks "sunxi" SPL header */ >> -#define SPL_HEADER_VERSION 2 >> +#define SPL_MAJOR_BITS 3 >> +#define SPL_MINOR_BITS 5 >> +#define SPL_VERSION(maj, min) >> \ >> + ((((maj) & ((1U << SPL_MAJOR_BITS) - 1)) << SPL_MINOR_BITS) | \ >> + ((min) & ((1U << SPL_MINOR_BITS) - 1))) >> + >> +#define SPL_HEADER_VERSION SPL_VERSION(0, 2) >> + >> +#define SPL_ENV_HEADER_VERSION SPL_VERSION(0, 1) >> +#define SPL_DT_HEADER_VERSION SPL_VERSION(0, 2) >> >> #ifdef CONFIG_SUNXI_HIGH_SRAM >> #define SPL_ADDR 0x10000 >> @@ -49,14 +58,14 @@ struct boot_file_head { >> uint32_t pub_head_size; >> uint8_t spl_signature[4]; >> }; >> - uint32_t fel_script_address; >> + uint32_t fel_script_address; /* since v0.1, set by sunxi-fel */ > >Thanks, it's nice to have these comments about the versions where these >features were introduced. > >> /* >> * 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 fel_uEnv_length; /* since v0.1, set by sunxi-fel */ >> /* >> * Offset of an ASCIIZ string (relative to the SPL header), which >> * contains the default device tree name >(CONFIG_DEFAULT_DEVICE_TREE). >> @@ -64,11 +73,11 @@ struct boot_file_head { >> * by flash programming tools for providing nice informative >messages >> * to the users. >> */ >> - uint32_t dt_name_offset; >> + uint32_t dt_name_offset; /* since v0.2, set by mksunxiboot */ >> uint32_t reserved1; >> uint32_t boot_media; /* written here by the boot ROM */ >> /* A padding area (may be used for storing text strings) */ >> - uint32_t string_pool[13]; >> + uint32_t string_pool[13]; /* since v0.2, filled by mksunxiboot */ > >The 0.2 version of the SPL header does not specify the exact >format of this 'string_pool' padding area. So I think that this >comment is unnecessary. > >In principle, the device tree name string can be stored even >somewhere in the const data section of the SPL binary and >referenced from the SPL header. Not that it makes any sense to >do this, but it is technically possible. > >> /* The header must be a multiple of 32 bytes (for VBAR alignment) >*/ >> }; >> >> diff --git a/board/sunxi/board.c b/board/sunxi/board.c >> index 3d364c6db5..a105d0a5ab 100644 >> --- a/board/sunxi/board.c >> +++ b/board/sunxi/board.c >> @@ -631,9 +631,9 @@ static void parse_spl_header(const uint32_t >spl_addr) >> return; /* signature mismatch, no usable header */ >> >> uint8_t spl_header_version = spl->spl_signature[3]; >> - if (spl_header_version != SPL_HEADER_VERSION) { >> + if (spl_header_version < SPL_ENV_HEADER_VERSION) { > >We have already discussed this particular part of code earlier: > https://lists.denx.de/pipermail/u-boot/2015-September/227999.html > >Essentially, unless we have a real use case for mixing the SPL and the >main U-Boot parts built from different U-Boot versions, I don't see any >purpose for doing anything other than just exact version check here. > >If this particular check fails (the SPL part does not match the main >U-Boot part), then something is already very wrong. > >> printf("sunxi SPL version mismatch: expected %u, got %u\n", >> - SPL_HEADER_VERSION, spl_header_version); >> + SPL_ENV_HEADER_VERSION, spl_header_version); >> return; >> } >> if (!spl->fel_script_address) _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
