Hook function before main() and after RTOS initialisation

[RobMeades]

Description

• Type: Enhancement
• Related issue: C030 boards fail new platform critical section test  #5995
• Priority: Blocker

The UBLOX_C030 series targets require that a voltage level in a chip on the board is configured during boot for reliable operation. This configuration has to be performed over I2C. The [ST standard] I2C drivers used on this target make calls to wait_us() during I2C read and write requests; this is a sensible thing to do since the RTOS can run other tasks while waiting. However, since wait_us() is an RTOS call, this means that the I2C [HAL] API cannot be called from HAL_MspInit(), the target configuration hook function, as the RTOS is not initialised at that point.

Hence we need a hook that can be used by target code after RTOS initialisation. I think that this will be a generally useful feature to have as there will be other cases where targets need configuration of items that are not as simple as a register.

FYI, we proposed using mbed_main() for this purpose in #6023 but we now understand that this is for user code and not for target code.

[0xc0170]

From the docs, an example for ARMCC:

 * Reset (TARGET)
 *     -> SystemInit (TARGET)
 *     -> __main (LIBC)
 *         -> __rt_entry (MBED: rtos/mbed_boot.c)
 *             -> __user_setup_stackheap (LIBC)
 *             -> mbed_set_stack_heap (MBED: rtos/mbed_boot.c)
 *             -> mbed_cpy_nvic (MBED: rtos/mbed_boot.c)
 *             -> mbed_sdk_init (TARGET)
 *             -> _platform_post_stackheap_init (RTX)
 *                 -> osKernelInitialize (RTX)
 *             -> mbed_start_main (MBED: rtos/mbed_boot.c)
 *                 -> osThreadNew (RTX)
 *                     -> pre_main(MBED: rtos/mbed_boot.c)
 *                         -> __rt_lib_init (LIBC)
 *                         -> $Sub$$main (MBED: rtos/mbed_boot.c)
 *                             -> mbed_main (MBED: rtos/mbed_boot.c)
 *                             -> main (APP)
 *                 -> osKernelStart (RTX)

There is couple of hooks, but as stated - targets hooks are invoked prior any rtos start or even libc (that is intentional as prior any libc calls, target should initialize what is required). In your case, however this has dependency on higher level functionality that has not yet been initialized.

Can this dependency be removed? Where is wait_us() used? I checked i2c_api.c file for STM targets, I can see waiting for flags with simple timeout counter.

@bulislaw @c1728p9 please review

[RobMeades]

The relevant lines in the i2c_api.c are in the i2c_read() and i2c_write() functions. We could ask ST to make a change here but it seems a sensible thing to do to me, since I2C is relatively slow, to let the RTOS in.

I can see that there must be target initialisation before the RTOS is started but I can also see a case for there being target initialisation code that requires RTOS functions; it is clearly permissible for a HW driver to make RTOS calls (wait_ms() and wait_us() are certainly used) and boot code might need to make use of drivers to talk to the HW, as we do.

The other alternative would be for the waitxx() function calls to behave well even before RTOS initialisation but that would then be a special case and might lead to unnecessary complexity in what should be simple functions.

[0xc0170]

Thanks for the pointers. This is in HAL, and those API are blocking, either block on some flag, or timeout or similar.

Use of wait API there is not disallowed until we hit a problem like this. I would suggest to use us ticker API to wait (simple wrapper to be implemented).

cc @ARMmbed/team-st-mcd

[LMESTM]

@0xc0170 @RobMeades
So are you saying that from now on, wait API is forbidden from HAL layer ? Or a case by case situation ?
I'm fine with removing the wait_us from the driver and find an alternative

[RobMeades]

Let's wait for @bulislaw and @c1728p9 to comment. I think the best user experience would be if the I2C HAL kept the wait_us() calls and a target hook were added post RTOS initialisation and before main() for purposes like ours, but there may be reasons why that is not a good idea.

[bulislaw]

I would like to avoid RTOS in HAL, as there is plenty of good use cases for system without RTOS or even HAL alone. But then we provide RTOS-less wait_us() but only if RTOS is not there, in this case ofc that's not the case. For blocking i2c_read/write() the wait_us() is not a problem even without RTOS.

As for entry points to mbed_main.c and the whole file, it calls for a major refactoring. I think the mbed_main() is quite pointless for the application as it's basically the same as main(). On the other hand there's no pre-RTOS entry point for the apps and there's no post-RTOS entry point for targets. I would be ok with adding another entry point for target just after RTOS has started.

@0xc0170 @c1728p9 any comments?

[0xc0170]

I would like to avoid RTOS in HAL, as there is plenty of good use cases for system without RTOS or even HAL alone. But then we provide RTOS-less wait_us() but only if RTOS is not there, in this case ofc that's not the case. For blocking i2c_read/write() the wait_us() is not a problem even without RTOS.

👍

If we can refactor that i2c , not to use wait_us(), but rather ticker API to get this blocking wait ? or even us ticker directly. In most cases the blocking wait is short.

I'm fine with removing the wait_us from the driver and find an alternative

That would be good to unblock the PR that @RobMeades referenced above.

As for entry points to mbed_main.c and the whole file, it calls for a major refactoring. I think the mbed_main() is quite pointless for the application as it's basically the same as main(). On the other hand there's no pre-RTOS entry point for the apps and there's no post-RTOS entry point for targets. I would be ok with adding another entry point for target just after RTOS has started.

I haven't seen mbed_main used.
Not certain for uses cases preRTOS/afterRTOS for targets. a target shall be RTOS agnostic.

[c1728p9]

Hi @RobMeades at least in C++ you should be able to call a function in global scope to get it to run alongside global object constructors. This would allow you to run the code you need after the RAM and the RTOS have been initialized but before main. Something like:

void my_boot_setup()
{
// configure voltage level
}
my_boot_setup();

[RobMeades]

@c1728p9: interesting. So if we created a .cpp file with the above in it and dumped that into our targets directory, would C++ link it and run it during LIBC initialisation? Like magic :-).

[pan-]

@c1728p9 That's not legal C++ code. Functions can be run before main only if they return an object and this object is captured by a global or static variable.

@RobMeades Note that there is no guarantee on initialization order: Another constructor may do undesired operations before the voltage has been set.

[RobMeades]

Hehe, it was a nice idea though. Anyway, it seems that, though there may be a case for such a hook function or for the refactoring of hook functions, the way forward in this particular case is to deal with the issue in hand and switch the wait_us() calls in the I2S driver for a us_ticker() wait instead.

@ARMmbed/team-st-mcd: do you agree and, if so, do you wish to do the change or would you like us to do so?

[LMESTM]

@RobMeades If that's ok with you, please feel free to do the change and validate that it solves your issue at the same time. We'll then run a non regression on I2C on your PR.

Note that another alternative would be wait_us to rely on non-RTOS implementation until RTOS has started :-)

I actually started looking at void wait_us(int us) in mbed_wait_api_rtos.cpp - is this the implementation we talk about?
https://github.com/ARMmbed/mbed-os/blob/master/platform/mbed_wait_api_rtos.cpp#L41

It's not clear to me why this actually fails here, as I2C driver calls to wait_us(1); we're below 1 ms and should not call OS services ... anyone could explain ?