diff mbox series

[v2,3/4] rockchip: rk35xx: Fix boot with a large fdt blob

Message ID 20230321214301.2590326-4-jonas@kwiboo.se
State Superseded
Delegated to: Kever Yang
Headers show
Series rockchip: Fixes for RK3568 and RK3588 | expand

Commit Message

Jonas Karlman March 21, 2023, 9:43 p.m. UTC
The TF-A blobs used to boot RK3568 and RK3588 boards is based on atf
v2.3. Mainline atf v2.3 contains an issue that could lead to a crash
when it fails to parse the fdt blob being passed as the platform param.
An issue that was fixed in atf v2.4.

The vendor TF-A seem to suffer from a similar issue, and this prevents
booting when fdt blob is large enough to trigger this condition.

Fix this by implying SPL_ATF_NO_PLATFORM_PARAM to let u-boot pass a
NULL pointer instead of the fdt blob as the platform param.

This fixes booting Radxa ROCK 3A after recent sync of device tree.

Fixes: 073d911ae64a ("rockchip: rk3568-rock-3a: Sync device tree from linux")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v2:
- New patch

 arch/arm/mach-rockchip/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jagan Teki April 6, 2023, 7:51 p.m. UTC | #1
On Wed, 22 Mar 2023 at 03:14, Jonas Karlman <jonas@kwiboo.se> wrote:
>
> The TF-A blobs used to boot RK3568 and RK3588 boards is based on atf
> v2.3. Mainline atf v2.3 contains an issue that could lead to a crash
> when it fails to parse the fdt blob being passed as the platform param.
> An issue that was fixed in atf v2.4.
>
> The vendor TF-A seem to suffer from a similar issue, and this prevents
> booting when fdt blob is large enough to trigger this condition.
>
> Fix this by implying SPL_ATF_NO_PLATFORM_PARAM to let u-boot pass a
> NULL pointer instead of the fdt blob as the platform param.
>
> This fixes booting Radxa ROCK 3A after recent sync of device tree.
>
> Fixes: 073d911ae64a ("rockchip: rk3568-rock-3a: Sync device tree from linux")
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> v2:
> - New patch
>
>  arch/arm/mach-rockchip/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index e5ac58ae60b5..d7e3784ba113 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -288,6 +288,7 @@ config ROCKCHIP_RK3568
>         select BOARD_LATE_INIT
>         select DM_REGULATOR_FIXED
>         select DM_RESET
> +       imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF
>         imply ROCKCHIP_COMMON_BOARD
>         imply ROCKCHIP_OTP
>         imply MISC_INIT_R
> @@ -309,6 +310,7 @@ config ROCKCHIP_RK3588

How it related to 3588, couldn't reproduce it from my end at least.
I'm using rk3588_bl31_v1.27.elf from rkbin.

Jagan.
Jonas Karlman April 7, 2023, 9:36 a.m. UTC | #2
Hi Jagan,

On 2023-04-06 21:51, Jagan Teki wrote:
> On Wed, 22 Mar 2023 at 03:14, Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> The TF-A blobs used to boot RK3568 and RK3588 boards is based on atf
>> v2.3. Mainline atf v2.3 contains an issue that could lead to a crash
>> when it fails to parse the fdt blob being passed as the platform param.
>> An issue that was fixed in atf v2.4.
>>
>> The vendor TF-A seem to suffer from a similar issue, and this prevents
>> booting when fdt blob is large enough to trigger this condition.
>>
>> Fix this by implying SPL_ATF_NO_PLATFORM_PARAM to let u-boot pass a
>> NULL pointer instead of the fdt blob as the platform param.
>>
>> This fixes booting Radxa ROCK 3A after recent sync of device tree.
>>
>> Fixes: 073d911ae64a ("rockchip: rk3568-rock-3a: Sync device tree from linux")
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> v2:
>> - New patch
>>
>>  arch/arm/mach-rockchip/Kconfig | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
>> index e5ac58ae60b5..d7e3784ba113 100644
>> --- a/arch/arm/mach-rockchip/Kconfig
>> +++ b/arch/arm/mach-rockchip/Kconfig
>> @@ -288,6 +288,7 @@ config ROCKCHIP_RK3568
>>         select BOARD_LATE_INIT
>>         select DM_REGULATOR_FIXED
>>         select DM_RESET
>> +       imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF
>>         imply ROCKCHIP_COMMON_BOARD
>>         imply ROCKCHIP_OTP
>>         imply MISC_INIT_R
>> @@ -309,6 +310,7 @@ config ROCKCHIP_RK3588
> 
> How it related to 3588, couldn't reproduce it from my end at least.
> I'm using rk3588_bl31_v1.27.elf from rkbin.

I have not been able to reproduce this issue on rk3588, only on rk3568.
I added the same imply to rk3588 because we have no insights into what
fixes the TF-A blob provided by rockchip have included. Passing 0 works
and seem like the more safer default option for a closed-source blob.

See commits [1] and [2] for the commits included in TF-A v2.4 related
to the platform param passed from u-boot. The fdt blob loaded into
memory may need to exceed 64KiB to trigger the issue observed on a
ROCK 3 Model A.

[1] https://github.com/ARM-software/arm-trusted-firmware/commit/8109f738ffa79a63735cba29da26e7c2859977b5
[2] https://github.com/ARM-software/arm-trusted-firmware/commit/e7b586987c0a46660aa8402f19d626a5489fe449

Regards,
Jonas

> 
> Jagan.
diff mbox series

Patch

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index e5ac58ae60b5..d7e3784ba113 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -288,6 +288,7 @@  config ROCKCHIP_RK3568
 	select BOARD_LATE_INIT
 	select DM_REGULATOR_FIXED
 	select DM_RESET
+	imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF
 	imply ROCKCHIP_COMMON_BOARD
 	imply ROCKCHIP_OTP
 	imply MISC_INIT_R
@@ -309,6 +310,7 @@  config ROCKCHIP_RK3588
 	select REGMAP
 	select SYSCON
 	select BOARD_LATE_INIT
+	imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF
 	imply ROCKCHIP_COMMON_BOARD
 	imply ROCKCHIP_OTP
 	imply MISC_INIT_R