Hi Pawel, On 11 August 2017 at 14:56, Paweł Jarosz <paweljarosz3...@gmail.com> wrote: > The Rockchip boot ROM requires a particular file format for booting from NAND: > > * It starts with 512-byte, rc4 encoded header and is aligned to nand page size > > * Then first 2KB of first stage loader (tpl) aligned to nand page size > * n empty pages > > * second 2KB of first stage loader (tpl) aligned to nand page size > * n empty pages > > * ... > > * first 2KB of second stage loader (spl) aligned to nand page size > * n empty pages > > * second 2KB of first stage loader (spl) aligned to nand page size > * n empty pages > > * ... > > Size of spl and tpl must be aligned to 2KB. > > example usage for nand with page size 16384 and one empty page in iteration: > > # mkimage -n rk3066 -T rknand -d > ./u-boot/tpl/u-boot-tpl.bin:./u-boot/spl/u-boot-spl.bin -X 16384,1 out
Please can you document the -X parameter in the mkimage man page? Also please split the support for -X into a separate patch from the one that adds your nand support. You are changing common code. > > Signed-off-by: Paweł Jarosz <paweljarosz3...@gmail.com> > --- > Changes since v1: > - none > > common/image.c | 1 + > include/image.h | 1 + > tools/Makefile | 2 +- > tools/imagetool.h | 1 + > tools/mkimage.c | 8 ++- > tools/rkcommon.c | 10 ++-- > tools/rkcommon.h | 10 +++- > tools/rknand.c | 156 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > tools/rksd.c | 2 +- > tools/rkspi.c | 2 +- > 10 files changed, 183 insertions(+), 10 deletions(-) > create mode 100644 tools/rknand.c > > diff --git a/common/image.c b/common/image.c > index 0f88984..1d677bc 100644 > --- a/common/image.c > +++ b/common/image.c > @@ -167,6 +167,7 @@ static const table_entry_t uimage_type[] = { > { IH_TYPE_FPGA, "fpga", "FPGA Image" }, > { IH_TYPE_TEE, "tee", "Trusted Execution > Environment Image",}, > { IH_TYPE_FIRMWARE_IVT, "firmware_ivt", "Firmware with HABv4 > IVT" }, > + { IH_TYPE_RKNAND, "rknand", "Rockchip NAND Boot Image" > }, > { -1, "", "", }, > }; > > diff --git a/include/image.h b/include/image.h > index c6f1513..7d90f36 100644 > --- a/include/image.h > +++ b/include/image.h > @@ -269,6 +269,7 @@ enum { > IH_TYPE_VYBRIDIMAGE, /* VYBRID .vyb Image */ > IH_TYPE_TEE, /* Trusted Execution Environment OS Image */ > IH_TYPE_FIRMWARE_IVT, /* Firmware Image with HABv4 IVT */ > + IH_TYPE_RKNAND, /* Rockchip NAND Boot Image */ > > IH_TYPE_COUNT, /* Number of image types */ > }; > diff --git a/tools/Makefile b/tools/Makefile > index 0743677..52858bc 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -79,7 +79,7 @@ RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \ > rsa-sign.o rsa-verify.o > rsa-checksum.o \ > rsa-mod-exp.o) > > -ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rksd.o rkspi.o > +ROCKCHIP_OBS = lib/rc4.o rkcommon.o rkimage.o rknand.o rksd.o rkspi.o > > # common objs for dumpimage and mkimage > dumpimage-mkimage-objs := aisimage.o \ > diff --git a/tools/imagetool.h b/tools/imagetool.h > index a8d5054..0b2a707 100644 > --- a/tools/imagetool.h > +++ b/tools/imagetool.h > @@ -77,6 +77,7 @@ struct image_tool_params { > bool quiet; /* Don't output text in normal operation */ > unsigned int external_offset; /* Add padding to external data */ > const char *engine_id; /* Engine to use for signing */ > + char *extraparams; /* Extra parameters for img creation (-X) */ Can you please expand this comment? What are the parameters for? What should they be set to? > }; > > /* > diff --git a/tools/mkimage.c b/tools/mkimage.c > index 28ff35e..ffc91d2 100644 > --- a/tools/mkimage.c > +++ b/tools/mkimage.c > @@ -144,7 +144,7 @@ static void process_args(int argc, char **argv) > int opt; > > while ((opt = getopt(argc, argv, > - > "a:A:b:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT:vVx")) != -1) { > + > "a:A:b:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT:vVxX:")) != -1) { > switch (opt) { > case 'a': > params.addr = strtoull(optarg, &ptr, 16); > @@ -279,6 +279,9 @@ static void process_args(int argc, char **argv) > case 'x': > params.xflag++; > break; > + case 'X': > + params.extraparams = optarg; > + break; > default: > usage("Invalid option"); > } > @@ -416,7 +419,8 @@ int main(int argc, char **argv) > exit (retval); > } > > - if ((params.type != IH_TYPE_MULTI) && (params.type != > IH_TYPE_SCRIPT)) { > + if ((params.type != IH_TYPE_MULTI) && (params.type != IH_TYPE_SCRIPT) > && > + (params.type != IH_TYPE_RKNAND)) { > dfd = open(params.datafile, O_RDONLY | O_BINARY); > if (dfd < 0) { > fprintf(stderr, "%s: Can't open %s: %s\n", > diff --git a/tools/rkcommon.c b/tools/rkcommon.c > index 04e8272..a2f2160 100644 > --- a/tools/rkcommon.c > +++ b/tools/rkcommon.c > @@ -73,6 +73,7 @@ struct spl_info { > > static struct spl_info spl_infos[] = { > { "rk3036", "RK30", 0x1000, false, false }, > + { "rk3066", "RK30", 0x8000, true, false }, > { "rk3188", "RK31", 0x8000 - 0x800, true, false }, > { "rk322x", "RK32", 0x8000 - 0x1000, false, false }, > { "rk3288", "RK32", 0x8000, false, false }, > @@ -167,7 +168,7 @@ bool rkcommon_spl_is_boot0(struct image_tool_params > *params) > return info->spl_boot0; > } > > -static void rkcommon_set_header0(void *buf, uint file_size, > +static void rkcommon_set_header0(void *buf, uint file_size, uint max_size, > struct image_tool_params *params) > { > struct header0_info *hdr = buf; > @@ -194,12 +195,13 @@ static void rkcommon_set_header0(void *buf, uint > file_size, > * see https://lists.denx.de/pipermail/u-boot/2017-May/293267.html > * for a more detailed explanation by Andy Yan > */ > - hdr->init_boot_size = hdr->init_size + RK_MAX_BOOT_SIZE / RK_BLK_SIZE; > + hdr->init_boot_size = hdr->init_size + DIV_ROUND_UP(max_size, > RK_BLK_SIZE); > + hdr->init_boot_size = ROUND(hdr->init_boot_size, 4); > > rc4_encode(buf, RK_BLK_SIZE, rc4_key); > } > > -int rkcommon_set_header(void *buf, uint file_size, > +int rkcommon_set_header(void *buf, uint file_size, uint max_size, > struct image_tool_params *params) > { > struct header1_info *hdr = buf + RK_SPL_HDR_START; > @@ -207,7 +209,7 @@ int rkcommon_set_header(void *buf, uint file_size, > if (file_size > rkcommon_get_spl_size(params)) > return -ENOSPC; > > - rkcommon_set_header0(buf, file_size, params); > + rkcommon_set_header0(buf, file_size, max_size, params); > > /* Set up the SPL name (i.e. copy spl_hdr over) */ > memcpy(&hdr->magic, rkcommon_get_spl_hdr(params), RK_SPL_HDR_SIZE); > diff --git a/tools/rkcommon.h b/tools/rkcommon.h > index 8790f1c..cd357c7 100644 > --- a/tools/rkcommon.h > +++ b/tools/rkcommon.h > @@ -45,6 +45,14 @@ const char *rkcommon_get_spl_hdr(struct image_tool_params > *params); > int rkcommon_get_spl_size(struct image_tool_params *params); > > /** > + * rkcommon_spl_is_boot0() - is magic included in spl > + * > + * Returns true if magic (for example RK30) is included in spl > + */ > + > +bool rkcommon_spl_is_boot0(struct image_tool_params *params); > + > +/** > * rkcommon_set_header() - set up the header for a Rockchip boot image > * > * This sets up a 2KB header which can be interpreted by the Rockchip boot > ROM. > @@ -53,7 +61,7 @@ int rkcommon_get_spl_size(struct image_tool_params *params); > * @file_size: Size of the file we want the boot ROM to load, in bytes > * @return 0 if OK, -ENOSPC if too large > */ > -int rkcommon_set_header(void *buf, uint file_size, > +int rkcommon_set_header(void *buf, uint file_size, uint max_size, > struct image_tool_params *params); > > /** > diff --git a/tools/rknand.c b/tools/rknand.c > new file mode 100644 > index 0000000..690af2d > --- /dev/null > +++ b/tools/rknand.c > @@ -0,0 +1,156 @@ > +/* > + * Copyright (c) 2017 Paweł Jarosz <paweljarosz3...@gmail.com> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include "imagetool.h" > +#include <image.h> > +#include <rc4.h> > +#include "mkimage.h" > +#include "rkcommon.h" > + > +enum { > + RKNAND_SECT_LEN = RK_BLK_SIZE * 4, > +}; > + > +struct rknand_info { > + uint32_t pagesize; > + uint32_t skippages; > + uint32_t tplsize; > + uint32_t splsize; > + uint32_t tplpaddedsize; > + uint32_t splpaddedsize; > + uint32_t itersize; > + uint32_t tplsplsize; > + char *tplfile; > + char *splfile; Please comment these members > +}; > + > +struct rknand_info ninfo; > + > +static uint32_t rknand_get_file_size(char *filename) > +{ Can you make the code common with imagetool_get_filesize()? > + int dfd; > + struct stat sbuf; > + > + dfd = open(filename, O_RDONLY | O_BINARY); > + if (dfd < 0) { > + fprintf(stderr, "Can't open %s: %s\n", filename, > strerror(errno)); > + exit(EXIT_FAILURE); > + } > + > + if (fstat(dfd, &sbuf) < 0) { > + fprintf(stderr, "Can't stat %s: %s\n", filename, > strerror(errno)); > + exit(EXIT_FAILURE); > + } > + > + close(dfd); > + > + return sbuf.st_size; > +} > + > +static void rknand_fill_ninfo(struct image_tool_params *params) > +{ > + sscanf(params->extraparams, "%u,%u", &ninfo.pagesize, > &ninfo.skippages); You should check errors here, and print a useful message on failure. You also need to return an error if invalid. > + > + ninfo.tplfile = params->datafile; > + if ((ninfo.splfile = strchr(params->datafile, ':')) != NULL) { > + *ninfo.splfile = '\0'; > + ninfo.splfile += 1; > + } > + > + ninfo.tplsize = rknand_get_file_size(ninfo.tplfile); > + ninfo.splsize = rknand_get_file_size(ninfo.splfile); > + > + ninfo.tplpaddedsize = ROUND(ninfo.tplsize + > + (rkcommon_spl_is_boot0(params) ? 0 : 4), RKNAND_SECT_LEN); > + > + ninfo.splpaddedsize = ROUND(ninfo.splsize, RKNAND_SECT_LEN); > + > + ninfo.itersize = ninfo.pagesize * (ninfo.skippages + 1); > + ninfo.tplsplsize = ((ninfo.tplpaddedsize + ninfo.splpaddedsize) / > + RKNAND_SECT_LEN) * ninfo.itersize; > +} > + > +static void rknand_set_header(void *buf, struct stat *sbuf, int ifd, > + struct image_tool_params *params) > +{ > + int sector, sploffset, splfd, ret; > + > + ret = rkcommon_set_header(buf, ninfo.tplsize, ninfo.splsize, params); > + if (ret) { > + printf("Warning: TPL image is too large (size %#x) and will " > + "not boot\n", ninfo.tplsize); > + } > + > + if ((splfd = open(ninfo.splfile, O_RDONLY | O_BINARY)) < 0) { > + fprintf (stderr, "%s: Can't open %s: %s\n", > + params->cmdname, ninfo.splfile, strerror(errno)); > + exit (EXIT_FAILURE); > + } > + > + sploffset = RKNAND_SECT_LEN + ninfo.tplpaddedsize; > + if (read(splfd, buf + sploffset, ninfo.splsize) != ninfo.splsize) { > + fprintf (stderr, "%s: Read error on %s: %s\n", > + params->cmdname, ninfo.splfile, strerror(errno)); > + exit (EXIT_FAILURE); > + } > + close(splfd); > + > + if (rkcommon_need_rc4_spl(params)) > + rkcommon_rc4_encode_spl(buf, sploffset, ninfo.splpaddedsize); > + > + /* > + * Spread the image out so we only use the first 2KB of each pagesize > + * region. This is a feature of the NAND format required by the > Rockchip > + * boot ROM. It seems similar to what SPI does. Can we make the code common? > + */ > + for (sector = ninfo.tplsplsize / ninfo.itersize - 1; sector >= 0; > sector--) { > + memmove(buf + sector * ninfo.itersize + ninfo.pagesize, > + buf + (sector + 1) * RKNAND_SECT_LEN, > RKNAND_SECT_LEN); > + > + if (sector < (ninfo.tplsplsize / ninfo.itersize - 1)) > + memset(buf + sector * ninfo.itersize + > ninfo.pagesize + > + RKNAND_SECT_LEN, 0xFF, ninfo.itersize - > + RKNAND_SECT_LEN); > + } > + memset(buf + RKNAND_SECT_LEN, 0xFF, ninfo.pagesize - RKNAND_SECT_LEN); > + memset(buf + ninfo.tplsplsize - ninfo.pagesize + RKNAND_SECT_LEN, > 0xFF, > + ninfo.pagesize - RKNAND_SECT_LEN); > +} > + > +static int rknand_check_image_type(uint8_t type) > +{ > + if (type == IH_TYPE_RKNAND) > + return EXIT_SUCCESS; > + else > + return EXIT_FAILURE; > +} > + > +static int rknand_vrec_header(struct image_tool_params *params, > + struct image_type_params *tparams) > +{ > + rknand_fill_ninfo(params); > + rkcommon_vrec_header(params, tparams, RKNAND_SECT_LEN); > + > + return ninfo.tplsplsize - tparams->header_size - ninfo.tplsize; > +} > + > +/* > + * rknand parameters > + */ > +U_BOOT_IMAGE_TYPE( > + rknand, > + "Rockchip NAND Boot Image support", > + 0, > + NULL, > + rkcommon_check_params, > + rkcommon_verify_header, > + rkcommon_print_header, > + rknand_set_header, > + NULL, > + rknand_check_image_type, > + NULL, > + rknand_vrec_header > +); > diff --git a/tools/rksd.c b/tools/rksd.c > index c56153d..164c1fb 100644 > --- a/tools/rksd.c > +++ b/tools/rksd.c > @@ -26,7 +26,7 @@ static void rksd_set_header(void *buf, struct stat *sbuf, > int ifd, > * header). > */ > size = params->file_size - RK_SPL_HDR_START; > - ret = rkcommon_set_header(buf, size, params); > + ret = rkcommon_set_header(buf, size, RK_MAX_BOOT_SIZE, params); > if (ret) { > /* TODO(s...@chromium.org): This method should return an > error */ > printf("Warning: SPL image is too large (size %#x) and will " > diff --git a/tools/rkspi.c b/tools/rkspi.c > index 4332ce1..5005051 100644 > --- a/tools/rkspi.c > +++ b/tools/rkspi.c > @@ -25,7 +25,7 @@ static void rkspi_set_header(void *buf, struct stat *sbuf, > int ifd, > int ret; > > size = params->orig_file_size; > - ret = rkcommon_set_header(buf, size, params); > + ret = rkcommon_set_header(buf, size, RK_MAX_BOOT_SIZE, params); > debug("size %x\n", size); > if (ret) { > /* TODO(s...@chromium.org): This method should return an > error */ > -- > 2.7.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot