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

xtimer: rework dependecies to ease backend switch #17809

Closed
wants to merge 9 commits into from

Conversation

fjmolinas
Copy link
Contributor

Contribution description

This PR sparks from #17721 (comment), trying to make the default backend change as close to a oneliner as possible. Because of #17721 (comment) I actually had to rework dependencies quite a bit, so Murdock might not be happy again... let's see.

With this change the default could be change with the following.

diff
diff --git a/makefiles/boards/ztimer_only.dep.mk b/makefiles/boards/ztimer_only.dep.mk
index f50810a051..3b6b20b03a 100644
--- a/makefiles/boards/ztimer_only.dep.mk
+++ b/makefiles/boards/ztimer_only.dep.mk
@@ -5,7 +5,7 @@
 # ztimer_xtimer_compat is used.
 
 ifneq (,$(filter xtimer,$(USEMODULE)))
-  ifeq (,$(filter ztimer_xtimer_compat ztimer64_xtimer_compat,$(USEMODULE)))
-    USEMODULE += xtimer_on_ztimer
+  ifeq (,$(filter xtimer_on_ztimer,$(USEMODULE)))
+    USEMODULE += ztimer_xtimer_compat
   endif
 endif
diff --git a/sys/xtimer/Kconfig b/sys/xtimer/Kconfig
index 3fe4a309bb..d4d538b034 100644
--- a/sys/xtimer/Kconfig
+++ b/sys/xtimer/Kconfig
@@ -19,19 +19,20 @@ if MODULE_XTIMER
 config XTIMER_ZTIMER_BACKEND
     bool "xtimer always implemented by ztimer"
     select MODULE_ZTIMER_USEC
+    default y
 
 choice XTIMER_BACKEND
     bool "xtimer backend"
 
-config MODULE_XTIMER_ON_ZTIMER
-    bool "xtimer api implemented by xtimer, with ztimer_usec backend"
-    depends on MODULE_ZTIMER_PERIPH_TIMER
-
 config MODULE_ZTIMER_XTIMER_COMPAT
     bool "xtimer api implemented by ztimer"
     depends on MODULE_ZTIMER_USEC
     select MODULE_DIV
 
+config MODULE_XTIMER_ON_ZTIMER
+    bool "xtimer api implemented by xtimer, with ztimer_usec backend"
+    depends on MODULE_ZTIMER_PERIPH_TIMER
+
 config MODULE_XTIMER_ON_PERIPH_TIMER
     bool "xtimer api implemented by xtimer, with periph_timer backend"
     select MODULE_DIV
diff --git a/sys/xtimer/Makefile.dep b/sys/xtimer/Makefile.dep
index cc643e88ad..4f964ee4e1 100644
--- a/sys/xtimer/Makefile.dep
+++ b/sys/xtimer/Makefile.dep
@@ -9,7 +9,6 @@ DEFAULT_MODULE += auto_init_xtimer
 # order issues, this way xtimer_on_periph_timer can be turned into a DEFAULT_MODULE
 # that can be disabled if ztimer_periph_timer
 FEATURES_REQUIRED += periph_timer
-DEFAULT_MODULE += xtimer_on_periph_timer
 
 # To avoid inclusion order changing dependencies this dependency is commented out
 # but it models xtimer_on_periph_timer dependency explicitly, see comment above
@@ -18,12 +17,8 @@ DEFAULT_MODULE += xtimer_on_periph_timer
   # USEMODULE += div
   # FEATURES_REQUIRED += periph_timer
 # endif
-
-ifneq (,$(filter ztimer_periph_timer,$(USEMODULE)))
-  DISABLE_MODULE += xtimer_on_periph_timer
-  ifeq (,$(filter ztimer_xtimer_compat,$(USEMODULE)))
-    USEMODULE += xtimer_on_ztimer
-  endif
+ifeq (,$(filter xtimer_on_%,$(USEMODULE)))
+  USEMODULE += ztimer_xtimer_compat
 endif
 
 ifneq (,$(filter ztimer_xtimer_compat,$(USEMODULE)))

Although adapting tests Makefile.ci will also be needed (17b3c6e and 6728d7c)

This PR also changes the auto-selection of the compat modules from ztimer to ztimer_periph_timer, it, therefore, allows having legacy timer and ztimer_msec running on rtt.

From local testing (To be confirmed by murdock), I think this change allows getting rid of the ZTIMER_USEC indirection as well as ZTIMER64_USEC indirection

Marking as WIP will I let Murdock check I didn't make a fundamentally wrong change.

Testing procedure

  • Green Murdock

Issues/PRs references

#17721

@fjmolinas fjmolinas added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 15, 2022
@github-actions github-actions bot added Area: arduino API Area: Arduino wrapper API Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework Area: timers Area: timer subsystems Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: native Platform: This PR/issue effects the native platform Area: CI Area: Continuous Integration of RIOT components labels Mar 15, 2022
@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 15, 2022
@fjmolinas fjmolinas force-pushed the pr_xtimer_dep_rework branch 2 times, most recently from caf0754 to c0289e4 Compare March 15, 2022 21:24
@github-actions github-actions bot removed the Area: CI Area: Continuous Integration of RIOT components label Mar 15, 2022
@fjmolinas fjmolinas removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Mar 16, 2022
@fjmolinas fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 16, 2022
@fjmolinas
Copy link
Contributor Author

Closing in favor off #17811

@fjmolinas fjmolinas closed this Mar 17, 2022
@fjmolinas fjmolinas deleted the pr_xtimer_dep_rework branch June 9, 2022 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: arduino API Area: Arduino wrapper API Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: sys Area: System Area: tests Area: tests and testing framework Area: timers Area: timer subsystems Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: native Platform: This PR/issue effects the native platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant