Hi, On Mon, Feb 25, 2013 at 7:04 AM, Tom Rini <tr...@ti.com> wrote: > On Mon, Feb 25, 2013 at 12:24:37PM +0000, Adnan Ali 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. >> >> Signed-off-by: Adnan Ali <adnan....@codethink.co.uk> > > A few things: > - In general in fs/btrfs/btrfs.c I see some coding style problems (lack > of spacing, non-printf's longer than 80-wide). Do these come from > syslinux and thus will make any re-syncs easier? > - It looks like you added support for CONFIG_CMD_FS_GENERIC, if so did > you test that? > - Can you please enable this support code on at least one platform, > preferably the one you've tested and developed with? > > [snip] >> diff --git a/fs/fs.c b/fs/fs.c > [snip] >> + //file handle is valid get the size of the file > > /* Style comments only */ > >> + len=filedata.size; > > And spacing between variable and assignment. > >> @@ -178,7 +248,6 @@ int fs_set_blk_dev(const char *ifname, const char >> *dev_part_str, int fstype) >> 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; > [snip] >> @@ -208,7 +280,6 @@ static void fs_close(void) >> int fs_ls(const char *dirname) >> { >> int ret; >> - >> switch (fs_type) { >> case FS_TYPE_FAT: >> ret = fs_ls_fat(dirname); > > Unrelated, please drop. > >> @@ -237,11 +311,13 @@ int fs_read(const char *filename, ulong addr, int >> offset, int len) >> case FS_TYPE_EXT: >> ret = fs_read_ext(filename, addr, offset, len); >> break; >> + case FS_TYPE_BTR: >> + ret = fs_read_btr(filename, addr, offset, len); >> + break; >> default: >> ret = fs_read_unsupported(filename, addr, offset, len); >> break; >> } >> - >> fs_close(); > > And unrelated whitespace changes here as well. > >> diff --git a/include/btrfs.h b/include/btrfs.h > [snip] >> +/* >> + * Extent structure: contains the mapping of some chunk of a file >> + * that is contiguous on disk. >> + */ >> +struct extent { >> + //sector_t pstart; /* Physical start sector */ >> + __le64 pstart; > > Fix please. > >> +/* >> + * Our definition of "not whitespace" >> + */ >> +static inline char not_whitespace(char c) >> +{ >> + return (unsigned char)c > ' '; >> +} > > Can't you just use isspace from <linux/ctypes.h> ? > >> diff --git a/include/crc32c.h b/include/crc32c.h >> new file mode 100644 >> index 0000000..d04916e >> --- /dev/null >> +++ b/include/crc32c.h >> @@ -0,0 +1,48 @@ >> +/* >> + * 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 >> + */ >> + >> +/* >> + * Steps through buffer one byte at at time, calculates reflected >> + * crc using table. >> + */ >> + >> +static 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; >> +} >> + >> +static inline void crc32c_init(u32 *crc32c_table, u32 pol) >> +{ >> + 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++) { >> + v = (v >> 1) ^ ((v & 1) ? poly : 0); >> + } >> + crc32c_table[i] = v; >> + } >> +} > > Simon, since you've just been over all the crc32 recently, do we have > these functions somewhere already that can be easily tapped in to? > Thanks!
Should be - see lib/crc32.c. There is already at least one other copy (ubifs I think) so we should try to avoid adding more. Maybe the polynomial is different here? But even so it should go with the existing code I think. Regards, Simon > > -- > Tom _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot