WARNING: This PR can easily break stuff... So the more testing the better. But testing is easy and fast :-)

@kenrabold: Mind to test this on the hifive1?
@gschorcht: Mind to test this on the ESP family?
@kYc0o: Mind to test this for some ATmega boards? (The ATmega boards had consistent stdio initialization, so if it works for one it should theoretically work for all...)
@MrKevinWeiss / @dylad / @kaspar030: Mind to test for your favorite ARM board?

Oh, the msp430 family also needs testing, of course...

Being honest, the impact is actually minor, but the chance to break stuff is major. So I still felt to better set the "Impact: major" label in order to ring some alarm bells :-)

I suggest to start with the testing once I managed to fix all compilation issues Murdock uncovers. I would also kindly ask to delay reviewing the code until that, so that I can squash/force-push right away. I'll leave a comment once I'm done and will stop un-authorized squashing/force pushing then.

Can do!

Done! Compilation errors fixed and squashed. Reviewing & testing can start now :-)

I ran all tests (release specs compile_and_test_for_board.py) on the nucleo-f103rb/f401re/f303re. The only tests that failed are likely not due to this PR

nucleo-f103rb
Failures during test:
- [tests/driver_grove_ledbar](tests/driver_grove_ledbar/test.failed)
- [tests/driver_my9221](tests/driver_my9221/test.failed)
- [tests/gnrc_ipv6_ext](tests/gnrc_ipv6_ext/test.failed)
- [tests/gnrc_rpl_srh](tests/gnrc_rpl_srh/test.failed)
- [tests/pkg_fatfs_vfs](tests/pkg_fatfs_vfs/test.failed)
- [tests/xtimer_periodic_wakeup](tests/xtimer_periodic_wakeup/test.failed)
- [tests/xtimer_usleep_short](tests/xtimer_usleep_short/test.failed)

nucleo-f401re
Failures during test:
- [tests/driver_grove_ledbar](tests/driver_grove_ledbar/test.failed)
- [tests/driver_my9221](tests/driver_my9221/test.failed)
- [tests/gnrc_ipv6_ext](tests/gnrc_ipv6_ext/test.failed)
- [tests/gnrc_rpl_srh](tests/gnrc_rpl_srh/test.failed)
- [tests/pkg_fatfs_vfs](tests/pkg_fatfs_vfs/test.failed)

nucleo-f303re
Failures during test:
- [tests/driver_grove_ledbar](tests/driver_grove_ledbar/test.failed)
- [tests/driver_my9221](tests/driver_my9221/test.failed)
- [tests/gnrc_ipv6_ext](tests/gnrc_ipv6_ext/test.failed)
- [tests/gnrc_rpl_srh](tests/gnrc_rpl_srh/test.failed)
- [tests/pkg_fatfs_vfs](tests/pkg_fatfs_vfs/test.failed)

@gschorcht @MrKevinWeiss: Thanks for the testing! :-)

The PR as is breaks the HiFive1 board. The code for the board was never setup to call periph_init(), so the modifications stop all stdio output. Easy fix however. Similar to the msba2 mods, you can add the periph_init() call in board.c for HiFive1:

    /* Initialize newlib-nano library stubs */
    nanostubs_init();

    /* Initialize peripherals */
    periph_init();
}

(and the #include "periph/init.h" )

You will also need to update the Makefile.include in boards\hifive1 to include the periph modules:

# Export the peripheral drivers to be linked into the final binary:
export USEMODULE += periph

# include common periph code
export USEMODULE += periph_common

With #10825 merged, this PR now works also on the HiFive1.

May I rebase against current master to fix the conflicts. (This would also make re-testing on the HiFive1 easier, as it than should work without needing to cherry-pick #10825)

I will just interpret the absence of rejections as an implicit agreement that I may rebase to solve the conflicts :-)

I did the rebase. Please test again. (Hopefully now the HiFive1 will also work :-))

