[U-Boot] [PATCH v3] Introduced btrfs file-system with btrload command
Simon Glass
sjg at chromium.org
Tue Feb 26 17:19:00 CET 2013
Hi Adnan,
On Tue, Feb 26, 2013 at 4:54 AM, Adnan Ali <adnan.ali at codethink.co.uk> wrote:
> On 25/02/13 23:02, Simon Glass wrote:
>>
>> Hi,
>>
>> On Mon, Feb 25, 2013 at 7:04 AM, Tom Rini <trini at 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.ali at 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 <herbert at 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.
>
> 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?
I suggest you pull the two functions out and try to see what is
different. You might need to define DYNAMIC_CRC_TABLE if the
polynomial has changed. 'sandbox' might help here (make
sandbox_config) since you can build and run on your linux box.
If you have changed the polynomial and it still doesn't work then
something is wrong...
I think adding the code to lib/crc32 with a new CONFIG is OK - but
probably better if you put it in a new file like crc32_c.c or similar.
Hopefully there is some common element to the algorithms?
Regards,
Simon
>>
>> Regards,
>> Simon
>>
>>
>>> --
>>> Tom
>
>
More information about the U-Boot
mailing list