rockchip: mkimage: rewrite padding calculation for SD/MMC and SPI images

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>
Acked-by: Simon Glass <sjg@chromium.org>

tools/rkcommon.c

@@ -200,9 +200,13 @@ void rkcommon_rc4_encode_spl(void *buf, unsigned int offset, unsigned int size)
 	}
 }
 
-void rkcommon_vrec_header(struct image_tool_params *params,
+int rkcommon_vrec_header(struct image_tool_params *params,
-			  struct image_type_params *tparams)
+			 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,17 @@ void rkcommon_vrec_header(struct image_tool_params *params,
 	/* Allocate, clear and install the header */
 	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;
 }

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,
+int rkcommon_vrec_header(struct image_tool_params *params,
-			  struct image_type_params *tparams);
+			 struct image_type_params *tparams,
+			 unsigned int alignment);
 
 #endif

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",
+		printf("Warning: SPL image is too large (size %#x) and will "
-		       size);
+		       "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;
+	/* We don't add any additional padding after the end of the image */
-
+	return rkcommon_vrec_header(params, tparams, 1);
-	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;
 }
 
 /*

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",
+		printf("Warning: SPL image is too large (size %#x) and will "
-		       size);
+		       "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;
+	int padding = rkcommon_vrec_header(params, tparams, 2048);
-
+	/*
-	rkcommon_vrec_header(params, tparams);
+	 * 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
-	pad_size = ROUND(rkcommon_get_spl_size(params), 0x800);
+	 * add up the header_size, file_size and padding ourselves.
-	params->orig_file_size = pad_size;
+	 */
+	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;
+	 * We need to store the original file-size (i.e. before padding), as
-	pad_size *= 2;
+	 * imagetool does not set this during its adjustment of file_size.
-	debug("pad_size %x\n", pad_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;
 }
 
 /*