Timer: fix to init in non-critical context

[lrusinowicz]

…dware timer in non-critical context.
This benefits target platforms having multiple MCUs/cores but shared other hardware resources - in this case timers.
Probably the first example of such platform is Cypress PSoC 6 MCU on Future Electronics Sequana board (upcoming new target). Implementation of hardware resource manager on this platform prevents initialization of hardware resources on M4 core in a critical/interrupt context, because it involves communication with M0 core to synchronize on resource usage.
Notice, that this is critical fix for this platform

@MarceloSalazar

Description

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

[0xc0170]

Is this the only issue you are having with this target - the rest of drivers are fine? Basically, this is one extra step (that is done in the reset method but that one is within critical section) added.

[lrusinowicz]

During tests on our platform I haven't found other driver issues. I guess only Timer is using critical section.

[deepikabhavnani]

This change might have impact on ARMv8 devices as well. CC @ARMmbed/team-nuvoton @gaborkertesz

[c1728p9]

@lrusinowicz if this is a requirement for your target I would recommend calling the ticker initialization as part of the boot sequence rather than when a Timer is created. That will ensure that all users of the ticker work correctly, rather than just Timer. I recommend placing the ticker initialization in the hook mbed_sdk_init.

[0xc0170]

This change might have impact on ARMv8 devices as well. CC @ARMmbed/team-nuvoton @gaborkertesz

This is what should be considered. I assumed that this is much bigger problem we might facing. Please review

[kjbracey]

Many HAL device calls are expected to be interrupt safe - useable from interrupts, and with interrupts disabled. Hence tweaks like this being needed: #8048

I think you may ultimately need to have polling comms mechanism that can be invoked for cases where you're in a critical section and need to talk to the other core, but maybe if it's init only that can be avoided.

For this case, if the problem only occurs on first call, this sort of fix seems reasonable - make sure a call happens early so all subsequent ones are safe, but not sure this is the correct place for it. It could result in interleaving calls to the read_ticker_us when Timers are constructed, when it's trying very hard to use the critical section to serialise all calls to the ticker data. I can see that causing issues in some platforms.

Simplest thing to do would just be to do the necessary init yourself in platform startup code, before entering main. But that may pull in code the application isn't going to use.

Calling _ticker_data->init() in the constructor of Timer might be a bit safer, looking at the implementations. They all return immediately if their initialised flag is set, so that should be better than doing the initialise via read. Still a race if the first 2 Timer constructions for a ticker data happen in parallel, but pretty unlikely.

[0xc0170]

@lrusinowicz I believe this was resolved differently and this one is not needed anymore (as it is) ?

[lrusinowicz]

@0xc0170 Yes, it's not needed for Sequana. Should I close it?