Re: [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands

Hi Stephen,

On Monday, October 22, 2012 6:43:51 PM, Stephen Warren wrote:

From: Stephen Warren swarren@nvidia.com

Implement "ls" and "fsload" commands that act like {fat,ext2}{ls,load}, and transparently handle either file-system. This scheme could easily be extended to other filesystem types; I only didn't do it for zfs because I don't have any filesystems of that type to test with.

Replace the implementation of {fat,ext[24]}{ls,load} with this new code too.

Signed-off-by: Stephen Warren swarren@nvidia.com

Sorry to come after this has been applied. I've been ultra busy lately.

[--snip--]

diff --git a/fs/fs.c b/fs/fs.c new file mode 100644 index 0000000..23ffa25 --- /dev/null +++ b/fs/fs.c @@ -0,0 +1,308 @@

[--snip--]

+int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],

• int fstype)
  

+{

• unsigned long addr;
• const char *addr_str;
• const char *filename;
• unsigned long bytes;
• unsigned long pos;
• int len_read;
• char buf[12];
• if (argc < 5)

With the arguments now made optional, this should rather be: + if (argc < 2)

• return CMD_RET_USAGE;
  

• if (fs_set_blk_dev(argv[1], argv[2], fstype))

With <dev[:part]> being optional, this should rather be: + if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL, fstype))

• return 1;
  

• if (argc >= 4) {

• addr = simple_strtoul(argv[3], NULL, 0);
  

0 is just natural here. However, this raises the issue of the users of the legacy fat and ext commands, which used 16 here. So should we use 0 because it is cleaner, or 16 in order not to break compatibility for existing users?

• } else {

• addr_str = getenv("loadaddr");
  

• if (addr_str != NULL)
  

• 	addr = simple_strtoul(addr_str, NULL, 16);
  

Ditto.

• else
  

• 	addr = CONFIG_SYS_LOAD_ADDR;
  

• }
• if (argc >= 5) {

• filename = argv[4];
  

• } else {

• filename = getenv("bootfile");
  

• if (!filename) {
  

• 	puts("** No boot file defined **\n");
  

• 	return 1;
  

• }
  

• }
• if (argc >= 6)

• bytes = simple_strtoul(argv[5], NULL, 0);
  

Ditto.

• else

• bytes = 0;
  

• if (argc >= 7)

• pos = simple_strtoul(argv[6], NULL, 0);
  

Ditto.

• else

• pos = 0;
  

• len_read = fs_read(filename, addr, pos, bytes);
• if (len_read <= 0)

• return 1;
  

• printf("%d bytes read\n", len_read);
• sprintf(buf, "0x%x", len_read);
• setenv("filesize", buf);
• return 0;

+}

+int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],

• int fstype)

+{

• if (argc < 2)

• return CMD_RET_USAGE;
  

• if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL, fstype))

• return 1;
  

• if (fs_ls(argc == 4 ? argv[3] : "/"))

IMHO, it would be better to just ignore the possible extra arguments, like in: + if (fs_ls(argc >= 4 ? argv[3] : "/"))

• return 1;
  

• return 0;

+}

[--snip--]

Best regards, Benoît