pm: provide way to disable uart_stdio rx

[roberthartung]

The pm_layered module is used to automatically enter power modes if they are not blocked. However, the U(S)ART is usually initialized automatically:

find . -name "board.c" | xargs grep "stdio_init"

./boards/telosb/board.c: uart_stdio_init();
./boards/waspmote-pro/board.c: uart_stdio_init();
./boards/z1/board.c: uart_stdio_init();
./boards/arduino-atmega-common/board.c: uart_stdio_init();

However, this always blocks even first sleep modes, because the uart driver will block the level. Therefore I propose to introduce a simple wrapper e.g.

#ifndef PM_DISABLE_UART
uart_stdio_init();
#endif

... or something similar that I can set in my application or pass to make in order to disable the default initialization of the uart. This will surely introduce problems when printf() calls are made,so a custom printf() function (PRINTF?) has to be used.
Are there any other ideas on how to solve this problem?
From what I observed, uart is the only thing that will currently block deep sleep capability.

I look forward to constructive discussions and propoals!

[jnohlgard]

It's usually the UART RX that will block the low power modes. In my opinion, it should be possible to disable UART STDIO RX for low power without messing with the board configurations (periph_conf.h).
Then the application will be able to define a flag in its Makefile to allow full low power modes, and disabling stdin.

Also, I'm not sure all drivers have been updated yet to conditionally block low power modes depending on the RX configuration. I have one PR open for the Kinetis CPUs to allow some UART RX functionality even with low power modes #7897 by configuring the RX pin as a wakeup source (there are some limitations to baud rate and module clock sources though), maybe you can do something similar for your platform?

[roberthartung]

@gebart ACK. That's the define/flag I mentioned above. We need an easy way to overcome this issue, where you want to deploy a node and don't care about UART anymore.

[jnohlgard]

@roberthartung I still think it's useful to have the possibility to use the UART for TX only even in low power applications, so the uart_stdio_init should still be run during boot on low power applications, only it should not enable the RX configuration if the low power flag is set. It helps tremendously with debugging to have some sort of output channel.

[dylad]

What about disabling the UART at runtime just before going to sleep and re-enable it when the device wakes up ?

[roberthartung]

@gebart Then we should name it PM_ENABLE_FULL, PM_DISABLE_UART_RX or something like that?

@dylad That's what I am doing now and it sucks. I don't want to do this for every application all over again. When deploying I just want to be able to call make BOARD=... PM_ENABLE_FULL=1 flash and be good to go. And this will just disable UART tx and rx.

However we can think about disabling the RX part and enabling tx when ever it is needed. How would we achieve this? This should be transparent to the user. So do we need to have #ifdefs or a special PM_PRINTF()?

#define PM_PRINTF(...) uart_enable_tx(); printf(__VA_ARGS); uart_disable_tx(); ? 

[roberthartung]

Additionally, this would require in separating tx and rx mode of a uart, which is somewhat wishful and we should work on that?

EDIT: I've just seen that there already is rx_cb in uart_init. So maybe extending the UART interface with uart_enable/disable_rx/tx() would do the job?

This would allow us to do uart_disable_rx() in low power mode and uart_enable/disable_tx() is used in PM_PRINFT. Additionally PM_PRINTF could be replaced by printf() or even completely left empty?

[dylad]

I'm just wondering... Could we just UNDEF both gpio within periph_conf.h file to prevent the stdio driver from enabling UART in your case ? or something like that ?

[roberthartung]

@dylad Well that's my point, it's not about my case - I have a working solution. We need a solution for all platforms in a uniform way. Additionally, I am not sure what you actually mean? UNDEF shouldn't be used anyway anymore - see #5757.

[dylad]

Sorry I was not clear in my previous post. What I really mean is the possibility to define both rx & tx pins as GPIO_UNDEF. Then we need to rewrite a bit of uart_stdio in order to check if rx & tx are defined (or defined as GPIO_UNDEF) and so enable or not this UART. This way, user just have to overwrite their periph_conf.h board file to disable this UART.

[vincent-d]

I have a protoype somewhere where the shell powers down the uart after an inactivity time, then enable the rx pin as external interrupt for wake up. When a message is received the uart is then powered back on. The first character is the missed...
To do that I had to add a function in the uart driver to turn on the rx pin as an interrupt.

The main drawback of that approach is that some UARTs can still be used in low power and do not need this hack to wake up. We might then decide to add this function (rx wake up) and do the whole process even for low power uarts, but then the power_on, power_off and rx_wake_up would be empty.

[roberthartung]

@dylad I am not sure how that should work by just defining the Pins? Can you Sketch something? Because the uart driver has to be separated for TX and RX, i.e. not enabling both TX and RX mode at the same time.

@vincent-d low power uarts have been discussed as Dagstuhl this year already. We really should also think about how to integrate these. Maybe a lpusart interface is required? TBD.
This might introduce more complexity, but will help to handle this case clean?
My main focus is on actual devices though. I am not sure that a shell application should be powered down - It's a shell app!! - so its intent is to use UART ;-)
So I am aiming at actual deployments where you don't need to have uart communication.

