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.

Sign up for GitHub

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

Jump to bottom

AVR-8: Implement Power Management #19784

Merged
merged 8 commits into from
Dec 1, 2023
Merged

AVR-8: Implement Power Management #19784

merged 8 commits into from
Dec 1, 2023

Conversation

nandojve
Copy link
Contributor

@nandojve nandojve commented Jul 1, 2023 •
edited
Loading

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 force-pushed the avr8/implement_pm branch from 8d09e1b to 601ba05 Compare July 2, 2023 09:58
@nandojve nandojve marked this pull request as ready for review July 2, 2023 10:01
@benpicco benpicco requested a review from roberthartung July 4, 2023 13:02
@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 •
edited
Loading

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 nandojve mentioned this pull request Jul 5, 2023
Draft
@nandojve nandojve force-pushed the avr8/implement_pm branch from 3d1e300 to 3432f5b Compare July 6, 2023 16:19
@nandojve
Copy link
Contributor Author

nandojve commented Jul 7, 2023

This seems ready to go!

@nandojve nandojve force-pushed the avr8/implement_pm branch from 3432f5b to d61157f Compare July 11, 2023 18:42
@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 : )

MrKevinWeiss
MrKevinWeiss reviewed Nov 27, 2023
View reviewed changes
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
@maribu maribu force-pushed the avr8/implement_pm branch from f7c6b02 to 86ef5fd Compare December 1, 2023 13:10
@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
@nandojve @maribu
cpu/avr8: Enable PM periph to all SoC
3b9368a 
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>
@nandojve @maribu
cpu/atxmega: Fix PM states on peripherals
549e2b4 
Fix the required PM state on i2c and spi peripherals.

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

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
@nandojve @maribu
cpu/atmega_common: Increase stack a little bit
c64a64f 
This is necessary to allow run the thread_duel example.

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
@nandojve @maribu
boards/atmega328p-xplained-mini: Add BTN0 and LED0
859d587 
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>
@nandojve @maribu
cpu/avr8_common: Fix PM and Common peripherals build
ab70f20 
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@posteo.net>
Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
@nandojve @MrKevinWeiss @maribu
cpu/atxmega: Drop unnecessary test config
166ee62 
Co-authored-by: MrKevinWeiss <weiss.kevin604@gmail.com>
Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
@maribu maribu force-pushed the avr8/implement_pm branch from 86ef5fd to a50ba48 Compare December 1, 2023 13:25
@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 :-/

@maribu
examples, tests: update Makefile.ci for AVR8
82d98ed 
Ran dist/tools/insufficient_memory for all AVR8 boards.
@maribu maribu force-pushed the avr8/implement_pm branch from a50ba48 to 82d98ed Compare December 1, 2023 18:37
@maribu maribu enabled auto-merge December 1, 2023 18:38
@maribu maribu mentioned this pull request Dec 1, 2023
Merged
@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
26 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
Reviewers

@MrKevinWeiss MrKevinWeiss MrKevinWeiss approved these changes

@benpicco benpicco benpicco left review comments

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@kYc0o kYc0o Awaiting requested review from kYc0o kYc0o is a code owner

@maribu maribu Awaiting requested review from maribu maribu is a code owner

@roberthartung roberthartung Awaiting requested review from roberthartung

@smlng smlng Awaiting requested review from smlng

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@jeandudey jeandudey Awaiting requested review from jeandudey

Assignees
No one assigned
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
Milestone
Release 2024.01
Development

Successfully merging this pull request may close these issues.
5 participants
@nandojve @riot-ci @maribu @MrKevinWeiss @benpicco