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

[Przemyslaw Marczak <p.marczak@samsung.com>]

Hello,

On 12/16/2014 11:26 PM, Simon Glass wrote:
> Hi Przemyslaw,
>
> On 12 December 2014 at 08:30, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>> Hello,
>>
>>
>> On 12/12/2014 01:32 AM, Simon Glass wrote:
>>>
>>> Hi Przemyslaw,
>>>
>>> On 11 December 2014 at 05:01, Przemyslaw Marczak <p.marczak@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.
>>>
>>>
>>>
>>> This is very interesting\! Is this the same failure that I saw on this
>>> thread?
>>>
>>>
>>> http://u-boot.10912.n7.nabble.com/PATCH-U-Boot-ARM-rpi-b-detect-board-revision-td196720.html
>>>
>>> (search for fatload)
>>>
>>> "I tried this out. It worked OK for me except that it can't find the
>>> device tree file bcm2835-rpi-b-rev2.dtb.
>>>
>>> Oddly I can fatload it from /bcm2835-rpi-b-rev2.dtb but when I try
>>> from /syslinux/..//bcm2835-rpi-b-rev2.dtb it fails and cannot find the
>>> file. Reducing the filename length to 8 chars works. I wonder what
>>> year of my life FAT will stop plaguing me? "
>>>
>>>
>>> Also can you write a test for this in test/fs/fs-test.sh?
>>>
>>> Regards,
>>> Simon
>>>
>>> [snip]
>>>
>>
>> Probably this is an another case which is caused by the sector buffer bug.
>> Does this patch fixed your issue?
>>
>> I have some simple test for manual use with the ums tool.
>> It just copy the test files to the tested fat16 partition mounted using the
>> UMS, next it computes CRC32 of those files on the host and next using U-Boot
>> fatload/crc32 commands - it tests the read feature. But it's not full
>> automated - I didn't work on getting the log from U-Boot console.
>>
>> So I could check if the file checksums are proper and if all files were
>> found on the partiion, by the U-Boot read command. It's not useful for the
>> "test/fs/fs-test.sh" because this is not designed for the sandbox.
>> My test writes some commands directly to U-Boot console, like this: echo
>> "some cmd" > /dev/ttyS0.
>>
>> Unfortunately the sandbox config seems to be broken.
>>
>> The bug was not so obvious, any read/write on fat partition can change fat
>> directory entries or add the new ones and then all data can be read right.
>>
>> I will send the scripts for such simple test.
>
> I'm not sure if it fixes my problem but it seems likely. I will see if
> I can make time to test it.
>
> If you want to write input data to U-Boot sandbox we can do that
> fairly easily. You can import cros_subprocess and use the function
> there to generate output from your test and inspect the input from
> U-Boot's command line. Let me know if you'd like an example.
>
> Regards,
> Simon
>

It sounds good. I can do that, as I wrote in my previous message, now 
I'm focused on the pmic, and this will be a side task for a break time.
I will look into the present tests and I think, that I will find an 
example there.

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