Rockbox

Tasklist

FS#7814 - Enable H300, X5 and M5 RTC Alarm

Attached to Project: Rockbox
Opened by Alexander Spyridakis (xaviergr) - Saturday, 22 September 2007, 02:03 GMT
Last edited by Alexander Spyridakis (xaviergr) - Saturday, 22 September 2007, 13:24 GMT
Task Type Patches
Category Drivers
Status New
Assigned To No-one
Operating System Coldfire-based
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 1
Private No

Details

This small patch enables the RTC Alarm for the H300 iriver players (and eventually X5 and M5).
It will enable the unit to turn on automatically on the time that was set, and enable the Wakeup Screen menu.

I was surprised to see that almost the same code was located on rtc_pcf50605.c and no one tried to enable it on pcf50606 too.
This task depends upon

Comment by Alexander Spyridakis (xaviergr) - Saturday, 22 September 2007, 10:07 GMT
I just remembered that the official bootloader doesn't have the alarm code needed for the unit to autopower on.
So until a new bootloader (revision 12546 and later) is flashed this patch is useless. :(

P.S: Keep in mind that recent svn bootloader revisions have a bug currently that will not let you boot into rockbox.
Comment by Pascal Briehl (ColdSphinX) - Saturday, 22 September 2007, 11:08 GMT
That patch schould also enable it for X5/M5 targets.
Comment by Pascal Briehl (ColdSphinX) - Saturday, 22 September 2007, 12:02 GMT
Now tested, it works on my X5 with default bootloader.
Hug on OF remnants.
Comment by Alexander Spyridakis (xaviergr) - Saturday, 22 September 2007, 13:07 GMT
Patch updated to enable alarm on X5.
Thanks I didn't notice that X5 used the same chip.
Comment by Alexander Spyridakis (xaviergr) - Saturday, 22 September 2007, 13:17 GMT
Sorry again, it seems that the patch works on M5 too.
I hope that I didn't forget another target.
Comment by Victor Rajewski (vik) - Sunday, 23 September 2007, 04:22 GMT
Works on my H300 with bootloader h12547.

Now I'm going to try to write a plugin to slowly ramp up the volume while playing, and this will be the perfect wake-up device
Comment by Thom Johansen (preglow) - Thursday, 27 September 2007, 19:50 GMT
Why duplicate all the code? Wouldn't it be better to unify the code into one file somehow?
Comment by Alexander Spyridakis (xaviergr) - Friday, 28 September 2007, 23:59 GMT
Thom, probably you are right, but I have to check for some parts of the code that maybe need some slight changes.
Anyway, all rtc code will eventually change when date is incorporated as a trigger on the rtc alarms.
Comment by Pascal Briehl (ColdSphinX) - Saturday, 29 September 2007, 14:55 GMT
After using the patch my year had always been reseted to "2035", there are my changes I did to fix the bug.
Comment by Pascal Briehl (ColdSphinX) - Saturday, 29 September 2007, 14:59 GMT
ups, wrong file
Comment by Pascal Briehl (ColdSphinX) - Saturday, 29 September 2007, 15:01 GMT
I'm going crazy -_-
Comment by Rani Hod (RaeNye) - Saturday, 29 September 2007, 16:19 GMT
Note: the dual-bootloader ( FS#5289 ) needs a small fix to support RTC.
Comment by Pascal Briehl (ColdSphinX) - Saturday, 29 September 2007, 16:58 GMT
hmm, i'm using an old dualbootloader and also testet the official bootloader and both recognized the alarm and woke rockbox up on my x5.
so I thought that a part of the hard implemented OF (like charging and usb, when powered off) sends the wakeup singnal.
Comment by Rani Hod (RaeNye) - Sunday, 30 September 2007, 23:29 GMT
The preloader (what you call "hard" OF) sets a byte in memory to notify OF that RTC alarm has occurred;
The dual bootloader checks this byte and should boot OF if it is set.
No idea why it works....
Comment by Steve Bavin (pondlife) - Tuesday, 16 October 2007, 16:11 GMT
This seems to work fine on H300, with an SVN bootloader, any idea what's stopping it being committed? Just the bootloader release?
Comment by Alexander Spyridakis (xaviergr) - Tuesday, 16 October 2007, 18:04 GMT
I think yes. (at least for H300)

But as preglow said the code might need to be merged with pcf50605 instead of copying it, though I want to make sure for some bits on the alarm setting that might not be needed on other targets.
Comment by Steve Bavin (pondlife) - Thursday, 01 November 2007, 09:00 GMT
Hmm, I've noticed that when using this patch, occasionally my H340 internal clock is unset (as in Rockbox displays a time of --:--). When I go into the time/date setting screen, the time is displayed as 00:00:00, but the date is correct.

I'm not sure whet triggers this behaviour yet, but I'm certain it only happens with this patch. Will carry on diagnosing...
Comment by Victor Rajewski (vik) - Thursday, 01 November 2007, 22:20 GMT
Steve: I get a similar thing with the clock (--:--) (heh, that looks a bit like a bum emoticon) but only for a few seconds from time to time; then the clock seems to get the correct time back. I'm not certain if this is related to this patch or not (I do have it applied, but have others too). I'm also on an H340.
Comment by Steve Bavin (pondlife) - Wednesday, 23 April 2008, 05:33 GMT
Further to my previous comment ("the date is correct"), Llorean informs me that if the date is unset then the build date is displayed as a sensible starting point, so the date was very likely lost too.
Comment by PaulJam (PaulJam) - Thursday, 05 June 2008, 15:49 GMT
Hi,
when i tried the patch on my h300 i experienced the same behavior like pondlife.
When the time display disappears and you open the clock plugin then the correct time is still shown, the date is also correct except for the date which is shown as 1999.
So the clock doesn't get completely unset, but only the date is wrong. The reason that the time disappears in the statusbar and WPS is that the function valid_time() (which returns false for years earlier than 2000) is called before displaying the time (the clock plugin doesn't do this check)

The reason that this happens seems to be that the functions pcf50606_read* and pcf50606_write* without using disable_irq_save() before and restore_irq() afterwards. This seems to be necessary on H300 in order to avoid conflicts with the button driver.
See here: http://www.rockbox.org/irc/log-20080605#12:54:12

When i use disable_irq_save() and restore_irq() similarly like it is done in rtc_read_datetime() (in \firmware\drivers\rtc\rtc_pcf50606.c) then the clock doesn't disappear anymore.
Comment by Steve Bavin (pondlife) - Thursday, 05 June 2008, 16:18 GMT
Does this work, then? Sorry, my H300 is not with me at the moment...
Comment by Steve Bavin (pondlife) - Thursday, 05 June 2008, 17:09 GMT
Take 3, much better, I hope!
Comment by Steve Bavin (pondlife) - Wednesday, 11 June 2008, 16:12 GMT
I've been using this with no problems on my H300 for the past week, and would like to commit it. Are there any X5 or M5 owners out there who have tested it? If not, I think I'll commit it for the H300 but leave them out for now.
Comment by Stephen Carroll (Stephenc82) - Monday, 16 June 2008, 15:40 GMT
I have used this patch on the X5 with no problems and wish it to be commited for the x5 aswell please.
Comment by tomas (hakimio) - Monday, 16 June 2008, 16:05 GMT
Working well in iAudio X5 here too.
Comment by Steve Bavin (pondlife) - Monday, 16 June 2008, 18:15 GMT
Any M5 owners out there? Or is it similar enough to the X5?
Comment by Lee Kang Hyuk (alwaysbluepop) - Tuesday, 17 June 2008, 14:21 GMT
Latest Iaudio Bootloader(v4) blocks the wake-up during autopower on. Alarm check code is needed for x5(and probably for m5) to boot into Rockbox.
Except the bootloader it worked fine enough.
Comment by Marianne Arnold (pixelma) - Wednesday, 18 June 2008, 13:05 GMT
Yes, I see the same on M5 too, that the alarm is triggered but the v4 bootloader prevents a normal power up as already said in IRC (there already was an explanation why).

And I also saw 2 small problems: the manuals wouldn't build correctly for the 3 targets with this patch and a button action conflict in the alarm settings screen on the M5/X5 pad that wouldn't let me increase the hour by pressing "right" - which would only accept the alarm setting and leave the screen. I had a look at alarm_menu.c and the button actions and contexts that's used there and could solve the problem (I wonder whether it actually shows somewhere else currently even without the patch). As it is a button action conflict it also shows in an X5 or M5 sim in case someone wants to try.

Attached are 2 patches (1 fix for each problem). I also tested whether the X5 keymap change (the same file is used for the M5 too) needs all 3 added actions in the settings context but it seems it's really needed to have the full controls. The manual patch was needed because the alarm settings section is automatically included through the features.tex (parsed from the lang's feature.txt) and this section uses a macro for the buttons which was simply missing in the platform files, so I added it there. Both patches are independent from the main alarm patch here because they touch different files, of course it's somehow related.
Comment by Rani Hod (RaeNye) - Wednesday, 18 June 2008, 21:30 GMT
Most of the work needed by the bootloader to correctly identify RTC alarm is already done by the preloader, which sets the byte in 0x30000000 to 1 if the device woke up due to RTC alarm. We just need not to overwrite this byte until we decide whether to boot or to shut down.
Comment by Raimo Radczewski (reemo) - Wednesday, 02 July 2008, 04:51 GMT
Patch from Steve (Thursday, 05 June 2008) works fine for my X5 too.
Thank you for waking me up ;)
Comment by Steve Bavin (pondlife) - Monday, 01 December 2008, 14:02 GMT
Resynced the code patch - also synced firmware/drivers/rtc/rtc_pcf50606.c with the existing firmware/drivers/rtc/rtc_pcf50605.c - it can be seen they are identical apart from the low-level calls e.g. disable_irq_save()/pcf50606_read_multiple()/restore_irq() vs. pcf50605_read_multiple(). I've synced the comments too, even when they refer to iPods...

I don't know if it's possible to merge the two completely, or whether an #ifdef hell could be used, but ideas are welcome.
Comment by Steve Bavin (pondlife) - Wednesday, 10 December 2008, 17:08 GMT
Bug report - with the last patch, I'm finding that my clock occasionally gets set to --:--.

I'm not sure what's changed - the only functional chsnge was to explicitly disable the alarm by:
+ /* Make sure we don't wake on RTC after shutting down */
+ pcf50606_write(0x8, pcf50606_read(0x8) & ~0x10);

Comment by PaulJam (PaulJam) - Thursday, 11 December 2008, 11:42 GMT
In the last version of the patch it looks like in the function 'rtc_check_alarm_flag()' that 'pcf50606_read()' is used without 'disable_irq_save()' and 'restore_irq()'
Comment by Steve Bavin (pondlife) - Thursday, 11 December 2008, 11:56 GMT
Ah, well spotted! Here's an update. /me wonders why the pcf50605 code doesn't need to do any IRQ disabling...
Comment by Steve Bavin (pondlife) - Thursday, 09 July 2009, 19:50 GMT
Anyone object if his is committed or should it be merged with the pcf50605 driver? If so, what's the best/conventional way to handle the differences?

Loading...