Re: [PATCH 3/6] rockchip: mkimage: Print boot0 and boot1 parameters

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

Hi Jonas,

On 2/5/25 5:42 PM, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 2025-02-05 17:04, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 1/29/25 11:36 PM, Jonas Karlman wrote:
>>> The v2 image format embeds boot0 and boot1 parameters, the vendor tool
>>> boot_merger may write these parameters based on the rkboot miniall.ini
>>> files.
>>>
>>> E.g. a RK3576 boot image may contain a boot1 parameter that signals
>>> BootROM or vendor blobs to use 1 GHz instead of the regular 24 MHz rate
>>> for the high precision timer.
>>>
>>> Add support for printing boot0 and boot1 parameters, e.g.:
>>>
>>>     > tools/mkimage -l rk3576_idblock_v1.09.107.img
>>>     Rockchip Boot Image (v2)
>>>     Boot1 2: 0x100
>>>     Image 1: 4096 @ 0x1000
>>>     - Load address: 0x3ffc0000
>>>     Image 2: 77824 @ 0x2000
>>>     - Load address: 0x3ff81000
>>>     Image 3: 262144 @ 0x15000
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>>    tools/rkcommon.c | 18 +++++++++++++++++-
>>>    1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/rkcommon.c b/tools/rkcommon.c
>>> index ad239917d2bd..324820717663 100644
>>> --- a/tools/rkcommon.c
>>> +++ b/tools/rkcommon.c
>>> @@ -62,6 +62,8 @@ struct image_entry {
>>>     * @boot_flag:	[3:0] hash type (0:none, 1:sha256, 2:sha512)
>>>     * @images:	images
>>>     * @hash:	hash or signature for header info
>>> + *
>>> + * Other fields are not used by U-Boot
>>>     */
>>>    struct header0_info_v2 {
>>>    	uint32_t magic;
>>> @@ -69,7 +71,9 @@ struct header0_info_v2 {
>>>    	uint16_t size;
>>>    	uint16_t num_images;
>>>    	uint32_t boot_flag;
>>> -	uint8_t reserved1[104];
>>> +	uint8_t reserved1[32];
>>> +	uint32_t boot0_param[10];
>>> +	uint32_t boot1_param[8];
>>>    	struct image_entry images[4];
>>>    	uint8_t reserved2[1064];
>>>    	uint8_t hash[512];
>>> @@ -491,6 +495,18 @@ static void rkcommon_print_header_v2(const struct header0_info_v2 *hdr)
>>>    
>>>    	printf("Rockchip Boot Image (v2)\n");
>>>    
>>> +	for (i = 0; i < ARRAY_SIZE(hdr->boot0_param); i++) {
>>> +		val = le32_to_cpu(hdr->boot0_param[i]);
>>> +		if (val)
>>> +			printf("Boot0 %d: 0x%x\n", i, val);
>>> +	}
>>> +
>>
>> This seems to indicate that there are 10 4B params for boot0, is that
>> correct? If that's the case I would at least add "param" before %d, the
>> output looked odd to me at first glance.
> 
> That should be correct and is what boot_merger can embed based on the
> [BOOT0_PARAM] section and WORD_n (n=0-9) values from MINIALL.ini.
> 
> The only reason I skipped "param" was because I thought it looked
> prettier to align the "BootX %d" and "Image %d" in the output, can
> change to include "param" :-)
> 

Hehe, could have seen:
Boot1:
- param2: 0x100
- param3: 0x500

as well. I am no UX expert so whatever works best for you :)

>>
>> If they aren't guaranteed to be individual 4B params, what about just
>> printing the whole boot0_param in hex format?
> 
> There is only very few WORD_ values in use in linux-6.1-stan-rkr5 rkbin
> MINIALL.ini files. So I opted to only print out the params that have a
> value different from the default 0x0.
> 
>>
>>> +	for (i = 0; i < ARRAY_SIZE(hdr->boot1_param); i++) {
>>> +		val = le32_to_cpu(hdr->boot1_param[i]);
>>> +		if (val)
>>> +			printf("Boot1 %d: 0x%x\n", i, val);
>>> +	}
>>> +
>>
>> Same remark as for boot0 params instead with 8 4B params for boot1.
> 
> Correct, boot_merger embed WORD_n (n=0-7) from the [BOOT1_PARAM] section
> and also only very few are in use.
> 
> My main test to validate this was to add different values to WORD_n
> under BOOT0/BOOT1_PARAM and then compare with the generated idblock.img.
> 

Cool. Do you know if there's some public sources with appropriate 
licensing for boot_merger for RK35xx? There used to be code in the 
U-Boot fork from Rockchip but they stopped updating boot_merger source a 
few years ago last time I checked.

Could be nice to have it in U-Boot too :)

Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Thanks!
Quentin