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

[Simon Glass]

Hi Kever,

On 20 March 2017 at 00:41, Kever Yang <kever.yang at 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 at 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 at 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