Merge pull request #1581 from dhalbert/ble-drv-root-pointer

Use critical section, not lock, in CharacteristicBuffer; use a root pointer for ble_drv list

Author: kattni

ports/atmel-samd/mpconfigport.h

@@ -79,11 +79,10 @@
 
 #include "peripherals/samd/dma.h"
 
+#include "py/circuitpy_mpconfig.h"
+
 #define MICROPY_PORT_ROOT_POINTERS \
     CIRCUITPY_COMMON_ROOT_POINTERS \
-    mp_obj_t playing_audio[AUDIO_DMA_CHANNEL_COUNT] \
-    ;
-
-#include "py/circuitpy_mpconfig.h"
+    mp_obj_t playing_audio[AUDIO_DMA_CHANNEL_COUNT];
 
 #endif  // __INCLUDED_MPCONFIGPORT_H

ports/nrf/bluetooth/ble_drv.c

@@ -35,27 +35,20 @@
 #include "nrf_soc.h"
 #include "nrfx_power.h"
 #include "py/misc.h"
+#include "py/mpstate.h"
 
 nrf_nvic_state_t nrf_nvic_state = { 0 };
 
 __attribute__((aligned(4)))
 static uint8_t m_ble_evt_buf[sizeof(ble_evt_t) + (BLE_GATT_ATT_MTU_DEFAULT)];
 
