From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adnan Ali Date: Tue, 26 Feb 2013 12:54:46 +0000 Subject: [U-Boot] [PATCH v3] Introduced btrfs file-system with btrload command In-Reply-To: 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: <512CB096.1000303@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 23:02, Simon Glass wrote: > Hi, > > On Mon, Feb 25, 2013 at 7:04 AM, 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? >> - 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 ? >> >>> 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! > 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. I have tried using crc code lib/crc32.c but it always failed even though i did change the polynomial but still result is search for file on btrfs fails due to bad crc calculation. I have also enable dynamic table creation but still result is same. so should add my my crc code under ifdef in lib/crc32.c what do you suggest? > Regards, > Simon > > >> -- >> Tom