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

cpu/stm32: add pm support in uart, spi and i2c (f4) #7787

Merged
merged 4 commits into from
Mar 6, 2018

Conversation

vincent-d
Copy link
Member

This add pm_layered-based power management in uart, spi and i2c (f4 only so far though).

@vincent-d vincent-d added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 23, 2017
* @name Power modes
* @{
*/
#define PM_STOP (1U)
Copy link
Member

Choose a reason for hiding this comment

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

/cpu/stm32_common/periph/pm.c should use these defines in pm_set.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@vincent-d vincent-d added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: drivers Area: Device drivers Area: pm Area: (Low) power management labels Nov 9, 2017
@vincent-d
Copy link
Member Author

Changed uart pm so that stop mode is blocked only if rx is used

@olegart
Copy link
Contributor

olegart commented Jan 24, 2018

I don't quite understand the concept.

E.g. we have sys/shell enabled — so it will block all power management until we manually disable corresponding UART every time before we go to sleep and reenable it right after wakeup?

Seems totally meaningless to me. Why anyone needs it? If you need shell access all the time, just disable PM at all.

@roberthartung
Copy link
Member

roberthartung commented Jan 24, 2018

@olegart a program with shell will never sleep, that's correct. Except if you have a low-power UART.

@kaspar030
Copy link
Contributor

I don't quite understand the concept.

I think you do. The concept is just not fully implemented, thus you're right, on most (if not all) platforms, simple UART RX will block most low power modes.

@olegart
Copy link
Contributor

olegart commented Jan 24, 2018

Yes, but why?

If I need permanent shell access, I'd rather disable LPM at all, no difference with current RIOT logic. If I need sleepy device with occasional shell access (e.g. we have devices with 15 seconds of regular shell right after boot), I'll disable/enable LPM when I need that access.

With proposed changes — nothing actually changes for us in the first case, and the second case just doesn't work anymore as I can't just disable UART peripheral currently in use by the shell.

@olegart
Copy link
Contributor

olegart commented Jan 24, 2018

Long story short,

  1. without layered PM — I can use shell (UART) whenever I want to by disabling LPM and I can have some sleep whenever I want to by enabling LPM

  2. with layered PM — I can't use shell and LPM on the same system, period.

That's not an added functionality, that's reduced functionality.

@kaspar030
Copy link
Contributor

we have devices with 15 seconds of regular shell right after boot

You can probably still do that. How's the timeout currently implemented?

@kaspar030
Copy link
Contributor

with layered PM — I can't use shell and LPM on the same system, period.

That is not correct.

@olegart
Copy link
Contributor

olegart commented Jan 24, 2018

How's the timeout currently implemented?

RTC timer setting pm_prevent_sleep global variable to 0 after 15 seconds. PM checks the variable before switching to STOP mode.

You can probably still do that.

No. There's no option to disable shell, shell_run function is a one-way ticket. If I power down UART port used by it, shell will hang on the very next attempt to TX anything.

@kaspar030
Copy link
Contributor

There's no option to disable shell, shell_run function is a one-way ticket. If I power down UART port used by it, shell will hang on the very next attempt to TX anything.

Let's please not mix up concepts and implementation.

Your use case could probably be implemented in uart_stdio (e.g., by adding uart_stdio_enable/disable_rx()), which would reinitialize the underlying uart with/without rx callback. Or in periph/uart, by offering a function to set/clear the rx uart independently from complete (re-)initialization.

You could also just unblock the corresponding powermode on your board once, after your timeout. Or add an option to shell to exit after a certain time.

The implementation of pm_layered is currently incomplete, but the general concept seems to map to most use-cases, on most hardware, so far.

@roberthartung
Copy link
Member

See also: #7947

@kaspar030
Copy link
Contributor

Please squash!

@vincent-d vincent-d force-pushed the pr/stm32_pm_periph branch from a2dc72e to 857b44a Compare March 6, 2018 14:14
@vincent-d
Copy link
Member Author

rebased and squashed

Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

@kaspar030
Copy link
Contributor

kaspar030 commented Mar 6, 2018

Due to the default pm_blocker, problems won't show up for a while... ;)

@kaspar030 kaspar030 merged commit 7f150d6 into RIOT-OS:master Mar 6, 2018
@toonst toonst deleted the pr/stm32_pm_periph branch August 21, 2018 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: pm Area: (Low) power management CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants