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

Implement alarm-module for RP2350 #9965

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

bablokb
Copy link

@bablokb bablokb commented Jan 16, 2025

This PR implements the alarm-module for the RP2350 variant of the pico and fixes #9491.

Notes

  • The PR does not use the new power-domains for the RP2350, but uses plain old sleep/dormant modes. An implementation based on powman is something for a future PR.
  • This PR also reimplements light-sleep/deep-sleep for the RP2040. As @dhalbert stated in Third sleep option: RAM-preserving "deep" sleep #9521: "So redefining light sleep as RAM-preserving, but not activity-preserving, might be worthwhile". With this PR, light-sleep has power consumption down to deep-sleep levels but is RAM-preserving.
  • The implementation is smart enough to detect a serial connection. In this case, nothing changes to the current implementation (well, at least for the RP2040 - the behavior on RP2350 needs some more tests, but at least USB-serial does not disconnect)
  • Anybody who needs "light-sleep but keep everything running" can use what I call "slow-sleep" (see next section with results below). Which is already much better now compared to the current light-sleep implementation. This could be integrated with an option to light_sleep_until_alarms(maximize_power_save=True). But for real life use running on batteries, I cannot imagine that anybody really wants more than the absolute necessary stuff running during sleep.
  • This PR needs thorough testing, which I will continue to do (especially with peripherals and the w-variants of the boards, and with the RP2350B). Any additional tests are highly appreciated.

Light-Sleep/Deep-Sleep Current for RP2xxx

All measurements at 5V with Nordic PPKII powering via USB
(i.e. USB-serial not connected).

Notes:

  • sleep: time.sleep()
  • s-sleep ("slow sleep"): cpu.frequency = 25000000; time.sleep()
  • LS: light-sleep
  • DS: deep-sleep
  • TA: TimeAlarm
  • PA: PinAlarm

RP2040 (Pico)

type 9.2.1 9.2.2-PR
sleep 18.6mA 18.6mA
s-sleep 7.7mA 7.7mA
LS+TA 15.0mA 1.6mA
LS+PA 15.0mA 1.2mA
DS+TA 7.1mA 1.6mA
DS+PA 1.3mA 1.2mA

RP2350A (Pico2)

type 9.2.1 9.2.2-PR
sleep 15.1mA 15.1mA
s-sleep 5.7mA 5.7mA
LS+TA n.a. 1.7mA
LS+PA n.a. 1.7mA
DS+TA n.a. 1.7mA
DS+PA n.a. 1.7mA

@tannewt tannewt self-requested a review January 16, 2025 17:57
@tannewt
Copy link
Member

tannewt commented Jan 16, 2025

Thanks for doing this!

So this will disable some peripherals during light sleep now?

Is the RP2350 sleep current high because it doesn't use POWMAN to shutdown ram?

@bablokb
Copy link
Author

bablokb commented Jan 16, 2025

So this will disable some peripherals during light sleep now?

To be tested, but probably yes, due to the switch to dormant-state. For light-sleep with serial-connection, there should be no change.

Is the RP2350 sleep current high because it doesn't use POWMAN to shutdown ram?

I think so, dormant does nothing with other units than the cores.

@bablokb
Copy link
Author

bablokb commented Jan 18, 2025

Please mark this as a draft PR. The Pico-W (and probably the Pico2-W) need some more investigation. In my tests, the CYW43 did complain with messages like:

[CYW43] cyw43_kso_set(1): failed
F2 not ready
F2 not ready
[CYW43] STALL(0;129-129): timeout

There is a cyw43_enter_deep_sleep() function called from common_hal_alarm_enter_deep_sleep(), but I am missing the counterpart. Since this only sets WL_REG_ON to false, it might be simple.

Pinging @eightycc: any ideas?

@eightycc
Copy link

@bablokb The [CYW43] cyw43_kso_set(1): failed message may be occurring because simply turning off power to the WIFI radio as is done in cyw43_enter_deep_sleep() does not put the driver into a quiescent state. Similarly, properly powering the WIFI radio back up requires a sequence of operations involving an interrupt from the CYW43 that appears to be entirely missing.

I'll reproduce the failure here and get back with a more detailed analysis.

@dhalbert dhalbert marked this pull request as draft January 18, 2025 13:42
@eightycc
Copy link

The best source of technical specs for the CYW43439 that I've been able to find is the source code for Infineon's WHD driver: https://github.com/Infineon/wifi-host-driver.

The KSO (Keep SDIO On) bit managed by cyw43_kso_set() appears to affect operation only when the CYW43's interface is in SDIO mode. Why it's getting touched when the interface is in gSPI mode is a bit of a puzzler.

@eightycc
Copy link

In common_hal_alarm_set_deep_sleep_alarms() we're missing code for putting the CYW43439 into a quiescent state before pulling the plug on the WIFI radio by calling cyw43_enter_deep_sleep(). We're also missing code for bringing the CYW43439 back up after exiting deep sleep. That requires pretty much a complete re-initialization of the chip. There's no similar code in Micropython (i.e., powering down/up the WIFI radio), so we're on our own for an implementation.

The CYW43439 does expose power management controls, but the best I say for the documentation is that it's murky. There might be an alternative to pulling the plug there?

@bablokb
Copy link
Author

bablokb commented Jan 19, 2025

@eightycc thanks for your input. I will do some more analyses and tests next week. Maybe it makes more sense to put the driver into a quiescent state and leave the chip as is. Or I will try to reinitialize the chip. Although I don't really like the idea, because currently this is a lengthy process with a hard-coded delay of one whole second.

@eightycc
Copy link

Because common_hal_alarm_set_deep_sleep_alarms() with its call to cyw43_enter_deep_sleep() leaves the CYW43439 in an unrecoverable state (at least with the code as it is), I'd suggest removing the call to cyw43_enter_deep_sleep() and seeing what the power consumption hit is. Just a nit to pick, but the function as is includes a hard-coded pin number.

It may be worthwhile to look into the power management features of the chip exposed by cyw43_wifi_pm() in cyw43-driver. Presently bindings_cyw43_wifi_enforce_pm() in ports/raspberrypi/bindings/cyw43/__init__.c disables all power management features by default.

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.

Low power support for RP2350
3 participants