-typedef struct event_handler {
-    struct event_handler *next;
-    void *param;
-    ble_drv_evt_handler_t func;
-} event_handler_t;
-
-static event_handler_t *m_event_handlers = NULL;
-
 void ble_drv_reset() {
     // Linked list items will be gc'd.
-    m_event_handlers = NULL;
+    MP_STATE_VM(ble_drv_evt_handler_entries) = NULL;
 }
 
 void ble_drv_add_event_handler(ble_drv_evt_handler_t func, void *param) {
-    event_handler_t *it = m_event_handlers;
+    ble_drv_evt_handler_entry_t *it = MP_STATE_VM(ble_drv_evt_handler_entries);
     while (it != NULL) {
         // If event handler and its corresponding param are already on the list, don't add again.
         if ((it->func == func) && (it->param == param)) {
@@ -65,17 +58,17 @@ void ble_drv_add_event_handler(ble_drv_evt_handler_t func, void *param) {
     }
 
     // Add a new handler to the front of the list
-    event_handler_t *handler = m_new_ll(event_handler_t, 1);
-    handler->next = m_event_handlers;
+    ble_drv_evt_handler_entry_t *handler = m_new_ll(ble_drv_evt_handler_entry_t, 1);
+    handler->next = MP_STATE_VM(ble_drv_evt_handler_entries);
     handler->param = param;
     handler->func = func;
 
-    m_event_handlers = handler;
+    MP_STATE_VM(ble_drv_evt_handler_entries) = handler;
 }
 
 void ble_drv_remove_event_handler(ble_drv_evt_handler_t func, void *param) {
-    event_handler_t *it = m_event_handlers;
-    event_handler_t **prev = &m_event_handlers;
+    ble_drv_evt_handler_entry_t *it = MP_STATE_VM(ble_drv_evt_handler_entries);
+    ble_drv_evt_handler_entry_t **prev = &MP_STATE_VM(ble_drv_evt_handler_entries);
     while (it != NULL) {
         if ((it->func == func) && (it->param == param)) {
             // Splice out the matching handler.
@@ -122,7 +115,7 @@ void SD_EVT_IRQHandler(void) {
             break;
         }
 
-        event_handler_t *it = m_event_handlers;
+        ble_drv_evt_handler_entry_t *it = MP_STATE_VM(ble_drv_evt_handler_entries);
         while (it != NULL) {
             it->func((ble_evt_t *)m_ble_evt_buf, it->param);
             it = it->next;

ports/nrf/bluetooth/ble_drv.h

@@ -50,6 +50,12 @@
 
 typedef void (*ble_drv_evt_handler_t)(ble_evt_t*, void*);
 
+typedef struct ble_drv_evt_handler_entry {
+    struct ble_drv_evt_handler_entry *next;
+    void *param;
+    ble_drv_evt_handler_t func;
+} ble_drv_evt_handler_entry_t;
+
 void ble_drv_reset(void);
 void ble_drv_add_event_handler(ble_drv_evt_handler_t func, void *param);
 void ble_drv_remove_event_handler(ble_drv_evt_handler_t func, void *param);

ports/nrf/common-hal/bleio/CharacteristicBuffer.c

@@ -29,7 +29,7 @@
 
 #include "ble_drv.h"
 #include "ble_gatts.h"
-#include "sd_mutex.h"
+#include "nrf_nvic.h"
 
 #include "lib/utils/interrupt_char.h"
 #include "py/runtime.h"
@@ -47,14 +47,14 @@ STATIC void characteristic_buffer_on_ble_evt(ble_evt_t *ble_evt, void *param) {
         ble_gatts_evt_write_t *evt_write = &ble_evt->evt.gatts_evt.params.write;
         // Event handle must match the handle for my characteristic.
         if (evt_write->handle == self->characteristic->handle) {
-            // Push all the data onto the ring buffer, but wait for any reads to finish.
-            sd_mutex_acquire_wait_no_vm(&self->ringbuf_mutex);
+            // Push all the data onto the ring buffer.
+            uint8_t is_nested_critical_region;
+            sd_nvic_critical_region_enter(&is_nested_critical_region);
             for (size_t i = 0; i < evt_write->len; i++) {
                 ringbuf_put(&self->ringbuf, evt_write->data[i]);
             }
-            // Don't check for errors: we're in an event handler.
-            sd_mutex_release(&self->ringbuf_mutex);
-        break;
+            sd_nvic_critical_region_exit(is_nested_critical_region);
+            break;
         }
     }
     }
@@ -72,7 +72,6 @@ void common_hal_bleio_characteristic_buffer_construct(bleio_characteristic_buffe
     // This is a macro.
     // true means long-lived, so it won't be moved.
     ringbuf_alloc(&self->ringbuf, buffer_size, true);
-    sd_mutex_new(&self->ringbuf_mutex);
 
     ble_drv_add_event_handler(characteristic_buffer_on_ble_evt, self);
 
@@ -92,29 +91,35 @@ int common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_
 #endif
     }
 
-    // Copy received data. Lock out writes while copying.
-    sd_mutex_acquire_wait(&self->ringbuf_mutex);
+    // Copy received data. Lock out write interrupt handler while copying.
+    uint8_t is_nested_critical_region;
+    sd_nvic_critical_region_enter(&is_nested_critical_region);
 
     size_t rx_bytes = MIN(ringbuf_count(&self->ringbuf), len);
     for ( size_t i = 0; i < rx_bytes; i++ ) {
         data[i] = ringbuf_get(&self->ringbuf);
     }
 
     // Writes now OK.
-    sd_mutex_release_check(&self->ringbuf_mutex);
+    sd_nvic_critical_region_exit(is_nested_critical_region);
 
     return rx_bytes;
 }
 
 uint32_t common_hal_bleio_characteristic_buffer_rx_characters_available(bleio_characteristic_buffer_obj_t *self) {
-    return ringbuf_count(&self->ringbuf);
+    uint8_t is_nested_critical_region;
+    sd_nvic_critical_region_enter(&is_nested_critical_region);
+    uint16_t count = ringbuf_count(&self->ringbuf);
+    sd_nvic_critical_region_exit(is_nested_critical_region);
+    return count;
 }
 
 void common_hal_bleio_characteristic_buffer_clear_rx_buffer(bleio_characteristic_buffer_obj_t *self) {
     // prevent conflict with uart irq
-    sd_mutex_acquire_wait(&self->ringbuf_mutex);
+    uint8_t is_nested_critical_region;
+    sd_nvic_critical_region_enter(&is_nested_critical_region);
     ringbuf_clear(&self->ringbuf);
-    sd_mutex_release_check(&self->ringbuf_mutex);
+    sd_nvic_critical_region_exit(is_nested_critical_region);
 }
 
 bool common_hal_bleio_characteristic_buffer_deinited(bleio_characteristic_buffer_obj_t *self) {

ports/nrf/common-hal/bleio/CharacteristicBuffer.h

@@ -38,7 +38,6 @@ typedef struct {
     uint32_t timeout_ms;
     // Ring buffer storing consecutive incoming values.
     ringbuf_t ringbuf;
-    nrf_mutex_t ringbuf_mutex;
 } bleio_characteristic_buffer_obj_t;
 
 #endif // MICROPY_INCLUDED_COMMON_HAL_BLEIO_CHARACTERISTICBUFFER_H

ports/nrf/mpconfigport.h

@@ -28,6 +28,8 @@
 #ifndef NRF5_MPCONFIGPORT_H__
 #define NRF5_MPCONFIGPORT_H__
 
+#include "ble_drv.h"
+
 #define MICROPY_CPYTHON_COMPAT                   (1)
 //#define MICROPY_MODULE_BUILTIN_INIT              (1)  // TODO check this
 //#define MICROPY_MODULE_WEAK_LINKS                (1)  // TODO check this
@@ -52,10 +54,11 @@
 // 24kiB stack
 #define CIRCUITPY_DEFAULT_STACK_SIZE            0x6000
 
+#include "py/circuitpy_mpconfig.h"
+
 #define MICROPY_PORT_ROOT_POINTERS \
     CIRCUITPY_COMMON_ROOT_POINTERS \
-    ;
+    ble_drv_evt_handler_entry_t* ble_drv_evt_handler_entries; \
 
-#include "py/circuitpy_mpconfig.h"
 
 #endif  // NRF5_MPCONFIGPORT_H__