Message ID | 1349913907-25845-2-git-send-email-swarren@wwwdotorg.org |
---|---|
State | RFC |
Headers | show |
Hi Stephen, On Thursday, October 11, 2012 2:05:07 AM, Stephen Warren wrote: > Implement "ls" and "fsload" commands that act like > {fat,ext2}{ls,load}, > and transparently handle either file-system. This scheme could easily > be > extended to other filesystem types; I only didn't do it for zfs > because > I don't have any filesystems of that type. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > There are a couple FIXMEs in here: > > 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or CONFIG_CMD_EXT2. > This > means that the new commands and code can only be enabled if the > "legacy" > {fat,ext2}{ls,load} are enabled. What we really want is CONFIG_FS_FAT > and > CONFIG_FS_EXT2 to enable the filesystem code, and then > CONFIG_CMD_FAT, > CONFIG_CMD_EXT2, CONFIG_CMD_FS to only affect the command > implementations. > However, that would require making every include/config/*.h that sets > the > current defines also set more. I suppose that's a fairly mechanical > change > though, so easy enough to implement. Does that seem like a reasonable > approach to people? What about making CONFIG_CMD_<FS> auto-define CONFIG_FS_<FS>, just like with a Kconfig select? > 2) In common/Makefile, I need to make this conditional upon > CONFIG_CMD_FS > or similar. > > Also, I wonder if the fs/* and common/* should be two separate > patches or > not? For me, it's fine as a single patch. > Makefile | 3 +- > common/Makefile | 2 + > common/cmd_fs.c | 86 ++++++++++++++++++++++ > fs/Makefile | 47 ++++++++++++ > fs/fs.c | 216 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > include/fs.h | 49 +++++++++++++ > 6 files changed, 402 insertions(+), 1 deletions(-) > create mode 100644 common/cmd_fs.c > create mode 100644 fs/Makefile > create mode 100644 fs/fs.c > create mode 100644 include/fs.h > > diff --git a/Makefile b/Makefile > index d7e2c2f..0b50eb7 100644 > --- a/Makefile > +++ b/Makefile > @@ -242,7 +242,8 @@ LIBS-y += drivers/net/npe/libnpe.o > endif > LIBS-$(CONFIG_OF_EMBED) += dts/libdts.o > LIBS-y += arch/$(ARCH)/lib/lib$(ARCH).o > -LIBS-y += fs/cramfs/libcramfs.o \ > +LIBS-y += fs/libfs.o \ > + fs/cramfs/libcramfs.o \ > fs/ext4/libext4fs.o \ > fs/fat/libfat.o \ > fs/fdos/libfdos.o \ > diff --git a/common/Makefile b/common/Makefile > index 125b2be..4f2d944 100644 > --- a/common/Makefile > +++ b/common/Makefile > @@ -103,6 +103,8 @@ COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o > ifdef CONFIG_FPGA > COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o > endif > +# FIXME: Need to make this conditional > +COBJS-y += cmd_fs.o > COBJS-$(CONFIG_CMD_GPIO) += cmd_gpio.o > COBJS-$(CONFIG_CMD_I2C) += cmd_i2c.o > COBJS-$(CONFIG_CMD_IDE) += cmd_ide.o > diff --git a/common/cmd_fs.c b/common/cmd_fs.c > new file mode 100644 > index 0000000..948a5e0 > --- /dev/null > +++ b/common/cmd_fs.c > @@ -0,0 +1,86 @@ > +/* > + * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved. > + * > + * Inspired by cmd_ext_common.c, cmd_fat.c. > + * > + * This program is free software; you can redistribute it and/or > modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but > WITHOUT > + * ANY WARRANTY; without even the implied warranty of > MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public > License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see > <http://www.gnu.org/licenses/>. > + */ > + > +#include <common.h> > +#include <command.h> > +#include <fs.h> > + > +int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const > argv[]) > +{ > + unsigned long addr; > + unsigned long bytes; > + unsigned long pos; > + int len_read; > + char buf[12]; > + > + if (argc < 5) > + return -1; What about "return CMD_RET_USAGE;"? > + > + if (fs_set_blk_dev(argv[1], argv[2])) > + return 1; > + > + addr = simple_strtoul(argv[3], NULL, 0); > + if (argc >= 6) > + bytes = simple_strtoul(argv[5], NULL, 0); > + else > + bytes = 0; > + if (argc >= 7) > + pos = simple_strtoul(argv[6], NULL, 0); > + else > + pos = 0; > + > + len_read = fs_read(argv[4], addr, pos, bytes); > + if (len_read <= 0) > + return 1; > + > + sprintf(buf, "0x%x", (unsigned int)len_read); > + setenv("filesize", buf); > + > + return 0; > +} > + > +U_BOOT_CMD( > + fsload, 7, 0, do_fsload, > + "load binary file from a filesystem", > + "<interface> [<dev[:part]>] <addr> <filename> [bytes [pos]]\n" > + " - Load binary file 'filename' from partition 'part' on > device\n" > + " type 'interface' instance 'dev' to address 'addr' in > memory.\n" > + " 'bytes' gives the size to load in bytes.\n" > + " If 'bytes' is 0 or omitted, the file is read until the > end.\n" > + " 'pos' gives the file byte position to start reading from.\n" > + " If 'pos' is 0 or omitted, the file is read from the start." > +); > + > +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > +{ What about adding + if (argc < 2) + return CMD_RET_USAGE; + ? > + if (fs_set_blk_dev(argv[1], argv[2])) > + return 1; > + > + if (fs_ls(argc == 4 ? argv[3] : "/")) > + return 1; > + > + return 0; > +} > + > +U_BOOT_CMD( > + ls, 4, 1, do_ls, > + "list files in a directory (default /)", > + "<interface> [<dev[:part]>] [directory]\n" > + " - List files in directory 'directory' of partition 'part' > on\n" > + " device type 'interface' instance 'dev'." > +); > diff --git a/fs/Makefile b/fs/Makefile > new file mode 100644 > index 0000000..d0ab3ae > --- /dev/null > +++ b/fs/Makefile > @@ -0,0 +1,47 @@ > +# > +# (C) Copyright 2000-2006 > +# Wolfgang Denk, DENX Software Engineering, wd@denx.de. > +# Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved. > +# > +# See file CREDITS for list of people who contributed to this > +# project. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of the GNU General Public License as > +# published by the Free Software Foundation; either version 2 of > +# the License, or (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write to the Free Software > +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, > +# MA 02111-1307 USA > +# > + > +include $(TOPDIR)/config.mk > + > +LIB = $(obj)libfs.o > + > +COBJS-y += fs.o > + > +COBJS := $(COBJS-y) > +SRCS := $(COBJS:.o=.c) > +OBJS := $(addprefix $(obj),$(COBJS)) > + > +all: $(LIB) > + > +$(LIB): $(obj).depend $(OBJS) > + $(call cmd_link_o_target, $(OBJS)) > + > +######################################################################### > + > +# defines $(obj).depend target > +include $(SRCTREE)/rules.mk > + > +sinclude $(obj).depend > + > +######################################################################### > diff --git a/fs/fs.c b/fs/fs.c > new file mode 100644 > index 0000000..ea77ac9 > --- /dev/null > +++ b/fs/fs.c > @@ -0,0 +1,216 @@ > +/* > + * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or > modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but > WITHOUT > + * ANY WARRANTY; without even the implied warranty of > MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public > License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see > <http://www.gnu.org/licenses/>. > + */ > + > +#include <config.h> > +#include <common.h> > +#include <part.h> > +#include <ext4fs.h> > +#include <fat.h> > +#include <fs.h> > + > +#define FS_TYPE_INVALID 0 > +#define FS_TYPE_FAT 1 > +#define FS_TYPE_EXT 2 > + > +static block_dev_desc_t *fs_dev_desc; > +static disk_partition_t fs_partition; > +static int fs_type; It should be initialized to FS_TYPE_INVALID to be clean. > + > +/* FIXME: CONFIG_CMD_FAT, CONFIG_CMD_EXT2 should be CONFIG_FS_FAT, > CONFIG_FS_EXT2 */ > +#ifdef CONFIG_CMD_FAT > +static int fs_probe_fat(void) > +{ > + return fat_set_blk_dev(fs_dev_desc, &fs_partition); > +} > + > +static void fs_close_fat(void) > +{ > +} > +#else > +static inline int fs_probe_fat(void) > +{ > + return -1; > +} > + > +static inline void fs_close_fat(void) > +{ > +} > +#endif > + > +#ifdef CONFIG_CMD_EXT2 > +static int fs_probe_ext(void) > +{ > + ext4fs_set_blk_dev(fs_dev_desc, &fs_partition); > + > + if (!ext4fs_mount(fs_partition.size)) { > + ext4fs_close(); > + return -1; > + } > + > + return 0; > +} > + > +static void fs_close_ext(void) > +{ > + ext4fs_close(); > +} > +#else > +static inline int fs_probe_ext(void) > +{ > + return -1; > +} > + > +static inline void fs_close_ext(void) > +{ > +} > +#endif > + > +int fs_set_blk_dev(const char *ifname, const char *dev_part_str) > +{ > + int part; > + > + switch (fs_type) { > + case FS_TYPE_FAT: > + fs_close_fat(); > + break; > + case FS_TYPE_EXT: > + fs_close_ext(); > + break; > + default: > + break; > + } > + > + fs_type = FS_TYPE_INVALID; What about adding an fs_close() function that would just be the piece of code above? It would have to be called at the end of command functions invoking fs_set_blk_dev(). In that way, ext4 would not be left open after such a command. That would be better if something else is done using ext4 between 2 such commands. > + > + part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc, > + &fs_partition, 1); > + if (part < 0) > + return -1; > + > + if (!fs_probe_fat()) { > + fs_type = FS_TYPE_FAT; > + return 0; > + } > + > + if (!fs_probe_ext()) { > + fs_type = FS_TYPE_EXT; > + return 0; > + } > + > + printf("** Unrecognized filesystem type **\n"); > + return -1; > +} > + > +static inline int fs_ls_unsupported(const char *dirname) > +{ > + printf("** Unrecognized filesystem type **\n"); > + return -1; > +} > + > +#ifdef CONFIG_CMD_FAT > +#define fs_ls_fat file_fat_ls > +#else > +#define fs_ls_fat fs_ls_unsupported > +#endif > + > +#ifdef CONFIG_CMD_EXT2 > +#define fs_ls_ext ext4fs_ls > +#else > +#define fs_ls_ext fs_ls_unsupported > +#endif > + > +int fs_ls(const char *dirname) > +{ > + switch (fs_type) { > + case FS_TYPE_FAT: > + return fs_ls_fat(dirname); > + case FS_TYPE_EXT: > + return fs_ls_ext(dirname); > + default: > + return fs_ls_unsupported(dirname); > + } > +} > + > +static inline int fs_read_unsupported(const char *filename, ulong > addr, int offset, int len) > +{ > + printf("** Unrecognized filesystem type **\n"); > + return -1; > +} > + > +#ifdef CONFIG_CMD_FAT > +static int fs_read_fat(const char *filename, ulong addr, int offset, > int len) > +{ > + int len_read; > + > + len_read = file_fat_read_at(filename, offset, > + (unsigned char *)addr, len); > + if (len_read == -1) { > + printf("** Unable to read file %s **\n", filename); > + return -1; > + } > + > + return len_read; > +} > +#else > +#define fs_read_fat fs_read_unsupported > +#endif > + > +#ifdef CONFIG_CMD_EXT2 > +static int fs_read_ext(const char *filename, ulong addr, int offset, > int len) > +{ > + int file_len; > + int len_read; > + > + if (offset != 0) { > + printf("** Cannot support non-zero offset **\n"); > + return -1; > + } > + > + file_len = ext4fs_open(filename); > + if (file_len < 0) { > + printf("** File not found %s **\n", filename); > + ext4fs_close(); > + return -1; > + } > + > + if (len == 0) > + len = file_len; > + > + len_read = ext4fs_read((char *)addr, len); > + ext4fs_close(); > + > + if (len_read != len) { > + printf("** Unable to read file %s **\n", filename); > + return -1; > + } > + > + return len_read; > +} > +#else > +#define fs_read_ext fs_read_unsupported > +#endif > + > +int fs_read(const char *filename, ulong addr, int offset, int len) > +{ > + switch (fs_type) { > + case FS_TYPE_FAT: > + return fs_read_fat(filename, addr, offset, len); > + case FS_TYPE_EXT: > + return fs_read_ext(filename, addr, offset, len); > + default: > + return fs_read_unsupported(filename, addr, offset, len); > + } > +} > diff --git a/include/fs.h b/include/fs.h > new file mode 100644 > index 0000000..a0fda28 > --- /dev/null > +++ b/include/fs.h > @@ -0,0 +1,49 @@ > +/* > + * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or > modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but > WITHOUT > + * ANY WARRANTY; without even the implied warranty of > MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public > License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see > <http://www.gnu.org/licenses/>. > + */ > +#ifndef _FS_H > +#define _FS_H > + > +/* > + * Tell the fs layer which block device an partition to use for > future > + * commands. This also internally identifies the filesystem that is > present > + * within the partition. > + * > + * Returns 0 on success. > + * Returns non-zero if there is an error accessing the disk or > partition, or > + * no known filesystem type could be recognized on it. > + */ > +int fs_set_blk_dev(const char *ifname, const char *dev_part_str); > + > +/* > + * Print the list of files on the partition previously set by > fs_set_blk_dev(), > + * in directory "dirname". > + * > + * Returns 0 on success. Returns non-zero on error. > + */ > +int fs_ls(const char *dirname); > + > +/* > + * Read file "filename" from the partition previously set by > fs_set_blk_dev(), > + * to address "addr", starting at byte offset "offset", and reading > "len" > + * bytes. "offset" may be 0 to read from the start of the file. > "len" may be > + * 0 to read the entire file. Note that not all filesystem types > support > + * either/both offset!=0 or len!=0. > + * > + * Returns number of bytes read on success. Returns <= 0 on error. > + */ > +int fs_read(const char *filename, ulong addr, int offset, int len); > + > +#endif /* _FS_H */ Best regards, Benoît
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10/10/12 17:05, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > Implement "ls" and "fsload" commands that act like > {fat,ext2}{ls,load}, and transparently handle either file-system. > This scheme could easily be extended to other filesystem types; I > only didn't do it for zfs because I don't have any filesystems of > that type. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> --- There are a > couple FIXMEs in here: > > 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or CONFIG_CMD_EXT2. > This means that the new commands and code can only be enabled if > the "legacy" {fat,ext2}{ls,load} are enabled. What we really want > is CONFIG_FS_FAT and CONFIG_FS_EXT2 to enable the filesystem code, > and then CONFIG_CMD_FAT, CONFIG_CMD_EXT2, CONFIG_CMD_FS to only > affect the command implementations. However, that would require > making every include/config/*.h that sets the current defines also > set more. I suppose that's a fairly mechanical change though, so > easy enough to implement. Does that seem like a reasonable approach > to people? How about a new CONFIG_CMD_GENERIC_FS for the new ls/fsload (and any later commands like write that get added) and once most filesystems are converted we can think about a transition plan? > 2) In common/Makefile, I need to make this conditional upon > CONFIG_CMD_FS or similar. > > Also, I wonder if the fs/* and common/* should be two separate > patches or not? One is fine. - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQdvgNAAoJENk4IS6UOR1Wx3gP/iRvYq0khe6ZPRbUcPyTg0r8 tkNJGfglXba52GHjU6URwSrPbN9qOHzh6fD0x26jOdj1vWlMv6nvwmyo3ttvyRLe 4VF7tvTRp3Zv461vGwm8XwAXlDXVrnsWeC4veWxo+/ptFaq2FTWYTVNo2MsqHSIN VNnxMdgtqUIU2kgx0uJst1Fl3olDaRlQmyf88SiRE2et21FQytk/LxAc50zmNr5J UWgVgzoUP+RnZwnZK2CWL9cAuGbEyLjQvoKK8V72dMvKzZgMFpyMEdk0+onKFYe1 Byai7INodWIhWrtQj4nGC/1WQC+kCteMvF3OTjuGY/bfDPqLYx3071kocrgYWSW4 URvvdv6hn1l+evN5BQ1erwAekwfgrfcKkavJwmuVES3ZESrEyComqWEjajqeTe/6 uIpZo58oFGojJZK1HcDmaVFyj7nxAzXloupmHiKq+xfXHbv60ZUZO6InEos/ZCjZ bpT92wyyqTeiD70glLvLRyStKzZidqeoVTkbGM0XUCA3d3RvxXdB4zfgIqDlhhCT EfhxKVkAZAzjsEn+U1/y5RWEEdD+Zaqi2xKwA+Ken9TJ4LFsjcOiQDPVZYklD7qu Xmte9GdxL4tSipu0hWxrkRjO7ap09wUoEk1d0jNSrwNbJALnUUkT4qfpYXcMaAhe okjWSANQaBZxbM0ziEoT =AnRj -----END PGP SIGNATURE-----
On 10/11/2012 10:47 AM, Tom Rini wrote: > On 10/10/12 17:05, Stephen Warren wrote: >> From: Stephen Warren <swarren@nvidia.com> > >> Implement "ls" and "fsload" commands that act like >> {fat,ext2}{ls,load}, and transparently handle either file-system. >> This scheme could easily be extended to other filesystem types; >> I only didn't do it for zfs because I don't have any filesystems >> of that type. > >> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- There are >> a couple FIXMEs in here: > >> 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or >> CONFIG_CMD_EXT2. This means that the new commands and code can >> only be enabled if the "legacy" {fat,ext2}{ls,load} are enabled. >> What we really want is CONFIG_FS_FAT and CONFIG_FS_EXT2 to enable >> the filesystem code, and then CONFIG_CMD_FAT, CONFIG_CMD_EXT2, >> CONFIG_CMD_FS to only affect the command implementations. >> However, that would require making every include/config/*.h that >> sets the current defines also set more. I suppose that's a fairly >> mechanical change though, so easy enough to implement. Does that >> seem like a reasonable approach to people? > > How about a new CONFIG_CMD_GENERIC_FS for the new ls/fsload (and > any later commands like write that get added) and once most > filesystems are converted we can think about a transition plan? Certainly. The issue is that the filesystem code itself is conditionally included based on CONFIG_CMD_FAT and CONFIG_CMD_EXT4. This would require that in order to use the new generic commands, you also had to support the old non-generic commands in order to get a linkable result. Perhaps that's fine for now, but by separating the existing CONFIG_CMD_FAT into two (CONFIG_CMD_FAT, CONFIG_FS_FAT), we could avoid that requirement. As Benoit mentioned, perhaps we could arrange that CONFIG_CMD_FAT selects CONFIG_FS_FAT somehow, although I haven't looked at U-Boot's configuration system to see if that's possible yet.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10/11/12 09:57, Stephen Warren wrote: > On 10/11/2012 10:47 AM, Tom Rini wrote: >> On 10/10/12 17:05, Stephen Warren wrote: >>> From: Stephen Warren <swarren@nvidia.com> >> >>> Implement "ls" and "fsload" commands that act like >>> {fat,ext2}{ls,load}, and transparently handle either >>> file-system. This scheme could easily be extended to other >>> filesystem types; I only didn't do it for zfs because I don't >>> have any filesystems of that type. >> >>> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- There >>> are a couple FIXMEs in here: >> >>> 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or >>> CONFIG_CMD_EXT2. This means that the new commands and code can >>> only be enabled if the "legacy" {fat,ext2}{ls,load} are >>> enabled. What we really want is CONFIG_FS_FAT and >>> CONFIG_FS_EXT2 to enable the filesystem code, and then >>> CONFIG_CMD_FAT, CONFIG_CMD_EXT2, CONFIG_CMD_FS to only affect >>> the command implementations. However, that would require >>> making every include/config/*.h that sets the current defines >>> also set more. I suppose that's a fairly mechanical change >>> though, so easy enough to implement. Does that seem like a >>> reasonable approach to people? >> >> How about a new CONFIG_CMD_GENERIC_FS for the new ls/fsload (and >> any later commands like write that get added) and once most >> filesystems are converted we can think about a transition plan? > > Certainly. > > The issue is that the filesystem code itself is conditionally > included based on CONFIG_CMD_FAT and CONFIG_CMD_EXT4. This would > require that in order to use the new generic commands, you also > had to support the old non-generic commands in order to get a > linkable result. Perhaps that's fine for now, but by separating the > existing CONFIG_CMD_FAT into two (CONFIG_CMD_FAT, CONFIG_FS_FAT), > we could avoid that requirement. As Benoit mentioned, perhaps we > could arrange that CONFIG_CMD_FAT selects CONFIG_FS_FAT somehow, > although I haven't looked at U-Boot's configuration system to see > if that's possible yet. Since we don't have Kconfig right now, it's not really easy to do that kind of thing. I think once we have more filesystems converted we can work out doing the mechanical separate of fs/ from common/ for fs code. - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQdvxhAAoJENk4IS6UOR1WOFIP/0VjExqx5FZ+Lau4wFtV8dI0 v5+jfwDUtG0L3h/lTv9NJkX5j9duXJVOcAkB4hVeWFAqd/fSdI0mlajPvR1rpS/J luYFvR/XdlIoQe65CbrKhFEzHH3pwcbYXd/Zz9wx2cvxmTz7NvbM3KZsvECPpYtO CEqNClM2e5Q0ebE/XKZ5xG8qJVWZbs48i8kkmgHQHsiplGEdf7IzNgOqiSFL5rgF DT6oRacoAzOLBwMo5/ssIiT04GgM+3Nac1UV/4GPKiNt62k4p6OfniUN4sL6N6Yl oq8aFR3ZWGZgHdHO+y+pp+4MLAlvbv8PxMXHhw/ZCCHgLR9sodvWQl/Pn3C9TFTC 0LdYgMvft5FBBqffI+XuPMomdKYLmmHtAhlh7lly9L5GQYw8iU25cfXP8ZeBUOX+ aY4nX/Wp2s+vtXawWUVv01r0HRZZp3r6FJNAXWvE9m2cB+7HiZQD+nJvkV7ZZk7E 6WRdRlI8NTLn6Sfh42d6AzigsYfAm2Xwp6B1/gGix075E30yhB1KGdxs/SpQW+0+ tdtFXij/jtxAyabTLNTivAXO44l6IBurtlAZqXeI1ur6gEJa0L1Ju0Mi6rFO/Vrg aQ/KBo+1DM4QgXxYkcYPxJ+KIJKQdsmIWEmpni+sgaLBgO43Ebh+lkLKHCm5NB0l kaIgld2V1OQQDmg/j4yW =Tegq -----END PGP SIGNATURE-----
On Thursday, October 11, 2012 7:05:37 PM, Tom Rini wrote: > On 10/11/12 09:57, Stephen Warren wrote: > > On 10/11/2012 10:47 AM, Tom Rini wrote: > >> On 10/10/12 17:05, Stephen Warren wrote: > >>> From: Stephen Warren <swarren@nvidia.com> > >> > >>> Implement "ls" and "fsload" commands that act like > >>> {fat,ext2}{ls,load}, and transparently handle either > >>> file-system. This scheme could easily be extended to other > >>> filesystem types; I only didn't do it for zfs because I don't > >>> have any filesystems of that type. > >> > >>> Signed-off-by: Stephen Warren <swarren@nvidia.com> --- There > >>> are a couple FIXMEs in here: > >> > >>> 1) In fs/fs.c, code is ifdef on CONFIG_CMD_FAT or > >>> CONFIG_CMD_EXT2. This means that the new commands and code can > >>> only be enabled if the "legacy" {fat,ext2}{ls,load} are > >>> enabled. What we really want is CONFIG_FS_FAT and > >>> CONFIG_FS_EXT2 to enable the filesystem code, and then > >>> CONFIG_CMD_FAT, CONFIG_CMD_EXT2, CONFIG_CMD_FS to only affect > >>> the command implementations. However, that would require > >>> making every include/config/*.h that sets the current defines > >>> also set more. I suppose that's a fairly mechanical change > >>> though, so easy enough to implement. Does that seem like a > >>> reasonable approach to people? > >> > >> How about a new CONFIG_CMD_GENERIC_FS for the new ls/fsload (and > >> any later commands like write that get added) and once most > >> filesystems are converted we can think about a transition plan? > > > > Certainly. > > > > The issue is that the filesystem code itself is conditionally > > included based on CONFIG_CMD_FAT and CONFIG_CMD_EXT4. This would > > require that in order to use the new generic commands, you also > > had to support the old non-generic commands in order to get a > > linkable result. Perhaps that's fine for now, but by separating the > > existing CONFIG_CMD_FAT into two (CONFIG_CMD_FAT, CONFIG_FS_FAT), > > we could avoid that requirement. As Benoit mentioned, perhaps we > > could arrange that CONFIG_CMD_FAT selects CONFIG_FS_FAT somehow, > > although I haven't looked at U-Boot's configuration system to see > > if that's possible yet. > > Since we don't have Kconfig right now, it's not really easy to do > that > kind of thing. I think once we have more filesystems converted we > can > work out doing the mechanical separate of fs/ from common/ for fs > code. This does not require Kconfig. include/config_fallbacks.h is already made for automatic config fixups. The following lines could simply be added to it: #if defined(CONFIG_CMD_FAT) && !defined(CONFIG_FS_FAT) #define CONFIG_FS_FAT #endif Best regards, Benoît
Hi On Wednesday 10 October 2012 18:05:07 Stephen Warren wrote: > ...snip... > Makefile | 3 +- > common/Makefile | 2 + > common/cmd_fs.c | 86 ++++++++++++++++++++++ > fs/Makefile | 47 ++++++++++++ > fs/fs.c | 216 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fs.h | > 49 +++++++++++++ > 6 files changed, 402 insertions(+), 1 deletions(-) > create mode 100644 common/cmd_fs.c > create mode 100644 fs/Makefile > create mode 100644 fs/fs.c > create mode 100644 include/fs.h I dont see how this is a "partition switch library", when it doesnt deal with partitions at all. Care to explain? Or maybe call it "filesystem-auto detection" or something? Best Regards Pavel Herrmann
On 10/13/2012 01:26 PM, Pavel Herrmann wrote: > Hi > > On Wednesday 10 October 2012 18:05:07 Stephen Warren wrote: >> ...snip... >> Makefile | 3 +- >> common/Makefile | 2 + >> common/cmd_fs.c | 86 ++++++++++++++++++++++ >> fs/Makefile | 47 ++++++++++++ >> fs/fs.c | 216 >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++ include/fs.h | >> 49 +++++++++++++ >> 6 files changed, 402 insertions(+), 1 deletions(-) >> create mode 100644 common/cmd_fs.c >> create mode 100644 fs/Makefile >> create mode 100644 fs/fs.c >> create mode 100644 include/fs.h > > I dont see how this is a "partition switch library", when it doesnt deal with > partitions at all. Care to explain? Or maybe call it "filesystem-auto > detection" or something? Oops. I will s/partition/filesystem/ in the patch subject.
diff --git a/Makefile b/Makefile index d7e2c2f..0b50eb7 100644 --- a/Makefile +++ b/Makefile @@ -242,7 +242,8 @@ LIBS-y += drivers/net/npe/libnpe.o endif LIBS-$(CONFIG_OF_EMBED) += dts/libdts.o LIBS-y += arch/$(ARCH)/lib/lib$(ARCH).o -LIBS-y += fs/cramfs/libcramfs.o \ +LIBS-y += fs/libfs.o \ + fs/cramfs/libcramfs.o \ fs/ext4/libext4fs.o \ fs/fat/libfat.o \ fs/fdos/libfdos.o \ diff --git a/common/Makefile b/common/Makefile index 125b2be..4f2d944 100644 --- a/common/Makefile +++ b/common/Makefile @@ -103,6 +103,8 @@ COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o ifdef CONFIG_FPGA COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o endif +# FIXME: Need to make this conditional +COBJS-y += cmd_fs.o COBJS-$(CONFIG_CMD_GPIO) += cmd_gpio.o COBJS-$(CONFIG_CMD_I2C) += cmd_i2c.o COBJS-$(CONFIG_CMD_IDE) += cmd_ide.o diff --git a/common/cmd_fs.c b/common/cmd_fs.c new file mode 100644 index 0000000..948a5e0 --- /dev/null +++ b/common/cmd_fs.c @@ -0,0 +1,86 @@ +/* + * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved. + * + * Inspired by cmd_ext_common.c, cmd_fat.c. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <common.h> +#include <command.h> +#include <fs.h> + +int do_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + unsigned long addr; + unsigned long bytes; + unsigned long pos; + int len_read; + char buf[12]; + + if (argc < 5) + return -1; + + if (fs_set_blk_dev(argv[1], argv[2])) + return 1; + + addr = simple_strtoul(argv[3], NULL, 0); + if (argc >= 6) + bytes = simple_strtoul(argv[5], NULL, 0); + else + bytes = 0; + if (argc >= 7) + pos = simple_strtoul(argv[6], NULL, 0); + else + pos = 0; + + len_read = fs_read(argv[4], addr, pos, bytes); + if (len_read <= 0) + return 1; + + sprintf(buf, "0x%x", (unsigned int)len_read); + setenv("filesize", buf); + + return 0; +} + +U_BOOT_CMD( + fsload, 7, 0, do_fsload, + "load binary file from a filesystem", + "<interface> [<dev[:part]>] <addr> <filename> [bytes [pos]]\n" + " - Load binary file 'filename' from partition 'part' on device\n" + " type 'interface' instance 'dev' to address 'addr' in memory.\n" + " 'bytes' gives the size to load in bytes.\n" + " If 'bytes' is 0 or omitted, the file is read until the end.\n" + " 'pos' gives the file byte position to start reading from.\n" + " If 'pos' is 0 or omitted, the file is read from the start." +); + +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + if (fs_set_blk_dev(argv[1], argv[2])) + return 1; + + if (fs_ls(argc == 4 ? argv[3] : "/")) + return 1; + + return 0; +} + +U_BOOT_CMD( + ls, 4, 1, do_ls, + "list files in a directory (default /)", + "<interface> [<dev[:part]>] [directory]\n" + " - List files in directory 'directory' of partition 'part' on\n" + " device type 'interface' instance 'dev'." +); diff --git a/fs/Makefile b/fs/Makefile new file mode 100644 index 0000000..d0ab3ae --- /dev/null +++ b/fs/Makefile @@ -0,0 +1,47 @@ +# +# (C) Copyright 2000-2006 +# Wolfgang Denk, DENX Software Engineering, wd@denx.de. +# Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved. +# +# See file CREDITS for list of people who contributed to this +# project. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, +# MA 02111-1307 USA +# + +include $(TOPDIR)/config.mk + +LIB = $(obj)libfs.o + +COBJS-y += fs.o + +COBJS := $(COBJS-y) +SRCS := $(COBJS:.o=.c) +OBJS := $(addprefix $(obj),$(COBJS)) + +all: $(LIB) + +$(LIB): $(obj).depend $(OBJS) + $(call cmd_link_o_target, $(OBJS)) + +######################################################################### + +# defines $(obj).depend target +include $(SRCTREE)/rules.mk + +sinclude $(obj).depend + +######################################################################### diff --git a/fs/fs.c b/fs/fs.c new file mode 100644 index 0000000..ea77ac9 --- /dev/null +++ b/fs/fs.c @@ -0,0 +1,216 @@ +/* + * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <config.h> +#include <common.h> +#include <part.h> +#include <ext4fs.h> +#include <fat.h> +#include <fs.h> + +#define FS_TYPE_INVALID 0 +#define FS_TYPE_FAT 1 +#define FS_TYPE_EXT 2 + +static block_dev_desc_t *fs_dev_desc; +static disk_partition_t fs_partition; +static int fs_type; + +/* FIXME: CONFIG_CMD_FAT, CONFIG_CMD_EXT2 should be CONFIG_FS_FAT, CONFIG_FS_EXT2 */ +#ifdef CONFIG_CMD_FAT +static int fs_probe_fat(void) +{ + return fat_set_blk_dev(fs_dev_desc, &fs_partition); +} + +static void fs_close_fat(void) +{ +} +#else +static inline int fs_probe_fat(void) +{ + return -1; +} + +static inline void fs_close_fat(void) +{ +} +#endif + +#ifdef CONFIG_CMD_EXT2 +static int fs_probe_ext(void) +{ + ext4fs_set_blk_dev(fs_dev_desc, &fs_partition); + + if (!ext4fs_mount(fs_partition.size)) { + ext4fs_close(); + return -1; + } + + return 0; +} + +static void fs_close_ext(void) +{ + ext4fs_close(); +} +#else +static inline int fs_probe_ext(void) +{ + return -1; +} + +static inline void fs_close_ext(void) +{ +} +#endif + +int fs_set_blk_dev(const char *ifname, const char *dev_part_str) +{ + int part; + + switch (fs_type) { + case FS_TYPE_FAT: + fs_close_fat(); + break; + case FS_TYPE_EXT: + fs_close_ext(); + break; + default: + break; + } + + fs_type = FS_TYPE_INVALID; + + part = get_device_and_partition(ifname, dev_part_str, &fs_dev_desc, + &fs_partition, 1); + if (part < 0) + return -1; + + if (!fs_probe_fat()) { + fs_type = FS_TYPE_FAT; + return 0; + } + + if (!fs_probe_ext()) { + fs_type = FS_TYPE_EXT; + return 0; + } + + printf("** Unrecognized filesystem type **\n"); + return -1; +} + +static inline int fs_ls_unsupported(const char *dirname) +{ + printf("** Unrecognized filesystem type **\n"); + return -1; +} + +#ifdef CONFIG_CMD_FAT +#define fs_ls_fat file_fat_ls +#else +#define fs_ls_fat fs_ls_unsupported +#endif + +#ifdef CONFIG_CMD_EXT2 +#define fs_ls_ext ext4fs_ls +#else +#define fs_ls_ext fs_ls_unsupported +#endif + +int fs_ls(const char *dirname) +{ + switch (fs_type) { + case FS_TYPE_FAT: + return fs_ls_fat(dirname); + case FS_TYPE_EXT: + return fs_ls_ext(dirname); + default: + return fs_ls_unsupported(dirname); + } +} + +static inline int fs_read_unsupported(const char *filename, ulong addr, int offset, int len) +{ + printf("** Unrecognized filesystem type **\n"); + return -1; +} + +#ifdef CONFIG_CMD_FAT +static int fs_read_fat(const char *filename, ulong addr, int offset, int len) +{ + int len_read; + + len_read = file_fat_read_at(filename, offset, + (unsigned char *)addr, len); + if (len_read == -1) { + printf("** Unable to read file %s **\n", filename); + return -1; + } + + return len_read; +} +#else +#define fs_read_fat fs_read_unsupported +#endif + +#ifdef CONFIG_CMD_EXT2 +static int fs_read_ext(const char *filename, ulong addr, int offset, int len) +{ + int file_len; + int len_read; + + if (offset != 0) { + printf("** Cannot support non-zero offset **\n"); + return -1; + } + + file_len = ext4fs_open(filename); + if (file_len < 0) { + printf("** File not found %s **\n", filename); + ext4fs_close(); + return -1; + } + + if (len == 0) + len = file_len; + + len_read = ext4fs_read((char *)addr, len); + ext4fs_close(); + + if (len_read != len) { + printf("** Unable to read file %s **\n", filename); + return -1; + } + + return len_read; +} +#else +#define fs_read_ext fs_read_unsupported +#endif + +int fs_read(const char *filename, ulong addr, int offset, int len) +{ + switch (fs_type) { + case FS_TYPE_FAT: + return fs_read_fat(filename, addr, offset, len); + case FS_TYPE_EXT: + return fs_read_ext(filename, addr, offset, len); + default: + return fs_read_unsupported(filename, addr, offset, len); + } +} diff --git a/include/fs.h b/include/fs.h new file mode 100644 index 0000000..a0fda28 --- /dev/null +++ b/include/fs.h @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef _FS_H +#define _FS_H + +/* + * Tell the fs layer which block device an partition to use for future + * commands. This also internally identifies the filesystem that is present + * within the partition. + * + * Returns 0 on success. + * Returns non-zero if there is an error accessing the disk or partition, or + * no known filesystem type could be recognized on it. + */ +int fs_set_blk_dev(const char *ifname, const char *dev_part_str); + +/* + * Print the list of files on the partition previously set by fs_set_blk_dev(), + * in directory "dirname". + * + * Returns 0 on success. Returns non-zero on error. + */ +int fs_ls(const char *dirname); + +/* + * Read file "filename" from the partition previously set by fs_set_blk_dev(), + * to address "addr", starting at byte offset "offset", and reading "len" + * bytes. "offset" may be 0 to read from the start of the file. "len" may be + * 0 to read the entire file. Note that not all filesystem types support + * either/both offset!=0 or len!=0. + * + * Returns number of bytes read on success. Returns <= 0 on error. + */ +int fs_read(const char *filename, ulong addr, int offset, int len); + +#endif /* _FS_H */