From 99da3b9646c22ee8232b0b238ffcaf74b250333f Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Thu, 21 Feb 2019 00:19:31 -0500 Subject: [PATCH] Use critical section, not lock, in CharacteristicBuffer; use a root pointer for ble_drv list --- ports/atmel-samd/mpconfigport.h | 9 +++--- ports/nrf/bluetooth/ble_drv.c | 25 ++++++--------- ports/nrf/bluetooth/ble_drv.h | 6 ++++ .../common-hal/bleio/CharacteristicBuffer.c | 31 +++++++++++-------- .../common-hal/bleio/CharacteristicBuffer.h | 1 - ports/nrf/mpconfigport.h | 11 ++++--- 6 files changed, 44 insertions(+), 39 deletions(-) diff --git a/ports/atmel-samd/mpconfigport.h b/ports/atmel-samd/mpconfigport.h index 036149ae04..dbf1c93f37 100644 --- a/ports/atmel-samd/mpconfigport.h +++ b/ports/atmel-samd/mpconfigport.h @@ -79,11 +79,10 @@ #include "peripherals/samd/dma.h" -#define MICROPY_PORT_ROOT_POINTERS \ - CIRCUITPY_COMMON_ROOT_POINTERS \ - mp_obj_t playing_audio[AUDIO_DMA_CHANNEL_COUNT] \ - ; - #include "py/circuitpy_mpconfig.h" +#define MICROPY_PORT_ROOT_POINTERS \ + CIRCUITPY_COMMON_ROOT_POINTERS \ + mp_obj_t playing_audio[AUDIO_DMA_CHANNEL_COUNT]; + #endif // __INCLUDED_MPCONFIGPORT_H diff --git a/ports/nrf/bluetooth/ble_drv.c b/ports/nrf/bluetooth/ble_drv.c index 4e75236911..bb395ce4c7 100644 --- a/ports/nrf/bluetooth/ble_drv.c +++ b/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; diff --git a/ports/nrf/bluetooth/ble_drv.h b/ports/nrf/bluetooth/ble_drv.h index a6f7b90337..8d3846a7b7 100644 --- a/ports/nrf/bluetooth/ble_drv.h +++ b/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); diff --git a/ports/nrf/common-hal/bleio/CharacteristicBuffer.c b/ports/nrf/common-hal/bleio/CharacteristicBuffer.c index 42e45abdfb..19b3b85ea7 100644 --- a/ports/nrf/common-hal/bleio/CharacteristicBuffer.c +++ b/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,8 +91,9 @@ 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++ ) { @@ -101,20 +101,25 @@ int common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer_obj_ } // 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) { diff --git a/ports/nrf/common-hal/bleio/CharacteristicBuffer.h b/ports/nrf/common-hal/bleio/CharacteristicBuffer.h index f5c7151547..b36f63fec3 100644 --- a/ports/nrf/common-hal/bleio/CharacteristicBuffer.h +++ b/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 diff --git a/ports/nrf/mpconfigport.h b/ports/nrf/mpconfigport.h index cb6e2e350f..1d863ed72c 100644 --- a/ports/nrf/mpconfigport.h +++ b/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 -#define MICROPY_PORT_ROOT_POINTERS \ - CIRCUITPY_COMMON_ROOT_POINTERS \ - ; - #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; \ + + #endif // NRF5_MPCONFIGPORT_H__