In my uart-based tests, I just disable/enable uart if it is not needed, which allows to enter deep sleep modes.

[roberthartung]

@dylad Ok. I see what you aim for - that might work. Will have to look into it. What happens if my program still contains printf though? This surely has to be thought through.

[photonthunder]

For my application it is very useful to leave the stdio/debug on even in deep sleep mode, that way you can log a suspect node's output. Our current implementation is to put the printf output into a buffer and then wait for the buffer to empty before sleeping. If you implemented a buffer approach, with a minimal printf (which would be a very nice feature), then you could just let the buffer fill if output is off?

[roberthartung]

well - I'd use puts() for that, not printf.. probably - not sure. I am looking more into long-term WSN applications where you deploy your network and it does its thing. There's nothing you can log things/errors to - it just has to work!

But missing a character is obsiously not acceptable @vincent-d - IMO we should only allow turning on/off the uart if we can make sure we're not missing any data! If we do -> no deep sleep available!

[roberthartung]

For my application it is very useful to leave the stdio/debug on even in deep sleep mode, that way you can log a suspect node's output. Our current implementation is to put the printf output into a buffer and then wait for the buffer to empty before sleeping. If you implemented a buffer approach, with a minimal printf (which would be a very nice feature), then you could just let the buffer fill if output is off?

Well then you can just do uart_off() at the beginning and uart_on()/uart_off() on the specific debug part in your code?

[photonthunder]

Good point, but I would prefer if these kind of features (minimal printf/buffered) could be incorporated into RIOT.

[vincent-d]

But missing a character is obsiously not acceptable @vincent-d - IMO we should only allow turning on/off the uart if we can make sure we're not missing any data! If we do -> no deep sleep available!

I agree, but all my UARTs can be disabled without losing character (I have a wake up pin from the other side) except the shell, which I use only for debug purpose. But I still need to allow low power. So I tried it in a hacky way.

[jnohlgard]

I see a number of things that needs improving in this area:

• Make sure that all uart peripheral drivers follow the documented behaviour that if the rx_cb parameter to uart_init is NULL, then the UART will be TX only.
• Only block power modes when necessary, e.g. don't block power modes in TX only, and don't block power modes if the UART supports waking from sleep on the RX pin (very much platform specific condition).
• uart_stdio needs a configuration option that disables RX or TX or both. No need to touch any printf or puts or whatever else in the code because everything still boils down to a uart_write call in the end.

@vincent-d The Kinetis solution is similar to what you describe regarding your (stm32 I guess?) solution, except there are no new functions, everything is handled privately inside the UART driver implementation. I have the same problem with the first character being messed up sometimes because the clock source for the UART does not start up quickly enough, but it depends on which clock source is selected for the module, the worst problems are when using a PLL for the clocking.
A mostly successful workaround is to type a bunch of spaces before each shell command which lets the device power on and miss some chars without missing the actual command. Another workaround is to reduce the UART baud rate to something really low, which sucks when the uart_write is using a busy spin wait for shuffling the bytes over the wire.

@photonthunder One level of buffering is already implemented inside newlib, which uses line buffering and a buffer of I think 64 bytes by default, but I assume you want something a little more configurable here?

[photonthunder]

@gebart, is there a way to keep the newlib implementation awake until empty (a flag or something)? Typically I just set up a ring buffer in uart.c and then just have a thread that keeps things awake until empty.

[dylad]

@roberthartung as @gebart said, the easiest way is to put a guard on driver side to ensure tx pin is enable or not (depends of the pin definition within periph_conf.h). Some drivers (not all I think) already check the behaviour of the rx pin before initialize the reception side.
Then you just have to make a PR for the patch on the UART driver for your platform and modify your periph_conf.h regarding your application to enable or not this UART.

[roberthartung]

@dylad that‘s the Point!! The Board is independent of the application! Changing the board‘s periph_conf.h for the applications is wrong! We need a solution on application basis.

[dylad]

Changing the board‘s periph_conf.h for the applications is wrong!

Then we have to do it either at runtime (need to write new function to control it) or use some flags.
I don't know what would be better...
Another solution would be to call uart_poweroff at the beginning of the application to disable hardware AND add a guard on UART driver to check if UART is hardware-enabled within the uart_write function. What do you think ?

[jnohlgard]

I agree that it should be possible to disable it per application without modifying the board periph_conf.h file. It could be implemented as simply as adding a preprocessor condition to set the RX callback to null during init if some flag is set, that way it will be possible to override from the project Makefile (CFLAGS+=-DUART_STDIN_DISABLE or similar).

[roberthartung]

@dylad Runtime always comes with an additional cost (more energy consumption?). Adding a guard has the same problem but that's the trade off you need to take, if you want to be flexible - I guess.
Therefore I think we need both. As @gebart we should think of adding a Makefile flag, which would then disable the UART completely, both for online stdin, stdout, or both! And you cannot turn it on in runtime.

The problem is that the board by default uses the uart module, right? so we cannot shift this to the application doing FEATURE_REQUIRED += periph_usart which then compiles the periph module and adds printf() calls, e.g. we have to pay attention to any printfs that might occur, e.g. replacing it with PRINTF? I don't know. Or is this not a problem? Are there any printf calls in the core RIOT code? probably not?

And for the runtime I'd vote for a guard, indicating the state of uart tx/tx modes (can we just read the register bit flags here?) and abort uart_writechar if tx is not enabled!
When you call uart_poweroff, the flag is set/unset (or simply rx/tx is deinitialized), and then it will be unblocked accordingly.
Nevertheless we need a separation for uart_poweroff_tx and uart_poweroff_rx, do you agree?

[jnohlgard]

You don't need to change anything with printf, without enable_debug there are almost no printf calls in the OS (shell does print something always). All printf and puts and other stdout writes will end up in the write syscall, which in turn calls uart_stdio_write, so all you need to do to disable your uart is to make the uart_stdio_write call return without actually writing anything. The printf call overhead for formatting the string will not be noticeable on the overall power consumption unless you are doing something exceptional like printing large arrays of numeric values in decimal, and then you should be able to disable that part of your application if you are building for lowest power consumption.

[kaspar030]

Thanks for bringing this up! Apart from always blocking the low-power mode, uart_stdio also uses quite some memory and code.

Are there any other ideas on how to solve this problem?

How about a pseudomodule uart_stdio_rx that is selected by default, but can be disabled. In uart_stdio we'd setup rx based on that module.

I'll come up with a PR.

[roberthartung]

@kaspar030 How about doing both uart_stdio_rx and uart_stdio_tx which are not selected by default and applications have to choose it explicitely? Or on the other hand, it can be selected by default, but being disabled.
BTW: How would you disable such a module?
EDIT: Still, this would be a solution for deploying. Then we'd need to have a runtime solution.

[dylad]

@kaspar030 Why only RX ? We should be able to disable TX too.

[kaspar030]

@kaspar030 Why only RX ? We should be able to disable TX too.

Is there a use-case for having only RX but not TX?

[kaspar030]

BTW: How would you disable such a module?

We could make it a DEFAULT_MODULE. Then it can be disabled usind DISABLE_MODULE.

[dylad]

Is there a use-case for having only RX but not TX?

This is not what I mean, I was thinking of disabling both RX and TX at the same time. So user can disable RX only or both RX and TX.

[roberthartung]

I agree, I don't see a use-case for only having RX, but not TX for now. See my comments in #7974 .

[vincent-d]

Being able to disable rx for stdio is one step forward, but I still think we need a consensus on how to handle low power with uart rx.
There is a lot of use cases where you could want to wake up from uart rx and loosing one character would be acceptable. But there is no common solution for that so far.

[roberthartung]

loosing one character would be acceptable

I can't agree on this one. We know that UART has bit errors, but accepting to loose a character in this use cases, when we're in low power modes / waking up, is not acceptable in my opinion and RIOT should not support is. Therefore I suggest, that we don't allow sleeping when UART is still in RX mode.
However, if you want RX in sleep modes, you should choose a platform that has a low power UART. Ideas, comments?

Apart from that I agree to @vincent-d that it is nice to be able to disable rx/tx at compile time, however disabling/enabling it at runtime also is important and this should be able for low-power UARTs and non-low-power UARTs.

[vincent-d]

@roberthartung I worked with different microcontrollers for an old project and none of them had low power UART. The wake up was done by sending a dummy character first. Maybe this is a bad design, but sometimes you don't have a choice.
I still believe this mode is usefull, but I agree it should not be the default implementation. But I don't see any way of having a common api/behavior for all kind of UARTs then.

[roberthartung]

@vincent-d Not saying that there should be a common API I guess, the API has to be rethinked on how to integrate LPM UARTs.
Additionally, that's what I am saying, it's bad practice to send a dummy, if you require very low power and send an additional character, this can matter in terms of energy consumption ... maybe ;) So not making it default is a good idea! But maybe you have a choice: but new hardware if you need deep sleep and wakeup from uart ;-) or use a pin to wakeup? not sure ... I can see the argument

[vincent-d]

Sometimes software design need to be adapted to bad hardware...and changing hardware is harder than changing software ;)

[jnohlgard]

I agree with @vincent-d, sometimes it is better to lose one char when waking up and still be able to use the UART, than the alternatives (blocking low power, or no UART RX at all).
There's a ton of different clock settings and low power modes on most modern MCUs, which means that there probably won't be a one size fits all solution. Some configurations may make the low power recovery time too long to be able to receive the first char, while other settings may wake up quickly enough to catch everything even with a slight phase shift when sampling the input signal.

[jnohlgard]

I would like to propose a different naming for the pseudomodules: uart_stdin and uart_stdout. Shorter to type and should be fairly obvious to most developers.

[kYc0o]

Is this still an issue with #11310 merged?

[aabadie]

Is this still an issue with #11310 merged?

No, so let's close this. Feel free to reopen if you disagree.