From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adnan Ali Date: Mon, 25 Feb 2013 16:08:21 +0000 Subject: [U-Boot] [PATCH v3] Introduced btrfs file-system with btrload command In-Reply-To: <20130225150438.GA11611@bill-the-cat> References: <1361795077-3674-1-git-send-email-adnan.ali@codethink.co.uk> <1361795077-3674-2-git-send-email-adnan.ali@codethink.co.uk> <20130225150438.GA11611@bill-the-cat> Message-ID: <512B8C75.5040101@codethink.co.uk> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 25/02/13 15:04, Tom Rini 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 > 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? Most of the ported code is unmodified so its coding style should be same as syslinux. > - It looks like you added support for CONFIG_CMD_FS_GENERIC, if so did > you test that? This command wasn't enabled in my configs. I haven't added any command i.e btrls for this as this feature is not supported yet. > - Can you please enable this support code on at least one platform, > preferably the one you've tested and developed with? Even if do enable support for this, it will also debug 'Unsupported filesystem type.' For the rest of changes you proposed i will change them and send as v4 patch. > > [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 ? > >> 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 >> + * >> + * 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! > Thanks Adnan