[U-Boot] [PATCH v4 0/3] spl: Add support to load FIT from Filesystem

[U-Boot] [PATCH v4 0/3] spl: Add support to load FIT from Filesystem

[Lokesh Vutla]

Some devices like MMC, USB can be formatted to a FS and can act as a boot media.
Given that FIT image load support in SPL support only raw devices, SPL should
also be able to support loading FIT image from a File system.
This series add support to load FIT image from a filesystem and also adding
hooks to FAT FS.

Changes since v3:
- Use the same function for reading the fit from FS or raw device.

Lokesh Vutla (3):
  spl: fit: Fix the number of bytes read when reading fdt from fit
  spl: Allow to load a FIT containing U-Boot from FS
  spl: Support loading a FIT from FAT FS

 common/spl/spl_fat.c | 31 ++++++++++++++++++++--
 common/spl/spl_fit.c | 75 ++++++++++++++++++++++++++++++++++++++++++----------
 include/spl.h        | 10 +++++++
 3 files changed, 100 insertions(+), 16 deletions(-)

-- 
2.1.4

[U-Boot] [PATCH v4 1/3] spl: fit: Fix the number of bytes read when reading fdt from fit

[Lokesh Vutla]

sectors field is not being updated when reading fdt from fit image. Because of
this size_of(u-boot.bin) is being read when reading fdt. Fixing it by updating
the sectors field properly.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 common/spl/spl_fit.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 1a5c027..b1cfa97 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -176,6 +176,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	 */
 	dst = load_ptr + data_size;
 	fdt_offset += base_offset;
+	sectors = (fdt_len + info->bl_len - 1) / info->bl_len;
 	count = info->read(info, sector + fdt_offset / info->bl_len, sectors,
 			   dst);
 	debug("fit read %x sectors to %x, dst %p, data_offset %x\n",
-- 
2.1.4

[U-Boot] [PATCH v4 2/3] spl: Allow to load a FIT containing U-Boot from FS

[Lokesh Vutla]

This provides a way to load a FIT containing U-Boot and a selection of device
tree files from a File system. Making sure that all the reads and writes
are aligned to their respective needs.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
- Assuming info->priv is used for passing filename for now and using this for 
  detecting if it a fs read. If this is not preferred I can create a new field
  for filename.

 common/spl/spl_fit.c | 76 +++++++++++++++++++++++++++++++++++++++++-----------
 include/spl.h        | 10 +++++++
 2 files changed, 71 insertions(+), 15 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index b1cfa97..d696354 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -82,6 +82,42 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp)
 	return -ENOENT;
 }
 
+static int get_aligned_image_offset(struct spl_load_info *info, int offset)
+{
+	/*
+	 * If it is a FS read, get the first address before offset which is
+	 * aligned to ARCH_DMA_MINALIGN. If it is raw read return the
+	 * block number to which offset belongs.
+	 */
+	if (info->priv)
+		return offset & ~(ARCH_DMA_MINALIGN - 1);
+	else
+		return offset / info->bl_len;
+}
+
+static int get_aligned_image_overhead(struct spl_load_info *info, int offset)
+{
+	/*
+	 * If it is a FS read, get the difference between the offset and
+	 * the first address before offset which is aligned to
+	 * ARCH_DMA_MINALIGN. If it is raw read return the offset within the
+	 * block.
+	 */
+	if (info->priv)
+		return offset & (ARCH_DMA_MINALIGN - 1);
+	else
+		return offset % info->bl_len;
+}
+
+static int get_aligned_image_size(struct spl_load_info *info, int data_size,
+				  int offset)
+{
+	if (info->priv)
+		return data_size + get_aligned_image_overhead(info, offset);
+	else
+		return (data_size + info->bl_len - 1) / info->bl_len;
+}
+
 int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 {
 	int sectors;
@@ -91,7 +127,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	void *load_ptr;
 	int fdt_offset, fdt_len;
 	int data_offset, data_size;
-	int base_offset;
+	int base_offset, align_len;
 	int src_sector;
 	void *dst;
 
@@ -117,8 +153,10 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	 * In fact the FIT has its own load address, but we assume it cannot
 	 * be before CONFIG_SYS_TEXT_BASE.
 	 */
-	fit = (void *)(CONFIG_SYS_TEXT_BASE - size - info->bl_len);
-	sectors = (size + info->bl_len - 1) / info->bl_len;
+	align_len = ARCH_DMA_MINALIGN - 1;
+	fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len -
+			align_len) & ~align_len);
+	sectors = get_aligned_image_size(info, size, 0);
 	count = info->read(info, sector, sectors, fit);
 	debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n",
 	      sector, sectors, fit, count);
@@ -151,19 +189,23 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	 * byte will be at 'load'. This may mean we need to load it starting
 	 * before then, since we can only read whole blocks.
 	 */
-	sectors = (data_size + info->bl_len - 1) / info->bl_len;
+	sectors = get_aligned_image_size(info, data_size, data_offset);
 	data_offset += base_offset;
 	load_ptr = (void *)load;
 	debug("U-Boot size %x, data %p\n", data_size, load_ptr);
-	dst = load_ptr - (data_offset % info->bl_len);
+	dst = load_ptr;
 
 	/* Read the image */
-	src_sector = sector + data_offset / info->bl_len;
-	debug("image: data_offset=%x, dst=%p, src_sector=%x, sectors=%x\n",
-	      data_offset, dst, src_sector, sectors);
+	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);
+	memcpy(dst, dst + get_aligned_image_overhead(info, data_offset),
+	       data_size);
 
 	/* Figure out which device tree the board wants to use */
 	fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
@@ -173,14 +215,15 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	/*
 	 * 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 = load_ptr + data_size;
+	dst = (void *)((load + data_size + align_len) & ~align_len);
 	fdt_offset += base_offset;
-	sectors = (fdt_len + info->bl_len - 1) / info->bl_len;
-	count = info->read(info, sector + fdt_offset / info->bl_len, sectors,
-			   dst);
-	debug("fit read %x sectors to %x, dst %p, data_offset %x\n",
-	      sectors, spl_image.load_addr, dst, fdt_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;
 
@@ -189,7 +232,10 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 	 * After this we will have the U-Boot image and its device tree ready
 	 * for us to start.
 	 */
-	memcpy(dst, dst + fdt_offset % info->bl_len, fdt_len);
+	debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
+	      fdt_len);
+	memcpy(load_ptr + data_size,
+	       dst + get_aligned_image_overhead(info, fdt_offset), fdt_len);
 
 	return 0;
 }
diff --git a/include/spl.h b/include/spl.h
index de4f70a..358e81b 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -45,6 +45,16 @@ struct spl_load_info {
 		      void *buf);
 };
 
+/**
+ * spl_load_simple_fit() - Loads a fit image from a device.
+ * @info:	Structure containing the information required to load data.
+ * @sector:	Sector number where FIT image is located in the device
+ * @fdt:	Pointer to the copied FIT header.
+ *
+ * Reads the FIT image @sector in the device. Loads u-boot image to
+ * specified load address and copies the dtb to end of u-boot image.
+ * Returns 0 on success.
+ */
 int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fdt);
 
 #define SPL_COPY_PAYLOAD_ONLY	1
-- 
2.1.4

[U-Boot] [PATCH v4 2/3] spl: Allow to load a FIT containing U-Boot from FS

[Michal Simek]

Hi,

On 3.5.2016 11:05, Lokesh Vutla wrote:
> This provides a way to load a FIT containing U-Boot and a selection of device
> tree files from a File system. Making sure that all the reads and writes
> are aligned to their respective needs.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> - Assuming info->priv is used for passing filename for now and using this for 
>   detecting if it a fs read. If this is not preferred I can create a new field
>   for filename.
> 
>  common/spl/spl_fit.c | 76 +++++++++++++++++++++++++++++++++++++++++-----------
>  include/spl.h        | 10 +++++++
>  2 files changed, 71 insertions(+), 15 deletions(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index b1cfa97..d696354 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -82,6 +82,42 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp)
>  	return -ENOENT;
>  }
>  
> +static int get_aligned_image_offset(struct spl_load_info *info, int offset)
> +{
> +	/*
> +	 * If it is a FS read, get the first address before offset which is
> +	 * aligned to ARCH_DMA_MINALIGN. If it is raw read return the
> +	 * block number to which offset belongs.
> +	 */
> +	if (info->priv)
> +		return offset & ~(ARCH_DMA_MINALIGN - 1);
> +	else
> +		return offset / info->bl_len;
> +}
> +
> +static int get_aligned_image_overhead(struct spl_load_info *info, int offset)
> +{
> +	/*
> +	 * If it is a FS read, get the difference between the offset and
> +	 * the first address before offset which is aligned to
> +	 * ARCH_DMA_MINALIGN. If it is raw read return the offset within the
> +	 * block.
> +	 */
> +	if (info->priv)
> +		return offset & (ARCH_DMA_MINALIGN - 1);
> +	else
> +		return offset % info->bl_len;
> +}
> +
> +static int get_aligned_image_size(struct spl_load_info *info, int data_size,
> +				  int offset)
> +{
> +	if (info->priv)
> +		return data_size + get_aligned_image_overhead(info, offset);
> +	else
> +		return (data_size + info->bl_len - 1) / info->bl_len;
> +}
> +
>  int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  {
>  	int sectors;
> @@ -91,7 +127,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  	void *load_ptr;
>  	int fdt_offset, fdt_len;
>  	int data_offset, data_size;
> -	int base_offset;
> +	int base_offset, align_len;
>  	int src_sector;
>  	void *dst;
>  
> @@ -117,8 +153,10 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  	 * In fact the FIT has its own load address, but we assume it cannot
>  	 * be before CONFIG_SYS_TEXT_BASE.
>  	 */
> -	fit = (void *)(CONFIG_SYS_TEXT_BASE - size - info->bl_len);
> -	sectors = (size + info->bl_len - 1) / info->bl_len;
> +	align_len = ARCH_DMA_MINALIGN - 1;
> +	fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len -
> +			align_len) & ~align_len);
> +	sectors = get_aligned_image_size(info, size, 0);
>  	count = info->read(info, sector, sectors, fit);
>  	debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n",
>  	      sector, sectors, fit, count);
> @@ -151,19 +189,23 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  	 * byte will be at 'load'. This may mean we need to load it starting
>  	 * before then, since we can only read whole blocks.
>  	 */
> -	sectors = (data_size + info->bl_len - 1) / info->bl_len;
> +	sectors = get_aligned_image_size(info, data_size, data_offset);
>  	data_offset += base_offset;
>  	load_ptr = (void *)load;
>  	debug("U-Boot size %x, data %p\n", data_size, load_ptr);
> -	dst = load_ptr - (data_offset % info->bl_len);
> +	dst = load_ptr;
>  
>  	/* Read the image */
> -	src_sector = sector + data_offset / info->bl_len;
> -	debug("image: data_offset=%x, dst=%p, src_sector=%x, sectors=%x\n",
> -	      data_offset, dst, src_sector, sectors);
> +	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);
> +	memcpy(dst, dst + get_aligned_image_overhead(info, data_offset),
> +	       data_size);
>  
>  	/* Figure out which device tree the board wants to use */
>  	fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
> @@ -173,14 +215,15 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  	/*
>  	 * 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 = load_ptr + data_size;
> +	dst = (void *)((load + data_size + align_len) & ~align_len);
>  	fdt_offset += base_offset;
> -	sectors = (fdt_len + info->bl_len - 1) / info->bl_len;
> -	count = info->read(info, sector + fdt_offset / info->bl_len, sectors,
> -			   dst);
> -	debug("fit read %x sectors to %x, dst %p, data_offset %x\n",
> -	      sectors, spl_image.load_addr, dst, fdt_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;
>  
> @@ -189,7 +232,10 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  	 * After this we will have the U-Boot image and its device tree ready
>  	 * for us to start.
>  	 */
> -	memcpy(dst, dst + fdt_offset % info->bl_len, fdt_len);
> +	debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
> +	      fdt_len);
> +	memcpy(load_ptr + data_size,
> +	       dst + get_aligned_image_overhead(info, fdt_offset), fdt_len);
>  
>  	return 0;
>  }
> diff --git a/include/spl.h b/include/spl.h
> index de4f70a..358e81b 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -45,6 +45,16 @@ struct spl_load_info {
>  		      void *buf);
>  };
>  
> +/**
> + * spl_load_simple_fit() - Loads a fit image from a device.
> + * @info:	Structure containing the information required to load data.
> + * @sector:	Sector number where FIT image is located in the device
> + * @fdt:	Pointer to the copied FIT header.
> + *
> + * Reads the FIT image @sector in the device. Loads u-boot image to
> + * specified load address and copies the dtb to end of u-boot image.
> + * Returns 0 on success.
> + */
>  int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fdt);
>  
>  #define SPL_COPY_PAYLOAD_ONLY	1
> 

this is not working for me. I haven't had a time to look at it
but you might know where the problem is.

Thanks,
Michal

U-Boot SPL 2016.05-rc3-00067-g01d410633461-dirty (May 03 2016 - 14:43:30)
Trying to boot from MMC1
reading u-boot.bin
reading u-boot.img
Found FIT
spl_fit_read: name u-boot.img, file_offset 0, size 44c, buf 7fffb40
reading u-boot.img
spl_fit_read: error reading image u-boot.img, ret - 0
fit read sector 0, sectors=1100, dst=0000000007fffb40, count=1100
data_offset=0, data_size=80310
U-Boot size 80310, data 0000000008000000
Aligned image read: dst=0000000008000000, src_sector=440, sectors=80310
spl_fit_read: name u-boot.img, file_offset 440, size 80310, buf 8000000
reading u-boot.img
spl_fit_read: error reading image u-boot.img, ret - 0
image: dst=0000000008000000, data_offset=44c, size=80310
Selecting config 'zynqmp-zcu102', fdt 'fdt at 1'
FIT: Selected 'zynqmp-zcu102'
spl_fit_read: name u-boot.img, file_offset 80740, size 6603, buf 8080340
reading u-boot.img
** Unable to read file u-boot.img **
spl_fit_read: error reading image u-boot.img, ret - -1
Aligned fdt read: dst 0000000008080340, src_sector = 80740, sectors 6603
reading u-boot.img
spl_load_image_fat: error reading image u-boot.img, err - -1
SPL: failed to boot from all boot devices
### ERROR ### Please RESET the board ###

[U-Boot] [PATCH v4 2/3] spl: Allow to load a FIT containing U-Boot from FS

[Lokesh Vutla]

On Tuesday 03 May 2016 06:27 PM, Michal Simek wrote:
> Hi,

[..snip..]

> 
> this is not working for me. I haven't had a time to look at it
> but you might know where the problem is.

> 
> Thanks,
> Michal
> 
> U-Boot SPL 2016.05-rc3-00067-g01d410633461-dirty (May 03 2016 - 14:43:30)
> Trying to boot from MMC1
> reading u-boot.bin
> reading u-boot.img
> Found FIT
> spl_fit_read: name u-boot.img, file_offset 0, size 44c, buf 7fffb40
> reading u-boot.img
> spl_fit_read: error reading image u-boot.img, ret - 0
> fit read sector 0, sectors=1100, dst=0000000007fffb40, count=1100
> data_offset=0, data_size=80310
> U-Boot size 80310, data 0000000008000000
> Aligned image read: dst=0000000008000000, src_sector=440, sectors=80310
> spl_fit_read: name u-boot.img, file_offset 440, size 80310, buf 8000000
> reading u-boot.img
> spl_fit_read: error reading image u-boot.img, ret - 0
> image: dst=0000000008000000, data_offset=44c, size=80310
> Selecting config 'zynqmp-zcu102', fdt 'fdt at 1'
> FIT: Selected 'zynqmp-zcu102'
> spl_fit_read: name u-boot.img, file_offset 80740, size 6603, buf 8080340
> reading u-boot.img
> ** Unable to read file u-boot.img **
> spl_fit_read: error reading image u-boot.img, ret - -1

The error started here. It is able to detect fdt from fit header but not
able to read it from file. If you don't mind can you tell me what is the
size of the file u-boot.img?

A dumb question: Can you confirm that on this same HEAD the RFC patch
from you works fine?

I verified this on 2 different platforms and worked fine. Lets see If I
am missing any corner case.

Thanks and regards,
Lokesh

> Aligned fdt read: dst 0000000008080340, src_sector = 80740, sectors 6603
> reading u-boot.img
> spl_load_image_fat: error reading image u-boot.img, err - -1
> SPL: failed to boot from all boot devices
> ### ERROR ### Please RESET the board ###
> 
> 

[U-Boot] [PATCH v4 2/3] spl: Allow to load a FIT containing U-Boot from FS

[Michal Simek]

On 3.5.2016 11:05, Lokesh Vutla wrote:
> This provides a way to load a FIT containing U-Boot and a selection of device
> tree files from a File system. Making sure that all the reads and writes
> are aligned to their respective needs.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> - Assuming info->priv is used for passing filename for now and using this for 
>   detecting if it a fs read. If this is not preferred I can create a new field
>   for filename.
> 
>  common/spl/spl_fit.c | 76 +++++++++++++++++++++++++++++++++++++++++-----------
>  include/spl.h        | 10 +++++++
>  2 files changed, 71 insertions(+), 15 deletions(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index b1cfa97..d696354 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -82,6 +82,42 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp)
>  	return -ENOENT;
>  }
>  
> +static int get_aligned_image_offset(struct spl_load_info *info, int offset)
> +{
> +	/*
> +	 * If it is a FS read, get the first address before offset which is
> +	 * aligned to ARCH_DMA_MINALIGN. If it is raw read return the
> +	 * block number to which offset belongs.
> +	 */
> +	if (info->priv)
> +		return offset & ~(ARCH_DMA_MINALIGN - 1);
> +	else

remove this else.

> +		return offset / info->bl_len;
> +}
> +
> +static int get_aligned_image_overhead(struct spl_load_info *info, int offset)
> +{
> +	/*
> +	 * If it is a FS read, get the difference between the offset and
> +	 * the first address before offset which is aligned to
> +	 * ARCH_DMA_MINALIGN. If it is raw read return the offset within the
> +	 * block.
> +	 */
> +	if (info->priv)
> +		return offset & (ARCH_DMA_MINALIGN - 1);
> +	else

remove this else.

> +		return offset % info->bl_len;
> +}
> +
> +static int get_aligned_image_size(struct spl_load_info *info, int data_size,
> +				  int offset)
> +{
> +	if (info->priv)
> +		return data_size + get_aligned_image_overhead(info, offset);
> +	else

remove this else.


> +		return (data_size + info->bl_len - 1) / info->bl_len;
> +}
> +
>  int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  {
>  	int sectors;
> @@ -91,7 +127,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  	void *load_ptr;
>  	int fdt_offset, fdt_len;
>  	int data_offset, data_size;
> -	int base_offset;
> +	int base_offset, align_len;
>  	int src_sector;
>  	void *dst;
>  
> @@ -117,8 +153,10 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  	 * In fact the FIT has its own load address, but we assume it cannot
>  	 * be before CONFIG_SYS_TEXT_BASE.
>  	 */
> -	fit = (void *)(CONFIG_SYS_TEXT_BASE - size - info->bl_len);
> -	sectors = (size + info->bl_len - 1) / info->bl_len;
> +	align_len = ARCH_DMA_MINALIGN - 1;
> +	fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len -
> +			align_len) & ~align_len);

Don't you want to use ALIGN macro here?

> +	sectors = get_aligned_image_size(info, size, 0);
>  	count = info->read(info, sector, sectors, fit);
>  	debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n",
>  	      sector, sectors, fit, count);
> @@ -151,19 +189,23 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  	 * byte will be at 'load'. This may mean we need to load it starting
>  	 * before then, since we can only read whole blocks.
>  	 */
> -	sectors = (data_size + info->bl_len - 1) / info->bl_len;
> +	sectors = get_aligned_image_size(info, data_size, data_offset);
>  	data_offset += base_offset;
>  	load_ptr = (void *)load;
>  	debug("U-Boot size %x, data %p\n", data_size, load_ptr);
> -	dst = load_ptr - (data_offset % info->bl_len);
> +	dst = load_ptr;
>  
>  	/* Read the image */
> -	src_sector = sector + data_offset / info->bl_len;
> -	debug("image: data_offset=%x, dst=%p, src_sector=%x, sectors=%x\n",
> -	      data_offset, dst, src_sector, sectors);
> +	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);
> +	memcpy(dst, dst + get_aligned_image_overhead(info, data_offset),
> +	       data_size);
>  
>  	/* Figure out which device tree the board wants to use */
>  	fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
> @@ -173,14 +215,15 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>  	/*
>  	 * 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 = load_ptr + data_size;
> +	dst = (void *)((load + data_size + align_len) & ~align_len);

Same here.

Thanks,
Michal

[U-Boot] [PATCH v4 2/3] spl: Allow to load a FIT containing U-Boot from FS

[Lokesh Vutla]

On Tuesday 03 May 2016 06:29 PM, Michal Simek wrote:
> On 3.5.2016 11:05, Lokesh Vutla wrote:
>> This provides a way to load a FIT containing U-Boot and a selection of device
>> tree files from a File system. Making sure that all the reads and writes
>> are aligned to their respective needs.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> - Assuming info->priv is used for passing filename for now and using this for 
>>   detecting if it a fs read. If this is not preferred I can create a new field
>>   for filename.
>>
>>  common/spl/spl_fit.c | 76 +++++++++++++++++++++++++++++++++++++++++-----------
>>  include/spl.h        | 10 +++++++
>>  2 files changed, 71 insertions(+), 15 deletions(-)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index b1cfa97..d696354 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -82,6 +82,42 @@ static int spl_fit_select_fdt(const void *fdt, int images, int *fdt_offsetp)
>>  	return -ENOENT;
>>  }
>>  
>> +static int get_aligned_image_offset(struct spl_load_info *info, int offset)
>> +{
>> +	/*
>> +	 * If it is a FS read, get the first address before offset which is
>> +	 * aligned to ARCH_DMA_MINALIGN. If it is raw read return the
>> +	 * block number to which offset belongs.
>> +	 */
>> +	if (info->priv)
>> +		return offset & ~(ARCH_DMA_MINALIGN - 1);
>> +	else
> 
> remove this else.

Sure.

> 
>> +		return offset / info->bl_len;
>> +}
>> +
>> +static int get_aligned_image_overhead(struct spl_load_info *info, int offset)
>> +{
>> +	/*
>> +	 * If it is a FS read, get the difference between the offset and
>> +	 * the first address before offset which is aligned to
>> +	 * ARCH_DMA_MINALIGN. If it is raw read return the offset within the
>> +	 * block.
>> +	 */
>> +	if (info->priv)
>> +		return offset & (ARCH_DMA_MINALIGN - 1);
>> +	else
> 
> remove this else.

Sure.

> 
>> +		return offset % info->bl_len;
>> +}
>> +
>> +static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>> +				  int offset)
>> +{
>> +	if (info->priv)
>> +		return data_size + get_aligned_image_overhead(info, offset);
>> +	else
> 
> remove this else.

Sure.

> 
> 
>> +		return (data_size + info->bl_len - 1) / info->bl_len;
>> +}
>> +
>>  int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>>  {
>>  	int sectors;
>> @@ -91,7 +127,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>>  	void *load_ptr;
>>  	int fdt_offset, fdt_len;
>>  	int data_offset, data_size;
>> -	int base_offset;
>> +	int base_offset, align_len;
>>  	int src_sector;
>>  	void *dst;
>>  
>> @@ -117,8 +153,10 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>>  	 * In fact the FIT has its own load address, but we assume it cannot
>>  	 * be before CONFIG_SYS_TEXT_BASE.
>>  	 */
>> -	fit = (void *)(CONFIG_SYS_TEXT_BASE - size - info->bl_len);
>> -	sectors = (size + info->bl_len - 1) / info->bl_len;
>> +	align_len = ARCH_DMA_MINALIGN - 1;
>> +	fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len -
>> +			align_len) & ~align_len);
> 
> Don't you want to use ALIGN macro here?

ALIGN() will return the next aligned address which is equal to:

fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len + align_len)
& ~align_len);

But here we need the previous aligned address so that we do not loose
any data.

Thanks and regards,
Lokesh

> 
>> +	sectors = get_aligned_image_size(info, size, 0);
>>  	count = info->read(info, sector, sectors, fit);
>>  	debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n",
>>  	      sector, sectors, fit, count);
>> @@ -151,19 +189,23 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>>  	 * byte will be at 'load'. This may mean we need to load it starting
>>  	 * before then, since we can only read whole blocks.
>>  	 */
>> -	sectors = (data_size + info->bl_len - 1) / info->bl_len;
>> +	sectors = get_aligned_image_size(info, data_size, data_offset);
>>  	data_offset += base_offset;
>>  	load_ptr = (void *)load;
>>  	debug("U-Boot size %x, data %p\n", data_size, load_ptr);
>> -	dst = load_ptr - (data_offset % info->bl_len);
>> +	dst = load_ptr;
>>  
>>  	/* Read the image */
>> -	src_sector = sector + data_offset / info->bl_len;
>> -	debug("image: data_offset=%x, dst=%p, src_sector=%x, sectors=%x\n",
>> -	      data_offset, dst, src_sector, sectors);
>> +	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);
>> +	memcpy(dst, dst + get_aligned_image_overhead(info, data_offset),
>> +	       data_size);
>>  
>>  	/* Figure out which device tree the board wants to use */
>>  	fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
>> @@ -173,14 +215,15 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>>  	/*
>>  	 * 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 = load_ptr + data_size;
>> +	dst = (void *)((load + data_size + align_len) & ~align_len);
> 
> Same here.
> 
> Thanks,
> Michal
> 

[U-Boot] [PATCH v4 3/3] spl: Support loading a FIT from FAT FS

[Lokesh Vutla]

Detect a FIT when loading from a FAT File system and handle it using the
new FIT SPL support.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 common/spl/spl_fat.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index d761b26..3788e4d 100644
--- a/common/spl/spl_fat.c
+++ b/common/spl/spl_fat.c
@@ -15,6 +15,7 @@
 #include <fat.h>
 #include <errno.h>
 #include <image.h>
+#include <libfdt.h>
 
 static int fat_registered;
 
@@ -39,6 +40,20 @@ static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
 	return err;
 }
 
+static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
+			  ulong size, void *buf)
+{
+	loff_t actread;
+	int ret;
+	char *filename = (char *)load->priv;
+
+	ret = fat_read_file(filename, buf, file_offset, size, &actread);
+	if (ret)
+		return ret;
+	else
+		return actread;
+}
+
 int spl_load_image_fat(struct blk_desc *block_dev,
 						int partition,
 						const char *filename)
@@ -57,9 +72,21 @@ int spl_load_image_fat(struct blk_desc *block_dev,
 	if (err <= 0)
 		goto end;
 
-	spl_parse_image_header(header);
+	if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
+	    image_get_magic(header) == FDT_MAGIC) {
+		struct spl_load_info load;
 
-	err = file_fat_read(filename, (u8 *)spl_image.load_addr, 0);
+		debug("Found FIT\n");
+		load.read = spl_fit_read;
+		load.bl_len = 1;
+		load.priv = (void *)filename;
+
+		return spl_load_simple_fit(&load, 0, header);
+	} else {
+		spl_parse_image_header(header);
+
+		err = file_fat_read(filename, (u8 *)spl_image.load_addr, 0);
+	}
 
 end:
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
-- 
2.1.4

[U-Boot] [PATCH v4 3/3] spl: Support loading a FIT from FAT FS

[Michal Simek]

On 3.5.2016 11:05, Lokesh Vutla wrote:
> Detect a FIT when loading from a FAT File system and handle it using the
> new FIT SPL support.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  common/spl/spl_fat.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
> index d761b26..3788e4d 100644
> --- a/common/spl/spl_fat.c
> +++ b/common/spl/spl_fat.c
> @@ -15,6 +15,7 @@
>  #include <fat.h>
>  #include <errno.h>
>  #include <image.h>
> +#include <libfdt.h>
>  
>  static int fat_registered;
>  
> @@ -39,6 +40,20 @@ static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
>  	return err;
>  }
>  
> +static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
> +			  ulong size, void *buf)
> +{
> +	loff_t actread;
> +	int ret;
> +	char *filename = (char *)load->priv;
> +
> +	ret = fat_read_file(filename, buf, file_offset, size, &actread);
> +	if (ret)
> +		return ret;
> +	else

you can remove this else.

Thanks,
Michal

[U-Boot] [PATCH v4 3/3] spl: Support loading a FIT from FAT FS

[Lokesh Vutla]

On Tuesday 03 May 2016 06:28 PM, Michal Simek wrote:
> On 3.5.2016 11:05, Lokesh Vutla wrote:
>> Detect a FIT when loading from a FAT File system and handle it using the
>> new FIT SPL support.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  common/spl/spl_fat.c | 31 +++++++++++++++++++++++++++++--
>>  1 file changed, 29 insertions(+), 2 deletions(-)
>>
>> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
>> index d761b26..3788e4d 100644
>> --- a/common/spl/spl_fat.c
>> +++ b/common/spl/spl_fat.c
>> @@ -15,6 +15,7 @@
>>  #include <fat.h>
>>  #include <errno.h>
>>  #include <image.h>
>> +#include <libfdt.h>
>>  
>>  static int fat_registered;
>>  
>> @@ -39,6 +40,20 @@ static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
>>  	return err;
>>  }
>>  
>> +static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
>> +			  ulong size, void *buf)
>> +{
>> +	loff_t actread;
>> +	int ret;
>> +	char *filename = (char *)load->priv;
>> +
>> +	ret = fat_read_file(filename, buf, file_offset, size, &actread);
>> +	if (ret)
>> +		return ret;
>> +	else
> 
> you can remove this else.

Sure. Will fix it in the next version.

Thanks and regards,
Lokesh

> 
> Thanks,
> Michal
>