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 <[email protected]>

---

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

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 <[email protected]>

---

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

common/spl/spl_fat.c +44 -6

index d16cd540e38a..4e319c5fa470 100644
@@ -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);
 }

fs/fat/fat.c +0 -0

index 600a90e30922..0d987e0465ee 100644

fs/fat/fat.c +0 -0

 		while (size >= mydata->sect_size) {
 			ret = disk_read(startsect++, 1, tmpbuf);
 			if (ret != 1) {

Hi Michal,

Quoted 111 lines · Michal Simek

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 <[email protected]>

---

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

common/spl/spl_fat.c +44 -6

index d16cd540e38a..4e319c5fa470 100644
@@ -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);
 }

fs/fat/fat.c +0 -0

index 600a90e30922..0d987e0465ee 100644
@@ -281,9 +281,9 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
 

fs/fat/fat.c get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size) · line 286

Quoted 7 lines

fs/fat/fat.c +2 -2

 	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.

Quoted 114 lines · Lokesh Vutla

Hi Michal,

Quoted 111 lines · Michal Simek

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 <[email protected]>

---

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

common/spl/spl_fat.c +44 -6

index d16cd540e38a..4e319c5fa470 100644
@@ -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);
 }

fs/fat/fat.c +0 -0

index 600a90e30922..0d987e0465ee 100644
@@ -281,9 +281,9 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
 

Quoted 13 lines

fs/fat/fat.c get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size) · line 286

Quoted 7 lines

fs/fat/fat.c +2 -2

 	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

Quoted 129 lines · Michal Simek

Quoted 114 lines · Lokesh Vutla

Hi Michal,

Quoted 111 lines · Michal Simek

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 <[email protected]>

---

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

common/spl/spl_fat.c +44 -6

index d16cd540e38a..4e319c5fa470 100644
@@ -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);
 }

fs/fat/fat.c +0 -0

index 600a90e30922..0d987e0465ee 100644
@@ -281,9 +281,9 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
 

Quoted 13 lines

fs/fat/fat.c get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size) · line 286

Quoted 7 lines

fs/fat/fat.c +2 -2

 	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.

Quoted 6 lines

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 at lists.denx.de/msg211628.html