Re: [U-Boot] [PATCH v1] spl: add support to booting with ATF

Hi Kever,

On 20 March 2017 at 00:41, Kever Yang kever.yang@rock-chips.com wrote:

ATF(ARM Trust Firmware) is used by ARM arch64 SoCs, find more infomation about ATF at:

SPL is consider as BL2 in ATF, it needs to load other part of ATF binary like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to prepare the parameter for BL31 which including entry and image information for all other images. Then the SPL handle PC to BL31 with the parameter, the BL31 will do the rest of work and at last get into U-Boot(BL33).

Signed-off-by: Kever Yang kever.yang@rock-chips.com

Changes in v1:

• license update
• split out as separate patch

common/spl/Kconfig | 14 +++ common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 92 +++++++++++++++++ include/atf_common.h | 276 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/spl.h | 1 + 6 files changed, 388 insertions(+) create mode 100644 common/spl/spl_atf.c create mode 100644 include/atf_common.h

diff --git a/common/spl/Kconfig b/common/spl/Kconfig index cba51f5..1bb4360 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -577,6 +577,20 @@ config SPL_YMODEM_SUPPORT means of transmitting U-Boot over a serial line for using in SPL, with a checksum to ensure correctness.

+config SPL_ATF_SUPPORT

•   bool "Support ARM trust firmware"
  

•   depends on SPL
  

•   help
  

•     ATF(ARM Trust Firmware) is component for ARM arch64 which need to
  

•     load by SPL(consider as BL2 in ATF).
  

•     More detail at: https://github.com/ARM-software/arm-trusted-firmware
  

I think it is Trusted rather than Trust. How about:

ATF (ARM Trusted Firmware) is a component for ARM aarch64 which is loaded by SPL (which is considered as BL2 in ATF terminology).

(I'm not 100% sure I have the right idea, though)

+config SPL_ATF_TEXT_BASE

•   depends on SPL_ATF_SUPPORT
  

•   hex "ATF TEXT BASE addr"
  

•   help
  

•     This is the base address in memory for ATF text and entry point.
  

config TPL_ENV_SUPPORT bool "Support an environment" depends on TPL diff --git a/common/spl/Makefile b/common/spl/Makefile index ed02635..620ae90 100644 --- a/common/spl/Makefile +++ b/common/spl/Makefile @@ -20,6 +20,7 @@ endif obj-$(CONFIG_SPL_UBI) += spl_ubi.o obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.o diff --git a/common/spl/spl.c b/common/spl/spl.c index 1729034..7daf7bd 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -390,6 +390,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024); #endif

+#ifdef CONFIG_SPL_ATF_SUPPORT

•   bl31_entry();
  

+#endif

Can you use if (IS_ENABLED(SPL_ATF_SUPPORT)) just to remove an #ifdef?

•   debug("loaded - jumping to U-Boot...");
    spl_board_prepare_for_boot();
    jump_to_image_no_args(&spl_image);
  

diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c new file mode 100644 index 0000000..ec3f675 --- /dev/null +++ b/common/spl/spl_atf.c @@ -0,0 +1,92 @@ +/*

• • Reference to the ARM TF Project,
• • plat/arm/common/arm_bl2_setup.c
• • Portions copyright (c) 2013-2016, ARM Limited and Contributors. All rights
• • reserved.
• • Copyright (C) 2016 Rockchip Electronic Co.,Ltd
• • Written by Kever Yang kever.yang@rock-chips.com
• • SPDX-License-Identifier: BSD-3-Clause
• */

+#include <common.h> +#include <errno.h> +#include <spl.h> +#include <atf_common.h>

Nit: move below common.h

+static struct bl2_to_bl31_params_mem_t bl31_params_mem; +static struct bl31_params_t *bl2_to_bl31_params;

+/*******************************************************************************

Can you remove all the extra stars and use the normal fnuction and structure comment style?

• • This function assigns a pointer to the memory that the platform has kept
• • aside to pass platform specific and trusted firmware related information
• • to BL31. This memory is allocated by allocating memory to
• • bl2_to_bl31_params_mem_t structure which is a superset of all the
• • structure whose information is passed to BL31
• • NOTE: This function should be called only once and should be done
• • before generating params to BL31
• ******************************************************************************/

Regards, Simon