Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Bugs
  • Category FM Tuner
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by fragilematter - 2010-03-30
Last edited by funman - 2010-03-30

FS#11152 - FM Tuner isn't stopped during at init

radio_stop() doesn’t actually do anything when it’s called by radio_init(), because of faulty variable initialization,

Here’s the rundown:
at the beginning of apps/recorder/radio.c, radio_status is declared and initialized with FMRADIO_OFF. When rockbox starts up, radio_init() is called, which in turn calls tuner_init() [to take care of the hardware bits] and radio_stop() [to power off the radio].
However, radio_stop() checks checks if radio_status == FMRADIO_OFF, and if it is the procedure exits without doing anything.

Solution:
initialize radio_status with FMRADIO_PLAYING.
Then radio_stop() will work as intended, and in the process it will reset radio_status to FMRADIO_OFF.

Tested and working on an iRiver H10 and a Sansa E200v2. Also, the fm radio regs on the iRiver show that proper initialization is occuring.

Closed by  funman
2010-03-30 19:21
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

r25403

in the same file, radio_start() has:

  if(radio_status == FMRADIO_PLAYING)
      return;

….

  if(radio_status == FMRADIO_OFF)
      tuner_power(true);

So you’re going to skip tuner_power(), right ?

Both h10 & e200v2 do not have tuner_power() (it’s present but has no effect)

I think I had tried this same patch when looking at disabling clipv1 FM and went another way because of tuner_power() : I put the chip in the disabled state in tuner_init() instead.

No, it’s about the fact that, with the current init sequence (and with radio_stop() essentially skipped), there is a chance that the tuner will be left in an inconsistent state at start.
On the H10, if you enable “Force Mono” for FM, the tuner will power-up with only the mute bit set and it will add noise to the audio path, even during playback of music. That state can only be reset by listening to the radio, thus ensuring that the radio will be muted afterwards.

I just wanted to say that the patch is incomplete, it will not run tuner_power() on targets where it’s needed

If I understand it correctly, radio_init() is only to be called during rockbox start-up. On the other hand, radio_start() is run only when the user accesses the radio.
tuner_power() is called in the section of radio_stop() that gets executed if radio_status == FMRADIO_OFF fails the comparison.

What am I missing?

I don’t know much the code, but tuner_power(true) at the start of radio_init() + your patch might be enough?

Well, unless radio_init() is called in some other place other than when the player boots, it should be ok. Tests on other devices are encouraged, though :D

It’s only called once (as INIT_ATTR suggests)

So we’d just need someone with a device that has a tuner power to test.

Do you agree with my previous comment : add tuner_power(true) at the start of radio_init() ?

radio_stop() already has a tuner_power(false) command.
Isn’t the tuner supposed to stay powered off if possible, until the user actually accesses the radio function? So radio_init() turns off the radio from the moment the device boots to the moment when someone starts the radio. Then radio_start() will call tuner_power(true) and everything should work out.

[boot] radio_status = FMRADIO_PLAYING

         tuner_init() -> radio_stop() -> radio_status = FMRADIO_OFF && tuner_power(false);

[user wants radio] radio_start() → tuner_power(true) && radio_status = FMRADIO_PLAYING;

I still don’t get how tuner_init() can work without a tuner_power(true) before it, but that’s unrelated to the patch.

I tried your patch on Clipv1 and it’s OK.

Can you add a comment next to radio_status to explain why you set it to playing ?

At that point when rockbox has just started up, the tuner is in whatever state the physical power-up sequence puts it in.
With the variable being set the way it was previously, rockbox only initialized the tuner and left it that way (without making sure that it’s muted, powered off or in whatever state rockbox would want it to be), unless the code to do that was in tuner_init().
Now tuner_init() gets all the hardware bits in the state that they should be in for the tuner to work and, by initializing radio_status with FMRADIO_PLAYING, we also make sure that radio_stop() mutes the tuner or, for the targets that support it, powers it off,

I stumbled upon this when I enabled force mono for fm. When rockbox started, it only loaded the setting and set the apropriate bit of the tuner (bit 4 of byte 3 set to 1 for the TEA5767), and left all the other regs to 0×0. In most of the targets this is just fine and the radio remains silent, even for others that have the TEA5767 apparently. But on (my) H10, this woke up the tuner and, since it wasn’t muted, allowed it to output noise over the player’s audio.
By explicitly passing through the whole radio_stop() function, the PLLs are initialized and, more importantly, the tuner is muted.
For other targets that support power, this also means that the tuner is formally powered off, without extra code to make that happen in the various targets’ tuner_init().

Commented patch added.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing