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

[Suriyan Ramasami]

Hello Pavel,

On Tue, Nov 4, 2014 at 11:26 AM, Pavel Machek <pavel at 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 at 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