-
Notifications
You must be signed in to change notification settings - Fork 2k
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
cpu/esp32: support for light/deep sleep and pm_layered #13416
Conversation
6581329
to
18644ed
Compare
#endif /* DOXYGEN */ | ||
|
||
/** | ||
* @brief Current an output pin can drive in active and sleep modes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you somehow rewrite this sentence?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, it says what it means. Any suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just "The current an output pin can drive in active and sleep modes"
@yegorich Thanks for the first review. I pushed a fixup with a first round of fixes. |
2bca3f9
to
02ad1e7
Compare
@leandrolanzieri As already told by email, I would like to get this PR into the next release because it seems to be a useful feature. Is it OK to set the |
Sure! |
nothing a btw.: Looks like this PR came just in time for a new watch 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I run tests/periph_pm
with CFLAGS='-DESP_PM_WUP_UART0=6 -DESP_PM_WUP_PINS=GPIO0 -DESP_PM_WUP_LEVEL=0'
and do a unblock_rtc 1 5
I get
2020-03-17 22:04:28,495 # Unblocking power mode 1 for 5 seconds. 2020-03-17 22:04:28,496 # BTN0 pressed.
That is, the GPIO interrupt is called when the RTC alarm rings.
cpu/esp32/periph/pm.c
Outdated
rtc_get_time(&tm_system); | ||
rtc_get_alarm(&tm_alarm); | ||
|
||
time_t t_system = mktime(&tm_system); | ||
time_t t_alarm = mktime(&tm_alarm); | ||
int _alarm_set = 0; | ||
|
||
if (t_alarm > t_system) { | ||
_alarm_set = 1; | ||
esp_sleep_enable_timer_wakeup((uint64_t)(t_alarm - t_system) * US_PER_SEC); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest moving this to periph/rtc
.
We started something similar for the sam0 platform where some hooks need to be executed before going to sleep / after leaving sleep.
Right now this is only implemented for periph/gpio
, but there is more to come.
This way you can make use of internal driver structures and don't have to clutter periph/pm
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will give a try.
cpu/esp32/periph/pm.c
Outdated
static const gpio_t wup_pins[] = { ESP_PM_WUP_PINS }; | ||
|
||
uint64_t wup_pin_mask = 0; | ||
for (unsigned i = 0; i < ARRAY_SIZE(wup_pins); i++) { | ||
wup_pin_mask |= 1ULL << wup_pins[i]; | ||
} | ||
#ifdef ESP_PM_WUP_LEVEL | ||
esp_sleep_enable_ext1_wakeup(wup_pin_mask, ESP_PM_WUP_LEVEL); | ||
#else /* ESP_PM_WUP_LEVEL */ | ||
esp_sleep_enable_ext1_wakeup(wup_pin_mask, ESP_EXT1_WAKEUP_ANY_HIGH); | ||
#endif /* ESP_PM_WUP_LEVEL */ | ||
#endif /* ESP_PM_WUP_PINS */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this be done in gpio_init_int()
?
The you could also drop the need of configuring the wake-up GPIOs at compile time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this be done in gpio_init_int()?
You mean for GPIOs (if they are RTC GPIOs) that are configured as GPIO_IN
could also be used as wake-up source? That's not a good idea. It must be clear to the user that GPIOs in light/deep sleep mode are handled differently than usual. GPIOs cannot be controlled individually in sleep modes. A wake-up occurs when either all GPIOs become LOW (ESP_EXT1_WAKEUP_ALL_LOW
) or any GPIO becomes HIGH (ESP_EXT1_WAKEUP_ANY_HIGH
).
For example, if you want to wake up by a LOW active interrupt line, ESP_EXT1_WAKEUP_ALL_LOW
would have to be selected as wake-up level. Then you would have to make sure that all other GPIOs configured as GPIO_IN
are already LOW. This is hardly controllable. Using an explicit setting of the only one GPIO that is used as wake-up source is much easier in that case.
Unfortunately, the wake-up functions in ESP32 are not so sophisticated. It is something like all or nothing. Multiple GPIOs can only be used as a wake-up source if, for example, all connected interrupt lines were HIGH active, which is normally not the case.
So the actual use case will be to use one GPIO for an external interrupt line to wake up the ESP32 (one example). This can then be LOW or HIGH active.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benpicco Unfortunately I can neither find the reason nor a solution for the problem that the GPIO interrupt is triggered in addition to the RTC alarm when leaving the sleep mode the first time. (Since it happens only the first time, I didn't realize that during my tests).
However, in a newer version of the SDK I have found a way to use GPIOs configured as input with interrupts directly as wakeup source. After merging the changes from new SDK version in the SDK version used by RIOT, I can use GPIOs directly as interrupt source 😄
Unfortunately only level interrupts and no edge interrupts are supported. What a bummer. 😟 I'm stumped. Ok, we could try to save the interrupt type of all GPIOs and map the edge interrupts to level interrupts before entering sleep mode and change them back when returning from the sleep mode. However, a unique mapping is difficult. Normally you would map GPIO_RISING
to GPIO_HIGH
and GPIO_FALLING
to GPIO_LOW
if it's only for waking up. Unfortunately, this does not work with our button in the tests/periph_pm
, because GPIO_RISING
is used, but the ESP32 button is LOW active, so it generates an interrupt when it is released and the mapping above would lead to GPIO_HIGH
as wakeup level.
Interesting. It uses an external RTC DS3231 because the internal one is not an RTC but only a RTT. BTW, I am thinking about removing the ESP32 RTC implementation and adding an RTT implementation. The RTC could than be realized with your RTT based RTC implementation. It is a good example how GPIOs are used in deep sleep here. All buttons are pulled down and drawn to HIGH when pressed. The deep sleep configuration would then be
Other GPIOs like GPIO4, GPIO16, ... are configured as inputs but not used as wake-up source when in deep sleep. One further remark. Unfortunately, it will probably never be possible to use BT on the ESP32 in RIOT. The BT stack of the SDK is very complex. It uses the underlying FreeRTOS very intensively and interacts with the hardware at a very low level that we will never be able to realize. Our FreeRTOS adaptation layer will never be able to provide all the required FreeRTOS features, also because FreeRTOS is tick oriented, while RIOT is tickless. |
3b7c530
to
0ccf4d3
Compare
@benpicco I have pushed some further small changes. The background is that I'm working on changing the ESP8266 port for an SDK version update from 3.1 to 3.3 which will also support WPA2 Enterprise and light/deep sleep. The last changes were mainly changes that will allow to use the same Should I squash the last changes to make them better readable? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, tests/periph_pm
works as expected and WiFi is stable too.
Just a few comments.
Feel free to squash.
cpu/esp32/periph/gpio.c
Outdated
#ifdef ESP_PM_WUP_LEVEL | ||
esp_sleep_enable_ext1_wakeup(wup_pin_mask, ESP_PM_WUP_LEVEL); | ||
#else /* ESP_PM_WUP_LEVEL */ | ||
esp_sleep_enable_ext1_wakeup(wup_pin_mask, ESP_EXT1_WAKEUP_ANY_HIGH); | ||
#endif /* ESP_PM_WUP_LEVEL */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A
#ifndef ESP_PM_WUP_LEVEL
#define ESP_PM_WUP_LEVEL ESP_EXT1_WAKEUP_ANY_HIGH
#endif
would make this less cluttered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also use these SDK defined macros in documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really mind, although there is an advantage in spelling it out:
ESP_EXT1_WAKEUP_ALL_LOW
: wake up when all selected GPIOs are lowESP_EXT1_WAKEUP_ANY_HIGH
: wake up when any of the selected GPIOs is high
The function were used to set the GPIO pads in sleep mode. This function isn't required for deep or light sleep.
`rtc_init` is used after light sleep to update the system time from RTC timer. The fix corrects a small difference of about 230 ms which would sum up with each wakeup from light sleep.
Commit 04bbca3 refines the documentation a bit. Commit f744cb0 fixes a small problem in the handling of events
The problem occured with commit 8cf397a
The multiple disconnects and the failing reconnect are wrong. |
f744cb0
to
3eb9062
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are good here.
All remaining comments are addressed.
The #ifdef
in vTaskDelay()
is unfortunate, but a #ifdef MODULE_XTIMER
could lead to hidden bugs, so this solution is OK.
Please squash.
Now, where the vendor files for light/deep sleep mode are added, function `pm_off` does not need to implement this mode by itself. Instead the existing deep sleep with disabled wakeup sources is used for pm_off.
The WiFi interface should be stopped before reboot or sleep. But stopping the WiFi interface disconnects an existing connection. Usually, esp_wifi_netdev tries to reconnect on an disconnect event. However, trying reconnect with a stopped WiFi interface may lead to a crash. Therefore, the stop event has to be handled.
1d88a79
to
4f97731
Compare
@benpicco Squashed. But Murdock shows number of compilation errors that are definitely unrelated to this PR. compilation of
|
Yea Murdock failed to fetch a pkg from Github - let's hope it's successful this time. |
Contribution description
This PR adds support for EPS32's light and deep sleep power modes and make them available by
pm_layered
.The power management of ESP32 supports now the following operating modes:
Since the peripherals are not working during light-sleep and deep-sleep, the CPU cannot be woken up by internal interrupt sources such as timers. Therefore, RIOT's layered power management can't select them as idle mode. The application has to select them explicitly using the
pm_set
function.To exit from light-sleep, the following wake-up sources can be used:
rtc_set
before callingpm_set
)ESP_PM_WUP_UART0
andESP_PM_WUP_UART1
)Please note Since only level interrupts are supported in light-sleep mode, defined edge interrupts of type
GPIO_RISING
andGPIO_FALLING
are implicitly mapped toGPIO_HIGH
andGPIO_LOW
, respectively, when entering Light-sleep mode.To exit from deep-sleep, only RTC resources can be used as wake-up sources:
rtc_set
before callingpm_set
)ESP_PM_WUP_PINS
andESP_PM_WUP_LEVEL
)In Deep-sleep mode the SRAM is powered down. However, the slow RTC memory can be retained. Therefore, data that must be retained during Deep-sleep and the subsequent system restart have to be stored in the slow RTC memory. For that purpose, use
__attribute__((section(".rtc.bss")))
to place uninitialized data in section.rtc.bss
, and__attribute__((section(".rtc.data")))
to place initialized data in section.rtc.data
.Testing procedure
Use a ESP32 board with a button at GPIO0 (usually default) and flash
tests/periph_pm
, for example:This configures the RxD signal of UART0 as wake-up source from light-sleep and the button at GPIO0 as wake-up source from light and deep sleep.
Connect the ESP32 board via a USB multimeter and perform the following tests while observing the values on the USB multimeter. (Please note: Although the ESP32 consumes only about 800 uA in light sleep mode and only 10 uA in deep sleep mode, the USB multimeter can measure values between 10 mA and 34 mA. The reason for this is that there are a number of on-board components, such as the USB2UART bridge, that generate a base load. For the test it is sufficient that in light sleep mode approx. 21 mA less is consumed and in deep sleep approx. 22 mA less than in normal operating mode.)
Unblock the light-sleep (1) mode for 5 seconds:
ESP32 should go into light sleep mode (1) for 5 seconds and then continue working correctly.
Unblock the deep-sleep mode (0) for 15 seconds and afterwards unblock the light-sleep (1) mode:
Once the light-sleep mode (1) is unblocked by the second command, ESP32 should go into the deep-sleep mode for the remaining time. The system should restart with cause
rst:0x5 (DEEPSLEEP_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
after that.Set light-sleep mode (1), wait a bit and press Enter at the console
The system should return from light-sleep once Enter was pressed.
Set deep-sleep mode (0), wait a bit and press the button at GPIO0.
The system should restart with cause
rst:0x5 (DEEPSLEEP_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
once the button was pressed.Issues/PRs references
Fixes issue #13365
PR #13417 should be merged before this PR to ensure consistent documentation.Depends on PR #13655