Fix CriticalSection lock for nrf51 targets. #77
Conversation
|
+1 |
|
Looks correct to me, but out of curiosity: Why isn't there an IRQ enable in the destructor? (It was like that before the change.) |
Which line are you referencing ? It restores primask, as it was before. |
| // get the state of exceptions for the CPU | ||
| _PRIMASK_state = __get_PRIMASK(); | ||
|
|
||
| // if exceptions are not enabled, there is nothing more to do |
There was a problem hiding this comment.
Please fix the formatting in the changes:
an example:
if (_PRIMASK_state == 1) {
_use_softdevice_routine = false;
} else {
|
@0xc0170 So the __disable_irq() only writes to PRIMASK? Wasn't aware of it; then no further comments. |
The previous implementation of CriticalSectionLock for nrf51 was only relying to softdevice primitives sd_nvic_critical_region_enter and sd_nvic_critical_region_exit . If the soft device is not enabled, these functions have no effect. This means that CriticalSectionLock was not locking anything if the softdevice was not enabled. This fix allow the usage of CriticalSectionLock, even if the softdevice is not enabled. If the softdevice is not enabled, all irq are disabled until the destruction of the lock.
pan-
force-pushed
the
fix_critical_section_lock_nrf51
branch
from
3e3e2bf
to
f90ab01
Compare
|
LGTM, however it is lot of code for for critical section :-) cc @bogdanm |
|
That does look like a lot of code, thus quite a few opportunities for race conditions. I'm going to trust @rgrover on this one, since I'm not that familiar with nRF51 targets. So, +1. |
Fix CriticalSection lock for nrf51 targets.
The previous implementation of CriticalSectionLock for nrf51 was only
relying to softdevice primitives sd_nvic_critical_region_enter and
sd_nvic_critical_region_exit .
If the soft device is not enabled, these functions have no effect. This
means that CriticalSectionLock was not locking anything if the softdevice
was not enabled.
This fix allow the usage of CriticalSectionLock, even if the softdevice is
not enabled. If the softdevice is not enabled, all irq are disabled until
the destruction of the lock.
This PR should be merged with: ARMmbed/mbed-hal-nrf51822-mcu#48