Skip to content

drivers: rtc: mc146818: driver fails multiple tests from rtc_api testsuite #59448

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

Closed
cfriedt opened this issue Jun 20, 2023 · 6 comments · Fixed by #60321
Closed

drivers: rtc: mc146818: driver fails multiple tests from rtc_api testsuite #59448

cfriedt opened this issue Jun 20, 2023 · 6 comments · Fixed by #60321
Assignees
Labels
area: Drivers area: RTC Real Time Clock area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Milestone

Comments

@cfriedt
Copy link
Member

cfriedt commented Jun 20, 2023

Describe the bug

The rtc_mc146818 driver is broken (and has actually always been broken). It's unclear to me how this driver was even merged (#56334) in the state that it was in (literally failing every test in the testsuite).

Some notes:

  1. The testsuite itself is broken
  • possibly incorrect bit in the API documentation int tm_year; /**< Year - 1900 */
    • based on stepping through code, it appears that the reference is actually assumed to be 1970 due to the use of gmtime_r() in the testsuite, although that is perhaps accidental
  • should exercise as many features / code paths as possible, so please enable these
    • CONFIG_RTC_ALARM=y
    • CONFIG_RTC_UPDATE=y
    • CONFIG_RTC_CALIBRATION=y
  • does the testsuite run in CI?
  • I would suggest adding to prj.conf to reduce CI churn
  1. This driver needs to be fixed
  • I'm not actually able to get the ISR to trigger
    • likely necessary in order to successfully pass a Y2K rollover test
  • test_alarm does not work
  • test_alarm_callback does not work
  • test_update_callback does not work
  • the driver should probably be using if (IS_ENABLED(..)) { ... } in several places instead of #ifdefs

Please also mention any information which could help others to understand
the problem you're facing:

  • What target platform are you using?: qemu_x86, qemu_x86_64
  • What have you tried to diagnose or workaround this issue? run / added tests / fixed bugs
  • Is this a regression? No. It has been broken since the initial commit

To Reproduce
Steps to reproduce the behavior:

  1. west build -p auto -b qemu_x86 -t run tests/drivers/rtc/rtc_api -- -DCONFIG_RTC_ALARM=y -DCONFIG_RTC_UPDATE=y -DCONFIG_RTC_CALIBRATION=y
  2. See error

Expected behavior
The testsuite should pass for all existing drivers within the tree.

Impact
Showstopper for now, because I would expect this RTC device to work on a tier0 platform. We were hoping to use this for additional POSIX improvements.

Logs and console output

west build -p auto -b qemu_x86 -t run tests/drivers/rtc/rtc_api -- -DCONFIG_RTC_ALARM=y -DCONFIG_RTC_UPDATE=y -DCONFIG_RTC_CALIBRATION=y
...
SeaBIOS (version zephyr-v1.0.0-0-g31d4e0e-dirty-20200714_234759-fv-az50-zephyr)
Booting from ROM..
*** Booting Zephyr OS build v3.4.0-167-g327eb119b6bc ***
Running TESTSUITE rtc_api
===================================================================
START - test_alarm

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/rtc/rtc_api/src/test_alarm.c:129: rtc_api_test_alarm: (ret == 1 is false)
Alarm should be pending
 FAIL - test_alarm in 13.017 seconds
===================================================================
START - test_alarm_callback

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/rtc/rtc_api/src/test_alarm_callback.c:145: rtc_api_test_alarm_callback: (callback_called_status == true is false)
Alarm callback should have been called
 FAIL - test_alarm_callback in 13.020 seconds
===================================================================
START - test_set_get_calibration

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/rtc/rtc_api/src/test_calibrate.c:54: rtc_api_test_set_get_calibration: (ret == 0 is false)
Failed to set calibration
 FAIL - test_set_get_calibration in 0.001 seconds
===================================================================
START - test_set_get_time
 PASS - test_set_get_time in 0.001 seconds
===================================================================
START - test_time_counting
 PASS - test_time_counting in 10.196 seconds
===================================================================
START - test_update_callback

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/rtc/rtc_api/src/test_update_callback.c:52: rtc_api_test_update_callback: (counter < 12 && counter > 8 is false)
Invalid update callback called counter
 FAIL - test_update_callback in 15.021 seconds
===================================================================
TESTSUITE rtc_api failed.

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

SUITE FAIL -  33.33% [rtc_api]: pass = 2, fail = 4, skip = 0, total = 6 duration = 51.256 seconds
 - FAIL - [rtc_api.test_alarm] duration = 13.017 seconds
 - FAIL - [rtc_api.test_alarm_callback] duration = 13.020 seconds
 - FAIL - [rtc_api.test_set_get_calibration] duration = 0.001 seconds
 - PASS - [rtc_api.test_set_get_time] duration = 0.001 seconds
 - PASS - [rtc_api.test_time_counting] duration = 10.196 seconds
 - FAIL - [rtc_api.test_update_callback] duration = 15.021 seconds

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

===================================================================
PROJECT EXECUTION FAILED

Environment (please complete the following information):

  • OS: Linux
  • Toolchain: Zephyr SDK v0.16.1
  • Commit SHA or Version used: ae11485

Additional context
Originally discovered while working on #59402 but that PR was merged before additional changes could be made.

@cfriedt cfriedt added bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug area: Drivers area: Tests Issues related to a particular existing or missing test area: RTC Real Time Clock labels Jun 20, 2023
@cfriedt cfriedt added this to the v3.4.1 milestone Jun 20, 2023
@bjarki-andreasen
Copy link
Collaborator

bjarki-andreasen commented Jul 3, 2023

Given that the mc146818 is an actual hardware device, I am uncertain how it would be possible to run the qemu_x86 emulator and get the driver to actually do anything. Is the qemu_x86 emulator emulating the hardware somehow? or am I completely lost here :)

The author who introduced the driver was working with the x86/rpl_crb board, which are real hardware AFAIK, I would expect the driver works when run on those boards

@bjarki-andreasen
Copy link
Collaborator

I believe the the issue stems from the ia32 board dtsi file having the RTC added to it by mistake, and the qemu board having an alias added for it, again by mistake. The qemu boards should be using the emulated rtc driver, not the mc146818.

@bjarki-andreasen
Copy link
Collaborator

Maybe I am a bit lost, the old COUNTER_CMOS seems to have worked before, so maybe it is being emulated somehow?, gonna have to look a bit closer :)

@cfriedt
Copy link
Member Author

cfriedt commented Jul 3, 2023

Given that the mc146818 is an actual hardware device, I am uncertain how it would be possible to run the qemu_x86 emulator and get the driver to actually do anything. Is the qemu_x86 emulator emulating the hardware somehow? or am I completely lost here :)

Yes, that's what Qemu does.

The author who introduced the driver was working with the x86/rpl_crb board, which are real hardware AFAIK, I would expect the driver works when run on those boards

Copy-paste from the terminal showing the testsuite passing would be a good place to start.

@bjarki-andreasen
Copy link
Collaborator

bjarki-andreasen commented Jul 3, 2023

After spending some time with it, I can see the MC146818 is setting the interrupt flag, but no interrupt is seen, using qemu monitor, there is no interrupt 8 raised (checked using info irq), indicating it could be a bug with the emulated MC146818...

Looking into it gave a wild idea, how about we just use the emulated RTC driver created for Zephyr, instead of using a driver, which is interacting with a quite complex emulated device? Is there a good reason for the additional complexity brought by using the QEMU emulated RTC?

@bjarki-andreasen
Copy link
Collaborator

Here is a PR with an actual fix for the device driver (and small adjustments to QEMU boards) #60321 :)

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 area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants