Re: [U-Boot] [PATCH v2 11/19] rockchip: mkimage: add support for rockchip nand boot image

Hi Pawel,

On 11 August 2017 at 14:56, Paweł Jarosz paweljarosz3691@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 paweljarosz3691@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 paweljarosz3691@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(sjg@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(sjg@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