Skip to content
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

tests: drivers: rtc: multiple minor issues in rtc_emul mock/fake device #59901

Closed
ghost opened this issue Jul 1, 2023 · 0 comments · Fixed by #59902
Closed

tests: drivers: rtc: multiple minor issues in rtc_emul mock/fake device #59901

ghost opened this issue Jul 1, 2023 · 0 comments · Fixed by #59902
Labels
area: RTC Real Time Clock bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@ghost
Copy link

ghost commented Jul 1, 2023

Describe the bug

The RTC tests and test mock (emulation) device have multiple minor issues:

  • The RTC tests that run in the CI/CD platform do not cover the alarm, update and calibration parts of the API by default although such tests exist in the tests tree.
  • The RTC test suite does not build on 64 bit architectures.
  • The RTC emulation device is enabled by default on native_posix for all tests, although it's only required for a single driver.
  • The RTC driver tests that ran in the CI/CD pipeline are instantiated on all platforms (to be filtered then via dt_alias_exists() back to native_posix in practice). The test cases should hint the CI/CD platform to focus on native_posix (while not keeping anybody from running them on other platforms if required)~~
  • The RTC emulation device is meant for testing purposes only but resides in the main driver tree which clutters devicetree and docs, may confuse users concerning its purpose (it confused me quite a bit), breaks encapsulation and creates redundant code. As customary, mock/fake drivers should be placed in the test tree.
  • A few minor locking/memory issues can be fixed on-the-fly plus the fake driver can be simplified a bit (not worth mentioning details, see PR that follows).

UPDATE: Removed items due to review comments on the linked PR, see there. Won't fix.

To Reproduce
./scripts/twister -T tests/drivers/rtc -c --integration

Expected behavior

  • use spinlocks instead of mutexes

Impact

minor impact:

  • missing test coverage - potential for regressions
  • some avoidable maintenance effort, unnecessary complexity
  • potential confusion of users re purpose of zephyr,rtc-emul (true, but no longer part of this issue, see update above)
  • a few extra seconds of execution on the CI/CD pipeline plus unnecessarily long "skipped" reports
  • unnecessary resource usage during tests (kernel resources, interrupts, mutex)

Logs and console output

n/a

Environment (please complete the following information):

dev env, CI/CD

Additional context

See #19030 (comment)

@ghost ghost added the bug The issue is a bug, or the PR is fixing a bug label Jul 1, 2023
@henrikbrixandersen henrikbrixandersen added the area: RTC Real Time Clock label Jul 4, 2023
@jhedberg jhedberg assigned ghost Jul 4, 2023
@jhedberg jhedberg added the priority: low Low impact/importance bug label Jul 4, 2023
carlescufi pushed a commit that referenced this issue Jul 12, 2023
Replaces the mutex with a lightweight spinlock.

Fixes: #59901

Signed-off-by: Florian Grandel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: RTC Real Time Clock bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants