[U-Boot] [PATCH V2 1/3] fs: delete unused Makefile

[U-Boot] [PATCH V2 1/3] fs: delete unused Makefile

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

fs/Makefile is unused. The top-level Makefile sets LIBS-y += fs/xxx and
hence causes make to directly descend two directory levels into each
individual filesystem, and it never descends into fs/ itself.

So, delete this useless file.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2: No change.

This series is based on all the disk/fs/FAT patches I have posted before,
as an FYI. Once I know exactly which of the bugfix patches made it into
the release, and u-boot/next works for Tegra, I can re-send a patch bomb
with all those and these patches in one go directly on top of u-boot/next
if you want.
---
 fs/Makefile |   42 ------------------------------------------
 1 files changed, 0 insertions(+), 42 deletions(-)
 delete mode 100644 fs/Makefile

diff --git a/fs/Makefile b/fs/Makefile
deleted file mode 100644
index 901e189..0000000
--- a/fs/Makefile
+++ /dev/null
@@ -1,42 +0,0 @@
-#
-# (C) Copyright 2000-2006
-# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
-#
-# See file CREDITS for list of people who contributed to this
-# project.
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License as
-# published by the Free Software Foundation; either version 2 of
-# the License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
-# MA 02111-1307 USA
-#
-#
-
-subdirs-$(CONFIG_CMD_CRAMFS) := cramfs
-subdirs-$(CONFIG_CMD_EXT4) += ext4
-ifndef CONFIG_CMD_EXT4
-subdirs-$(CONFIG_CMD_EXT2) += ext4
-endif
-subdirs-$(CONFIG_CMD_FAT) += fat
-subdirs-$(CONFIG_CMD_FDOS) += fdos
-subdirs-$(CONFIG_CMD_JFFS2) += jffs2
-subdirs-$(CONFIG_CMD_REISER) += reiserfs
-subdirs-$(CONFIG_YAFFS2) += yaffs2
-subdirs-$(CONFIG_CMD_UBIFS) += ubifs
-subdirs-$(CONFIG_CMD_ZFS) += zfs
-
-SUBDIRS	:= $(subdirs-y)
-
-$(obj).depend all:
-	@for dir in $(SUBDIRS) ; do \
-		$(MAKE) -C $$dir $@ ; done
-- 
1.7.0.4

[U-Boot] [PATCH V2 2/3] fs: separate CONFIG_FS_{FAT, EXT4} from CONFIG_CMD_{FAT, EXT*}

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

This makes the FAT and ext4 filesystem implementations build if
CONFIG_FS_{FAT,EXT4} are defined, rather than basing the build on
whether CONFIG_CMD_{FAT,EXT*} are defined. This will allow the
filesystems to be built separately from the filesystem-specific commands
that use them. This paves the way for the creation of filesystem-generic
commands that used the filesystems, without requiring the filesystem-
specific commands.

Minor documentation changes are made for this change.

The new config options are automatically selected by the old config
options to retain backwards-compatibility.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2: New patch.
---
 README                     |    4 +++-
 doc/README.ext4            |   13 +++++++++++++
 fs/ext4/Makefile           |    7 ++-----
 fs/ext4/ext4_common.c      |    2 +-
 fs/ext4/ext4_common.h      |    4 ++--
 fs/ext4/ext4fs.c           |    2 +-
 fs/fat/Makefile            |    4 ++--
 include/config_fallbacks.h |   13 +++++++++++++
 include/ext4fs.h           |    2 +-
 9 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/README b/README
index 4901710..d86068d 100644
--- a/README
+++ b/README
@@ -799,9 +799,11 @@ The following options need to be configured:
 		CONFIG_CMD_EEPROM	* EEPROM read/write support
 		CONFIG_CMD_ELF		* bootelf, bootvx
 		CONFIG_CMD_EXPORTENV	* export the environment
+		CONFIG_CMD_EXT2		* ext2 command support
+		CONFIG_CMD_EXT4		* ext4 command support
 		CONFIG_CMD_SAVEENV	  saveenv
 		CONFIG_CMD_FDC		* Floppy Disk Support
-		CONFIG_CMD_FAT		* FAT partition support
+		CONFIG_CMD_FAT		* FAT command support
 		CONFIG_CMD_FDOS		* Dos diskette Support
 		CONFIG_CMD_FLASH	  flinfo, erase, protect
 		CONFIG_CMD_FPGA		  FPGA device initialization support
diff --git a/doc/README.ext4 b/doc/README.ext4
index b3ea8b7..b7d0ad3 100644
--- a/doc/README.ext4
+++ b/doc/README.ext4
@@ -1,15 +1,28 @@
 This patch series adds support for ext4 ls,load and write features in uboot
 Journaling is supported for write feature.
 
+To enable support for the ext4 (and ext2) filesystem implementation,
+#define CONFIG_FS_EXT4
+
+If you want write support,
+#define CONFIG_EXT4_WRITE
+
 To Enable ext2 ls and load commands, modify the board specific config file with
 #define CONFIG_CMD_EXT2
+This automatically defines CONFIG_FS_EXT4 for you.
 
 To Enable ext4 ls and load commands, modify the board specific config file with
 #define CONFIG_CMD_EXT4
+This automatically defines CONFIG_FS_EXT4 for you.
 
 To enable ext4 write command, modify the board specific config file with
 #define CONFIG_CMD_EXT4
 #define CONFIG_CMD_EXT4_WRITE
+These automatically define CONFIG_FS_EXT4 and CONFIG_EXT4_WRITE for you.
+
+Also relevant are the generic filesystem commands,
+#define CONFIG_CMD_FS_GENERIC
+This does not automatically enable EXT4 support for you.
 
 Steps to test:
 
diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
index 82cd9ae..bb801f9 100644
--- a/fs/ext4/Makefile
+++ b/fs/ext4/Makefile
@@ -30,11 +30,8 @@ include $(TOPDIR)/config.mk
 LIB	= $(obj)libext4fs.o
 
 AOBJS	=
-COBJS-$(CONFIG_CMD_EXT4) := ext4fs.o ext4_common.o dev.o
-ifndef CONFIG_CMD_EXT4
-COBJS-$(CONFIG_CMD_EXT2) := ext4fs.o ext4_common.o dev.o
-endif
-COBJS-$(CONFIG_CMD_EXT4_WRITE) += ext4_journal.o crc16.o
+COBJS-$(CONFIG_FS_EXT4) := ext4fs.o ext4_common.o dev.o
+COBJS-$(CONFIG_EXT4_WRITE) += ext4_journal.o crc16.o
 
 SRCS	:= $(AOBJS:.o=.S) $(COBJS-y:.o=.c)
 OBJS	:= $(addprefix $(obj),$(AOBJS) $(COBJS-y))
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 3deffd5..1083813 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -56,7 +56,7 @@ int ext4fs_indir3_blkno = -1;
 struct ext2_inode *g_parent_inode;
 static int symlinknest;
 
-#if defined(CONFIG_CMD_EXT4_WRITE)
+#if defined(CONFIG_EXT4_WRITE)
 uint32_t ext4fs_div_roundup(uint32_t size, uint32_t n)
 {
 	uint32_t res = size / n;
diff --git a/fs/ext4/ext4_common.h b/fs/ext4/ext4_common.h
index f728134..87cab16 100644
--- a/fs/ext4/ext4_common.h
+++ b/fs/ext4/ext4_common.h
@@ -37,7 +37,7 @@
 #include <ext4fs.h>
 #include <malloc.h>
 #include <asm/errno.h>
-#if defined(CONFIG_CMD_EXT4_WRITE)
+#if defined(CONFIG_EXT4_WRITE)
 #include "ext4_journal.h"
 #include "crc16.h"
 #endif
@@ -71,7 +71,7 @@ int ext4fs_find_file(const char *path, struct ext2fs_node *rootnode,
 int ext4fs_iterate_dir(struct ext2fs_node *dir, char *name,
 			struct ext2fs_node **fnode, int *ftype);
 
-#if defined(CONFIG_CMD_EXT4_WRITE)
+#if defined(CONFIG_EXT4_WRITE)
 uint32_t ext4fs_div_roundup(uint32_t size, uint32_t n);
 int ext4fs_checksum_update(unsigned int i);
 int ext4fs_get_parent_inode_num(const char *dirname, char *dname, int flags);
diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c
index 93dcb7e..c0c1a41 100644
--- a/fs/ext4/ext4fs.c
+++ b/fs/ext4/ext4fs.c
@@ -196,7 +196,7 @@ int ext4fs_read(char *buf, unsigned len)
 	return ext4fs_read_file(ext4fs_file, 0, len, buf);
 }
 
-#if defined(CONFIG_CMD_EXT4_WRITE)
+#if defined(CONFIG_EXT4_WRITE)
 static void ext4fs_update(void)
 {
 	short i;
diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index 9635d36..3e33e38 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -24,11 +24,11 @@ include $(TOPDIR)/config.mk
 LIB	= $(obj)libfat.o
 
 AOBJS	=
-COBJS-$(CONFIG_CMD_FAT)	:= fat.o
+COBJS-$(CONFIG_FS_FAT)	:= fat.o
 COBJS-$(CONFIG_FAT_WRITE):= fat_write.o
 
 ifndef CONFIG_SPL_BUILD
-COBJS-$(CONFIG_CMD_FAT)	+= file.o
+COBJS-$(CONFIG_FS_FAT)	+= file.o
 endif
 
 SRCS	:= $(AOBJS:.o=.S) $(COBJS-y:.o=.c)
diff --git a/include/config_fallbacks.h b/include/config_fallbacks.h
index 430890c..bfb9680 100644
--- a/include/config_fallbacks.h
+++ b/include/config_fallbacks.h
@@ -13,4 +13,17 @@
 #define CONFIG_SYS_BAUDRATE_TABLE	{ 9600, 19200, 38400, 57600, 115200 }
 #endif
 
+#if defined(CONFIG_CMD_FAT) && !defined(CONFIG_FS_FAT)
+#define CONFIG_FS_FAT
+#endif
+
+#if (defined(CONFIG_CMD_EXT4) || defined(CONFIG_CMD_EXT2)) && \
+						!defined(CONFIG_FS_EXT4)
+#define CONFIG_FS_EXT4
+#endif
+
+#if defined(CONFIG_CMD_EXT4_WRITE) && !defined(CONFIG_EXT4_WRITE)
+#define CONFIG_EXT4_WRITE
+#endif
+
 #endif	/* __CONFIG_FALLBACKS_H */
diff --git a/include/ext4fs.h b/include/ext4fs.h
index b6eedde..35be720 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -116,7 +116,7 @@ struct ext_filesystem {
 extern struct ext2_data *ext4fs_root;
 extern struct ext2fs_node *ext4fs_file;
 
-#if defined(CONFIG_CMD_EXT4_WRITE)
+#if defined(CONFIG_EXT4_WRITE)
 extern struct ext2_inode *g_parent_inode;
 extern int gd_index;
 extern int gindex;
-- 
1.7.0.4

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Stephen Warren]

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.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2:
* Build cmd_fs.c when CONFIG_CMD_FS_GENERIC not always.
* Use new CONFIG_FS_{FAT,EXT4} rather than CONFIG_CMD_{FAT,EXT2}.
* Implemented fs_close() and call it from the tail of fs_ls() and fs_read().
  This ensures that any other usage of e.g. the ext4 library between fs_*()
  calls, then the two usages won't interfere.
* Re-organized fs.c to reduce the number of ifdef blocks.
* Added argc checking to do_ls().
---
 Makefile        |    3 +-
 common/Makefile |    1 +
 common/cmd_fs.c |   89 ++++++++++++++++++++++
 fs/Makefile     |   47 +++++++++++
 fs/fs.c         |  227 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/fs.h    |   49 ++++++++++++
 6 files changed, 415 insertions(+), 1 deletions(-)
 create mode 100644 common/cmd_fs.c
 create mode 100644 fs/Makefile
 create mode 100644 fs/fs.c
 create mode 100644 include/fs.h

diff --git a/Makefile b/Makefile
index d7e2c2f..0b50eb7 100644
--- a/Makefile
+++ b/Makefile
@@ -242,7 +242,8 @@ LIBS-y += drivers/net/npe/libnpe.o
 endif
 LIBS-$(CONFIG_OF_EMBED) += dts/libdts.o
 LIBS-y += arch/$(ARCH)/lib/lib$(ARCH).o
-LIBS-y += fs/cramfs/libcramfs.o \
+LIBS-y += fs/libfs.o \
+	fs/cramfs/libcramfs.o \
 	fs/ext4/libext4fs.o \
 	fs/fat/libfat.o \
 	fs/fdos/libfdos.o \
diff --git a/common/Makefile b/common/Makefile
index 125b2be..c6c31f7 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -103,6 +103,7 @@ COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
 ifdef CONFIG_FPGA
 COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
 endif
+COBJS-$(CONFIG_CMD_FS_GENERIC) += cmd_fs.o
 COBJS-$(CONFIG_CMD_GPIO) += cmd_gpio.o
 COBJS-$(CONFIG_CMD_I2C) += cmd_i2c.o
 COBJS-$(CONFIG_CMD_IDE) += cmd_ide.o
diff --git a/common/cmd_fs.c b/common/cmd_fs.c
new file mode 100644
index 0000000..9df0a66
--- /dev/null
+++ b/common/cmd_fs.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Inspired by cmd_ext_common.c, cmd_fat.c.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <common.h>
+#include <command.h>
+#include <fs.h>
+
+int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	unsigned long addr;
+	unsigned long bytes;
+	unsigned long pos;
+	int len_read;
+	char buf[12];
+
+	if (argc < 5)
+		return CMD_RET_USAGE;
+
+	if (fs_set_blk_dev(argv[1], argv[2]))
+		return 1;
+
+	addr = simple_strtoul(argv[3], NULL, 0);
+	if (argc >= 6)
+		bytes = simple_strtoul(argv[5], NULL, 0);
+	else
+		bytes = 0;
+	if (argc >= 7)
+		pos = simple_strtoul(argv[6], NULL, 0);
+	else
+		pos = 0;
+
+	len_read = fs_read(argv[4], addr, pos, bytes);
+	if (len_read <= 0)
+		return 1;
+
+	sprintf(buf, "0x%x", (unsigned int)len_read);
+	setenv("filesize", buf);
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	fsload,	7,	0,	do_fsload,
+	"load binary file from a filesystem",
+	"<interface> [<dev[:part]>] <addr> <filename> [bytes [pos]]\n"
+	"    - Load binary file 'filename' from partition 'part' on device\n"
+	"       type 'interface' instance 'dev' to address 'addr' in memory.\n"
+	"      'bytes' gives the size to load in bytes.\n"
+	"      If 'bytes' is 0 or omitted, the file is read until the end.\n"
+	"      'pos' gives the file byte position to start reading from.\n"
+	"      If 'pos' is 0 or omitted, the file is read from the start."
+);
+
+int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL))
+		return 1;
+
+	if (fs_ls(argc == 4 ? argv[3] : "/"))
+		return 1;
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	ls,	4,	1,	do_ls,
+	"list files in a directory (default /)",
+	"<interface> [<dev[:part]>] [directory]\n"
+	"    - List files in directory 'directory' of partition 'part' on\n"
+	"      device type 'interface' instance 'dev'."
+);
diff --git a/fs/Makefile b/fs/Makefile
new file mode 100644
index 0000000..d0ab3ae
--- /dev/null
+++ b/fs/Makefile
@@ -0,0 +1,47 @@
+#
+# (C) Copyright 2000-2006
+# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+# Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	= $(obj)libfs.o
+
+COBJS-y				+= fs.o
+
+COBJS	:= $(COBJS-y)
+SRCS	:= $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+
+all:	$(LIB)
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/fs/fs.c b/fs/fs.c
new file mode 100644
index 0000000..18cfaff
--- /dev/null
+++ b/fs/fs.c
@@ -0,0 +1,227 @@
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+#include <common.h>
+#include <part.h>
+#include <ext4fs.h>
+#include <fat.h>
+#include <fs.h>
+
+#define FS_TYPE_INVALID	0
+#define FS_TYPE_FAT	1
+#define FS_TYPE_EXT	2
+
+static block_dev_desc_t *fs_dev_desc;
+static disk_partition_t fs_partition;
+static int fs_type = FS_TYPE_INVALID;
+
+static inline int fs_ls_unsupported(const char *dirname)
+{
+	printf("** Unrecognized filesystem type **\n");
+	return -1;
+}
+
+static inline int fs_read_unsupported(const char *filename, ulong addr,
+				      int offset, int len)
+{
+	printf("** Unrecognized filesystem type **\n");
+	return -1;
+}
+
+#ifdef CONFIG_FS_FAT
+static int fs_probe_fat(void)
+{
+	return fat_set_blk_dev(fs_dev_desc, &fs_partition);
+}
+
+static void fs_close_fat(void)
+{
+}
+
+#define fs_ls_fat file_fat_ls
+
+static int fs_read_fat(const char *filename, ulong addr, int offset, int len)
+{
+	int len_read;
+
+	len_read = file_fat_read_at(filename, offset,
+				    (unsigned char *)addr, len);
+	if (len_read == -1) {
+		printf("** Unable to read file %s **\n", filename);
+		return -1;
+	}
+
+	return len_read;
+}
+#else
+static inline int fs_probe_fat(void)
+{
+	return -1;
+}
+
+static inline void fs_close_fat(void)
+{
+}
+
+#define fs_ls_fat fs_ls_unsupported
+#define fs_read_fat fs_read_unsupported
+#endif
+
+#ifdef CONFIG_FS_EXT4
+static int fs_probe_ext(void)
+{
+	ext4fs_set_blk_dev(fs_dev_desc, &fs_partition);
+
+	if (!ext4fs_mount(fs_partition.size)) {
+		ext4fs_close();
+		return -1;
+	}
+
+	return 0;
+}
+
+static void fs_close_ext(void)
+{
+	ext4fs_close();
+}
+
+#define fs_ls_ext ext4fs_ls
+
+static int fs_read_ext(const char *filename, ulong addr, int offset, int len)
+{
+	int file_len;
+	int len_read;
+
+	if (offset != 0) {
+		printf("** Cannot support non-zero offset **\n");
+		return -1;
+	}
+
+	file_len = ext4fs_open(filename);
+	if (file_len < 0) {
+		printf("** File not found %s **\n", filename);
+		ext4fs_close();
+		return -1;
+	}
+
+	if (len == 0)
+		len = file_len;
+
+	len_read = ext4fs_read((char *)addr, len);
+	ext4fs_close();
+
+	if (len_read != len) {
+		printf("** Unable to read file %s **\n", filename);
+		return -1;
+	}
+
+	return len_read;
+}
+#else
+static inline int fs_probe_ext(void)
+{
+	return -1;
+}
+
+static inline void fs_close_ext(void)
+{
+}
+
+#define fs_ls_ext fs_ls_unsupported
+#define fs_read_ext fs_read_unsupported
+#endif
+
+int fs_set_blk_dev(const char *ifname, const char *dev_part_str)
+{
+	int part;
+
+	part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc,
+					&fs_partition, 1);
+	if (part < 0)
+		return -1;
+
+	if (!fs_probe_fat()) {
+		fs_type = FS_TYPE_FAT;
+		return 0;
+	}
+
+	if (!fs_probe_ext()) {
+		fs_type = FS_TYPE_EXT;
+		return 0;
+	}
+
+	printf("** Unrecognized filesystem type **\n");
+	return -1;
+}
+
+static void fs_close(void)
+{
+	switch (fs_type) {
+	case FS_TYPE_FAT:
+		fs_close_fat();
+		break;
+	case FS_TYPE_EXT:
+		fs_close_ext();
+		break;
+	default:
+		break;
+	}
+
+	fs_type = FS_TYPE_INVALID;
+}
+
+int fs_ls(const char *dirname)
+{
+	int ret;
+
+	switch (fs_type) {
+	case FS_TYPE_FAT:
+		ret = fs_ls_fat(dirname);
+		break;
+	case FS_TYPE_EXT:
+		ret = fs_ls_ext(dirname);
+		break;
+	default:
+		ret = fs_ls_unsupported(dirname);
+		break;
+	}
+
+	fs_close();
+
+	return ret;
+}
+
+int fs_read(const char *filename, ulong addr, int offset, int len)
+{
+	int ret;
+
+	switch (fs_type) {
+	case FS_TYPE_FAT:
+		ret = fs_read_fat(filename, addr, offset, len);
+		break;
+	case FS_TYPE_EXT:
+		ret = fs_read_ext(filename, addr, offset, len);
+		break;
+	default:
+		ret = fs_read_unsupported(filename, addr, offset, len);
+		break;
+	}
+
+	fs_close();
+
+	return ret;
+}
diff --git a/include/fs.h b/include/fs.h
new file mode 100644
index 0000000..a0fda28
--- /dev/null
+++ b/include/fs.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _FS_H
+#define _FS_H
+
+/*
+ * Tell the fs layer which block device an partition to use for future
+ * commands. This also internally identifies the filesystem that is present
+ * within the partition.
+ *
+ * Returns 0 on success.
+ * Returns non-zero if there is an error accessing the disk or partition, or
+ * no known filesystem type could be recognized on it.
+ */
+int fs_set_blk_dev(const char *ifname, const char *dev_part_str);
+
+/*
+ * Print the list of files on the partition previously set by fs_set_blk_dev(),
+ * in directory "dirname".
+ *
+ * Returns 0 on success. Returns non-zero on error.
+ */
+int fs_ls(const char *dirname);
+
+/*
+ * 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.
+ *
+ * Returns number of bytes read on success. Returns <= 0 on error.
+ */
+int fs_read(const char *filename, ulong addr, int offset, int len);
+
+#endif /* _FS_H */
-- 
1.7.0.4

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Benoît Thébaudeau]

Hi Stephen,

On Thursday, October 11, 2012 8:59:29 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.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v2:
> * Build cmd_fs.c when CONFIG_CMD_FS_GENERIC not always.
> * Use new CONFIG_FS_{FAT,EXT4} rather than CONFIG_CMD_{FAT,EXT2}.
> * Implemented fs_close() and call it from the tail of fs_ls() and
> fs_read().
>   This ensures that any other usage of e.g. the ext4 library between
>   fs_*()
>   calls, then the two usages won't interfere.
> * Re-organized fs.c to reduce the number of ifdef blocks.
> * Added argc checking to do_ls().
> ---
>  Makefile        |    3 +-
>  common/Makefile |    1 +
>  common/cmd_fs.c |   89 ++++++++++++++++++++++
>  fs/Makefile     |   47 +++++++++++
>  fs/fs.c         |  227
>  +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/fs.h    |   49 ++++++++++++
>  6 files changed, 415 insertions(+), 1 deletions(-)
>  create mode 100644 common/cmd_fs.c
>  create mode 100644 fs/Makefile
>  create mode 100644 fs/fs.c
>  create mode 100644 include/fs.h
> 
> diff --git a/Makefile b/Makefile
> index d7e2c2f..0b50eb7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -242,7 +242,8 @@ LIBS-y += drivers/net/npe/libnpe.o
>  endif
>  LIBS-$(CONFIG_OF_EMBED) += dts/libdts.o
>  LIBS-y += arch/$(ARCH)/lib/lib$(ARCH).o
> -LIBS-y += fs/cramfs/libcramfs.o \
> +LIBS-y += fs/libfs.o \
> +	fs/cramfs/libcramfs.o \
>  	fs/ext4/libext4fs.o \
>  	fs/fat/libfat.o \
>  	fs/fdos/libfdos.o \
> diff --git a/common/Makefile b/common/Makefile
> index 125b2be..c6c31f7 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -103,6 +103,7 @@ COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
>  ifdef CONFIG_FPGA
>  COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
>  endif
> +COBJS-$(CONFIG_CMD_FS_GENERIC) += cmd_fs.o
>  COBJS-$(CONFIG_CMD_GPIO) += cmd_gpio.o
>  COBJS-$(CONFIG_CMD_I2C) += cmd_i2c.o
>  COBJS-$(CONFIG_CMD_IDE) += cmd_ide.o
> diff --git a/common/cmd_fs.c b/common/cmd_fs.c
> new file mode 100644
> index 0000000..9df0a66
> --- /dev/null
> +++ b/common/cmd_fs.c
> @@ -0,0 +1,89 @@
> +/*
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * Inspired by cmd_ext_common.c, cmd_fat.c.
> + *
> + * This program is free software; you can redistribute it and/or
> modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see
> <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <fs.h>
> +
> +int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> +{
> +	unsigned long addr;
> +	unsigned long bytes;
> +	unsigned long pos;
> +	int len_read;
> +	char buf[12];
> +
> +	if (argc < 5)
> +		return CMD_RET_USAGE;
> +
> +	if (fs_set_blk_dev(argv[1], argv[2]))
> +		return 1;
> +
> +	addr = simple_strtoul(argv[3], NULL, 0);
> +	if (argc >= 6)
> +		bytes = simple_strtoul(argv[5], NULL, 0);
> +	else
> +		bytes = 0;
> +	if (argc >= 7)
> +		pos = simple_strtoul(argv[6], NULL, 0);
> +	else
> +		pos = 0;
> +
> +	len_read = fs_read(argv[4], addr, pos, bytes);
> +	if (len_read <= 0)
> +		return 1;
> +
> +	sprintf(buf, "0x%x", (unsigned int)len_read);
> +	setenv("filesize", buf);
> +
> +	return 0;
> +}
> +
> +U_BOOT_CMD(
> +	fsload,	7,	0,	do_fsload,
> +	"load binary file from a filesystem",
> +	"<interface> [<dev[:part]>] <addr> <filename> [bytes [pos]]\n"
> +	"    - Load binary file 'filename' from partition 'part' on
> device\n"
> +	"       type 'interface' instance 'dev' to address 'addr' in
> memory.\n"
> +	"      'bytes' gives the size to load in bytes.\n"
> +	"      If 'bytes' is 0 or omitted, the file is read until the
> end.\n"
> +	"      'pos' gives the file byte position to start reading from.\n"
> +	"      If 'pos' is 0 or omitted, the file is read from the start."
> +);
> +
> +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +
> +	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL))
> +		return 1;
> +
> +	if (fs_ls(argc == 4 ? argv[3] : "/"))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +U_BOOT_CMD(
> +	ls,	4,	1,	do_ls,
> +	"list files in a directory (default /)",
> +	"<interface> [<dev[:part]>] [directory]\n"
> +	"    - List files in directory 'directory' of partition 'part'
> on\n"
> +	"      device type 'interface' instance 'dev'."
> +);
> diff --git a/fs/Makefile b/fs/Makefile
> new file mode 100644
> index 0000000..d0ab3ae
> --- /dev/null
> +++ b/fs/Makefile
> @@ -0,0 +1,47 @@
> +#
> +# (C) Copyright 2000-2006
> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> +# Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> +#
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> +# MA 02111-1307 USA
> +#
> +
> +include $(TOPDIR)/config.mk
> +
> +LIB	= $(obj)libfs.o
> +
> +COBJS-y				+= fs.o
> +
> +COBJS	:= $(COBJS-y)
> +SRCS	:= $(COBJS:.o=.c)
> +OBJS	:= $(addprefix $(obj),$(COBJS))
> +
> +all:	$(LIB)
> +
> +$(LIB):	$(obj).depend $(OBJS)
> +	$(call cmd_link_o_target, $(OBJS))
> +
> +#########################################################################
> +
> +# defines $(obj).depend target
> +include $(SRCTREE)/rules.mk
> +
> +sinclude $(obj).depend
> +
> +#########################################################################
> diff --git a/fs/fs.c b/fs/fs.c
> new file mode 100644
> index 0000000..18cfaff
> --- /dev/null
> +++ b/fs/fs.c
> @@ -0,0 +1,227 @@
> +/*
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see
> <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <part.h>
> +#include <ext4fs.h>
> +#include <fat.h>
> +#include <fs.h>
> +
> +#define FS_TYPE_INVALID	0
> +#define FS_TYPE_FAT	1
> +#define FS_TYPE_EXT	2
> +
> +static block_dev_desc_t *fs_dev_desc;
> +static disk_partition_t fs_partition;
> +static int fs_type = FS_TYPE_INVALID;
> +
> +static inline int fs_ls_unsupported(const char *dirname)
> +{
> +	printf("** Unrecognized filesystem type **\n");
> +	return -1;
> +}
> +
> +static inline int fs_read_unsupported(const char *filename, ulong
> addr,
> +				      int offset, int len)
> +{
> +	printf("** Unrecognized filesystem type **\n");
> +	return -1;
> +}
> +
> +#ifdef CONFIG_FS_FAT
> +static int fs_probe_fat(void)
> +{
> +	return fat_set_blk_dev(fs_dev_desc, &fs_partition);
> +}
> +
> +static void fs_close_fat(void)
> +{
> +}
> +
> +#define fs_ls_fat file_fat_ls
> +
> +static int fs_read_fat(const char *filename, ulong addr, int offset,
> int len)
> +{
> +	int len_read;
> +
> +	len_read = file_fat_read_at(filename, offset,
> +				    (unsigned char *)addr, len);
> +	if (len_read == -1) {
> +		printf("** Unable to read file %s **\n", filename);
> +		return -1;
> +	}
> +
> +	return len_read;
> +}
> +#else
> +static inline int fs_probe_fat(void)
> +{
> +	return -1;
> +}
> +
> +static inline void fs_close_fat(void)
> +{
> +}
> +
> +#define fs_ls_fat fs_ls_unsupported
> +#define fs_read_fat fs_read_unsupported
> +#endif
> +
> +#ifdef CONFIG_FS_EXT4
> +static int fs_probe_ext(void)
> +{
> +	ext4fs_set_blk_dev(fs_dev_desc, &fs_partition);
> +
> +	if (!ext4fs_mount(fs_partition.size)) {
> +		ext4fs_close();
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static void fs_close_ext(void)
> +{
> +	ext4fs_close();
> +}
> +
> +#define fs_ls_ext ext4fs_ls
> +
> +static int fs_read_ext(const char *filename, ulong addr, int offset,
> int len)
> +{
> +	int file_len;
> +	int len_read;
> +
> +	if (offset != 0) {
> +		printf("** Cannot support non-zero offset **\n");
> +		return -1;
> +	}
> +
> +	file_len = ext4fs_open(filename);
> +	if (file_len < 0) {
> +		printf("** File not found %s **\n", filename);
> +		ext4fs_close();
> +		return -1;
> +	}
> +
> +	if (len == 0)
> +		len = file_len;
> +
> +	len_read = ext4fs_read((char *)addr, len);
> +	ext4fs_close();
> +
> +	if (len_read != len) {
> +		printf("** Unable to read file %s **\n", filename);
> +		return -1;
> +	}
> +
> +	return len_read;
> +}
> +#else
> +static inline int fs_probe_ext(void)
> +{
> +	return -1;
> +}
> +
> +static inline void fs_close_ext(void)
> +{
> +}
> +
> +#define fs_ls_ext fs_ls_unsupported
> +#define fs_read_ext fs_read_unsupported
> +#endif
> +
> +int fs_set_blk_dev(const char *ifname, const char *dev_part_str)
> +{
> +	int part;
> +
> +	part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc,
> +					&fs_partition, 1);
> +	if (part < 0)
> +		return -1;
> +
> +	if (!fs_probe_fat()) {
> +		fs_type = FS_TYPE_FAT;
> +		return 0;
> +	}
> +
> +	if (!fs_probe_ext()) {
> +		fs_type = FS_TYPE_EXT;
> +		return 0;
> +	}
> +
> +	printf("** Unrecognized filesystem type **\n");
> +	return -1;
> +}
> +
> +static void fs_close(void)
> +{
> +	switch (fs_type) {
> +	case FS_TYPE_FAT:
> +		fs_close_fat();
> +		break;
> +	case FS_TYPE_EXT:
> +		fs_close_ext();
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	fs_type = FS_TYPE_INVALID;
> +}
> +
> +int fs_ls(const char *dirname)
> +{
> +	int ret;
> +
> +	switch (fs_type) {
> +	case FS_TYPE_FAT:
> +		ret = fs_ls_fat(dirname);
> +		break;
> +	case FS_TYPE_EXT:
> +		ret = fs_ls_ext(dirname);
> +		break;
> +	default:
> +		ret = fs_ls_unsupported(dirname);
> +		break;
> +	}
> +
> +	fs_close();
> +
> +	return ret;
> +}
> +
> +int fs_read(const char *filename, ulong addr, int offset, int len)
> +{
> +	int ret;
> +
> +	switch (fs_type) {
> +	case FS_TYPE_FAT:
> +		ret = fs_read_fat(filename, addr, offset, len);
> +		break;
> +	case FS_TYPE_EXT:
> +		ret = fs_read_ext(filename, addr, offset, len);
> +		break;
> +	default:
> +		ret = fs_read_unsupported(filename, addr, offset, len);
> +		break;
> +	}
> +
> +	fs_close();
> +
> +	return ret;
> +}
> diff --git a/include/fs.h b/include/fs.h
> new file mode 100644
> index 0000000..a0fda28
> --- /dev/null
> +++ b/include/fs.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see
> <http://www.gnu.org/licenses/>.
> + */
> +#ifndef _FS_H
> +#define _FS_H
> +
> +/*
> + * Tell the fs layer which block device an partition to use for
> future
> + * commands. This also internally identifies the filesystem that is
> present
> + * within the partition.
> + *
> + * Returns 0 on success.
> + * Returns non-zero if there is an error accessing the disk or
> partition, or
> + * no known filesystem type could be recognized on it.
> + */
> +int fs_set_blk_dev(const char *ifname, const char *dev_part_str);
> +
> +/*
> + * Print the list of files on the partition previously set by
> fs_set_blk_dev(),
> + * in directory "dirname".
> + *
> + * Returns 0 on success. Returns non-zero on error.
> + */
> +int fs_ls(const char *dirname);
> +
> +/*
> + * 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.
> + *
> + * Returns number of bytes read on success. Returns <= 0 on error.
> + */
> +int fs_read(const char *filename, ulong addr, int offset, int len);
> +
> +#endif /* _FS_H */

I'm fine with this new version of the series:
Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>

Best regards,
Beno?t

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Rob Herring]

On 10/11/2012 01:59 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.
> 

This is exactly what I want to get to. However, I think the first step
should be to unify the filesystem api similar to the block device api.
This would avoid the wrapper here and yet another copy of nearly
identical code. Then we can unify the implementations of *ls and *load.

Rob

> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v2:
> * Build cmd_fs.c when CONFIG_CMD_FS_GENERIC not always.
> * Use new CONFIG_FS_{FAT,EXT4} rather than CONFIG_CMD_{FAT,EXT2}.
> * Implemented fs_close() and call it from the tail of fs_ls() and fs_read().
>   This ensures that any other usage of e.g. the ext4 library between fs_*()
>   calls, then the two usages won't interfere.
> * Re-organized fs.c to reduce the number of ifdef blocks.
> * Added argc checking to do_ls().
> ---
>  Makefile        |    3 +-
>  common/Makefile |    1 +
>  common/cmd_fs.c |   89 ++++++++++++++++++++++
>  fs/Makefile     |   47 +++++++++++
>  fs/fs.c         |  227 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/fs.h    |   49 ++++++++++++
>  6 files changed, 415 insertions(+), 1 deletions(-)
>  create mode 100644 common/cmd_fs.c
>  create mode 100644 fs/Makefile
>  create mode 100644 fs/fs.c
>  create mode 100644 include/fs.h
> 
> diff --git a/Makefile b/Makefile
> index d7e2c2f..0b50eb7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -242,7 +242,8 @@ LIBS-y += drivers/net/npe/libnpe.o
>  endif
>  LIBS-$(CONFIG_OF_EMBED) += dts/libdts.o
>  LIBS-y += arch/$(ARCH)/lib/lib$(ARCH).o
> -LIBS-y += fs/cramfs/libcramfs.o \
> +LIBS-y += fs/libfs.o \
> +	fs/cramfs/libcramfs.o \
>  	fs/ext4/libext4fs.o \
>  	fs/fat/libfat.o \
>  	fs/fdos/libfdos.o \
> diff --git a/common/Makefile b/common/Makefile
> index 125b2be..c6c31f7 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -103,6 +103,7 @@ COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
>  ifdef CONFIG_FPGA
>  COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
>  endif
> +COBJS-$(CONFIG_CMD_FS_GENERIC) += cmd_fs.o
>  COBJS-$(CONFIG_CMD_GPIO) += cmd_gpio.o
>  COBJS-$(CONFIG_CMD_I2C) += cmd_i2c.o
>  COBJS-$(CONFIG_CMD_IDE) += cmd_ide.o
> diff --git a/common/cmd_fs.c b/common/cmd_fs.c
> new file mode 100644
> index 0000000..9df0a66
> --- /dev/null
> +++ b/common/cmd_fs.c
> @@ -0,0 +1,89 @@
> +/*
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * Inspired by cmd_ext_common.c, cmd_fat.c.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <fs.h>
> +
> +int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	unsigned long addr;
> +	unsigned long bytes;
> +	unsigned long pos;
> +	int len_read;
> +	char buf[12];
> +
> +	if (argc < 5)
> +		return CMD_RET_USAGE;
> +
> +	if (fs_set_blk_dev(argv[1], argv[2]))
> +		return 1;
> +
> +	addr = simple_strtoul(argv[3], NULL, 0);
> +	if (argc >= 6)
> +		bytes = simple_strtoul(argv[5], NULL, 0);
> +	else
> +		bytes = 0;
> +	if (argc >= 7)
> +		pos = simple_strtoul(argv[6], NULL, 0);
> +	else
> +		pos = 0;
> +
> +	len_read = fs_read(argv[4], addr, pos, bytes);
> +	if (len_read <= 0)
> +		return 1;
> +
> +	sprintf(buf, "0x%x", (unsigned int)len_read);
> +	setenv("filesize", buf);
> +
> +	return 0;
> +}
> +
> +U_BOOT_CMD(
> +	fsload,	7,	0,	do_fsload,
> +	"load binary file from a filesystem",
> +	"<interface> [<dev[:part]>] <addr> <filename> [bytes [pos]]\n"
> +	"    - Load binary file 'filename' from partition 'part' on device\n"
> +	"       type 'interface' instance 'dev' to address 'addr' in memory.\n"
> +	"      'bytes' gives the size to load in bytes.\n"
> +	"      If 'bytes' is 0 or omitted, the file is read until the end.\n"
> +	"      'pos' gives the file byte position to start reading from.\n"
> +	"      If 'pos' is 0 or omitted, the file is read from the start."
> +);
> +
> +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +
> +	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL))
> +		return 1;
> +
> +	if (fs_ls(argc == 4 ? argv[3] : "/"))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +U_BOOT_CMD(
> +	ls,	4,	1,	do_ls,
> +	"list files in a directory (default /)",
> +	"<interface> [<dev[:part]>] [directory]\n"
> +	"    - List files in directory 'directory' of partition 'part' on\n"
> +	"      device type 'interface' instance 'dev'."
> +);
> diff --git a/fs/Makefile b/fs/Makefile
> new file mode 100644
> index 0000000..d0ab3ae
> --- /dev/null
> +++ b/fs/Makefile
> @@ -0,0 +1,47 @@
> +#
> +# (C) Copyright 2000-2006
> +# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> +# Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> +#
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation; either version 2 of
> +# the License, or (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> +# MA 02111-1307 USA
> +#
> +
> +include $(TOPDIR)/config.mk
> +
> +LIB	= $(obj)libfs.o
> +
> +COBJS-y				+= fs.o
> +
> +COBJS	:= $(COBJS-y)
> +SRCS	:= $(COBJS:.o=.c)
> +OBJS	:= $(addprefix $(obj),$(COBJS))
> +
> +all:	$(LIB)
> +
> +$(LIB):	$(obj).depend $(OBJS)
> +	$(call cmd_link_o_target, $(OBJS))
> +
> +#########################################################################
> +
> +# defines $(obj).depend target
> +include $(SRCTREE)/rules.mk
> +
> +sinclude $(obj).depend
> +
> +#########################################################################
> diff --git a/fs/fs.c b/fs/fs.c
> new file mode 100644
> index 0000000..18cfaff
> --- /dev/null
> +++ b/fs/fs.c
> @@ -0,0 +1,227 @@
> +/*
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <part.h>
> +#include <ext4fs.h>
> +#include <fat.h>
> +#include <fs.h>
> +
> +#define FS_TYPE_INVALID	0
> +#define FS_TYPE_FAT	1
> +#define FS_TYPE_EXT	2
> +
> +static block_dev_desc_t *fs_dev_desc;
> +static disk_partition_t fs_partition;
> +static int fs_type = FS_TYPE_INVALID;
> +
> +static inline int fs_ls_unsupported(const char *dirname)
> +{
> +	printf("** Unrecognized filesystem type **\n");
> +	return -1;
> +}
> +
> +static inline int fs_read_unsupported(const char *filename, ulong addr,
> +				      int offset, int len)
> +{
> +	printf("** Unrecognized filesystem type **\n");
> +	return -1;
> +}
> +
> +#ifdef CONFIG_FS_FAT
> +static int fs_probe_fat(void)
> +{
> +	return fat_set_blk_dev(fs_dev_desc, &fs_partition);
> +}
> +
> +static void fs_close_fat(void)
> +{
> +}
> +
> +#define fs_ls_fat file_fat_ls
> +
> +static int fs_read_fat(const char *filename, ulong addr, int offset, int len)
> +{
> +	int len_read;
> +
> +	len_read = file_fat_read_at(filename, offset,
> +				    (unsigned char *)addr, len);
> +	if (len_read == -1) {
> +		printf("** Unable to read file %s **\n", filename);
> +		return -1;
> +	}
> +
> +	return len_read;
> +}
> +#else
> +static inline int fs_probe_fat(void)
> +{
> +	return -1;
> +}
> +
> +static inline void fs_close_fat(void)
> +{
> +}
> +
> +#define fs_ls_fat fs_ls_unsupported
> +#define fs_read_fat fs_read_unsupported
> +#endif
> +
> +#ifdef CONFIG_FS_EXT4
> +static int fs_probe_ext(void)
> +{
> +	ext4fs_set_blk_dev(fs_dev_desc, &fs_partition);
> +
> +	if (!ext4fs_mount(fs_partition.size)) {
> +		ext4fs_close();
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static void fs_close_ext(void)
> +{
> +	ext4fs_close();
> +}
> +
> +#define fs_ls_ext ext4fs_ls
> +
> +static int fs_read_ext(const char *filename, ulong addr, int offset, int len)
> +{
> +	int file_len;
> +	int len_read;
> +
> +	if (offset != 0) {
> +		printf("** Cannot support non-zero offset **\n");
> +		return -1;
> +	}
> +
> +	file_len = ext4fs_open(filename);
> +	if (file_len < 0) {
> +		printf("** File not found %s **\n", filename);
> +		ext4fs_close();
> +		return -1;
> +	}
> +
> +	if (len == 0)
> +		len = file_len;
> +
> +	len_read = ext4fs_read((char *)addr, len);
> +	ext4fs_close();
> +
> +	if (len_read != len) {
> +		printf("** Unable to read file %s **\n", filename);
> +		return -1;
> +	}
> +
> +	return len_read;
> +}
> +#else
> +static inline int fs_probe_ext(void)
> +{
> +	return -1;
> +}
> +
> +static inline void fs_close_ext(void)
> +{
> +}
> +
> +#define fs_ls_ext fs_ls_unsupported
> +#define fs_read_ext fs_read_unsupported
> +#endif
> +
> +int fs_set_blk_dev(const char *ifname, const char *dev_part_str)
> +{
> +	int part;
> +
> +	part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc,
> +					&fs_partition, 1);
> +	if (part < 0)
> +		return -1;
> +
> +	if (!fs_probe_fat()) {
> +		fs_type = FS_TYPE_FAT;
> +		return 0;
> +	}
> +
> +	if (!fs_probe_ext()) {
> +		fs_type = FS_TYPE_EXT;
> +		return 0;
> +	}
> +
> +	printf("** Unrecognized filesystem type **\n");
> +	return -1;
> +}
> +
> +static void fs_close(void)
> +{
> +	switch (fs_type) {
> +	case FS_TYPE_FAT:
> +		fs_close_fat();
> +		break;
> +	case FS_TYPE_EXT:
> +		fs_close_ext();
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	fs_type = FS_TYPE_INVALID;
> +}
> +
> +int fs_ls(const char *dirname)
> +{
> +	int ret;
> +
> +	switch (fs_type) {
> +	case FS_TYPE_FAT:
> +		ret = fs_ls_fat(dirname);
> +		break;
> +	case FS_TYPE_EXT:
> +		ret = fs_ls_ext(dirname);
> +		break;
> +	default:
> +		ret = fs_ls_unsupported(dirname);
> +		break;
> +	}
> +
> +	fs_close();
> +
> +	return ret;
> +}
> +
> +int fs_read(const char *filename, ulong addr, int offset, int len)
> +{
> +	int ret;
> +
> +	switch (fs_type) {
> +	case FS_TYPE_FAT:
> +		ret = fs_read_fat(filename, addr, offset, len);
> +		break;
> +	case FS_TYPE_EXT:
> +		ret = fs_read_ext(filename, addr, offset, len);
> +		break;
> +	default:
> +		ret = fs_read_unsupported(filename, addr, offset, len);
> +		break;
> +	}
> +
> +	fs_close();
> +
> +	return ret;
> +}
> diff --git a/include/fs.h b/include/fs.h
> new file mode 100644
> index 0000000..a0fda28
> --- /dev/null
> +++ b/include/fs.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef _FS_H
> +#define _FS_H
> +
> +/*
> + * Tell the fs layer which block device an partition to use for future
> + * commands. This also internally identifies the filesystem that is present
> + * within the partition.
> + *
> + * Returns 0 on success.
> + * Returns non-zero if there is an error accessing the disk or partition, or
> + * no known filesystem type could be recognized on it.
> + */
> +int fs_set_blk_dev(const char *ifname, const char *dev_part_str);
> +
> +/*
> + * Print the list of files on the partition previously set by fs_set_blk_dev(),
> + * in directory "dirname".
> + *
> + * Returns 0 on success. Returns non-zero on error.
> + */
> +int fs_ls(const char *dirname);
> +
> +/*
> + * 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.
> + *
> + * Returns number of bytes read on success. Returns <= 0 on error.
> + */
> +int fs_read(const char *filename, ulong addr, int offset, int len);
> +
> +#endif /* _FS_H */
> 

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Stephen Warren]

On 10/15/2012 10:33 AM, Rob Herring wrote:
> On 10/11/2012 01:59 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.
>>
> 
> This is exactly what I want to get to.

That's good.

> However, I think the first step
> should be to unify the filesystem api similar to the block device api.
> This would avoid the wrapper here and yet another copy of nearly
> identical code. Then we can unify the implementations of *ls and *load.

I think we will always need some form of wrapper.

At the very least, we will need fs_set_blk_dev() (or a function that
does the same thing under a different name) in order to probe which type
of filesystem is present, just like we have get_partition_info() for
partitions, which scans for all known forms of partition table.

The only way to avoid wrappers for the other functions (ls, load) would
be if fs_set_blk_dev() were to set up some global variable pointing at
the filesystem implementation struct. If it did that, client code could
call directly into the filesystem rather than calling into the wrapper
functions to indirect to the correct filesystem. This might be a
reasonable design, although even if that is the long-term plan, I don't
think it precludes using the wrapper approach first, then refactoring
the wrapper away as functionality (e.g. fs_{probe,close}_{fat,ext4})
moves into the filesystem implementations; this seems like a good first
step on the way.

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Tom Rini]

On Mon, Oct 15, 2012 at 10:47:49AM -0600, Stephen Warren wrote:
> On 10/15/2012 10:33 AM, Rob Herring wrote:
> > On 10/11/2012 01:59 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.
> >>
> > 
> > This is exactly what I want to get to.
> 
> That's good.
> 
> > However, I think the first step
> > should be to unify the filesystem api similar to the block device api.
> > This would avoid the wrapper here and yet another copy of nearly
> > identical code. Then we can unify the implementations of *ls and *load.
> 
> I think we will always need some form of wrapper.
> 
> At the very least, we will need fs_set_blk_dev() (or a function that
> does the same thing under a different name) in order to probe which type
> of filesystem is present, just like we have get_partition_info() for
> partitions, which scans for all known forms of partition table.
> 
> The only way to avoid wrappers for the other functions (ls, load) would
> be if fs_set_blk_dev() were to set up some global variable pointing at
> the filesystem implementation struct. If it did that, client code could
> call directly into the filesystem rather than calling into the wrapper
> functions to indirect to the correct filesystem. This might be a
> reasonable design, although even if that is the long-term plan, I don't
> think it precludes using the wrapper approach first, then refactoring
> the wrapper away as functionality (e.g. fs_{probe,close}_{fat,ext4})
> moves into the filesystem implementations; this seems like a good first
> step on the way.

Baring further discussion, I intend to grab this really soon, as it
sounds like it's a functional starting point, however we wish to make
this happen.  Or am I not following?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121018/e92c752d/attachment.pgp>

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Rob Herring]

On 10/18/2012 06:01 PM, Tom Rini wrote:
> On Mon, Oct 15, 2012 at 10:47:49AM -0600, Stephen Warren wrote:
>> On 10/15/2012 10:33 AM, Rob Herring wrote:
>>> On 10/11/2012 01:59 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.
>>>>
>>>
>>> This is exactly what I want to get to.
>>
>> That's good.
>>
>>> However, I think the first step
>>> should be to unify the filesystem api similar to the block device api.
>>> This would avoid the wrapper here and yet another copy of nearly
>>> identical code. Then we can unify the implementations of *ls and *load.
>>
>> I think we will always need some form of wrapper.
>>
>> At the very least, we will need fs_set_blk_dev() (or a function that
>> does the same thing under a different name) in order to probe which type
>> of filesystem is present, just like we have get_partition_info() for
>> partitions, which scans for all known forms of partition table.
>>
>> The only way to avoid wrappers for the other functions (ls, load) would
>> be if fs_set_blk_dev() were to set up some global variable pointing at
>> the filesystem implementation struct. If it did that, client code could
>> call directly into the filesystem rather than calling into the wrapper
>> functions to indirect to the correct filesystem. This might be a
>> reasonable design, although even if that is the long-term plan, I don't
>> think it precludes using the wrapper approach first, then refactoring
>> the wrapper away as functionality (e.g. fs_{probe,close}_{fat,ext4})
>> moves into the filesystem implementations; this seems like a good first
>> step on the way.
> 
> Baring further discussion, I intend to grab this really soon, as it
> sounds like it's a functional starting point, however we wish to make
> this happen.  Or am I not following?  Thanks!

It's your call. I'd rather see clean-up first and features second, but
that's just me. Either way works. The amount of duplication in u-boot
just annoys me. Hopefully the DM work will fix some of it.

Rob

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/18/12 16:12, Rob Herring wrote:
> On 10/18/2012 06:01 PM, Tom Rini wrote:
>> On Mon, Oct 15, 2012 at 10:47:49AM -0600, Stephen Warren wrote:
>>> On 10/15/2012 10:33 AM, Rob Herring wrote:
>>>> On 10/11/2012 01:59 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.
>>>>> 
>>>> 
>>>> This is exactly what I want to get to.
>>> 
>>> That's good.
>>> 
>>>> However, I think the first step should be to unify the
>>>> filesystem api similar to the block device api. This would
>>>> avoid the wrapper here and yet another copy of nearly 
>>>> identical code. Then we can unify the implementations of *ls
>>>> and *load.
>>> 
>>> I think we will always need some form of wrapper.
>>> 
>>> At the very least, we will need fs_set_blk_dev() (or a function
>>> that does the same thing under a different name) in order to
>>> probe which type of filesystem is present, just like we have
>>> get_partition_info() for partitions, which scans for all known
>>> forms of partition table.
>>> 
>>> The only way to avoid wrappers for the other functions (ls,
>>> load) would be if fs_set_blk_dev() were to set up some global
>>> variable pointing at the filesystem implementation struct. If
>>> it did that, client code could call directly into the
>>> filesystem rather than calling into the wrapper functions to
>>> indirect to the correct filesystem. This might be a reasonable
>>> design, although even if that is the long-term plan, I don't 
>>> think it precludes using the wrapper approach first, then
>>> refactoring the wrapper away as functionality (e.g.
>>> fs_{probe,close}_{fat,ext4}) moves into the filesystem
>>> implementations; this seems like a good first step on the way.
>> 
>> Baring further discussion, I intend to grab this really soon, as
>> it sounds like it's a functional starting point, however we wish
>> to make this happen.  Or am I not following?  Thanks!
> 
> It's your call. I'd rather see clean-up first and features second,
> but that's just me. Either way works. The amount of duplication in
> u-boot just annoys me. Hopefully the DM work will fix some of it.

I too would like to see more clean-up, and I do expect progress down
that line, in this area, from Stephen as part of this being accepted.
 Unless the two of you would like to start collaborating on a slightly
different path?

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQgI9vAAoJENk4IS6UOR1WLWgP/2MTdElBAG+EYGY8aoLuT0yA
HSLRjz63xdM2Y2sQnI5OTyKQjuAGPjpGlZg9UHkrbBiyGixdcZwjZ+zZlVsjUIRC
eL9Zd5SByyZkYdw+d5BHFt2R6nTS7PTLGxKcfGnz03NKgY8+jP3gGjkY8xx6Egv8
z6SOxSPXAM16A2YZBkR+1JIR58ZQ4rBVTl1pOuFoFugC11ALcpCYqDfkCc/iOcSj
IPdD9/hfU75NBTHuLRyeBjO0s2Z5OTrPR/SNyQhCxouLxiIRyBvACIbKxBHJByl8
8e8W5k+zBw5+FvA+ioh7JCKFNzgwDf7rVc4hIX98lrfvB+bUUJ8AJrLdDVhybf84
BdDSpJ8Pq0ShQZXw3DbylnHhG8H3JuoH0ZfqHE8ws0IfdQQ8QhEosiV1h8x9sc0u
zBLPY9ZkF75HKoHi0B0Aq+atzSUOy2I5PpsaeaX1Lyi4tLhsupE5nXuS1Pv8Jp+3
zwPoGRVZavpEfhvcrNNCTdfWDaTuhzOEM/PvGHrJQimpSw7YC03617muMjwVNk4B
uJVY9k5iuNCpQOiAxm7/XV0+L9U2FFpj733f6w85p0bCLRnj2JTIyLy6bGNFJlP7
S3BLnPaHgbH90dRJRnvnnIT31Qove/9/2DxKIE1DYWmfULPNPTVkscUigA5Bw/16
BYU/4XZShmDnzPHJBTef
=OMSB
-----END PGP SIGNATURE-----

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Stephen Warren]

On 10/18/2012 05:23 PM, Tom Rini wrote:
> On 10/18/12 16:12, Rob Herring wrote:
>> On 10/18/2012 06:01 PM, Tom Rini wrote:
...
>>>>> On 10/11/2012 01:59 PM, Stephen Warren wrote:
>>>>>> 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.
...
>>> Baring further discussion, I intend to grab this really soon,
>>> as it sounds like it's a functional starting point, however we
>>> wish to make this happen.  Or am I not following?  Thanks!
> 
>> It's your call. I'd rather see clean-up first and features
>> second, but that's just me. Either way works. The amount of
>> duplication in u-boot just annoys me. Hopefully the DM work will
>> fix some of it.
> 
> I too would like to see more clean-up,

Which clean-up exactly?

The only duplication I see here is that ext2load/fatload could be
modified to simply call into do_fsload. That'd be pretty simple, I
think, assuming the behaviour change was OK (e.g. fatload would
suddenly support either FAT or ext2*), and that cmd_fs.c and fs.c
would both always be pulled in.

Re: refactoring of the interface to the filesystem code: I'm curious
what the DM-related plans are for filesystems. It seems that any such
refactoring would be part of that work. Unfortunately I haven't been
paying any attention to who might be proposing doing what and when
there. Would it be appropriate to defer any fs-related API changes
until any DM+fs rework went it to avoid conflicts or duplicate work?

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Rob Herring]

On 10/19/2012 11:56 AM, Stephen Warren wrote:
> On 10/18/2012 05:23 PM, Tom Rini wrote:
>> On 10/18/12 16:12, Rob Herring wrote:
>>> On 10/18/2012 06:01 PM, Tom Rini wrote:
> ...
>>>>>> On 10/11/2012 01:59 PM, Stephen Warren wrote:
>>>>>>> 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.
> ...
>>>> Baring further discussion, I intend to grab this really soon,
>>>> as it sounds like it's a functional starting point, however we
>>>> wish to make this happen.  Or am I not following?  Thanks!
>>
>>> It's your call. I'd rather see clean-up first and features
>>> second, but that's just me. Either way works. The amount of
>>> duplication in u-boot just annoys me. Hopefully the DM work will
>>> fix some of it.
>>
>> I too would like to see more clean-up,
> 
> Which clean-up exactly?
> 
> The only duplication I see here is that ext2load/fatload could be
> modified to simply call into do_fsload. That'd be pretty simple, I
> think, assuming the behaviour change was OK (e.g. fatload would
> suddenly support either FAT or ext2*), and that cmd_fs.c and fs.c
> would both always be pulled in.

Can't you make do_fsload support either specifying the fs for legacy use
or detecting it on the new commands?

> Re: refactoring of the interface to the filesystem code: I'm curious
> what the DM-related plans are for filesystems. It seems that any such
> refactoring would be part of that work. Unfortunately I haven't been
> paying any attention to who might be proposing doing what and when
> there. Would it be appropriate to defer any fs-related API changes
> until any DM+fs rework went it to avoid conflicts or duplicate work?

I've had the same questions as well...

Rob

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Stephen Warren]

On 10/19/2012 01:18 PM, Rob Herring wrote:
> On 10/19/2012 11:56 AM, Stephen Warren wrote:
>> On 10/18/2012 05:23 PM, Tom Rini wrote:
>>> On 10/18/12 16:12, Rob Herring wrote:
>>>> On 10/18/2012 06:01 PM, Tom Rini wrote:
>> ...
>>>>>>> On 10/11/2012 01:59 PM, Stephen Warren wrote:
>>>>>>>> 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.
>> ...
>>>>> Baring further discussion, I intend to grab this really soon,
>>>>> as it sounds like it's a functional starting point, however we
>>>>> wish to make this happen.  Or am I not following?  Thanks!
>>>
>>>> It's your call. I'd rather see clean-up first and features
>>>> second, but that's just me. Either way works. The amount of
>>>> duplication in u-boot just annoys me. Hopefully the DM work will
>>>> fix some of it.
>>>
>>> I too would like to see more clean-up,
>>
>> Which clean-up exactly?
>>
>> The only duplication I see here is that ext2load/fatload could be
>> modified to simply call into do_fsload. That'd be pretty simple, I
>> think, assuming the behaviour change was OK (e.g. fatload would
>> suddenly support either FAT or ext2*), and that cmd_fs.c and fs.c
>> would both always be pulled in.
> 
> Can't you make do_fsload support either specifying the fs for legacy use
> or detecting it on the new commands?

Yes, I suppose I could:

* Add a bit-mask of legal filesystems as a parameter to fs_set_blk_dev().

* Move the body of do_fsload() into some common called by do_fsload(),
do_ext2load(), do_fatload(), each passing in the appropriate bit-mask,
which gets passed down to fs_set_blk_dev().

That'd be easy, and probably entail only extremely minimal code-size
increases for an ext2-only or FAT-only build; just a few bytes for a few
more function calls. Sound like a plan?

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/19/12 12:26, Stephen Warren wrote:

[snip]
> Yes, I suppose I could:
> 
> * Add a bit-mask of legal filesystems as a parameter to 
> fs_set_blk_dev().
> 
> * Move the body of do_fsload() into some common called by 
> do_fsload(), do_ext2load(), do_fatload(), each passing in the 
> appropriate bit-mask, which gets passed down to fs_set_blk_dev().
> 
> That'd be easy, and probably entail only extremely minimal 
> code-size increases for an ext2-only or FAT-only build; just a few 
> bytes for a few more function calls. Sound like a plan?

Sounds good to me.  I bet you'll find some other clean-ups while
you're in there.  Marek, any comments from the DM perspective here?

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQgbODAAoJENk4IS6UOR1Wk1gP/0yNXaGlxt2LPDcJ5ux7Vc1W
Y0m5gnOzcF3gehW7EArvy+pIhJMD0Gu4XVdrY0A7GKdJiYs2Nv56vJpkYMu8MfU+
trMquPFqGeAe5Uq2LRVmEKIb3+YdbEv7xuiMThwt8zpEtbQ6ujZheeYHPDoAZwdL
jI4geIsOHibo24+Encux8AlcDJuP0PSfsrY4bvBMlriuAx/D7yCeaVJcbtb9mSQM
S/PYkTIHOfUzrntK4nrjoi4YHW39wUQv+5B5wUWEHeDnfhkba/uD2HsoG57RuiT+
cc7jMokhtz1vIzZHs2glfQI3UHEcoMo4NY6sznghCkHtNRb0cKlo+YUWoarjPpjV
z0aygHKz5ovC9SguzNmGSiXkz/GFI5Pwi2vneaaVp2eRn6hsPkQadQbiNFhjXKo/
L7Olu8iiAOqvkVTcYZ68PmEp75kX+WQ/o59lrFvRl8Ny3ir55r7oBAsrTyFgOmmi
I4q/VablIKl9VtvD27lzbAB2tHfggJOrO0XU2iv4mZxDkY/N/qvtfLpEmzZXbv7r
VnK2MUKGR2t7hue64llrNIffivBonrseoyUtbSMummzANYN/NJDIg87K8DYlLFvz
c6FiDvtbEpUxVTzIkALb5MVfOWaKbFqnq6mffwG9Y41X0ZfpRpV54TPA/wTo8CyQ
GrEJAzv8QBlWrsXjNqEm
=NCgN
-----END PGP SIGNATURE-----

[U-Boot] [PATCH V2 3/3] fs: add partition switch libary, implement ls and fsload commands

[Marek Vasut]

Dear Tom Rini,

> On 10/19/12 12:26, Stephen Warren wrote:
> 
> [snip]
> 
> > Yes, I suppose I could:
> > 
> > * Add a bit-mask of legal filesystems as a parameter to
> > fs_set_blk_dev().
> > 
> > * Move the body of do_fsload() into some common called by
> > do_fsload(), do_ext2load(), do_fatload(), each passing in the
> > appropriate bit-mask, which gets passed down to fs_set_blk_dev().
> > 
> > That'd be easy, and probably entail only extremely minimal
> > code-size increases for an ext2-only or FAT-only build; just a few
> > bytes for a few more function calls. Sound like a plan?
> 
> Sounds good to me.  I bet you'll find some other clean-ups while
> you're in there.  Marek, any comments from the DM perspective here?

CCing pavel, block goo is on his slate.

Best regards,
Marek Vasut