[PATCH v3 4/5] rockchip: mkimage: Update init size limit

[Jonas Karlman]

Hi Johan,

On 2023-02-18 12:48, Johan Jonker wrote:
> 
> 
> On 2/18/23 05:43, Johan Jonker wrote:
>> Hi Jonas,
>>
>> On 2/17/23 21:52, Jonas Karlman wrote:
>>> Sync init size limit from vendor u-boot:
>>
>> This sync might not be correct.
>> Please recheck with each SoC or limit your change to the rk3328 SoC if prove fails.
>> Could Kever disclose SoC details here?
>>
>> Johan 
>>
>>>
>>>  px30: 12KiB (+2KiB)
>>
>>>  rk3066: 32KiB (+2KiB)
>>
>>
>> On the rk3066 the limitation depends on the bootrom logic and the tpl location it is loaded in memory:
>>
>> //SPL
>> 	flash_boot_size = idb_buf[0].flash_boot_size;
>> 	size = flash_boot_size - 5;
>> 	if ( size >= 32763 )
>> 		flash_boot_size = 10;
>> //TPL
>> 	flash_data_size = idb_buf[0].flash_data_size;
>> 	if (flash_data_size - 4 >= 61 ||
>> 	    flash_boot_size < flash_data_size ||
>> 	    flash_data_size & 3) {
>> 		flash_data_size = 4;
>> 	}
>>
>> 	offset = idb_buf[0].boot_code1_offset + start;
>>
>> ===
>>
>> CONFIG_TPL_TEXT_BASE=0x10080C00
>> TPL/SPL truncated to 2048 = 4 sectors of 512bytes per NAND page.
>> Header size = 4 x 512bytes
>>
>> limit1: flash_data_size - 4 >= 61
>> limit2: flash_boot_size < flash_data_size
>>

Interesting details, not sure from where this is referenced, is this
from the bootrom code?

If my understanding is correct these refer to the same thing:

usBootDataSize = flash_data_size = init_size
usBootCodeSize = flash_boot_size = init_boot_size

With 32KiB limit these would then in extreme case be:

flash_data_size = 4 + 64 = 68 (full use of 32KiB)
flash_boot_size = 68 + 1024 = 1092 (RK_MAX_BOOT_SIZE)

and with a 30KiB limit:

flash_data_size = 4 + 60 = 64 (full use of 30KiB)
flash_boot_size = 68 + 1024 = 1088 (RK_MAX_BOOT_SIZE)

With these limitations I fully understand why the value for rk3066
should not be changed, thanks.

>> ===
>>
>> 	usFlashDataSec = (ALIGN(dwLoaderDataSize, 2048)) / SECTOR_SIZE;
>> 	usFlashBootSec = (ALIGN(dwLoaderSize, 2048)) / SECTOR_SIZE;
>>
>> 	dwSectorNum = 4 + usFlashDataSec + usFlashBootSec;
>>
>> 	pSec0->usBootDataSize = usFlashDataSec;
>> 	pSec0->usBootCodeSize = usFlashDataSec + usFlashBootSec;
>>
>>>  rk3328: 30KiB (+2KiB)
>>>  rk3568: 60KiB (-16KiB)
>>>
>>> This makes it possible to use latest vendor TPL with RK3328 without
>>> getting a size limit error running the mkimage command.
>>>
>>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>>> ---
>>> v3:
>>> - Sync with vendor u-boot as-is
>>> - Update commit message to include size changes
>>>
>>> v2:
>>> - New patch
>>>
>>>  tools/rkcommon.c | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/tools/rkcommon.c b/tools/rkcommon.c
>>> index 1f1eaa16752b..630e54b1a54d 100644
>>> --- a/tools/rkcommon.c
>>> +++ b/tools/rkcommon.c
>>> @@ -121,20 +121,20 @@ struct spl_info {
>>>  };
>>>  
>>>  static struct spl_info spl_infos[] = {
>>> -	{ "px30", "RK33", 0x2800, false, RK_HEADER_V1 },
>>> +	{ "px30", "RK33", 0x4000 - 0x1000, false, RK_HEADER_V1 },
>>>  	{ "rk3036", "RK30", 0x1000, false, RK_HEADER_V1 },
>>
> 
>>> -	{ "rk3066", "RK30", 0x8000 - 0x800, true, RK_HEADER_V1 },
>>
>> This is OK.
>>
>>> -	{ "rk3128", "RK31", 0x1800, false, RK_HEADER_V1 },
>>
> 
>>> +	{ "rk3066", "RK30", 0x8000, true, RK_HEADER_V1 },
>>
>> This wrong.
> 
> This 0x8000 value was introduced in the manufacturer kernel with this patch.
> rockchip: mkimage: add support for rockchip nand boot image
> https://github.com/rockchip-linux/u-boot/commit/6f14746b0c5977b93f126c43b2a80198758399b9>> 
> However mainline u-boot for rk3066 makes use of BROM.
> rockchip: rk3188: use boot0 hook to load up SPL in 2 steps
> https://source.denx.de/u-boot/u-boot/-/commit/4d9253fb76f59c6f474ca54fe2d45c5706cd86e3>> 
> It follows the same size rules as for rk3188.
> /* spl size 32kb sram - 2kb bootrom */

>From what I could find in datasheet and TRM, the rk3066 have 64KiB sram
and the rk3188 have 32KiB, but I have learned you can not always trust
the datasheet and TRM :-)

> 
> Unless Philipp Tomsich or someone else explains that it should be something different, please keep it as it is.

I fully agree, I will keep the value for rk3066 as it is.

The limit for rk3328 and rk3568 are the only ones I can confirm fixes
existing issues.

rk3328: vendor tpl size is exceeding the current limit of 28KiB
rk3568: only has 64KiB sram, current limit of 76 KiB do not fit

Will limit the change to only include rk3328 and rk3568.

Regards,
Jonas

> 
> Johan
> 
> 
>>
>> printf "%d\n" $(((0x8000 - 0x800 ) / 512))
>> 60 sectors of size 512
>>
>>
>>> +	{ "rk3128", "RK31", 0x2000 - 0x800, false, RK_HEADER_V1 },
>>>  	{ "rk3188", "RK31", 0x8000 - 0x800, true, RK_HEADER_V1 },
>>>  	{ "rk322x", "RK32", 0x8000 - 0x1000, false, RK_HEADER_V1 },
>>>  	{ "rk3288", "RK32", 0x8000, false, RK_HEADER_V1 },
>>>  	{ "rk3308", "RK33", 0x40000 - 0x1000, false, RK_HEADER_V1 },
>>> -	{ "rk3328", "RK32", 0x8000 - 0x1000, false, RK_HEADER_V1 },
>>> +	{ "rk3328", "RK32", 0x8000 - 0x800, false, RK_HEADER_V1 },
>>>  	{ "rk3368", "RK33", 0x8000 - 0x1000, false, RK_HEADER_V1 },
>>>  	{ "rk3399", "RK33", 0x30000 - 0x2000, false, RK_HEADER_V1 },
>>>  	{ "rv1108", "RK11", 0x1800, false, RK_HEADER_V1 },
>>>  	{ "rv1126", "110B", 0x10000 - 0x1000, false, RK_HEADER_V1 },
>>> -	{ "rk3568", "RK35", 0x14000 - 0x1000, false, RK_HEADER_V2 },
>>> +	{ "rk3568", "RK35", 0x10000 - 0x1000, false, RK_HEADER_V2 },
>>>  };
>>>  
>>>  /**