[PATCH 2/5] rockchip: rk3399-pinebook-pro: Fix SPL max size and SPI flash payload offset

[Jonas Karlman]

Hi Quentin,

On 2023-06-28 15:49, Quentin Schulz wrote:
> Hi Jonas,
> 
> On 6/27/23 21:10, Jonas Karlman wrote:
>> TPL max size is limited to 184 KB, SPL is loaded to 0x0 and TF-A is
>> loaded to 0x40000, this limit SPL max size to 256 KB. With BootRom only
>> reading first 2 KB per 4 KB page of SPI flash, 880 KB may be needed for
>> TPL+SPL in a worst-case scenario. (184 KB + 256 KB) x 2 = 880 KB
>>
>> Use 0xE0000 (896 KB) as the payload offset in SPI flash, this allows
>> for a payload of 3168 KB before env offset start to overlap.
>>
>> Also add CONFIG_ROCKCHIP_SPI_IMAGE=y to build a bootable SPI flash
>> image, u-boot-rockchip-spi.bin.
>>
>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>> ---
>>   arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi | 4 ----
>>   configs/pinebook-pro-rk3399_defconfig        | 4 +++-
>>   2 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
>> index ea7a5a17ae0f..88a77cad8d43 100644
>> --- a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
>> +++ b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
>> @@ -10,10 +10,6 @@
>>   	chosen {
>>   		u-boot,spl-boot-order = "same-as-spl", &sdhci, &spiflash, &sdmmc;
>>   	};
>> -
>> -	config {
>> -		u-boot,spl-payload-offset = <0x60000>; /* @ 384KB */
>> -	};
> 
> Just a nitpick/question (and for the whole series): Is there any 
> specific reason for going back to the Kconfig way 
> (CONFIG_SYS_SPI_U_BOOT_OFFS) instead of just modifying the offset in the 
> -u-boot DTSI?

The main reason is that SYS_SPI_U_BOOT_OFFS is used as payload offset by
binman in rockchip-u-boot.dtsi, for building u-boot-rockchip-spi.bin.
And also to avoid any confusion on what value is used.

rockchip-u-boot.dtsi:
  offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;

> 
> On a different note, that also means that we're effectively breaking 
> current setups by moving the environment some other location. I cannot 
> talk for the maintainer(s) and user(s) but I thought it was worth 
> mentioning.

The environment should not be affected by these changes. All rk3399
boards using ENV_IS_IN_SPI_FLASH all use ENV_OFFSET=0x3F8000. u-boot.itb
or u-boot.img can be up to 3168 KB before it affects environment.

  0x000000: idbloader.img (TPL+SPL)
  0x0E0000: u-boot.itb or u-boot.img (was before at 0x060000)
  0x3F8000: environment, same as before

I am not expecting any issues for users, as long as the updated
instructions are followed when updating u-boot in SPI flash, i.e.
writing u-boot-rockchip-spi.bin at start of SPI flash.

Regards,
Jonas

> 
>>   };
>>   
>>   &edp {
>> diff --git a/configs/pinebook-pro-rk3399_defconfig b/configs/pinebook-pro-rk3399_defconfig
>> index 58a8b91aa60f..1109ebb7387b 100644
>> --- a/configs/pinebook-pro-rk3399_defconfig
>> +++ b/configs/pinebook-pro-rk3399_defconfig
>> @@ -12,6 +12,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>>   CONFIG_DEFAULT_DEVICE_TREE="rk3399-pinebook-pro"
>>   CONFIG_DM_RESET=y
>>   CONFIG_ROCKCHIP_RK3399=y
>> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>>   CONFIG_TARGET_PINEBOOK_PRO_RK3399=y
>>   CONFIG_SPL_STACK=0x400000
>>   CONFIG_DEBUG_UART_BASE=0xFF1A0000
>> @@ -26,7 +27,7 @@ CONFIG_USE_PREBOOT=y
>>   CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-pinebook-pro.dtb"
>>   CONFIG_DISPLAY_BOARDINFO_LATE=y
>>   CONFIG_MISC_INIT_R=y
>> -CONFIG_SPL_MAX_SIZE=0x2e000
>> +CONFIG_SPL_MAX_SIZE=0x40000
> 
> nitpick: We *could* have this in another commit after we move the 
> environment to another location later in the SPI flash.
> 
> Cheers,
> Quentin