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

[Kever Yang]

Hi Simon,

On 03/22/2017 09:05 PM, Simon Glass wrote:
> 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)

Your understand is correct, will update in next version, thanks.
>
>> +
>> +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?

OK, will update in next version.
>
>> +
>>          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

OK.
>
>> +
>> +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?

OK, will update. this is copy from ATF project.

Thanks,
- Kever
>> + * 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
>
>
>