[PATCH v2 1/2] spl: Use standard FIT entries
Michal Simek
michal.simek at xilinx.com
Mon Oct 12 09:51:51 CEST 2020
On 12. 10. 20 5:34, Simon Glass wrote:
> On Mon, 5 Oct 2020 at 02:53, Michal Simek <michal.simek at xilinx.com> wrote:
>>
>> SPL is creating fit-images DT node when loadables are recorded in selected
>> configuration. Entries which are created are using entry-point and
>> load-addr property names. But there shouldn't be a need to use non standard
>> properties because entry/load are standard FIT properties. But using
>> standard FIT properties enables option to use generic FIT functions to
>> descrease SPL size. Here is result for ZynqMP virt configuration:
>> xilinx_zynqmp_virt: spl/u-boot-spl:all -82 spl/u-boot-spl:rodata -22 spl/u-boot-spl:text -60
>>
>> The patch causes change in run time fit image record.
>> Before:
>> fit-images {
>> uboot {
>> os = "u-boot";
>> type = "firmware";
>> size = <0xfd520>;
>> entry-point = <0x8000000>;
>> load-addr = <0x8000000>;
>> };
>> };
>>
>> After:
>> fit-images {
>> uboot {
>> os = "u-boot";
>> type = "firmware";
>> size = <0xfd520>;
>> entry = <0x8000000>;
>> load = <0x8000000>;
>> };
>> };
>>
>> Replacing calling fdt_getprop_u32() by fit_image_get_entry/load() also
>> enables support for reading entry/load properties recorded in 64bit format.
>>
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>> Changes in v2:
>> - Also fix opensbi
>> - Add record to doc/uImage.FIT/howto.txt - reported by Simon
>>
>> Would be good to know history of fit-images and it's property names but
>> there shouldn't be a need to use non standard names where we have
>> FIT_*_PROP recorded as macros in include/image.h.
>>
>> Concern regarding backward compatibility is definitely valid but not sure
>> how many systems can be affected by this change.
>>
>> Adding temporary support for entry-point/load-addr is also possible.
>> Or second way around is to create new wrappers as
>> fit_image_get_entry_point()/fit_image_get_load_addr() or
>> call fit_image_get_address() directly.
>>
>> ---
>> common/fdt_support.c | 4 +-
>> common/spl/spl_atf.c | 7 ++--
>> common/spl/spl_fit.c | 6 ++-
>> common/spl/spl_opensbi.c | 8 ++--
>> doc/uImage.FIT/howto.txt | 84 ++++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 98 insertions(+), 11 deletions(-)
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> optional nit below
>
>>
>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>> index a565b470f81e..b8a8768a2147 100644
>> --- a/common/fdt_support.c
>> +++ b/common/fdt_support.c
>> @@ -616,9 +616,9 @@ int fdt_record_loadable(void *blob, u32 index, const char *name,
>> * However, spl_fit.c is not 64bit safe either: i.e. we should not
>> * have an issue here.
>> */
>> - fdt_setprop_u32(blob, node, "load-addr", load_addr);
>> + fdt_setprop_u32(blob, node, "load", load_addr);
>> if (entry_point != -1)
>> - fdt_setprop_u32(blob, node, "entry-point", entry_point);
>> + fdt_setprop_u32(blob, node, "entry", entry_point);
>> fdt_setprop_u32(blob, node, "size", size);
>> if (type)
>> fdt_setprop_string(blob, node, "type", type);
>> diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
>> index b54b4f0d22e2..9bd25f6b32c1 100644
>> --- a/common/spl/spl_atf.c
>> +++ b/common/spl/spl_atf.c
>> @@ -132,10 +132,11 @@ static int spl_fit_images_find(void *blob, int os)
>> uintptr_t spl_fit_images_get_entry(void *blob, int node)
>> {
>> ulong val;
>> + int ret;
>>
>> - val = fdt_getprop_u32(blob, node, "entry-point");
>> - if (val == FDT_ERROR)
>> - val = fdt_getprop_u32(blob, node, "load-addr");
>> + ret = fit_image_get_entry(blob, node, &val);
>> + if (ret)
>> + ret = fit_image_get_load(blob, node, &val);
>>
>> debug("%s: entry point 0x%lx\n", __func__, val);
>> return val;
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index 0e27ad1d6a55..aeb8aeda2b19 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -332,9 +332,13 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
>> }
>>
>> if (image_info) {
>> + ulong entry_point;
>> +
>> image_info->load_addr = load_addr;
>> image_info->size = length;
>> - image_info->entry_point = fdt_getprop_u32(fit, node, "entry");
>> +
>> + if (!fit_image_get_entry(fit, node, &entry_point))
>> + image_info->entry_point = entry_point;
>> }
>>
>> return 0;
>> diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
>> index 14f335f75f02..41e0746bb012 100644
>> --- a/common/spl/spl_opensbi.c
>> +++ b/common/spl/spl_opensbi.c
>> @@ -61,11 +61,9 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image)
>> }
>>
>> /* Get U-Boot entry point */
>> - uboot_entry = fdt_getprop_u32(spl_image->fdt_addr, uboot_node,
>> - "entry-point");
>> - if (uboot_entry == FDT_ERROR)
>> - uboot_entry = fdt_getprop_u32(spl_image->fdt_addr, uboot_node,
>> - "load-addr");
>> + ret = fit_image_get_entry(spl_image->fdt_addr, uboot_node, &uboot_entry);
>> + if (ret)
>> + ret = fit_image_get_load(spl_image->fdt_addr, uboot_node, &uboot_entry);
>>
>> /* Prepare obensbi_info object */
>> opensbi_info.magic = FW_DYNAMIC_INFO_MAGIC_VALUE;
>> diff --git a/doc/uImage.FIT/howto.txt b/doc/uImage.FIT/howto.txt
>> index 8592719685eb..a136f1466056 100644
>> --- a/doc/uImage.FIT/howto.txt
>> +++ b/doc/uImage.FIT/howto.txt
>> @@ -66,6 +66,90 @@ can point to a script which generates this image source file during
>> the build process. It gets passed a list of device tree files (taken from the
>> CONFIG_OF_LIST symbol).
>>
>> +The SPL also records to a DT all additional images (called loadables) which are
>> +loaded. The information about loadables locations is passed via the DT node with
>> +fit-images name.
>> +
>> +Loadables Example
>> +-----------------
>> +Consider the following case for an ARM64 platform where U-Boot runs in EL2
>> +started by ATF where SPL is loading U-Boot (as loadables) and ATF (as firmware).
>> +
>> +/dts-v1/;
>> +
>> +/ {
>> + description = "Configuration to load ATF before U-Boot";
>> +
>> + images {
>> + uboot {
>> + description = "U-Boot (64-bit)";
>> + data = /incbin/("u-boot-nodtb.bin");
>> + type = "firmware";
>> + os = "u-boot";
>> + arch = "arm64";
>> + compression = "none";
>> + load = <0x8000000>;
>> + entry = <0x8000000>;
>> + hash {
>> + algo = "md5";
>> + };
>> + };
>> + atf {
>> + description = "ARM Trusted Firmware";
>> + data = /incbin/("bl31.bin");
>> + type = "firmware";
>> + os = "arm-trusted-firmware";
>> + arch = "arm64";
>> + compression = "none";
>> + load = <0xfffea000>;
>> + entry = <0xfffea000>;
>> + hash {
>> + algo = "md5";
>> + };
>> + };
>> + fdt_1 {
>> + description = "zynqmp-zcu104-revC";
>> + data = /incbin/("arch/arm/dts/zynqmp-zcu104-revC.dtb");
>> + type = "flat_dt";
>> + arch = "arm64";
>> + compression = "none";
>> + load = <0x100000>;
>> + hash {
>> + algo = "md5";
>> + };
>> + };
>> + };
>> + configurations {
>> + default = "config_1";
>> +
>> + config_1 {
>> + description = "zynqmp-zcu104-revC";
>> + firmware = "atf";
>> + loadables = "uboot";
>> + fdt = "fdt_1";
>> + };
>> + };
>> +};
>> +
>> +In this case the SPL records via fit-images DT node the information about
>> +loadables U-Boot image.
>> +
>> +ZynqMP> fdt addr $fdtcontroladdr
>> +ZynqMP> fdt print /fit-images
>> +fit-images {
>> + uboot {
>> + os = "u-boot";
>> + type = "firmware";
>> + size = <0x0011f430>;
>> + entry = <0x00000000 0x08000000>;
>> + load = <0x00000000 0x08000000>;
>
> I suggest an example that actually needs 64-bit addresses.
Updated in v3.
Thanks,
Michal
More information about the U-Boot
mailing list