In (first) breaking and (then) fixing the rkspi tool, I realised that the calculation of the required padding (for the header-size and the 2K-in-every-4K SPI layout) was not as self-explainatory as it could have been. This change rewrites the code (using new, common functions in rkcommon.c) and adds verbose in-line comments to ensure that we won't fall into the same pit in the future...
Tested on the RK3399 (with has a boot0-style payload) with SD/MMC and SPI.
Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com ---
tools/rkcommon.c | 20 ++++++++++++++++++-- tools/rkcommon.h | 10 ++++++++-- tools/rksd.c | 23 ++++++++++++----------- tools/rkspi.c | 41 +++++++++++++++++++++++++++-------------- 4 files changed, 65 insertions(+), 29 deletions(-)
diff --git a/tools/rkcommon.c b/tools/rkcommon.c index 6cdb749..1311d65 100644 --- a/tools/rkcommon.c +++ b/tools/rkcommon.c @@ -199,9 +199,13 @@ void rkcommon_rc4_encode_spl(void *buf, unsigned int offset, unsigned int size) } }
-void rkcommon_vrec_header(struct image_tool_params *params, - struct image_type_params *tparams) +int rkcommon_vrec_header(struct image_tool_params *params, + struct image_type_params *tparams, + unsigned int alignment) { + unsigned int unpadded_size; + unsigned int padded_size; + /* * The SPL image looks as follows: * @@ -228,4 +232,16 @@ void rkcommon_vrec_header(struct image_tool_params *params, tparams->hdr = malloc(tparams->header_size); memset(tparams->hdr, 0, tparams->header_size); tparams->header_size = tparams->header_size; + + /* + * If someone passed in 0 for the alignment, we'd better handle + * it correctly... + */ + if (!alignment) + alignment = 1; + + unpadded_size = tparams->header_size + params->file_size; + padded_size = ROUND(unpadded_size, alignment); + + return padded_size - unpadded_size; } diff --git a/tools/rkcommon.h b/tools/rkcommon.h index cc161a6..a21321f 100644 --- a/tools/rkcommon.h +++ b/tools/rkcommon.h @@ -83,8 +83,14 @@ void rkcommon_rc4_encode_spl(void *buf, unsigned int offset, unsigned int size); * @params: Pointer to the tool params structure * @tparams: Pointer tot the image type structure (for setting * the header and header_size) + * @alignment: Alignment (a power of two) that the image should be + * padded to (e.g. 512 if we want to align with SD/MMC + * blocksizes or 2048 for the SPI format) + * + * @return bytes of padding required/added (does not include the header_size) */ -void rkcommon_vrec_header(struct image_tool_params *params, - struct image_type_params *tparams); +int rkcommon_vrec_header(struct image_tool_params *params, + struct image_type_params *tparams, + unsigned int alignment);
#endif diff --git a/tools/rksd.c b/tools/rksd.c index ac8a67d..6dafedf 100644 --- a/tools/rksd.c +++ b/tools/rksd.c @@ -29,12 +29,20 @@ static void rksd_set_header(void *buf, struct stat *sbuf, int ifd, unsigned int size; int ret;
+ printf("params->file_size %d\n", params->file_size); + printf("params->orig_file_size %d\n", params->orig_file_size); + + /* + * We need to calculate this using 'RK_SPL_HDR_START' and not using + * 'tparams->header_size', as the additional byte inserted when + * 'is_boot0' is true counts towards the payload. + */ size = params->file_size - RK_SPL_HDR_START; ret = rkcommon_set_header(buf, 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 not boot\n", - size); + printf("Warning: SPL image is too large (size %#x) and will " + "not boot\n", size); } }
@@ -51,18 +59,11 @@ static int rksd_check_image_type(uint8_t type) return EXIT_FAILURE; }
-/* We pad the file out to a fixed size - this method returns that size */ static int rksd_vrec_header(struct image_tool_params *params, struct image_type_params *tparams) { - int pad_size; - - rkcommon_vrec_header(params, tparams); - - pad_size = RK_SPL_HDR_START + rkcommon_get_spl_size(params); - debug("pad_size %x\n", pad_size); - - return pad_size - params->file_size - tparams->header_size; + /* We don't add any additional padding after the end of the image */ + return rkcommon_vrec_header(params, tparams, 1); }
/* diff --git a/tools/rkspi.c b/tools/rkspi.c index 0a15229..87bd1a9 100644 --- a/tools/rkspi.c +++ b/tools/rkspi.c @@ -39,8 +39,8 @@ static void rkspi_set_header(void *buf, struct stat *sbuf, int ifd, debug("size %x\n", size); if (ret) { /* TODO(sjg@chromium.org): This method should return an error */ - printf("Warning: SPL image is too large (size %#x) and will not boot\n", - size); + printf("Warning: SPL image is too large (size %#x) and will " + "not boot\n", size); }
/* @@ -71,23 +71,36 @@ static int rkspi_check_image_type(uint8_t type) return EXIT_FAILURE; }
-/* We pad the file out to a fixed size - this method returns that size */ +/* + * The SPI payload needs to be padded out to make space for odd half-sector + * layout used in flash (i.e. only the first 2K of each 4K sector is used). + */ static int rkspi_vrec_header(struct image_tool_params *params, struct image_type_params *tparams) { - int pad_size; - - rkcommon_vrec_header(params, tparams); - - pad_size = ROUND(rkcommon_get_spl_size(params), 0x800); - params->orig_file_size = pad_size; + int padding = rkcommon_vrec_header(params, tparams, 2048); + /* + * The file size has not been adjusted at this point (our caller will + * eventually add the header/padding to the file_size), so we need to + * add up the header_size, file_size and padding ourselves. + */ + int padded_size = tparams->header_size + params->file_size + padding;
- /* We will double the image size due to the SPI format */ - pad_size += RK_SPL_HDR_START; - pad_size *= 2; - debug("pad_size %x\n", pad_size); + /* + * We need to store the original file-size (i.e. before padding), as + * imagetool does not set this during its adjustment of file_size. + */ + params->orig_file_size = padded_size;
- return pad_size - params->file_size - tparams->header_size; + /* + * Converting to the SPI format (i.e. splitting each 4K page into two + * 2K subpages and then padding these 2K pages up to take a complete + * 4K sector again) will will double the image size. + * + * Thus we return the padded_size as an additional padding requirement + * (be sure to add this to the padding returned from the common code). + */ + return padded_size + padding; }
/*