Re: [PATCH 5/6] rockchip: mkimage: Add support for up to 4 input files

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Jonas,

On 1/29/25 11:36 PM, Jonas Karlman wrote:
> The v2 image format can support up to 4 embedded images that can be
> loaded by the BootROM using the back-to-bootrom method.
> 
> Currently two input files can be passed in using the datafile parameter,
> separated by a colon (":").
> 
> Extend the datafile parameter parsing to support up to 4 input files
> separated by a colon (":") for use with the v2 image format.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   tools/rkcommon.c | 93 +++++++++++++++++++++++-------------------------
>   1 file changed, 44 insertions(+), 49 deletions(-)
> 
> diff --git a/tools/rkcommon.c b/tools/rkcommon.c
> index 542aca931693..4ff48e81a636 100644
> --- a/tools/rkcommon.c
> +++ b/tools/rkcommon.c
> @@ -148,17 +148,15 @@ static struct spl_info spl_infos[] = {
>   /**
>    * struct spl_params - spl params parsed in check_params()
>    *
> - * @init_file:		Init data file path
> - * @init_size:		Aligned size of init data in bytes
> - * @boot_file:		Boot data file path
> - * @boot_size:		Aligned size of boot data in bytes
> + * @file:	image file path
> + * @size:	aligned size of image in bytes

Not really matching reality though. Could make it easier maybe to have 
an intermediary

struct spl_params_image {
     char *file;
     uint32_t size;
};

and then have

struct spl_params {
     struct spl_params_image images[4];
};

?

>    */
>   
>   struct spl_params {
> -	char *init_file;
> -	uint32_t init_size;
> -	char *boot_file;
> -	uint32_t boot_size;
> +	struct {
> +		char *file;
> +		uint32_t size;
> +	} images[4];
>   };
>   
>   static struct spl_params spl_params = { 0 };
> @@ -238,31 +236,32 @@ int rkcommon_check_params(struct image_tool_params *params)
>   	if (!rkcommon_get_spl_info(params->imagename))
>   		goto err_spl_info;
>   
> -	spl_params.init_file = params->datafile;
> +	spl_params.images[0].file = params->datafile;
> +	for (i = 1; i < ARRAY_SIZE(spl_params.images); i++) {
> +		spl_params.images[i].file =
> +				strchr(spl_params.images[i - 1].file, ':');
> +		if (!spl_params.images[i].file)
> +			break;
>   
> -	spl_params.boot_file = strchr(spl_params.init_file, ':');
> -	if (spl_params.boot_file) {
> -		*spl_params.boot_file = '\0';
> -		spl_params.boot_file += 1;
> +		*spl_params.images[i].file = '\0';
> +		spl_params.images[i].file += 1;
>   	}
>   
> -	size = rkcommon_get_aligned_filesize(params, spl_params.init_file);
> -	if (size < 0)
> -		return EXIT_FAILURE;
> -	spl_params.init_size = size;
> +	for (i = 0; i < ARRAY_SIZE(spl_params.images); i++) {
> +		if (!spl_params.images[i].file)
> +			break;
>   
> -	/* Boot file is optional, and only for back-to-bootrom functionality. */
> -	if (spl_params.boot_file) {
> -		size = rkcommon_get_aligned_filesize(params, spl_params.boot_file);
> +		size = rkcommon_get_aligned_filesize(params,
> +						     spl_params.images[i].file);
>   		if (size < 0)
>   			return EXIT_FAILURE;
> -		spl_params.boot_size = size;
> +		spl_params.images[i].size = size;
>   	}
>   

Can't we merge the two for-loops?

The patch diff makes sense to me :)

Cheers,
Quentin