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

[U-Boot] [PATCH V4 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>
Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
Acked-by: Simon Glass <sjg@chromium.org>
---
v4: No change.
v3: No change.
v2: No change.
---
 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 V4 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>
Reviewed-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
---
v4: No change.
v3: No change.
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 9804cea..f22ed1b 100644
--- a/README
+++ b/README
@@ -801,9 +801,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 d6d55b9..323875f 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 3a5ef20..06536ba 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 23298fc..3b59d15 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 V4 3/3] fs: add filesystem 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.

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

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v4:
* Use FS_TYPE_* everywhere, rather than separate FS_* and FS_TYPE_*.
* Make fs_set_blk_dev() loop over a table of filesystems.
v3:
* Updated the implementation of the new commands to be suitable for use
  as the body of {fat,ext[24]}{ls,load} too.
* Enhanced the implementation to make more fsload parameters optional
  (load address, filename); they can now come from environment variables
  like ext2load supported.
* Moved implementation into fs.c.
* Removed cmd_ext_common.c.
* s/partition/filesystem/ in patch subject.
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         |    6 +-
 common/cmd_ext2.c       |   13 +--
 common/cmd_ext4.c       |   12 +--
 common/cmd_ext_common.c |  197 ------------------------------
 common/cmd_fat.c        |   76 +-----------
 common/cmd_fs.c         |   51 ++++++++
 fs/Makefile             |   47 +++++++
 fs/fs.c                 |  308 +++++++++++++++++++++++++++++++++++++++++++++++
 include/ext_common.h    |    3 -
 include/fs.h            |   65 ++++++++++
 11 files changed, 483 insertions(+), 298 deletions(-)
 delete mode 100644 common/cmd_ext_common.c
 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 ab34fa7..b92e2af 100644
--- a/Makefile
+++ b/Makefile
@@ -260,7 +260,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 a4eb477..cc22c52 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -88,11 +88,6 @@ COBJS-$(CONFIG_CMD_ELF) += cmd_elf.o
 COBJS-$(CONFIG_SYS_HUSH_PARSER) += cmd_exit.o
 COBJS-$(CONFIG_CMD_EXT4) += cmd_ext4.o
 COBJS-$(CONFIG_CMD_EXT2) += cmd_ext2.o
-ifdef CONFIG_CMD_EXT4
-COBJS-y	+= cmd_ext_common.o
-else
-COBJS-$(CONFIG_CMD_EXT2) += cmd_ext_common.o
-endif
 COBJS-$(CONFIG_CMD_FAT) += cmd_fat.o
 COBJS-$(CONFIG_CMD_FDC)$(CONFIG_CMD_FDOS) += cmd_fdc.o
 COBJS-$(CONFIG_OF_LIBFDT) += cmd_fdt.o fdt_support.o
@@ -102,6 +97,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_ext2.c b/common/cmd_ext2.c
index c27d9c7..06d0234 100644
--- a/common/cmd_ext2.c
+++ b/common/cmd_ext2.c
@@ -37,15 +37,11 @@
 /*
  * Ext2fs support
  */
-#include <common.h>
-#include <ext_common.h>
+#include <fs.h>
 
 int do_ext2ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	if (do_ext_ls(cmdtp, flag, argc, argv))
-		return -1;
-
-	return 0;
+	return do_ls(cmdtp, flag, argc, argv, FS_TYPE_EXT);
 }
 
 /******************************************************************************
@@ -53,10 +49,7 @@ int do_ext2ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
  */
 int do_ext2load (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	if (do_ext_load(cmdtp, flag, argc, argv))
-		return -1;
-
-	return 0;
+	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_EXT);
 }
 
 U_BOOT_CMD(
diff --git a/common/cmd_ext4.c b/common/cmd_ext4.c
index ca46561..237bc19 100644
--- a/common/cmd_ext4.c
+++ b/common/cmd_ext4.c
@@ -47,10 +47,10 @@
 #include <image.h>
 #include <linux/ctype.h>
 #include <asm/byteorder.h>
-#include <ext_common.h>
 #include <ext4fs.h>
 #include <linux/stat.h>
 #include <malloc.h>
+#include <fs.h>
 
 #if defined(CONFIG_CMD_USB) && defined(CONFIG_USB_STORAGE)
 #include <usb.h>
@@ -59,18 +59,12 @@
 int do_ext4_load(cmd_tbl_t *cmdtp, int flag, int argc,
 						char *const argv[])
 {
-	if (do_ext_load(cmdtp, flag, argc, argv))
-		return -1;
-
-	return 0;
+	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_EXT);
 }
 
 int do_ext4_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 {
-	if (do_ext_ls(cmdtp, flag, argc, argv))
-		return -1;
-
-	return 0;
+	return do_ls(cmdtp, flag, argc, argv, FS_TYPE_EXT);
 }
 
 #if defined(CONFIG_CMD_EXT4_WRITE)
diff --git a/common/cmd_ext_common.c b/common/cmd_ext_common.c
deleted file mode 100644
index 1952f4d..0000000
--- a/common/cmd_ext_common.c
+++ /dev/null
@@ -1,197 +0,0 @@
-/*
- * (C) Copyright 2011 - 2012 Samsung Electronics
- * EXT2/4 filesystem implementation in Uboot by
- * Uma Shankar <uma.shankar@samsung.com>
- * Manjunatha C Achar <a.manjunatha@samsung.com>
- *
- * Ext4fs support
- * made from existing cmd_ext2.c file of Uboot
- *
- * (C) Copyright 2004
- * esd gmbh <www.esd-electronics.com>
- * Reinhard Arlt <reinhard.arlt@esd-electronics.com>
- *
- * made from cmd_reiserfs by
- *
- * (C) Copyright 2003 - 2004
- * Sysgo Real-Time Solutions, AG <www.elinos.com>
- * Pavel Bartusek <pba@sysgo.com>
- *
- * 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
- *
- */
-
-/*
- * Changelog:
- *	0.1 - Newly created file for ext4fs support. Taken from cmd_ext2.c
- *	        file in uboot. Added ext4fs ls load and write support.
- */
-
-#include <common.h>
-#include <part.h>
-#include <config.h>
-#include <command.h>
-#include <image.h>
-#include <linux/ctype.h>
-#include <asm/byteorder.h>
-#include <ext_common.h>
-#include <ext4fs.h>
-#include <linux/stat.h>
-#include <malloc.h>
-
-#if defined(CONFIG_CMD_USB) && defined(CONFIG_USB_STORAGE)
-#include <usb.h>
-#endif
-
-#if !defined(CONFIG_DOS_PARTITION) && !defined(CONFIG_EFI_PARTITION)
-#error DOS or EFI partition support must be selected
-#endif
-
-#define DOS_PART_MAGIC_OFFSET		0x1fe
-#define DOS_FS_TYPE_OFFSET		0x36
-#define DOS_FS32_TYPE_OFFSET		0x52
-
-int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
-						char *const argv[])
-{
-	char *filename = NULL;
-	int dev, part;
-	ulong addr = 0;
-	int filelen;
-	disk_partition_t info;
-	block_dev_desc_t *dev_desc;
-	char buf[12];
-	unsigned long count;
-	const char *addr_str;
-
-	count = 0;
-	addr = simple_strtoul(argv[3], NULL, 16);
-	filename = getenv("bootfile");
-	switch (argc) {
-	case 3:
-		addr_str = getenv("loadaddr");
-		if (addr_str != NULL)
-			addr = simple_strtoul(addr_str, NULL, 16);
-		else
-			addr = CONFIG_SYS_LOAD_ADDR;
-
-		break;
-	case 4:
-		break;
-	case 5:
-		filename = argv[4];
-		break;
-	case 6:
-		filename = argv[4];
-		count = simple_strtoul(argv[5], NULL, 16);
-		break;
-
-	default:
-		return cmd_usage(cmdtp);
-	}
-
-	if (!filename) {
-		puts("** No boot file defined **\n");
-		return 1;
-	}
-
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
-	if (part < 0)
-		return 1;
-
-	dev = dev_desc->dev;
-	printf("Loading file \"%s\" from %s device %d%c%c\n",
-		filename, argv[1], dev,
-		part ? ':' : ' ', part ? part + '0' : ' ');
-
-	ext4fs_set_blk_dev(dev_desc, &info);
-
-	if (!ext4fs_mount(info.size)) {
-		printf("** Bad ext2 partition or disk - %s %d:%d **\n",
-		       argv[1], dev, part);
-		ext4fs_close();
-		goto fail;
-	}
-
-	filelen = ext4fs_open(filename);
-	if (filelen < 0) {
-		printf("** File not found %s\n", filename);
-		ext4fs_close();
-		goto fail;
-	}
-	if ((count < filelen) && (count != 0))
-		filelen = count;
-
-	if (ext4fs_read((char *)addr, filelen) != filelen) {
-		printf("** Unable to read \"%s\" from %s %d:%d **\n",
-		       filename, argv[1], dev, part);
-		ext4fs_close();
-		goto fail;
-	}
-
-	ext4fs_close();
-	/* Loading ok, update default load address */
-	load_addr = addr;
-
-	printf("%d bytes read\n", filelen);
-	sprintf(buf, "%X", filelen);
-	setenv("filesize", buf);
-
-	return 0;
-fail:
-	return 1;
-}
-
-int do_ext_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
-{
-	const char *filename = "/";
-	int dev;
-	int part;
-	block_dev_desc_t *dev_desc;
-	disk_partition_t info;
-
-	if (argc < 2)
-		return cmd_usage(cmdtp);
-
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
-	if (part < 0)
-		return 1;
-
-	if (argc == 4)
-		filename = argv[3];
-
-	dev = dev_desc->dev;
-	ext4fs_set_blk_dev(dev_desc, &info);
-
-	if (!ext4fs_mount(info.size)) {
-		printf("** Bad ext2 partition or disk - %s %d:%d **\n",
-		       argv[1], dev, part);
-		ext4fs_close();
-		goto fail;
-	}
-
-	if (ext4fs_ls(filename)) {
-		printf("** Error extfs_ls() **\n");
-		ext4fs_close();
-		goto fail;
-	};
-
-	ext4fs_close();
-	return 0;
-
-fail:
-	return 1;
-}
diff --git a/common/cmd_fat.c b/common/cmd_fat.c
index c38302d..c865d6d 100644
--- a/common/cmd_fat.c
+++ b/common/cmd_fat.c
@@ -31,54 +31,11 @@
 #include <ata.h>
 #include <part.h>
 #include <fat.h>
-
+#include <fs.h>
 
 int do_fat_fsload (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	long size;
-	unsigned long offset;
-	unsigned long count = 0;
-	unsigned long pos = 0;
-	char buf [12];
-	block_dev_desc_t *dev_desc=NULL;
-	disk_partition_t info;
-	int part, dev;
-
-	if (argc < 5) {
-		printf("usage: fatload <interface> [<dev[:part]>] "
-			"<addr> <filename> [bytes [pos]]\n");
-		return 1;
-	}
-
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
-	if (part < 0)
-		return 1;
-
-	dev = dev_desc->dev;
-	if (fat_set_blk_dev(dev_desc, &info) != 0) {
-		printf("\n** Unable to use %s %d:%d for fatload **\n",
-			argv[1], dev, part);
-		return 1;
-	}
-	offset = simple_strtoul(argv[3], NULL, 16);
-	if (argc >= 6)
-		count = simple_strtoul(argv[5], NULL, 16);
-	if (argc >= 7)
-		pos = simple_strtoul(argv[6], NULL, 16);
-	size = file_fat_read_at(argv[4], pos, (unsigned char *)offset, count);
-
-	if(size==-1) {
-		printf("\n** Unable to read \"%s\" from %s %d:%d **\n",
-			argv[4], argv[1], dev, part);
-		return 1;
-	}
-
-	printf("\n%ld bytes read\n", size);
-
-	sprintf(buf, "%lX", size);
-	setenv("filesize", buf);
-
-	return 0;
+	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_FAT);
 }
 
 
@@ -96,34 +53,7 @@ U_BOOT_CMD(
 
 int do_fat_ls (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
-	char *filename = "/";
-	int ret, dev, part;
-	block_dev_desc_t *dev_desc=NULL;
-	disk_partition_t info;
-
-	if (argc < 2) {
-		printf("usage: fatls <interface> [<dev[:part]>] [directory]\n");
-		return 0;
-	}
-
-	part = get_device_and_partition(argv[1], argv[2], &dev_desc, &info, 1);
-	if (part < 0)
-		return 1;
-
-	dev = dev_desc->dev;
-	if (fat_set_blk_dev(dev_desc, &info) != 0) {
-		printf("\n** Unable to use %s %d:%d for fatls **\n",
-			argv[1], dev, part);
-		return 1;
-	}
-	if (argc == 4)
-		ret = file_fat_ls(argv[3]);
-	else
-		ret = file_fat_ls(filename);
-
-	if(ret!=0)
-		printf("No Fat FS detected\n");
-	return ret;
+	return do_ls(cmdtp, flag, argc, argv, FS_TYPE_FAT);
 }
 
 U_BOOT_CMD(
diff --git a/common/cmd_fs.c b/common/cmd_fs.c
new file mode 100644
index 0000000..296124b
--- /dev/null
+++ b/common/cmd_fs.c
@@ -0,0 +1,51 @@
+/*
+ * 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_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_ANY);
+}
+
+U_BOOT_CMD(
+	fsload,	7,	0,	do_fsload_wrapper,
+	"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_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	return do_ls(cmdtp, flag, argc, argv, FS_TYPE_ANY);
+}
+
+U_BOOT_CMD(
+	ls,	4,	1,	do_ls_wrapper,
+	"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..23ffa25
--- /dev/null
+++ b/fs/fs.c
@@ -0,0 +1,308 @@
+/*
+ * 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>
+
+static block_dev_desc_t *fs_dev_desc;
+static disk_partition_t fs_partition;
+static int fs_type = FS_TYPE_ANY;
+
+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
+
+static const struct {
+	int fstype;
+	int (*probe)(void);
+} fstypes[] = {
+	{
+		.fstype = FS_TYPE_FAT,
+		.probe = fs_probe_fat,
+	},
+	{
+		.fstype = FS_TYPE_EXT,
+		.probe = fs_probe_ext,
+	},
+};
+
+int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
+{
+	int part, i;
+
+	part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc,
+					&fs_partition, 1);
+	if (part < 0)
+		return -1;
+
+	for (i = 0; i < ARRAY_SIZE(fstypes); i++) {
+		if ((fstype != FS_TYPE_ANY) && (fstype != fstypes[i].fstype))
+			continue;
+
+		if (!fstypes[i].probe()) {
+			fs_type = fstypes[i].fstype;
+			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_ANY;
+}
+
+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;
+}
+
+int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+		int fstype)
+{
+	unsigned long addr;
+	const char *addr_str;
+	const char *filename;
+	unsigned long bytes;
+	unsigned long pos;
+	int len_read;
+	char buf[12];
+
+	if (argc < 5)
+		return CMD_RET_USAGE;
+
+	if (fs_set_blk_dev(argv[1], argv[2], fstype))
+		return 1;
+
+	if (argc >= 4) {
+		addr = simple_strtoul(argv[3], NULL, 0);
+	} else {
+		addr_str = getenv("loadaddr");
+		if (addr_str != NULL)
+			addr = simple_strtoul(addr_str, NULL, 16);
+		else
+			addr = CONFIG_SYS_LOAD_ADDR;
+	}
+	if (argc >= 5) {
+		filename = argv[4];
+	} else {
+		filename = getenv("bootfile");
+		if (!filename) {
+			puts("** No boot file defined **\n");
+			return 1;
+		}
+	}
+	if (argc >= 6)
+		bytes = simple_strtoul(argv[5], NULL, 0);
+	else
+		bytes = 0;
+	if (argc >= 7)
+		pos = simple_strtoul(argv[6], NULL, 0);
+	else
+		pos = 0;
+
+	len_read = fs_read(filename, addr, pos, bytes);
+	if (len_read <= 0)
+		return 1;
+
+	printf("%d bytes read\n", len_read);
+
+	sprintf(buf, "0x%x", len_read);
+	setenv("filesize", buf);
+
+	return 0;
+}
+
+int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+	int fstype)
+{
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL, fstype))
+		return 1;
+
+	if (fs_ls(argc == 4 ? argv[3] : "/"))
+		return 1;
+
+	return 0;
+}
diff --git a/include/ext_common.h b/include/ext_common.h
index ce73857..86373a6 100644
--- a/include/ext_common.h
+++ b/include/ext_common.h
@@ -195,7 +195,4 @@ int do_ext4_load(cmd_tbl_t *cmdtp, int flag, int argc,
 int do_ext4_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
 int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc,
 				char *const argv[]);
-int do_ext_load(cmd_tbl_t *cmdtp, int flag, int argc,
-					char *const argv[]);
-int do_ext_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
 #endif
diff --git a/include/fs.h b/include/fs.h
new file mode 100644
index 0000000..f396d84
--- /dev/null
+++ b/include/fs.h
@@ -0,0 +1,65 @@
+/*
+ * 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
+
+#include <common.h>
+
+#define FS_TYPE_ANY	0
+#define FS_TYPE_FAT	1
+#define FS_TYPE_EXT	2
+
+/*
+ * 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. The identification process may be limited to a
+ * specific filesystem type by passing FS_* in the fstype parameter.
+ *
+ * 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, int fstype);
+
+/*
+ * 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);
+
+/*
+ * Common implementation for various filesystem commands, optionally limited
+ * to a specific filesystem type via the fstype parameter.
+ */
+int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+		int fstype);
+int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
+		int fstype);
+
+#endif /* _FS_H */
-- 
1.7.0.4

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

