diff mbox

[U-Boot,v3] FAT: Make it possible to read from any file position

Message ID 1060843124.4677782.1347992096193.JavaMail.root@advansee.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Benoît Thébaudeau Sept. 18, 2012, 6:14 p.m. UTC 
When storage devices contain files larger than the embedded RAM, it is useful to
be able to read these files by chunks, e.g. for a software update to the
embedded NAND Flash from an external storage device (USB stick, SD card, etc.).

Hence, this patch makes it possible by adding a new FAT API to read files from a
given position. This patch also adds this feature to the fatload command.

Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
Cc: Tom Rini <trini@ti.com>
Cc: Wolfgang Denk <wd@denx.de>
---
Changes for v2:
 - Add missing variable renaming to fat_write.c.
Changes for v3:
 - Make this new feature available through the fatload command.

 .../common/cmd_fat.c                               |   25 +++--
 .../fs/fat/fat.c                                   |   98 ++++++++++++++++----
 .../fs/fat/fat_write.c                             |   18 ++--
 .../include/fat.h                                  |    2 +
 4 files changed, 105 insertions(+), 38 deletions(-)

Comments

Tom Rini Sept. 27, 2012, 4:20 p.m. UTC | #1 
On Tue, Sep 18, 2012 at 08:14:56AM -0000, Beno?t Th?baudeau wrote:

> When storage devices contain files larger than the embedded RAM, it is useful to
> be able to read these files by chunks, e.g. for a software update to the
> embedded NAND Flash from an external storage device (USB stick, SD card, etc.).
> 
> Hence, this patch makes it possible by adding a new FAT API to read files from a
> given position. This patch also adds this feature to the fatload command.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Wolfgang Denk <wd@denx.de>

With a reformatting of the commit message and making the changes
re-apply to master, I've applied this to u-boot/master, thanks!
Benoît Thébaudeau Sept. 28, 2012, 11:32 a.m. UTC | #2 
Hi Tom,

On Thursday, September 27, 2012 6:20:55 PM, Tom Rini wrote:
> On Tue, Sep 18, 2012 at 08:14:56AM -0000, Beno?t Th?baudeau wrote:
> 
> > When storage devices contain files larger than the embedded RAM, it
> > is useful to
> > be able to read these files by chunks, e.g. for a software update
> > to the
> > embedded NAND Flash from an external storage device (USB stick, SD
> > card, etc.).
> > 
> > Hence, this patch makes it possible by adding a new FAT API to read
> > files from a
> > given position. This patch also adds this feature to the fatload
> > command.
> > 
> > Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> > Cc: Tom Rini <trini@ti.com>
> > Cc: Wolfgang Denk <wd@denx.de>
> 
> With a reformatting of the commit message and making the changes
> re-apply to master, I've applied this to u-boot/master, thanks!

Perfect.

I have a concern regarding the two patches that came in between:
[1] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=cfda5aeab89d73779e26f0d34cf10f64caa67431
[2] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=10a37fd7a40826c43a63591855346adf1a1ac02d

Perhaps I'm wrong, and I admit I have not tested them, but looking at the code,
it seems to me that [2] breaks the "-" in the following feature of [1]:
"With the common function "dev:part" can come from the environment and a '-' can
be used in that case."
Hence, tests like "if (argc < 5)" in do_fat_fsload() get broken.

Also, the command usage message marks dev:part as optional, but it does not say
that this should actually be replaced by "-" if skipped.

Best regards,
Benoît
Stephen Warren Sept. 28, 2012, 3:34 p.m. UTC | #3 
On 09/28/2012 05:32 AM, Benoît Thébaudeau wrote:
> Hi Tom,
> 
> On Thursday, September 27, 2012 6:20:55 PM, Tom Rini wrote:
>> On Tue, Sep 18, 2012 at 08:14:56AM -0000, Beno?t Th?baudeau wrote:
>>
>>> When storage devices contain files larger than the embedded RAM, it
>>> is useful to
>>> be able to read these files by chunks, e.g. for a software update
>>> to the
>>> embedded NAND Flash from an external storage device (USB stick, SD
>>> card, etc.).
>>>
>>> Hence, this patch makes it possible by adding a new FAT API to read
>>> files from a
>>> given position. This patch also adds this feature to the fatload
>>> command.
>>>
>>> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
>>> Cc: Tom Rini <trini@ti.com>
>>> Cc: Wolfgang Denk <wd@denx.de>
>>
>> With a reformatting of the commit message and making the changes
>> re-apply to master, I've applied this to u-boot/master, thanks!
> 
> Perfect.
> 
> I have a concern regarding the two patches that came in between:
> [1] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=cfda5aeab89d73779e26f0d34cf10f64caa67431
> [2] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=10a37fd7a40826c43a63591855346adf1a1ac02d
> 
> Perhaps I'm wrong, and I admit I have not tested them, but looking at the code,
> it seems to me that [2] breaks the "-" in the following feature of [1]:
> "With the common function "dev:part" can come from the environment and a '-' can
> be used in that case."

I've sent a patch to fix this.
Tom Rini Sept. 28, 2012, 4:17 p.m. UTC | #4 
On Fri, Sep 28, 2012 at 09:34:42AM -0600, Stephen Warren wrote:
> On 09/28/2012 05:32 AM, Beno??t Th??baudeau wrote:
> > Hi Tom,
> > 
> > On Thursday, September 27, 2012 6:20:55 PM, Tom Rini wrote:
> >> On Tue, Sep 18, 2012 at 08:14:56AM -0000, Beno?t Th?baudeau wrote:
> >>
> >>> When storage devices contain files larger than the embedded RAM, it
> >>> is useful to
> >>> be able to read these files by chunks, e.g. for a software update
> >>> to the
> >>> embedded NAND Flash from an external storage device (USB stick, SD
> >>> card, etc.).
> >>>
> >>> Hence, this patch makes it possible by adding a new FAT API to read
> >>> files from a
> >>> given position. This patch also adds this feature to the fatload
> >>> command.
> >>>
> >>> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> >>> Cc: Tom Rini <trini@ti.com>
> >>> Cc: Wolfgang Denk <wd@denx.de>
> >>
> >> With a reformatting of the commit message and making the changes
> >> re-apply to master, I've applied this to u-boot/master, thanks!
> > 
> > Perfect.
> > 
> > I have a concern regarding the two patches that came in between:
> > [1] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=cfda5aeab89d73779e26f0d34cf10f64caa67431
> > [2] http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=10a37fd7a40826c43a63591855346adf1a1ac02d
> > 
> > Perhaps I'm wrong, and I admit I have not tested them, but looking at the code,
> > it seems to me that [2] breaks the "-" in the following feature of [1]:
> > "With the common function "dev:part" can come from the environment and a '-' can
> > be used in that case."
> 
> I've sent a patch to fix this.

Thanks guys, I was a little worried about that merge, but not as worried
as I should have been.
diff mbox

Patch

diff --git u-boot-037e9d3.orig/common/cmd_fat.c u-boot-037e9d3/common/cmd_fat.c
index 559a16d..2e34c54 100644
--- u-boot-037e9d3.orig/common/cmd_fat.c
+++ u-boot-037e9d3/common/cmd_fat.c
@@ -37,7 +37,8 @@  int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	long size;
 	unsigned long offset;
-	unsigned long count;
+	unsigned long count = 0;
+	unsigned long pos = 0;
 	char buf [12];
 	block_dev_desc_t *dev_desc=NULL;
 	int dev=0;
@@ -46,7 +47,7 @@  int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 	if (argc < 5) {
 		printf( "usage: fatload <interface> <dev[:part]> "
-			"<addr> <filename> [bytes]\n");
+			"<addr> <filename> [bytes [pos]]\n");
 		return 1;
 	}
 
@@ -69,11 +70,11 @@  int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return 1;
 	}
 	offset = simple_strtoul(argv[3], NULL, 16);
-	if (argc == 6)
+	if (argc >= 6)
 		count = simple_strtoul(argv[5], NULL, 16);
