diff mbox

[U-Boot,v6,2/6] fs: interface changes to accomodate files greater than 2GB

Message ID 1415069408-2978-3-git-send-email-suriyan.r@gmail.com
State Changes Requested
Delegated to: Simon Glass
Headers show

Commit Message

Suriyan Ramasami Nov. 4, 2014, 2:49 a.m. UTC
Change the interface for the generic FS functions to take in an extra
parameter of type "loff_t *" to return the size. The return values of
these funtions now serve as an indicator of error conditions alone.

Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com>
---

Changes in v6:
* Simon - Split this into a separate patch

Changes in v5:
* Simon - update fs.h with comments for fs_read/fs_write/fs_size

 common/cmd_fs.c | 17 +++++++++++++
 fs/fs.c         | 77 ++++++++++++++++++++++++++++++++++-----------------------
 include/fs.h    | 41 ++++++++++++++++++------------
 3 files changed, 88 insertions(+), 47 deletions(-)

Comments

Pavel Machek Nov. 4, 2014, 7:26 p.m. UTC | #1
On Mon 2014-11-03 18:49:58, Suriyan Ramasami wrote:
> Change the interface for the generic FS functions to take in an extra
> parameter of type "loff_t *" to return the size. The return values of
> these funtions now serve as an indicator of error conditions alone.
> 
> Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com>
> ---
> 
> Changes in v6:
> * Simon - Split this into a separate patch
> 
> Changes in v5:
> * Simon - update fs.h with comments for fs_read/fs_write/fs_size
> 
>  common/cmd_fs.c | 17 +++++++++++++
>  fs/fs.c         | 77 ++++++++++++++++++++++++++++++++++-----------------------
>  include/fs.h    | 41 ++++++++++++++++++------------
>  3 files changed, 88 insertions(+), 47 deletions(-)
> 
> diff --git a/common/cmd_fs.c b/common/cmd_fs.c
> index 6754340..f70cb8a 100644
> --- a/common/cmd_fs.c
> +++ b/common/cmd_fs.c
> @@ -51,6 +51,23 @@ U_BOOT_CMD(
>  	"      If 'pos' is 0 or omitted, the file is read from the start."
>  )
>  
> +static int do_save_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
> +				char * const argv[])
> +{
> +	return do_save(cmdtp, flag, argc, argv, FS_TYPE_ANY);
> +}
> +
> +U_BOOT_CMD(
> +	save,	7,	0,	do_save_wrapper,
> +	"save file to a filesystem",
> +	"<interface> [<dev[:part]> [<addr> [<filename> [bytes [pos]]]]]\n"

That's not correct, AFAICT.

int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
                int fstype)
{
	unsigned long addr;
	const char *filename;
        unsigned long bytes;
	unsigned long pos;
	int len;
        unsigned long time;

	if (argc < 6 || argc > 7)
                return CMD_RET_USAGE;

More arguments are mandatory then you describe here.

> +	"    - Save binary file 'filename' to partition 'part' on device\n"
> +	"      type 'interface' instance 'dev' from addr 'addr' in memory.\n"
> +	"      'bytes' gives the size to save in bytes and is mandatory.\n"
> +	"      'pos' gives the file byte position to start writing to.\n"
> +	"      If 'pos' is 0 or omitted, the file is written from the start."

Plus maybe note if it can extend existing files and create files if
they don't exit would be nice here?
Suriyan Ramasami Nov. 4, 2014, 9:39 p.m. UTC | #2
Hello Pavel,

On Tue, Nov 4, 2014 at 11:26 AM, Pavel Machek <pavel@denx.de> wrote:
> On Mon 2014-11-03 18:49:58, Suriyan Ramasami wrote:
>> Change the interface for the generic FS functions to take in an extra
>> parameter of type "loff_t *" to return the size. The return values of
>> these funtions now serve as an indicator of error conditions alone.
>>
>> Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com>
>> ---
>>
>> Changes in v6:
>> * Simon - Split this into a separate patch
>>
>> Changes in v5:
>> * Simon - update fs.h with comments for fs_read/fs_write/fs_size
>>
>>  common/cmd_fs.c | 17 +++++++++++++
>>  fs/fs.c         | 77 ++++++++++++++++++++++++++++++++++-----------------------
>>  include/fs.h    | 41 ++++++++++++++++++------------
>>  3 files changed, 88 insertions(+), 47 deletions(-)
>>
>> diff --git a/common/cmd_fs.c b/common/cmd_fs.c
>> index 6754340..f70cb8a 100644
>> --- a/common/cmd_fs.c
>> +++ b/common/cmd_fs.c
>> @@ -51,6 +51,23 @@ U_BOOT_CMD(
>>       "      If 'pos' is 0 or omitted, the file is read from the start."
>>  )
>>
>> +static int do_save_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
>> +                             char * const argv[])
>> +{
>> +     return do_save(cmdtp, flag, argc, argv, FS_TYPE_ANY);
>> +}
>> +
>> +U_BOOT_CMD(
>> +     save,   7,      0,      do_save_wrapper,
>> +     "save file to a filesystem",
>> +     "<interface> [<dev[:part]> [<addr> [<filename> [bytes [pos]]]]]\n"
>
> That's not correct, AFAICT.
>
> int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>                 int fstype)
> {
>         unsigned long addr;
>         const char *filename;
>         unsigned long bytes;
>         unsigned long pos;
>         int len;
>         unsigned long time;
>
>         if (argc < 6 || argc > 7)
>                 return CMD_RET_USAGE;
>
> More arguments are mandatory then you describe here.
>

You are correct. All the way upto "bytes" is mandatory. I shall change
the arguments as follows:
"<interface> <dev:part> <addr> <filename> <bytes> [pos]\n"

>> +     "    - Save binary file 'filename' to partition 'part' on device\n"
>> +     "      type 'interface' instance 'dev' from addr 'addr' in memory.\n"
>> +     "      'bytes' gives the size to save in bytes and is mandatory.\n"
>> +     "      'pos' gives the file byte position to start writing to.\n"
>> +     "      If 'pos' is 0 or omitted, the file is written from the start."
>
> Plus maybe note if it can extend existing files and create files if
> they don't exit would be nice here?
>
As this is the generic interface and interacts with fat/ext/sandbox,
it depends on their respective implementation. For example a non zero
"pos" is not yet handled in ext4, whereas its supported in "fat".
Hence, I wonder if its apt at this stage to make those statements.

Thanks for the comments!
- Suriyan

> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Simon Glass Nov. 5, 2014, 12:49 a.m. UTC | #3
Hi,

On 3 November 2014 18:49, Suriyan Ramasami <suriyan.r@gmail.com> wrote:
> Change the interface for the generic FS functions to take in an extra
> parameter of type "loff_t *" to return the size. The return values of
> these funtions now serve as an indicator of error conditions alone.
>
> Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com>

This looks good apart from Pavel's comments and my two minor nits below.

> ---
>
> Changes in v6:
> * Simon - Split this into a separate patch
>
> Changes in v5:
> * Simon - update fs.h with comments for fs_read/fs_write/fs_size
>
>  common/cmd_fs.c | 17 +++++++++++++
>  fs/fs.c         | 77 ++++++++++++++++++++++++++++++++++-----------------------
>  include/fs.h    | 41 ++++++++++++++++++------------
>  3 files changed, 88 insertions(+), 47 deletions(-)
>
> diff --git a/common/cmd_fs.c b/common/cmd_fs.c
> index 6754340..f70cb8a 100644
> --- a/common/cmd_fs.c
> +++ b/common/cmd_fs.c
> @@ -51,6 +51,23 @@ U_BOOT_CMD(
>         "      If 'pos' is 0 or omitted, the file is read from the start."
>  )
>
> +static int do_save_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
> +                               char * const argv[])
> +{
> +       return do_save(cmdtp, flag, argc, argv, FS_TYPE_ANY);
> +}
> +
> +U_BOOT_CMD(
> +       save,   7,      0,      do_save_wrapper,
> +       "save file to a filesystem",
> +       "<interface> [<dev[:part]> [<addr> [<filename> [bytes [pos]]]]]\n"
> +       "    - Save binary file 'filename' to partition 'part' on device\n"
> +       "      type 'interface' instance 'dev' from addr 'addr' in memory.\n"
> +       "      'bytes' gives the size to save in bytes and is mandatory.\n"
> +       "      'pos' gives the file byte position to start writing to.\n"
> +       "      If 'pos' is 0 or omitted, the file is written from the start."
> +)
> +
>  static int do_ls_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
>                                 char * const argv[])
>  {
> diff --git a/fs/fs.c b/fs/fs.c
> index dd680f3..0f5a1f4 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -46,19 +46,21 @@ static inline int fs_exists_unsupported(const char *filename)
>         return 0;
>  }
>
> -static inline int fs_size_unsupported(const char *filename)
> +static inline int fs_size_unsupported(const char *filename, loff_t *size)
>  {
>         return -1;
>  }
>
>  static inline int fs_read_unsupported(const char *filename, void *buf,
> -                                     int offset, int len)
> +                                     loff_t offset, loff_t len,
> +                                     loff_t *actread)
>  {
>         return -1;
>  }
>
>  static inline int fs_write_unsupported(const char *filename, void *buf,
> -                                     int offset, int len)
> +                                     loff_t offset, loff_t len,
> +                                     loff_t *actwrite)
>  {
>         return -1;
>  }
> @@ -82,9 +84,11 @@ struct fstype_info {
>                      disk_partition_t *fs_partition);
>         int (*ls)(const char *dirname);
>         int (*exists)(const char *filename);
> -       int (*size)(const char *filename);
> -       int (*read)(const char *filename, void *buf, int offset, int len);
> -       int (*write)(const char *filename, void *buf, int offset, int len);
> +       int (*size)(const char *filename, loff_t *size);
> +       int (*read)(const char *filename, void *buf, loff_t offset,
> +                   loff_t len, loff_t *actread);
> +       int (*write)(const char *filename, void *buf, loff_t offset,
> +                    loff_t len, loff_t *actwrite);
>         void (*close)(void);
>  };
>
> @@ -99,7 +103,11 @@ static struct fstype_info fstypes[] = {
>                 .exists = fat_exists,
>                 .size = fat_size,
>                 .read = fat_read_file,
> +#ifdef CONFIG_FAT_WRITE
> +               .write = file_fat_write,
> +#else
>                 .write = fs_write_unsupported,
> +#endif
>         },
>  #endif
>  #ifdef CONFIG_FS_EXT4
> @@ -112,7 +120,11 @@ static struct fstype_info fstypes[] = {
>                 .exists = ext4fs_exists,
>                 .size = ext4fs_size,
>                 .read = ext4_read_file,
> +#ifdef CONFIG_CMD_EXT4_WRITE
> +               .write = ext4_write_file,
> +#else
>                 .write = fs_write_unsupported,
> +#endif
>         },
>  #endif
>  #ifdef CONFIG_SANDBOX
> @@ -233,20 +245,21 @@ int fs_exists(const char *filename)
>         return ret;
>  }
>
> -int fs_size(const char *filename)
> +int fs_size(const char *filename, loff_t *size)
>  {
>         int ret;
>
>         struct fstype_info *info = fs_get_info(fs_type);
>
> -       ret = info->size(filename);
> +       ret = info->size(filename, size);
>
>         fs_close();
>
>         return ret;
>  }
>
> -int fs_read(const char *filename, ulong addr, int offset, int len)
> +int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
> +           loff_t *actread)
>  {
>         struct fstype_info *info = fs_get_info(fs_type);
>         void *buf;
> @@ -257,11 +270,11 @@ int fs_read(const char *filename, ulong addr, int offset, int len)
>          * means read the whole file.
>          */
>         buf = map_sysmem(addr, len);
> -       ret = info->read(filename, buf, offset, len);
> +       ret = info->read(filename, buf, offset, len, actread);
>         unmap_sysmem(buf);
>
>         /* If we requested a specific number of bytes, check we got it */
> -       if (ret >= 0 && len && ret != len) {
> +       if (ret == 0 && len && *actread != len) {
>                 printf("** Unable to read file %s **\n", filename);
>                 ret = -1;
>         }
> @@ -270,17 +283,18 @@ int fs_read(const char *filename, ulong addr, int offset, int len)
>         return ret;
>  }
>
> -int fs_write(const char *filename, ulong addr, int offset, int len)
> +int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
> +            loff_t *actwrite)
>  {
>         struct fstype_info *info = fs_get_info(fs_type);
>         void *buf;
>         int ret;
>
>         buf = map_sysmem(addr, len);
> -       ret = info->write(filename, buf, offset, len);
> +       ret = info->write(filename, buf, offset, len, actwrite);
>         unmap_sysmem(buf);
>
> -       if (ret >= 0 && ret != len) {
> +       if (ret < 0 && len != *actwrite) {
>                 printf("** Unable to write file %s **\n", filename);
>                 ret = -1;
>         }
> @@ -292,7 +306,7 @@ int fs_write(const char *filename, ulong addr, int offset, int len)
>  int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>                 int fstype)
>  {
> -       int size;
> +       loff_t size;
>
>         if (argc != 4)
>                 return CMD_RET_USAGE;
> @@ -300,8 +314,7 @@ int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>         if (fs_set_blk_dev(argv[1], argv[2], fstype))
>                 return 1;
>
> -       size = fs_size(argv[3]);
> -       if (size < 0)
> +       if (fs_size(argv[3], &size) < 0)
>                 return CMD_RET_FAILURE;
>
>         setenv_hex("filesize", size);
> @@ -315,9 +328,10 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>         unsigned long addr;
>         const char *addr_str;
>         const char *filename;
> -       unsigned long bytes;
> -       unsigned long pos;
> -       int len_read;
> +       loff_t bytes;
> +       loff_t pos;
> +       loff_t len_read;
> +       int ret;
>         unsigned long time;
>         char *ep;
>
> @@ -359,12 +373,12 @@ int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>                 pos = 0;
>
>         time = get_timer(0);
> -       len_read = fs_read(filename, addr, pos, bytes);
> +       ret = fs_read(filename, addr, pos, bytes, &len_read);
>         time = get_timer(time);
> -       if (len_read <= 0)
> +       if (ret < 0)
>                 return 1;
>
> -       printf("%d bytes read in %lu ms", len_read, time);
> +       printf("%llu bytes read in %lu ms", len_read, time);
>         if (time > 0) {
>                 puts(" (");
>                 print_size(len_read / time * 1000, "/s");
> @@ -408,9 +422,10 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>  {
>         unsigned long addr;
>         const char *filename;
> -       unsigned long bytes;
> -       unsigned long pos;
> -       int len;
> +       loff_t bytes;
> +       loff_t pos;
> +       loff_t len;
> +       int ret;
>         unsigned long time;
>
>         if (argc < 6 || argc > 7)
> @@ -419,8 +434,8 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>         if (fs_set_blk_dev(argv[1], argv[2], fstype))
>                 return 1;
>
> -       filename = argv[3];
> -       addr = simple_strtoul(argv[4], NULL, 16);
> +       addr = simple_strtoul(argv[3], NULL, 16);
> +       filename = argv[4];
>         bytes = simple_strtoul(argv[5], NULL, 16);
>         if (argc >= 7)
>                 pos = simple_strtoul(argv[6], NULL, 16);
> @@ -428,12 +443,12 @@ int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>                 pos = 0;
>
>         time = get_timer(0);
> -       len = fs_write(filename, addr, pos, bytes);
> +       ret = fs_write(filename, addr, pos, bytes, &len);
>         time = get_timer(time);
> -       if (len <= 0)
> +       if (ret < 0)
>                 return 1;
>
> -       printf("%d bytes written in %lu ms", len, time);
> +       printf("%llu bytes written in %lu ms", len, time);
>         if (time > 0) {
>                 puts(" (");
>                 print_size(len / time * 1000, "/s");
> diff --git a/include/fs.h b/include/fs.h
> index 06a45f2..6bd17c8 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -51,32 +51,41 @@ int fs_ls(const char *dirname);
>  int fs_exists(const char *filename);
>
>  /*
> - * Determine a file's size
> + * fs_size - Determine a file's size
>   *
> - * Returns the file's size in bytes, or a negative value if it doesn't exist.
> + * @filename: Name of the file
> + * @size: Size of file
> + * @return 0 if ok with valid *size, negative on error
>   */
> -int fs_size(const char *filename);
> +int fs_size(const char *filename, loff_t *size);
>
>  /*
> - * Read file "filename" from the partition previously set by fs_set_blk_dev(),
> - * to address "addr", starting at byte offset "offset", and reading "len"
> - * bytes. "offset" may be 0 to read from the start of the file. "len" may be
> - * 0 to read the entire file. Note that not all filesystem types support
> - * either/both offset!=0 or len!=0.
> + * fs_read - Read file from the partition previously set by fs_set_blk_dev()
> + * Note that not all filesystem types support either/both offset!=0 or len!=0.
>   *
> - * Returns number of bytes read on success. Returns <= 0 on error.
> + * @filename: Name of file to read from
> + * @addr: The address to read into
> + * @offset: The offset in file to read from
> + * @len: The number of bytes to read. Maybe 0 to read entire file
> + * @actread: The actual number of bytes read

Returns the actual number...

> + * @return 0 if ok with valid *actread, -1 on error conditions
>   */
> -int fs_read(const char *filename, ulong addr, int offset, int len);
> +int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
> +           loff_t *actread);
>
>  /*
> - * Write file "filename" to the partition previously set by fs_set_blk_dev(),
> - * from address "addr", starting at byte offset "offset", and writing "len"
> - * bytes. "offset" may be 0 to write to the start of the file. Note that not
> - * all filesystem types support offset!=0.
> + * fs_write - Write file to the partition previously set by fs_set_blk_dev()
> + * Note that not all filesystem types support offset!=0.
>   *
> - * Returns number of bytes read on success. Returns <= 0 on error.
> + * @filename: Name of file to read from
> + * @addr: The address to read into
> + * @offset: The offset in file to read from. Maybe 0 to write to start of file
> + * @len: The number of bytes to write
> + * @actwrite: The actual number of bytes written

Returns the actual number...

> + * @return 0 if ok with valid *actwrite, -1 on error conditions
>   */
> -int fs_write(const char *filename, ulong addr, int offset, int len);
> +int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
> +            loff_t *actwrite);
>
>  /*
>   * Common implementation for various filesystem commands, optionally limited
> --
> 1.9.1
>

Regards,
Simon
diff mbox

Patch

diff --git a/common/cmd_fs.c b/common/cmd_fs.c
index 6754340..f70cb8a 100644
--- a/common/cmd_fs.c
+++ b/common/cmd_fs.c
@@ -51,6 +51,23 @@  U_BOOT_CMD(
 	"      If 'pos' is 0 or omitted, the file is read from the start."
 )
 
+static int do_save_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
+				char * const argv[])
+{
+	return do_save(cmdtp, flag, argc, argv, FS_TYPE_ANY);
+}
+
+U_BOOT_CMD(
+	save,	7,	0,	do_save_wrapper,
+	"save file to a filesystem",
+	"<interface> [<dev[:part]> [<addr> [<filename> [bytes [pos]]]]]\n"
+	"    - Save binary file 'filename' to partition 'part' on device\n"
+	"      type 'interface' instance 'dev' from addr 'addr' in memory.\n"
+	"      'bytes' gives the size to save in bytes and is mandatory.\n"
+	"      'pos' gives the file byte position to start writing to.\n"
+	"      If 'pos' is 0 or omitted, the file is written from the start."
+)
+
 static int do_ls_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
 				char * const argv[])
 {
diff --git a/fs/fs.c b/fs/fs.c
index dd680f3..0f5a1f4 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -46,19 +46,21 @@  static inline int fs_exists_unsupported(const char *filename)
 	return 0;
 }
 
-static inline int fs_size_unsupported(const char *filename)
+static inline int fs_size_unsupported(const char *filename, loff_t *size)
 {
 	return -1;
 }
 
 static inline int fs_read_unsupported(const char *filename, void *buf,
-				      int offset, int len)
+				      loff_t offset, loff_t len,
+				      loff_t *actread)
 {
 	return -1;
 }
 
 static inline int fs_write_unsupported(const char *filename, void *buf,
-				      int offset, int len)
+				      loff_t offset, loff_t len,
+				      loff_t *actwrite)
 {
 	return -1;
 }
@@ -82,9 +84,11 @@  struct fstype_info {
 		     disk_partition_t *fs_partition);
 	int (*ls)(const char *dirname);
 	int (*exists)(const char *filename);
-	int (*size)(const char *filename);
-	int (*read)(const char *filename, void *buf, int offset, int len);
-	int (*write)(const char *filename, void *buf, int offset, int len);
+	int (*size)(const char *filename, loff_t *size);
+	int (*read)(const char *filename, void *buf, loff_t offset,
+		    loff_t len, loff_t *actread);
+	int (*write)(const char *filename, void *buf, loff_t offset,
+		     loff_t len, loff_t *actwrite);
 	void (*close)(void);
 };
 
@@ -99,7 +103,11 @@  static struct fstype_info fstypes[] = {
 		.exists = fat_exists,
 		.size = fat_size,
 		.read = fat_read_file,
+#ifdef CONFIG_FAT_WRITE
+		.write = file_fat_write,
+#else
 		.write = fs_write_unsupported,
+#endif
 	},
 #endif
 #ifdef CONFIG_FS_EXT4
@@ -112,7 +120,11 @@  static struct fstype_info fstypes[] = {
 		.exists = ext4fs_exists,
 		.size = ext4fs_size,
 		.read = ext4_read_file,
+#ifdef CONFIG_CMD_EXT4_WRITE
+		.write = ext4_write_file,
+#else
 		.write = fs_write_unsupported,
+#endif
 	},
 #endif
 #ifdef CONFIG_SANDBOX
@@ -233,20 +245,21 @@  int fs_exists(const char *filename)
 	return ret;
 }
 
-int fs_size(const char *filename)
+int fs_size(const char *filename, loff_t *size)
 {
 	int ret;
 
 	struct fstype_info *info = fs_get_info(fs_type);
 
-	ret = info->size(filename);
+	ret = info->size(filename, size);
 
 	fs_close();
 
 	return ret;
 }
 
-int fs_read(const char *filename, ulong addr, int offset, int len)
+int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
+	    loff_t *actread)
 {
 	struct fstype_info *info = fs_get_info(fs_type);
 	void *buf;
@@ -257,11 +270,11 @@  int fs_read(const char *filename, ulong addr, int offset, int len)
 	 * means read the whole file.
 	 */
 	buf = map_sysmem(addr, len);
-	ret = info->read(filename, buf, offset, len);
+	ret = info->read(filename, buf, offset, len, actread);
 	unmap_sysmem(buf);
 
 	/* If we requested a specific number of bytes, check we got it */
-	if (ret >= 0 && len && ret != len) {
+	if (ret == 0 && len && *actread != len) {
 		printf("** Unable to read file %s **\n", filename);
 		ret = -1;
 	}
@@ -270,17 +283,18 @@  int fs_read(const char *filename, ulong addr, int offset, int len)
 	return ret;
 }
 
-int fs_write(const char *filename, ulong addr, int offset, int len)
+int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
+	     loff_t *actwrite)
 {
 	struct fstype_info *info = fs_get_info(fs_type);
 	void *buf;
 	int ret;
 
 	buf = map_sysmem(addr, len);
-	ret = info->write(filename, buf, offset, len);
+	ret = info->write(filename, buf, offset, len, actwrite);
 	unmap_sysmem(buf);
 
-	if (ret >= 0 && ret != len) {
+	if (ret < 0 && len != *actwrite) {
 		printf("** Unable to write file %s **\n", filename);
 		ret = -1;
 	}
@@ -292,7 +306,7 @@  int fs_write(const char *filename, ulong addr, int offset, int len)
 int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		int fstype)
 {
-	int size;
+	loff_t size;
 
 	if (argc != 4)
 		return CMD_RET_USAGE;
@@ -300,8 +314,7 @@  int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 	if (fs_set_blk_dev(argv[1], argv[2], fstype))
 		return 1;
 
-	size = fs_size(argv[3]);
-	if (size < 0)
+	if (fs_size(argv[3], &size) < 0)
 		return CMD_RET_FAILURE;
 
 	setenv_hex("filesize", size);
@@ -315,9 +328,10 @@  int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 	unsigned long addr;
 	const char *addr_str;
 	const char *filename;
-	unsigned long bytes;
-	unsigned long pos;
-	int len_read;
+	loff_t bytes;
+	loff_t pos;
+	loff_t len_read;
+	int ret;
 	unsigned long time;
 	char *ep;
 
@@ -359,12 +373,12 @@  int do_load(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		pos = 0;
 
 	time = get_timer(0);
-	len_read = fs_read(filename, addr, pos, bytes);
+	ret = fs_read(filename, addr, pos, bytes, &len_read);
 	time = get_timer(time);
-	if (len_read <= 0)
+	if (ret < 0)
 		return 1;
 
-	printf("%d bytes read in %lu ms", len_read, time);
+	printf("%llu bytes read in %lu ms", len_read, time);
 	if (time > 0) {
 		puts(" (");
 		print_size(len_read / time * 1000, "/s");
@@ -408,9 +422,10 @@  int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 {
 	unsigned long addr;
 	const char *filename;
-	unsigned long bytes;
-	unsigned long pos;
-	int len;
+	loff_t bytes;
+	loff_t pos;
+	loff_t len;
+	int ret;
 	unsigned long time;
 
 	if (argc < 6 || argc > 7)
@@ -419,8 +434,8 @@  int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 	if (fs_set_blk_dev(argv[1], argv[2], fstype))
 		return 1;
 
-	filename = argv[3];
-	addr = simple_strtoul(argv[4], NULL, 16);
+	addr = simple_strtoul(argv[3], NULL, 16);
+	filename = argv[4];
 	bytes = simple_strtoul(argv[5], NULL, 16);
 	if (argc >= 7)
 		pos = simple_strtoul(argv[6], NULL, 16);
@@ -428,12 +443,12 @@  int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 		pos = 0;
 
 	time = get_timer(0);
-	len = fs_write(filename, addr, pos, bytes);
+	ret = fs_write(filename, addr, pos, bytes, &len);
 	time = get_timer(time);
-	if (len <= 0)
+	if (ret < 0)
 		return 1;
 
-	printf("%d bytes written in %lu ms", len, time);
+	printf("%llu bytes written in %lu ms", len, time);
 	if (time > 0) {
 		puts(" (");
 		print_size(len / time * 1000, "/s");
diff --git a/include/fs.h b/include/fs.h
index 06a45f2..6bd17c8 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -51,32 +51,41 @@  int fs_ls(const char *dirname);
 int fs_exists(const char *filename);
 
 /*
- * Determine a file's size
+ * fs_size - Determine a file's size
  *
- * Returns the file's size in bytes, or a negative value if it doesn't exist.
+ * @filename: Name of the file
+ * @size: Size of file
+ * @return 0 if ok with valid *size, negative on error
  */
-int fs_size(const char *filename);
+int fs_size(const char *filename, loff_t *size);
 
 /*
- * Read file "filename" from the partition previously set by fs_set_blk_dev(),
- * to address "addr", starting at byte offset "offset", and reading "len"
- * bytes. "offset" may be 0 to read from the start of the file. "len" may be
- * 0 to read the entire file. Note that not all filesystem types support
- * either/both offset!=0 or len!=0.
+ * fs_read - Read file from the partition previously set by fs_set_blk_dev()
+ * Note that not all filesystem types support either/both offset!=0 or len!=0.
  *
- * Returns number of bytes read on success. Returns <= 0 on error.
+ * @filename: Name of file to read from
+ * @addr: The address to read into
+ * @offset: The offset in file to read from
+ * @len: The number of bytes to read. Maybe 0 to read entire file
+ * @actread: The actual number of bytes read
+ * @return 0 if ok with valid *actread, -1 on error conditions
  */
-int fs_read(const char *filename, ulong addr, int offset, int len);
+int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len,
+	    loff_t *actread);
 
 /*
- * Write file "filename" to the partition previously set by fs_set_blk_dev(),
- * from address "addr", starting at byte offset "offset", and writing "len"
- * bytes. "offset" may be 0 to write to the start of the file. Note that not
- * all filesystem types support offset!=0.
+ * fs_write - Write file to the partition previously set by fs_set_blk_dev()
+ * Note that not all filesystem types support offset!=0.
  *
- * Returns number of bytes read on success. Returns <= 0 on error.
+ * @filename: Name of file to read from
+ * @addr: The address to read into
+ * @offset: The offset in file to read from. Maybe 0 to write to start of file
+ * @len: The number of bytes to write
+ * @actwrite: The actual number of bytes written
+ * @return 0 if ok with valid *actwrite, -1 on error conditions
  */
-int fs_write(const char *filename, ulong addr, int offset, int len);
+int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len,
+	     loff_t *actwrite);
 
 /*
  * Common implementation for various filesystem commands, optionally limited