* [U-Boot] [PATCH V4 1/3] fs: delete unused Makefile
@ 2012-10-22 16:43 Stephen Warren
2012-10-22 16:43 ` [U-Boot] [PATCH V4 2/3] fs: separate CONFIG_FS_{FAT, EXT4} from CONFIG_CMD_{FAT, EXT*} Stephen Warren
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Stephen Warren @ 2012-10-22 16:43 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>
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [U-Boot] [PATCH V4 2/3] fs: separate CONFIG_FS_{FAT, EXT4} from CONFIG_CMD_{FAT, EXT*} 2012-10-22 16:43 [U-Boot] [PATCH V4 1/3] fs: delete unused Makefile Stephen Warren @ 2012-10-22 16:43 ` Stephen Warren 2012-10-22 16:43 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Stephen Warren 2012-10-29 22:55 ` [U-Boot] [PATCH V4 1/3] fs: delete unused Makefile Tom Rini 2 siblings, 0 replies; 20+ messages in thread From: Stephen Warren @ 2012-10-22 16:43 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-22 16:43 [U-Boot] [PATCH V4 1/3] fs: delete unused Makefile Stephen Warren 2012-10-22 16:43 ` [U-Boot] [PATCH V4 2/3] fs: separate CONFIG_FS_{FAT, EXT4} from CONFIG_CMD_{FAT, EXT*} Stephen Warren @ 2012-10-22 16:43 ` Stephen Warren 2012-10-30 11:05 ` Andreas Bießmann ` (2 more replies) 2012-10-29 22:55 ` [U-Boot] [PATCH V4 1/3] fs: delete unused Makefile Tom Rini 2 siblings, 3 replies; 20+ messages in thread From: Stephen Warren @ 2012-10-22 16:43 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 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-22 16:43 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Stephen Warren @ 2012-10-30 11:05 ` Andreas Bießmann 2012-10-30 16:47 ` Tom Rini 2012-10-30 17:50 ` [U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC Stephen Warren 2012-10-30 20:23 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Benoît Thébaudeau 2012-10-31 10:43 ` Andreas Bießmann 2 siblings, 2 replies; 20+ messages in thread From: Andreas Bießmann @ 2012-10-30 11:05 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-30 11:05 ` Andreas Bießmann @ 2012-10-30 16:47 ` Tom Rini 2012-10-30 18:29 ` [U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation Andreas Bießmann 2012-10-30 17:50 ` [U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC Stephen Warren 1 sibling, 1 reply; 20+ messages in thread From: Tom Rini @ 2012-10-30 16:47 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation 2012-10-30 16:47 ` Tom Rini @ 2012-10-30 18:29 ` Andreas Bießmann 2012-10-30 18:41 ` Stephen Warren 0 siblings, 1 reply; 20+ messages in thread From: Andreas Bießmann @ 2012-10-30 18:29 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation 2012-10-30 18:29 ` [U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation Andreas Bießmann @ 2012-10-30 18:41 ` Stephen Warren 2012-10-30 22:19 ` Tom Rini 2012-10-31 9:42 ` Andreas Bießmann 0 siblings, 2 replies; 20+ messages in thread From: Stephen Warren @ 2012-10-30 18:41 UTC (permalink / raw) To: u-boot 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? ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation 2012-10-30 18:41 ` Stephen Warren @ 2012-10-30 22:19 ` Tom Rini 2012-10-31 9:42 ` Andreas Bießmann 1 sibling, 0 replies; 20+ messages in thread From: Tom Rini @ 2012-10-30 22:19 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation 2012-10-30 18:41 ` Stephen Warren 2012-10-30 22:19 ` Tom Rini @ 2012-10-31 9:42 ` Andreas Bießmann 1 sibling, 0 replies; 20+ messages in thread From: Andreas Bießmann @ 2012-10-31 9:42 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC 2012-10-30 11:05 ` Andreas Bießmann 2012-10-30 16:47 ` Tom Rini @ 2012-10-30 17:50 ` Stephen Warren 2012-10-31 9:47 ` Andreas Bießmann 1 sibling, 1 reply; 20+ messages in thread From: Stephen Warren @ 2012-10-30 17:50 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC 2012-10-30 17:50 ` [U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC Stephen Warren @ 2012-10-31 9:47 ` Andreas Bießmann 2012-11-04 18:28 ` Tom Rini 0 siblings, 1 reply; 20+ messages in thread From: Andreas Bießmann @ 2012-10-31 9:47 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC 2012-10-31 9:47 ` Andreas Bießmann @ 2012-11-04 18:28 ` Tom Rini 0 siblings, 0 replies; 20+ messages in thread From: Tom Rini @ 2012-11-04 18:28 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-22 16:43 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Stephen Warren 2012-10-30 11:05 ` Andreas Bießmann @ 2012-10-30 20:23 ` Benoît Thébaudeau 2012-10-30 21:18 ` Stephen Warren 2012-10-31 10:43 ` Andreas Bießmann 2 siblings, 1 reply; 20+ messages in thread From: Benoît Thébaudeau @ 2012-10-30 20:23 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-30 20:23 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Benoît Thébaudeau @ 2012-10-30 21:18 ` Stephen Warren 2012-10-30 21:29 ` Tom Rini 2012-10-30 21:59 ` Benoît Thébaudeau 0 siblings, 2 replies; 20+ messages in thread From: Stephen Warren @ 2012-10-30 21:18 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-30 21:18 ` Stephen Warren @ 2012-10-30 21:29 ` Tom Rini 2012-10-30 21:59 ` Benoît Thébaudeau 1 sibling, 0 replies; 20+ messages in thread From: Tom Rini @ 2012-10-30 21:29 UTC (permalink / raw) To: u-boot -----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----- ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-30 21:18 ` Stephen Warren 2012-10-30 21:29 ` Tom Rini @ 2012-10-30 21:59 ` Benoît Thébaudeau 2012-10-30 22:01 ` Stephen Warren 1 sibling, 1 reply; 20+ messages in thread From: Benoît Thébaudeau @ 2012-10-30 21:59 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-30 21:59 ` Benoît Thébaudeau @ 2012-10-30 22:01 ` Stephen Warren 0 siblings, 0 replies; 20+ messages in thread From: Stephen Warren @ 2012-10-30 22:01 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-22 16:43 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Stephen Warren 2012-10-30 11:05 ` Andreas Bießmann 2012-10-30 20:23 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Benoît Thébaudeau @ 2012-10-31 10:43 ` Andreas Bießmann 2012-10-31 17:03 ` Stephen Warren 2 siblings, 1 reply; 20+ messages in thread From: Andreas Bießmann @ 2012-10-31 10:43 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands 2012-10-31 10:43 ` Andreas Bießmann @ 2012-10-31 17:03 ` Stephen Warren 0 siblings, 0 replies; 20+ messages in thread From: Stephen Warren @ 2012-10-31 17:03 UTC (permalink / raw) To: u-boot 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"? ^ permalink raw reply [flat|nested] 20+ messages in thread
* [U-Boot] [PATCH V4 1/3] fs: delete unused Makefile 2012-10-22 16:43 [U-Boot] [PATCH V4 1/3] fs: delete unused Makefile Stephen Warren 2012-10-22 16:43 ` [U-Boot] [PATCH V4 2/3] fs: separate CONFIG_FS_{FAT, EXT4} from CONFIG_CMD_{FAT, EXT*} Stephen Warren 2012-10-22 16:43 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Stephen Warren @ 2012-10-29 22:55 ` Tom Rini 2 siblings, 0 replies; 20+ messages in thread From: Tom Rini @ 2012-10-29 22:55 UTC (permalink / raw) To: u-boot 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> ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2012-11-04 18:28 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-22 16:43 [U-Boot] [PATCH V4 1/3] fs: delete unused Makefile Stephen Warren
2012-10-22 16:43 ` [U-Boot] [PATCH V4 2/3] fs: separate CONFIG_FS_{FAT, EXT4} from CONFIG_CMD_{FAT, EXT*} Stephen Warren
2012-10-22 16:43 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Stephen Warren
2012-10-30 11:05 ` Andreas Bießmann
2012-10-30 16:47 ` Tom Rini
2012-10-30 18:29 ` [U-Boot] [PATCH] fs/fs.c: fix fs_set_blk_dev() for manual relocation Andreas Bießmann
2012-10-30 18:41 ` Stephen Warren
2012-10-30 22:19 ` Tom Rini
2012-10-31 9:42 ` Andreas Bießmann
2012-10-30 17:50 ` [U-Boot] [PATCH] fs: handle CONFIG_NEEDS_MANUAL_RELOC Stephen Warren
2012-10-31 9:47 ` Andreas Bießmann
2012-11-04 18:28 ` Tom Rini
2012-10-30 20:23 ` [U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands Benoît Thébaudeau
2012-10-30 21:18 ` Stephen Warren
2012-10-30 21:29 ` Tom Rini
2012-10-30 21:59 ` Benoît Thébaudeau
2012-10-30 22:01 ` Stephen Warren
2012-10-31 10:43 ` Andreas Bießmann
2012-10-31 17:03 ` Stephen Warren
2012-10-29 22:55 ` [U-Boot] [PATCH V4 1/3] fs: delete unused Makefile Tom Rini
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.