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

AVR-8: Implement Power Management #19784

Merged
merged 8 commits into from
Dec 1, 2023
Merged

Conversation

nandojve
Copy link
Contributor

@nandojve nandojve commented Jul 1, 2023

Contribution description

The Xmega have a Power Management implementation. This moves that code to AVR-8 common and enable Power Management for all variants.

Testing procedure

This was tested using examples/thread_duel and tests/periph/pm examples on atxmega-a1u-xpro and atmega328p-xplained-mini boards. It may requires more tests and any help is welcome.

Issues/PRs references

This is a preparation for future scheduling and IRQ optimizations.

@github-actions github-actions bot added Platform: AVR Platform: This PR/issue effects AVR-based platforms Area: cpu Area: CPU/MCU ports Area: Kconfig Area: Kconfig integration Area: boards Area: Board ports labels Jul 1, 2023
@nandojve nandojve marked this pull request as ready for review July 2, 2023 10:01
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jul 4, 2023
@riot-ci
Copy link

riot-ci commented Jul 4, 2023

Murdock results

✔️ PASSED

82d98ed examples, tests: update Makefile.ci for AVR8

Success Failures Total Runtime
8082 0 8082 10m:09s

Artifacts

@nandojve nandojve force-pushed the avr8/implement_pm branch 3 times, most recently from 1f05ebc to 3d1e300 Compare July 5, 2023 18:21
@nandojve
Copy link
Contributor Author

nandojve commented Jul 5, 2023

I have impression that Murdock is randomly failing.

@maribu
Copy link
Member

maribu commented Jul 5, 2023

The order in which app+board pairs are build is pretty random and the CI run stops once the first failure is hit. This could indeed result in pretty random failures, if more than on app fails.

The lasted failure (different modules used when building with TEST_KCONFIG=1 and without) could be fixed e.g. using

diff --git a/cpu/avr8_common/Kconfig b/cpu/avr8_common/Kconfig
index 03a0f40b5a..d82c187042 100644
--- a/cpu/avr8_common/Kconfig
+++ b/cpu/avr8_common/Kconfig
@@ -12,6 +12,8 @@ config CPU_ARCH_AVR8
     select HAS_ARCH_AVR8
     select HAS_PERIPH_PM
 
+    select MODULE_AVR8_COMMON_PERIPH if TEST_KCONFIG
+    select MODULE_PM_LAYERED if TEST_KCONFIG
     select MODULE_MALLOC_THREAD_SAFE if TEST_KCONFIG
     select MODULE_TINY_STRERROR_AS_STRERROR if TEST_KCONFIG
     # static C++ constructors need guards for thread safe initialization
@@ -41,6 +43,13 @@ config MODULE_AVR8_COMMON
     help
         AVR-8 common code.
 
+config MODULE_AVR8_COMMON_PERIPH
+    bool
+    depends on TEST_KCONFIG
+    default y
+    help
+        Common peripheral drivers used across different AVR-8 MCU families.
+
 # the atmel port uses stdio_uart by default
 choice STDIO_IMPLEMENTATION
     default MODULE_STDIO_UART

This one:

-- running on worker riotbuild-numa-2 thread 1, build number 186110.
make: Entering directory '/tmp/dwq.0.2615164153767623/2950a57bddbed179a6e917a6f6caf29d/examples/suit_update'
# Reset package to checkout state.
rm -rf /tmp/dwq.0.2615164153767623/2950a57bddbed179a6e917a6f6caf29d/build/pkg/c25519
rm -rf /tmp/dwq.0.2615164153767623/2950a57bddbed179a6e917a6f6caf29d/build/pkg-build/libcose
rm -rf /tmp/dwq.0.2615164153767623/2950a57bddbed179a6e917a6f6caf29d/build/pkg-build/nanocbor
make: Leaving directory '/tmp/dwq.0.2615164153767623/2950a57bddbed179a6e917a6f6caf29d/examples/suit_update'
make: Entering directory '/tmp/dwq.0.2615164153767623/2950a57bddbed179a6e917a6f6caf29d/examples/suit_update'
make: *** No rule to make target '/tmp/dwq.0.2615164153767623/2950a57bddbed179a6e917a6f6caf29d/examples/suit_update/bin/native/suit_update/fw.1688580754.bin', needed by 'link'.  Stop.
make: *** Waiting for unfinished jobs....
Building application "suit_update" for "native" with MCU "native".

is indeed a race condition in the build system that is triggered when compiling that app for native with -j :/ I can (occasionally) reproduce that one locally. I was hoping that @kaspar030 would take a look 😇

@nandojve
Copy link
Contributor Author

nandojve commented Jul 7, 2023

This seems ready to go!

@nandojve
Copy link
Contributor Author

rebased since #19777 got merged.

@nandojve
Copy link
Contributor Author

nandojve commented Sep 9, 2023

I was wondering if there is anything pending on this : )

@nandojve
Copy link
Contributor Author

ping

@MrKevinWeiss
Copy link
Contributor

I can pick this up after I finish with the release, feel free to ping me in a few days.

@nandojve
Copy link
Contributor Author

Ping @MrKevinWeiss : )

Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a quick look things seem OK, maybe some logic issues for your consideration. I will try to test tomorrow when I am in the office. (I need to also familiarize myself with the whole PM stuff).

cpu/atmega_common/periph/i2c.c Show resolved Hide resolved
@github-actions github-actions bot added the Area: examples Area: Example Applications label Dec 1, 2023
@maribu
Copy link
Member

maribu commented Dec 1, 2023

I'll also do the rebase to resolve the merge conflict

nandojve and others added 7 commits December 1, 2023 14:12
This refactor the current xmega PM peripheral to avr8 common and extend
PM to cpus families.

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
Fix the required PM state on i2c and spi peripherals.

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
Add PM blocks to adc/i2c/spi peripherals.

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
This is necessary to allow run the thread_duel example.

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
The board have one user button and a user led but are not enabled.
This add necessary support to use the button and the led. It include
the configs to use with SAUL and button interrupt.

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@posteo.net>
Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
Co-authored-by: MrKevinWeiss <weiss.kevin604@gmail.com>
Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
@maribu maribu enabled auto-merge December 1, 2023 13:32
@maribu maribu added this pull request to the merge queue Dec 1, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 1, 2023
@maribu
Copy link
Member

maribu commented Dec 1, 2023

Looks like #20130 contains a bug :-/

Ran dist/tools/insufficient_memory for all AVR8 boards.
@maribu maribu enabled auto-merge December 1, 2023 18:38
@maribu maribu 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 Dec 1, 2023
@maribu maribu added this pull request to the merge queue Dec 1, 2023
Merged via the queue into RIOT-OS:master with commit ac409eb Dec 1, 2023
27 checks passed
@nandojve nandojve deleted the avr8/implement_pm branch December 2, 2023 10:35
@nandojve
Copy link
Contributor Author

nandojve commented Dec 2, 2023

Thank you for all help @maribu and @MrKevinWeiss

@MrKevinWeiss
Copy link
Contributor

Thanks for the contribution!

@MrKevinWeiss MrKevinWeiss added this to the Release 2024.01 milestone Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework CI: no fast fail don't abort PR build after first error CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: AVR Platform: This PR/issue effects AVR-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants