diff mbox

[U-Boot,RFC] SPL: FIT: Enable SPL_FIT_LOAD for sd bootmode for fat partions

Message ID b89a6d53dae934b3960c2d6fe764a282006063d9.1461835876.git.michal.simek@xilinx.com
State RFC
Delegated to: Tom Rini
Headers show

Commit Message

Michal Simek April 28, 2016, 9:31 a.m. UTC
Support U-Boot SPL to load FIT image from fat partition.
Fit image can be setup via CONFIG_SPL_FS_LOAD_KERNEL_NAME.
Falcon mode is not supported.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 common/spl/spl_fat.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
 fs/fat/fat.c         |  4 ++--
 2 files changed, 46 insertions(+), 8 deletions(-)

Comments

Simon Glass May 1, 2016, 6:54 p.m. UTC | #1
On 28 April 2016 at 03:31, Michal Simek <michal.simek@xilinx.com> wrote:
> Support U-Boot SPL to load FIT image from fat partition.
> Fit image can be setup via CONFIG_SPL_FS_LOAD_KERNEL_NAME.
> Falcon mode is not supported.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
>  common/spl/spl_fat.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
>  fs/fat/fat.c         |  4 ++--
>  2 files changed, 46 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Lokesh Vutla May 2, 2016, 4:06 a.m. UTC | #2
Hi Michal,

On Thursday 28 April 2016 03:01 PM, Michal Simek wrote:
> Support U-Boot SPL to load FIT image from fat partition.
> Fit image can be setup via CONFIG_SPL_FS_LOAD_KERNEL_NAME.
> Falcon mode is not supported.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  common/spl/spl_fat.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
>  fs/fat/fat.c         |  4 ++--
>  2 files changed, 46 insertions(+), 8 deletions(-)
> 
> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
> index d16cd540e38a..4e319c5fa470 100644
> --- a/common/spl/spl_fat.c
> +++ b/common/spl/spl_fat.c
> @@ -13,6 +13,7 @@
>  #include <spl.h>
>  #include <asm/u-boot.h>
>  #include <fat.h>
> +#include <libfdt.h>
>  #include <errno.h>
>  #include <image.h>
>  
> @@ -39,6 +40,29 @@ static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
>  	return err;
>  }
>  
> +#ifdef CONFIG_SPL_LOAD_FIT
> +static ulong spl_fat_file_read(struct spl_load_info *load, ulong sector,
> +			       ulong count, void *buf)
> +{
> +	int err;
> +	loff_t actread;
> +	char *filename = (char *)load->priv;
> +
> +	debug("%s: name %s, sector %lx, count %lx, buf %lx\n",
> +	      __func__, filename,  sector, count, (ulong)buf);
> +
> +	err = file_fat_read_at(filename, sector, buf, count, &actread);
> +	if (err < 0) {
> +		printf("%s: error reading image %s, err - %d\n",
> +		       __func__, filename, err);
> +		return err;
> +	}
> +
> +	debug("actread %lx\n", (ulong)actread);
> +	return actread;
> +}
> +#endif
> +
>  int spl_load_image_fat(struct blk_desc *block_dev,
>  						int partition,
>  						const char *filename)
> @@ -57,16 +81,29 @@ 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;
> +
> +		debug("Found FIT\n");
> +		load.priv = (char *)filename;
> +		load.bl_len = 1;
> +		load.read = spl_fat_file_read;
> +		spl_load_simple_fit(&load, 0, header);
> +	} else {
> +		debug("Legacy image\n");
>  
> -	err = file_fat_read(filename, (u8 *)(uintptr_t)spl_image.load_addr, 0);
> +		spl_parse_image_header(header);
>  
> +		err = file_fat_read(filename,
> +				    (u8 *)(uintptr_t)spl_image.load_addr, 0);
>  end:
>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> -	if (err <= 0)
> -		printf("%s: error reading image %s, err - %d\n",
> -		       __func__, filename, err);
> +		if (err <= 0)
> +			printf("%s: error reading image %s, err - %d\n",
> +			       __func__, filename, err);
>  #endif
> +	}
>  
>  	return (err <= 0);
>  }
> @@ -81,6 +118,7 @@ int spl_load_image_fat_os(struct blk_desc *block_dev, int partition)
>  	if (err)
>  		return err;
>  
> +#if !defined(CONFIG_SPL_LOAD_FIT)
>  #if defined(CONFIG_SPL_ENV_SUPPORT) && defined(CONFIG_SPL_OS_BOOT)
>  	file = getenv("falcon_args_file");
>  	if (file) {
> @@ -116,7 +154,7 @@ defaults:
>  #endif
>  		return -1;
>  	}
> -
> +#endif
>  	return spl_load_image_fat(block_dev, partition,
>  			CONFIG_SPL_FS_LOAD_KERNEL_NAME);
>  }
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 600a90e30922..0d987e0465ee 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -281,9 +281,9 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
>  
>  	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
>  		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
> -
> +#if !defined(CONFIG_SPL_LOAD_FIT)
>  		printf("FAT: Misaligned buffer address (%p)\n", buffer);
> -
> +#endif

IMO, this is a hack. Why should fs worry about if it as fit image or
not. Also the read performance will be very slow if you do not pass the
aligned buffer address.  I had a different approach[1] for this: first
copy the image to aligned buffer and then do a memcpy to the proper
destination(which showed a better performance). May be this is wrong.

Simon, which one do you prefer?


[1] http://patchwork.ozlabs.org/patch/610371/ (Still working on this :( )


Thanks and regards,
Lokesh

>  		while (size >= mydata->sect_size) {
>  			ret = disk_read(startsect++, 1, tmpbuf);
>  			if (ret != 1) {
>
Michal Simek May 2, 2016, 7:57 a.m. UTC | #3
On 2.5.2016 06:06, Lokesh Vutla wrote:
> Hi Michal,
> 
> On Thursday 28 April 2016 03:01 PM, Michal Simek wrote:
>> Support U-Boot SPL to load FIT image from fat partition.
>> Fit image can be setup via CONFIG_SPL_FS_LOAD_KERNEL_NAME.
>> Falcon mode is not supported.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  common/spl/spl_fat.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
>>  fs/fat/fat.c         |  4 ++--
>>  2 files changed, 46 insertions(+), 8 deletions(-)
>>
>> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
>> index d16cd540e38a..4e319c5fa470 100644
>> --- a/common/spl/spl_fat.c
>> +++ b/common/spl/spl_fat.c
>> @@ -13,6 +13,7 @@
>>  #include <spl.h>
>>  #include <asm/u-boot.h>
>>  #include <fat.h>
>> +#include <libfdt.h>
>>  #include <errno.h>
>>  #include <image.h>
>>  
>> @@ -39,6 +40,29 @@ static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
>>  	return err;
>>  }
>>  
>> +#ifdef CONFIG_SPL_LOAD_FIT
>> +static ulong spl_fat_file_read(struct spl_load_info *load, ulong sector,
>> +			       ulong count, void *buf)
>> +{
>> +	int err;
>> +	loff_t actread;
>> +	char *filename = (char *)load->priv;
>> +
>> +	debug("%s: name %s, sector %lx, count %lx, buf %lx\n",
>> +	      __func__, filename,  sector, count, (ulong)buf);
>> +
>> +	err = file_fat_read_at(filename, sector, buf, count, &actread);
>> +	if (err < 0) {
>> +		printf("%s: error reading image %s, err - %d\n",
>> +		       __func__, filename, err);
>> +		return err;
>> +	}
>> +
>> +	debug("actread %lx\n", (ulong)actread);
>> +	return actread;
>> +}
>> +#endif
>> +
>>  int spl_load_image_fat(struct blk_desc *block_dev,
>>  						int partition,
>>  						const char *filename)
>> @@ -57,16 +81,29 @@ 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;
>> +
>> +		debug("Found FIT\n");
>> +		load.priv = (char *)filename;
>> +		load.bl_len = 1;
>> +		load.read = spl_fat_file_read;
>> +		spl_load_simple_fit(&load, 0, header);
>> +	} else {
>> +		debug("Legacy image\n");
>>  
>> -	err = file_fat_read(filename, (u8 *)(uintptr_t)spl_image.load_addr, 0);
>> +		spl_parse_image_header(header);
>>  
>> +		err = file_fat_read(filename,
>> +				    (u8 *)(uintptr_t)spl_image.load_addr, 0);
>>  end:
>>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
>> -	if (err <= 0)
>> -		printf("%s: error reading image %s, err - %d\n",
>> -		       __func__, filename, err);
>> +		if (err <= 0)
>> +			printf("%s: error reading image %s, err - %d\n",
>> +			       __func__, filename, err);
>>  #endif
>> +	}
>>  
>>  	return (err <= 0);
>>  }
>> @@ -81,6 +118,7 @@ int spl_load_image_fat_os(struct blk_desc *block_dev, int partition)
>>  	if (err)
>>  		return err;
>>  
>> +#if !defined(CONFIG_SPL_LOAD_FIT)
>>  #if defined(CONFIG_SPL_ENV_SUPPORT) && defined(CONFIG_SPL_OS_BOOT)
>>  	file = getenv("falcon_args_file");
>>  	if (file) {
>> @@ -116,7 +154,7 @@ defaults:
>>  #endif
>>  		return -1;
>>  	}
>> -
>> +#endif
>>  	return spl_load_image_fat(block_dev, partition,
>>  			CONFIG_SPL_FS_LOAD_KERNEL_NAME);
>>  }
>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> index 600a90e30922..0d987e0465ee 100644
>> --- a/fs/fat/fat.c
>> +++ b/fs/fat/fat.c
>> @@ -281,9 +281,9 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
>>  
>>  	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
>>  		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
>> -
>> +#if !defined(CONFIG_SPL_LOAD_FIT)
>>  		printf("FAT: Misaligned buffer address (%p)\n", buffer);
>> -
>> +#endif
> 
> IMO, this is a hack. Why should fs worry about if it as fit image or
> not. Also the read performance will be very slow if you do not pass the
> aligned buffer address.  I had a different approach[1] for this: first
> copy the image to aligned buffer and then do a memcpy to the proper
> destination(which showed a better performance). May be this is wrong.

I agree that's why this was RFC not regular patch.
I have looked at your solution and also Simon's comments and truth is
that your patch has a lot of duplicated stuff.
This solution is smaller.
Regarding buffer alignment. I think this can be simply added to read
function to keep it in the right place.

Thanks,
Michal
Lokesh Vutla May 3, 2016, 9:10 a.m. UTC | #4
On Monday 02 May 2016 01:27 PM, Michal Simek wrote:
> On 2.5.2016 06:06, Lokesh Vutla wrote:
>> Hi Michal,
>>
>> On Thursday 28 April 2016 03:01 PM, Michal Simek wrote:
>>> Support U-Boot SPL to load FIT image from fat partition.
>>> Fit image can be setup via CONFIG_SPL_FS_LOAD_KERNEL_NAME.
>>> Falcon mode is not supported.
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>>  common/spl/spl_fat.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
>>>  fs/fat/fat.c         |  4 ++--
>>>  2 files changed, 46 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
>>> index d16cd540e38a..4e319c5fa470 100644
>>> --- a/common/spl/spl_fat.c
>>> +++ b/common/spl/spl_fat.c
>>> @@ -13,6 +13,7 @@
>>>  #include <spl.h>
>>>  #include <asm/u-boot.h>
>>>  #include <fat.h>
>>> +#include <libfdt.h>
>>>  #include <errno.h>
>>>  #include <image.h>
>>>  
>>> @@ -39,6 +40,29 @@ static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
>>>  	return err;
>>>  }
>>>  
>>> +#ifdef CONFIG_SPL_LOAD_FIT
>>> +static ulong spl_fat_file_read(struct spl_load_info *load, ulong sector,
>>> +			       ulong count, void *buf)
>>> +{
>>> +	int err;
>>> +	loff_t actread;
>>> +	char *filename = (char *)load->priv;
>>> +
>>> +	debug("%s: name %s, sector %lx, count %lx, buf %lx\n",
>>> +	      __func__, filename,  sector, count, (ulong)buf);
>>> +
>>> +	err = file_fat_read_at(filename, sector, buf, count, &actread);
>>> +	if (err < 0) {
>>> +		printf("%s: error reading image %s, err - %d\n",
>>> +		       __func__, filename, err);
>>> +		return err;
>>> +	}
>>> +
>>> +	debug("actread %lx\n", (ulong)actread);
>>> +	return actread;
>>> +}
>>> +#endif
>>> +
>>>  int spl_load_image_fat(struct blk_desc *block_dev,
>>>  						int partition,
>>>  						const char *filename)
>>> @@ -57,16 +81,29 @@ 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;
>>> +
>>> +		debug("Found FIT\n");
>>> +		load.priv = (char *)filename;
>>> +		load.bl_len = 1;
>>> +		load.read = spl_fat_file_read;
>>> +		spl_load_simple_fit(&load, 0, header);
>>> +	} else {
>>> +		debug("Legacy image\n");
>>>  
>>> -	err = file_fat_read(filename, (u8 *)(uintptr_t)spl_image.load_addr, 0);
>>> +		spl_parse_image_header(header);
>>>  
>>> +		err = file_fat_read(filename,
>>> +				    (u8 *)(uintptr_t)spl_image.load_addr, 0);
>>>  end:
>>>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
>>> -	if (err <= 0)
>>> -		printf("%s: error reading image %s, err - %d\n",
>>> -		       __func__, filename, err);
>>> +		if (err <= 0)
>>> +			printf("%s: error reading image %s, err - %d\n",
>>> +			       __func__, filename, err);
>>>  #endif
>>> +	}
>>>  
>>>  	return (err <= 0);
>>>  }
>>> @@ -81,6 +118,7 @@ int spl_load_image_fat_os(struct blk_desc *block_dev, int partition)
>>>  	if (err)
>>>  		return err;
>>>  
>>> +#if !defined(CONFIG_SPL_LOAD_FIT)
>>>  #if defined(CONFIG_SPL_ENV_SUPPORT) && defined(CONFIG_SPL_OS_BOOT)
>>>  	file = getenv("falcon_args_file");
>>>  	if (file) {
>>> @@ -116,7 +154,7 @@ defaults:
>>>  #endif
>>>  		return -1;
>>>  	}
>>> -
>>> +#endif
>>>  	return spl_load_image_fat(block_dev, partition,
>>>  			CONFIG_SPL_FS_LOAD_KERNEL_NAME);
>>>  }
>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>>> index 600a90e30922..0d987e0465ee 100644
>>> --- a/fs/fat/fat.c
>>> +++ b/fs/fat/fat.c
>>> @@ -281,9 +281,9 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
>>>  
>>>  	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
>>>  		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
>>> -
>>> +#if !defined(CONFIG_SPL_LOAD_FIT)
>>>  		printf("FAT: Misaligned buffer address (%p)\n", buffer);
>>> -
>>> +#endif
>>
>> IMO, this is a hack. Why should fs worry about if it as fit image or
>> not. Also the read performance will be very slow if you do not pass the
>> aligned buffer address.  I had a different approach[1] for this: first
>> copy the image to aligned buffer and then do a memcpy to the proper
>> destination(which showed a better performance). May be this is wrong.
> 
> I agree that's why this was RFC not regular patch.
> I have looked at your solution and also Simon's comments and truth is
> that your patch has a lot of duplicated stuff.
> This solution is smaller.
> Regarding buffer alignment. I think this can be simply added to read
> function to keep it in the right place.

We don't want to take care of the alignment in read function of each fs
type. IMHO, this should be handled by fit framework itself. I have
updated my DMA alignment patch to take care of FS read as well[1]. Can
you see if that helps?

[1] https://www.mail-archive.com/u-boot@lists.denx.de/msg211628.html

Thanks and regards,
Lokesh

> 
> Thanks,
> Michal
> 
>
Michal Simek May 3, 2016, 1:30 p.m. UTC | #5
On 3.5.2016 11:10, Lokesh Vutla wrote:
> 
> 
> On Monday 02 May 2016 01:27 PM, Michal Simek wrote:
>> On 2.5.2016 06:06, Lokesh Vutla wrote:
>>> Hi Michal,
>>>
>>> On Thursday 28 April 2016 03:01 PM, Michal Simek wrote:
>>>> Support U-Boot SPL to load FIT image from fat partition.
>>>> Fit image can be setup via CONFIG_SPL_FS_LOAD_KERNEL_NAME.
>>>> Falcon mode is not supported.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>
>>>>  common/spl/spl_fat.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------
>>>>  fs/fat/fat.c         |  4 ++--
>>>>  2 files changed, 46 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
>>>> index d16cd540e38a..4e319c5fa470 100644
>>>> --- a/common/spl/spl_fat.c
>>>> +++ b/common/spl/spl_fat.c
>>>> @@ -13,6 +13,7 @@
>>>>  #include <spl.h>
>>>>  #include <asm/u-boot.h>
>>>>  #include <fat.h>
>>>> +#include <libfdt.h>
>>>>  #include <errno.h>
>>>>  #include <image.h>
>>>>  
>>>> @@ -39,6 +40,29 @@ static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
>>>>  	return err;
>>>>  }
>>>>  
>>>> +#ifdef CONFIG_SPL_LOAD_FIT
>>>> +static ulong spl_fat_file_read(struct spl_load_info *load, ulong sector,
>>>> +			       ulong count, void *buf)
>>>> +{
>>>> +	int err;
>>>> +	loff_t actread;
>>>> +	char *filename = (char *)load->priv;
>>>> +
>>>> +	debug("%s: name %s, sector %lx, count %lx, buf %lx\n",
>>>> +	      __func__, filename,  sector, count, (ulong)buf);
>>>> +
>>>> +	err = file_fat_read_at(filename, sector, buf, count, &actread);
>>>> +	if (err < 0) {
>>>> +		printf("%s: error reading image %s, err - %d\n",
>>>> +		       __func__, filename, err);
>>>> +		return err;
>>>> +	}
>>>> +
>>>> +	debug("actread %lx\n", (ulong)actread);
>>>> +	return actread;
>>>> +}
>>>> +#endif
>>>> +
>>>>  int spl_load_image_fat(struct blk_desc *block_dev,
>>>>  						int partition,
>>>>  						const char *filename)
>>>> @@ -57,16 +81,29 @@ 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;
>>>> +
>>>> +		debug("Found FIT\n");
>>>> +		load.priv = (char *)filename;
>>>> +		load.bl_len = 1;
>>>> +		load.read = spl_fat_file_read;
>>>> +		spl_load_simple_fit(&load, 0, header);
>>>> +	} else {
>>>> +		debug("Legacy image\n");
>>>>  
>>>> -	err = file_fat_read(filename, (u8 *)(uintptr_t)spl_image.load_addr, 0);
>>>> +		spl_parse_image_header(header);
>>>>  
>>>> +		err = file_fat_read(filename,
>>>> +				    (u8 *)(uintptr_t)spl_image.load_addr, 0);
>>>>  end:
>>>>  #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
>>>> -	if (err <= 0)
>>>> -		printf("%s: error reading image %s, err - %d\n",
>>>> -		       __func__, filename, err);
>>>> +		if (err <= 0)
>>>> +			printf("%s: error reading image %s, err - %d\n",
>>>> +			       __func__, filename, err);
>>>>  #endif
>>>> +	}
>>>>  
>>>>  	return (err <= 0);
>>>>  }
>>>> @@ -81,6 +118,7 @@ int spl_load_image_fat_os(struct blk_desc *block_dev, int partition)
>>>>  	if (err)
>>>>  		return err;
>>>>  
>>>> +#if !defined(CONFIG_SPL_LOAD_FIT)
>>>>  #if defined(CONFIG_SPL_ENV_SUPPORT) && defined(CONFIG_SPL_OS_BOOT)
>>>>  	file = getenv("falcon_args_file");
>>>>  	if (file) {
>>>> @@ -116,7 +154,7 @@ defaults:
>>>>  #endif
>>>>  		return -1;
>>>>  	}
>>>> -
>>>> +#endif
>>>>  	return spl_load_image_fat(block_dev, partition,
>>>>  			CONFIG_SPL_FS_LOAD_KERNEL_NAME);
>>>>  }
>>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>>>> index 600a90e30922..0d987e0465ee 100644
>>>> --- a/fs/fat/fat.c
>>>> +++ b/fs/fat/fat.c
>>>> @@ -281,9 +281,9 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
>>>>  
>>>>  	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
>>>>  		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
>>>> -
>>>> +#if !defined(CONFIG_SPL_LOAD_FIT)
>>>>  		printf("FAT: Misaligned buffer address (%p)\n", buffer);
>>>> -
>>>> +#endif
>>>
>>> IMO, this is a hack. Why should fs worry about if it as fit image or
>>> not. Also the read performance will be very slow if you do not pass the
>>> aligned buffer address.  I had a different approach[1] for this: first
>>> copy the image to aligned buffer and then do a memcpy to the proper
>>> destination(which showed a better performance). May be this is wrong.
>>
>> I agree that's why this was RFC not regular patch.
>> I have looked at your solution and also Simon's comments and truth is
>> that your patch has a lot of duplicated stuff.
>> This solution is smaller.
>> Regarding buffer alignment. I think this can be simply added to read
>> function to keep it in the right place.
> 
> We don't want to take care of the alignment in read function of each fs
> type. IMHO, this should be handled by fit framework itself. I have
> updated my DMA alignment patch to take care of FS read as well[1]. Can
> you see if that helps?
> 
> [1] https://www.mail-archive.com/u-boot@lists.denx.de/msg211628.html

Look at my responses in v4. Your implementation is not working for me.

Thanks,
Michal
diff mbox

Patch

diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index d16cd540e38a..4e319c5fa470 100644
--- a/common/spl/spl_fat.c
+++ b/common/spl/spl_fat.c
@@ -13,6 +13,7 @@ 
 #include <spl.h>
 #include <asm/u-boot.h>
 #include <fat.h>
+#include <libfdt.h>
 #include <errno.h>
 #include <image.h>
 
@@ -39,6 +40,29 @@  static int spl_register_fat_device(struct blk_desc *block_dev, int partition)
 	return err;
 }
 
+#ifdef CONFIG_SPL_LOAD_FIT
+static ulong spl_fat_file_read(struct spl_load_info *load, ulong sector,
+			       ulong count, void *buf)
+{
+	int err;
+	loff_t actread;
+	char *filename = (char *)load->priv;
+
+	debug("%s: name %s, sector %lx, count %lx, buf %lx\n",
+	      __func__, filename,  sector, count, (ulong)buf);
+
+	err = file_fat_read_at(filename, sector, buf, count, &actread);
+	if (err < 0) {
+		printf("%s: error reading image %s, err - %d\n",
+		       __func__, filename, err);
+		return err;
+	}
+
+	debug("actread %lx\n", (ulong)actread);
+	return actread;
+}
+#endif
+
 int spl_load_image_fat(struct blk_desc *block_dev,
 						int partition,
 						const char *filename)
@@ -57,16 +81,29 @@  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;
+
+		debug("Found FIT\n");
+		load.priv = (char *)filename;
+		load.bl_len = 1;
+		load.read = spl_fat_file_read;
+		spl_load_simple_fit(&load, 0, header);
+	} else {
+		debug("Legacy image\n");
 
-	err = file_fat_read(filename, (u8 *)(uintptr_t)spl_image.load_addr, 0);
+		spl_parse_image_header(header);
 
+		err = file_fat_read(filename,
+				    (u8 *)(uintptr_t)spl_image.load_addr, 0);
 end:
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
-	if (err <= 0)
-		printf("%s: error reading image %s, err - %d\n",
-		       __func__, filename, err);
+		if (err <= 0)
+			printf("%s: error reading image %s, err - %d\n",
+			       __func__, filename, err);
 #endif
+	}
 
 	return (err <= 0);
 }
@@ -81,6 +118,7 @@  int spl_load_image_fat_os(struct blk_desc *block_dev, int partition)
 	if (err)
 		return err;
 
+#if !defined(CONFIG_SPL_LOAD_FIT)
 #if defined(CONFIG_SPL_ENV_SUPPORT) && defined(CONFIG_SPL_OS_BOOT)
 	file = getenv("falcon_args_file");
 	if (file) {
@@ -116,7 +154,7 @@  defaults:
 #endif
 		return -1;
 	}
-
+#endif
 	return spl_load_image_fat(block_dev, partition,
 			CONFIG_SPL_FS_LOAD_KERNEL_NAME);
 }
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 600a90e30922..0d987e0465ee 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -281,9 +281,9 @@  get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
 
 	if ((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1)) {
 		ALLOC_CACHE_ALIGN_BUFFER(__u8, tmpbuf, mydata->sect_size);
-
+#if !defined(CONFIG_SPL_LOAD_FIT)
 		printf("FAT: Misaligned buffer address (%p)\n", buffer);
-
+#endif
 		while (size >= mydata->sect_size) {
 			ret = disk_read(startsect++, 1, tmpbuf);
 			if (ret != 1) {