Rockbox

Well, it’s the darndest thing… after applying this patch, something called <MMC1> now appears at the top of my Sansa’s Files menu. Opening that gives me access to all the good stuff on my 2G SD card.

Congrats, Toni… looks good to me. Thanks a million!

One more thing… if by “hot swapping,” you mean removing and inserting a card while the e200 is powered up, I’ve tried it and experienced no freezeups whatsoever. The <MMC1> folder just comes and goes from the Files menu.

i tested the patch with a sandisk microSD card from my camera, no freeze (but just a quick check), copied some pictures from the card to the internal memory, without any problem.

great work again toni, keep it up!

and i think it wouldn’t a big deal to commit this patch to svn, because you only can have problems if you have a microSD card inserted, right?

Toni,

great work :)
Having a quick look at the patch (after actually applying it and being excited because it works) I’m fairly sure the HAVE_MMC define is used badly throughout the code, Accordin to ammicon it is used for targets with slow disks and no spinup time and low ram, which the sansa doesnt fall into, so i’m hoping that my attached version of your patch fixes your crashes which could have been because of that.

This doesnt fix tagcache or dircache at all, but it does change the <MMC> to <mSD> :) (also, if this is all good, the mmc_* function names will need changing)

   changes (13.7 KiB)

I did some mods here to avoid the occasional freezes but without luck yet. Instead with a new bootloader and after the mods it now takes ~3sec to detect the SD card, even after reverting to the original patch. During this detection time the sansa is not responsive and seems to hang in the card initialization routine. I will try to fix this issue before committing the patch…. And what about <SDCARD1> instead of <mSD1>? And yes, I would like to change mmc_* functions to extcard_* functions (if there is no better idea).

did you try with my patch? I still think the mis-use of HAVE_MMC could be your problem (I lost my card the other night, and buying a new one today to keep testing)

<SDCARD1> is just as good as <mSD1>, it would be even better if we could use the dos prtitio name there, but without some extra work I dont tinhk thats doable.

jdgordon,
yes, I tried your patch. But without luck. Still some freezes and detection time around 3sec (hama 512MB). This is too much compared to my first trials. Either I partly damaged the card/slot or the ‘old’ bootloader causes this detection time increase. Is the detection time on yours also ~3sec? Do you use an updated bootloader?

I’ve tried Toni’s original patch and am now using jdgordon’s version. Both work fine for me with no discernible difference other than the renaming of the card in the menu. I am using the current bootloader and I get instant detection. Still no freezes at all.

Toni, wow wierd…
Im using the latest bootloader and detectio times are <1s… so maybe it your card/slot?

we had a very quick talk in irc abot this and its OK to commit without fixing dircache/ramDB but this isnt going to get the last targe with external cards, so both shuold be fixed up in the fllness of time to work properly :)

jdgordon’s works for me… I am waiting for a commit *g*

I think it has a infiinte loop possibility, it happened a fwe tims this morning when i was playing with hotswap. scroll light worked so it wasnt a crash.

slightly OT, but I’ve been playing around with giving each card a “unique” id (really just incrementing the count of known cards) which works (although badly…), but the hope is that if we can get this workin nicely, hopefully we can get database to hide files which are not on the currently inserted card.

Yes, right. The ata-e200.c still needs a lot of failsafe handling. I found some infinite loop possibilities. But because of my own sd card problem, I currently hesitate to work on this.

Toni: Any idea if support for SDHC cards is possible? Or is the 2GB limit in the original firmware a problem for us too?

I fixed the obvious possible infinite loops in sd_init_device() by giving it a 5s timeout before just giving up and exting the function, and havnt noticed any freezes, so I dont know if that helped or not :(

Also incuded in this patch is the .rb_cardid detection I mntiond above (probably not wanted just yet, but dont have tim to strip it from the patch), and lastly, I have fixed up the mmc stuff so we now use hotswap instead of MMC and integrationg into target tree….

   hotswap.diff (33.5 KiB)

saratoga:
I don’t know exactly (no >2GB card here), but it seems, the external card can be handled in the same way as internal ones; switching the banks should do the job hopefully.

jdgordon:
I tried your latest patch against current svn: After adding the missing hotswap-target.h (adding prototypes), I get constant freezing during booting. Removing the patch solves this issue. Do I have to destrap something?

hmm, thats odd. I’ve had no freezing suring boot, does it only freeze if the card is inserted?

same patch as above but with the missing .h

   hotswap.diff (35.1 KiB)

Yes both, with and without sd-card inserted. Is the hotswap.diff really up to date?

I didnt touch it after uploading it last night. so yeah.
what freezing are yo having? does it unfreeze after 5s or its a full lockup? (come on irc…)

updated with the TIME_AFTER() params fixed, and a retry loop added if the card takes longer than 5s to start working.. (although this may nmot be the best place to put the loop?)

   hotswap.diff (35.4 KiB)

Jonathan, please upload your latest patch again. I accidentally removed the file inthe flyspray transition.

I hope this is the righ version :p

Bjorn, why arnt I getting notification emails?

   hotswap.diff (35.7 KiB)

This patch seems out of sync with SVN as a result of r13499. I get errors in apps/settings_list.c from applying this patch.

These are the errors… maybe it’ll help.
patching file apps/settings_list.c
Hunk #1 FAILED at 1188.
1 out of 1 hunk FAILED – saving rejects to file apps/settings_list.c.rej

really… an email and 2 messages on the tracker? If it falls out of sync there is a fairly good chance that the patch author will notice it next time they do a build, or someone else will upload a fixed patch… PATIENCE….

   hotswap.diff (29.3 KiB)

Yep… and I feel like a real idiot, because I figured out how to fix it myself anyway. I apologize for the commotion.

Resync with SVN. Increase the size of sd_stack in ata-e200.c because some people were experiencing stack overflow.

Charles

   hotswap.patch (36.6 KiB)

How about this?
- resynched with svn
- readds sdcard debug info
- the ideas from previous patches included
- panics on sdcard driver timeout detection (helps supporting)
- uses ‘card’ instead of ‘hotswap’ prefixes
- major rework of the sdcard driver (performance, simplification, minor bugfixes)

   hotswap1.patch (62.6 KiB)

nice work on the panicing :) just did a hotswap and on insert got error 14 EC_FIFO_READ_FULL.

Why rename the hotswap prefixes to card? I used hotswap so it would be blatantly obvious that it relates to the HAVE_HOTSWAP define and hotswap files. card I tihnk could be confusing later on.

in the patch there is both SYS_CARD_EXTRACTED and SYS_HOTSWAP_EXTRACTED….

hmmm, seems it likes panincing with error 14 if I start it with a msd inserted.

also, if all the cards (mSD and MMC) have the serial# then we can get rid of the .rb_cardid stuff in tree.c and just use that.

edit: bugger, swapped cards again, and crashed with no panic.

jdgordon:
I also get this error on the external card after many successful readings. So it is not special to my suspicous card. Increasing the timeout value for the ‘wait for FIFO full’ condition did not solve the issue here. OK, will think of your
other comments.

damn, at least we know its not a dodgy card though which is good.

I cannot get this patch working. I added this patch, and only this patch, to clean SVN and the build fails.

major_works:
The patching was ok? Did you do a ‘make clean’ + ‘../tools/configure’?

I cannot get this patch working. I added this patch, and only this patch, to clean SVN and the build fails.

Not sure what a “make clean” is, but I patched it to brand-new, checked-out SVN. Yes, it patched, but it failed in the build.

I have no problems at all.

Not sure what a “make clean” is,

Make clean clears out all the compile bits, but leave your configuration as it is.

but I patched it to brand-new, checked-out SVN. Yes, it patched, but it failed in the build.

If you don’t tell us what errors you are getting, no one can help you.

Charles

I cannot get this patch working. I added this patch, and only this patch, to clean SVN and the build fails.

OK, just did an update. You need to hand patch the reject for misc.c.

Charles

The ‘wait for FIFO full’ error code during reading seems to be the result of an “internal ECC failure”.
I got that bit set after executing the STATUS_REG command. This error appears ALWAYS when reading special
sectors on the external card (address here: 0×3068000). But reading the data via a PC connection gives
the correct result. So this error seems to be recoverable. Probably by changing the timings? Until that
issue is solved, I hesitate to commit this patch. But I currenly have no idea, how to solve this issue.

This patch when applied causes the Bookmark feature to not work. The option is there but when you press Select to create a bookmark nothing happens. I made a test build with my other patches (albumart, bmpresize, & radio) excluding the hotswap patch and the Bookmark feature works.

I attached the .rej file that I get when I apply the hotswap patch.

   misc.c.rej (0 KiB)

Opps,
I also modified the hotswap1.patch so it doesn’t ask for the location of the files to be patched

   hotswap2.patch (60.5 KiB)

This patch when applied causes the Bookmark feature to not work.

Do you mean songs on a MicroSD card (I don’t have one so I can’t test it out). However, bookmarking works fine for me with the hotswap1.patch applied.

Charles

No just in general through out the whole player.

The only error I get when I apply the patch is:
patching file /test/apps/misc.c
Hunk #1 FAILED at 47.
Hunk #2 succeeded at 816 (offset 1 line).
1 out of 2 hunks FAILED – saving rejects to file /test/apps/misc.c.rej

also modified the hotswap1.patch so it doesn’t ask for the location of the files to be patched

You should really learn about patch levels (-p option) in the patch program. You are bound to encounter problems like this in the future when applying patches.

No just in general through out the whole player.

Strange try my resynced patch and see if bookmarks are working for you.

Charles

resynced with SVN.

   hotswap1.patch (61.4 KiB)

Toni, could we perhaps retry instead of panicing if we get that FIFO full error? and I agreee that we shhuoldnt commit while we have this error.

Thanks Charles,
Now the Bookmarks work.

Can’t get current SVN to build with this patch applied, also it give out an error when patching. I attached the .rej file.

patching file firmware/target/arm/sandisk/sansa-e200/ata-e200.c
Hunk #3 FAILED at 67.
Hunk #4 succeeded at 243 (offset 1 line).
Hunk #5 succeeded at 282 (offset 1 line).
Hunk #6 succeeded at 298 (offset 1 line).
Hunk #7 succeeded at 366 with fuzz 1 (offset -1 lines).
Hunk #8 FAILED at 417.
Hunk #9 FAILED at 434.
Hunk #10 FAILED at 499.
Hunk #11 succeeded at 580 (offset -1 lines).
Hunk #12 succeeded at 650 (offset -1 lines).
4 out of 12 hunks FAILED – saving rejects to file firmware/target/arm/sandisk/s
ansa-e200/ata-e200.c.rej

   ata-e200.c.rej (19.8 KiB)

please.. pretty please.. dont comment when the patch falls out of sync, whent hat happens, it either means we have given up on the patch (unlikely), or we are too busy with other stuff to sync it… when we have time to play with it again we will realise it doesnt sycn and fix it ourselfs… or someone who knows how will post a fixed patch….

Ok, i’ve attempted to resync it, the recording patch broke it, and I’m not 100% sure its done correctly,.. attached is the patch and the conflicts (did a svn revert to the revision before the rec commit, patched, then updated.)

edit: forgot hotswap-target.h

   ata-e200-merged.c (21.8 KiB)
   hotswap.diff (64.2 KiB)

New version, uses IRQ instead of the tick task to detect, and fixes the ata-e200.c remerge (i stuffed it in the previous patch :p )

edit: yes, i just commited a quick one which breaks 1 line in this patch… you shuold b able to work it out yourself though :D

   hotswap.diff (60.8 KiB)

updated again to work with svn.
Toni, had any luck crashing it lately? The only panic I saw today was when the interrupts were being used wrongly :)

also, removed the card id stuff… it can be added later if we end up using it, but I tihnk the serial number for the card would be better

   hotswap.diff (58.3 KiB)

with the latest patch added, i can’t build the bootloader.

and i found a reproducable bug:
enter the file menu
move to some dir on the SD card
exit to main menu
reenter to the files menu
exit again to the main menu
and then if i try to enter the files menu again, it seems i’m stuck in an infinite loop

small hack session update:
Rene’s bug is very producable and im slightly suspecting this is a bug in the FAT/dir driver.. unless of course it doesnt happen on the ondio…

I have fixed the bootloader and sim to both compile, ideally, I’d like to be able to boot rockbox.mi4 off the sd card later, but for now the only chage was to get it to compile.

now onto new bugs after my hacking.
I had a feeling that we were crashign it because card_info[1].initialized = false; wasnt being set before the hotswap_inserted broadcast, so I moved that to above the if (!current_status) check in microsd_int() so it always happens. This shouldnt have caused any problems but it possibly has :(

Booting with a mSD card inserted gives interesting bugs (with svn bootloader). Either it will load rockbox correctly and panic with error 14 (btw, I saw PANIC: 1 which was odd… but only once.), or it will load rockbox, but it wont mount the internal disk! going into files shos the contents of the mSD as the / AS WELL as showing <microSD1> and <microSD2> which both are the same (the 2nd one is probably because I increas NUM_VOLUMES to 8 instead of 2 which I thought may be causing problems also.)

other than that, I have nothing to report… hopefully Toni has got somewhere?

OK, I give up for tonight.. attached is my not-working-so-nicely patch..

Through painfull debugging to the lcd I have narrowed the crash/loop down to firmware/drivers/fat.c..
somehwere between
if ( numsec > (long)fat_bpb→bpb_secperclus || !cluster ) {

          long oldcluster = cluster;
          if (write)
              cluster = next_write_cluster(file, cluster, &sector);
          else {
              cluster = get_next_cluster(IF_MV2(fat_bpb,) cluster);
              sector = cluster2sec(IF_MV2(fat_bpb,) cluster);
          }

in fat_readwrite()..

I give up for now…
does anyone know if this bug is because of something I did? or this has been there from the start but never reported before?

   hotswap.diff (59.8 KiB)

Jonathan: I’m seeing the bugs that you mention, i.e. *PANIC* Error 14, etc. Also sometimes Error 7. I have no idea if it’s anything you did. What I do know is that for me, the appearance of these bugs coincided with the commit of recording capability.

It’s possible it has to do with letting the driver yield and this might cause a timeout. It probably needs more strategic placement and some refinement in the status checking loops. I never did a scientific test but going back to no yields seems like it would cause too much of a holdup for other threads since the operation of the device seemed to smooth out a lot after adding it.

Recording relies on a yielding driver simply because yielding in recording itself when flushing causes problems for HD-based models since the ATA driver yields almost too much already.

comenting out the priority_yield() call in ata-e200.c doesnt change anything (didnt check if recording still works though)

It seems that changing the mutex’s to spinlocks caused the problem. I’ve chaged them back and am doing a quick recording text to make sure it still works.
Was changing them requied for recording to work? doing a recording which is greater than 32mb shold be enough to find out if its broken right?

edit: ok, getting annoying… changing the spinlocks to mutexes with the patch from last night still causes problems. is the ata driver really that finiky?

edit2: fuck! reverted to before the rec commit, merged the patch from that time, fixed it up, changed spinlock to mutex, updated then to current svn and stll crashing going root menu → sdcard :’(

I had this patch applied and I noticed it prints the filename of everything it reads. What’s with that? Wasn’t a debug build or anything. No crash but no external SD card either.

Things should run in a more timely manner with spinlocks and I’d recommend staying with it unless you want priority inversion problems in the driver. It will suffer the same as the ata driver on HD targets and the I2C on pp targets (which once made the player unresponsive during buffering) if it doesn’t use them…yielding or not. OT: Really, the need for them is a scheduler bug IMO.

BTW: Is there any contention that can result from attemping access of both simultaneously? I’m just thinking of stuff randomly atm though.

sorry, the filenames was me debugging, I thought I cleaned all of that from the diff, must have missed one.
I’m sure it could be posible that both disks could be trying to be accessed at the same time, but in both read and write the current disk is checked against and will initialise the mSD card if the hard diskwas the last accesed, and both use the same locks, so that shuoldnt cause problems…

edit: small update.. it seems the merge screwed the spilock initing… mutex_init() was being called at ata_init() AND spinlock_init() was beign called at the end of sd_init_device(), however, removing the one in sd_init_device() doesnt fix the problem yet (that is only called when the the caller has the lock so dont try to unlock it again…) It still freezes up somewhere

We are working again!

the major problem now is that it will crash/infinite loop very reliably if we try being smart at all and not re-initializing the disk before a read/write. so its a very noticable slow down on boot (an extra second or 2 maybe), not noticable anywhere else though. Havnt tried recording though.

Mike: I have to use mutex’s through the code… using spinlocks will crash very reliably if you start music from the sd card, then go back to the root menu while its buffering (may actually crash before that, but doing that twice was enough to prove it didnt work for me.)

hmm… that was odd… I’m listening to music on the sd card atm, and it just changed tracks to a different folder (on the sd card) in the middle of a song… probbaly unrelated though becuase it didnt crash at all….?

please test the patch and let me know how you go :)

   hotswap.diff (58.7 KiB)

Using the ‘latest’ sansa (new scrollwheel, shiny backing) Rockbox freezes at the logo with this patch. When not using this patch, it’s fine. Tested with JdGordon’s attachment from both June 7 and June 10

I won’t accept that spinlocks cause it since that makes no sense whatsoever in itself. It’s a symptom of another problem with the code and they simply reveal it. How could timely thread execution through the code break it unless it relies some extra delay or something? Why should a waiting thread be forced to sleep? There’s no difficulty with the SD driver in current SVN. Having a system where a background thread like the database thead keeping playback and recording from being able to access the “disk” isn’t really cool.

The need to reinit, use regular mutexes (any lock mechanism should at least function) and other problems I see you’re having shows that it needs a real going over. I’ve no guess at what it is atm. :\

I agree they shuoldnt be causing problems, but they are.
The only context switch in the ata code now is the priority-yield right? could that be causing a deadlock?
Im going to play around with the old versions of the patch (before the ata-e200 rework toni did) and see if it works better.

No lockup will be possible from mutexes/spinlocks unless there’s an exit path that doesn’t unlock the guarded section. There’s no case of a mutual wait for a resource owned by the other…or I didn’t see one off hand. The main difference here is that without the context switch, the mutex is unneeded anyway with just one core cooperatively scheduling threads as no other thread can run while one is running in the driver.

Maybe the lock isn’t entered at the right time and should happen sooner?

I was looking closer and find a few things fishy:

The radio driver is doing outl(inl(0×70000080) | 0×4, 0×70000080) when the radio is started. The sd card driver turns this bit on and off depending on the current volume and I’m guessing the radio driver shouldn’t touch it.

Perhaps the message handlers in sd_thread should enter the lock when changing the file system around.

Why is a full init being done every time the current volume is changed? Ex: Even with filtering the init, why do a DEV_RS all the time?

More stuff but I won’t run the web browser out of memory listing it all ;)

/me needs to obtain and read the SD card manuals now

Synced to current SVN. Uses the right GPIO IRQ bit. Before it was just detecting the change when a button was moved. Card appears/disappears correctly when popped in/out. Hope I didn’t forget any files that the patch adds. :) Things need much work in general to have this hotswap thing stable and fast.

   sansa-sd-hotswap3.patch (59.5 KiB)

This should have significant speed advantage over the last patch and somewhat over SVN code. Enables the virtual LED just for kicks too. Has some residue from my messing around trying to reduce the amount of card communication during card switches but I have to post this even without all that worked out. I think it could be committed essentially in this state.

I can’t say I’m at all happy with the robustness of rockbox in in general in handling errors from a precipitously extracted SD card. </gripe>

   sansa-sd-hotswap4.patch (68.7 KiB)

just tried it out, got a freeze the first time i plugged a chip in.
panic, error SDCard: 1

Like Jonathan, I got a freeze on first insertion of a card, only mine said Error 10. After rebooting, it worked flawlessly.

By the way, it also gives me *PANIC* Error SDCard: 10 if I pull the card while playing a song off it. Not that I’d plan on doing that ordinarily – I just wondered how it’d handle that.

By the way, it also gives me *PANIC* Error SDCard: 10 if I pull the card while playing a song off it. Not that I’d plan on doing that ordinarily – I just wondered how it’d handle that.

Mea culpa… please disregard the foregoing comments (especially the repeat, which I don’t know how I managed).

The problems I reported were actually with sansa-sd-hotswap3.patch. When I recompiled using fresh source and sansa-sd-hotswap4.patch, they seem to have been rectified. I get no freezeups at all. In fact, pulling the card while playing a track seems not to faze it in the slightest; it just keeps playing. I even FFed to near the end of a track from the card and then pulled it out. It moved on to the next one as if the card were still inserted.

Johnathan and I worked out a few small changes in IRC and he reported good behavior too so his report is a bit out of date. Some cards need more time after plugging and .1s seem to do the trick as well as a messaging order change. I’ll post those changes shortly.

With the tweaks. Works for me. No funky comments. Ready to commit this if it all tests out ok. :)

   sansa-sd-hotswap5.patch (67.3 KiB)

Sansa still freezes on startup as noted before. After an hour or so it says Error SDCard: 10 as it did before.

Sansa still freezes on startup as noted before. After an hour or so it says Error SDCard: 10 as it did before.

Apparently it likes to repost information when I refresh…

It appears that some e250s have two chips on the daughterboard (each 1024 MB) while others have one chip (2048 MB)

Jack,

I take it that the problems occur with the SD card, but if no SD card is plugged everything is ok with the internal flash?

Could you perhaps post the debug info for your memories (System|Debug|View microSD Info)? All info from each page in a .txt file would be fine. Your info may tell something important about timings.

I have a 4GB model with 2x2048MB so multiple chips in itself aren’t the problem but perhaps the 1024MB flashes are slower in some respect?

Personally, I’d be ok with committing this so long as main drive access in not hurt since nothing would be lost over current SVN and it would encourage wider trials. Many would also benefit with being able to use the SD card right off the bat. Work on getting it operational for everyone would of course continue.

Mike, no he was saying he doesnt have a sdcard, it was happening with none instered.

Did he? I don’t read him saying that anywhere (mentions new scrollwheel and shiny backing) but ok. I suppose a comparison is in order between the new and old code which I’ve not done yet. I’d still like the debug info about it anyhow.

woops, he said it in irc last night.

I can not get debug info from Rockbox. As I said before, when the patch is applied, I can not access Rockbox. It freezes at the Rockbox logo for over an hour until returning Error SDCard: 10, which is why I was asking on IRC if it was possible to modify the bootloader or Rockbox to display some sort of debug information before/during the time that the rockbox logo appears (since it freezes at the logo, debug info would then be visible for me to copy it)

I somehow fixed my problem by inserting a MicroSD card, now it works both with and without the card inserted, so I have no problem with this being commited.

Try booting into the original firmware and then restarting the patched rockbox without the card. I want to see if it reverts to the previous behavior.

This patch seems to be working fine for me (using a 1GB Kingston card).

Jack,

You there? Could you please check that the patch doesn’t revert to the previous behavior after booting to the original firmware? If it does, there’s something else that needs setting up (but I can’t imagine what yet) and if it doesn’t, perhaps something in the card slot was physically stuck and dislodged after inserting a card?

Sorry about the delay Mike - I’ve been on vacation the past week. The problem doesn’t revert after loading the OF - it still works properly.

Jack, good deal. Must have been a physically stuck switch. I think I’m up for committing this then. I’ve made a couple changes to the filesystem mounting where if it happens to get two inserts for some reason, it will unmount a remount but it’s just a safety measure and should have no negative effect on file access. Look for it in SVN anytime from now ‘till before the end of the weekend (unless I get a big “STOP!” :).