Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello Simon, On 12/18/2014 04:39 AM, Simon Glass wrote: Hi Przemyslaw, On 17 December 2014 at 02:03, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello, On 12/16/2014 11:26 PM, Simon Glass wrote: Hi Przemyslaw, On 12 December 2014 at 08:30, Przemyslaw Marczak p.marc...@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.marc...@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 Before, I wrote, that sandbox seems to be broken, sorry for this - it was just my dirty repo - sandbox compiles and works well. Somewhat bewildering, but it does not in fact fix my problem. Here is a compressed version of the fat filesystem if you want to take a look: https://drive.google.com/file/d/0B7WYZbZ9zd-3NGRMNkFQQTdtV2M/view?usp=sharing Regards, Simon I had put this image on my Trats2 device on partition mmc 0:6 using ums and dd linux command. Next I put latest mainline u-boot, commit: e3bf81b1e841ecabe7c8b3d48621256db8b8623e : Merge branch 'master' of git://git.denx.de/u-boot-mpc85xx So this is the version with the fat bug. But I can see and load the file: bcm2835-rpi-b-rev2.dtb. Trats2 # fatls mmc 0:6 17840 bootcode.bin 120 cmdline.txt 1331 config.txt 6115 fixup.dat 2324 fixup_cd.dat 9166 fixup_x.dat 3232856 kernel.img 2615064 start.elf 533080 start_cd.elf 3572200 start_x.elf 137 issue.txt 18974 license.oracle 295524 u-boot.bin 1331 config.txt~ extlinux/ 3368648 zimage 3963 bcm2835-rpi-b.dtb 3963 bcm2835.dtb 3963 bcm2835-rpi-b-rev2.dtb 18 file(s), 1 dir(s) Trats2 # fatload mmc 0:6 0x4000 bcm2835-rpi-b-rev2.dtb reading bcm2835-rpi-b-rev2.dtb 3963 bytes read in 5 ms (773.4 KiB/s) Trats2 # crc32 0x4000 0xf7b CRC32 for 4000 ... 4f7a == c36ee8db Trats2 # The only missing file is boot.scr, it's ignored by ls command but can be loaded... Trats2 # fatload mmc 0:6 0x4000 boot.scr
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hi Przemyslaw, On 18 December 2014 at 03:26, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello Simon, On 12/18/2014 04:39 AM, Simon Glass wrote: Hi Przemyslaw, On 17 December 2014 at 02:03, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello, On 12/16/2014 11:26 PM, Simon Glass wrote: Hi Przemyslaw, On 12 December 2014 at 08:30, Przemyslaw Marczak p.marc...@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.marc...@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 Before, I wrote, that sandbox seems to be broken, sorry for this - it was just my dirty repo - sandbox compiles and works well. Somewhat bewildering, but it does not in fact fix my problem. Here is a compressed version of the fat filesystem if you want to take a look: https://drive.google.com/file/d/0B7WYZbZ9zd-3NGRMNkFQQTdtV2M/view?usp=sharing Regards, Simon I had put this image on my Trats2 device on partition mmc 0:6 using ums and dd linux command. Next I put latest mainline u-boot, commit: e3bf81b1e841ecabe7c8b3d48621256db8b8623e : Merge branch 'master' of git://git.denx.de/u-boot-mpc85xx So this is the version with the fat bug. But I can see and load the file: bcm2835-rpi-b-rev2.dtb. Trats2 # fatls mmc 0:6 17840 bootcode.bin 120 cmdline.txt 1331 config.txt 6115 fixup.dat 2324 fixup_cd.dat 9166 fixup_x.dat 3232856 kernel.img 2615064 start.elf 533080 start_cd.elf 3572200 start_x.elf 137 issue.txt 18974 license.oracle 295524 u-boot.bin 1331 config.txt~ extlinux/ 3368648 zimage 3963 bcm2835-rpi-b.dtb 3963 bcm2835.dtb 3963 bcm2835-rpi-b-rev2.dtb 18 file(s), 1 dir(s) Trats2 # fatload mmc 0:6 0x4000 bcm2835-rpi-b-rev2.dtb reading bcm2835-rpi-b-rev2.dtb 3963 bytes read in 5 ms (773.4 KiB/s) Trats2 #
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello, On 12/18/2014 02:14 PM, Simon Glass wrote: Hi Przemyslaw, On 18 December 2014 at 03:26, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello Simon, On 12/18/2014 04:39 AM, Simon Glass wrote: Hi Przemyslaw, On 17 December 2014 at 02:03, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello, On 12/16/2014 11:26 PM, Simon Glass wrote: Hi Przemyslaw, On 12 December 2014 at 08:30, Przemyslaw Marczak p.marc...@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.marc...@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 Before, I wrote, that sandbox seems to be broken, sorry for this - it was just my dirty repo - sandbox compiles and works well. Somewhat bewildering, but it does not in fact fix my problem. Here is a compressed version of the fat filesystem if you want to take a look: https://drive.google.com/file/d/0B7WYZbZ9zd-3NGRMNkFQQTdtV2M/view?usp=sharing Regards, Simon I had put this image on my Trats2 device on partition mmc 0:6 using ums and dd linux command. Next I put latest mainline u-boot, commit: e3bf81b1e841ecabe7c8b3d48621256db8b8623e : Merge branch 'master' of git://git.denx.de/u-boot-mpc85xx So this is the version with the fat bug. But I can see and load the file: bcm2835-rpi-b-rev2.dtb. Trats2 # fatls mmc 0:6 17840 bootcode.bin 120 cmdline.txt 1331 config.txt 6115 fixup.dat 2324 fixup_cd.dat 9166 fixup_x.dat 3232856 kernel.img 2615064 start.elf 533080 start_cd.elf 3572200 start_x.elf 137 issue.txt 18974 license.oracle 295524 u-boot.bin 1331 config.txt~ extlinux/ 3368648 zimage 3963 bcm2835-rpi-b.dtb 3963 bcm2835.dtb 3963 bcm2835-rpi-b-rev2.dtb 18 file(s), 1 dir(s) Trats2 # fatload mmc 0:6 0x4000 bcm2835-rpi-b-rev2.dtb reading bcm2835-rpi-b-rev2.dtb 3963 bytes read in 5 ms (773.4 KiB/s) Trats2 # crc32 0x4000 0xf7b CRC32 for
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hi Przemyslaw, On 18 December 2014 at 06:31, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello, On 12/18/2014 02:14 PM, Simon Glass wrote: Hi Przemyslaw, On 18 December 2014 at 03:26, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello Simon, On 12/18/2014 04:39 AM, Simon Glass wrote: Hi Przemyslaw, On 17 December 2014 at 02:03, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello, On 12/16/2014 11:26 PM, Simon Glass wrote: Hi Przemyslaw, On 12 December 2014 at 08:30, Przemyslaw Marczak p.marc...@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.marc...@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 Before, I wrote, that sandbox seems to be broken, sorry for this - it was just my dirty repo - sandbox compiles and works well. Somewhat bewildering, but it does not in fact fix my problem. Here is a compressed version of the fat filesystem if you want to take a look: https://drive.google.com/file/d/0B7WYZbZ9zd-3NGRMNkFQQTdtV2M/view?usp=sharing Regards, Simon I had put this image on my Trats2 device on partition mmc 0:6 using ums and dd linux command. Next I put latest mainline u-boot, commit: e3bf81b1e841ecabe7c8b3d48621256db8b8623e : Merge branch 'master' of git://git.denx.de/u-boot-mpc85xx So this is the version with the fat bug. But I can see and load the file: bcm2835-rpi-b-rev2.dtb. Trats2 # fatls mmc 0:6 17840 bootcode.bin 120 cmdline.txt 1331 config.txt 6115 fixup.dat 2324 fixup_cd.dat 9166 fixup_x.dat 3232856 kernel.img 2615064 start.elf 533080 start_cd.elf 3572200 start_x.elf 137 issue.txt 18974 license.oracle 295524 u-boot.bin 1331 config.txt~ extlinux/ 3368648 zimage 3963 bcm2835-rpi-b.dtb 3963 bcm2835.dtb 3963
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello, On 12/18/2014 02:36 PM, Simon Glass wrote: Hi Przemyslaw, On 18 December 2014 at 06:31, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello, On 12/18/2014 02:14 PM, Simon Glass wrote: Hi Przemyslaw, On 18 December 2014 at 03:26, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello Simon, On 12/18/2014 04:39 AM, Simon Glass wrote: Hi Przemyslaw, On 17 December 2014 at 02:03, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello, On 12/16/2014 11:26 PM, Simon Glass wrote: Hi Przemyslaw, On 12 December 2014 at 08:30, Przemyslaw Marczak p.marc...@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.marc...@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 Before, I wrote, that sandbox seems to be broken, sorry for this - it was just my dirty repo - sandbox compiles and works well. Somewhat bewildering, but it does not in fact fix my problem. Here is a compressed version of the fat filesystem if you want to take a look: https://drive.google.com/file/d/0B7WYZbZ9zd-3NGRMNkFQQTdtV2M/view?usp=sharing Regards, Simon I had put this image on my Trats2 device on partition mmc 0:6 using ums and dd linux command. Next I put latest mainline u-boot, commit: e3bf81b1e841ecabe7c8b3d48621256db8b8623e : Merge branch 'master' of git://git.denx.de/u-boot-mpc85xx So this is the version with the fat bug. But I can see and load the file: bcm2835-rpi-b-rev2.dtb. Trats2 # fatls mmc 0:6 17840 bootcode.bin 120 cmdline.txt 1331 config.txt 6115 fixup.dat 2324 fixup_cd.dat 9166 fixup_x.dat 3232856 kernel.img 2615064 start.elf 533080 start_cd.elf 3572200 start_x.elf 137 issue.txt 18974 license.oracle 295524 u-boot.bin 1331 config.txt~ extlinux/ 3368648 zimage 3963 bcm2835-rpi-b.dtb 3963 bcm2835.dtb 3963 bcm2835-rpi-b-rev2.dtb 18 file(s), 1
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hi,
On 11 December 2014 at 05:01, Przemyslaw Marczak p.marc...@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.marc...@samsung.com
Cc: Mikhail Zolotaryov le...@lebon.org.ua
Cc: Tom Rini tr...@ti.com
Cc: Stephen Warren swar...@nvidia.com
Cc: Simon Glass s...@chromium.org
Cc: Suriyan Ramasami suriya...@gmail.com
Cc: Lukasz Majewski l.majew...@samsung.com
Cc: Wolfgang Denk w...@denx.de
---
fs/fat/fat.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
Tested-by: Simon Glass s...@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?
if (read_bootsectandvi(bs, volinfo, mydata-fatsize)) {
debug(Error: reading boot sector\n);
@@ -910,23 +913,35 @@ int do_fat_read_at(const char *filename, loff_t pos,
void *buffer,
}
j = 0;
+ k = 0;
while (1) {
int i;
- if (j == 0) {
+ if (mydata-fatsize == 32 || !k) {
+ dir_ptr = do_fat_read_at_block;
+ k = 1;
+ } else {
+ dir_ptr = (do_fat_read_at_block + mydata-sect_size);
+ memcpy(do_fat_read_at_block, dir_ptr,
mydata-sect_size);
+ }
+
+ do_read = 1;
+
+ if (mydata-fatsize == 32 j)
+ do_read = 0;
+
+ if (do_read) {
debug(FAT read sect=%d, clust_size=%d,
DIRENTSPERBLOCK=%zd\n,
cursect, mydata-clust_size, DIRENTSPERBLOCK);
- if (disk_read(cursect,
- (mydata-fatsize == 32) ?
- (mydata-clust_size) :
- PREFETCH_BLOCKS,
- do_fat_read_at_block) 0) {
+ read_blk = (mydata-fatsize == 32) ?
+ mydata-clust_size : PREFETCH_BLOCKS;
+ if (disk_read(cursect, read_blk, dir_ptr) 0) {
debug(Error: reading rootdir block\n);
goto exit;
}
- dentptr = (dir_entry *) do_fat_read_at_block;
+ dentptr = (dir_entry *)dir_ptr;
}
for (i = 0; i DIRENTSPERBLOCK; i++) {
@@ -951,7 +966,7 @@ int do_fat_read_at(const char *filename, loff_t pos, void
*buffer,
get_vfatname(mydata,
root_cluster,
-do_fat_read_at_block,
+dir_ptr,
dentptr, l_name);
if (dols == LS_ROOT) {
--
1.9.1
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello,
On 12/18/2014 02:47 PM, Simon Glass wrote:
Hi,
On 11 December 2014 at 05:01, Przemyslaw Marczak p.marc...@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.marc...@samsung.com
Cc: Mikhail Zolotaryov le...@lebon.org.ua
Cc: Tom Rini tr...@ti.com
Cc: Stephen Warren swar...@nvidia.com
Cc: Simon Glass s...@chromium.org
Cc: Suriyan Ramasami suriya...@gmail.com
Cc: Lukasz Majewski l.majew...@samsung.com
Cc: Wolfgang Denk w...@denx.de
---
fs/fat/fat.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
Tested-by: Simon Glass s...@chomium.org
Thank you.
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.
k is a counter for a first time read. I will change this code and add
some comment.
+ int do_read;
+ __u8 *dir_ptr;
Why does it use __u8 instead of u8 or uint8_t for example?
if (read_bootsectandvi(bs, volinfo, mydata-fatsize)) {
debug(Error: reading boot sector\n);
@@ -910,23 +913,35 @@ int do_fat_read_at(const char *filename, loff_t pos, void
*buffer,
}
j = 0;
+ k = 0;
while (1) {
int i;
- if (j == 0) {
+ if (mydata-fatsize == 32 || !k) {
+ dir_ptr = do_fat_read_at_block;
+ k = 1;
+ } else {
+ dir_ptr = (do_fat_read_at_block + mydata-sect_size);
+ memcpy(do_fat_read_at_block, dir_ptr,
mydata-sect_size);
+ }
+
+ do_read = 1;
+
+ if (mydata-fatsize == 32 j)
+ do_read = 0;
+
+ if (do_read) {
debug(FAT read sect=%d, clust_size=%d,
DIRENTSPERBLOCK=%zd\n,
cursect, mydata-clust_size, DIRENTSPERBLOCK);
- if (disk_read(cursect,
- (mydata-fatsize == 32) ?
- (mydata-clust_size) :
- PREFETCH_BLOCKS,
- do_fat_read_at_block) 0) {
+ read_blk = (mydata-fatsize == 32) ?
+ mydata-clust_size : PREFETCH_BLOCKS;
+ if (disk_read(cursect, read_blk, dir_ptr) 0) {
debug(Error: reading rootdir block\n);
goto exit;
}
- dentptr = (dir_entry *) do_fat_read_at_block;
+ dentptr = (dir_entry *)dir_ptr;
}
for (i = 0; i DIRENTSPERBLOCK; i++) {
@@ -951,7 +966,7 @@ int do_fat_read_at(const char *filename, loff_t pos, void
*buffer,
get_vfatname(mydata,
root_cluster,
-do_fat_read_at_block,
+dir_ptr,
dentptr, l_name);
if (dols == LS_ROOT) {
--
1.9.1
Regards,
Simon
Thank you and best regards,
--
Przemyslaw Marczak
Samsung RD Institute Poland
Samsung Electronics
p.marc...@samsung.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello,
On 12/18/2014 02:47 PM, Simon Glass wrote:
Hi,
On 11 December 2014 at 05:01, Przemyslaw Marczak p.marc...@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.marc...@samsung.com
Cc: Mikhail Zolotaryov le...@lebon.org.ua
Cc: Tom Rini tr...@ti.com
Cc: Stephen Warren swar...@nvidia.com
Cc: Simon Glass s...@chromium.org
Cc: Suriyan Ramasami suriya...@gmail.com
Cc: Lukasz Majewski l.majew...@samsung.com
Cc: Wolfgang Denk w...@denx.de
---
fs/fat/fat.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
Tested-by: Simon Glass s...@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?
if (read_bootsectandvi(bs, volinfo, mydata-fatsize)) {
debug(Error: reading boot sector\n);
@@ -910,23 +913,35 @@ int do_fat_read_at(const char *filename, loff_t pos, void
*buffer,
}
j = 0;
+ k = 0;
while (1) {
int i;
- if (j == 0) {
+ if (mydata-fatsize == 32 || !k) {
+ dir_ptr = do_fat_read_at_block;
+ k = 1;
+ } else {
+ dir_ptr = (do_fat_read_at_block + mydata-sect_size);
+ memcpy(do_fat_read_at_block, dir_ptr,
mydata-sect_size);
+ }
+
+ do_read = 1;
+
+ if (mydata-fatsize == 32 j)
+ do_read = 0;
+
+ if (do_read) {
debug(FAT read sect=%d, clust_size=%d,
DIRENTSPERBLOCK=%zd\n,
cursect, mydata-clust_size, DIRENTSPERBLOCK);
- if (disk_read(cursect,
- (mydata-fatsize == 32) ?
- (mydata-clust_size) :
- PREFETCH_BLOCKS,
- do_fat_read_at_block) 0) {
+ read_blk = (mydata-fatsize == 32) ?
+ mydata-clust_size : PREFETCH_BLOCKS;
+ if (disk_read(cursect, read_blk, dir_ptr) 0) {
debug(Error: reading rootdir block\n);
goto exit;
}
- dentptr = (dir_entry *) do_fat_read_at_block;
+ dentptr = (dir_entry *)dir_ptr;
}
for (i = 0; i DIRENTSPERBLOCK; i++) {
@@ -951,7 +966,7 @@ int do_fat_read_at(const char *filename, loff_t pos, void
*buffer,
get_vfatname(mydata,
root_cluster,
-do_fat_read_at_block,
+dir_ptr,
dentptr, l_name);
if (dols == LS_ROOT) {
--
1.9.1
Regards,
Simon
Thanks,
--
Przemyslaw Marczak
Samsung RD Institute Poland
Samsung Electronics
p.marc...@samsung.com
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hi Przemyslaw,
On 18 December 2014 at 07:32, Przemyslaw Marczak p.marc...@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.marc...@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.marc...@samsung.com
Cc: Mikhail Zolotaryov le...@lebon.org.ua
Cc: Tom Rini tr...@ti.com
Cc: Stephen Warren swar...@nvidia.com
Cc: Simon Glass s...@chromium.org
Cc: Suriyan Ramasami suriya...@gmail.com
Cc: Lukasz Majewski l.majew...@samsung.com
Cc: Wolfgang Denk w...@denx.de
---
fs/fat/fat.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
Tested-by: Simon Glass s...@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?
if (read_bootsectandvi(bs, volinfo, mydata-fatsize)) {
debug(Error: reading boot sector\n);
@@ -910,23 +913,35 @@ int do_fat_read_at(const char *filename, loff_t
pos, void *buffer,
}
j = 0;
+ k = 0;
while (1) {
int i;
- if (j == 0) {
+ if (mydata-fatsize == 32 || !k) {
+ dir_ptr = do_fat_read_at_block;
+ k = 1;
+ } else {
+ dir_ptr = (do_fat_read_at_block +
mydata-sect_size);
+ memcpy(do_fat_read_at_block, dir_ptr,
mydata-sect_size);
+ }
+
+ do_read = 1;
+
+ if (mydata-fatsize == 32 j)
+ do_read = 0;
+
+ if (do_read) {
debug(FAT read sect=%d, clust_size=%d,
DIRENTSPERBLOCK=%zd\n,
cursect, mydata-clust_size,
DIRENTSPERBLOCK);
- if (disk_read(cursect,
- (mydata-fatsize == 32) ?
- (mydata-clust_size) :
- PREFETCH_BLOCKS,
- do_fat_read_at_block) 0) {
+ read_blk = (mydata-fatsize == 32) ?
+ mydata-clust_size : PREFETCH_BLOCKS;
+ if (disk_read(cursect, read_blk, dir_ptr) 0) {
debug(Error: reading rootdir block\n);
goto exit;
}
- dentptr = (dir_entry *) do_fat_read_at_block;
+ dentptr = (dir_entry *)dir_ptr;
}
for (i = 0; i DIRENTSPERBLOCK; i++) {
@@ -951,7 +966,7 @@ int do_fat_read_at(const char *filename, loff_t pos,
void *buffer,
get_vfatname(mydata,
root_cluster,
-
do_fat_read_at_block,
+dir_ptr,
dentptr, l_name);
if (dols == LS_ROOT) {
--
1.9.1
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello,
On 12/18/2014 03:34 PM, Simon Glass wrote:
Hi Przemyslaw,
On 18 December 2014 at 07:32, Przemyslaw Marczak p.marc...@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.marc...@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.marc...@samsung.com
Cc: Mikhail Zolotaryov le...@lebon.org.ua
Cc: Tom Rini tr...@ti.com
Cc: Stephen Warren swar...@nvidia.com
Cc: Simon Glass s...@chromium.org
Cc: Suriyan Ramasami suriya...@gmail.com
Cc: Lukasz Majewski l.majew...@samsung.com
Cc: Wolfgang Denk w...@denx.de
---
fs/fat/fat.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
Tested-by: Simon Glass s...@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.
if (read_bootsectandvi(bs, volinfo, mydata-fatsize)) {
debug(Error: reading boot sector\n);
@@ -910,23 +913,35 @@ int do_fat_read_at(const char *filename, loff_t
pos, void *buffer,
}
j = 0;
+ k = 0;
while (1) {
int i;
- if (j == 0) {
+ if (mydata-fatsize == 32 || !k) {
+ dir_ptr = do_fat_read_at_block;
+ k = 1;
+ } else {
+ dir_ptr = (do_fat_read_at_block +
mydata-sect_size);
+ memcpy(do_fat_read_at_block, dir_ptr,
mydata-sect_size);
+ }
+
+ do_read = 1;
+
+ if (mydata-fatsize == 32 j)
+ do_read = 0;
+
+ if (do_read) {
debug(FAT read sect=%d, clust_size=%d,
DIRENTSPERBLOCK=%zd\n,
cursect, mydata-clust_size,
DIRENTSPERBLOCK);
- if (disk_read(cursect,
- (mydata-fatsize == 32) ?
- (mydata-clust_size) :
- PREFETCH_BLOCKS,
- do_fat_read_at_block) 0) {
+ read_blk = (mydata-fatsize == 32) ?
+ mydata-clust_size : PREFETCH_BLOCKS;
+ if (disk_read(cursect, read_blk, dir_ptr) 0) {
debug(Error: reading rootdir block\n);
goto exit;
}
- dentptr = (dir_entry *) do_fat_read_at_block;
+ dentptr = (dir_entry *)dir_ptr;
}
for (i = 0; i DIRENTSPERBLOCK; i++) {
@@ -951,7 +966,7 @@ int do_fat_read_at(const char *filename, loff_t pos,
void *buffer,
get_vfatname(mydata,
root_cluster,
-
do_fat_read_at_block,
+dir_ptr,
dentptr, l_name);
if (dols == LS_ROOT) {
--
1.9.1
Regards,
Simon
Thanks,
--
Przemyslaw Marczak
Samsung RD Institute Poland
Samsung Electronics
p.marc...@samsung.com
___
U-Boot mailing list
U-Boot@lists.denx.de
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hi, On 18 December 2014 at 07:40, Przemyslaw Marczak p.marc...@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.marc...@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.marc...@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.marc...@samsung.com Cc: Mikhail Zolotaryov le...@lebon.org.ua Cc: Tom Rini tr...@ti.com Cc: Stephen Warren swar...@nvidia.com Cc: Simon Glass s...@chromium.org Cc: Suriyan Ramasami suriya...@gmail.com Cc: Lukasz Majewski l.majew...@samsung.com Cc: Wolfgang Denk w...@denx.de --- fs/fat/fat.c | 33 - 1 file changed, 24 insertions(+), 9 deletions(-) Tested-by: Simon Glass s...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello, On 12/18/2014 03:56 PM, Simon Glass wrote: Hi, On 18 December 2014 at 07:40, Przemyslaw Marczak p.marc...@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.marc...@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.marc...@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.marc...@samsung.com Cc: Mikhail Zolotaryov le...@lebon.org.ua Cc: Tom Rini tr...@ti.com Cc: Stephen Warren swar...@nvidia.com Cc: Simon Glass s...@chromium.org Cc: Suriyan Ramasami suriya...@gmail.com Cc: Lukasz Majewski l.majew...@samsung.com Cc: Wolfgang Denk w...@denx.de --- fs/fat/fat.c | 33 - 1 file changed, 24 insertions(+), 9 deletions(-) Tested-by: Simon Glass s...@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 RD Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello, On 12/16/2014 11:26 PM, Simon Glass wrote: Hi Przemyslaw, On 12 December 2014 at 08:30, Przemyslaw Marczak p.marc...@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.marc...@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 RD Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello, On 12/16/2014 11:26 PM, Simon Glass wrote: Hi Przemyslaw, On 12 December 2014 at 08:30, Przemyslaw Marczak p.marc...@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.marc...@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 Before, I wrote, that sandbox seems to be broken, sorry for this - it was just my dirty repo - sandbox compiles and works well. Best regards, -- Przemyslaw Marczak Samsung RD Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hi Przemyslaw, On 17 December 2014 at 02:03, Przemyslaw Marczak p.marc...@samsung.com wrote: Hello, On 12/16/2014 11:26 PM, Simon Glass wrote: Hi Przemyslaw, On 12 December 2014 at 08:30, Przemyslaw Marczak p.marc...@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.marc...@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 Before, I wrote, that sandbox seems to be broken, sorry for this - it was just my dirty repo - sandbox compiles and works well. Somewhat bewildering, but it does not in fact fix my problem. Here is a compressed version of the fat filesystem if you want to take a look: https://drive.google.com/file/d/0B7WYZbZ9zd-3NGRMNkFQQTdtV2M/view?usp=sharing Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hi Przemyslaw, On 12 December 2014 at 08:30, Przemyslaw Marczak p.marc...@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.marc...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hello, On 12/12/2014 01:32 AM, Simon Glass wrote: Hi Przemyslaw, On 11 December 2014 at 05:01, Przemyslaw Marczak p.marc...@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 RD Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
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.marc...@samsung.com
Cc: Mikhail Zolotaryov le...@lebon.org.ua
Cc: Tom Rini tr...@ti.com
Cc: Stephen Warren swar...@nvidia.com
Cc: Simon Glass s...@chromium.org
Cc: Suriyan Ramasami suriya...@gmail.com
Cc: Lukasz Majewski l.majew...@samsung.com
Cc: Wolfgang Denk w...@denx.de
---
fs/fat/fat.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
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;
+ int do_read;
+ __u8 *dir_ptr;
if (read_bootsectandvi(bs, volinfo, mydata-fatsize)) {
debug(Error: reading boot sector\n);
@@ -910,23 +913,35 @@ int do_fat_read_at(const char *filename, loff_t pos, void
*buffer,
}
j = 0;
+ k = 0;
while (1) {
int i;
- if (j == 0) {
+ if (mydata-fatsize == 32 || !k) {
+ dir_ptr = do_fat_read_at_block;
+ k = 1;
+ } else {
+ dir_ptr = (do_fat_read_at_block + mydata-sect_size);
+ memcpy(do_fat_read_at_block, dir_ptr,
mydata-sect_size);
+ }
+
+ do_read = 1;
+
+ if (mydata-fatsize == 32 j)
+ do_read = 0;
+
+ if (do_read) {
debug(FAT read sect=%d, clust_size=%d,
DIRENTSPERBLOCK=%zd\n,
cursect, mydata-clust_size, DIRENTSPERBLOCK);
- if (disk_read(cursect,
- (mydata-fatsize == 32) ?
- (mydata-clust_size) :
- PREFETCH_BLOCKS,
- do_fat_read_at_block) 0) {
+ read_blk = (mydata-fatsize == 32) ?
+ mydata-clust_size : PREFETCH_BLOCKS;
+ if (disk_read(cursect, read_blk, dir_ptr) 0) {
debug(Error: reading rootdir block\n);
goto exit;
}
- dentptr = (dir_entry *) do_fat_read_at_block;
+ dentptr = (dir_entry *)dir_ptr;
}
for (i = 0; i DIRENTSPERBLOCK; i++) {
@@ -951,7 +966,7 @@ int do_fat_read_at(const char *filename, loff_t pos, void
*buffer,
get_vfatname(mydata,
root_cluster,
-do_fat_read_at_block,
+dir_ptr,
dentptr, l_name);
if (dols == LS_ROOT) {
--
1.9.1
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] fs: fat: read: fix fat16 ls/read issue
Hi Przemyslaw, On 11 December 2014 at 05:01, Przemyslaw Marczak p.marc...@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] ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
