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

C6 deep sleep driver MVP #918

Merged
merged 7 commits into from
Jan 3, 2024
Merged

C6 deep sleep driver MVP #918

merged 7 commits into from
Jan 3, 2024

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Nov 9, 2023

cc #375

This PR aims to implement deep sleep support for the HP core of ESP32-C6. This is a more-or-less direct port of esp-idf (v5.1.2).

We deviate from the esp-idf implementation where it doesn't fit into our current architecture:

  • we create a default sleep configuration structure first, that we then manipulate
  • we apply hardware changes based on the completed sleep configuration structure
  • then we enter and maybe exit sleep similar to esp-idf

The difference is, that in our case, the configured wakeup sources modify the sleep config directly, instead of an after-the-fact setup down the line. This, in theory, enables us to simplify the config structure from its current "in-memory register block" representation. This is not done for the sake of reducing porting effort.

Only LP_IO is implemented as a wakeup source, and probably only deep sleep works.

Status:

  • startup code
  • default sleep config
  • deep sleep config
  • wakeup sources - LP_IO only for now

Must

  • The code compiles without errors or warnings.
  • All examples work.
  • cargo fmt was run.
  • Your changes were added to the CHANGELOG.md in the proper section.
  • You updated existing examples or added examples (if applicable).
  • Added examples are checked in CI

Nice to have

  • You add a description of your work to this PR.
  • You added proper docs for your newly added features and code.

@bugadani bugadani force-pushed the c6sleep branch 6 times, most recently from 6b7ef60 to e8f468e Compare November 9, 2023 18:18
@bugadani
Copy link
Contributor Author

bugadani commented Nov 9, 2023

I would appreciate if someone could help me test if the current state boots and doesn't break peripherals and radio, otherwise I'll carry on and test it myself when I have my new hardware built next week or after that.

@bjoernQ
Copy link
Contributor

bjoernQ commented Nov 10, 2023

Tested this with a couple of examples in esp-hal - still works.
Tested with esp-wifi - still works.

@bugadani bugadani force-pushed the c6sleep branch 6 times, most recently from 0fa2d96 to 0ae0102 Compare November 13, 2023 14:55
@bugadani bugadani changed the title C6 sleep driver C6 sleep driver MVP Nov 14, 2023
@jessebraham
Copy link
Member

Just checking in here, was this ready for review or are there still remaining tasks to resolve?

@bugadani
Copy link
Contributor Author

I'm trying to progress with my project to a point where I can start testing this.

@bugadani bugadani force-pushed the c6sleep branch 3 times, most recently from c4af537 to 4988a35 Compare November 23, 2023 15:24
@bugadani
Copy link
Contributor Author

Device now seems to be entering sleep, but can't yet wake up on GPIO level change.

@bugadani bugadani force-pushed the c6sleep branch 7 times, most recently from 111b48b to 77c783a Compare November 25, 2023 13:58
@bugadani bugadani force-pushed the c6sleep branch 2 times, most recently from 8f4a59f to 22d70f6 Compare November 29, 2023 13:09
@bugadani
Copy link
Contributor Author

030a38e - wakeup has been achieved

@bugadani bugadani changed the title C6 sleep driver MVP C6 deep sleep driver MVP Dec 21, 2023
@bugadani bugadani marked this pull request as ready for review December 21, 2023 15:55
Copy link
Contributor

@JurajSadel JurajSadel left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for working on this! The example works fine for me, I left just 2 nitpicks.

esp-hal-common/src/rtc_cntl/sleep/mod.rs Outdated Show resolved Hide resolved
esp-hal-common/src/rtc_cntl/sleep/esp32c6.rs Show resolved Hide resolved
@t-moe
Copy link
Contributor

t-moe commented Dec 28, 2023

I'm not sure how much work this is, but I could really use Timer wakeup apart from the GPIO....

@bugadani
Copy link
Contributor Author

I'm not sure how much work this is

After wasting three weeks on this I decided to draw the line at the bare minimum I personally needed. I'm not sure how much work it is, depends on whether it's implemented in esp-idf (probably not extreme amounts of work) or not (low power systems aren't documented currently so it may not even be possible right now).

At any rate, this PR is big enough as it is, I'm kinda burnt out on this and timer support should be mostly additive on top of this.

Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this!

@jessebraham jessebraham added this pull request to the merge queue Jan 3, 2024
Merged via the queue into esp-rs:main with commit e9d6a21 Jan 3, 2024
17 checks passed
@bugadani bugadani deleted the c6sleep branch January 3, 2024 15:18
Volkalex28 pushed a commit to Volkalex28/esp-hal that referenced this pull request Feb 6, 2024
* Restructure sleep-related files

* Port most of esp-idf deep sleep code

* Add example

* Remove extra newline

* Hide RtcioWakeupSource from esp32 api

* Explain commented constants

---------

Co-authored-by: Jesse Braham <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants