public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Przemyslaw Marczak <p.marczak@samsung.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Date: Fri, 12 Dec 2014 16:30:05 +0100	[thread overview]
Message-ID: <548B09FD.2080005@samsung.com> (raw)
In-Reply-To: <CAPnjgZ0nearfBEZoKREMjKo_m9gE+SPD=cwBX+ea-HSPyG6aQg@mail.gmail.com>

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.

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

  reply	other threads:[~2014-12-12 15:30 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-11 12:01 [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue Przemyslaw Marczak
2014-12-12  0:32 ` Simon Glass
2014-12-12 15:30   ` Przemyslaw Marczak [this message]
2014-12-12 15:52     ` [U-Boot] [PATCH] fat: scripts for prepare and test read fat files Przemyslaw Marczak
2014-12-12 15:54       ` Przemyslaw Marczak
2014-12-16 20:41         ` Simon Glass
2014-12-17  8:53           ` Przemyslaw Marczak
2014-12-16 22:26     ` [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue Simon Glass
2014-12-17  8:53       ` Przemyslaw Marczak
2014-12-17  9:03       ` Przemyslaw Marczak
2014-12-18  3:39         ` Simon Glass
2014-12-18 10:26           ` Przemyslaw Marczak
2014-12-18 13:14             ` Simon Glass
2014-12-18 13:31               ` Przemyslaw Marczak
2014-12-18 13:36                 ` Simon Glass
2014-12-18 13:41                   ` Przemyslaw Marczak
2014-12-18 13:47 ` Simon Glass
2014-12-18 14:06   ` Przemyslaw Marczak
2014-12-18 14:32   ` Przemyslaw Marczak
2014-12-18 14:34     ` Simon Glass
2014-12-18 14:40       ` Przemyslaw Marczak
2014-12-18 14:56         ` Simon Glass
2014-12-18 15:12           ` Przemyslaw Marczak
2014-12-18 15:21 ` [U-Boot] [PATCH v2] " Przemyslaw Marczak
2014-12-18 16:14   ` [U-Boot] [PATCH v3] " Przemyslaw Marczak
2015-01-07 15:12     ` [U-Boot] [U-Boot,v3] " Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=548B09FD.2080005@samsung.com \
    --to=p.marczak@samsung.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox