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

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

On 2/5/25 8:00 PM, Jonas Karlman wrote:
> 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@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?
> 

Something like:

"""
	for (i = 0; i < ARRAY_SIZE(spl_params.images); i++) {
		int size;

		if (i == 0)
			spl_params.images[i].file = params->datafile;
		else
			spl_params.images[i].file =
					strchr(spl_params.images[i - 1].file, ':');

		if (!spl_params.images[i].file)
			break;

		*spl_params.images[i].file = '\0';
		spl_params.images[i].file += 1;

		size = rkcommon_get_aligned_filesize(params,
						     spl_params.images[i].file);
		if (size < 0)
			return EXIT_FAILURE;

		spl_params.images[i].size = size;
	}
"""

maybe?

**NOT TESTED**

Cheers,
Quentin