Hi Andre,

On 28 February 2017 at 19:25, Andre Przywara andre.przywara@arm.com wrote:

Currently the SPL FIT loader uses the spl_fit_select_fdt() function to find the offset to the right DTB within the FIT image. For this it iterates over all subnodes of the /configuration node in the FIT tree and compares all "description" strings therein using a board specific matching function. If that finds a match, it uses the string in the "fdt" property of that subnode to locate the matching subnode in the /images node, which points to the DTB data. Now this works very well, but is quite specific to cover this particular use case. To open up the door for a more generic usage, let's split this function into:

1. a function that just returns the node offset for the matching configuration node (spl_fit_find_config_node())
2. a function that returns the image data any given property in a given configuration node points to, additionally using a given index into a possbile list of strings (spl_fit_select_index())

This allows us to replace the specific function above by asking for the image the _first string of the "fdt" property_ in the matching configuration subnode points to.

This patch introduces no functional changes, it just refactors the code to allow reusing it later.

(diff is overly clever here and produces a hard-to-read patch, so I recommend to throw a look at the result instead).

First I want to commend you on your excellent commit messages. For example this one explains the current situation, the change your commit performs and the motivation for that change. With these more complicated / core pieces, it is very valuable and you are an example to us all :-)

Signed-off-by: Andre Przywara andre.przywara@arm.com

common/spl/spl_fit.c | 83 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 31 deletions(-).

Reviewed-by: Simon Glass sjg@chromium.org

I think we need a pytest for this somewhere in this series. With sandbox_spl we can run spl/u-boot-spl and it jumps to u-boot. Can we use this to check that the right thing happens in a few simple cases?

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index aae556f..ba45e65 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -22,13 +22,11 @@ static ulong fdt_getprop_u32(const void *fdt, int node, const char *prop) return fdt32_to_cpu(*cell); }

-static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp) +static int spl_fit_find_config_node(const void *fdt)

Can you comment this function please? I should have done this myself.

{

•   const char *name, *fdt_name;
  

•   int conf, node, fdt_node;
  

•   int len;
  

•   const char *name;
  

•   int conf, node, len;
  
  

•   *fdt_offsetp = 0;
    conf = fdt_path_offset(fdt, FIT_CONFS_PATH);
    if (conf < 0) {
            debug("%s: Cannot find /configurations node: %d\n", __func__,
  

@@ -50,39 +48,61 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp) continue;

            debug("Selecting config '%s'", name);

•           fdt_name = fdt_getprop(fdt, node, FIT_FDT_PROP, &len);
  

•           if (!fdt_name) {
  

•                   debug("%s: Cannot find fdt name property: %d\n",
  

•                         __func__, len);
  

•                   return -EINVAL;
  

•           }
  
  

•           debug(", fdt '%s'\n", fdt_name);
  

•           fdt_node = fdt_subnode_offset(fdt, images, fdt_name);
  

•           if (fdt_node < 0) {
  

•                   debug("%s: Cannot find fdt node '%s': %d\n",
  

•                         __func__, fdt_name, fdt_node);
  

•                   return -EINVAL;
  

•           return node;
  

•   }
  

•   return -1;
  

How about -ENOENT?

+}

+static int spl_fit_select_index(const void *fit, int images, int *offsetp,

•                           const char *type, int index)
  

And this one.

On 02/28/2017 08:25 PM, Andre Przywara wrote:

At the moment we load two images from a FIT image: the actual U-Boot image and the DTB. Both times we have very similar code to deal with alignment requirement the media we load from imposes upon us. Factor out this code into a new function, which we just call twice.

Signed-off-by: Andre Przywara andre.przywara@arm.com

Acked-by: Andrew F. Davis afd@ti.com

+Franklin,

This patch, and #12 look like something you would be interested in for your 66AK2G0x work.

Andrew

common/spl/spl_fit.c | 129 +++++++++++++++++++++++---------------------------- 1 file changed, 57 insertions(+), 72 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 572a5db..ad5ba15 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -18,7 +18,7 @@ static ulong fdt_getprop_u32(const void *fdt, int node, const char *prop)

cell = fdt_getprop(fdt, node, prop, &len); if (len != sizeof(*cell))

• return -1U;
  

• return -1UL;
  

  return fdt32_to_cpu(*cell);

}

@@ -139,19 +139,63 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size, return (data_size + info->bl_len - 1) / info->bl_len; }

+static int spl_load_fit_image(struct spl_load_info *info, ulong sector,

• 	      void *fit, ulong base_offset, int node,
  

• 	      struct spl_image_info *image_info)
  

+{

• ulong offset;
• size_t length;
• ulong load_addr, load_ptr, entry;
• void *src;
• ulong overhead;
• int nr_sectors;
• int align_len = ARCH_DMA_MINALIGN - 1;
• offset = fdt_getprop_u32(fit, node, "data-offset") + base_offset;
• length = fdt_getprop_u32(fit, node, "data-size");
• load_addr = fdt_getprop_u32(fit, node, "load");
• if (load_addr == -1UL && image_info)

• load_addr = image_info->load_addr;
  

• load_ptr = (load_addr + align_len) & ~align_len;
• entry = fdt_getprop_u32(fit, node, "entry");
• overhead = get_aligned_image_overhead(info, offset);
• nr_sectors = get_aligned_image_size(info, length, offset);
• if (info->read(info, sector + get_aligned_image_offset(info, offset),

•        nr_sectors, (void*)load_ptr) != nr_sectors)
  

• return -EIO;
  

• debug("image: dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset,

•      (unsigned long)length);
  

• src = (void *)load_ptr + overhead;

+#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS

• board_fit_image_post_process(&src, &length);

+#endif

• memcpy((void*)load_addr, src, length);
• 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;
  

• }
• return 0;

+}

int spl_load_simple_fit(struct spl_image_info *spl_image, struct spl_load_info *info, ulong sector, void *fit) { int sectors;

• ulong size, load;

• ulong size; unsigned long count;
• struct spl_image_info image_info; int node, images;

• void *load_ptr;

• int fdt_offset, fdt_len;

• int data_offset, data_size; int base_offset, align_len = ARCH_DMA_MINALIGN - 1;

• int src_sector;

• void *dst, *src;

  /*

  • Figure out where the external images start. This is the base for the

@@ -203,82 +247,23 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, return -1; }

• /* Get its information and set up the spl_image structure */
• data_offset = fdt_getprop_u32(fit, node, "data-offset");
• data_size = fdt_getprop_u32(fit, node, "data-size");
• load = fdt_getprop_u32(fit, node, "load");
• debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
• spl_image->load_addr = load;
• spl_image->entry_point = load;

• /* Load the image and set up the spl_image structure */
• spl_load_fit_image(info, sector, fit, base_offset, node, spl_image); spl_image->os = IH_OS_U_BOOT;

• /*

• * Work out where to place the image. We read it so that the first
  

• * byte will be at 'load'. This may mean we need to load it starting
  

• * before then, since we can only read whole blocks.
  

• */
  

• data_offset += base_offset;
• sectors = get_aligned_image_size(info, data_size, data_offset);
• load_ptr = (void *)load;
• debug("U-Boot size %x, data %p\n", data_size, load_ptr);
• dst = load_ptr;
• /* Read the image */
• src_sector = sector + get_aligned_image_offset(info, data_offset);
• debug("Aligned image read: dst=%p, src_sector=%x, sectors=%x\n",

•      dst, src_sector, sectors);
  

• count = info->read(info, src_sector, sectors, dst);
• if (count != sectors)

• return -EIO;
  

• debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,

•      data_size);
  

• src = dst + get_aligned_image_overhead(info, data_offset);

-#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS

• board_fit_image_post_process((void **)&src, (size_t *)&data_size);

-#endif

• memcpy(dst, src, data_size);

• /* Figure out which device tree the board wants to use */ node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0); if (node < 0) { debug("%s: cannot find FDT node\n", __func__); return node; }

• fdt_offset = fdt_getprop_u32(fit, node, "data-offset");

• fdt_len = fdt_getprop_u32(fit, node, "data-size");

• /*

• * Read the device tree and place it after the image. There may be
  

• * some extra data before it since we can only read entire blocks.
  

• * And also align the destination address to ARCH_DMA_MINALIGN.
  

• */
  

• dst = (void *)((load + data_size + align_len) & ~align_len);

• fdt_offset += base_offset;

• sectors = get_aligned_image_size(info, fdt_len, fdt_offset);

• src_sector = sector + get_aligned_image_offset(info, fdt_offset);

• count = info->read(info, src_sector, sectors, dst);

• debug("Aligned fdt read: dst %p, src_sector = %x, sectors %x\n",

•      dst, src_sector, sectors);
  

• if (count != sectors)

• return -EIO;
  
  

  /*

• * Copy the device tree so that it starts immediately after the image.
  

• * After this we will have the U-Boot image and its device tree ready
  

• * for us to start.
  

• * Read the device tree and place it after the image.
  

• * Align the destination address to ARCH_DMA_MINALIGN.
  

  */

• debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,

•      fdt_len);
  

• src = dst + get_aligned_image_overhead(info, fdt_offset);
• dst = load_ptr + data_size;

-#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS

• board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);

-#endif

• memcpy(dst, src, fdt_len);

• image_info.load_addr = spl_image->load_addr + spl_image->size;

• spl_load_fit_image(info, sector, fit, base_offset, node, &image_info);

  return 0;

}

Hi Andre,

On 28 February 2017 at 19:25, Andre Przywara andre.przywara@arm.com wrote:

At the moment we load two images from a FIT image: the actual U-Boot image and the DTB. Both times we have very similar code to deal with alignment requirement the media we load from imposes upon us. Factor out this code into a new function, which we just call twice.

Signed-off-by: Andre Przywara andre.przywara@arm.com

common/spl/spl_fit.c | 129 +++++++++++++++++++++++---------------------------- 1 file changed, 57 insertions(+), 72 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 572a5db..ad5ba15 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -18,7 +18,7 @@ static ulong fdt_getprop_u32(const void *fdt, int node, const char *prop)

    cell = fdt_getprop(fdt, node, prop, &len);
    if (len != sizeof(*cell))

•           return -1U;
  

•           return -1UL;
  

Looks like an unrelated change but I see you need it below, so OK.

    return fdt32_to_cpu(*cell);

}

@@ -139,19 +139,63 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size, return (data_size + info->bl_len - 1) / info->bl_len; }

+static int spl_load_fit_image(struct spl_load_info *info, ulong sector,

•                         void *fit, ulong base_offset, int node,
  

•                         struct spl_image_info *image_info)
  

Please add a full comment to this one.

+{

•   ulong offset;
  

•   size_t length;
  

•   ulong load_addr, load_ptr, entry;
  

•   void *src;
  

•   ulong overhead;
  

•   int nr_sectors;
  

•   int align_len = ARCH_DMA_MINALIGN - 1;
  

•   offset = fdt_getprop_u32(fit, node, "data-offset") + base_offset;
  

I think you need to check that offset is not -1 first.

•   length = fdt_getprop_u32(fit, node, "data-size");
  

•   load_addr = fdt_getprop_u32(fit, node, "load");
  

•   if (load_addr == -1UL && image_info)
  

•           load_addr = image_info->load_addr;
  

•   load_ptr = (load_addr + align_len) & ~align_len;
  

•   entry = fdt_getprop_u32(fit, node, "entry");
  

•   overhead = get_aligned_image_overhead(info, offset);
  

•   nr_sectors = get_aligned_image_size(info, length, offset);
  

•   if (info->read(info, sector + get_aligned_image_offset(info, offset),
  

•                  nr_sectors, (void*)load_ptr) != nr_sectors)
  

•           return -EIO;
  

•   debug("image: dst=%lx, offset=%lx, size=%lx\n", load_ptr, offset,
  

•         (unsigned long)length);
  

•   src = (void *)load_ptr + overhead;
  

+#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS

•   board_fit_image_post_process(&src, &length);
  

+#endif

•   memcpy((void*)load_addr, src, length);
  

•   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;
  

•   }
  

•   return 0;
  

+}

int spl_load_simple_fit(struct spl_image_info *spl_image, struct spl_load_info *info, ulong sector, void *fit) { int sectors;

•   ulong size, load;
  

•   ulong size;
    unsigned long count;
  

•   struct spl_image_info image_info;
    int node, images;
  

•   void *load_ptr;
  

•   int fdt_offset, fdt_len;
  

•   int data_offset, data_size;
    int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
  

•   int src_sector;
  

•   void *dst, *src;
  
    /*
     * Figure out where the external images start. This is the base for the
  

@@ -203,82 +247,23 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, return -1; }

•   /* Get its information and set up the spl_image structure */
  

•   data_offset = fdt_getprop_u32(fit, node, "data-offset");
  

•   data_size = fdt_getprop_u32(fit, node, "data-size");
  

•   load = fdt_getprop_u32(fit, node, "load");
  

•   debug("data_offset=%x, data_size=%x\n", data_offset, data_size);
  

•   spl_image->load_addr = load;
  

•   spl_image->entry_point = load;
  

•   /* Load the image and set up the spl_image structure */
  

•   spl_load_fit_image(info, sector, fit, base_offset, node, spl_image);
  

Check error return?

    spl_image->os = IH_OS_U_BOOT;

•   /*
  

•    * Work out where to place the image. We read it so that the first
  

•    * byte will be at 'load'. This may mean we need to load it starting
  

•    * before then, since we can only read whole blocks.
  

•    */
  

•   data_offset += base_offset;
  

•   sectors = get_aligned_image_size(info, data_size, data_offset);
  

•   load_ptr = (void *)load;
  

•   debug("U-Boot size %x, data %p\n", data_size, load_ptr);
  

•   dst = load_ptr;
  

•   /* Read the image */
  

•   src_sector = sector + get_aligned_image_offset(info, data_offset);
  

•   debug("Aligned image read: dst=%p, src_sector=%x, sectors=%x\n",
  

•         dst, src_sector, sectors);
  

•   count = info->read(info, src_sector, sectors, dst);
  

•   if (count != sectors)
  

•           return -EIO;
  

•   debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,
  

•         data_size);
  

•   src = dst + get_aligned_image_overhead(info, data_offset);
  

-#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS

•   board_fit_image_post_process((void **)&src, (size_t *)&data_size);
  

-#endif

•   memcpy(dst, src, data_size);
  

•   /* Figure out which device tree the board wants to use */
    node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0);
    if (node < 0) {
            debug("%s: cannot find FDT node\n", __func__);
            return node;
    }
  

•   fdt_offset = fdt_getprop_u32(fit, node, "data-offset");
  

•   fdt_len = fdt_getprop_u32(fit, node, "data-size");
  

•   /*
  

•    * Read the device tree and place it after the image. There may be
  

•    * some extra data before it since we can only read entire blocks.
  

•    * And also align the destination address to ARCH_DMA_MINALIGN.
  

•    */
  

•   dst = (void *)((load + data_size + align_len) & ~align_len);
  

•   fdt_offset += base_offset;
  

•   sectors = get_aligned_image_size(info, fdt_len, fdt_offset);
  

•   src_sector = sector + get_aligned_image_offset(info, fdt_offset);
  

•   count = info->read(info, src_sector, sectors, dst);
  

•   debug("Aligned fdt read: dst %p, src_sector = %x, sectors %x\n",
  

•         dst, src_sector, sectors);
  

•   if (count != sectors)
  

•           return -EIO;
  
    /*
  

•    * Copy the device tree so that it starts immediately after the image.
  

•    * After this we will have the U-Boot image and its device tree ready
  

•    * for us to start.
  

•    * Read the device tree and place it after the image.
  

•    * Align the destination address to ARCH_DMA_MINALIGN.
     */
  

•   debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
  

•         fdt_len);
  

•   src = dst + get_aligned_image_overhead(info, fdt_offset);
  

•   dst = load_ptr + data_size;
  

-#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS

•   board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);
  

-#endif

•   memcpy(dst, src, fdt_len);
  

•   image_info.load_addr = spl_image->load_addr + spl_image->size;
  

•   spl_load_fit_image(info, sector, fit, base_offset, node, &image_info);
  

Check error return?

    return 0;

}

2.8.2

Regards, Simon

On 08/03/17 21:00, Simon Glass wrote:

Hi Andre,

On 28 February 2017 at 19:25, Andre Przywara andre.przywara@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.przywara@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 added a comment there to point this out.

Cheers, Andre.

...

•           spl_image->entry_point = spl_image->load_addr;
  

•   return 0;
  

}

2.8.2

Regards, Simon

Hi Olliver,

thanks for having a look.

On 29/03/17 15:43, Olliver Schinagl wrote:

Hey Andre,

On 01-03-17 03:25, Andre Przywara wrote:

...

mksunxiboot limits the size of the resulting SPL binaries to pretty conservative values to cover all SoCs and all boot media (NAND). It turns out that we have limit checks in place in the build process, so mksunxiboot can be relaxed and allow packaging binaries up to the actual 32KB the mask boot ROM actually imposes. This allows to have a bigger SPL, which is crucial for AArch64 builds.

Signed-off-by: Andre Przywara andre.przywara@arm.com

tools/mksunxiboot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c index 0f0b003..6bb649c 100644 --- a/tools/mksunxiboot.c +++ b/tools/mksunxiboot.c @@ -48,7 +48,7 @@ int gen_check_sum(struct boot_file_head *head_p) #define ALIGN(x, a) __ALIGN_MASK((x), (typeof(x))(a)-1) #define __ALIGN_MASK(x, mask) (((x)+(mask))&~(mask))

-#define SUN4I_SRAM_SIZE 0x7600 /* 0x7748+ is used by BROM */ +#define SUN4I_SRAM_SIZE 0x8000 /* SoC with smaller size are limited before */

This seems a little scary,

It is indeed (that's why I moved the SPL stack away in a later patch), but we really need it.

but besides that, you need to rename the define to SUNXI imo too then.

OK.

SUN4I, SUN5I and SUN7I (I think) all have the 24K limit imposed by the BROM. Only the later gens have a 32k limit. Since it is nice to have a single binary for all, name the define as SUNXI too I think. As a comment, I would put the responsibility elsewhere (instead of making it an assumption).

What my admittedly clumsy comment wanted to say is that U-Boot's linker script puts the 24K limit in place for those SoCs already. So we fail much earlier, so at least from a U-Boot point of view this is fine.

With that said, why do we have to check it at all then, other then to give a warning? The size is limited before already, right?

Frankly I didn't want to tinker with mksunxiboot too much, especially since I added mksunxiboot's functionality already to mkimage (in a local branch here). But this is a separate series and I wanted to not hold this series back any longer.

Cheers, Andre.

Olliver

...

#define SRAM_LOAD_MAX_SIZE (SUN4I_SRAM_SIZE - sizeof(struct boot_file_head))

/*

Hi Andre,

On 28 February 2017 at 19:25, Andre Przywara andre.przywara@arm.com wrote:

The generic ARMv8 assembly code contains routines for setting up a CCN interconnect, though the Freescale SoCs are the only user. Link this code only for Freescale targets, this saves some precious bytes in the chronically tight SPL.

Signed-off-by: Andre Przywara andre.przywara@arm.com

arch/arm/cpu/armv8/fsl-layerscape/Makefile | 1 + arch/arm/lib/Makefile | 1 - 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Makefile b/arch/arm/cpu/armv8/fsl-layerscape/Makefile index c9ab93e..ca09973 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/Makefile +++ b/arch/arm/cpu/armv8/fsl-layerscape/Makefile @@ -7,6 +7,7 @@ obj-y += cpu.o obj-y += lowlevel.o obj-y += soc.o +obj-y += ccn504.o

Don't you need to move the file into the same directory?

obj-$(CONFIG_MP) += mp.o obj-$(CONFIG_OF_LIBFDT) += fdt.o obj-$(CONFIG_SPL) += spl.o diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 71de1ca..60ffb4a 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -43,7 +43,6 @@ obj-y += stack.o ifdef CONFIG_CPU_V7M obj-y += interrupts_m.o else ifdef CONFIG_ARM64 -obj-y += ccn504.o ifneq ($(CONFIG_GICV2)$(CONFIG_GICV3),) obj-y += gic_64.o endif -- 2.8.2

Regards, Simon

On 01/03/17 03:03, Icenowy Zheng wrote:

01.03.2017, 10:26, "Andre Przywara" andre.przywara@arm.com:

...

For a board or platform to support FIT loading in the SPL, it has to provide a board_fit_config_name_match() routine, which helps to select one of possibly multiple DTBs contained in a FIT image. Provide a simple function which chooses the DT name U-Boot was configured with. If the DT name is one of the two Pine64 versions, determine the exact model by checking the DRAM size.

I think we shouldn't have is specially for Pine64 here, but make a framework for other boards that can be easily checked.

Then make Pine64 series the first user of this framework.

Well, actually this whole board_fit_config_name_match() *is* the framework to differentiate boards at runtime. I don't see a reason why we should make it more complicated than it already is. With that last patch in the series we leave it to the SPL header to select the board.

So it's really just the two different Pine64 models that need extra hand holding here.

So I would hold off the magic framework until we know that we really need it (a second user) and *what* we really need.

Cheers, Andre.

...

Signed-off-by: Andre Przywara andre.przywara@arm.com

board/sunxi/board.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c index a510422..2ddff28 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -725,3 +725,26 @@ int ft_board_setup(void *blob, bd_t *bd) #endif return 0; }

+#ifdef CONFIG_SPL_LOAD_FIT +int board_fit_config_name_match(const char *name) +{

• const char *cmp_str;

+#ifdef CONFIG_DEFAULT_DEVICE_TREE

• cmp_str = CONFIG_DEFAULT_DEVICE_TREE;

+#else

• return 0;

+#endif

+/* Differentiate the two Pine64 board DTs by their DRAM size. */

• if (strstr(name, "-pine64") && strstr(cmp_str, "-pine64")) {
• if ((gd->ram_size > 512 * 1024 * 1024))
• return !strstr(name, "plus");
• else
• return !!strstr(name, "plus");
• } else {
• return strcmp(name, cmp_str);
• }

+}

