Skip to content

drivers: rtc: mc146818: fix y2k bug #59402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented Jun 19, 2023

That's correct. We are still fixing the Y2K bug in 2023 \o/

  • write century to RAM register 0x32
  • ensure year register is in [0,99] (inclusive)

Aside from that, there were a few other errors in the driver.

  • translate epoch-centric RTC API year to begin at 1900
  • fix off-by-one error with month limit
  • fix off-by-one error with wday
  • fix off-by-one-hundred error with year limit
  • adjust timeptr values in rtc_mc146818_validate_time()
  • adjust timeptr values in rtc_mc146818_validate_alarm()

With the above, the testsuite passes!

Fixes #59387

west build -p auto -b qemu_x86_64 -t run tests/drivers/rtc/rtc_api
...
*** Booting Zephyr OS build v3.4.0-167-g327eb119b6bc ***
Running TESTSUITE rtc_api
===================================================================
START - test_set_get_time
 PASS - test_set_get_time in 0.002 seconds
===================================================================
START - test_time_counting
 PASS - test_time_counting in 10.197 seconds
===================================================================
TESTSUITE rtc_api succeeded

------ TESTSUITE SUMMARY START ------

SUITE PASS - 100.00% [rtc_api]: pass = 2, fail = 0, skip = 0, total = 2 duration = 10.199 seconds
 - PASS - [rtc_api.test_set_get_time] duration = 0.002 seconds
 - PASS - [rtc_api.test_time_counting] duration = 10.197 seconds

------ TESTSUITE SUMMARY END ------

===================================================================
PROJECT EXECUTION SUCCESSFUL

@zephyrbot zephyrbot added the area: RTC Real Time Clock label Jun 19, 2023
@cfriedt cfriedt added this to the v3.4.1 milestone Jun 19, 2023
That's correct. We are still fixing the Y2K bug in 2023 \o/

* write century to RAM register 0x32
* ensure year register is in [0,99] (inclusive)

Aside from that, there were a few other errors in the driver.

* translate epoch-centric RTC API year to begin at 1900
* fix off-by-one error with month limit
* fix off-by-one error with wday
* fix off-by-one-hundred error with year limit
* adjust timeptr values in rtc_mc146818_validate_time()
* adjust timeptr values in rtc_mc146818_validate_alarm()

With the above, the testsuite passes!

Signed-off-by: Christopher Friedt <[email protected]>
@cfriedt
Copy link
Member Author

cfriedt commented Jun 19, 2023

One additional modification

  • adjust timeptr values in rtc_mc146818_validate_alarm()

@cfriedt
Copy link
Member Author

cfriedt commented Jun 19, 2023

I haven't yet tested rolling over from 1999 to 2000 - should we?? What if something terrible were to happen?? 😬

@cfriedt cfriedt added the bug The issue is a bug, or the PR is fixing a bug label Jun 19, 2023
@bjarki-andreasen
Copy link
Collaborator

Nice work!

If you want to test rolling over from 1999 to 2000, change this define to 946684795UL and rebuild the test suite :)

@cfriedt
Copy link
Member Author

cfriedt commented Jun 20, 2023

Let me add a y2k testcase. Any others we should add currently? 2038?

@bjarki-andreasen
Copy link
Collaborator

bjarki-andreasen commented Jun 20, 2023

We can add 1999 to 2000 to the test suite, but that should probably go into its own its own PR :) 2038 is not relevant since RTCs don't track time using integers, that would have to be part of some counter test suite to be relevant i think :)

@cfriedt
Copy link
Member Author

cfriedt commented Jun 20, 2023

We can add 1999 to 2000 to the test suite, but that should probably go into its own its own PR :)

It's practically done already - I might as well add it here. Plus The Zephyr Way is to add tests along with bugfixes, when possible.

2038 is not relevant since RTCs don't track time using integers, that would have to be part of some counter test suite to be relevant i think :)

True, yea.

@carlescufi carlescufi merged commit c8e0022 into zephyrproject-rtos:main Jun 20, 2023
@cfriedt
Copy link
Member Author

cfriedt commented Jun 20, 2023

Hmm... should have put a DNM on this - driver also fails to pass testsuite when CONFIG_RTC_ALARM is set and CONFIG_RTC_UPDATE are set. The driver failed before this change as well, so at least there is no regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Drivers area: RTC Real Time Clock bug The issue is a bug, or the PR is fixing a bug
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

drivers: rtc: mc146818 fails rtc_api testsuite
5 participants