[Tom Rini]

On Mon, Oct 22, 2012 at 10:43:49AM -0600, Stephen Warren 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.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Beno??t Th??baudeau <benoit.thebaudeau@advansee.com>
> Acked-by: Simon Glass <sjg@chromium.org>

This, and the series, now applied to u-boot/master, 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/20121029/1f2d4722/attachment.pgp>

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

[Andreas Bießmann]

Dear all,

On 22.10.2012 18:43, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Implement "ls" and "fsload" commands that act like {fat,ext2}{ls,load},
> and transparently handle either file-system. This scheme could easily be
> extended to other filesystem types; I only didn't do it for zfs because
> I don't have any filesystems of that type to test with.
> 
> Replace the implementation of {fat,ext[24]}{ls,load} with this new code
> too.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

this patch (namely 045fa1e1142552799ad3203e9e0bc22a11e866ea) seems to
break avr32 on runtime. It seems there is a new array introduced (the
fstypes array in fs/fs.c) which do not provide a relocation method
(CONFIG_NEEDS_MANUAL_RELOC). This is currently only a weak assumption,
but has any other requiring manual relocation m86k, mips, nds32m sparc)
also encountered this problem?

> ---

<snip>

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

<snip>

> +
> +static const struct {
> +	int fstype;
> +	int (*probe)(void);
> +} fstypes[] = {
> +	{
> +		.fstype = FS_TYPE_FAT,
> +		.probe = fs_probe_fat,
> +	},
> +	{
> +		.fstype = FS_TYPE_EXT,
> +		.probe = fs_probe_ext,
> +	},
> +};

I currently think this should be manually relocated for those arches
which need the manual relocation.

> +
> +int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
> +{
> +	int part, i;
> +
> +	part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc,
> +					&fs_partition, 1);
> +	if (part < 0)
> +		return -1;
> +
> +	for (i = 0; i < ARRAY_SIZE(fstypes); i++) {
> +		if ((fstype != FS_TYPE_ANY) && (fstype != fstypes[i].fstype))
> +			continue;
> +
> +		if (!fstypes[i].probe()) {
> +			fs_type = fstypes[i].fstype;
> +			return 0;
> +		}
> +	}
> +
> +	printf("** Unrecognized filesystem type **\n");
> +	return -1;
> +}

Best regards

Andreas Bie?mann

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

[Tom Rini]

On Tue, Oct 30, 2012 at 12:05:37PM +0100, Andreas Bie?mann wrote:
> Dear all,
> 
> On 22.10.2012 18:43, Stephen Warren wrote:
> > From: Stephen Warren <swarren@nvidia.com>
> > 
> > Implement "ls" and "fsload" commands that act like {fat,ext2}{ls,load},
> > and transparently handle either file-system. This scheme could easily be
> > extended to other filesystem types; I only didn't do it for zfs because
> > I don't have any filesystems of that type to test with.
> > 
> > Replace the implementation of {fat,ext[24]}{ls,load} with this new code
> > too.
> > 
> > Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> this patch (namely 045fa1e1142552799ad3203e9e0bc22a11e866ea) seems to
> break avr32 on runtime. It seems there is a new array introduced (the
> fstypes array in fs/fs.c) which do not provide a relocation method
> (CONFIG_NEEDS_MANUAL_RELOC). This is currently only a weak assumption,
> but has any other requiring manual relocation m86k, mips, nds32m sparc)
> also encountered this problem?

Urk, sorry.  Can you prepare and test a patch?  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/20121030/d9c0af31/attachment.pgp>

[U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Without this, fstypes[].probe points at the wrong place, so calling the
function results in undefined behaviour.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Note: I have compile-tested this with the relocation code forcibly
enabled, but have no way of actually testing the result.
---
 fs/fs.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index 23ffa25..e148a07 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -21,6 +21,8 @@
 #include <fat.h>
 #include <fs.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static block_dev_desc_t *fs_dev_desc;
 static disk_partition_t fs_partition;
 static int fs_type = FS_TYPE_ANY;
@@ -141,7 +143,7 @@ static inline void fs_close_ext(void)
 #define fs_read_ext fs_read_unsupported
 #endif
 
-static const struct {
+static struct {
 	int fstype;
 	int (*probe)(void);
 } fstypes[] = {
@@ -158,6 +160,15 @@ static const struct {
 int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
 {
 	int part, i;
+#ifdef CONFIG_NEEDS_MANUAL_RELOC
+	static int relocated;
+
+	if (!relocated) {
+		for (i = 0; i < ARRAY_SIZE(fstypes); i++)
+			fstypes[i].probe += gd->reloc_off;
+		relocated = 1;
+	}
+#endif
 
 	part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc,
 					&fs_partition, 1);
-- 
1.7.0.4

[U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation

[Andreas Bießmann]

Commit 045fa1e1142552799ad3203e9e0bc22a11e866ea introduce an array with
filesystem accessors. On arches which need manual reloc this is broken cause the
function pointers still pointing to the privious location, fix it.

Signed-off-by: Andreas Bie?mann <andreas.devel@googlemail.com>
Cc: swarren at wwwdotorg.org
Cc: trini at ti.com
---
 fs/fs.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index 23ffa25..66feb30 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -21,6 +21,8 @@
 #include <fat.h>
 #include <fs.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 static block_dev_desc_t *fs_dev_desc;
 static disk_partition_t fs_partition;
 static int fs_type = FS_TYPE_ANY;
@@ -141,7 +143,7 @@ static inline void fs_close_ext(void)
 #define fs_read_ext fs_read_unsupported
 #endif
 
-static const struct {
+static struct {
 	int fstype;
 	int (*probe)(void);
 } fstypes[] = {
@@ -158,6 +160,18 @@ static const struct {
 int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
 {
 	int part, i;
+#ifdef CONFIG_NEEDS_MANUAL_RELOC
+	static int relocated = 0;
+
+	/* relocate fstypes[].probe */
+	if (!relocated) {
+		int i;
+		for (i = 0; i < ARRAY_SIZE(fstypes); i++)
+			if (fstypes[i].probe != NULL)
+				fstypes[i].probe += gd->reloc_off;
+		relocated = 1;
+	}
+#endif
 
 	part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc,
 					&fs_partition, 1);
-- 
1.8.0

[U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation

[Stephen Warren]

On 10/30/2012 12:29 PM, Andreas Bie?mann wrote:
> Commit 045fa1e1142552799ad3203e9e0bc22a11e866ea introduce an array with
> filesystem accessors. On arches which need manual reloc this is broken cause the
> function pointers still pointing to the privious location, fix it.

We found the same code to copy:-)

> diff --git a/fs/fs.c b/fs/fs.c

>  int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)

> +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> +	static int relocated = 0;

checkpatch bitches about the "= 0" there. I assume BSS initialization
isn't also broken when CONFIG_NEEDS_MANUAL_RELOC is enabled?

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

[Benoît Thébaudeau]

Hi Stephen,

On Monday, October 22, 2012 6:43:51 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Implement "ls" and "fsload" commands that act like
> {fat,ext2}{ls,load},
> and transparently handle either file-system. This scheme could easily
> be
> extended to other filesystem types; I only didn't do it for zfs
> because
> I don't have any filesystems of that type to test with.
> 
> Replace the implementation of {fat,ext[24]}{ls,load} with this new
> code
> too.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

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

[--snip--]
> diff --git a/fs/fs.c b/fs/fs.c
> new file mode 100644
> index 0000000..23ffa25
> --- /dev/null
> +++ b/fs/fs.c
> @@ -0,0 +1,308 @@
[--snip--]
> +int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[],
> +		int fstype)
> +{
> +	unsigned long addr;
> +	const char *addr_str;
> +	const char *filename;
> +	unsigned long bytes;
> +	unsigned long pos;
> +	int len_read;
> +	char buf[12];
> +
> +	if (argc < 5)

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

> +		return CMD_RET_USAGE;
> +
> +	if (fs_set_blk_dev(argv[1], argv[2], fstype))

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

> +		return 1;
> +
> +	if (argc >= 4) {
> +		addr = simple_strtoul(argv[3], NULL, 0);

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

> +	} else {
> +		addr_str = getenv("loadaddr");
> +		if (addr_str != NULL)
> +			addr = simple_strtoul(addr_str, NULL, 16);

Ditto.

> +		else
> +			addr = CONFIG_SYS_LOAD_ADDR;
> +	}
> +	if (argc >= 5) {
> +		filename = argv[4];
> +	} else {
> +		filename = getenv("bootfile");
> +		if (!filename) {
> +			puts("** No boot file defined **\n");
> +			return 1;
> +		}
> +	}
> +	if (argc >= 6)
> +		bytes = simple_strtoul(argv[5], NULL, 0);

Ditto.

> +	else
> +		bytes = 0;
> +	if (argc >= 7)
> +		pos = simple_strtoul(argv[6], NULL, 0);

Ditto.

> +	else
> +		pos = 0;
> +
> +	len_read = fs_read(filename, addr, pos, bytes);
> +	if (len_read <= 0)
> +		return 1;
> +
> +	printf("%d bytes read\n", len_read);
> +
> +	sprintf(buf, "0x%x", len_read);
> +	setenv("filesize", buf);
> +
> +	return 0;
> +}
> +
> +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> +	int fstype)
> +{
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +
> +	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL, fstype))
> +		return 1;
> +
> +	if (fs_ls(argc == 4 ? argv[3] : "/"))

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

> +		return 1;
> +
> +	return 0;
> +}
[--snip--]

Best regards,
Beno?t

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

[Stephen Warren]

On 10/30/2012 02:23 PM, Beno?t Th?baudeau wrote:
> Hi Stephen,
> 
> On Monday, October 22, 2012 6:43:51 PM, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> Implement "ls" and "fsload" commands that act like
>> {fat,ext2}{ls,load},
>> and transparently handle either file-system. This scheme could easily
>> be
>> extended to other filesystem types; I only didn't do it for zfs
>> because
>> I don't have any filesystems of that type to test with.
>>
>> Replace the implementation of {fat,ext[24]}{ls,load} with this new
>> code
>> too.

>> diff --git a/fs/fs.c b/fs/fs.c

>> +int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const

>> +		return 1;
>> +
>> +	if (argc >= 4) {
>> +		addr = simple_strtoul(argv[3], NULL, 0);
> 
> 0 is just natural here. However, this raises the issue of the users of the
> legacy fat and ext commands, which used 16 here. So should we use 0 because it
> is cleaner, or 16 in order not to break compatibility for existing users?

Oh yes, that is a problem.

I think we should use 0 here for the new commands at least; it has
always annoyed me that U-Boot assumes that clearly non-hex values (since
there is no 0x) are actually hex. I often have to manually convert
values because of this (e.g. host-based ls decimal output to U-Boot
command input expecting hex without leading 0x.)

However, we do need to fix this for the existing legacy commands. I
suggest we either:

a) Add a new parameter to do_fsload() indicating which base to use.

or:

b) Assume base 0 if fstype==FS_TYPE_ANY, otherwise use 16.

(a) is probably cleaner.

For the environment variables, I suppose we need to continue to
explicitly request base 16 conversion, since those variables could be
used with either the old or the new commands.

>> +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>> +	int fstype)
>> +{
>> +	if (argc < 2)
>> +		return CMD_RET_USAGE;
>> +
>> +	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL, fstype))
>> +		return 1;
>> +
>> +	if (fs_ls(argc == 4 ? argv[3] : "/"))
> 
> IMHO, it would be better to just ignore the possible extra arguments, like in:
> +	if (fs_ls(argc >= 4 ? argv[3] : "/"))

Here I don't agree. If the command expects a certain set of arguments,
we should validate that the user provided exactly that set, and no more.
If we allow arbitrary cruft, then if we need to add new arguments later,
we won't be able to guarantee that handling those extra arguments won't
break some existing broken usage of the command.

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

[Tom Rini]

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

On 10/30/12 14:18, Stephen Warren wrote:
> On 10/30/2012 02:23 PM, Beno?t Th?baudeau wrote:
>> Hi Stephen,
>> 
>> On Monday, October 22, 2012 6:43:51 PM, Stephen Warren wrote:
>>> From: Stephen Warren <swarren@nvidia.com>
>>> 
>>> Implement "ls" and "fsload" commands that act like 
>>> {fat,ext2}{ls,load}, and transparently handle either
>>> file-system. This scheme could easily be extended to other
>>> filesystem types; I only didn't do it for zfs because I don't
>>> have any filesystems of that type to test with.
>>> 
>>> Replace the implementation of {fat,ext[24]}{ls,load} with this
>>> new code too.
> 
>>> diff --git a/fs/fs.c b/fs/fs.c
> 
>>> +int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char *
>>> const
> 
>>> +		return 1; + +	if (argc >= 4) { +		addr =
>>> simple_strtoul(argv[3], NULL, 0);
>> 
>> 0 is just natural here. However, this raises the issue of the
>> users of the legacy fat and ext commands, which used 16 here. So
>> should we use 0 because it is cleaner, or 16 in order not to
>> break compatibility for existing users?
> 
> Oh yes, that is a problem.

I feel I should be donning a brown paper bag here as well.

> I think we should use 0 here for the new commands at least; it has 
> always annoyed me that U-Boot assumes that clearly non-hex values
> (since there is no 0x) are actually hex. I often have to manually
> convert values because of this (e.g. host-based ls decimal output
> to U-Boot command input expecting hex without leading 0x.)

I feel your pain (and have gotten in the habit of doing lots of printf
+ shell math) but don't like changing expectations on the users, even
if they are seemingly odd (since they've gotten used to them too).

> However, we do need to fix this for the existing legacy commands.
> I suggest we either:
> 
> a) Add a new parameter to do_fsload() indicating which base to
> use.
> 
> or:
> 
> b) Assume base 0 if fstype==FS_TYPE_ANY, otherwise use 16.
> 
> (a) is probably cleaner.
> 
> For the environment variables, I suppose we need to continue to 
> explicitly request base 16 conversion, since those variables could
> be used with either the old or the new commands.

I agree with (a) along with adding to the help which parts are read in
as decimal at the cli.

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

iQIcBAEBAgAGBQJQkEbSAAoJENk4IS6UOR1W6RsP/3Smcwi5j2SMtVe0P+YjGHrX
brNurEoFv60ezqEhssYjxwopTJJpl8Xo8kqS2pcY7JbR8DXszMhacoWhMDwqxqQD
DS0tgbMXmXC+hSOs9nM6WY/auJUHbFkC4BbTCaxN8VZ2ovhX/3n//g6Jpbc7G4uC
5Lt0uYercP1NHVEeEoKAyWwklsndS2/+ywj4LN8GWfa7eA/GY0/RIhx56XCSHYLd
ogXpOlRY0vZo0egj3a/UNMw3FLCy3XEOKQCGIU6TF5cYK+ZCS15irAFa+o1iv5oB
am8Qsu6R6mQr3IgqmM91YV29KlhDYoAfNc8OHJovDYkSvqqon9M3Trr57b3mo0ko
FYePs/w529yATS/FeuQgw62/dJd/dccNB/dvna7CjfOW/0B84R/xAAgCnPJn7Jhw
+AOjGcILrNGAUQP7kcdNRDRKHelBN1JKsiJdnQpX1m70iAviFAJRV1tl5jktKu8j
s57vLfN1uQ9qxcsZ7wyUKkFSfZm2AvdYAl85X9ATPYXKa6Z1acwECZbPXczwRHFm
7bdYZbB9dFIUrE8UMGqYbZUNVwcm+5i5d7pUHDD0rOzklyrqBgrQ/v4FzUSsQQdN
eTM8DyrgCqCIbIz5mg63Mhykc20eRwTipp8Laflvp8+/nFUo21lxfvz1wbSMg/n1
/8mEDflYHo4fGKzq414r
=J3B9
-----END PGP SIGNATURE-----

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

[Benoît Thébaudeau]

On Tuesday, October 30, 2012 10:18:03 PM, Stephen Warren wrote:
> On 10/30/2012 02:23 PM, Beno?t Th?baudeau wrote:
> >> +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> >> argv[],
> >> +	int fstype)
> >> +{
> >> +	if (argc < 2)
> >> +		return CMD_RET_USAGE;
> >> +
> >> +	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL,
> >> fstype))
> >> +		return 1;
> >> +
> >> +	if (fs_ls(argc == 4 ? argv[3] : "/"))
> > 
> > IMHO, it would be better to just ignore the possible extra
> > arguments, like in:
> > +	if (fs_ls(argc >= 4 ? argv[3] : "/"))
> 
> Here I don't agree. If the command expects a certain set of
> arguments,
> we should validate that the user provided exactly that set, and no
> more.
> If we allow arbitrary cruft, then if we need to add new arguments
> later,
> we won't be able to guarantee that handling those extra arguments
> won't
> break some existing broken usage of the command.

My comment was misleading. Actually, with the current code, do_ls() can not be
called (except directly) if there are more than 4 arguments, because of the way
the ls command is declared through U_BOOT_CMD(). Hence, if ">=" is used,
arguments can be added later without changing existing lines.

And if we consider a direct call to do_ls() skipping the command system, then
this function should return CMD_RET_USAGE if argc > 4.

Best regards,
Beno?t

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

[Stephen Warren]

On 10/30/2012 03:59 PM, Beno?t Th?baudeau wrote:
> On Tuesday, October 30, 2012 10:18:03 PM, Stephen Warren wrote:
>> On 10/30/2012 02:23 PM, Beno?t Th?baudeau wrote:
>>>> +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const
>>>> argv[],
>>>> +	int fstype)
>>>> +{
>>>> +	if (argc < 2)
>>>> +		return CMD_RET_USAGE;
>>>> +
>>>> +	if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL,
>>>> fstype))
>>>> +		return 1;
>>>> +
>>>> +	if (fs_ls(argc == 4 ? argv[3] : "/"))
>>>
>>> IMHO, it would be better to just ignore the possible extra
>>> arguments, like in:
>>> +	if (fs_ls(argc >= 4 ? argv[3] : "/"))
>>
>> Here I don't agree. If the command expects a certain set of
>> arguments,
>> we should validate that the user provided exactly that set, and no
>> more.
>> If we allow arbitrary cruft, then if we need to add new arguments
>> later,
>> we won't be able to guarantee that handling those extra arguments
>> won't
>> break some existing broken usage of the command.
> 
> My comment was misleading. Actually, with the current code, do_ls() can not be
> called (except directly) if there are more than 4 arguments, because of the way
> the ls command is declared through U_BOOT_CMD(). Hence, if ">=" is used,
> arguments can be added later without changing existing lines.

Ah OK, that makes sense.

> And if we consider a direct call to do_ls() skipping the command system, then
> this function should return CMD_RET_USAGE if argc > 4.

True.

[U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation

[Tom Rini]

On Tue, Oct 30, 2012 at 12:41:02PM -0600, Stephen Warren wrote:
> On 10/30/2012 12:29 PM, Andreas Bie??mann wrote:
> > Commit 045fa1e1142552799ad3203e9e0bc22a11e866ea introduce an array with
> > filesystem accessors. On arches which need manual reloc this is broken cause the
> > function pointers still pointing to the privious location, fix it.
> 
> We found the same code to copy:-)
> 
> > diff --git a/fs/fs.c b/fs/fs.c
> 
> >  int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
> 
> > +#ifdef CONFIG_NEEDS_MANUAL_RELOC
> > +	static int relocated = 0;
> 
> checkpatch bitches about the "= 0" there. I assume BSS initialization
> isn't also broken when CONFIG_NEEDS_MANUAL_RELOC is enabled?

The only other example of relocation this way also does an inital
assignment to 0.  Andreas, can you run-time test?  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/20121030/a40656d0/attachment.pgp>

[U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation

[Andreas Bießmann]

Dear Stephen Warren,

On 30.10.2012 19:41, Stephen Warren wrote:
> On 10/30/2012 12:29 PM, Andreas Bie?mann wrote:
>> Commit 045fa1e1142552799ad3203e9e0bc22a11e866ea introduce an array with
>> filesystem accessors. On arches which need manual reloc this is broken cause the
>> function pointers still pointing to the privious location, fix it.
> 
> We found the same code to copy:-)

we did ;)

>> diff --git a/fs/fs.c b/fs/fs.c
> 
>>  int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
> 
>> +#ifdef CONFIG_NEEDS_MANUAL_RELOC
>> +	static int relocated = 0;
> 
> checkpatch bitches about the "= 0" there. I assume BSS initialization
> isn't also broken when CONFIG_NEEDS_MANUAL_RELOC is enabled?

For AVR32 BSS initialization is working correctly, I do not know how the
other arches behave, but I think it is Ok to remove the '0' here.
Also the check for fstypes[i].probe != NULL in my patch seems to be not
necessary. So I would favor using your patch then.

Best regards

Andreas Bie?mann

[U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC

[Andreas Bießmann]

Dear Stephen Warren,

On 30.10.2012 18:50, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Without this, fstypes[].probe points at the wrong place, so calling the
> function results in undefined behaviour.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Tested on AVR32 (atstk1002)

Tested-by: Andreas Bie?mann <andreas.devel@googlemail.com>

> ---
> Note: I have compile-tested this with the relocation code forcibly
> enabled, but have no way of actually testing the result.
> ---

Best regards

Andreas Bie?mann

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

[Andreas Bießmann]

Dear Stephen Warren,

On 22.10.2012 18:43, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Implement "ls" and "fsload" commands that act like {fat,ext2}{ls,load},
> and transparently handle either file-system. This scheme could easily be
> extended to other filesystem types; I only didn't do it for zfs because
> I don't have any filesystems of that type to test with.
> 
> Replace the implementation of {fat,ext[24]}{ls,load} with this new code
> too.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v4:
> * Use FS_TYPE_* everywhere, rather than separate FS_* and FS_TYPE_*.
> * Make fs_set_blk_dev() loop over a table of filesystems.
> v3:
> * Updated the implementation of the new commands to be suitable for use
>   as the body of {fat,ext[24]}{ls,load} too.
> * Enhanced the implementation to make more fsload parameters optional
>   (load address, filename); they can now come from environment variables
>   like ext2load supported.
> * Moved implementation into fs.c.
> * Removed cmd_ext_common.c.
> * s/partition/filesystem/ in patch subject.
> 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         |    6 +-
>  common/cmd_ext2.c       |   13 +--
>  common/cmd_ext4.c       |   12 +--
>  common/cmd_ext_common.c |  197 ------------------------------
>  common/cmd_fat.c        |   76 +-----------
>  common/cmd_fs.c         |   51 ++++++++
>  fs/Makefile             |   47 +++++++
>  fs/fs.c                 |  308 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/ext_common.h    |    3 -
>  include/fs.h            |   65 ++++++++++
>  11 files changed, 483 insertions(+), 298 deletions(-)
>  delete mode 100644 common/cmd_ext_common.c
>  create mode 100644 common/cmd_fs.c
>  create mode 100644 fs/Makefile
>  create mode 100644 fs/fs.c
>  create mode 100644 include/fs.h
> 

<snip>

> diff --git a/common/cmd_fs.c b/common/cmd_fs.c
> new file mode 100644
> index 0000000..296124b
> --- /dev/null
> +++ b/common/cmd_fs.c
> @@ -0,0 +1,51 @@
> +/*
> + * 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_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_ANY);
> +}
> +
> +U_BOOT_CMD(
> +	fsload,	7,	0,	do_fsload_wrapper,

I just realized that cmd_jffs2.c also defines a U_BOOT_CMD fsload ...
this may also be a problem for users of the old command!

However if one defines CONFIG_CMD_JFFS2 _and_ CONFIG_CMD_FS_GENERIC the
linker will throw following warning:

---8<---
/tmp/build_avr32/common/cmd_jffs2.o:(.u_boot_list.cmd.fsload+0x0):
multiple definition of `_u_boot_list_cmd_fsload'
/tmp/build_avr32/common/cmd_fs.o:(.u_boot_list.cmd.fsload+0x0): first
defined here
/tmp/build_avr32/common/cmd_jffs2.o:(.u_boot_list.cmd.ls+0x0): multiple
definition of `_u_boot_list_cmd_ls'
/tmp/build_avr32/common/cmd_fs.o:(.u_boot_list.cmd.ls+0x0): first
defined here
--->8---

Was this intended?

Best regards

Andreas Bie?mann

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

[Stephen Warren]

On 10/31/2012 04:43 AM, Andreas Bie?mann wrote:
> Dear Stephen Warren,
> 
> On 22.10.2012 18:43, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> Implement "ls" and "fsload" commands that act like {fat,ext2}{ls,load},
>> and transparently handle either file-system. This scheme could easily be
>> extended to other filesystem types; I only didn't do it for zfs because
>> I don't have any filesystems of that type to test with.
>>
>> Replace the implementation of {fat,ext[24]}{ls,load} with this new code
>> too.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> ---
>> v4:
>> * Use FS_TYPE_* everywhere, rather than separate FS_* and FS_TYPE_*.
>> * Make fs_set_blk_dev() loop over a table of filesystems.
>> v3:
>> * Updated the implementation of the new commands to be suitable for use
>>   as the body of {fat,ext[24]}{ls,load} too.
>> * Enhanced the implementation to make more fsload parameters optional
>>   (load address, filename); they can now come from environment variables
>>   like ext2load supported.
>> * Moved implementation into fs.c.
>> * Removed cmd_ext_common.c.
>> * s/partition/filesystem/ in patch subject.
>> 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         |    6 +-
>>  common/cmd_ext2.c       |   13 +--
>>  common/cmd_ext4.c       |   12 +--
>>  common/cmd_ext_common.c |  197 ------------------------------
>>  common/cmd_fat.c        |   76 +-----------
>>  common/cmd_fs.c         |   51 ++++++++
>>  fs/Makefile             |   47 +++++++
>>  fs/fs.c                 |  308 +++++++++++++++++++++++++++++++++++++++++++++++
>>  include/ext_common.h    |    3 -
>>  include/fs.h            |   65 ++++++++++
>>  11 files changed, 483 insertions(+), 298 deletions(-)
>>  delete mode 100644 common/cmd_ext_common.c
>>  create mode 100644 common/cmd_fs.c
>>  create mode 100644 fs/Makefile
>>  create mode 100644 fs/fs.c
>>  create mode 100644 include/fs.h
>>
> 
> <snip>
> 
>> diff --git a/common/cmd_fs.c b/common/cmd_fs.c
>> new file mode 100644
>> index 0000000..296124b
>> --- /dev/null
>> +++ b/common/cmd_fs.c
>> @@ -0,0 +1,51 @@
>> +/*
>> + * 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_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> +{
>> +	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_ANY);
>> +}
>> +
>> +U_BOOT_CMD(
>> +	fsload,	7,	0,	do_fsload_wrapper,
> 
> I just realized that cmd_jffs2.c also defines a U_BOOT_CMD fsload ...
> this may also be a problem for users of the old command!
> 
> However if one defines CONFIG_CMD_JFFS2 _and_ CONFIG_CMD_FS_GENERIC the
> linker will throw following warning:
> 
> ---8<---
> /tmp/build_avr32/common/cmd_jffs2.o:(.u_boot_list.cmd.fsload+0x0):
> multiple definition of `_u_boot_list_cmd_fsload'
> /tmp/build_avr32/common/cmd_fs.o:(.u_boot_list.cmd.fsload+0x0): first
> defined here
> /tmp/build_avr32/common/cmd_jffs2.o:(.u_boot_list.cmd.ls+0x0): multiple
> definition of `_u_boot_list_cmd_ls'
> /tmp/build_avr32/common/cmd_fs.o:(.u_boot_list.cmd.ls+0x0): first
> defined here
> --->8---
> 
> Was this intended?

No, this is certainly a problem. I guess we need to rename the new command.

I had originally thought about calling it plain "load" rather than
"fsload" (just like it's "ls" not "fsls"). However, that seemed far too
generic. However, given this conflict, and the fact that I don't think
there's any existing "load" command, perhaps we have no choice but to
s/fsload/load/, unless anyone has any better ideas? "fileload"?

[U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC

[Tom Rini]

On Wed, Oct 31, 2012 at 10:47:22AM +0100, Andreas Bie?mann wrote:
> Dear Stephen Warren,
> 
> On 30.10.2012 18:50, Stephen Warren wrote:
> > From: Stephen Warren <swarren@nvidia.com>
> > 
> > Without this, fstypes[].probe points at the wrong place, so calling the
> > function results in undefined behaviour.
> > 
> > Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> Tested on AVR32 (atstk1002)
> 
> Tested-by: Andreas Bie?mann <andreas.devel@googlemail.com>

Applied to u-boot/master, 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/20121104/de25a932/attachment.pgp>