extmod/modbluetooth: Refactor stack/hci/driver/port bindings.

Previously the interaction between the different layers of the Bluetooth
stack was different on each port and each stack.  This commit defines
common interfaces between them and implements them for cyw43, btstack,
nimble, stm32, unix.

10 files changed, 283 insertions, 99 deletions

diff --git a/extmod/nimble/hal/hal_uart.c b/extmod/nimble/hal/hal_uart.c
index fba82b830..c6d0850fe 100644
--- a/extmod/nimble/hal/hal_uart.c
+++ b/extmod/nimble/hal/hal_uart.c
@@ -26,11 +26,9 @@
 
 #include "py/runtime.h"
 #include "py/mphal.h"
-#include "pin_static_af.h"
 #include "nimble/ble.h"
 #include "extmod/nimble/hal/hal_uart.h"
-#include "extmod/modbluetooth_hci.h"
-#include "extmod/nimble/nimble/nimble_hci_uart.h"
+#include "extmod/mpbthci.h"
 
 #if MICROPY_PY_BLUETOOTH && MICROPY_BLUETOOTH_NIMBLE
 
@@ -39,6 +37,9 @@ static void *hal_uart_tx_arg;
 static hal_uart_rx_cb_t hal_uart_rx_cb;
 static void *hal_uart_rx_arg;
 
