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

Michal Simek
2016-05-02T07:57:52Z

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