drivers/rtc/rtc_mc146818: Fix RTC driver for QEMU boards #60321

bjarki-andreasen · 2023-07-12T19:03:47Z

This commit fixes the driver, allowing it to pass
the RTC API test suite for set/get time, alarms and the update callback (the hw does not support calibration)

This requires an additional flag to the HPET module which disables legacy IRQs, allowing the RTCs IRQ to be raised.

Furthermore, the commit cleans up the RTC in the devicetree for the IA32 dtsi and the qemu_x64 dts files, and updates the yaml files for QEMU to indicate the support for the RTC subsystem.

Fixes #59448

bjarki-andreasen · 2023-07-12T19:07:26Z

Proof of it working @cfriedt :D

[0/1] To exit from QEMU enter: 'CTRL+a, x'[QEMU] CPU: qemu32,+nx,+pae
SeaBIOS (version zephyr-v1.0.0-0-g31d4e0e-dirty-20200714_234759-fv-az50-zephyr)
Booting from ROM..
*** Booting Zephyr OS build zephyr-v3.4.0-974-g7fba7d395750 ***
Running TESTSUITE rtc_api
===================================================================
START - test_alarm
 PASS - test_alarm in 26.041 seconds
===================================================================
START - test_alarm_callback
 PASS - test_alarm_callback in 26.040 seconds
===================================================================
START - test_set_get_time
 PASS - test_set_get_time in 0.001 seconds
===================================================================
START - test_time_counting
 PASS - test_time_counting in 9.689 seconds
===================================================================
START - test_update_callback
 PASS - test_update_callback in 15.020 seconds
===================================================================
START - test_y2k
 PASS - test_y2k in 2.010 seconds
===================================================================
TESTSUITE rtc_api succeeded

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

SUITE PASS - 100.00% [rtc_api]: pass = 6, fail = 0, skip = 0, total = 6 duration = 78.801 seconds
 - PASS - [rtc_api.test_alarm] duration = 26.041 seconds
 - PASS - [rtc_api.test_alarm_callback] duration = 26.040 seconds
 - PASS - [rtc_api.test_set_get_time] duration = 0.001 seconds
 - PASS - [rtc_api.test_time_counting] duration = 9.689 seconds
 - PASS - [rtc_api.test_update_callback] duration = 15.020 seconds
 - PASS - [rtc_api.test_y2k] duration = 2.010 seconds

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

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

cfriedt

Tiny fix

drivers/rtc/rtc_mc146818.c

bjarki-andreasen · 2023-07-13T19:42:15Z

@dcpleung @andyross

I need help solving a test timeout occurring for a library named p4workq, which times out if no-legacy-irq; is used for the HPET for the QEMU boards. The test timing out is a stress test, but it seems it is a bug rather than an increased workload causing the timeout.

I am running the test using west twister -p qemu_x86_tiny -T zephyr/tests/lib/p4workq

The stress test has a define #define MAX_EVENTS which scales the amount of work which needs to be done. I ran the test for a few different values:

100 -> 13.27 seconds
300 -> 13.10 seconds
400 -> 13.37 seconds
410 -> timed out after 64 seconds...

I am enabling no-legacy-irq; to allow the QEMU q35 RTC's IRQ to fire.

I need help to move forward :)

cfriedt

It looks like a lot of changes in the driver are potentially unrelated to the actual fix.

I would suggest separating any cosmetic changes, and functional changes to separate commits.

Other than that, I think to avoid breaking other things (libp4work or whatever it is), it might be better to make the necessary DT adjustments in a board overlay in the testsuite.

Also, is it possible to ensure the driver is built and tested in CI? Do we have it in a build all test somewhere?

cfriedt · 2023-07-15T14:09:38Z

drivers/rtc/rtc_mc146818.c

@@ -51,7 +51,6 @@
 #define RTC_UIP		RTC_REG_A
 #define RTC_DATA	RTC_REG_B
 #define RTC_FLAG	RTC_REG_C
-#define RTC_ALARM_MDAY	RTC_REG_D


How much of this file actually needed to be changed in order for the fix to work?

At a glance, it seems that most of the changes here are cosmetic (formatting, name changes, ...). Is it possible to reduce the change to only what is required for the fix?

These changes are nearly the bare minimum required to get the driver to an acceptable state (I did a small amount of cleanup). There is still quite a bit to go which I agree is not within the scope of this PR.

boards/x86/qemu_x86/qemu_x86.dts

dts/x86/intel/ia32.dtsi

cfriedt

Tests are passing 🙌

cfriedt · 2023-07-15T23:12:59Z

I realize I just approved this, but it looks like a few of the RTC kconfig options are missing from tests/drivers/rtc/rtc_api/prj.conf

From what I understand all of the rtcs exercised in this testsuite support all of the options, so might be better to add them there as well.

bjarki-andreasen · 2023-07-16T00:02:01Z

@cfriedt

Only the mandatory tests from the test suite are run by default, which is why the features RTC_ALARM and RTC_UPDATE and RTC_CALIBRATE are not included in the main .prj Features are selected in the board overlays. The MC146818 only supports RTC_ALARM and RTC_UPDATE for example, not RTC_CALIBRATE :)

cfriedt · 2023-07-16T00:34:22Z

Only the mandatory tests from the test suite are run by default, which is why the features RTC_ALARM and RTC_UPDATE and RTC_CALIBRATE are not included in the main .prj Features are selected in the board overlays. The MC146818 only supports RTC_ALARM and RTC_UPDATE for example, not RTC_CALIBRATE :)

Sure, but just remember we need code paths tested, even if they are disabled by default.

What if the default prj.conf was to enable all options, and individual board overlays could opt out of testing unsupported options?

The MC146818 driver was not properly initialized by the driver, interrupts where not handled correctly, and the alarm feature was not implemented properly. This commit fixes these issues, while removing some code which became redundant as the MC146818 driver was patched. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

cfriedt · 2023-07-25T09:55:09Z

is @akanisetti able to review / approve as well?

cfriedt · 2023-07-25T10:48:06Z

@bjarki-trackunit - you might need to find another reviewer with approve privileges.

bjarki-andreasen · 2023-07-25T15:00:17Z

@henrikbrixandersen @erwango Could you please review this PR? :)

drivers/rtc/rtc_mc146818.c

This commit adds input clock selection to the RTC driver. This is required to allow for the real hardware to operate. The QEMU emulated hardware ignores the input clock settings. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen · 2023-08-08T10:35:58Z

@nordicjm @cfriedt Could you kindly re-review? :)

cfriedt

One tiny spelling mistake - I'll re-approve when fixed.

tests/drivers/rtc/rtc_api/boards/qemu_x86_64.overlay

This commit updates the qemu_x86 board's yaml file to indicate its support for the RTC subsystem. Board overlay and conf for the qemu_x86 has been added to the RTC test suite to enable the MC146818 and its dependencies. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

The update callback test had a bug which prevented it from running on 64-bit architectures. This patch makes the test agnostic to 64-bit and 32-bit architectures. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

This commit adds overlay and conf for the qemu_x86_64 board to the RTC API test suite, and adds support for the RTC subsystem to the qemu_x86_64's yaml file. The commit also specifies integration platforms for the RTC API test suite, since the qemu_x86_64 board runs in real-time, causing it to time out if the test suite runs for it. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen · 2023-08-09T09:54:22Z

@cfriedt pretty please :D

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch 2 times, most recently from 7d18931 to 5e4a9a3 Compare July 12, 2023 19:06

bjarki-andreasen mentioned this pull request Jul 12, 2023

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

Closed

bjarki-andreasen changed the title ~~drivers/rtc/rtc_mc146818: Fix driver for QEMU~~ drivers/rtc/rtc_mc146818: Fix RTC driver for QEMU boards Jul 12, 2023

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch 3 times, most recently from 21eeed4 to a2e16bc Compare July 12, 2023 19:13

bjarki-andreasen mentioned this pull request Jul 12, 2023

boards/qemu_x86: Fix emulated boards and their RTCs #59967

Closed

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch from a2e16bc to 00a450d Compare July 12, 2023 22:09

cfriedt reviewed Jul 12, 2023

View reviewed changes

drivers/rtc/rtc_mc146818.c Show resolved Hide resolved

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch 2 times, most recently from 23e6a2a to 9ec1897 Compare July 13, 2023 16:04

cfriedt requested changes Jul 15, 2023

View reviewed changes

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch 4 times, most recently from 1957f43 to 44bc4a0 Compare July 15, 2023 18:02

bjarki-andreasen requested a review from cfriedt July 15, 2023 18:45

cfriedt previously approved these changes Jul 15, 2023

View reviewed changes

bjarki-andreasen marked this pull request as ready for review July 16, 2023 11:30

bjarki-andreasen requested review from nashif and dcpleung as code owners July 16, 2023 11:30

bjarki-andreasen self-assigned this Jul 16, 2023

zephyrbot added area: RTC Real Time Clock platform: X86 x86 and x86-64 labels Jul 16, 2023

bjarki-andreasen dismissed cfriedt’s stale review via 983721f July 24, 2023 14:40

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch from 2739f21 to 983721f Compare July 24, 2023 14:40

bjarki-andreasen requested a review from rlubos as a code owner July 24, 2023 14:40

zephyrbot added the area: Networking label Jul 24, 2023

zephyrbot requested a review from ssharks July 24, 2023 14:41

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch from 983721f to 79ee044 Compare July 24, 2023 14:42

cfriedt previously approved these changes Jul 25, 2023

View reviewed changes

akanisetti approved these changes Jul 25, 2023

View reviewed changes

nordicjm requested changes Jul 27, 2023

View reviewed changes

drivers/rtc/rtc_mc146818.c Outdated Show resolved Hide resolved

bjarki-andreasen dismissed cfriedt’s stale review via 4121df5 July 27, 2023 12:22

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch from 79ee044 to 4121df5 Compare July 27, 2023 12:22

bjarki-andreasen requested a review from nordicjm July 27, 2023 12:22

cfriedt previously approved these changes Aug 8, 2023

View reviewed changes

tests/drivers/rtc/rtc_api/boards/qemu_x86_64.overlay Outdated Show resolved Hide resolved

Bjarki Arge Andreasen added 2 commits August 8, 2023 15:40

tests/drivers/rtc/rtc_api: Patch 64-bit arch support for test

be0ec10

The update callback test had a bug which prevented it from running on 64-bit architectures. This patch makes the test agnostic to 64-bit and 32-bit architectures. Signed-off-by: Bjarki Arge Andreasen <[email protected]>

bjarki-andreasen dismissed cfriedt’s stale review via 2821b41 August 8, 2023 13:41

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch from 4121df5 to 2821b41 Compare August 8, 2023 13:41

bjarki-andreasen force-pushed the patch_fix_mc146818_with_qemu branch from 2821b41 to 8094763 Compare August 8, 2023 18:17

nordicjm approved these changes Aug 9, 2023

View reviewed changes

cfriedt approved these changes Aug 9, 2023

View reviewed changes

cfriedt merged commit 5f80f30 into zephyrproject-rtos:main Aug 9, 2023

bjarki-andreasen deleted the patch_fix_mc146818_with_qemu branch August 9, 2023 13:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers/rtc/rtc_mc146818: Fix RTC driver for QEMU boards #60321

drivers/rtc/rtc_mc146818: Fix RTC driver for QEMU boards #60321

bjarki-andreasen commented Jul 12, 2023 •

edited by cfriedt

Loading

bjarki-andreasen commented Jul 12, 2023

cfriedt left a comment

bjarki-andreasen commented Jul 13, 2023 •

edited

Loading

cfriedt left a comment

cfriedt Jul 15, 2023

bjarki-andreasen Jul 15, 2023 •

edited

Loading

cfriedt left a comment

cfriedt commented Jul 15, 2023

bjarki-andreasen commented Jul 16, 2023 •

edited

Loading

cfriedt commented Jul 16, 2023 •

edited

Loading

cfriedt commented Jul 25, 2023

cfriedt commented Jul 25, 2023

bjarki-andreasen commented Jul 25, 2023

bjarki-andreasen commented Aug 8, 2023

cfriedt left a comment

bjarki-andreasen commented Aug 9, 2023

drivers/rtc/rtc_mc146818: Fix RTC driver for QEMU boards #60321

drivers/rtc/rtc_mc146818: Fix RTC driver for QEMU boards #60321

Conversation

bjarki-andreasen commented Jul 12, 2023 • edited by cfriedt Loading

bjarki-andreasen commented Jul 12, 2023

cfriedt left a comment

Choose a reason for hiding this comment

bjarki-andreasen commented Jul 13, 2023 • edited Loading

cfriedt left a comment

Choose a reason for hiding this comment

cfriedt Jul 15, 2023

Choose a reason for hiding this comment

bjarki-andreasen Jul 15, 2023 • edited Loading

Choose a reason for hiding this comment

cfriedt left a comment

Choose a reason for hiding this comment

cfriedt commented Jul 15, 2023

bjarki-andreasen commented Jul 16, 2023 • edited Loading

cfriedt commented Jul 16, 2023 • edited Loading

cfriedt commented Jul 25, 2023

cfriedt commented Jul 25, 2023

bjarki-andreasen commented Jul 25, 2023

bjarki-andreasen commented Aug 8, 2023

cfriedt left a comment

Choose a reason for hiding this comment

bjarki-andreasen commented Aug 9, 2023

bjarki-andreasen commented Jul 12, 2023 •

edited by cfriedt

Loading

bjarki-andreasen commented Jul 13, 2023 •

edited

Loading

bjarki-andreasen Jul 15, 2023 •

edited

Loading

bjarki-andreasen commented Jul 16, 2023 •

edited

Loading

cfriedt commented Jul 16, 2023 •

edited

Loading