Hi Andre, On 26 March 2017 at 19:19, André Przywara <andre.przyw...@arm.com> wrote: > On 08/03/17 21:00, Simon Glass wrote: >> Hi Andre, >> >> On 28 February 2017 at 19:25, Andre Przywara <andre.przyw...@arm.com> wrote: >>> So far we were not using the FIT image format to its full potential: >>> The SPL FIT loader was just loading the first image from the /images >>> node plus one of the listed DTBs. >>> Now with the refactored loader code it's easy to load an arbitrary >>> number of images in addition to the two mentioned above. >>> As described in the FIT image source file format description, iterate >>> over all images listed at the "loadables" property in the configuration >>> node and load every image at its desired location. >>> This allows to load any kind of images: >>> - firmware images to execute before U-Boot proper (for instance >>> ARM Trusted Firmware (ATF)) >>> - firmware images for management processors (SCP, arisc, ...) >>> - firmware images for devices like WiFi controllers >>> - bit files for FPGAs >>> - additional configuration data >>> - kernels and/or ramdisks >>> The actual usage of this feature would be platform and/or board specific. >>> >>> Signed-off-by: Andre Przywara <andre.przyw...@arm.com> >>> --- >>> common/spl/spl_fit.c | 32 ++++++++++++++++++++++++++++---- >>> 1 file changed, 28 insertions(+), 4 deletions(-) >>> >>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c >>> index ad5ba15..5583e09 100644 >>> --- a/common/spl/spl_fit.c >>> +++ b/common/spl/spl_fit.c >>> @@ -178,10 +178,7 @@ static int spl_load_fit_image(struct spl_load_info >>> *info, ulong sector, >>> if (image_info) { >>> image_info->load_addr = load_addr; >>> image_info->size = length; >>> - if (entry == -1UL) >>> - image_info->entry_point = load_addr; >>> - else >>> - image_info->entry_point = entry; >>> + image_info->entry_point = entry; >> >> Need to update function comment to indicate that it can put -1 in here. >> >>> } >>> >>> return 0; >>> @@ -196,6 +193,7 @@ int spl_load_simple_fit(struct spl_image_info >>> *spl_image, >>> struct spl_image_info image_info; >>> int node, images; >>> int base_offset, align_len = ARCH_DMA_MINALIGN - 1; >>> + int index = 0; >>> >>> /* >>> * Figure out where the external images start. This is the base for >>> the >>> @@ -240,6 +238,11 @@ int spl_load_simple_fit(struct spl_image_info >>> *spl_image, >>> if (node < 0) { >>> debug("could not find firmware image, trying >>> loadables...\n"); >>> node = spl_fit_get_image_node(fit, images, "loadables", 0); >>> + /* >>> + * If we pick the U-Boot image from "loadables", start at >>> + * the second image when later loading additional images. >>> + */ >>> + index = 1; >>> } >>> if (node < 0) { >>> debug("%s: Cannot find u-boot image node: %d\n", >>> @@ -265,5 +268,26 @@ int spl_load_simple_fit(struct spl_image_info >>> *spl_image, >>> image_info.load_addr = spl_image->load_addr + spl_image->size; >>> spl_load_fit_image(info, sector, fit, base_offset, node, >>> &image_info); >>> >>> + /* Now check if there are more images for us to load */ >>> + for (; ; index++) { >>> + node = spl_fit_get_image_node(fit, images, "loadables", >>> index); >>> + if (node < 0) >>> + break; >>> + >>> + spl_load_fit_image(info, sector, fit, base_offset, node, >>> + &image_info); >> >> Error check? >> >>> + >>> + /* >>> + * If the "firmware" image did not provide an entry point, >>> + * use the first valid entry point from the loadables. >>> + */ >>> + if (spl_image->entry_point == -1UL && >>> + image_info.entry_point != -1UL) >>> + spl_image->entry_point = image_info.entry_point; >>> + } >>> + >>> + if (spl_image->entry_point == -1UL || spl_image->entry_point == 0) >> >> Why does 0 mean there is no entry point? I suppose that is safe, but >> would anyone use this? > > So this is due to U-Boot's own Makefile, which sets > CONFIG_SYS_UBOOT_START to 0 if it isn't already defined. This then gets > passed on to mkimage -f auto as the -e argument. > So today's FIT images have 0 as an entry point - a least on sunxi. As I > wanted to retain compatibility with that, I added this check. > Maybe sunxi (and other platforms?) should explicitly define the entry > point to avoid it being set to 0, or we should default to the load > address as the fallback in absence of such an explicit definition. > > In any case I wanted to keep existing u-boot.img files booting, so I > added this safe guard here to do "the right thing (tm)".
I very much doubt that 0 is needed as a default, so it should be OK to break that. I'm almost certain there is no test to verify it. Sometimes there are wierd things in the code that we should try to drop. > > I added a comment there to point this out. > > Cheers, > Andre. Regards, Simon -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.