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

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 while (size >= mydata->sect_size) { ret = disk_read(startsect++, 1, tmpbuf); if (ret != 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

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) {

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

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

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
participants (3)
-
Lokesh Vutla
-
Michal Simek
-
Simon Glass