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

Power management and notifications to other peripherals? #13422

Closed
jue89 opened this issue Feb 20, 2020 · 17 comments
Closed

Power management and notifications to other peripherals? #13422

jue89 opened this issue Feb 20, 2020 · 17 comments
Assignees
Labels
Area: pm Area: (Low) power management State: stale State: The issue / PR has no activity for >185 days Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: question The issue poses a question regarding usage of RIOT

Comments

@jue89
Copy link
Contributor

jue89 commented Feb 20, 2020

Description

Due to the discussion with @benpicco and PR #13421 and #13415 I thought about the design of the power management in RIOT. I'd like to share my thoughts. Maybe this could help to enhance RIOT in terms of PM.

I have the following problem:

  • A SPI device is connected to the SAMR30 CPU. It requires MOSI, SCK and CS always to be pushed or pulled. Floating pins lead to unwanted current draw of the SPI device. (External pull-up resp. pull-down resistors are not an option here ...)
  • If the SAMR30 (or SAML21) goes into STANDBY oder BACKUP mode, the SPI peripheral switches all output lines to high Z. This conflicts with the statement above. Thus, I have to mux the output pins manually to GPIO OUT before entering any power saving mode.

I am proposing some feature like this: If the PM switches the power mode, all internal peripheral driver receive some kind of notification. In this case, the SPI peripheral driver could mux the pins to GPIO_OUT if the CPU wants to get into STANDBY or BACKUP mode. If the PM switches back into any other MODE, the pins are muxed back to the SPI peripheral.

Furthermore if some kind of feature would be welcome, I would completely rework PR #13415 and get rid of the reconfiguration of GPIO-related stuff inside the power management. I would rather listen inside of the GPIO peripheral driver for PM level changes and switch clocks inside the GPIO driver.

What do you think?

If the idea is liked: How to implement those loosly-coupled notifications/hooks?

@benpicco benpicco added the Area: pm Area: (Low) power management label Feb 20, 2020
@roberthartung
Copy link
Member

@jue89 Have you looked into pm_layered? In general I like the concept we have implemented there!

@benpicco
Copy link
Contributor

Isn't pm_layered only concerned which power states are available, that is peripherals can block certain sleep modes when they do e.g. a DMA transfer.

This is the other way round: When entering a sleep mode the peripherals should be re-configured to draw less power if they are not needed.

@kaspar030
Copy link
Contributor

I am proposing some feature like this: If the PM switches the power mode, all internal peripheral driver receive some kind of notification. In this case, the SPI peripheral driver could mux the pins to GPIO_OUT if the CPU wants to get into STANDBY or BACKUP mode. If the PM switches back into any other MODE, the pins are muxed back to the SPI peripheral.

That is a very good idea that I intended to add to pm_layered. Basically, there'd be a list of callbacks that should be called before entering a low power mode and after waking up, before switching to ISR or thread.

The actual mechanism could be done more general, as a list of callbacks on any event could be used somewhere else. Maybe multiple parties are interested on a button press, or a network link up event might be interesting to both dhcp and a periodic "send away sensor data" thread.

For PM, it might be tricky to get code executed before the waking up ISR, on all platforms. On ARM, I think disabled interrupts + WFI might do it.

@jue89
Copy link
Contributor Author

jue89 commented Feb 20, 2020

@roberthartung: Yes I am aware of pm_layered. But I understand this more to be an interface for the application and hardware-independent drivers. They can block certain power states. But I don't want to block them. I want to react if I am changing power states.

@roberthartung
Copy link
Member

That's the point I have here. Who changes the power state and what is the reason for it? The beauty about the pm_layered concept is that we always chose the lowest possible PM. The user (the application) uses the driver it needs and based on that we enter LPM.

@kaspar030
Copy link
Contributor

he beauty about the pm_layered concept is that we always chose the lowest possible PM.

And that is awesome and hopefully here to stay. But some things might need to act upon those power mode changes. E.g. as in the example given by @jue89, the SPI pins should be reconfigured before going to sleep and after waking up.

@jue89
Copy link
Contributor Author

jue89 commented Feb 20, 2020

I am actually using pm_layered in my application. But pm_layered also boils down to a pm_set() call to periph_pm after it had found the lowest non-blocked power state. So I think periph_pm is the right place to put this feature. So, the proposed optimization would not just affect periph_pm but also pm_layered.

Futhermore, we could start with the sam0 CPU family without affecting any other CPU.

@roberthartung
Copy link
Member

he beauty about the pm_layered concept is that we always chose the lowest possible PM.

And that is awesome and hopefully here to stay. But some things might need to act upon those power mode changes. E.g. as in the example given by @jue89, the SPI pins should be reconfigured before going to sleep and after waking up.

Okay I understand that we need something like this now. What would have to be changed in pm_layered if we only use pm_layered for determin the lowest power state?

@jue89
Copy link
Contributor Author

jue89 commented Feb 20, 2020

Okay I understand that we need something like this now. What would have to be changed in pm_layered if we only use pm_layered for determin the lowest power state?

I would say: nothing. If this is done right, it just should work.

@benemorius
Copy link
Member

The way to do this with the current pm_layered paradigm is to block whichever mode causes pins to go high-impedance. I don't know the story with SAMR30 or SAML21 but if they're low-power conscious then this shouldn't mean blocking their ~5uA states. But perhaps they are not low-power conscious mcus.

We do support a number of non-low-power mcus and I think we should be mindful of that as we work to refine the power management API. I mean we shouldn't expect to have the same support on a low-power mcu as we do on everything else. Or equivalently, those who are using an "other" mcu are presumably not looking for the best pm_layered support, neither from the silicon nor the OS. So if an mcu does something crazy like lose RAM retention at a 100uA sleep state, I think we shouldn't base our API around that exception.

However this is surely not the only need for having such callback notifications and if that's the case then I think pm_layered is possibly the right place to put it.

It's true that pm_layered is currently only concerned with keeping track of the lowest available power mode. This would be a change from that. But I'm not sure that we can support the majority of use cases without adding such functionality.

@MichelRottleuthner
Copy link
Contributor

I also had that idea on my list of future projects, so I like the concept too.
With that we are already three people who had this on their mind so there seems to be at least some valid need and use for it.
One thing that always popped up next to it was that it could be helpful for such things to have a common struct as a "base class" for our different peripherals and devices (i.e. a device struct).
That could then be used for generic operations like "check if everyone is ready for LPM" or "force powerdown of all devs". But I guess making this optional would be a strict requirement to keep everything as slim as possible if required.

We do support a number of non-low-power mcus and I think we should be mindful of that as we work to refine the power management API.

Do you have an example for MCUs that don't have any modes that allow for reduced consumption?
Sure, not all of them can go sub-µA but I never worked with any MCU that doesn't allow for tweaking to reduce power consumption to some extent.

@jue89
Copy link
Contributor Author

jue89 commented Feb 21, 2020

I actually do not intend to completely revolutionize the driver model and power management ;)

In PR #13415 I show some kind of callbacks without touching the public PM API at all. My changes are all happening inside the SAM0 family (or currently just SAML21 ...). I would appreciate if you have a look into this approach. Code says more than 1000 words ;)

@benemorius
Copy link
Member

Do you have an example for MCUs that don't have any modes that allow for reduced consumption?

I should clarify that I'm not talking about mcus with no power modes at all. I agree those may not exist.

Sure, not all of them can go sub-µA but I never worked with any MCU that doesn't allow for tweaking to reduce power consumption to some extent.

What I intend to advocate is that in refining the power management API we pay more attention to mcus that have good power management, rather than these other mcus that support it "to some extent".

For example esp32 doesn't retain ram in the <~100uA sleep states. I don't think we should worry about that. If you're using esp32, you don't get to go super low power without resetting and that's not our fault even though we could technically make some efforts to help with that. Similarly the stm32 family has some mcus that have good low power support but it also (presumably?) has others with limitations similar to esp32 because stm32 is a very broad family and some of them just have no reason to focus on low power. Another example is these SAM devices that tristate the SPI pins in low power states which is not very great for low power support. An mcu with better low-power support doesn't do that.

If I'm trying to build some low-power widget, I'm not going to just pick any random mcu and then hope that my OS of choice works around the fact that I made a bad mcu selection. I'm going to pick a good low-power mcu and then trust that the OS will have decent support for the mcu in its intended use case. It's sort of similar to how we don't try to add software PWM to an mcu that doesn't natively support PWM even though we could.

I think the power management API should have the best support for the mcus that have the best low-power support because I think that's consistent with the majority of use cases. This will help us keep the API simple. We can always expand it in the future once it's more established.

@benemorius
Copy link
Member

I actually do not intend to completely revolutionize the driver model and power management ;)

In PR #13415 I show some kind of callbacks without touching the public PM API at all. My changes are all happening inside the SAM0 family (or currently just SAML21 ...). I would appreciate if you have a look into this approach. Code says more than 1000 words ;)

Despite all what I just said, I do actually think that we should consider this change to power management.

Although I don't have an example off the top of my head, intuitively I feel like it's going to be necessary to have application-level callbacks for entering and leaving power modes. In any case, it would certainly help bridge the gap between mcus with good low-power support and those with less good support.

As an end-user I can imagining wanting to tell the OS, "well, when X peripheral is awake anyway I'd like to go ahead and enable Y bit in Z register, but I definitely don't want it enabled while sleeping".

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: question The issue poses a question regarding usage of RIOT labels Jul 6, 2020
@stale
Copy link

stale bot commented Jan 9, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Jan 9, 2021
@aabadie
Copy link
Contributor

aabadie commented Jan 10, 2021

unstale

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Jan 10, 2021
@MrKevinWeiss MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 2, 2022
@stale stale bot closed this as completed Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: pm Area: (Low) power management State: stale State: The issue / PR has no activity for >185 days Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: question The issue poses a question regarding usage of RIOT
Projects
None yet
Development

No branches or pull requests

10 participants