Full support for entering and exiting DORMANT mode

[andrewh42]

I noticed that there were the beginnings of some support for the RP2040 dormant lowest-power sleep but that several loose ends remained.

I've taken care not to make any breaking changes in this pull request to maximise its prospects. However, I think an interesting future change would be to add an addiitional pll::State to represent a powered down PLL. This would streamline the process of restarting the PLLs following dormancy.

Looking forward to your feedback and hopefully an approved pull request. I'm keen to hear of any areas that don't meet rp-rs standards or where I've done something that isn't idiomatic - apologies in advance for any crimes against these!

Andrew

[ithinuel]

Thank you!

I had started doing things in that direction but never got to complete it 🤦.
Because the Dormant mode doesn't really need to be represented (the device does not run while in dormant mode).

I think Initialized and Dormant can be merged into a Unstable state.

You can find my WIP here: https://github.com/ithinuel/rp-hal/tree/clock_dormant_recovery

[andrewh42]

Thanks @ithinuel, that's a good suggestion. I've replaced the XOSC Initialized and Dormant states with an Unstable state. And ROSC no longer changes state on after waking.

I couldn’t see the benefit of adding StableOscillatorTokenAlreadyIssuedError however, given that await_stabilization() appears to be idempotent, and the token can only be redeemed once via get_stable(), regardless of how many times we request a token. Would you be able to help me understand what I’m missing here please?

[andrewh42]

Hi @ithinuel, is there anything I can do to help progress this PR?

[jannic]

I never used the dormant state and didn't think about it enough to decide if these changes are sufficient to make dormant mode usable. From reading the diffs I get the impression that the changes make sense, and I don't see anything obviously wrong. @ithinuel, as you already worked on your own implementation of dormant support, you are probably more qualified to review this merge request. But if you don't have time to do a review, I could plug together some test setup on a breadboard to try out the example, and approve the PR if everything works as expected.

@andrewh42, you wrote that you avoided breaking changes to make merging easier. As the next release probably will include breaking changes anyway, are there any improvements you'd want to add?

[andrewh42]

@andrewh42, you wrote that you avoided breaking changes to make merging easier. As the next release probably will include breaking changes anyway, are there any improvements you'd want to add?

Thanks for asking @jannic. I was thinking it could be useful to add an addiitional pll::State to represent a powered down PLL. At present the PLL's disable() reconstructs the Disabled state by reading the PLL's registers, so that initialize() can be used to restart the disabled PLL. The alternative I have in mind is to replace disable() with, say, stop() and return a new Stopped state that then requires less work to restart. initialize() might then return a Stopped PLL, in order to keep the code DRY. Giving us the following state transitions when starting up:

Disabled -> Stopped -> Locking -> Locked

And these transitions when stopping and then restarting (e.g. as part of going dormant):

Locked -> Stopped -> Locking -> Locked

Does that sound reasonable? Either way, I'd prefer to do this in a separate PR so this one doesn't stick around for too long and gather more complexity. Does that sound OK?

[andrewh42]

Hi @ithinuel and @jannic, is there anything I can do to help complete this PR?

[jannic]

Hi @ithinuel and @jannic, is there anything I can do to help complete this PR?

Sorry for not answering sooner. Currently, my focus is on getting 0.10.0 out the door, so I neglected some older pull requests.

Currently, there are some merge conflicts. Could you try to resolve them?

[andrewh42]

Hi @ithinuel and @jannic, is there anything I can do to help complete this PR?

Sorry for not answering sooner. Currently, my focus is on getting 0.10.0 out the door, so I neglected some older pull requests.

Currently, there are some merge conflicts. Could you try to resolve them?

No worries @jannic, done.

[jannic]

Thanks, Andrew! I think the pull request can be merged now. @ithinuel, any objections?

[jannic]

BTW I also verified that the dormant mode actually makes a difference. Even with a cheap USB power meter, I see 0.02A in wfi mode, while in dormant mode, it shows 0.00A. The absolute numbers might be very inaccurate, but it's obvious that the dormant mode is different from the wfi sleep.

[ithinuel]

I'm happy with this. We can always come back later to this if we need to make improvements.
I have a ppk2 at home so I could take some samples and post screenshots here.