arm: rockchip: Fix binman_init failure on RK3568

For some newer SoCs like RK3568, the Rockchip has not released
any DDR drivers yet so idbloader needs to create manually using
DDR binaries offered by rkbin. This indeed no requirement to
enable TPL in the U-Boot source code.

If we mark TPL disabled and mark BINMAN enabled by default then
there would be an issue of binman_init failure during board
relocation. This is true as binman failed to find the top-level
node like u-boot-tpl here.

Here is the boot issue observed in RK3566 board,

 U-Boot 2023.01-rc4-00057-gac2505d463-dirty (Jan 04 2023 - 23:44:18 +0530)

 Model: Radxa Compute Module 3(CM3) IO Board
 DRAM:  2 GiB
 binman_init failed:-2
 initcall sequence 000000007ffd2008 failed at call 0000000000a18cac (err=-2)
 ### ERROR ### Please RESET the board ###

This might be fixed via binman node in rockchip-u-boot.dtsi however
disable binman for rk3568 for now as we are at the end of the release
cycle.

Fixes: 05713d570762 ("rockchip: generate u-boot-rockchip.bin with binman
for ARM64 boards")
Cc: Quentin Schulz <quentin.schulz@theobroma-systems.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Hi Jagan,

On 1/4/23 20:18, Jagan Teki wrote:
> For some newer SoCs like RK3568, the Rockchip has not released
> any DDR drivers yet so idbloader needs to create manually using
> DDR binaries offered by rkbin. This indeed no requirement to
> enable TPL in the U-Boot source code.
> 
> If we mark TPL disabled and mark BINMAN enabled by default then
> there would be an issue of binman_init failure during board
> relocation. This is true as binman failed to find the top-level
> node like u-boot-tpl here.
> 
> Here is the boot issue observed in RK3566 board,
> 
>   U-Boot 2023.01-rc4-00057-gac2505d463-dirty (Jan 04 2023 - 23:44:18 +0530)
> 
>   Model: Radxa Compute Module 3(CM3) IO Board
>   DRAM:  2 GiB
>   binman_init failed:-2

This function seems to not be called if you do NOT define BINMAN_FDT 
Kconfig option. Is this something you could check? I see that STM32 
boards and some Mediatek ones have this disabled. The benefit would be 
to still have binman build the images like all other rockchip platforms 
but let your RK336x board boot properly.

I'm wondering what this binman_init is needed for? It seems to me that 
the functions defined in lib/binman.c are actually used only in x86 
platforms?

Cheers,
Quentin
  

On 2023/1/5 03:18, Jagan Teki wrote:
> For some newer SoCs like RK3568, the Rockchip has not released
> any DDR drivers yet so idbloader needs to create manually using
> DDR binaries offered by rkbin. This indeed no requirement to
> enable TPL in the U-Boot source code.
>
> If we mark TPL disabled and mark BINMAN enabled by default then
> there would be an issue of binman_init failure during board
> relocation. This is true as binman failed to find the top-level
> node like u-boot-tpl here.
>
> Here is the boot issue observed in RK3566 board,
>
>   U-Boot 2023.01-rc4-00057-gac2505d463-dirty (Jan 04 2023 - 23:44:18 +0530)
>
>   Model: Radxa Compute Module 3(CM3) IO Board
>   DRAM:  2 GiB
>   binman_init failed:-2
>   initcall sequence 000000007ffd2008 failed at call 0000000000a18cac (err=-2)
>   ### ERROR ### Please RESET the board ###
>
> This might be fixed via binman node in rockchip-u-boot.dtsi however
> disable binman for rk3568 for now as we are at the end of the release
> cycle.
>
> Fixes: 05713d570762 ("rockchip: generate u-boot-rockchip.bin with binman
> for ARM64 boards")
> Cc: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>   arch/arm/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index cac4fa09fd..0e7a511c79 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1934,7 +1934,7 @@ config ARCH_STM32MP
>   config ARCH_ROCKCHIP
>   	bool "Support Rockchip SoCs"
>   	select BLK
> -	select BINMAN if SPL_OPTEE || SPL
> +	select BINMAN if SPL_OPTEE || (SPL && !ROCKCHIP_RK3568)

BINMAN should be available only when both TPL and SPL are available.

if no TPL, means there is no DDR binary available, so not able to use 
binman to generate

the image.


Thanks,
- Kever
>   	select DM
>   	select DM_GPIO
>   	select DM_I2C
  

Hi Kever,

On 1/5/23 10:41, Kever Yang wrote:
> 
> On 2023/1/5 03:18, Jagan Teki wrote:
>> For some newer SoCs like RK3568, the Rockchip has not released
>> any DDR drivers yet so idbloader needs to create manually using
>> DDR binaries offered by rkbin. This indeed no requirement to
>> enable TPL in the U-Boot source code.
>>
>> If we mark TPL disabled and mark BINMAN enabled by default then
>> there would be an issue of binman_init failure during board
>> relocation. This is true as binman failed to find the top-level
>> node like u-boot-tpl here.
>>
>> Here is the boot issue observed in RK3566 board,
>>
>>   U-Boot 2023.01-rc4-00057-gac2505d463-dirty (Jan 04 2023 - 23:44:18 
>> +0530)
>>
>>   Model: Radxa Compute Module 3(CM3) IO Board
>>   DRAM:  2 GiB
>>   binman_init failed:-2
>>   initcall sequence 000000007ffd2008 failed at call 0000000000a18cac 
>> (err=-2)
>>   ### ERROR ### Please RESET the board ###
>>
>> This might be fixed via binman node in rockchip-u-boot.dtsi however
>> disable binman for rk3568 for now as we are at the end of the release
>> cycle.
>>
>> Fixes: 05713d570762 ("rockchip: generate u-boot-rockchip.bin with binman
>> for ARM64 boards")
>> Cc: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>> ---
>>   arch/arm/Kconfig | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index cac4fa09fd..0e7a511c79 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1934,7 +1934,7 @@ config ARCH_STM32MP
>>   config ARCH_ROCKCHIP
>>       bool "Support Rockchip SoCs"
>>       select BLK
>> -    select BINMAN if SPL_OPTEE || SPL
>> +    select BINMAN if SPL_OPTEE || (SPL && !ROCKCHIP_RK3568)
> 
> BINMAN should be available only when both TPL and SPL are available.
> 
> if no TPL, means there is no DDR binary available, so not able to use 
> binman to generate
> 
> the image.
> 

Binman can still generate u-boot.itb, u-boot-spl.bin and co. The final 
u-boot-rockchip.bin won't work indeed, until we fix this in another 
ifdef madness for cases where TPL/SPL is an external blob and not 
created by U-Boot.

In next release (not 2023.01) I have high hopes we can have fully 
migrated to binman, which makes me a bit sad we go a step back by 
disabling it for RK356x :/

But since the release is a question of days now, maybe that's fine, I 
don't know. You're the maintainer :)

Cheers,
Quentin
  

Hi Quentin,

On Thu, Jan 5, 2023 at 2:58 PM Quentin Schulz
<quentin.schulz@theobroma-systems.com> wrote:
>
> Hi Jagan,
>
> On 1/4/23 20:18, Jagan Teki wrote:
> > For some newer SoCs like RK3568, the Rockchip has not released
> > any DDR drivers yet so idbloader needs to create manually using
> > DDR binaries offered by rkbin. This indeed no requirement to
> > enable TPL in the U-Boot source code.
> >
> > If we mark TPL disabled and mark BINMAN enabled by default then
> > there would be an issue of binman_init failure during board
> > relocation. This is true as binman failed to find the top-level
> > node like u-boot-tpl here.
> >
> > Here is the boot issue observed in RK3566 board,
> >
> >   U-Boot 2023.01-rc4-00057-gac2505d463-dirty (Jan 04 2023 - 23:44:18 +0530)
> >
> >   Model: Radxa Compute Module 3(CM3) IO Board
> >   DRAM:  2 GiB
> >   binman_init failed:-2
>
> This function seems to not be called if you do NOT define BINMAN_FDT
> Kconfig option. Is this something you could check? I see that STM32
> boards and some Mediatek ones have this disabled. The benefit would be
> to still have binman build the images like all other rockchip platforms
> but let your RK336x board boot properly.

I did check by disabling BINMAN_FDT before sending this patch, looks
like I have tested with the existing build. Now yes, BINMAN_FDT
disabled seems to boot fine.

I think it makes sense to disable BINMAN_FDT on respective defconfig
as we might move binman in the future.

Any comments Kever?

Jagan.