[PATCH] Revert "rockchip: Only call binman when TPL available"

[Jonas Karlman]

Hi Simon and Quentin,

On 2023-02-04 23:23, Simon Glass wrote:
> Hi Quentin,
> 
> On Tue, 31 Jan 2023 at 03:54, Quentin Schulz
> <quentin.schulz at theobroma-systems.com> wrote:
>>
>> Hi Kever,
>>
>> On 1/31/23 03:53, Kever Yang wrote:
>>> Hi
>>>
>>>      I think I do this modify for those soc not have TPL, or else it
>>> will cause the CI build error.
>>>
>>>
>>> TPL/ddr binary is mandatory in all rockchip SoCs now, but the mainline
>>> U-Boot may not
>>>
>>> able to provide the the available TPL, will need ddr init binary from
>>> rockchip rkbin repository
>>>
>>> instead.
>>>
>>> So the policy is clear, the binary output on mainline U-Boot rockchip
>>> platform:
>>>
>>> - TPL available: u-boot-rockchip.bin with TPL, SPL, ATF, U-Boot proper;
>>>
>>> - TPL not available: u-boot.itb with ATF, U-Boot proper.
>>>
>>>                                          idbloader.bin generate by the
>>> TPL(ddr init binary instead) and SPL via mkimage cmd;
>>>
>>
>> If I understand correctly, the TPL might not be generated by U-Boot but
>> externally provided by Rockchip in binary form? For those cases, the
>> defconfig is expected to only request an SPL build I assume. However,
>> idbloader.bin (from rockchip, U-Boot TPL replacement) shall still be
>> passed with U-Boot SPL to mkimage?
>>
>> In that case, I guess we could have a Kconfig option like
>> CONFIG_EXTERNAL_TPL dependent on !TPL and have the user pass a TPL
>> external path environment variable (EXTERNAL_TPL) the same way we do for
>> BL31 or TEE in cmd_binman.
>>
>> Then we should be able to adapt rockchip-u-boot.dtsi to manage this and
>> still build a valid u-boot-rockchip.bin, e.g. replace
>> https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/rockchip-u-boot.dtsi#L23>>> #ifdef CONFIG_TPL
>>                         multiple-data-files;
>>
>>                         u-boot-tpl {
>>                         };
>> #endif
>> with
>> #if defined(CONFIG_TPL) || defined(CONFIG_EXTERNAL_TPL)
>>                         multiple-data-files;
>> # ifdef CONFIG_TPL
>>                         u-boot-tpl {
>>                         };
>> # else
>>                          blob-ext {
>>                              filename = "${EXTERNAL_TPL}"
>>                          };
>> # endif
>> #endif
>>
>> or something more appropriate for binman, like adapting u-boot-tpl to
>> handle external blobs for example.
> 
> That sounds right to me. But I would like a new entry type for this
> (rockchip-tpl ?), as we do with BL31, etc.
> 
> Also, avoid the #idef around multiple-data-files - it is just easier
> to always enable that.

I just sent out a series [1] based on a local patch I was already using,
added a more generic external-tpl entry to binman instead of a rockchip
specific one.

[1] https://patchwork.ozlabs.org/project/uboot/cover/20230205202116.2891673-1-jonas@kwiboo.se/

Regards,
Jonas

> 
> Regards,
> Simon