FS#12094 - RDA5802 tuner: clean up register 0x4 handling, drop implicitly enabled soft-mute

Attached to Project: Rockbox
Opened by Stephan Grossklass (keyb_gr) - Tuesday, 03 May 2011, 20:51 GMT
Last edited by Bertrik Sikken (bertrik) - Saturday, 04 June 2011, 20:18 GMT
Task Type Patches
Category FM Tuner
Status Closed
Assigned To No-one
Operating System Sansa AMSv2
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


The attached patch does two things:

1. The initial value of register 0x4 is changed to make sense according to the RDA5802E datasheet. Originally, it sets bit 10 (value 0x0400), which is undocumented and does not seem to have any effect. However, I believe that the original intent was to set bit 11, which switches deemphasis to 50 µs. This would be in line with the other initial settings for frequency range (87.5 .. 108 MHz) and tuning steps (100 kHz), typical for Europe etc. (which is 50 µs territory). Therefore, the new value of of this register is 0x0800.

2. There was a line in the muting functionality which directly set register 0x4 bits 9 and 10, effectively causing them to be always enabled. (This is presumed to predate the availability of a datasheet and might even be a remnant from the Si4702 driver.) Bit 9 controls the "soft-mute" feature (volume reduction on weak stations, not associated with regular muting), which is enabled by setting it. Therefore, this was always on, without being set explicitly. The line in question is removed.

At this point it is unclear to me whether we do or do not want soft-muting (or whether it should be user-selectable even) - this needs further discussion. If we do, it should be enabled explicitly, either via the initial value of register 0x4 bit 9 or using the SYSCONFIG1_SMUTE define already present.

Two versions of this patch (against r29814) are attached:

* The standard version (std) contains the changes as outlined above.
* The other one additionally increases LNA working current from 2.5 mA to its maximum of 3.0 mA for maximum sensitivity. This is done by setting register 0x5 bits 5:4 from 10 to 11. More testing is needed to determine whether this really has the desired effect of improving reception on weak stations (it ought to give a few dB more signal, and I do think RSSI readings have increased a bit) or whether SNR does not improve enough to be worth the additional current draw.

Practical effects of this patch:
Soft-mute function is disabled, so volume levels on weak stations will remain constant and noise is heard in all its glory.
Battery life during radio operation is likely to be somewhat reduced on the "more gain" version, as LNA operating current increases by 0.5 mA. The effect on frontend dynamic range is unclear (the chip does seem to have LNA AGC, so nothing bad should happen).
This task depends upon

Closed by  Bertrik Sikken (bertrik)
Saturday, 04 June 2011, 20:18 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed as SVN r29959
Comment by Bertrik Sikken (bertrik) - Thursday, 05 May 2011, 11:46 GMT
I'm fine with changing the initial values, but I expect the regional settings (channel spacing and de-emphasis) to be set anyway by the higher level radio code.

I consider soft-mute a feature, so I would like to have this enabled by default. I don't think this needs to be user-configurable. If we do that, we might need to do it for other tuners as well.
Do you have any indication that we can at least select a specific RSSI range where soft-mute does its thing (we do have control over this in the si470x range of tuners, but I don't see it in the RDA5802 datasheet).

I personally don't mind a current increase of 0.5 mA for the LNA if this improves reception. The total radio current appears to be about 21 mA (from the datasheet), so 0.5 mA is only a small fraction of total current.
Comment by Stephan Grossklass (keyb_gr) - Friday, 13 May 2011, 21:06 GMT
Nope, the RDA5802 seems a good bit more basic here. With the audio frequency response also being off, the programming may have been a bit of a rush job anyway. Not entirely unexpected for a Chinese company.

OK, here are two new diffs with soft-mute back on.

It's true, half a mA shouldn't make too much of a difference. Radio is a power hog in any case, at least relatively speaking and on the Clip+. Seems silly when that same chip could run for like 100 hours on two AA cells, but that's a 3.7 V 200 mAh battery for ya. That makes the 15 hour runtime during MP3 playback all the more respectable, though of course the SoC "cheats" by using DC/DC converters and lower internal supplies. Can't do that on a radio IC.
Comment by Bertrik Sikken (bertrik) - Thursday, 02 June 2011, 20:48 GMT
Stephan, what do you think about this as a cleanup?
It removes unnecessary init for read-only registers, uses sane values for registers that we know, and the defaults from the sandisk OF for the undocumented registers.
It also removes the extra changes during RADIO_MUTE.

A bit of experimentation showed me that undocumented register 0x07 (I've called this SYSCONFIG4) seems to affect the stereo blend, in particular bit 14.

On top of this, we can still increase the LNA current a bit of course.
Comment by Bertrik Sikken (bertrik) - Saturday, 04 June 2011, 10:02 GMT
Oops, I forgot the softmute bit in my most recent patch, but I think you get the idea.
Comment by Stephan Grossklass (keyb_gr) - Saturday, 04 June 2011, 13:32 GMT
That kind of more explicit handling of reg values is better.

You init cache[16] with 8 values, which is all we ever need, but it doesn't exactly strike me as textbook level. Anyway...

Did some value restoration in SYSCONFIG1/2/3, some stuff had gotten lost there. I don't think it makes sense to have seek threshold at minimum, and we better stay in I2S slave mode. Soft muting is now enabled.
Comment by Bertrik Sikken (bertrik) - Saturday, 04 June 2011, 14:48 GMT
As far as I understand, we don't use the seek threshold, it is only used for automatic tuning (which is not used in rockbox now). Also the i2s slave mode shouldn't matter because i2s is disabled anyway.