CMOS RTC gives inconsistent data sporadicly

[bugraaydogar]

Hi all,

I have a X86 based system running Windows Embedded CE. My problem is with internal CMOS RTC in the companion controller chip.

My source code is almost same with the link below.
https://msdn.microsoft.com/en-us/library/aa450606.aspx

Problem Description :

- I update/set the time every 10 seconds.
- System checks the time every 1 seconds.

I see some inconsistent date/time value at second step. For instance;
Assume that I set the time 2015 10 13 10:30:45

2030 10 13 10:30:46
2030 10 13 10:30:47
2015 10 13 10:30:48

Another example;
Assume that I set the time 2015 10 13 11:45:45

For 5 seconds everything is fine then I read,

2002 10 13 11:45:50
2002 10 13 11:45:51
2002 10 13 11:45:52

This error continued until next set time operation.
Not just year changes, that could be month or hour or minute too.

I also checked the implementation in different platforms like Linux and here in OSDev wiki page for CMOS RTC. They are identical to Windows implementation.
Moreover this source code [link aboved] works perfect with a different x-86 based system. So I thought that might be because of my companion controller.
Any of you faced the issue like that before or do you have any idea about what is going on ?

My another question is about Data Mode register in RTC. What i see is that it changes from 0 to 1 randomly.
For instance when I set time it is 0 which states BCD mode. When I get time it is 1 which states binary mode. Is this a normal behavior ?

Thanks in advance,
Bugra

[alexfru]

There's a possible race condition:

Code: Select all

do {
    ...
    while (CMOS_Read(RTC_STATUS_A) & RTC_SRA_UIP);
    // HERE
    lpst1->wYear = CMOS_Read(RTC_YEAR);
    // HERE
    ...
} while (!IsTimeEqual (lpst1, lpst2));

When you read UIP=0, you have only 244 microseconds to read the time and date (according to my doc on VIA's VT8255 RTC). If the above loop gets preempted for too long, strange things may happen. Are you sure the above isn't preempted for too long in your case?

How about first waiting until UIP=1, then waiting until UIP=0, then reading the time and date regs? Will that fail to work as well?

[Brendan]

Hi,

My source code is almost same with the link below.
https://msdn.microsoft.com/en-us/library/aa450606.aspx

Not only is that source code buggy; if the bugs were fixed it'd still be a poorly designed pile of garbage.

For example; an update can begin immediately after this check:

Code: Select all

    do {
        cStatusRegA = CMOS_Read( RTC_STATUS_A);
    } while ( cStatusRegA & RTC_SRA_UIP );

And this code does NOT disable updates:

Code: Select all

    // Disable updates while the values are changed
    cStatusRegB = CMOS_Read( RTC_STATUS_B );
    cStatusRegB |= RTC_SRB_UPDT;
    CMOS_Write( RTC_STATUS_B, cStatusRegB );

There is nothing in status register B that disables updates (and nothing in the entire RTC chip that allows updates to be disabled); there is only a flag in status register B that enables/disables an interrupt when an update completes.

Because of both these problems; the update may be in progress while you're in the middle of writing to the (year, month, day, ...) registers; and this can/will cause values to be corrupted. For example, if the old values were 23:59:59 and you're trying to set the new values 1:00:00 then you can end up 2:00:00 or 1:01:00 or 1:00:01 (and their silly "HARDWARE_TIME_SET_PROBLEM" work-around, which I expect exists because they couldn't figure out why their buggy code is buggy, only works around one of those cases).

The correct way to do this is wait until the update completes (so you know the next update won't begin for at least ~900 ms, and therefore won't begin while you're changing the RTC's time). This means using something more like:

Code: Select all

    // Wait until update starts
    do {
        cStatusRegA = CMOS_Read( RTC_STATUS_A);
    } while ( cStatusRegA & RTC_SRA_UIP == 0 );

    // Wait until update completes
    do {
        cStatusRegA = CMOS_Read( RTC_STATUS_A);
    } while ( cStatusRegA & RTC_SRA_UIP != 0 );

This will mean waiting for up to 1 second each time you set the RTC; which is painful.

For this reason; you want to use the "interrupt on update" feature. When this interrupt is enabled the RTC generates an IRQ after an update completes. Basically; "Bare_SetRealTime()" would store the new values in RAM without waiting at all, and the IRQ handler (that is run after an update has completed) would check if there's new values in RAM and update the RTC.

However; reading the RTC registers (and setting them) is slow and you want to avoid it. What you really want to do is read the RTC once during boot (and never read them ever again for any reason); and use that "interrupt on update" feature (or the more precise "periodic update interrupt", or a completely different timer like PIT or HPET) to update your own/internal "current time" variables instead.

Finally; when reading/writing to any register in the CMOS (including RTC registers) you set the "index" IO port, then any other code (on another CPU, another thread, whatever) has a chance to screw up the index you just set; then you read/write to the "data" IO port (using the corrupted/wrong index). To guard against this you must have some sort of global lock that is used by all code that reads/writes any CMOS register in the entire OS. You can't just write your own private little "CMOS_Write()" and "CMOS_Read()" functions.


Cheers,

Brendan

[bugraaydogar]

Hi Brendan , Hi Alex

Thanks for your support and ideas about the issue.

I was very hopeful that waiting for UIP bit to be 1 then 0 will work. But apparently not. That's so strange. I applied this change to Bare_GetRealTime and Bare_SetRealTime functions.
I have an app which calls GetSystemTime in a while loop(MS function which calls Bare_GetRealTime at the end), Bare_GetReatTime function last around 10 us. How this is possible?

My understanding about UIP bit was like in below, [0 : Not updating, 1 : Updating] and Updating last around 244 us.


0's last around 900 ms, 1's last 244 us or sth like that.

Thanks,
Bugra

[Brendan]

Hi,

I was very hopeful that waiting for UIP bit to be 1 then 0 will work. But apparently not. That's so strange. I applied this change to Bare_GetRealTime and Bare_SetRealTime functions.
I have an app which calls GetSystemTime in a while loop(MS function which calls Bare_GetRealTime at the end), Bare_GetReatTime function last around 10 us. How this is possible?

Typically (for ancient/legacy devices like CMOS) each IO port access costs about 1 us. To read one CMOS register you have to set the "index" IO port, then read from the "data" IO port. That's two IO port accesses, or about 2 us. If you read 7 CMOS registers (register B, century, year, month, day, hour, second) then 7 * 2 = 14 us.

Combining these things; you should expect between 0 ms and 1000 ms of waiting, then 14 us after that to read CMOS registers.

It might only take ~14 us (if you're extremely lucky and happen to begin waiting just before UIP goes from 1 to 0). If you're always extremely lucky, then your code to wait for UIP is broken.

Note: The exact length of time that UIP is 1 (and length of time that UIP is 0) is different for different hardware. For some RTC chips it might be 750 ms and 250 ms. For others it might be 950 ms and 50 ms. The length of time can also be different on the same RTC (e.g. updating from 0:00:00 to 0:00:01 can be fast, and updating from 23:59:59 to 0:00:00 can be slow). The only guarantee is that the frequency is (within a small tolerance of) 1 Hz.


Cheers,

Brendan