-	else
-		count = 0;
-	size = file_fat_read(argv[4], (unsigned char *)offset, count);
+	if (argc >= 7)
+		pos = simple_strtoul(argv[6], NULL, 16);
+	size = file_fat_read_at(argv[4], pos, (unsigned char *)offset, count);
 
 	if(size==-1) {
 		printf("\n** Unable to read \"%s\" from %s %d:%d **\n",
@@ -91,11 +92,15 @@  int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 
 U_BOOT_CMD(
-	fatload,	6,	0,	do_fat_fsload,
+	fatload,	7,	0,	do_fat_fsload,
 	"load binary file from a dos filesystem",
-	"<interface> <dev[:part]>  <addr> <filename> [bytes]\n"
-	"    - load binary file 'filename' from 'dev' on 'interface'\n"
-	"      to address 'addr' from dos filesystem"
+	"<interface> <dev[:part]>  <addr> <filename> [bytes [pos]]\n"
+	"    - Load binary file 'filename' from 'dev' on 'interface'\n"
+	"      to address 'addr' from dos filesystem.\n"
+	"      'pos' gives the file position to start loading from.\n"
+	"      If 'pos' is omitted, 0 is used. 'pos' requires 'bytes'.\n"
+	"      'bytes' gives the size to load. If 'bytes' is 0 or omitted,\n"
+	"      the load stops on end of file."
 );
 
 int do_fat_ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
diff --git u-boot-037e9d3.orig/fs/fat/fat.c u-boot-037e9d3/fs/fat/fat.c
index f7bb1da..c8beb30 100644
--- u-boot-037e9d3.orig/fs/fat/fat.c
+++ u-boot-037e9d3/fs/fat/fat.c
@@ -328,13 +328,16 @@  get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size)
 }
 
 /*
- * Read at most 'maxsize' bytes from the file associated with 'dentptr'
+ * Read at most 'maxsize' bytes from 'pos' in the file associated with 'dentptr'
  * into 'buffer'.
  * Return the number of bytes read or -1 on fatal errors.
  */
+__u8 get_contents_vfatname_block[MAX_CLUSTSIZE]
+	__aligned(ARCH_DMA_MINALIGN);
+
 static long
-get_contents(fsdata *mydata, dir_entry *dentptr, __u8 *buffer,
-	     unsigned long maxsize)
+get_contents(fsdata *mydata, dir_entry *dentptr, unsigned long pos,
+	     __u8 *buffer, unsigned long maxsize)
 {
 	unsigned long filesize = FAT2CPU32(dentptr->size), gotsize = 0;
 	unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
@@ -344,12 +347,59 @@  get_contents(fsdata *mydata, dir_entry *dentptr, __u8 *buffer,
 
 	debug("Filesize: %ld bytes\n", filesize);
 
-	if (maxsize > 0 && filesize > maxsize)
-		filesize = maxsize;
+	if (pos >= filesize) {
+		debug("Read position past EOF: %lu\n", pos);
+		return gotsize;
+	}
+
+	if (maxsize > 0 && filesize > pos + maxsize)
+		filesize = pos + maxsize;
 
 	debug("%ld bytes\n", filesize);
 
 	actsize = bytesperclust;
+
+	/* go to cluster at pos */
+	while (actsize <= pos) {
+		curclust = get_fatent(mydata, curclust);
+		if (CHECK_CLUST(curclust, mydata->fatsize)) {
+			debug("curclust: 0x%x\n", curclust);
+			debug("Invalid FAT entry\n");
+			return gotsize;
+		}
+		actsize += bytesperclust;
+	}
+
+	/* actsize > pos */
+	actsize -= bytesperclust;
+	filesize -= actsize;
+	pos -= actsize;
+
+	/* align to beginning of next cluster if any */
+	if (pos) {
+		actsize = min(filesize, bytesperclust);
+		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
+				(int)actsize) != 0) {
+			printf("Error reading cluster\n");
+			return -1;
+		}
+		filesize -= actsize;
+		actsize -= pos;
+		memcpy(buffer, get_contents_vfatname_block + pos, actsize);
+		gotsize += actsize;
+		if (!filesize)
+			return gotsize;
+		buffer += actsize;
+
+		curclust = get_fatent(mydata, curclust);
+		if (CHECK_CLUST(curclust, mydata->fatsize)) {
+			debug("curclust: 0x%x\n", curclust);
+			debug("Invalid FAT entry\n");
+			return gotsize;
+		}
+	}
+
+	actsize = bytesperclust;
 	endclust = curclust;
 
 	do {
@@ -433,9 +483,6 @@  static int slot2str(dir_slot *slotptr, char *l_name, int *idx)
  * into 'retdent'
  * Return 0 on success, -1 otherwise.
  */
-__u8 get_vfatname_block[MAX_CLUSTSIZE]
-	__aligned(ARCH_DMA_MINALIGN);
-
 static int
 get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
 	     dir_entry *retdent, char *l_name)
@@ -474,13 +521,13 @@  get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
 			return -1;
 		}
 
-		if (get_cluster(mydata, curclust, get_vfatname_block,
+		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
 				mydata->clust_size * mydata->sect_size) != 0) {
 			debug("Error: reading directory block\n");
 			return -1;
 		}
 
-		slotptr2 = (dir_slot *)get_vfatname_block;
+		slotptr2 = (dir_slot *)get_contents_vfatname_block;
 		while (counter > 0) {
 			if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK)
 			    & 0xff) != counter)
@@ -491,7 +538,7 @@  get_vfatname(fsdata *mydata, int curclust, __u8 *cluster,
 
 		/* Save the real directory entry */
 		realdent = (dir_entry *)slotptr2;
-		while ((__u8 *)slotptr2 > get_vfatname_block) {
+		while ((__u8 *)slotptr2 > get_contents_vfatname_block) {
 			slotptr2--;
 			slot2str(slotptr2, l_name, &idx);
 		}
@@ -770,11 +817,12 @@  exit:
 	return ret;
 }
 
-__u8 do_fat_read_block[MAX_CLUSTSIZE]
+__u8 do_fat_read_at_block[MAX_CLUSTSIZE]
 	__aligned(ARCH_DMA_MINALIGN);
 
 long
-do_fat_read(const char *filename, void *buffer, unsigned long maxsize, int dols)
+do_fat_read_at(const char *filename, unsigned long pos, void *buffer,
+	       unsigned long maxsize, int dols)
 {
 	char fnamecopy[2048];
 	boot_sector bs;
@@ -888,12 +936,12 @@  do_fat_read(const char *filename, void *buffer, unsigned long maxsize, int dols)
 					(mydata->fatsize == 32) ?
 					(mydata->clust_size) :
 					PREFETCH_BLOCKS,
-					do_fat_read_block) < 0) {
+					do_fat_read_at_block) < 0) {
 				debug("Error: reading rootdir block\n");
 				goto exit;
 			}
 
-			dentptr = (dir_entry *) do_fat_read_block;
+			dentptr = (dir_entry *) do_fat_read_at_block;
 		}
 
 		for (i = 0; i < DIRENTSPERBLOCK; i++) {
@@ -913,7 +961,7 @@  do_fat_read(const char *filename, void *buffer, unsigned long maxsize, int dols)
 
 					get_vfatname(mydata,
 						     root_cluster,
-						     do_fat_read_block,
+						     do_fat_read_at_block,
 						     dentptr, l_name);
 
 					if (dols == LS_ROOT) {
@@ -1116,7 +1164,7 @@  rootdir_done:
 		}
 	}
 
-	ret = get_contents(mydata, dentptr, buffer, maxsize);
+	ret = get_contents(mydata, dentptr, pos, buffer, maxsize);
 	debug("Size: %d, got: %ld\n", FAT2CPU32(dentptr->size), ret);
 
 exit:
@@ -1124,6 +1172,12 @@  exit:
 	return ret;
 }
 
+long
+do_fat_read(const char *filename, void *buffer, unsigned long maxsize, int dols)
+{
+	return do_fat_read_at(filename, 0, buffer, maxsize, dols);
+}
+
 int file_fat_detectfs(void)
 {
 	boot_sector bs;
@@ -1192,8 +1246,14 @@  int file_fat_ls(const char *dir)
 	return do_fat_read(dir, NULL, 0, LS_YES);
 }
 
-long file_fat_read(const char *filename, void *buffer, unsigned long maxsize)
+long file_fat_read_at(const char *filename, unsigned long pos, void *buffer,
+		      unsigned long maxsize)
 {
 	printf("reading %s\n", filename);
-	return do_fat_read(filename, buffer, maxsize, LS_NO);
+	return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO);
+}
+
+long file_fat_read(const char *filename, void *buffer, unsigned long maxsize)
+{
+	return file_fat_read_at(filename, 0, buffer, maxsize);
 }
diff --git u-boot-037e9d3.orig/fs/fat/fat_write.c u-boot-037e9d3/fs/fat/fat_write.c
index a6181e7..5829adf 100644
--- u-boot-037e9d3.orig/fs/fat/fat_write.c
+++ u-boot-037e9d3/fs/fat/fat_write.c
@@ -328,7 +328,7 @@  static void flush_dir_table(fsdata *mydata, dir_entry **dentptr);
 static void
 fill_dir_slot(fsdata *mydata, dir_entry **dentptr, const char *l_name)
 {
-	dir_slot *slotptr = (dir_slot *)get_vfatname_block;
+	dir_slot *slotptr = (dir_slot *)get_contents_vfatname_block;
 	__u8 counter = 0, checksum;
 	int idx = 0, ret;
 	char s_name[16];
@@ -373,7 +373,7 @@  static __u32 dir_curclust;
  * a slot) into 'l_name'. If successful also copy the real directory entry
  * into 'retdent'
  * If additional adjacent cluster for directory entries is read into memory,
- * then 'get_vfatname_block' is copied into 'get_dentfromdir_block' and
+ * then 'get_contents_vfatname_block' is copied into 'get_dentfromdir_block' and
  * the location of the real directory entry is returned by 'retdent'
  * Return 0 on success, -1 otherwise.
  */
@@ -416,13 +416,13 @@  get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster,
 
 		dir_curclust = curclust;
 
-		if (get_cluster(mydata, curclust, get_vfatname_block,
+		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
 				mydata->clust_size * mydata->sect_size) != 0) {
 			debug("Error: reading directory block\n");
 			return -1;
 		}
 
-		slotptr2 = (dir_slot *)get_vfatname_block;
+		slotptr2 = (dir_slot *)get_contents_vfatname_block;
 		while (counter > 0) {
 			if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK)
 			    & 0xff) != counter)
@@ -433,7 +433,7 @@  get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster,
 
 		/* Save the real directory entry */
 		realdent = (dir_entry *)slotptr2;
-		while ((__u8 *)slotptr2 > get_vfatname_block) {
+		while ((__u8 *)slotptr2 > get_contents_vfatname_block) {
 			slotptr2--;
 			slot2str(slotptr2, l_name, &idx);
 		}
@@ -459,9 +459,9 @@  get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster,
 	*retdent = realdent;
 
 	if (slotptr2) {
-		memcpy(get_dentfromdir_block, get_vfatname_block,
+		memcpy(get_dentfromdir_block, get_contents_vfatname_block,
 			mydata->clust_size * mydata->sect_size);
-		cur_position = (__u8 *)realdent - get_vfatname_block;
+		cur_position = (__u8 *)realdent - get_contents_vfatname_block;
 		*retdent = (dir_entry *) &get_dentfromdir_block[cur_position];
 	}
 
@@ -980,11 +980,11 @@  static int do_fat_write(const char *filename, void *buffer,
 	if (disk_read(cursect,
 		(mydata->fatsize == 32) ?
 		(mydata->clust_size) :
-		PREFETCH_BLOCKS, do_fat_read_block) < 0) {
+		PREFETCH_BLOCKS, do_fat_read_at_block) < 0) {
 		debug("Error: reading rootdir block\n");
 		goto exit;
 	}
-	dentptr = (dir_entry *) do_fat_read_block;
+	dentptr = (dir_entry *) do_fat_read_at_block;
 
 	name_len = strlen(filename);
 	if (name_len >= VFAT_MAXLEN_BYTES)
diff --git u-boot-037e9d3.orig/include/fat.h u-boot-037e9d3/include/fat.h
index f1b4a0d..cc85b06 100644
--- u-boot-037e9d3.orig/include/fat.h
+++ u-boot-037e9d3/include/fat.h
@@ -208,6 +208,8 @@  file_read_func		file_fat_read;
 int file_cd(const char *path);
 int file_fat_detectfs(void);
 int file_fat_ls(const char *dir);
+long file_fat_read_at(const char *filename, unsigned long pos, void *buffer,
+		      unsigned long maxsize);
 long file_fat_read(const char *filename, void *buffer, unsigned long maxsize);
 const char *file_getfsname(int idx);
 int fat_register_device(block_dev_desc_t *dev_desc, int part_no);