[U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue

[Przemyslaw Marczak]

Hello,

On 12/18/2014 03:56 PM, Simon Glass wrote:
> Hi,
>
> On 18 December 2014 at 07:40, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
>> Hello,
>>
>>
>> On 12/18/2014 03:34 PM, Simon Glass wrote:
>>>
>>> Hi Przemyslaw,
>>>
>>> On 18 December 2014 at 07:32, Przemyslaw Marczak <p.marczak at samsung.com>
>>> wrote:
>>>>
>>>> Hello,
>>>>
>>>> On 12/18/2014 02:47 PM, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 11 December 2014 at 05:01, Przemyslaw Marczak <p.marczak at samsung.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> The present fat implementation ignores FAT16 long name
>>>>>> directory entries which aren't placed in a single sector.
>>>>>>
>>>>>> This was becouse of the buffer was always filled by the
>>>>>> two sectors, and the loop was made also for two sectors.
>>>>>>
>>>>>> If some file long name entries are stored in two sectors,
>>>>>> the we have two cases:
>>>>>>
>>>>>> Case 1:
>>>>>> Both of sectors are in the buffer - all required data
>>>>>> for long file name is in the buffer.
>>>>>> - Read OK!
>>>>>>
>>>>>> Case 2:
>>>>>> The current directory entry is placed at the end of the
>>>>>> second buffered sector. And the next entries are placed
>>>>>> in a sector which is not buffered yet. Then two next
>>>>>> sectors are buffered and the mentioned entry is ignored.
>>>>>> - Read fail!
>>>>>>
>>>>>> This commit fixes this issue by:
>>>>>> - read two sectors after loop on each single is done
>>>>>> - keep the last used sector as a first in the buffer
>>>>>>      before the read of two next
>>>>>>
>>>>>> The commit doesn't affects the fat32 imlementation,
>>>>>> which works good as previous.
>>>>>>
>>>>>> Signed-off-by: Przemyslaw Marczak <p.marczak at samsung.com>
>>>>>> Cc: Mikhail Zolotaryov <lebon at lebon.org.ua>
>>>>>> Cc: Tom Rini <trini at ti.com>
>>>>>> Cc: Stephen Warren <swarren at nvidia.com>
>>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>>> Cc: Suriyan Ramasami <suriyan.r at gmail.com>
>>>>>> Cc: Lukasz Majewski <l.majewski at samsung.com>
>>>>>> Cc: Wolfgang Denk <wd at denx.de>
>>>>>> ---
>>>>>>     fs/fat/fat.c | 33 ++++++++++++++++++++++++---------
>>>>>>     1 file changed, 24 insertions(+), 9 deletions(-)
>>>>>
>>>>>
>>>>>
>>>>> Tested-by: Simon Glass <sjg at chomium.org>
>>>>>
>>>>>>
>>>>>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>>>>>> index 04a51db..afbf12d 100644
>>>>>> --- a/fs/fat/fat.c
>>>>>> +++ b/fs/fat/fat.c
>>>>>> @@ -823,8 +823,11 @@ int do_fat_read_at(const char *filename, loff_t
>>>>>> pos,
>>>>>> void *buffer,
>>>>>>            int ret = -1;
>>>>>>            int firsttime;
>>>>>>            __u32 root_cluster = 0;
>>>>>> +       __u32 read_blk;
>>>>>>            int rootdir_size = 0;
>>>>>> -       int j;
>>>>>> +       int j, k;
>>>>>
>>>>>
>>>>>
>>>>> What is k? Can we use a proper variable name? Also for j. That might
>>>>> save needing a comment for them.
>>>>>
>>>>>> +       int do_read;
>>>>>> +       __u8 *dir_ptr;
>>>>>
>>>>>
>>>>>
>>>>> Why does it use __u8 instead of u8 or uint8_t for example?
>>>>
>>>>
>>>>
>>>> __u8 is used in a whole fat code, and also as a directory entry buffer,
>>>> so
>>>> why not to keep the whole code style?
>>>
>>>
>>> OK, sounds good.
>>>
>>> Do you have any ideas on the bug I reported?
>>>
>>
>> No, but I think that this is not any fat issue.
>
> Can you explain what you mean here? What other software could be causing this?
>
> Regards,
> Simon
>

The fat code is quite unreadable, but it is simple.
Passing the "/syslinux/..//bcm2835-rpi-b-rev2.dtb" as the fatload 
argument is just treat as a "/syslinux" directory which doesn't exists.
So the file can't be read, right?

Best regards,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com