+#endif

2.8.2

-- 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+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.

---

U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/listinfo/u-boot

Hi Andre,

On Wed, Mar 01, 2017 at 02:25:26AM +0000, Andre Przywara wrote:

The Pine64 (and all other 64-bit Allwinner boards) need to load an ARM Trusted Firmware image beside the actual U-Boot proper. This can now be easily achieved by using the just extended SPL FIT loading support, so enable it in the Pine64 defconfig. Also add the FIT image as a build target to 64-bit sunxi board to trigger the respective Makefile rules.

Signed-off-by: Andre Przywara andre.przywara@arm.com

configs/pine64_plus_defconfig | 6 ++++++ include/configs/sunxi-common.h | 4 ++++ 2 files changed, 10 insertions(+)

diff --git a/configs/pine64_plus_defconfig b/configs/pine64_plus_defconfig index 7c7d86f..2b47157 100644 --- a/configs/pine64_plus_defconfig +++ b/configs/pine64_plus_defconfig @@ -3,9 +3,14 @@ CONFIG_RESERVE_ALLWINNER_BOOT0_HEADER=y CONFIG_ARCH_SUNXI=y CONFIG_MACH_SUN50I=y CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus" +CONFIG_OF_LIST="sun50i-a64-pine64 sun50i-a64-pine64-plus" # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_CONSOLE_MUX=y CONFIG_SPL=y +CONFIG_FIT=y +CONFIG_SPL_FIT=y +CONFIG_SPL_LOAD_FIT=y +CONFIG_SPL_OF_LIBFDT=y

I'm not sure we want to force down that support to all our users.

We should definitely make that easy (basically just switching on an option), for example a few selects here might be good. But switching that on, and only for a few boards seems both weird and inconsistent.

# CONFIG_CMD_IMLS is not set # CONFIG_CMD_FLASH is not set # CONFIG_CMD_FPGA is not set @@ -14,3 +19,4 @@ CONFIG_SPL=y # CONFIG_SPL_EFI_PARTITION is not set CONFIG_SUN8I_EMAC=y CONFIG_USB_EHCI_HCD=y +CONFIG_SPL_FIT_GENERATOR="board/sunxi/mksunxi_fit_atf.sh"

And that could be a default value.

Maxime