FS#6419 - RTC Mod for H1x0 series

Attached to Project: Rockbox
Opened by Robert Kukla (roolku) - Thursday, 07 December 2006, 16:25 GMT
Task Type Patches
Category Drivers
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


This is the patch for the RTC Mod for H1x0 series using an DS1339c as described here:

The main part of it is concerned with reviewing and separating out the i2c driver from the eeprom driver so it can be shared between the eeprom and the new rtc driver (which is rather trivial). It also contains a fix for two bugs concerning the multi byte read (which is not used in the eeprom driver so wasn't a problem yet). The i2c-bitbang.c should probably go into the h120 target tree, but peturs aliasing of the functions for h300 makes this a little awkward.

The rtc functionality can be disabled by commenting out the following line in config-h120.h

#define CONFIG_RTC RTC_DS1339C

This task depends upon

Closed by  Robert Kukla (roolku)
Wednesday, 28 February 2007, 13:42 GMT
Reason for closing:  Accepted
Comment by Robert Kukla (roolku) - Saturday, 09 December 2006, 02:15 GMT
The patch has been updated to include the drivers for the wake-up alarm (see wiki). Currently only the functionality of the Archos alarm mod is implemented, although the ds1339c offers more options and a second alarm.

The following two lines need to be uncommented in config-h120.h
#define CONFIG_RTC RTC_DS1339C

Remember to update the software before you install the mod - otherwise you will get caught out with the play button constantly pressed (the patched rockbox will disable it at startup).
Comment by Gianluca (Datman) - Monday, 11 December 2006, 16:12 GMT
Very useful! H120 needs a RTC.
I have had a fight against the compiler (and he won... :( !!!).
I haven't a lot of free time, then I hope I'll find the patch in a next daily build, so that I'll go to the hardware.

Gianluca "Datman"
Comment by Peter Olson (Peter200lx) - Wednesday, 03 January 2007, 03:16 GMT
It worked a couple of builds ago, but fails to compile against the most recent CVS build (2007-01-02 15:48) when the config-h120.h lines are uncommented. Here are the error messages that it gives:

LD rockbox.elf
/home/user/rockbox-devel/build_h120/apps/alarm_menu.o: In function `alarm_screen':
alarm_menu.c:(.text+0x16): undefined reference to `rtc_get_alarm'
alarm_menu.c:(.text+0x3c): undefined reference to `rtc_enable_alarm'
alarm_menu.c:(.text+0x1f2): undefined reference to `rtc_init'
alarm_menu.c:(.text+0x200): undefined reference to `rtc_set_alarm'
alarm_menu.c:(.text+0x20a): undefined reference to `rtc_enable_alarm'
alarm_menu.c:(.text+0x2d8): undefined reference to `rtc_enable_alarm'
/home/user/rockbox-devel/build_h120/apps/main.o: In function `init':
main.c:(.text+0x20e): undefined reference to `rtc_init'
/home/user/rockbox-devel/build_h120/apps/tree.o: In function `start_resume':
tree.c:(.text+0x5f2): undefined reference to `rtc_check_alarm_started'
tree.c:(.text+0x600): undefined reference to `rtc_enable_alarm'
/home/user/rockbox-devel/build_h120/librockbox.a(powermgmt.o): In function `power_thread_rtc_process':
powermgmt.c:(.text+0x536): undefined reference to `rtc_check_alarm_flag'
powermgmt.c:(.text+0x542): undefined reference to `rtc_enable_alarm'
/home/user/rockbox-devel/build_h120/librockbox.a(timefuncs.o): In function `get_time':
timefuncs.c:(.text+0xb2): undefined reference to `rtc_read_datetime'
/home/user/rockbox-devel/build_h120/librockbox.a(timefuncs.o): In function `set_time':
timefuncs.c:(.text+0x260): undefined reference to `rtc_write_datetime'
collect2: ld returned 1 exit status
make[1]: *** [/home/user/rockbox-devel/build_h120/apps/rockbox.elf] Error 1
make: *** [all] Error 2

Comment by Robert Kukla (roolku) - Thursday, 04 January 2007, 01:41 GMT
The addition of the rtc driver for the gigabeat broke the patch. Now fixed.
Comment by Robert Kukla (roolku) - Tuesday, 06 February 2007, 15:45 GMT
First attempt in adding auto detection for the MOD. It works as follows:

- during rtc_init() the global variable rtc_detected is set to true if the RTC mod is detected
- if CONFIG_RTC == RTC_DS1399_DS3231 an rtc_detected check is done before performing RTC related task such as
+ create entries for "set date/time" and "set alarm" menus
+ display time in status bar
+ evaluate WPS %C tags
+ filename generation
+ scrobbler entries
+ FAT time generation
+ time functions

I went for low impact on existing code. Some bits (i.e. fat.c could do with some re-structuring.)

Please test (especially on units without the MOD) and let me know of any problems.

(The patch conflicts with the system_settings_menu change of the REP - you can find a build that has been adjusted here: )
Comment by Robert Kukla (roolku) - Tuesday, 06 February 2007, 19:50 GMT
Sorry I got the link wrong. The version with MOD auto detection is:
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 13 February 2007, 07:42 GMT
I have reviewed the patch and here are some comments:

1) You don't respect the indentation. Please do.

2) I think the i2c-bitbang.c file should be in the target/coldfire/iriver tree.

3) I think it would be nicer if rtc_detected would be a function, but it's not a big deal.
Comment by Robert Kukla (roolku) - Tuesday, 13 February 2007, 09:14 GMT
Thank you very much for checking it out.

1) I have left the original indentation in place intentionally so the diff would only show the real changes and not the added white space, but I will adjust it

2) I will move it. Shall I place eeprom_24cxx.c there as well?

3) I had it like that originally (as this is what the radio detection does), but could not see a good reason for justifying the additional function call overhead to check a boolean var.

Hopefully I will get it done today.
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 13 February 2007, 11:05 GMT
1) Well, what I meant was that your added code was not aligned with the existing code. Example:

case 'c': /* Real Time Clock display */
+#if CONFIG_RTC == RTC_DS1339_DS3231
+ if(!rtc_detected)
+ return NULL;
+ else

2) Not sure. If it is fairly generic, I think it could stay in the firmware/drivers tree.

3) Don't bother.
Comment by Robert Kukla (roolku) - Tuesday, 13 February 2007, 22:39 GMT
Okay I have:

- fixed indentation
- moved the i2c-bitbang.c file to target/coldfire/iriver
- adapted the menu to JdGordons' new menu system (I think it handles dynamic menus a lot less elegantly than previously though)
- made the #include rtc.h conditional on CONFIG_RTC to be consistent with radio.h
- made sure the h120 sim works

I have compiled it for h120, h120 sim and gigabeat and run it on a modified and an unmodified h120.
Comment by Timo Horstschäfer (x1jmp) - Tuesday, 20 February 2007, 19:13 GMT
Synced to current svn.
Comment by Robert Kukla (roolku) - Tuesday, 20 February 2007, 21:19 GMT
Updated to current svn, a few fixes and some restructuring in the logic of fat.c. Please test (especially with non-rtc units).
Comment by Robert Kukla (roolku) - Wednesday, 21 February 2007, 23:45 GMT
- unconditionally disable square wave output on every boot - prevents problems with ALARM MOD on first start up for DS1339; DS3231 should be unaffected
- not tested under 'real' conditions
Comment by Robert Kukla (roolku) - Wednesday, 28 February 2007, 12:06 GMT
This is the RC which I will commit after a little more testing. I tidied up the split between the i2c for H100 and h300 and some other minor changes.