- [tests/driver_grove_ledbar](tests/driver_grove_ledbar/test.failed)
- [tests/driver_hd44780](tests/driver_hd44780/test.failed)
- [tests/driver_my9221](tests/driver_my9221/test.failed)
- [tests/gnrc_ipv6_ext](tests/gnrc_ipv6_ext/test.failed)
- [tests/gnrc_rpl_srh](tests/gnrc_rpl_srh/test.failed)
- [tests/pkg_fatfs_vfs](tests/pkg_fatfs_vfs/test.failed)
- [tests/pthread_rwlock](tests/pthread_rwlock/test.failed)
- [tests/xtimer_periodic_wakeup](tests/xtimer_periodic_wakeup/test.failed)
- [tests/xtimer_usleep_short](tests/xtimer_usleep_short/test.failed)

The failing tests on the nucleo-f103rb, still don't think any problems caused by this PR.

- [tests/driver_grove_ledbar](tests/driver_grove_ledbar/test.failed)
- [tests/driver_hd44780](tests/driver_hd44780/test.failed)
- [tests/driver_my9221](tests/driver_my9221/test.failed)
- [tests/gnrc_ipv6_ext](tests/gnrc_ipv6_ext/test.failed)
- [tests/gnrc_rpl_srh](tests/gnrc_rpl_srh/test.failed)
- [tests/pkg_fatfs_vfs](tests/pkg_fatfs_vfs/test.failed)

Seems OK for the nucleo-f303re

@francois-berder: Thanks for pointing that out. Looks like a failed assertion. Maybe you can recompile with CFLAGS += DEBUG_ASSERT_VERBOSE added to your Makefile and test again to get more output?

So I finally found why it was not working on the WiFire board (and Clicker as well). By default, STDIO_UART_DEV is set to UART_DEV(0). Since UART_DEV is not redefined, this means that STDIO_UART_DEV has value 0 and the call to uart_init(STDIO_UART_DEV, ...) triggers the assertion in cpu/mips_pic32_common/periph/uart.c:51 because, on PIC32MX and PIC32MZ, UART peripherals indexing start from 1 (as I think all other peripherals).

Here are the patches for pic32-wifire and pic32-clicker boards (I have tested this PR on both boards): mips-patches.zip

@francois-berder: Thanks for identifying and fixing the problem. I added your patches to this PR

@kenrabold or @pyropeter: Could one of you test again with the HiFive1 and confirm that it is now working?

@OlegHahm: I heard you have some msp430 laying around? Would you mind to give this PR a test?

Would someone mind to confirm that it works still on AVR? (I tested successfully on the Arduino Uno and Arduino Mega2560, but I'm afraid of the "Works on My Machine Syndrome".)

Btw: @haukepetersen noticed that stdio_rtt no longer works in master. So testing on the RuuviTag or the Thingy:52 is not possible anyway.

@maribu For this PR to work on HiFive1, this change is needed in the HiFive1 board_init() function:

void board_init(void)
{
    /* Initialize CPU and clocks */
    board_init_clock();
    cpu_init();

The board_init_clock() call must happen before the cpu_init(). Because periph_init() is now called by cpu_init(), and that periph_init() will call down into stdio_init() which then calls uart_init(), the UART on the HiFive1 is configured based on the power on clock, and then that gets changed by board_init_clock(). That results in the divisor for the UART not being correct and the text going to serial output is all messed up.

So, just update the board.c for the HiFive1 to be like the code above and this PR will work.

@kenrabold: Thanks for pointing that out. Can you test again to confirm it now works?

@maribu - Yes, that works

OK, I tested on the MSP430 platform myself using the wonderful FIT/IoT-Lab. It seems to work on every supported platform. May I squash?

@haukepetersen: This latest commit implements what I suggested as alternative to your feedback

I see, but it does not solve the issue for abusing the periph_comon/init.c for non-peripheral implemenations.

I see, but it does not solve the issue for abusing the periph_comon/init.c for non-peripheral implemenations.

Maybe one could view stdio as an peripheral device in a broader sense, as it is an upper layer for interacting with peripheral devices such as keyboard/terminal/serial port on a PC and UART/RTT in the context of RIOT. Those devices are definitely peripheral devices and stdio is RIOT's default interface to UART when there is a human on the other side of the UART port.

Looking into the code of stdio_init() for the UART case, it basically initializes the UART device, thus, a peripheral device. So this argument is not as weak as it appears on the first view.

Looking into the code of stdio_init() for the UART case, it basically initializes the UART device, thus, a peripheral device. So this argument is not as weak as it appears on the first view.

IMO that proves only that stdio_uart has periph/uart as a dependency. Thus initializing stdio_uart before the uart periph subsystem got a chance to be initialized (through periph_init()) is at least dangerous (or well, will lead to crashes).

I'd assume that stdio_rtt would have a similar issue. It depends on xtimer, which depends on periph_timer, which is also not initialized at this point.

I agree with @kaspar030, that just because stdio is using a periph driver, it is not a periph module on its own.

But one important point: the periph_common/init is there for initializing peripherals, that can be statically configured without taking any 'using module's' needs into account (e.g. i2c, spi, ...). 'timer' and 'uart' are a little different, as they are under full control if their users (e.g. xtimer, stdio, ...), and so these periph drivers are initialized by those modules directly, and are therefore independent from the periph_common module.

So again, we all agree, that stdio needs some extra care when it comes to initialization order. So I don't see a reason not to include its initialization in the platform specific code, especially considering the potential stdlib differences.

Side note (revisiting the discussion above): the segger_rtt mapping was not broken, it was my local python that prevented it from working. So now I can test those platforms in question again (at least when back in the office).

@kaspar030, @haukepetersen:
OK, so the bottom line is that stdio initialization should be moved into cpu_init() just before the call to periph_init(), right?

How about in addition to that we add a test to periph_init() to verify that stdio is initialized and working during automated tests? That way it could be kind of enforced that DEBUG() is safe to call at least at that point in time during boot.

OK, so the bottom line is that stdio initialization should be moved into cpu_init() just before the call to periph_init(), right?

yes from my side.

How about in addition to that we add a test to periph_init() to verify that stdio is initialized and working during automated tests? That way it could be kind of enforced that DEBUG() is safe to call at least at that point in time during boot.

Personally, I think this is overkill. Of course we need to make sure, that DEBUG does not trigger any harmful behavior (as in hardfaults etc). So IMHO DEBUG, or more precise the STDIO, should just do nothing in case STDIO is not (yet) initialized or even selected for a build.

What we might want to think about in parallel, is to add some specific, general documentation on the boot order and the 'special cases' like stdio init and similar. Also a general rule about where it is save to use DEBUG and STDIO in the boot sequence could be added.

Honestly, I'm not fully sure if a panic on DEBUG() before stdio is available is better than not having any indication that something went wrong. DEBUG() is not supposed to happen in production code. And a developer that added DEBUG() to some code and gets no output might have to look at a lot of things before tracing the issue down to not having stdio available.

But more generally: @MrKevinWeiss already implemented a safeguard in #10615 that drops UART output when used prior to initialization. But that change affects DEBUG() and production code alike and could potentially result in DEBUG() output getting lost. This PR is intended to be a kind-of follow up to make sure that this never happens.

Personally, I think this is overkill

Let me point out again that the test I suggested to add to periph_init() for automated testing is not supposed to be compiled in by default (as that is only useful for automated testing). It could be as simple as:

#ifdef TEST_STDIO_OUTPUT_ON_PERIPH_INIT
    puts("Initializing peripheral devices...");
#endif

I remembered #10615 incorrectly. It will only drop output with DEVELHELP to fix boards not booting with ENABLE_DEBUG in some periph code.

I'd like to take a step back and focus on the core problem at hand: a clear definition of the point in time in the boot sequence, from which it is safe to use STDIO. We all agree, that this should as soon as possible, to allow for using STDIO early on during boot. If this is 'standardized', IMO there is no need for additional checking code if STDIO is available or not.

So what are the challenges:

• (a) different stdio implementations have diverging dependencies on periph and other libraries (stdio_uart uses periph_uart, while stdio_rtt uses xtimer and with this periph_timer). So this is not something we can easily generalize.
• (b) depending on the used platform, there are potential some prerequisites, that have to be done before stdio can be initialized
• (c) ?

(b) is easy to solve by moving the init code into the platform specific init before periph_init().
For (a) it is not quite as trivial -> using stdio_uart one can use DEBUG in the timer driver, if using stdio_rtt one can clearly not. Calling it from periph_init does also not really solve this, right?!

I'm closing this PR now in favor of creating individual PRs per platform that move stdio_init() prior to periph_init(), ideally in cpu_init() just before periph_init().

@francois-berder: Would you mind to create such a PR for the mips platform? This PR would greatly depend on the code you contributed anyway.