On 12/03/13 22:37, Simon Glass wrote:
Hi Adnan,

On Tue, Mar 12, 2013 at 7:53 AM, Adnan Ali <adnan....@codethink.co.uk> wrote:
Hi


On 12/03/13 14:15, Simon Glass wrote:
Hi Adnan,

On Mon, Mar 11, 2013 at 6:18 AM, Adnan Ali <adnan....@codethink.co.uk>
wrote:
Introduces btrfs file-system to read file from
volume/sub-volumes with btrload command. This
implementation has read-only support.
This btrfs implementation is based on syslinux btrfs
code, commit 269ebc845ebc8b46ef4b0be7fa0005c7fdb95b8d.

v5:     merged with master.
v4:     btrls command added.
v3:     patch re-formated.
v2:     patch error removed.

Signed-off-by: Adnan Ali <adnan....@codethink.co.uk>
---
   Makefile                   |    1 +
   common/Makefile            |    1 +
   common/cmd_btr.c           |   65 +++
   fs/btrfs/Makefile          |   51 ++
   fs/btrfs/btrfs.c           | 1348
++++++++++++++++++++++++++++++++++++++++++++
   fs/btrfs/crc32_c.c         |   54 ++
   fs/fs.c                    |   10 +
   include/btrfs.h            |  416 ++++++++++++++
   include/config_fallbacks.h |    4 +
   include/fs.h               |    1 +
   10 files changed, 1951 insertions(+)
   create mode 100644 common/cmd_btr.c
   create mode 100644 fs/btrfs/Makefile
   create mode 100644 fs/btrfs/btrfs.c
   create mode 100644 fs/btrfs/crc32_c.c
   create mode 100644 include/btrfs.h

I have ignored the code before this point in btrfs.c since it is
imported and you want to keep the code style as is, but if you don't
mind I will make a few comments after that point>
     Here are my comments

+struct btrfs_info fs;
+
+/*
+ *  mount btrfs file-system
+ */
+int btrfs_probe(block_dev_desc_t *rbdd, disk_partition_t *info)
+{
+        btrfs_block_dev_desc = rbdd;
+        part_info = info;
+        btr_part_offset = info->start;
+       if (btrfs_fs_init(&fs) < 0 ) {
+          debug("btrfs probe failed\n");
+          return -1;
+       }
You should use tabs for intend (some of this bit uses spaces, some uses
tabs).
    Some how i missed will check it again. Ack

+
+       return 0;
+}
+
+/*
+ *  Read file data
+ */
+int btrfs_read_file(const char *filename, void *buf, int offset, int
len)
+{
+       int file_len=0;
+        int len_read;
+        struct com32_filedata filedata;
+        int handle;
+        if (offset != 0) {
+                debug("** Cannot support non-zero offset **\n");
+                return -1;
+        }
+
+        handle=btrfs_open_file(filename, &filedata);
+        if (handle < 0) {
+                debug("** File not found %s Invalid handle**\n",
filename);
+                return -1;
+        }
+
+        /*file handle is valid get the size of the file*/
+        len = filedata.size;
+        if (len == 0)
+                len = file_len;
+
+        len_read = getfssec(&filedata, (char *)buf);
+        if (len_read != len) {
+                debug("** Unable to read file %s **\n", filename);
+                return -1;
+        }
+
+        return len_read;
+
+}
+
+/*
+ * Show directory entries
+ */
+char btrfs_ls(char *dirname)
+{
+  struct btrfs_dirent *de;
+  struct _DIR_ *dir;
+
+  if(*dirname == '/' && *(dirname+1) == 0)
+     *dirname = '.';
+
+  dir = opendir(dirname);
+  if (dir == NULL)
+        return -1;
+
+  while ((de = readdir(dir)) != NULL)
+        ;
This doesn't seem to print anything.
      readdir->btrfs_read, prints contents on media.

+
+  return 0;
+}
+
+/*
+ *  umount btrfs file-system
+ */
+void btrfs_close(void)
+{
+
Remove blank line
    Ack

+}
diff --git a/fs/btrfs/crc32_c.c b/fs/btrfs/crc32_c.c
new file mode 100644
index 0000000..78d0447
--- /dev/null
+++ b/fs/btrfs/crc32_c.c
@@ -0,0 +1,54 @@
+/*
+ * Copied from Linux kernel crypto/crc32c.c
+ * Copyright (c) 2004 Cisco Systems, Inc.
+ * Copyright (c) 2008 Herbert Xu <herb...@gondor.apana.org.au>
+ *
+ * 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 is the CRC-32C table
+ * Generated with:
+ * width = 32 bits
+ * poly = 0x1EDC6F41
+ * reflect input bytes = true
+ * reflect output bytes = true
+ */
Old comment?
    this crc was part of port

+
+/*
+ * Steps through buffer one byte at at time, calculates reflected
+ * crc using table.
+ */
+#include <linux/stat.h>
+#include <command.h>
+#include <asm/byteorder.h>
+#include <linux/compiler.h>
+#include <common.h>
+#include <config.h>
+
+inline u32 crc32c_cal(u32 crc, const char *data, size_t length, u32
*crc32c_table)
+{
+       while (length--)
+               crc = crc32c_table[(u8)(crc ^ *data++)] ^ (crc >> 8);
+
+       return crc;
+}
+
+inline void crc32c_init(u32 *crc32c_table, u32 pol)
The 'inline' isn't needed.
     Ack

+{
+       int i, j;
+       u32 v;
+       const u32 poly = pol; /* Bit-reflected CRC32C polynomial */
+
+       for (i = 0; i < 256; i++) {
+               v = i;
+               for (j = 0; j < 8; j++) {
Can remove this inner {} since you only have one line of code.
      Ack

+                       v = (v >> 1) ^ ((v & 1) ? poly : 0);
+               }
+               crc32c_table[i] = v;
+       }
+}
I suggest you move your crc_32.c file to lib/ since it might be more
generally useful. You should probably include the function prototypes
in include/crc.h.
   The crc32_c.c was part of port as header file and you previously said
to move to btrfs folder and make it c file.

Also you seem to recalculate the crc table on every operation. Is is
possible to only do this ones on startup?
   Yes every time prob is called its calculated. It can be calculated at
boot time. Can you point me to file and function that it can be called.
before that can we make decision on crc32_c.c.
Perhaps you could call it once in your btrfs_probe() function, and set
a flag once you have done it once?


Plus once i make all these changes will it be accepted than ;).
I hope so. I think it fits in nicely. Did you have any comments as to
why the btrload command doesn't have [bytes [pos] like fatload, for
example?
  I will send patch v6 to cover all things you pointed out that isn't
  related to port.
I think [bytes [pos]] can be done in btrfs. I prefered adding subvolume which was unique to btrfs. I can add that later send patch for this, once this goes
 in main stream.  If that is acceptable to you.

Regards,
Simon


Regards,
Simon
Regards
Adnan

Thanks
Adnan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to