All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [RFC PATCH 1/2] fs: delete unused Makefile
@ 2012-10-11  0:05 Stephen Warren
  2012-10-11  0:05 ` [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands Stephen Warren
  2012-10-13  0:31 ` [U-Boot] [RFC PATCH 1/2] fs: delete unused Makefile Simon Glass
  0 siblings, 2 replies; 10+ messages in thread
From: Stephen Warren @ 2012-10-11  0:05 UTC (permalink / raw)
  To: u-boot

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>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands
  2012-10-11  0:05 [U-Boot] [RFC PATCH 1/2] fs: delete unused Makefile Stephen Warren
@ 2012-10-11  0:05 ` Stephen Warren
  2012-10-11 13:33   ` Benoît Thébaudeau
                     ` (2 more replies)
  2012-10-13  0:31 ` [U-Boot] [RFC PATCH 1/2] fs: delete unused Makefile Simon Glass
  1 sibling, 3 replies; 10+ messages in thread
From: Stephen Warren @ 2012-10-11  0:05 UTC (permalink / raw)
  To: u-boot

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.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
There are a couple FIXMEs in here:

1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or CONFIG_CMD_EXT2. This
means that the new commands and code can only be enabled if the "legacy"
{fat,ext2}{ls,load} are enabled. What we really want is CONFIG_FS_FAT and
CONFIG_FS_EXT2 to enable the filesystem code, and then CONFIG_CMD_FAT,
CONFIG_CMD_EXT2, CONFIG_CMD_FS to only affect the command implementations.
However, that would require making every include/config/*.h that sets the
current defines also set more. I suppose that's a fairly mechanical change
though, so easy enough to implement. Does that seem like a reasonable
approach to people?

2) In common/Makefile, I need to make this conditional upon CONFIG_CMD_FS
or similar.

Also, I wonder if the fs/* and common/* should be two separate patches or
not?

 Makefile        |    3 +-
 common/Makefile |    2 +
 common/cmd_fs.c |   86 ++++++++++++++++++++++
 fs/Makefile     |   47 ++++++++++++
 fs/fs.c         |  216 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/fs.h    |   49 +++++++++++++
 6 files changed, 402 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..4f2d944 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -103,6 +103,8 @@ COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
 ifdef CONFIG_FPGA
 COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
 endif
+# FIXME: Need to make this conditional
+COBJS-y += 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..948a5e0
--- /dev/null
+++ b/common/cmd_fs.c
@@ -0,0 +1,86 @@
+/*
+ * 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 -1;
+
+	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 (fs_set_blk_dev(argv[1], argv[2]))
+		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..ea77ac9
--- /dev/null
+++ b/fs/fs.c
@@ -0,0 +1,216 @@
+/*
+ * 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;
+
+/* FIXME: CONFIG_CMD_FAT, CONFIG_CMD_EXT2 should be CONFIG_FS_FAT, CONFIG_FS_EXT2 */
+#ifdef CONFIG_CMD_FAT
+static int fs_probe_fat(void)
+{
+	return fat_set_blk_dev(fs_dev_desc, &fs_partition);
+}
+
+static void fs_close_fat(void)
+{
+}
+#else
+static inline int fs_probe_fat(void)
+{
+	return -1;
+}
+
+static inline void fs_close_fat(void)
+{
+}
+#endif
+
+#ifdef CONFIG_CMD_EXT2
+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();
+}
+#else
+static inline int fs_probe_ext(void)
+{
+	return -1;
+}
+
+static inline void fs_close_ext(void)
+{
+}
+#endif
+
+int fs_set_blk_dev(const char *ifname, const char *dev_part_str)
+{
+	int part;
+
+	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;
+
+	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 inline int fs_ls_unsupported(const char *dirname)
+{
+	printf("** Unrecognized filesystem type **\n");
+	return -1;
+}
+
+#ifdef CONFIG_CMD_FAT
+#define fs_ls_fat file_fat_ls
+#else
+#define fs_ls_fat fs_ls_unsupported
+#endif
+
+#ifdef CONFIG_CMD_EXT2
+#define fs_ls_ext ext4fs_ls
+#else
+#define fs_ls_ext fs_ls_unsupported
+#endif
+
+int fs_ls(const char *dirname)
+{
+	switch (fs_type) {
+	case FS_TYPE_FAT:
+		return fs_ls_fat(dirname);
+	case FS_TYPE_EXT:
+		return fs_ls_ext(dirname);
+	default:
+		return fs_ls_unsupported(dirname);
+	}
+}
+
+static inline int fs_read_unsupported(const char *filename, ulong addr, int offset, int len)
+{
+	printf("** Unrecognized filesystem type **\n");
+	return -1;
+}
+
+#ifdef CONFIG_CMD_FAT
+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
+#define fs_read_fat fs_read_unsupported
+#endif
+
+#ifdef CONFIG_CMD_EXT2
+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
+#define fs_read_ext fs_read_unsupported
+#endif
+
+int fs_read(const char *filename, ulong addr, int offset, int len)
+{
+	switch (fs_type) {
+	case FS_TYPE_FAT:
+		return fs_read_fat(filename, addr, offset, len);
+	case FS_TYPE_EXT:
+		return fs_read_ext(filename, addr, offset, len);
+	default:
+		return fs_read_unsupported(filename, addr, offset, len);
+	}
+}
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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands
  2012-10-11  0:05 ` [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands Stephen Warren
@ 2012-10-11 13:33   ` Benoît Thébaudeau
  2012-10-11 16:47   ` Tom Rini
  2012-10-13 19:26   ` Pavel Herrmann
  2 siblings, 0 replies; 10+ messages in thread
From: Benoît Thébaudeau @ 2012-10-11 13:33 UTC (permalink / raw)
  To: u-boot

Hi Stephen,

On Thursday, October 11, 2012 2:05:07 AM, 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.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> There are a couple FIXMEs in here:
> 
> 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or CONFIG_CMD_EXT2.
> This
> means that the new commands and code can only be enabled if the
> "legacy"
> {fat,ext2}{ls,load} are enabled. What we really want is CONFIG_FS_FAT
> and
> CONFIG_FS_EXT2 to enable the filesystem code, and then
> CONFIG_CMD_FAT,
> CONFIG_CMD_EXT2, CONFIG_CMD_FS to only affect the command
> implementations.
> However, that would require making every include/config/*.h that sets
> the
> current defines also set more. I suppose that's a fairly mechanical
> change
> though, so easy enough to implement. Does that seem like a reasonable
> approach to people?

What about making CONFIG_CMD_<FS> auto-define CONFIG_FS_<FS>, just like with a
Kconfig select?

> 2) In common/Makefile, I need to make this conditional upon
> CONFIG_CMD_FS
> or similar.
> 
> Also, I wonder if the fs/* and common/* should be two separate
> patches or
> not?

For me, it's fine as a single patch.

>  Makefile        |    3 +-
>  common/Makefile |    2 +
>  common/cmd_fs.c |   86 ++++++++++++++++++++++
>  fs/Makefile     |   47 ++++++++++++
>  fs/fs.c         |  216
>  +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/fs.h    |   49 +++++++++++++
>  6 files changed, 402 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..4f2d944 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -103,6 +103,8 @@ COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
>  ifdef CONFIG_FPGA
>  COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
>  endif
> +# FIXME: Need to make this conditional
> +COBJS-y += 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..948a5e0
> --- /dev/null
> +++ b/common/cmd_fs.c
> @@ -0,0 +1,86 @@
> +/*
> + * 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 -1;

What about "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[])
> +{

What about adding
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
?

> +	if (fs_set_blk_dev(argv[1], argv[2]))
> +		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..ea77ac9
> --- /dev/null
> +++ b/fs/fs.c
> @@ -0,0 +1,216 @@
> +/*
> + * 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;

It should be initialized to FS_TYPE_INVALID to be clean.

> +
> +/* FIXME: CONFIG_CMD_FAT, CONFIG_CMD_EXT2 should be CONFIG_FS_FAT,
> CONFIG_FS_EXT2 */
> +#ifdef CONFIG_CMD_FAT
> +static int fs_probe_fat(void)
> +{
> +	return fat_set_blk_dev(fs_dev_desc, &fs_partition);
> +}
> +
> +static void fs_close_fat(void)
> +{
> +}
> +#else
> +static inline int fs_probe_fat(void)
> +{
> +	return -1;
> +}
> +
> +static inline void fs_close_fat(void)
> +{
> +}
> +#endif
> +
> +#ifdef CONFIG_CMD_EXT2
> +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();
> +}
> +#else
> +static inline int fs_probe_ext(void)
> +{
> +	return -1;
> +}
> +
> +static inline void fs_close_ext(void)
> +{
> +}
> +#endif
> +
> +int fs_set_blk_dev(const char *ifname, const char *dev_part_str)
> +{
> +	int part;
> +
> +	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;

What about adding an fs_close() function that would just be the piece of code
above? It would have to be called at the end of command functions invoking
fs_set_blk_dev(). In that way, ext4 would not be left open after such a command.
That would be better if something else is done using ext4 between 2 such
commands.

> +
> +	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 inline int fs_ls_unsupported(const char *dirname)
> +{
> +	printf("** Unrecognized filesystem type **\n");
> +	return -1;
> +}
> +
> +#ifdef CONFIG_CMD_FAT
> +#define fs_ls_fat file_fat_ls
> +#else
> +#define fs_ls_fat fs_ls_unsupported
> +#endif
> +
> +#ifdef CONFIG_CMD_EXT2
> +#define fs_ls_ext ext4fs_ls
> +#else
> +#define fs_ls_ext fs_ls_unsupported
> +#endif
> +
> +int fs_ls(const char *dirname)
> +{
> +	switch (fs_type) {
> +	case FS_TYPE_FAT:
> +		return fs_ls_fat(dirname);
> +	case FS_TYPE_EXT:
> +		return fs_ls_ext(dirname);
> +	default:
> +		return fs_ls_unsupported(dirname);
> +	}
> +}
> +
> +static inline int fs_read_unsupported(const char *filename, ulong
> addr, int offset, int len)
> +{
> +	printf("** Unrecognized filesystem type **\n");
> +	return -1;
> +}
> +
> +#ifdef CONFIG_CMD_FAT
> +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
> +#define fs_read_fat fs_read_unsupported
> +#endif
> +
> +#ifdef CONFIG_CMD_EXT2
> +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
> +#define fs_read_ext fs_read_unsupported
> +#endif
> +
> +int fs_read(const char *filename, ulong addr, int offset, int len)
> +{
> +	switch (fs_type) {
> +	case FS_TYPE_FAT:
> +		return fs_read_fat(filename, addr, offset, len);
> +	case FS_TYPE_EXT:
> +		return fs_read_ext(filename, addr, offset, len);
> +	default:
> +		return fs_read_unsupported(filename, addr, offset, len);
> +	}
> +}
> 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 */

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands
  2012-10-11  0:05 ` [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands Stephen Warren
  2012-10-11 13:33   ` Benoît Thébaudeau
@ 2012-10-11 16:47   ` Tom Rini
  2012-10-11 16:57     ` Stephen Warren
  2012-10-13 19:26   ` Pavel Herrmann
  2 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2012-10-11 16:47 UTC (permalink / raw)
  To: u-boot

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

On 10/10/12 17:05, 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.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- There are a
>  couple FIXMEs in here:
> 
> 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or CONFIG_CMD_EXT2.
>  This means that the new commands and code can only be enabled if 
> the "legacy" {fat,ext2}{ls,load} are enabled. What we really want 
> is CONFIG_FS_FAT and CONFIG_FS_EXT2 to enable the filesystem code,
>  and then CONFIG_CMD_FAT, CONFIG_CMD_EXT2, CONFIG_CMD_FS to only 
> affect the command implementations. However, that would require 
> making every include/config/*.h that sets the current defines also
>  set more. I suppose that's a fairly mechanical change though, so 
> easy enough to implement. Does that seem like a reasonable approach
> to people?

How about a new CONFIG_CMD_GENERIC_FS for the new ls/fsload (and any
later commands like write that get added) and once most filesystems
are converted we can think about a transition plan?

> 2) In common/Makefile, I need to make this conditional upon 
> CONFIG_CMD_FS or similar.
> 
> Also, I wonder if the fs/* and common/* should be two separate 
> patches or not?

One is fine.

- -- 
Tom

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

iQIcBAEBAgAGBQJQdvgNAAoJENk4IS6UOR1Wx3gP/iRvYq0khe6ZPRbUcPyTg0r8
tkNJGfglXba52GHjU6URwSrPbN9qOHzh6fD0x26jOdj1vWlMv6nvwmyo3ttvyRLe
4VF7tvTRp3Zv461vGwm8XwAXlDXVrnsWeC4veWxo+/ptFaq2FTWYTVNo2MsqHSIN
VNnxMdgtqUIU2kgx0uJst1Fl3olDaRlQmyf88SiRE2et21FQytk/LxAc50zmNr5J
UWgVgzoUP+RnZwnZK2CWL9cAuGbEyLjQvoKK8V72dMvKzZgMFpyMEdk0+onKFYe1
Byai7INodWIhWrtQj4nGC/1WQC+kCteMvF3OTjuGY/bfDPqLYx3071kocrgYWSW4
URvvdv6hn1l+evN5BQ1erwAekwfgrfcKkavJwmuVES3ZESrEyComqWEjajqeTe/6
uIpZo58oFGojJZK1HcDmaVFyj7nxAzXloupmHiKq+xfXHbv60ZUZO6InEos/ZCjZ
bpT92wyyqTeiD70glLvLRyStKzZidqeoVTkbGM0XUCA3d3RvxXdB4zfgIqDlhhCT
EfhxKVkAZAzjsEn+U1/y5RWEEdD+Zaqi2xKwA+Ken9TJ4LFsjcOiQDPVZYklD7qu
Xmte9GdxL4tSipu0hWxrkRjO7ap09wUoEk1d0jNSrwNbJALnUUkT4qfpYXcMaAhe
okjWSANQaBZxbM0ziEoT
=AnRj
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands
  2012-10-11 16:47   ` Tom Rini
@ 2012-10-11 16:57     ` Stephen Warren
  2012-10-11 17:05       ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Warren @ 2012-10-11 16:57 UTC (permalink / raw)
  To: u-boot

On 10/11/2012 10:47 AM, Tom Rini wrote:
> On 10/10/12 17:05, 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.
> 
>> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- There are
>> a couple FIXMEs in here:
> 
>> 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or
>> CONFIG_CMD_EXT2. This means that the new commands and code can
>> only be enabled if the "legacy" {fat,ext2}{ls,load} are enabled.
>> What we really want is CONFIG_FS_FAT and CONFIG_FS_EXT2 to enable
>> the filesystem code, and then CONFIG_CMD_FAT, CONFIG_CMD_EXT2,
>> CONFIG_CMD_FS to only affect the command implementations.
>> However, that would require making every include/config/*.h that
>> sets the current defines also set more. I suppose that's a fairly
>> mechanical change though, so easy enough to implement. Does that
>> seem like a reasonable approach to people?
> 
> How about a new CONFIG_CMD_GENERIC_FS for the new ls/fsload (and
> any later commands like write that get added) and once most
> filesystems are converted we can think about a transition plan?

Certainly.

The issue is that the filesystem code itself is conditionally included
based on CONFIG_CMD_FAT and CONFIG_CMD_EXT4. This would require that
in order to use the new generic commands, you also had to support the
old non-generic commands in order to get a linkable result. Perhaps
that's fine for now, but by separating the existing CONFIG_CMD_FAT
into two (CONFIG_CMD_FAT, CONFIG_FS_FAT), we could avoid that
requirement. As Benoit mentioned, perhaps we could arrange that
CONFIG_CMD_FAT selects CONFIG_FS_FAT somehow, although I haven't
looked at U-Boot's configuration system to see if that's possible yet.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands
  2012-10-11 16:57     ` Stephen Warren
@ 2012-10-11 17:05       ` Tom Rini
  2012-10-11 17:45         ` Benoît Thébaudeau
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2012-10-11 17:05 UTC (permalink / raw)
  To: u-boot

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

On 10/11/12 09:57, Stephen Warren wrote:
> On 10/11/2012 10:47 AM, Tom Rini wrote:
>> On 10/10/12 17:05, 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.
>> 
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- There 
>>> are a couple FIXMEs in here:
>> 
>>> 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or 
>>> CONFIG_CMD_EXT2. This means that the new commands and code can
>>>  only be enabled if the "legacy" {fat,ext2}{ls,load} are 
>>> enabled. What we really want is CONFIG_FS_FAT and 
>>> CONFIG_FS_EXT2 to enable the filesystem code, and then 
>>> CONFIG_CMD_FAT, CONFIG_CMD_EXT2, CONFIG_CMD_FS to only affect 
>>> the command implementations. However, that would require
>>> making every include/config/*.h that sets the current defines
>>> also set more. I suppose that's a fairly mechanical change
>>> though, so easy enough to implement. Does that seem like a
>>> reasonable approach to people?
>> 
>> How about a new CONFIG_CMD_GENERIC_FS for the new ls/fsload (and
>>  any later commands like write that get added) and once most 
>> filesystems are converted we can think about a transition plan?
> 
> Certainly.
> 
> The issue is that the filesystem code itself is conditionally 
> included based on CONFIG_CMD_FAT and CONFIG_CMD_EXT4. This would 
> require that in order to use the new generic commands, you also
> had to support the old non-generic commands in order to get a
> linkable result. Perhaps that's fine for now, but by separating the
> existing CONFIG_CMD_FAT into two (CONFIG_CMD_FAT, CONFIG_FS_FAT),
> we could avoid that requirement. As Benoit mentioned, perhaps we
> could arrange that CONFIG_CMD_FAT selects CONFIG_FS_FAT somehow,
> although I haven't looked at U-Boot's configuration system to see
> if that's possible yet.

Since we don't have Kconfig right now, it's not really easy to do that
kind of thing.  I think once we have more filesystems converted we can
work out doing the mechanical separate of fs/ from common/ for fs code.

- -- 
Tom

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

iQIcBAEBAgAGBQJQdvxhAAoJENk4IS6UOR1WOFIP/0VjExqx5FZ+Lau4wFtV8dI0
v5+jfwDUtG0L3h/lTv9NJkX5j9duXJVOcAkB4hVeWFAqd/fSdI0mlajPvR1rpS/J
luYFvR/XdlIoQe65CbrKhFEzHH3pwcbYXd/Zz9wx2cvxmTz7NvbM3KZsvECPpYtO
CEqNClM2e5Q0ebE/XKZ5xG8qJVWZbs48i8kkmgHQHsiplGEdf7IzNgOqiSFL5rgF
DT6oRacoAzOLBwMo5/ssIiT04GgM+3Nac1UV/4GPKiNt62k4p6OfniUN4sL6N6Yl
oq8aFR3ZWGZgHdHO+y+pp+4MLAlvbv8PxMXHhw/ZCCHgLR9sodvWQl/Pn3C9TFTC
0LdYgMvft5FBBqffI+XuPMomdKYLmmHtAhlh7lly9L5GQYw8iU25cfXP8ZeBUOX+
aY4nX/Wp2s+vtXawWUVv01r0HRZZp3r6FJNAXWvE9m2cB+7HiZQD+nJvkV7ZZk7E
6WRdRlI8NTLn6Sfh42d6AzigsYfAm2Xwp6B1/gGix075E30yhB1KGdxs/SpQW+0+
tdtFXij/jtxAyabTLNTivAXO44l6IBurtlAZqXeI1ur6gEJa0L1Ju0Mi6rFO/Vrg
aQ/KBo+1DM4QgXxYkcYPxJ+KIJKQdsmIWEmpni+sgaLBgO43Ebh+lkLKHCm5NB0l
kaIgld2V1OQQDmg/j4yW
=Tegq
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands
  2012-10-11 17:05       ` Tom Rini
@ 2012-10-11 17:45         ` Benoît Thébaudeau
  0 siblings, 0 replies; 10+ messages in thread
From: Benoît Thébaudeau @ 2012-10-11 17:45 UTC (permalink / raw)
  To: u-boot

On Thursday, October 11, 2012 7:05:37 PM, Tom Rini wrote:
> On 10/11/12 09:57, Stephen Warren wrote:
> > On 10/11/2012 10:47 AM, Tom Rini wrote:
> >> On 10/10/12 17:05, 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.
> >> 
> >>> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- There
> >>> are a couple FIXMEs in here:
> >> 
> >>> 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or
> >>> CONFIG_CMD_EXT2. This means that the new commands and code can
> >>>  only be enabled if the "legacy" {fat,ext2}{ls,load} are
> >>> enabled. What we really want is CONFIG_FS_FAT and
> >>> CONFIG_FS_EXT2 to enable the filesystem code, and then
> >>> CONFIG_CMD_FAT, CONFIG_CMD_EXT2, CONFIG_CMD_FS to only affect
> >>> the command implementations. However, that would require
> >>> making every include/config/*.h that sets the current defines
> >>> also set more. I suppose that's a fairly mechanical change
> >>> though, so easy enough to implement. Does that seem like a
> >>> reasonable approach to people?
> >> 
> >> How about a new CONFIG_CMD_GENERIC_FS for the new ls/fsload (and
> >>  any later commands like write that get added) and once most
> >> filesystems are converted we can think about a transition plan?
> > 
> > Certainly.
> > 
> > The issue is that the filesystem code itself is conditionally
> > included based on CONFIG_CMD_FAT and CONFIG_CMD_EXT4. This would
> > require that in order to use the new generic commands, you also
> > had to support the old non-generic commands in order to get a
> > linkable result. Perhaps that's fine for now, but by separating the
> > existing CONFIG_CMD_FAT into two (CONFIG_CMD_FAT, CONFIG_FS_FAT),
> > we could avoid that requirement. As Benoit mentioned, perhaps we
> > could arrange that CONFIG_CMD_FAT selects CONFIG_FS_FAT somehow,
> > although I haven't looked at U-Boot's configuration system to see
> > if that's possible yet.
> 
> Since we don't have Kconfig right now, it's not really easy to do
> that
> kind of thing.  I think once we have more filesystems converted we
> can
> work out doing the mechanical separate of fs/ from common/ for fs
> code.

This does not require Kconfig. include/config_fallbacks.h is already made for
automatic config fixups. The following lines could simply be added to it:

#if defined(CONFIG_CMD_FAT) && !defined(CONFIG_FS_FAT)
#define CONFIG_FS_FAT
#endif

Best regards,
Beno?t

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [RFC PATCH 1/2] fs: delete unused Makefile
  2012-10-11  0:05 [U-Boot] [RFC PATCH 1/2] fs: delete unused Makefile Stephen Warren
  2012-10-11  0:05 ` [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands Stephen Warren
@ 2012-10-13  0:31 ` Simon Glass
  1 sibling, 0 replies; 10+ messages in thread
From: Simon Glass @ 2012-10-13  0:31 UTC (permalink / raw)
  To: u-boot

Hi Stephen,

On Wed, Oct 10, 2012 at 5:05 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> 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.

I noticed this recently, and I can't see any reason for it, unless
there was an intent to remove the top-level Makefile stuff one day?

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

Acked-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

> ---
>  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 mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands
  2012-10-11  0:05 ` [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands Stephen Warren
  2012-10-11 13:33   ` Benoît Thébaudeau
  2012-10-11 16:47   ` Tom Rini
@ 2012-10-13 19:26   ` Pavel Herrmann
  2012-10-15 15:43     ` Stephen Warren
  2 siblings, 1 reply; 10+ messages in thread
From: Pavel Herrmann @ 2012-10-13 19:26 UTC (permalink / raw)
  To: u-boot

Hi

On Wednesday 10 October 2012 18:05:07 Stephen Warren wrote:
>  ...snip... 
>  Makefile        |    3 +-
>  common/Makefile |    2 +
>  common/cmd_fs.c |   86 ++++++++++++++++++++++
>  fs/Makefile     |   47 ++++++++++++
>  fs/fs.c         |  216
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fs.h    |  
> 49 +++++++++++++
>  6 files changed, 402 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

I dont see how this is a "partition switch library", when it doesnt deal with 
partitions at all. Care to explain? Or maybe call it "filesystem-auto 
detection" or something?

Best Regards
Pavel Herrmann

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands
  2012-10-13 19:26   ` Pavel Herrmann
@ 2012-10-15 15:43     ` Stephen Warren
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2012-10-15 15:43 UTC (permalink / raw)
  To: u-boot

On 10/13/2012 01:26 PM, Pavel Herrmann wrote:
> Hi
> 
> On Wednesday 10 October 2012 18:05:07 Stephen Warren wrote:
>>  ...snip... 
>>  Makefile        |    3 +-
>>  common/Makefile |    2 +
>>  common/cmd_fs.c |   86 ++++++++++++++++++++++
>>  fs/Makefile     |   47 ++++++++++++
>>  fs/fs.c         |  216
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fs.h    |  
>> 49 +++++++++++++
>>  6 files changed, 402 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
> 
> I dont see how this is a "partition switch library", when it doesnt deal with 
> partitions at all. Care to explain? Or maybe call it "filesystem-auto 
> detection" or something?

Oops. I will s/partition/filesystem/ in the patch subject.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-10-15 15:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-11  0:05 [U-Boot] [RFC PATCH 1/2] fs: delete unused Makefile Stephen Warren
2012-10-11  0:05 ` [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands Stephen Warren
2012-10-11 13:33   ` Benoît Thébaudeau
2012-10-11 16:47   ` Tom Rini
2012-10-11 16:57     ` Stephen Warren
2012-10-11 17:05       ` Tom Rini
2012-10-11 17:45         ` Benoît Thébaudeau
2012-10-13 19:26   ` Pavel Herrmann
2012-10-15 15:43     ` Stephen Warren
2012-10-13  0:31 ` [U-Boot] [RFC PATCH 1/2] fs: delete unused Makefile Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.