From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 51673C61DA4 for ; Tue, 14 Feb 2023 03:43:12 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 763E985805; Tue, 14 Feb 2023 04:43:09 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=rock-chips.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id EAD9A8580E; Tue, 14 Feb 2023 04:43:07 +0100 (CET) Received: from mail-m11874.qiye.163.com (mail-m11874.qiye.163.com [115.236.118.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 5B151857E0 for ; Tue, 14 Feb 2023 04:43:04 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=rock-chips.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=kever.yang@rock-chips.com Received: from [172.16.12.93] (unknown [58.22.7.114]) by mail-m11874.qiye.163.com (Hmail) with ESMTPA id EF9E93C01A1; Tue, 14 Feb 2023 11:42:49 +0800 (CST) Message-ID: <8c20f078-0ede-7e58-fa33-857640d4eec5@rock-chips.com> Date: Tue, 14 Feb 2023 11:42:49 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH 2/3] rockchip: Require an external TPL binary when TPL is missing Content-Language: en-US To: Quentin Schulz , Jonas Karlman , Simon Glass , Philipp Tomsich , Joseph Chen , Alper Nebi Yasak Cc: Jagan Teki , Heinrich Schuchardt , u-boot@lists.denx.de References: <20230205202116.2891673-1-jonas@kwiboo.se> <20230205202116.2891673-3-jonas@kwiboo.se> <7dfdb217-71fe-a89e-0268-f9efb8941de8@theobroma-systems.com> <61fcf963-730a-82f6-9467-29337108540e@rock-chips.com> From: Kever Yang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-HM-Spam-Status: e1kfGhgUHx5ZQUpXWQgPGg8OCBgUHx5ZQUlOS1dZFg8aDwILHllBWSg2Ly tZV1koWUFDSUNOT01LS0k3V1ktWUFJV1kPCRoVCBIfWUFZGh8ZQ1YaSh9CGhlJHxhLTEtVEwETFh oSFyQUDg9ZV1kYEgtZQVlOQ1VJSVVMVUpKT1lXWRYaDxIVHRRZQVlPS0hVSkpLSEpMVUpLS1VLWQ Y+ X-HM-Sender-Digest: e1kMHhlZQR0aFwgeV1kSHx4VD1lBWUc6PDI6Czo5Lz0TUR8TGRM3FE8T TkkaChFVSlVKTUxNSE9NSkxLTEtMVTMWGhIXVRAeDR4JVQIaFRw7CRQYEFYYExILCFUYFBZFWVdZ EgtZQVlOQ1VJSVVMVUpKT1lXWQgBWUFCSE1CNwY+ X-HM-Tid: 0a864e049b5b2eb0kusnef9e93c01a1 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On 2023/2/9 00:06, Quentin Schulz wrote: > Hi Kever, > > On 2/8/23 16:41, Kever Yang wrote: >> Hi Quentin, >> >> On 2023/2/6 19:26, Quentin Schulz wrote: >>> Hi Jonas, >>> >>> On 2/5/23 21:21, Jonas Karlman wrote: >>>> Rockchip SoCs typically use U-Boot TPL to initialize DRAM, then jumps >>>> back to boot-rom to load the next stage of the boot flow, U-Boot SPL. >>>> >>>> For RK356x there is currently no support to initialize DRAM using >>>> U-Boot >>>> TPL and instead an external TPL binary must be used to generate a >>>> working u-boot-rockchip.bin image. >>>> >>>> Use the new external-tpl entry unless CONFIG_TPL=y to indicate that an >>>> external TPL binary must be provided to generate a working firmware. >>>> >>>> Signed-off-by: Jonas Karlman >>>> --- >>>>   Makefile                          |  1 + >>>>   arch/arm/dts/rockchip-u-boot.dtsi | 16 ++++++++++++---- >>>>   tools/binman/missing-blob-help    |  5 +++++ >>>>   3 files changed, 18 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/Makefile b/Makefile >>>> index 7eaf45496c1c..7e9272be937f 100644 >>>> --- a/Makefile >>>> +++ b/Makefile >>>> @@ -1332,6 +1332,7 @@ cmd_binman = $(srctree)/tools/binman/binman >>>> $(if $(BINMAN_DEBUG),-D) \ >>>>           -a opensbi-path=${OPENSBI} \ >>>>           -a default-dt=$(default_dt) \ >>>>           -a scp-path=$(SCP) \ >>>> +        -a external-tpl-path=$(EXTERNAL_TPL) \ >>>>           -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \ >>>>           -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \ >>>>           -a spl-dtb=$(CONFIG_SPL_OF_REAL) \ >>>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi >>>> b/arch/arm/dts/rockchip-u-boot.dtsi >>>> index 6c662a72d4f9..bc3bc9bc3e37 100644 >>>> --- a/arch/arm/dts/rockchip-u-boot.dtsi >>>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi >>>> @@ -20,12 +20,16 @@ >>>>           mkimage { >>>>               filename = "idbloader.img"; >>>>               args = "-n", CONFIG_SYS_SOC, "-T", "rksd"; >>>> -#ifdef CONFIG_TPL >>>>               multiple-data-files; >>>>   +#ifdef CONFIG_TPL >>>>               u-boot-tpl { >>>> -            }; >>>> +#else >>>> +            external-tpl { >>>> +                filename = "ddr.bin"; >>>> +                missing-msg = "external-tpl-rockchip"; >>>>   #endif >>>> +            }; >>> >>> NACK. This forces the use of a TPL (either built by U-Boot or >>> external) which is not always the case. There are still boards >>> without a TPL which work perfectly fine (at least I would hope so :) ). >>> >>> Basically there are three possible cases: >>> SPL >>> TPL+SPL >>> TPL(external blob)+SPL >> >> I'm afraid all the boards support by mainline U-Boot is using >> "TPL(U-Boot or external)+SPL+U-Boot" mode, and this mode also always >> used by rockchip vendor branch. >> > > That seems to be incorrect. > > for conf in $(git grep -l ARCH_ROCKCHIP configs); do >     make $(basename $conf) > /dev/null 2>&1 >     if ! grep -q CONFIG_TPL=y .config; then >         echo $conf does not enable TPL; >     fi > done > > returns: > configs/chromebit_mickey_defconfig does not enable TPL > configs/chromebook_bob_defconfig does not enable TPL > configs/chromebook_jerry_defconfig does not enable TPL > configs/chromebook_kevin_defconfig does not enable TPL > configs/chromebook_minnie_defconfig does not enable TPL > configs/chromebook_speedy_defconfig does not enable TPL > configs/elgin-rv1108_defconfig does not enable TPL > configs/evb-rk3036_defconfig does not enable TPL > configs/evb-rk3128_defconfig does not enable TPL > configs/evb-rk3308_defconfig does not enable TPL > configs/evb-rk3568_defconfig does not enable TPL > configs/evb-rv1108_defconfig does not enable TPL > configs/ficus-rk3399_defconfig does not enable TPL > configs/geekbox_defconfig does not enable TPL > configs/kylin-rk3036_defconfig does not enable TPL > configs/miqi-rk3288_defconfig does not enable TPL > configs/phycore-rk3288_defconfig does not enable TPL > configs/popmetal-rk3288_defconfig does not enable TPL > configs/roc-cc-rk3308_defconfig does not enable TPL > configs/rock2_defconfig does not enable TPL > configs/rock_defconfig does not enable TPL > configs/sheep-rk3368_defconfig does not enable TPL > > is there something I'm missing? Most of them use simple SPL+U-Boot(for 32bit processer) or have external TPL. We can keep an option to support SPL only case for now. > >> For "SPL+U-Boot" use case, it only happen many years ago in some of >> rk3288 board and rk3399 board, >> >> the SPL will need to support both sdram driver and storage(SPI, >> SDCard, eMMC) driver and without back to BootRom, >> >> and other function may add to SPL, but the sram size limit is always >> there, so all the rk3288 and rk3399 board have migrate to use >> "TPL+SPL+U-Boot.itb" mode. >> > > It seems not all have migrated. > >> The "TPL+SPL+U-Boot.itb" is much clear and easy to maintine for all >> the boards and will be the only accept mode for new board support in >> the future on rockchip platform: >> >> - TPL for dram init; >> >> - SPL for storage init and load next stage firmware, decode FIT image >> with ATF/OPTEE support, secure boot support and etc; >> >> - U-Boot.itb including ATF and U-Boot proper, maybe also OPTEE. >> >> This model can very easy to do the debug with replace the binary from >> rockchip  vendor tree during board bringup. >> > > That's fine by me, but we currently have Rockchip boards in mainline > which do NOT have TPL enabled, so we need to handle those properly, in > binman since it's now the only way to generate the images. > >> The SPL+U-Boot mode can only happen for legacy board with only need >> U-Boot raw image instead of itb which including trust support, >> > > What makes SPL+U-Boot not capable of using U-Boot.itb or support > secure-boot/trust? It's possible to use SPL+U-Boot.itb, but it's not easy to maintain, because this model need to make all the driver available(FDT support, SDMMC/EMMC driver, FIT decode and etc)in the SPL, the SPL running in SRAM which has limit size, often cause size overflow issue. Since  BACK_TO_BOOTROM feature is available for all the rockchip SoCs, we can us TPL+SPL instead for much flexible solution. Thanks, - Kever > >> this kind of board can get the image with only one mkimage command, I >> don't think it will need binman support because it only have one image. >> > > I suggest we leave it as it is today, handling TPL+SPL+U-Boot AND > SPL+U-Boot, or we "upgrade" current boards which do not have TPL > support enabled yet so we only need to have TPL+SPL+U-Boot to support > and maintenance is easier in the long term (for the latter case, you > can then enforce it by having ARCH_ROCKCHIP imply TPL for example). > > Cheers, > Quentin