This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#8663 - Data corruption on usb write on sansa (SD driver bugs)
Attached to Project:
Rockbox
Opened by Frank Gevaerts (fg) - Friday, 29 February 2008, 00:00 GMT+2
Last edited by Alex Parker (BigBambi) - Sunday, 06 June 2010, 12:52 GMT+2
Opened by Frank Gevaerts (fg) - Friday, 29 February 2008, 00:00 GMT+2
Last edited by Alex Parker (BigBambi) - Sunday, 06 June 2010, 12:52 GMT+2
|
DetailsThe udelay() in ata-c200_e200.c is not needed on my c250, and actually seems to cause write errors during USB connections. This patch makes it conditionnal on SANSA_E200.
Not committed yet because of the potential problems if I'm wrong |
This task depends upon
If someone would like to experiment with DMA read/writes, I've attached the
disassembly from the c200 bootloader.
I've removed that udelay, build with USE_ROCKBOX_USB and USB_HIGH_SPEED.
Then I've connected sansa to computer, copied xubuntu-7.10pl-desktop-i386.iso to microSD card, during copying dmesg said many high speed device reset and two "usb 2-3: device descriptor read/all, error -110" between device resets.
I've put card in card reader then, diffed file against the original - it was exactly the same!
Then, I've started sansa, put microsd card into it, copied file from card to internet memory on sansa (in rockbox), rebooted sansa into OF, and diffed that file against the original one at my computer - it was eactly the same as well.
Further testing shows that on my c250 it doesn't seem to happen (or happens much less) on the internal flash. Only the micro-sd slot seems to be affected.
I would like to change the register definitions accordingly to the documentation of these chips to make further work a bit easier.
They would read like MMC_STRPCL, MMC_STAT, MMC_CLKRT and so on.
Does anybody mind?
Most of them obviously make sense, some unused registers have been identified by probing their default values with e200tool.
The rest is still guessing, especially the CMDAT and I_MASK bits.
It would be nice, if someone would consider to commit this or comment on what to change.
It shouldn't harm anything, as there are no functional changes yet.
The asm code will only work with an aligned buffer. With an unaligned buffer, the old slow routine is used along with an additional udelay to prevent the bug (that's odd, yes). But do we actually ever have an unaligned buffer with ata_write_sectors?
I did very intense testing and it turned out to be absolutely reliable with my e260 and a Sandisk 4GB SDHC card for several weeks now. Write speed is around 3.5 to 4 MB/s on the SD card, the internal flash is slower.
Though, I'm quite uncertain on how it will behave with different players or SD cards because of this fragile timing. Maybe even the speed of the SD card might have its impact (I only own one card). The speed of the internal flash might differ between devices as well. So it will still need a lot of testing. Backups!
1/ compile rockbox for clean svn trunk
2/ install on player using original firmware for transfer
3/ disconnect player
4/ connect using rockbox and copy 550 files (mostly music tracks)
5/ disconnect player
6/ connect using rockbox and run md5sum -c reference.md5 to check if files were ok or not.
Just to make sure that the microsd errors weren't due to a crappy partition, I reformated the disk (with udelay removed) and ran the test (with udelay included) again.
svn-trunk.17708.internal.check
550 OK
svn-trunk.17708.microsd.check
481 FAILED
27 FAILED open or read
42 OK
svn-trunk.17708.microsd.validation.check
465 FAILED
7 FAILED open or read
78 OK
svn-trunk.17708.no-udelay-in-sd-ata.internal.check
460 OK
svn-trunk.17708.no-udelay-in-sd-ata.microsd.check
550 OK
As you can no doubt see, removing the udelay fixes writes to the microsd card.
Antoine, I assume you are referring to Franks initial patch. For me, that udelay was definitely required on my e260. But I always used high speed USB for my tests.
(I already was able to copy files without the udelay over high speed, but I haven't had time to do the md5sums yet. What kind of issues were you getting? Are/were they easily reproducible?)
- increased the udelay from 2 to 3 (Apparently, I was too optimistic)
- cleaned up the asm code
It's quite a bit slower but should be more reliable.
From computer to internal flash I transferred 257 files totaling 1.2 GB. Only one file was corrupted.
From SD to internal flash I transferred 234 files totaling 1.6 GB. Only three were corrupted.
Almost there. Even with the additional delays, high speed transfer is easily 3 times faster than full speed.
I just tried to do the tests with the OF. Although I verified that there was no corruption, my computer lost connection several times (KDE automatically unmounted and remounted) which caused me to restart my verification several times. I did not have this problem with rockbox. I'm curious if other people can reproduce my problems with your latest patch.
Without the patch, running hi-speed I get corruption on 203 of 253 files. Unfortunately, I left my SD card at work, so I can't test it.
Would have been interesting, which kind of corruption your file had.
Jonathan, thanks again. Did you retry the database rebuild? And can you somehow confirm that the remaining corruptions are in fact from the 'two bytes inserted' bug? In my tests it either occurred quite frequently, or it was completely gone. It never happened that sporadically to me. I don't know how to check that with Linux though. Hmm, if the file sizes would be different, then it's definitely not this bug. If they're the same, it probably is.
As I don't have any corruptions at all anymore, I don't know how to improve this any further.
- #ifdef only for e200 as no other targets have been tested yet.
- Not using sd_poll_status() inside the loop because yielding may disturb the timing.
This will hopefully fix the last remaining file corruptions.
- The new code will be used only in USB mode (usb_exclusive_ata), the current code remains completely unchanged.
This way
- We don't have to care for the buffer alignment as the USB buffer will be aligned always.
- Testing will be much easier because only USB mode has to be tested.
- It will not break anything when not in USB mode.
Note that if there was any non-USB related corruption, this will still be there
(I didn't apply any of the patches from this task, to maximise my chances of triggering the corruption)
Seems to work pretty well. The write speeds seems slower but the read speeds are comparable. On write, I'm getting just over 2 MB/s.
Thanks Frank.
Jonathan, I wonder why our test results are so different. Just to make sure, did you remove my patch before testing r17997?
I just double checked with current SVN and a fresh formatted SD card, but the corruption is still there. Then I did a comparison without SERIALIZE_WRITES and found no significant difference. Within a 10MB test file I had the 2 bytes inserted up to 10 times with SERIALIZE_WRITES and up to 8 times without (5 tries on each).
- copied a directory of 415 MB (95 files) within rockbox from internal flash to sd in 2:24 min
- bitwise compared all files via USB - no error
- deleted the directory on sd
- raised the boost counter in the Debug/CPU frequency screen
- copied same files again in 2:21 min
- bitwise compared all files via USB - no error again
- copied a 12 MB file from PC to sd via USB and got 2 bytes inserted at 15 random positions
Since the measured times don't differ much, I believe the CPU is boosted during a file copy as well. So that's probably not the reason.
Another difference between internal copying and USB transfers might be the buffer size. I'll do some tests on it ASAP.
So that's probably the significant difference which makes the bug appear with USB only. But what to do about it? Buffer in IRAM?
Oops, did I say that? ;-)
SERIALIZE_WRITES enabled
SD card: 2.5 MB/s, no corruption
internal flash: 2.1 MB/s, no corruption
SERIALIZE_WRITES disabled
SD card: 2.8 MB/s, no corruption
internal flash: 2.3 MB/s, no corruption
So I think, SERIALIZE_WRITES can be reverted. If the existing code in ata_write_sectors() is considered as proven to work, this patch should be safe. For gaining more speed (up to 3.6 MB/s on SD), my above patch might be considered. But that would surely need some public testing phase, as it introduces new time critical code. Basically, both patches do the same thing: speeding up the FIFO feeding by either using cache or ASM. I wonder what if both patches would be combined...
SERIALIZE_WRITES disabled
SD card: 1.76 MB/s, 5/506 files corrupted
SERIALIZE_WRITES enabled
SD card: 1.57 MB/s, 17/506 files corrupted
What a weird bug...
Did you also check the filesystem for errors between tests, or did you reformat? I'd like to rule out pre-existing errors as a cause for the higher number of errors in the SERIALIZE_WRITES case.
Just to be sure : is this the same sd card that you used previously when you found no errors? Also, how big was your test then?
Steve, any tool for testing USB memory sticks will do. I often used this one: ftp://ftp.heise.de/pub/ct/ctsi/h2testw_1.4.zip
Start testing 10 MB, if that works 100 MB, then the whole card.
Panasonic has a utility for properly formatting SD cards here: http://panasonic.jp/support/global/cs/sd/download/sd_formatter.html See if this makes any difference.
when I have my usb printer connected then there's corruption when copying files to internal flash, but if I unplug the printer all data is ok.
(I've done 4 tests using 5.3gb of data - two with printer connected (different files were corrupted), two without (all data matched))
I used the program that Martin recommended for testing USB sticks and I get far fewer errors. Using SERIALIZE_WRITES and the caching patch, I twice filled my 4GB SDHC with no detected error. Once it detected about 800K of sectors that were wrong.
Before, I tested by copying my music library over to the card, and then comparing each file bit-by-bit. This is fairly easy under the linux command prompt. I just repeated this test for SERIALIZE_WRITES with caching just to be sure. 10 of 509 music files were corrupted. No such corruption occurs with just SERIALIZE_WRITES.
I have no idea why many files would cause a problem, but just a few wouldn't.
Regarding speed: my write rates are much lower than Martin's. This seems to be independent of whether I access the SD or built-in flash. It is much slower than the OF.
OF - 4.81 MB/s
SERIALIZE_WRITES - 1.52 MB/s
SERIALIZE_WRITES w / caching - 1.67 MB/s
caching w/o SERIALIZE_WRITES - 1.90 MB/s
Yesterday I had a corrupted file where a range of sectors hasn't been written at all. It contained the contents of the file which was stored previously at that position. The screenshot desowin did at http://www.rockbox.org/irc/log-20080822#22:25:13 looks pretty much the same. I wasn't able to reproduce this yet. It seems to happen extremely rarely, at least with my hardware.
Jonathan, do you have the ability to prove what kind of corruption you're still experiencing?
Steve, for high speed USB you have to define USE_HIGH_SPEED as well.
I wanted to chkdsk it, but when I connected sansa to my laptop, it just froze whole Vista. (it unfreezed after I unplugged sansa)
But anyway, as we see, it's hardly possible to find a workaround that will work for everyone. So we really have to find the underlying problem, instead of trying to cure the symptoms. Maybe I already found out something, but yet it raises another problem. Investigating further...
I already compared this controller to various SoC. The closest match is probably the PXA27x series plus a bit of PXA255 (search Google for PXA27x developer's manual). My patch at http://www.rockbox.org/tracker/task/8663#comment23382 was about renaming the register and bit definitions accordingly to this manual. It's out of sync now, but I'd resync it, if it would have a chance for committing. Actually, there is too much work in it to be dumped silently.
FS#8663. Because of missing specifications the solution provided here is all 'try and error' and needs a lot of testing before committing. I misused the debug menu "CPU frequency" for test purpose.Warning: Because the write test procedure uses raw sector access, some/many files will get destroyed, when running the "CPU frequency" debug menu!!!
If you still want to help, then please test your internal/external cards on e200/c200 in the following way:
- run debug menu "CPU frequency" with this patch (leave the test with 'context menu', long press of DOWN button)
- change number of write sectors
- change delay (0 ... 200 loops => 0...10usec @80MHz for interrupt simulation)
- change int/ext card
- change 30/80MHz
Always watch the number of bad sector writes in the display. Of course there should be no bad sector writes at all. Report your results back.
Additional infos:
I have experimented with the 'two bytes inserted' bug on the e200 and I come to the same conclusion as MartinRitter: There seems to be a hw related bug in the sd fifo handling. I used MartinRitter's copy-optimized patch with a modified ordering of FIFO_EMPTY poll status (to avoid the lockup problem) and replaced the udelay() with a more precise small asm wait loop, which needs 4 cpu cycles. By increasing the loop counter I got following results:
CPU @ 80MHz:
loop counter < 12: bug appears frequently
loop counter 12 ... 86: bug disappears
loop counter 86 ... 106: bug reappears
loop counter > 106: bug disappears
CPU @3 0MHz:
loop counter < 10: bug appears frequently
loop counter > 10: bug disappears
dram<->iram: has no effect.
int.card<->ext.card one external card showed the same bug, another did not show the bug
30MHz<->80MHz has effect. See above.
In a second approach, see this patch, I modified the UNKNOWN register in SD_WRITE_MULTIPLE_BLOCK call. The udelay() was no more necessary with the slow copy routine. I didn't get the bug at 30MHz/80MHz, dram/iram, int.card/ext.card and all number of sectors tested. The slow copy routine copies 1Byte/8cycles, which makes 10MB/s @80MHz.
I coped the windows 7 beta image from my ntfs partition to the sansa in linux.
$ md5sum /media/XP/Windows7/7000.0.081212-1400_client_de-de_Ultimate-GB1CULXFRE_DE_DVD.iso
52173cce05f14f14f6516ca4b5fe3482 /media/XP/Windows7/7000.0.081212-1400_client_de-de_Ultimate-GB1CULXFRE_DE_DVD.iso
Filesize: 3,357,288,448
$ md5sum /media/SANSA\ E200/7000.0.081212-1400_client_de-de_Ultimate-GB1CULXFRE_DE_DVD.iso
52173cce05f14f14f6516ca4b5fe3482 /media/SANSA E200/7000.0.081212-1400_client_de-de_Ultimate-GB1CULXFRE_DE_DVD.iso
Filesize: 3,357,288,448
Rockbox md5sum plugin:
D41D8CD98F00B204E9800998ECF8427E /7000.0.081212-1400_client_de-de_Ultimate-GB1CULXFRE_DE_DVD.iso
Filesize: -937,678,848 (Rockbox file properties plugin)
Several resets occured:
[37955.448216] sd 53:0:0:1: Attached scsi generic sg8 type 0
[38347.840024] usb 3-1: reset high speed USB device using ehci_hcd and address 51
[39124.828069] usb 3-1: reset high speed USB device using ehci_hcd and address 51
[39680.436020] usb 3-1: reset high speed USB device using ehci_hcd and address 51
[39716.480021] usb 3-1: reset high speed USB device using ehci_hcd and address 51
[40524.172035] usb 3-1: reset high speed USB device using ehci_hcd and address 51
[41218.636014] usb 3-1: reset high speed USB device using ehci_hcd and address 51
[41238.420016] usb 3-1: reset high speed USB device using ehci_hcd and address 51
[41240.148018] usb 3-1: reset high speed USB device using ehci_hcd and address 51
[41240.416021] usb 3-1: reset high speed USB device using ehci_hcd and address 51
Another note: The first 1,7GB transfered very fine (no resets), but then it got utterly slow, with many pauses and the resets occuring.
1. The critical time span seems to be from signaling FIFO_EMPTY until the FIFO is filled completely. It is possible to start filling the FIFO immediately after detecting FIFO_EMPTY without any delay at the maximum possible speed. But the last word must not be written to DATA_REG before at least 3 usec have elapsed since FIFO_EMPTY.
2. Setting SD_STATE_REG to SD_RCV right before the 'for' loop in sd_write_sectors seems to prevent the bug in case of higher delays. It somehow changes the behavior of the controller. As a side effect, we additionally need to wait for FIFO_EMPTY right before waiting for DATA_DONE at the end of the loop. Without this, the last sector was not written sometimes.
@kugel: Did you use the standard ata_write_sectors() or ata_write_sectors()_mod for the copy action
@MartinR: Great findings. Can you post a patch here?
Applying the patch as is (with the last parameter of SD_WRITE_MULTIPLE_BLOCK set to 0xd), I did not notice any sensitivity to the delay parameter. Encouraged by this, I replaced sd_write_sectors in ata-sd-pp.c with the sd_write_sectors_mod included in the patch. After extensive testing, I have not found any corruption copying files over USB to and from internal storage and a 2GB microSD card (copying GB of data and 100s of files).
What's the difference in the new magic number (0xd) used in the call to SD_WRITE_MULTIPLE_BLOCK? Taking the SVN code and just changing 0x1c2d to 0xd caused corruption. So it's not just the magic number that makes the new code work.
Attached is the patch I'm using for weeks now. Comments inside. Your modified CMDAT flags seem to have a similar result as my SD_RCV approach, but without the side effect of the missing last sector. So I would prefer your approach here.
Because there seems to be no proper bugfix in the near future I am all for including yours or mine (or a combination of both) workarounds in SVN, if there is no negative feedback here.
What do you think about my register renaming patch here: http://www.rockbox.org/tracker/task/8663#comment23382
It got no attention so far. Do you want me to resync it?
I bet I'm not the only one who's keen on getting rid of the OF entirely.
Also, re the #defines patch. I like to have that in too. Hopefully it'll make the pp sd driver more understandable and more adoptable (with an eye on the yet-missing bank switching mechanism in the as3525 sd driver).
I've did some tests:
copy 1.4GB music files from PC to Sansa: MD5 all fine.
move 1.3GB music files from MicroSD to internal storage within Rockbox: All md5s are fine.
move 1.3GB back from internal storage to MicroSD *while music is playing* to get alternating CPU clocks: all md5s are fine.
edit: 2 more tests:
move the same 1.3GB back to internal again with music playing: all md5s are fine
copy the same 1.3GB to Microsd with music playing (music from the folder to be copied, I'm fairly sure I began copying before buffering ended): all md5s are fine
I made the SERIALIZE_WRITES dependant on CPU_PP for now. If it's the safer transfer method, it should probably be used by work-in-progress targets.
I'm all for committing!
!!!!Warning!!!!:
Because the write test procedure uses raw sector access, the file system may get corrupted when running the "CPU frequency" debug menu!
Sansa e200, c200 and GoGear testers are needed to test this patch, because the timings may differ between these players. To do the test:
- run debug menu "CPU frequency" with this patch
- change number of write sectors (1-128)
- change aligned/misaligned data access
- change int/ext card
- change 30/80MHz
and watch the number of bad sector writes on the display. Preferably the test should run with music playing. Report your results back.
Anyway, after the reboot, I transferred several GB of data without a hitch. I will note that my transfer speed with the patch is 1.1 MB/s, whereas I was getting closer to 2 MB/s before applying the patch, and close to 4 MB/s with the OF.
Used h2testw to test write & verify over USB 17MB on internal storage with no errors. Wrote & verified 7.41GB on external storage with no errors. Copied test files off via USB and verified on hard drive with no errors. Copied files back over USB and verified on player with no errors. Write speed ~3.2 MB/s. Read speed ~6.1 MB/s.
The only odd behavior seen was with moving the h2testw generated files off the ext storage to my pc over USB. A short time after starting the operation I decided to cancel. The move process and explorer became unresponsive while attempting to cancel. Unplugging the USB cable unstuck the processes and after replugging the cable I was able to reinitiate the move with no problems.
+1 to commit!
http://www.codinghorror.com/blog/images/works-on-my-machine-starburst.png
I copied 7GB of data to external storage (SDHC) and found no errors at all (I tested all md5 sums). However, the transfer hanged the first time after sending 20MB and I had to unplug the usb cable to recover. This didn't happen again, and apparently caused no error on filesystem. Transfer speeds were about 3MB/s.
Before I do will someone please clarify for me -- As of r19911, all I have to do is enable USB transfer, through my make file? Or is there also a patch on this page I need?
That is all I have done (change make file), and I'm getting bugs (not file/file system corruption though). Should I post them here?
I understand what the patches here, are for. The only change I have made before compiling is to enable USB transfer through the make file, but since r19911 is coming from work on this page, and this page is referenced in r19911, I figured this is where to report bugs on it. (I figured a normal bug report was wrong since I had to change the make file in order to encounter my bugs)
-------------------------------------------------------------------------------------------------------------------------------------------------
Toni:
Before I tell you the two bugs, I want you to know that I have not used any patches from this page! The only change I made was to enable USB transfer through my make file. Do I also need a patch from this page?
Two bugs, but neither is file or file system corruption.
They both occur after disconnect (when the connection was through RBs' firmware).
One is, right after disconnect, I get a screen that I cannot exit (requiring a hard reset). The message on that screen is:
========================
data abort
at 00009240 (0)
========================
The other is that when I click "Resume Playback" OR open an audio file through the "Files" menu, I get no playback (the time position stays at "00:00"). The only way to get playback working again, is to open a bookmark -- after that I can open files through the Files menu again.