Rockbox

i’m curious : have you tried this in combination with  FS#10048  ?

No, I have not had a chance to try  FS#10048  at all up to this point.

First, I think a good thing would be to use some kind of mechanism to ensure that the delays used in the fmradio i2c driver are always about the same length. This can be done with a timer but unfortunately both timers in the as3525 are already used (TIMER1 as a general timer and TIMER2 to call the tick tasks). How about using the watchdog timer as a delay timer for delays on the order of a microsecond? This would of course mean that we lose the normal functionality of the watchdog timer, but I think we’re not using it now anyway.

Second, I’d like to simplify the “generic i2c” driver. We adapted the “generic i2c” driver for the ams targets, we are currently the only users of that driver but it’s not doing exactly what the OF does. In my opinion this driver can be made much simpler, for example scl_hi and scl_lo can be merged into a single scl_set function, scl_input and scl_output can be merged into a single scl_dir function, also the 6 different kind of delays can be merged into a single delay that takes an argument (e.g. number of us to wait, or number of half-bit units to wait)

The watchdog peripheral is already used for resetting the devices.

How about writing a usleep() function using TIMER2_VALUE register, since this timer is aways running ?

I believe that would also make FM radio work in  FS#10048  as well.

Huh? As far as I can see TIMER2 is in periodic mode (as opposed to free running mode) so it gets reset regularly. The watchdog is not used, except just before reboot. Anyway I noticed that the watchdog runs at 1/256 of PCLK. so max. resolution is only 4 us.

We know the timer frequency, and we also know to which value it is reset (since there is only one call to tick_start(1000/HZ) in kernel.c), so we can detect timer wraps.

Is 4us (250kHz) too large, do we need 400kHz ?

If your radio didn’t work, can you try this patch?
A look in the disassembly and a quick calculation shows that the delay is probably already long enough already (6 instructions per loop).
The most important fix is an extra delay in i2c_start that seemed to be missing. Also it removes some things that seem to be unnecessary.

   fmradio_i2c_fix.patch (0.8 KiB)

tested this patch with r21151 svn

Still hangs when going to the FM Radio screen. Changing the fm_delay() still seems to correct the problem.

I committed the most important part of my previous patch (there seemed to be an actual bug in i2c_start). What kind of delay do you need roughly to make radio work (while boosted)?

I looked up the datasheet of the radio chip and it seems to support 400 kHz i2c. I’m a bit surprised that apparently a larger delay is needed, maybe there’s another bug in the generic i2c driver somewhere. A dissassembly of the current code in SVN shows 6 cycles for the delay loop, so even when no pipeline stalls occur due to branching, the default delay of 100 loops should take at least 2.4 us at 248 MHz CPU clock, which should be more than enough.

With svn, I had to increase the number of loops to 400 to get it to work (399 still failed). The patch increased to 500 just to be safe.

Also, the failure is not a deadlock, as the backlight still goes off at its normal timeout and a keypress will turn it back on. The dap will turn off as normal with the power button.

I have also noticed that if I made the delay is too long, it will hang with the logo on the screen and never get to the main menu. I guess that the problem here is with the test for the FM Radio hardware on startup.

Using patches from  FS#10048 , I have noticed that the radio will sometimes work with the additional delay and sometimes not. Using this patch: http://www.rockbox.org/tracker/task/10048#comment30627 I was not able to get the radio working with any delay setting. With the previous patch, the radio worked with the 500 loop delay.

Another go at some generic_i2c fixes. I looked at the i2c spec and the OF code again and it seems some more delays are missing. This patches adds those delays. Can you try this patch without increasing fm_delay?

On my clip, with  FS#10048 , with fm_delay actually intentionally lowered down to 20 cycles and with CPU boosted, this still works.

   generic_i2c_fix.patch (1.3 KiB)

tested with r21192

Still hangs before entering the FM Radio screen. Increasing fm_delay() loops allows proper operation.

Hm, still weird. I’m surprised such a big delay is needed while it seems to work with just 20 cycles on my clip. This makes me think there’s something else going on. Ideas:
* perhaps i2c bus method selection fails in some way (the fm chip can use both i2c and some other 3-wire protocol). This “just worked” so far and has not really been looked into, but maybe it’s giving trouble now at higher CPU speeds.
* perhaps there’s still some bug in the generic_i2c driver. I think there’s a tiny time window where a short circuit can exist on the SDA line between the fm radio and the as3525 controller: just after sending a byte to the fmradio, we expect an ACK bit back from the fmradio but the SDA line is still in output mode. However the OF seems to do it like this as well.
* perhaps the firmware version of the fmradio chip has an influence on behaviour. The si4702 comes in two firmware versions, 16 and 19. Can you check the chip ids (first two values in the fmradio debug screen)? The radio has to be enabled for this debug screen to work. My clip shows 0×1242 0×1053 indicating an si4702 with firmware version 19.

- “Can you check the chip ids (first two values in the fmradio debug screen)”

Uhm… fmradio debug screen shows only zeros on my e250v2 (see screendump)

   dump 090606-113327.bmp (75.7 KiB)

Can you have a look in the fmradio debug screen, while the radio is playing?

I can’t get the radio to work… even with this patch (one of them or both).

Chip ids on my e260v2 are 0×1242 0×0850

That means Chip version 4, si 4702, Firmware version 16
and bertrik’s Clip is version 8, si 4702, Firmware version 19

