Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Operating System/Drivers
  • Assigned To No-one
  • Operating System Another
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.4
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by FlynDice - 2009-10-10
Last edited by FlynDice - 2009-10-15

FS#10669 - AMS Sansa Reimplement Voltage Scaling

This patch reimplements voltage scaling for the AMS Sansas. I have set the reduced voltage level at 1.05 V in the hope that the lower voltage will be more likely to expose any bugs that may show up. When actually implemented I would expect to use 1.10 as the reduced voltage. kugel and mc2739 came up with this solution several month ago I have simply massaged the implementation here a bit.

This patch uses cpu_boost() during sd card accesses when a microsd is present to raise the core voltage back to 1.20V. This means that for the clip, and also for the other targets when a microsd card is not present, it does not use cpu_boost() on card access and remains at the lower voltage.

I have run this at 1.05 on my e280v2 for several hours now both with and without a microsd with no problems.

Closed by  FlynDice
2009-10-15 19:58
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

r23193

This breaks my microsd card access. The player when initially switched on will not read the card. This is when i access it through files form the main menu. If i pop the card out and in again it reads the contents and i can get access to play music from the card. After a while it crashes out with error.

Undefined instruction
at: 30800CF8

Side note, the the previous patch at 1.10v worked okay. I have an 8 GB class 2 Sandisk SDHC on an e260V2.

Works fine on the Clip

The Fuze deadlocks when I remove the µSDHC (Transcend 4GB Class 6)

Works fine on my e260v2. My card is a Sandisk 4GB class 2 SDHC.

The error i am getting is directly related to the card. As we would expect. This does not seem related to cpu boost and i have inspected the buffer thread information. I have tried inserting a card and then just listening to the FM radio and the player will still crash with the same error.

I do not know how long before you drop the voltage back to the 1.05 after card insertion, maybe this coincides with the error. I have tried this with another 8 GB Sandisk Class 2 SDHC card this one with different disk info in the debug menu and very much the same results.


											

Modified my voltage back up to 1.10 and it is running okay now. I have been playing around with inserting and removing the card. Sometimes it is missing that i have taken the card out or put it in. This causes it to think there is an SD card inserted when i take it out and removes it when i am putting it in. I am not sure how you are detecting that the card has been ejected or put in but that currently can misread what is happening.

Since the card detection seems to be creating a problem I don’t think it’s worth the effort to try to sort it out. This patch simplifies things and uses cpu_boost() on anything that has a microsd slot(all except clip…)

The voltage is still at 1.05 but is easily changed to 1.10. If 1.05 gives you problems try changing line 370 in system-as3525 from CVDD_1_05 to CVDD_1_10.

This new patch still works fine for me.

Edit: I have noticed some blue pixels that I didn’t see before applying this patch. They show up consistently on the Rockbox logo displayed by the main firmware (not bootloader). They are about half way down the display in one line. This is with r23123. I do not see them with a clean build.

Edit 2: I have reapplied the first patch with r23123 and do not see the blue pixels.

I can’t duplicate the blue pixels here. I’ve got r23123 with this patch applied and I’ve tried rebooting a whole bunch of times with no luck.
I am puzzled why the first patch would not exhibit the same problem as it is basically the same process with a check to see if there is a card present.
And no, I am not doubting your results as you are the ultimate tester!

Does raising the voltage to 1.10 have any effect?

No blue pixels with the voltage at 1.10V

Ok, I think this one should work on all players with no problems. I have not dropped the voltage on this one so it stays at 1.10.
It does check for the presence of a µSD card to boost, a much better way than the first patch, so if you don’t have a µSD card installed, it should not boost.
I think this one is ready to commit if we don’t see any problems.

Latest patch is working fine here.

No problems reading, writing, deleting, inserting, removing or playback on µSD card. No display anomalies.

Seems to work fine on Fuze (after modifying config-fuze.h)

I saw a freeze when removing the µSD while buffering, however I didn’t test if the same freeze happen without this patch.

Removing µSD when idle or when audio has been buffered shows no problem

EDIT: I see you unboost unconditionally and this might be a problem since the counter could become negative

That figures, I actually unboosted unconditionally to avoid the problem of someone removing the µSD during the disk access and ending up stuck at boosted… But it seems you got a different problem.
The way i read cpu_boost(bool on_off) in system.c there is a safety check to reset the boost counter to 0 if it goes negative.

I will modify the config files and post another patch shortly.

Ah right I see the safety check, but you could accidentally cancel boosting when another thread needs it?

It should be possible to check for boosting state during SD removal with not much trouble

Ok, This one also unboosts if there is a µSD present.

I’ve edited the config files for e200v2, fuze, clip, and c200v2 to activate voltage scaling.

I tested removing the card while buffering and got lockups with both current svn and this patch so I am pretty sure this patch does not impact that issue.

you just forgot the m200v4 ;)

If all as3525 models use this, then I think this should be hardcoded (EDIT: I mean remove the ADJUST_CPU_VOLTAGE define)

The patch doesn’t deal with µSD removal when an operation is in progress, but since this locks up the Sansa anyway I think you should just commit when you feel it’s alright.

- Remove HAVE_ADJUSTABLE_CPU_VOLTAGE define and enable for all Sansa AMS
- Make CPU boosting depend on HAVE_MULTIDRIVE (unneeded for internal storage)
- Keep track of boosting status inside sd_enable() so we don’t need to check for card removal

EDIT: tested on Fuze & Clip , battery_bench running on both

I don’t think you need the #ifdef HAVE_MULTIDRIVE unless the intent is to leave out that code on the clip. card_detect_target() should always return false for the clip.
cpu_boosted seems to take care of the card removal problem for us just fine….

I think it makes the code clearer to read.

But perhaps it should be conditional on HOTSWAP.

By the way the various
#if defined(SANSA_E200V2) || defined(SANSA_FUZE) || defined(SANSA_C200V2)
should be removed since they always are enclosed on MULTIDRIVE or HOTSWAP.

And I think most if not all the MULTIDRIVE should be changed to HOTSWAP (which means hotswappable µSD cards)
Because MULTIDRIVE could be true when using a ramdisk for example

Sync + use HAVE_HOTSWAP

My comments were addressed in r23180

This runs fine and looks good to me(after I found amiconn’s comments on irc). Do you want to wait on the battery benches before committing?

The fuze bench just finished : 12h30 (I don’t remember what SVN gives).
The clip freezed at playback though

No need to wait, please commit

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing