diff options
author | Henry Gabryjelski <henrygab@users.noreply.github.com> | 2019-08-13 04:46:38 +0300 |
---|---|---|
committer | Daniel Garcia <danielgarcia@gmail.com> | 2019-08-13 04:46:38 +0300 |
commit | 535188dfab7551d6053724a5dd74e0b2b04a7be6 (patch) | |
tree | 872248bea502603f648f8ec01fe075d5fe352f45 | |
parent | 443259740cc7e6b120d1e4aae0c4b4b8d1dfef52 (diff) |
Fix nRF52 race condition / hard lock (#857)
* Textbook example of where volatile is useful.
* Use intrinsics that ensure memory barrier for sequence buffer lock.
* Update wait time to ensure prior data had a chance to latch.
-rw-r--r-- | platforms/arm/nrf52/clockless_arm_nrf52.h | 25 |
1 files changed, 20 insertions, 5 deletions
diff --git a/platforms/arm/nrf52/clockless_arm_nrf52.h b/platforms/arm/nrf52/clockless_arm_nrf52.h index 94fd3ed8..56a1dbe0 100644 --- a/platforms/arm/nrf52/clockless_arm_nrf52.h +++ b/platforms/arm/nrf52/clockless_arm_nrf52.h @@ -46,7 +46,7 @@ private: // may as well be static, as can only attach one LED string per _DATA_PIN.... static uint16_t s_SequenceBuffer[_PWM_BUFFER_COUNT]; static uint16_t s_SequenceBufferValidElements; - static uint32_t s_SequenceBufferInUse; + static volatile uint32_t s_SequenceBufferInUse; static CMinWait<_WAIT_TIME_MICROSECONDS> mWait; // ensure data has time to latch FASTLED_NRF52_INLINE_ATTRIBUTE static void startPwmPlayback_InitializePinState() { @@ -123,6 +123,18 @@ private: FASTLED_NRF52_INLINE_ATTRIBUTE static void startPwmPlayback_StartTask(NRF_PWM_Type * pwm) { nrf_pwm_task_trigger(pwm, NRF_PWM_TASK_SEQSTART0); } + FASTLED_NRF52_INLINE_ATTRIBUTE static void spinAcquireSequenceBuffer() { + while (!tryAcquireSequenceBuffer()); + } + FASTLED_NRF52_INLINE_ATTRIBUTE static bool tryAcquireSequenceBuffer() { + return __sync_bool_compare_and_swap(&s_SequenceBufferInUse, 0, 1); + } + FASTLED_NRF52_INLINE_ATTRIBUTE static void releaseSequenceBuffer() { + uint32_t tmp = __sync_val_compare_and_swap(&s_SequenceBufferInUse, 1, 0); + if (tmp != 1) { + // TODO: Error / Assert / log ? + } + } public: static void isr_handler() { @@ -134,8 +146,10 @@ public: if (nrf_pwm_event_check(pwm,NRF_PWM_EVENT_STOPPED)) { nrf_pwm_event_clear(pwm,NRF_PWM_EVENT_STOPPED); + // update the minimum time to next call + mWait.mark(); // mark the sequence as no longer in use -- pointer, comparator, exchange value - __sync_fetch_and_and(&s_SequenceBufferInUse, 0); + releaseSequenceBuffer(); // prevent further interrupts from PWM events nrf_pwm_int_set(pwm, 0); // disable PWM interrupts - None of the PWM IRQs are shared @@ -166,9 +180,10 @@ public: virtual void showPixels(PixelController<_RGB_ORDER> & pixels) { // wait for the only sequence buffer to become available - while (s_SequenceBufferInUse != 0); + spinAcquireSequenceBuffer(); prepareSequenceBuffers(pixels); - mWait.wait(); // ensure min time between updates + // ensure any prior data had time to latch + mWait.wait(); startPwmPlayback(s_SequenceBufferValidElements); return; } @@ -307,7 +322,7 @@ public: template <uint8_t _DATA_PIN, int _T1, int _T2, int _T3, EOrder _RGB_ORDER, int _XTRA0, bool _FLIP, int _WAIT_TIME_MICROSECONDS> uint16_t ClocklessController<_DATA_PIN, _T1, _T2, _T3, _RGB_ORDER, _XTRA0, _FLIP, _WAIT_TIME_MICROSECONDS>::s_SequenceBufferValidElements = 0; template <uint8_t _DATA_PIN, int _T1, int _T2, int _T3, EOrder _RGB_ORDER, int _XTRA0, bool _FLIP, int _WAIT_TIME_MICROSECONDS> -uint32_t ClocklessController<_DATA_PIN, _T1, _T2, _T3, _RGB_ORDER, _XTRA0, _FLIP, _WAIT_TIME_MICROSECONDS>::s_SequenceBufferInUse = 0; +uint32_t volatile ClocklessController<_DATA_PIN, _T1, _T2, _T3, _RGB_ORDER, _XTRA0, _FLIP, _WAIT_TIME_MICROSECONDS>::s_SequenceBufferInUse = 0; template <uint8_t _DATA_PIN, int _T1, int _T2, int _T3, EOrder _RGB_ORDER, int _XTRA0, bool _FLIP, int _WAIT_TIME_MICROSECONDS> uint16_t ClocklessController<_DATA_PIN, _T1, _T2, _T3, _RGB_ORDER, _XTRA0, _FLIP, _WAIT_TIME_MICROSECONDS>::s_SequenceBuffer[_PWM_BUFFER_COUNT]; template <uint8_t _DATA_PIN, int _T1, int _T2, int _T3, EOrder _RGB_ORDER, int _XTRA0, bool _FLIP, int _WAIT_TIME_MICROSECONDS> |