The datasheet I have is for firmware version 19, we should get a copy of version 16 and see if there is any differences.

I have the datasheet for the si4700/4701 revision B, firmware version 15. The bit mapping for the REV, DEV and FIRMWARE fields is a bit different for the 4700/4701, but it does mention 0x4XX and 0x8XX as possible values (XX = firmware version). So this could be an older chip that behaves a little differently.

I have radio working on my e280v2 with svn patched with  FS#10048 . The only changes I made were to run in fastbus only mode at 62 MHz boosted and 31MHz unboosted. I will try to test higher frequencies and find the failure point. I have the same chip ids as mc2739.

I don’t know if this will help but I tried several higher FCLK frequencies and found that at FCLK 124MHz the radio is still functional and it is not functional at the next lowest FCLK I could get to boot which was 186 MHz.

For me (e260v2), the radio stops working at vanilla SVN r21088 - the otherwise excellent FlynDice clocking rationalisation patch ;)
Changing fm_delay to 500 does not help.

However changing DBOP clock to:

#define AS3525_DBOP_FREQ         (AS3525_PCLK_FREQ/2)

in an otherwise vanilla r21088 gets the radio going.

This comes from experimenting, not from any knowledge of how or why DBOP clock affects the FM chip.

I have not yet tried the generic_i2c.c patch.

FTR the chip ID is 0×1242 0×0850

The same change to DBOP clock also gets the radio working on r21184+FS10048+FS10284.
The UI seems reasonably responsive with this setup - test_fps says 44.7 fps unboosted, spacerocks is playable.

Weird…

I looked a bit at the fuze and e200v2 display driver and noticed that the fuze display driver configures GPIO A7 as output and sets it low, this pin is also used for FM radio I2C. The e200v2 display driver doesn’t do this. I hope there isn’t really any dual-use for this pin so this could simply be a bug, but maybe it has a side-effect of making the FM radio work. Can someone with a fuze try to remove the GPIO A7 stuff (both direction setting and level setting) from the fuze lcd driver and see if this causes the radio to stop working?

I also looked a bit closer at the e200v2 OF. It actually uses a really tiny delay, just 3 loops of 4 instructions. One thing in the e200v2 OF that I didn’t find in the Clip OF is that the SDA and SCL I2C lines are both set to 1 for some time before sending an I2C start condition.

Radio (and LCD) still works on the Fuze, with this modification.

Attached are a couple of patches to experiment with, can you apply each and see if it works? Please use a clean tree from current SVN before applying each patch.
fix1: increase si4700 crystal oscillator startup delay
fix2: wait some time after setting i2c SCL signal as output
fix3: more extensive i2c initialisation

   e200_radio_fix1.patch (0.4 KiB)
   e200_radio_fix2.patch (0.3 KiB)
   e200_radio_fix3.patch (0.4 KiB)

Tested against r21245 clean tree

fix1: radio does not work, screen cleared except for backdrop and statusbar, not deadlocked (backlight goes off, keypress turns backlight on)
fix2: hangs at Rockbox logo, not deadlocked (backlight goes off, keypress turns backlight on)
fix3: radio does not work, screen cleared except for backdrop and statusbar, not deadlocked (backlight goes off, keypress turns backlight on)

Hmm, too bad. Attached is another attempt. This time a small modification in the generic i2c driver, can you try this?

   e200_radio_fix4.patch (0.6 KiB)

Fix4 on r21245 - radio does not work, screen cleared except for backdrop and statusbar, not deadlocked (backlight goes off, keypress turns backlight on)
I assume the patches are intended to be applied separately, not all at once to the same source tree?

Another experiment: the following patch works around a while-loop in the si4700 driver that may be hanging, can you try if this works better? The radio may not get tuned correctly but there’s at least a chance to see the actual radio screen and it might gives us more clues.

   e200_radio_fix5.patch (0.7 KiB)

Tried the patch. Now it showed at least the normal radio screen. The handling (channel,volume e.g.) seems normal but there is no sound.
Like you suspected it was waiting in the loop for the ready signal and it never arrives.

Ah, now we’re getting somewhere, can you get a screenshot/screendump of the FM radio debug screen after visiting the FM radio screen?

Tested against r21253 clean tree

Same results as jago

Attached screenshot of FM radio debug screen

   dump 090611-175017.bmp (75.7 KiB)

For comparison, I thought you might also like to have a screenshot of the FM radio debug screen from a working build. This is from r21171 with the increased fm_delay()

   dump 090611-193651.bmp (75.7 KiB)

Thanks, those screendumps are very useful. It looks like the i2c communication with the radio chip is actually fine. The problem seems to be that the fm radio chip hasn’t fully or properly been powered up. For example the lower part of the chip id register (second number) is different, which is consistent with what the datasheet says.

Perhaps it simply needs more time than expected to power up. Can you try attached patch?

   si4700_enable_delay.patch (0.5 KiB)

Unfortunately the patch has no effect. Testet against r21246.

Can you try the combination of e200_radio_fix1.patch, e200_radio_fix5.patch and si4700_enable_delay.patch?

Tested with r21268

Radio still not working with e200_radio_fix1.patch, e200_radio_fix5.patch and si4700_enable_delay.patch

the c200v2 seems to have the same problem (using si4700 as well)

The problem with the e200v2 radio turned out to be a problem with the initialisation sequence of the Si4702 with firmware version 16, that appears to be slightly different from Si4702 radios with firmware version 19.