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

[Jonas Karlman]

Hi Quentin,

On 2025-02-05 17:43, Quentin Schulz wrote:
> 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 at 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];
> };
> 
> ?

Sound good, will use in v2.

> 
>>    */
>>   
>>   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?

Possible, suspect I kept it as two loops to avoid having to work on [i]
and [i - 1] too much in same loop. Do you have any suggestion on how to
merge the two for-loops to simplify this?

Regards,
Jonas

> 
> The patch diff makes sense to me :)
> 
> Cheers,
> Quentin