+// Provided by the port, and also possibly shared with the driver.
+extern uint8_t mp_bluetooth_hci_cmd_buf[4 + 256];
+
 int hal_uart_init_cbs(uint32_t port, hal_uart_tx_cb_t tx_cb, void *tx_arg, hal_uart_rx_cb_t rx_cb, void *rx_arg) {
     hal_uart_tx_cb = tx_cb;
     hal_uart_tx_arg = tx_arg;
@@ -48,9 +49,7 @@ int hal_uart_init_cbs(uint32_t port, hal_uart_tx_cb_t tx_cb, void *tx_arg, hal_u
 }
 
 int hal_uart_config(uint32_t port, uint32_t baudrate, uint32_t bits, uint32_t stop, uint32_t parity, uint32_t flow) {
-    mp_bluetooth_hci_uart_init(port);
-    mp_bluetooth_hci_uart_set_baudrate(baudrate);
-    return mp_bluetooth_hci_uart_activate();
+    return mp_bluetooth_hci_uart_init(port, baudrate);
 }
 
 void hal_uart_start_tx(uint32_t port) {
@@ -71,7 +70,7 @@ void hal_uart_start_tx(uint32_t port) {
     printf("\n");
     #endif
 
-    mp_bluetooth_nimble_hci_uart_tx_strn((void*)mp_bluetooth_hci_cmd_buf, len);
+    mp_bluetooth_hci_uart_write(mp_bluetooth_hci_cmd_buf, len);
 }
 
 int hal_uart_close(uint32_t port) {
@@ -79,7 +78,17 @@ int hal_uart_close(uint32_t port) {
 }
 
 void mp_bluetooth_nimble_hci_uart_process(void) {
-    mp_bluetooth_nimble_hci_uart_rx(hal_uart_rx_cb, hal_uart_rx_arg);
+    bool host_wake = mp_bluetooth_hci_controller_woken();
+
+    int chr;
+    while ((chr = mp_bluetooth_hci_uart_readchar()) >= 0) {
+        // printf("UART RX: %02x\n", data);
+        hal_uart_rx_cb(hal_uart_rx_arg, chr);
+    }
+
+    if (host_wake) {
+        mp_bluetooth_hci_controller_sleep_maybe();
+    }
 }
 
 #endif // MICROPY_PY_BLUETOOTH && MICROPY_BLUETOOTH_NIMBLE
diff --git a/extmod/nimble/hal/hal_uart.h b/extmod/nimble/hal/hal_uart.h
index 0ef04bc81..1ff1c1743 100644
--- a/extmod/nimble/hal/hal_uart.h
+++ b/extmod/nimble/hal/hal_uart.h
@@ -1,3 +1,29 @@
+/*
+ * This file is part of the MicroPython project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2020 Jim Mussared
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
 #ifndef MICROPY_INCLUDED_EXTMOD_NIMBLE_HAL_HAL_UART_H
 #define MICROPY_INCLUDED_EXTMOD_NIMBLE_HAL_HAL_UART_H
 
@@ -10,10 +36,13 @@
 typedef int (*hal_uart_tx_cb_t)(void *arg);
 typedef int (*hal_uart_rx_cb_t)(void *arg, uint8_t data);
 
-// Called by NimBLE, implemented in hal_uart.c.
+// --- Called by NimBLE, implemented in hal_uart.c. ---------------------------
 int hal_uart_init_cbs(uint32_t port, hal_uart_tx_cb_t tx_cb, void *tx_arg, hal_uart_rx_cb_t rx_cb, void *rx_arg);
 int hal_uart_config(uint32_t port, uint32_t baud, uint32_t bits, uint32_t stop, uint32_t parity, uint32_t flow);
 void hal_uart_start_tx(uint32_t port);
 int hal_uart_close(uint32_t port);
 
+// --- Called by the MicroPython port when UART data is available -------------
+void mp_bluetooth_nimble_hci_uart_process(void);
+
 #endif // MICROPY_INCLUDED_EXTMOD_NIMBLE_HAL_HAL_UART_H
diff --git a/extmod/nimble/logcfg/logcfg.h b/extmod/nimble/logcfg/logcfg.h
new file mode 100644
index 000000000..e9023dae6
--- /dev/null
+++ b/extmod/nimble/logcfg/logcfg.h
@@ -0,0 +1,45 @@
+/**
+ * This file was generated by Apache newt version: 1.8.0-dev
+ */
+
+#ifndef MICROPY_INCLUDED_EXTMOD_NIMBLE_LOGCFG_LOGCFG_H
+#define MICROPY_INCLUDED_EXTMOD_NIMBLE_LOGCFG_LOGCFG_H
+
+#include "py/mphal.h"
+#include "modlog/modlog.h"
+#include "log_common/log_common.h"
+
+#define MICROPY_PY_BLUETOOTH_DIAGNOSTIC_LOGGING (1)
+
+#if MICROPY_PY_BLUETOOTH_DIAGNOSTIC_LOGGING
+#define DFLT_LOG_DEBUG(...) MODLOG_DEBUG(4, __VA_ARGS__)
+#else
+#define DFLT_LOG_DEBUG(...) IGNORE(__VA_ARGS__)
+#endif
+
+#if MICROPY_PY_BLUETOOTH_DIAGNOSTIC_LOGGING > 1
+#define BLE_HS_LOG_DEBUG(...) MODLOG_DEBUG(4, __VA_ARGS__)
+#else
+#define BLE_HS_LOG_DEBUG(...) IGNORE(__VA_ARGS__)
+#endif
+
+#define BLE_HS_LOG_INFO(...) MODLOG_INFO(4, __VA_ARGS__)
+#define BLE_HS_LOG_WARN(...) MODLOG_WARN(4, __VA_ARGS__)
+#define BLE_HS_LOG_ERROR(...) MODLOG_ERROR(4, __VA_ARGS__)
+#define BLE_HS_LOG_CRITICAL(...) MODLOG_CRITICAL(4, __VA_ARGS__)
+#define BLE_HS_LOG_DISABLED(...) MODLOG_DISABLED(4, __VA_ARGS__)
+
+#define DFLT_LOG_INFO(...) MODLOG_INFO(0, __VA_ARGS__)
+#define DFLT_LOG_WARN(...) MODLOG_WARN(0, __VA_ARGS__)
+#define DFLT_LOG_ERROR(...) MODLOG_ERROR(0, __VA_ARGS__)
+#define DFLT_LOG_CRITICAL(...) MODLOG_CRITICAL(0, __VA_ARGS__)
+#define DFLT_LOG_DISABLED(...) MODLOG_DISABLED(0, __VA_ARGS__)
+
+#define MFG_LOG_DEBUG(...) IGNORE(__VA_ARGS__)
+#define MFG_LOG_INFO(...) IGNORE(__VA_ARGS__)
+#define MFG_LOG_WARN(...) IGNORE(__VA_ARGS__)
+#define MFG_LOG_ERROR(...) IGNORE(__VA_ARGS__)
+#define MFG_LOG_CRITICAL(...) IGNORE(__VA_ARGS__)
+#define MFG_LOG_DISABLED(...) MODLOG_DISABLED(128, __VA_ARGS__)
+
+#endif // MICROPY_INCLUDED_EXTMOD_NIMBLE_LOGCFG_LOGCFG_H
diff --git a/extmod/nimble/modbluetooth_nimble.c b/extmod/nimble/modbluetooth_nimble.c
index 36bfa6ccc..61ba3a3ab 100644
--- a/extmod/nimble/modbluetooth_nimble.c
+++ b/extmod/nimble/modbluetooth_nimble.c
@@ -33,6 +33,7 @@
 
 #include "extmod/nimble/modbluetooth_nimble.h"
 #include "extmod/modbluetooth.h"
+#include "extmod/mpbthci.h"
 
 #include "host/ble_hs.h"
 #include "host/util/util.h"
@@ -64,10 +65,11 @@ STATIC int8_t ble_hs_err_to_errno_table[] = {
 };
 
 STATIC int ble_hs_err_to_errno(int err) {
+    DEBUG_printf("ble_hs_err_to_errno: %d\n", err);
     if (!err) {
         return 0;
     }
-    if (0 <= err && err < MP_ARRAY_SIZE(ble_hs_err_to_errno_table) && ble_hs_err_to_errno_table[err]) {
+    if (err >= 0 && (unsigned)err < MP_ARRAY_SIZE(ble_hs_err_to_errno_table) && ble_hs_err_to_errno_table[err]) {
         return ble_hs_err_to_errno_table[err];
     } else {
         return MP_EIO;
@@ -150,6 +152,12 @@ STATIC void sync_cb(void) {
     int rc;
     ble_addr_t addr;
 
+    DEBUG_printf("sync_cb: state=%d\n", mp_bluetooth_nimble_ble_state);
+
+    if (mp_bluetooth_nimble_ble_state != MP_BLUETOOTH_NIMBLE_BLE_STATE_WAITING_FOR_SYNC) {
+        return;
+    }
+
     rc = ble_hs_util_ensure_addr(0); // prefer public address
     if (rc != 0) {
         // https://mynewt.apache.org/latest/tutorials/ble/eddystone.html#configure-the-nimble-stack-with-an-address
@@ -264,11 +272,67 @@ STATIC int gap_event_cb(struct ble_gap_event *event, void *arg) {
     return 0;
 }
 
+#if !MICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY
+
+// On ports such as ESP32 where we only implement the bindings, then
+// the port must provide these functions.
+// But for STM32 / Unix-H4, we provide a default implementation of the
+// port-specific functionality.
+// TODO: In the future if a port ever needs to customise these functions
+// then investigate using MP_WEAK or splitting them out to another .c file.
+
+#include "transport/uart/ble_hci_uart.h"
+
+void mp_bluetooth_nimble_port_hci_init(void) {
+    DEBUG_printf("mp_bluetooth_nimble_port_hci_init (nimble default)\n");
+    // This calls mp_bluetooth_hci_uart_init (via ble_hci_uart_init --> hal_uart_config --> mp_bluetooth_hci_uart_init).
+    ble_hci_uart_init();
+    mp_bluetooth_hci_controller_init();
+}
+
+void mp_bluetooth_nimble_port_hci_deinit(void) {
+    DEBUG_printf("mp_bluetooth_nimble_port_hci_deinit (nimble default)\n");
+    mp_bluetooth_hci_controller_deinit();
+    mp_bluetooth_hci_uart_deinit();
+}
+
+void mp_bluetooth_nimble_port_start(void) {
+    DEBUG_printf("mp_bluetooth_nimble_port_start (nimble default)\n");
+    // By default, assume port is already running its own background task (e.g. SysTick on STM32).
+    // ESP32 runs a FreeRTOS task, Unix has a thread.
+}
+
+// Called when the host stop procedure has completed.
+STATIC void ble_hs_shutdown_stop_cb(int status, void *arg) {
+    (void)status;
+    (void)arg;
+    mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF;
+}
+
+STATIC struct ble_hs_stop_listener ble_hs_shutdown_stop_listener;
+
+void mp_bluetooth_nimble_port_shutdown(void) {
+    DEBUG_printf("mp_bluetooth_nimble_port_shutdown (nimble default)\n");
+    // By default, just call ble_hs_stop directly and wait for the stack to stop.
+
+    mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_STOPPING;
+
+    ble_hs_stop(&ble_hs_shutdown_stop_listener, ble_hs_shutdown_stop_cb, NULL);
+
+    while (mp_bluetooth_nimble_ble_state != MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF) {
+        MICROPY_EVENT_POLL_HOOK
+    }
+}
+
+#endif // !MICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY
+
 int mp_bluetooth_init(void) {
     DEBUG_printf("mp_bluetooth_init\n");
     // Clean up if necessary.
     mp_bluetooth_deinit();
 
+    mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_STARTING;
+
     ble_hs_cfg.reset_cb = reset_cb;
     ble_hs_cfg.sync_cb = sync_cb;
     ble_hs_cfg.gatts_register_cb = gatts_register_cb;
@@ -277,56 +341,70 @@ int mp_bluetooth_init(void) {
     MP_STATE_PORT(bluetooth_nimble_root_pointers) = m_new0(mp_bluetooth_nimble_root_pointers_t, 1);
     mp_bluetooth_gatts_db_create(&MP_STATE_PORT(bluetooth_nimble_root_pointers)->gatts_db);
 
-    mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_STARTING;
+    #if !MICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY
+    // Dereference any previous NimBLE mallocs.
+    MP_STATE_PORT(bluetooth_nimble_memory) = NULL;
+    #endif
 
-    mp_bluetooth_nimble_port_preinit();
+    // Allow port (ESP32) to override NimBLE's HCI init.
+    // Otherwise default implementation above calls ble_hci_uart_init().
+    mp_bluetooth_nimble_port_hci_init();
+
+    // Initialise NimBLE memory and data structures.
     nimble_port_init();
-    mp_bluetooth_nimble_port_postinit();
 
     // By default, just register the default gap/gatt service.
     ble_svc_gap_init();
     ble_svc_gatt_init();
 
+    // Make sure that the HCI UART and event handling task is running.
     mp_bluetooth_nimble_port_start();
 
+    // Static initialization is complete, can start processing events.
+    mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_WAITING_FOR_SYNC;
+
     // Wait for sync callback
     while (mp_bluetooth_nimble_ble_state != MP_BLUETOOTH_NIMBLE_BLE_STATE_ACTIVE) {
         MICROPY_EVENT_POLL_HOOK
     }
+
     DEBUG_printf("mp_bluetooth_init: ready\n");
 
     return 0;
 }
 
-// Called when the host stop procedure has completed.
-STATIC void ble_hs_shutdown_stop_cb(int status, void *arg) {
-    mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF;
-}
-
-STATIC struct ble_hs_stop_listener ble_hs_shutdown_stop_listener;
-
 void mp_bluetooth_deinit(void) {
     DEBUG_printf("mp_bluetooth_deinit\n");
     if (mp_bluetooth_nimble_ble_state == MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF) {
         return;
     }
 
-    mp_bluetooth_gap_advertise_stop();
-    #if MICROPY_PY_BLUETOOTH_ENABLE_CENTRAL_MODE
-    mp_bluetooth_gap_scan_stop();
-    #endif
-
-    mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_STOPPING;
+    // Must call ble_hs_stop() in a port-specific way to stop the background
+    // task. Default implementation provided above.
+    if (mp_bluetooth_nimble_ble_state == MP_BLUETOOTH_NIMBLE_BLE_STATE_ACTIVE) {
+        mp_bluetooth_gap_advertise_stop();
+        #if MICROPY_PY_BLUETOOTH_ENABLE_CENTRAL_MODE
+        mp_bluetooth_gap_scan_stop();
+        #endif
 
-    ble_hs_stop(&ble_hs_shutdown_stop_listener, ble_hs_shutdown_stop_cb, NULL);
+        DEBUG_printf("mp_bluetooth_deinit: starting port shutdown\n");
 
-    while (mp_bluetooth_nimble_ble_state != MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF) {
-        MICROPY_EVENT_POLL_HOOK
+        mp_bluetooth_nimble_port_shutdown();
+        assert(mp_bluetooth_nimble_ble_state == MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF);
+    } else {
+        mp_bluetooth_nimble_ble_state = MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF;
     }
 
-    mp_bluetooth_nimble_port_deinit();
+    // Shutdown the HCI controller.
+    mp_bluetooth_nimble_port_hci_deinit();
 
     MP_STATE_PORT(bluetooth_nimble_root_pointers) = NULL;
+
+    #if !MICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY
+    // Dereference any previous NimBLE mallocs.
+    MP_STATE_PORT(bluetooth_nimble_memory) = NULL;
+    #endif
+
     DEBUG_printf("mp_bluetooth_deinit: shut down\n");
 }
 
@@ -485,7 +563,7 @@ int mp_bluetooth_gatts_register_service_begin(bool append) {
 
     if (!append) {
         // Unref any previous service definitions.
-        for (int i = 0; i < MP_STATE_PORT(bluetooth_nimble_root_pointers)->n_services; ++i) {
+        for (size_t i = 0; i < MP_STATE_PORT(bluetooth_nimble_root_pointers)->n_services; ++i) {
             MP_STATE_PORT(bluetooth_nimble_root_pointers)->services[i] = NULL;
         }
         MP_STATE_PORT(bluetooth_nimble_root_pointers)->n_services = 0;
diff --git a/extmod/nimble/modbluetooth_nimble.h b/extmod/nimble/modbluetooth_nimble.h
index f44e1d69d..7e401781e 100644
--- a/extmod/nimble/modbluetooth_nimble.h
+++ b/extmod/nimble/modbluetooth_nimble.h
@@ -43,15 +43,27 @@ typedef struct _mp_bluetooth_nimble_root_pointers_t {
 enum {
     MP_BLUETOOTH_NIMBLE_BLE_STATE_OFF,
     MP_BLUETOOTH_NIMBLE_BLE_STATE_STARTING,
+    MP_BLUETOOTH_NIMBLE_BLE_STATE_WAITING_FOR_SYNC,
     MP_BLUETOOTH_NIMBLE_BLE_STATE_ACTIVE,
     MP_BLUETOOTH_NIMBLE_BLE_STATE_STOPPING,
 };
 
 extern volatile int mp_bluetooth_nimble_ble_state;
 
-void mp_bluetooth_nimble_port_preinit(void);
-void mp_bluetooth_nimble_port_postinit(void);
-void mp_bluetooth_nimble_port_deinit(void);
+// --- Optionally provided by the MicroPython port. ---------------------------
+// (default implementations provided by modbluetooth_nimble.c)
+
+// Tell the port to init the UART and start the HCI controller.
+void mp_bluetooth_nimble_port_hci_init(void);
+
+// Tell the port to deinit the UART and shutdown the HCI controller.
+void mp_bluetooth_nimble_port_hci_deinit(void);
+
+// Tell the port to run its background task (i.e. poll the UART and pump events).
 void mp_bluetooth_nimble_port_start(void);
 
+// Tell the port to stop its background task.
+void mp_bluetooth_nimble_port_shutdown(void);
+
+
 #endif // MICROPY_INCLUDED_EXTMOD_NIMBLE_MODBLUETOOTH_NIMBLE_H
diff --git a/extmod/nimble/nimble.mk b/extmod/nimble/nimble.mk
index 4e1642c98..f8a68bc6f 100644
--- a/extmod/nimble/nimble.mk
+++ b/extmod/nimble/nimble.mk
@@ -2,16 +2,19 @@
 
 ifeq ($(MICROPY_BLUETOOTH_NIMBLE),1)
 
-EXTMOD_SRC_C += extmod/nimble/modbluetooth_nimble.c
+EXTMOD_DIR = extmod
+NIMBLE_EXTMOD_DIR = $(EXTMOD_DIR)/nimble
 
-CFLAGS_MOD += -DMICROPY_BLUETOOTH_NIMBLE=1
+EXTMOD_SRC_C += $(NIMBLE_EXTMOD_DIR)/modbluetooth_nimble.c
 
-NIMBLE_EXTMOD_DIR = extmod/nimble
+CFLAGS_MOD += -DMICROPY_BLUETOOTH_NIMBLE=1
 
 # Use NimBLE from the submodule in lib/mynewt-nimble by default,
 # allowing a port to use their own system version (e.g. ESP32).
 MICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY ?= 0
 
+CFLAGS_MOD += -DMICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY=$(MICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY)
+
 ifeq ($(MICROPY_BLUETOOTH_NIMBLE_BINDINGS_ONLY),0)
 
 NIMBLE_LIB_DIR = lib/mynewt-nimble
@@ -82,7 +85,7 @@ LIB_SRC_C += $(addprefix $(NIMBLE_LIB_DIR)/, \
 	)
 
 EXTMOD_SRC_C += $(addprefix $(NIMBLE_EXTMOD_DIR)/, \
-	nimble/npl_os.c \
+	nimble/nimble_npl_os.c \
 	hal/hal_uart.c \
 	)
 
@@ -98,7 +101,7 @@ INC += -I$(TOP)/$(NIMBLE_LIB_DIR)/nimble/include
 INC += -I$(TOP)/$(NIMBLE_LIB_DIR)/nimble/transport/uart/include
 INC += -I$(TOP)/$(NIMBLE_LIB_DIR)/porting/nimble/include
 
-$(BUILD)/$(NIMBLE_LIB_DIR)/%.o: CFLAGS += -Wno-maybe-uninitialized -Wno-pointer-arith -Wno-unused-but-set-variable -Wno-format
+$(BUILD)/$(NIMBLE_LIB_DIR)/%.o: CFLAGS += -Wno-maybe-uninitialized -Wno-pointer-arith -Wno-unused-but-set-variable -Wno-format -Wno-sign-compare
 
 endif
 
diff --git a/extmod/nimble/nimble/nimble_hci_uart.h b/extmod/nimble/nimble/nimble_hci_uart.h
deleted file mode 100644
index 646a12dac..000000000
--- a/extmod/nimble/nimble/nimble_hci_uart.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * This file is part of the MicroPython project, http://micropython.org/
- *
- * The MIT License (MIT)
- *
- * Copyright (c) 2019 Damien P. George
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-#ifndef MICROPY_INCLUDED_EXTMOD_NIMBLE_NIMBLE_NIMBLE_HCI_UART_H
-#define MICROPY_INCLUDED_EXTMOD_NIMBLE_NIMBLE_NIMBLE_HCI_UART_H
-
-// Extensions to extmod/modbluetooth_hci.h specific to NimBLE.
-
-#include "extmod/nimble/hal/hal_uart.h"
-
-// Helpers called from ports.
-void mp_bluetooth_nimble_hci_uart_process(void);
-
-// Must be provided by the port.
-void mp_bluetooth_nimble_hci_uart_rx(hal_uart_rx_cb_t rx_cb, void *rx_arg);
-void mp_bluetooth_nimble_hci_uart_tx_strn(const char *str, uint len);
-
-#endif // MICROPY_INCLUDED_EXTMOD_NIMBLE_NIMBLE_NIMBLE_HCI_UART_H
diff --git a/extmod/nimble/nimble/npl_os.c b/extmod/nimble/nimble/nimble_npl_os.c
index 20c260405..ba3031a8f 100644
--- a/extmod/nimble/nimble/npl_os.c
+++ b/extmod/nimble/nimble/nimble_npl_os.c
@@ -29,16 +29,18 @@
 #include "py/runtime.h"
 #include "nimble/ble.h"
 #include "nimble/nimble_npl.h"
-#include "nimble/nimble_hci_uart.h"
+#include "extmod/nimble/hal/hal_uart.h"
 
-#define DEBUG_OS_printf(...) //printf(__VA_ARGS__)
-#define DEBUG_MALLOC_printf(...) //printf(__VA_ARGS__)
-#define DEBUG_EVENT_printf(...) //printf(__VA_ARGS__)
-#define DEBUG_MUTEX_printf(...) //printf(__VA_ARGS__)
-#define DEBUG_SEM_printf(...) //printf(__VA_ARGS__)
-#define DEBUG_CALLOUT_printf(...) //printf(__VA_ARGS__)
-#define DEBUG_TIME_printf(...) //printf(__VA_ARGS__)
-#define DEBUG_CRIT_printf(...) //printf(__VA_ARGS__)
+#include "extmod/modbluetooth.h"
+
+#define DEBUG_OS_printf(...) // printf(__VA_ARGS__)
+#define DEBUG_MALLOC_printf(...) // printf(__VA_ARGS__)
+#define DEBUG_EVENT_printf(...) // printf(__VA_ARGS__)
+#define DEBUG_MUTEX_printf(...) // printf(__VA_ARGS__)
+#define DEBUG_SEM_printf(...) // printf(__VA_ARGS__)
+#define DEBUG_CALLOUT_printf(...) // printf(__VA_ARGS__)
+#define DEBUG_TIME_printf(...) // printf(__VA_ARGS__)
+#define DEBUG_CRIT_printf(...) // printf(__VA_ARGS__)
 
 bool ble_npl_os_started(void) {
     DEBUG_OS_printf("ble_npl_os_started\n");
@@ -138,7 +140,7 @@ int nimble_sprintf(char *str, const char *fmt, ...) {
 
 struct ble_npl_eventq *global_eventq = NULL;
 
-void os_eventq_run_all(void) {
+void mp_bluetooth_nimble_os_eventq_run_all(void) {
     for (struct ble_npl_eventq *evq = global_eventq; evq != NULL; evq = evq->nextq) {
         while (evq->head != NULL) {
             struct ble_npl_event *ev = evq->head;
@@ -238,23 +240,39 @@ ble_npl_error_t ble_npl_sem_init(struct ble_npl_sem *sem, uint16_t tokens) {
 
 ble_npl_error_t ble_npl_sem_pend(struct ble_npl_sem *sem, ble_npl_time_t timeout) {
     DEBUG_SEM_printf("ble_npl_sem_pend(%p, %u) count=%u\n", sem, (uint)timeout, (uint)sem->count);
+
+    // This is called by NimBLE to synchronously wait for an HCI ACK. The
+    // corresponding ble_npl_sem_release is called directly by the UART rx
+    // handler (i.e. hal_uart_rx_cb in extmod/nimble/hal/hal_uart.c).
+
+    // So this implementation just polls the UART until either the semaphore
+    // is released, or the timeout occurs.
+
     if (sem->count == 0) {
         uint32_t t0 = mp_hal_ticks_ms();
         while (sem->count == 0 && mp_hal_ticks_ms() - t0 < timeout) {
-            // This function may be called at thread-level, so execute
-            // mp_bluetooth_nimble_hci_uart_process at raised priority.
+            // This can be called either from code running in NimBLE's "task"
+            // (i.e. PENDSV) or directly by application code, so for the
+            // latter case, prevent the "task" from running while we poll the
+            // UART directly.
             MICROPY_PY_BLUETOOTH_ENTER
             mp_bluetooth_nimble_hci_uart_process();
             MICROPY_PY_BLUETOOTH_EXIT
+
             if (sem->count != 0) {
                 break;
             }
-            __WFI();
+
+            // Because we're polling, might as well wait for a UART IRQ indicating
+            // more data available.
+            mp_bluetooth_nimble_hci_uart_wfi();
         }
+
         if (sem->count == 0) {
-            printf("timeout\n");
+            printf("NimBLE: HCI ACK timeout\n");
             return BLE_NPL_TIMEOUT;
         }
+
         DEBUG_SEM_printf("got response in %u ms\n", (int)(mp_hal_ticks_ms() - t0));
     }
     sem->count -= 1;
@@ -277,7 +295,7 @@ uint16_t ble_npl_sem_get_count(struct ble_npl_sem *sem) {
 
 static struct ble_npl_callout *global_callout = NULL;
 
-void os_callout_process(void) {
+void mp_bluetooth_nimble_os_callout_process(void) {
     uint32_t tnow = mp_hal_ticks_ms();
     for (struct ble_npl_callout *c = global_callout; c != NULL; c = c->nextc) {
         if (!c->active) {
@@ -386,12 +404,24 @@ void ble_npl_time_delay(ble_npl_time_t ticks) {
 /******************************************************************************/
 // CRITICAL
 
+// This is used anywhere NimBLE modifies global data structures.
+// We need to protect between:
+//  - A MicroPython VM thread.
+//  - The NimBLE "task" (e.g. PENDSV on STM32, pthread on Unix).
+// On STM32, by disabling PENDSV, we ensure that either:
+//  - If we're in the NimBLE task, we're exclusive anyway.
+//  - If we're in a VM thread, we can't be interrupted by the NimBLE task, or switched to another thread.
+// On Unix, there's a global mutex.
+
+// TODO: Both ports currently use MICROPY_PY_BLUETOOTH_ENTER in their implementation,
+// maybe this doesn't need to be port-specific?
+
 uint32_t ble_npl_hw_enter_critical(void) {
     DEBUG_CRIT_printf("ble_npl_hw_enter_critical()\n");
-    return raise_irq_pri(15);
+    return mp_bluetooth_nimble_hci_uart_enter_critical();
 }
 
 void ble_npl_hw_exit_critical(uint32_t ctx) {
     DEBUG_CRIT_printf("ble_npl_hw_exit_critical(%u)\n", (uint)ctx);
-    restore_irq_pri(ctx);
+    mp_bluetooth_nimble_hci_uart_exit_critical(ctx);
 }
diff --git a/extmod/nimble/nimble/nimble_npl_os.h b/extmod/nimble/nimble/nimble_npl_os.h
index 3d886fedb..554252066 100644
--- a/extmod/nimble/nimble/nimble_npl_os.h
+++ b/extmod/nimble/nimble/nimble_npl_os.h
@@ -24,11 +24,15 @@
  * THE SOFTWARE.
  */
 
-#ifndef MICROPY_INCLUDED_STM32_NIMBLE_NIMBLE_NPL_OS_H
-#define MICROPY_INCLUDED_STM32_NIMBLE_NIMBLE_NPL_OS_H
+#ifndef MICROPY_INCLUDED_STM32_NIMBLE_NIMBLE_NIMBLE_NPL_OS_H
+#define MICROPY_INCLUDED_STM32_NIMBLE_NIMBLE_NIMBLE_NPL_OS_H
+
+// This is included by nimble/nimble_npl.h -- include that rather than this file directly.
 
 #include <stdint.h>
 
+// --- Configuration of NimBLE data structures --------------------------------
+
 #define BLE_NPL_OS_ALIGNMENT (4)
 #define BLE_NPL_TIME_FOREVER (0xffffffff)
 
@@ -63,4 +67,15 @@ struct ble_npl_sem {
     volatile uint16_t count;
 };
 
+// --- Called by the MicroPython port -----------------------------------------
+
+void mp_bluetooth_nimble_os_eventq_run_all(void);
+void mp_bluetooth_nimble_os_callout_process(void);
+
+// --- Must be provided by the MicroPython port -------------------------------
+
+void mp_bluetooth_nimble_hci_uart_wfi(void);
+uint32_t mp_bluetooth_nimble_hci_uart_enter_critical(void);
+void mp_bluetooth_nimble_hci_uart_exit_critical(uint32_t atomic_state);
+
 #endif // MICROPY_INCLUDED_STM32_NIMBLE_NIMBLE_NPL_OS_H
diff --git a/extmod/nimble/syscfg/syscfg.h b/extmod/nimble/syscfg/syscfg.h
index 6ac420f35..8a6f2338b 100644
--- a/extmod/nimble/syscfg/syscfg.h
+++ b/extmod/nimble/syscfg/syscfg.h
@@ -2,12 +2,14 @@
 #define MICROPY_INCLUDED_EXTMOD_NIMBLE_SYSCFG_H
 
 #include "py/mphal.h"
-#include "uart.h"
+
+#include "mpnimbleport.h"
 
 void *nimble_malloc(size_t size);
 void nimble_free(void *ptr);
 void *nimble_realloc(void *ptr, size_t size);
 
+// Redirect NimBLE malloc to the GC heap.
 #define malloc(size) nimble_malloc(size)
 #define free(ptr) nimble_free(ptr)
 #define realloc(ptr, size) nimble_realloc(ptr, size)
@@ -88,6 +90,7 @@ int nimble_sprintf(char *str, const char *fmt, ...);
 #define MYNEWT_VAL_BLE_GATT_WRITE_NO_RSP (MYNEWT_VAL_BLE_ROLE_CENTRAL)
 #define MYNEWT_VAL_BLE_GATT_WRITE_RELIABLE (MYNEWT_VAL_BLE_ROLE_CENTRAL)
 #define MYNEWT_VAL_BLE_HOST (1)
+#define MYNEWT_VAL_BLE_HS_AUTO_START (1)
 #define MYNEWT_VAL_BLE_HS_DEBUG (0)
 #define MYNEWT_VAL_BLE_HS_FLOW_CTRL (0)
 #define MYNEWT_VAL_BLE_HS_FLOW_CTRL_ITVL (1000)