Hi Andre, On 11/11/20 6:52 AM, Andre Przywara wrote: > So far we used the separate mksunxiboot tool for generating a bootable > image for Allwinner SPLs, probably just for historical reasons. > > Use the mkimage framework to generate a so called eGON image the > Allwinner BROM expects. > The new image type is called "sunxi_egon", to differentiate it > from the (still to be implemented) secure boot TOC0 image. > > Signed-off-by: Andre Przywara <andre.przyw...@arm.com> > Reviewed-by: Simon Glass <s...@chromium.org> > Reviewed-by: Jagan Teki <ja...@openedev.com> > Tested-by: Jagan Teki <ja...@amarulasolutions.com> # BPI-M64 > > --- > common/image.c | 1 + > include/image.h | 1 + > tools/Makefile | 1 + > tools/sunxi_egon.c | 133 +++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 136 insertions(+) > create mode 100644 tools/sunxi_egon.c > > diff --git a/common/image.c b/common/image.c > index 451fc689a89..6923dac7c07 100644 > --- a/common/image.c > +++ b/common/image.c > @@ -189,6 +189,7 @@ static const table_entry_t uimage_type[] = { > { IH_TYPE_STM32IMAGE, "stm32image", "STMicroelectronics STM32 > Image" }, > { IH_TYPE_MTKIMAGE, "mtk_image", "MediaTek BootROM loadable > Image" }, > { IH_TYPE_COPRO, "copro", "Coprocessor Image"}, > + { IH_TYPE_SUNXI_EGON, "sunxi_egon", "Allwinner eGON Boot Image" > }, > { -1, "", "", }, > }; > > diff --git a/include/image.h b/include/image.h > index 00bc03bebec..89bfca2155a 100644 > --- a/include/image.h > +++ b/include/image.h > @@ -308,6 +308,7 @@ enum { > IH_TYPE_IMX8MIMAGE, /* Freescale IMX8MBoot Image */ > IH_TYPE_IMX8IMAGE, /* Freescale IMX8Boot Image */ > IH_TYPE_COPRO, /* Coprocessor Image for remoteproc*/ > + IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */ > > IH_TYPE_COUNT, /* Number of image types */ > }; > diff --git a/tools/Makefile b/tools/Makefile > index 51123fd9298..2c9a0c4c4e7 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -104,6 +104,7 @@ dumpimage-mkimage-objs := aisimage.o \ > stm32image.o \ > $(ROCKCHIP_OBS) \ > socfpgaimage.o \ > + sunxi_egon.o \ > lib/crc16.o \ > lib/sha1.o \ > lib/sha256.o \ > diff --git a/tools/sunxi_egon.c b/tools/sunxi_egon.c > new file mode 100644 > index 00000000000..8dcc4c87413 > --- /dev/null > +++ b/tools/sunxi_egon.c > @@ -0,0 +1,133 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * (C) Copyright 2018 Arm Ltd. > + */ > + > +#include "imagetool.h" > +#include <image.h> > + > +#include <asm/arch-sunxi/spl.h> > + > +/* checksum initialiser value */ > +#define STAMP_VALUE 0x5F0A6C39
Using spaces here seems unintentional. > + > +/* > + * NAND requires 8K padding. SD/eMMC gets away with 512 bytes, > + * but let's use the larger padding to cover both. > + */ > +#define PAD_SIZE 8192 > + > +static int egon_check_params(struct image_tool_params *params) > +{ > + /* We just need a binary image file. */ > + return !params->dflag; > +} > + > +static int egon_verify_header(unsigned char *ptr, int image_size, > + struct image_tool_params *params) > +{ > + const struct boot_file_head *header = (void *)ptr; > + uint32_t length; > + > + /* First 4 bytes must be an ARM branch instruction. */ > + if ((le32_to_cpu(header->b_instruction) & 0xff000000) != 0xea000000) > + return EXIT_FAILURE; > + > + if (memcmp(header->magic, BOOT0_MAGIC, 8)) sizeof(header->magic)? (same for the memcpy below) > + return EXIT_FAILURE; > + > + length = le32_to_cpu(header->length); > + /* Must be at least 512 byte aligned. */ > + if (length & 511) > + return EXIT_FAILURE; > + > + /* > + * Image could also contain U-Boot proper, so could be bigger. > + * But it must not be shorter. > + */ > + if (image_size < length) > + return EXIT_FAILURE; > + > + return EXIT_SUCCESS; > +} > + > +static void egon_print_header(const void *buf) > +{ > + const struct boot_file_head *header = buf; > + > + printf("Allwinner eGON header, size: %d bytes\n", > + le32_to_cpu(header->length)); While the data being printed comes from the header, it describes the image / image type as a whole, so "image" seems more appropriate. > + if (!memcmp(header->spl_signature, SPL_SIGNATURE, 3)) > + printf("\tSPL header version %d.%d\n", > + header->spl_signature[3] >> SPL_MINOR_BITS, > + header->spl_signature[3] & ((1U << SPL_MINOR_BITS) - 1)); > + if (header->spl_signature[3] >= SPL_DT_HEADER_VERSION) > + printf("\tDT name: %s\n", > + (char *)buf + le32_to_cpu(header->dt_name_offset)); This needs to be inside the SPL_SIGNATURE check, and the DT name is only valid if dt_name_offset is nonzero. Maybe use a local variable for the version? > +} > + > +static void egon_set_header(void *buf, struct stat *sbuf, int infd, > + struct image_tool_params *params) > +{ > + struct boot_file_head *header = buf; > + uint32_t *buf32 = buf; > + uint32_t checksum = 0, value; > + int i; > + > + /* Generate an ARM branch instruction to jump over the header. */ > + value = 0xea000000 | (sizeof(struct boot_file_head) / 4 - 2); > + header->b_instruction = cpu_to_le32(value); > + > + memcpy(header->magic, BOOT0_MAGIC, 8); > + header->check_sum = cpu_to_le32(STAMP_VALUE); > + header->length = cpu_to_le32(ALIGN(sbuf->st_size, PAD_SIZE)); When this function runs, padding has already been applied, so you can use simply sbuf->st_size (or params->file_size). > + > + memcpy(header->spl_signature, SPL_SIGNATURE, 3); > + > + /* If an image name has been provided, use it as the DT name. */ > + if (params->imagename && params->imagename[0]) { > + header->spl_signature[3] = SPL_DT_HEADER_VERSION; > + > + value = offsetof(struct boot_file_head, string_pool); > + header->dt_name_offset = cpu_to_le32(value); > + > + strncpy((char *)header->string_pool, params->imagename, 52); > + /* Make sure we have a terminating zero byte. */ > + ((char *)header->string_pool)[51] = 0; > + } else > + header->spl_signature[3] = SPL_ENV_HEADER_VERSION; > + > + /* Calculate the checksum. Yes, it's that simple. */ > + for (i = 0; i < sbuf->st_size / 4; i++) > + checksum += le32_to_cpu(buf32[i]); > + header->check_sum = cpu_to_le32(checksum); > +} > + > +static int egon_check_image_type(uint8_t type) > +{ > + return type == IH_TYPE_SUNXI_EGON ? 0 : 1; > +} > + > +static int egon_vrec_header(struct image_tool_params *params, > + struct image_type_params *tparams) > +{ > + tparams->hdr = calloc(sizeof(struct boot_file_head), 1); > + > + /* Return padding to 8K blocks. */ > + return PAD_SIZE - (params->file_size & (PAD_SIZE - 1)); I think "ALIGN(params->file_size, PAD_SIZE) - params->file_size;" is much clearer. And for both patches: Tested-by: Samuel Holland <sam...@sholland.org> Cheers, Samuel > +} > + > +U_BOOT_IMAGE_TYPE( > + sunxi_egon, > + "Allwinner eGON Boot Image support", > + sizeof(struct boot_file_head), > + NULL, > + egon_check_params, > + egon_verify_header, > + egon_print_header, > + egon_set_header, > + NULL, > + egon_check_image_type, > + NULL, > + egon_vrec_header > +); >