Re: [U-Boot] [RFC PATCH 2/2] fs: add partition switch libary, implement ls and fsload commands

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