Merge pull request #2664 from particle-iot/enhancement/rtl872x-dynami…

…c-wifi-power

[rtl872x] dynamically enable Wi-Fi stack on demand and fix BLE race c…

hal/inc/ble_hal.h

@@ -495,6 +495,15 @@ int hal_ble_stack_init(void* reserved);
  */
 int hal_ble_stack_deinit(void* reserved);
 
+/**
+ * Check if BLE stack is initialized.
+ *
+ * @param[in]   reserved    Reserved for future use.
+ *
+ * @returns     true if it is initialized, otherwise, false.
+ */
+bool hal_ble_is_initialized(void* reserved);
+
 /**
  * Select the antenna for BLE radio.
  *

hal/inc/hal_dynalib_ble.h

@@ -115,6 +115,7 @@ DYNALIB_FN(73, hal_ble, hal_ble_gap_get_pairing_config, int(hal_ble_pairing_conf
 
 DYNALIB_FN(74, hal_ble, hal_ble_gatt_get_att_mtu, ssize_t(hal_ble_conn_handle_t, void*))
 DYNALIB_FN(75, hal_ble, hal_ble_gatt_client_att_mtu_exchange, int(hal_ble_conn_handle_t, void*))
+DYNALIB_FN(76, hal_ble, hal_ble_is_initialized, bool(void*))
 
 DYNALIB_END(hal_ble)
 

hal/network/ncp_client/realtek/rtl_ncp_client.cpp

@@ -202,7 +202,6 @@ int RealtekNcpClient::init(const NcpClientConfig& conf) {
     RCC_PeriphClockCmd(APBPeriph_I2S0, APBPeriph_I2S0_CLOCK, ENABLE);
     RCC_PeriphClockCmd(APBPeriph_SECURITY_ENGINE, APBPeriph_SEC_ENG_CLOCK, ENABLE);
     RCC_PeriphClockCmd(APBPeriph_LXBUS, APBPeriph_LXBUS_CLOCK, ENABLE);
-    SPARK_ASSERT(wifi_on(RTW_MODE_STA) == 0);
     rltkOff();
     return SYSTEM_ERROR_NONE;
 }
@@ -444,30 +443,7 @@ int RealtekNcpClient::scan(WifiScanCallback callback, void* data) {
     if (ctx.results.size() == 0) {
         // Workaround for a weird state we might enter where the wifi driver
         // is not returning any results
-        hal_ble_lock(nullptr);
-        bool advertising = hal_ble_gap_is_advertising(nullptr) ||
-                           hal_ble_gap_is_connecting(nullptr, nullptr) ||
-                           hal_ble_gap_is_connected(nullptr, nullptr);
-        hal_ble_stack_deinit(nullptr);
-        rtwCoexPreventCleanup(0);
-
-        HAL_Delay_Milliseconds(100);
-
-        wifi_off();
-
-        RCC_PeriphClockCmd(APBPeriph_WL, APBPeriph_WL_CLOCK, DISABLE);
-        RCC_PeriphClockCmd(APBPeriph_WL, APBPeriph_WL_CLOCK, ENABLE);
-        rtwCoexCleanup(0);
-
-        SPARK_ASSERT(wifi_on(RTW_MODE_STA) == 0);
-
-        if (hal_ble_stack_init(nullptr) == SYSTEM_ERROR_NONE) {
-            if (advertising) {
-                hal_ble_gap_start_advertising(nullptr);
-            }
-        }
-
-        hal_ble_unlock(nullptr);
+        rtwRadioReset();
     }
     return rtl_error_to_system(rtlError);
 }
@@ -539,6 +515,7 @@ int RealtekNcpClient::rltkOff() {
     LOG(INFO, "rltkOff");
     // This doesn't work
     // wifi_rf_off();
+    rtwRadioRelease(RTW_RADIO_WIFI);
     LOG(INFO, "rltkOff done");
     ncpPowerState(NcpPowerState::OFF);
     return SYSTEM_ERROR_NONE;
@@ -548,6 +525,7 @@ int RealtekNcpClient::rltkOff() {
 int RealtekNcpClient::rltkOn() {
     // This doesn't work
     // wifi_rf_on();
+    rtwRadioAcquire(RTW_RADIO_WIFI);
     ncpPowerState(NcpPowerState::ON);
     return SYSTEM_ERROR_NONE;
 }

hal/src/nRF52840/ble_hal.cpp

@@ -3950,6 +3950,11 @@ int hal_ble_stack_deinit(void* reserved) {
     return SYSTEM_ERROR_NONE;
 }
 
+bool hal_ble_is_initialized(void* reserved) {
+    BleLock lk;
+    return BleObject::getInstance().initialized();
+}
+
 int hal_ble_select_antenna(hal_ble_ant_type_t antenna, void* reserved) {
     return BleObject::getInstance().selectAntenna(antenna);
 }

hal/src/rtl872x/ble_hal.cpp

@@ -1027,12 +1027,7 @@ int BleGap::init() {
     if (initialized_) {
         return SYSTEM_ERROR_NONE;
     }
-    // Access wifiNetworkManager() to make sure that RealtekNcpClient is initialized
-    // and WiFi stack has been initialized as well, as there is a dependency on its state
-    // for btgap to function correctly.
-    const auto mgr = wifiNetworkManager();
-    CHECK_TRUE(mgr, SYSTEM_ERROR_INTERNAL);
-
+    rtwRadioAcquire(RTW_RADIO_BLE);
     state_.raw = 0;
 
     SCOPE_GUARD({
@@ -1138,20 +1133,35 @@ int BleGap::start() {
 int BleGap::stop() {
     // NOTE: ignoring errors
     if (btStackStarted_) {
+        // Abort any commands, e.g. the re-adv command after disconnection.
+        if (cmdThread_ && !os_thread_is_current(cmdThread_)) {
+            if (enqueue(BLE_CMD_EXIT_THREAD) == SYSTEM_ERROR_NONE) {
+                os_thread_join(cmdThread_);
+                cmdThread_ = nullptr;
+            }
+        }
+        if (cmdQueue_) {
+            if (!cmdThread_) {
+                os_queue_destroy(cmdQueue_, nullptr);
+                cmdQueue_ = nullptr;
+            } else {
+                uint8_t command;
+                while (!os_queue_take(cmdQueue_, &command, 0, nullptr)) {}
+            }
+        }
+
         // NOTE: we have to wait for the BLE stack to get initialized otherwise other operations
         // with it may cause race conditions, memory leaks and other problems
         waitState(BleGapDevState().init(GAP_INIT_STATE_STACK_READY), BLE_STATE_DEFAULT_TIMEOUT, true /* force poll */);
         disconnectAll();
         if (isAdvertising_) {
             // This will also wait for advertisements to stop
             stopAdvertising();
-            isAdvertising_ = false;
         }
 
         if (isScanning_) {
             stopScanning();
             le_scan_stop(); // Just in case
-            isScanning_ = false;
         }
 
         // Prevent BLE stack from generating coexistence events, otherwise we may leak memory
@@ -1177,22 +1187,8 @@ int BleGap::stop() {
     // This shoulld be called after BT stack is stopped so that BLE events in queue can be safely cleared.
     BleEventDispatcher::getInstance().stop();
 
-    if (cmdThread_ && !os_thread_is_current(cmdThread_)) {
-        if (enqueue(BLE_CMD_EXIT_THREAD) == SYSTEM_ERROR_NONE) {
-            os_thread_join(cmdThread_);
-            cmdThread_ = nullptr;
-        }
-    }
-    if (cmdQueue_) {
-        if (!cmdThread_) {
-            os_queue_destroy(cmdQueue_, nullptr);
-            cmdQueue_ = nullptr;
-        } else {
-            uint8_t command;
-            while (!os_queue_take(cmdQueue_, &command, 0, nullptr)) {}
-        }
-    }
-
+    isAdvertising_ = false;
+    isScanning_ = false;
     initialized_ = false;
     btStackStarted_ = false;
 
@@ -1217,9 +1213,10 @@ int BleGap::stop() {
         advTimeoutTimer_ = nullptr;
     }
 
-    CHECK(BleGatt::getInstance().deinit());
+    BleGatt::getInstance().deinit();
 
     RCC_PeriphClockCmd(APBPeriph_UART1, APBPeriph_UART1_CLOCK, DISABLE);
+    rtwRadioRelease(RTW_RADIO_BLE);
     return SYSTEM_ERROR_NONE;
 }
 
@@ -1845,8 +1842,11 @@ int BleGap::connectCancel(const hal_ble_addr_t* address) {
     if (!WAIT_TIMED(BLE_OPERATION_TIMEOUT_MS, connecting_)) {
         return SYSTEM_ERROR_TIMEOUT;
     }
-    std::lock_guard<RecursiveMutex> lk(connectionsMutex_);
-    auto connection = fetchConnection(address);
+    BleConnection* connection = nullptr;
+    {
+        std::lock_guard<RecursiveMutex> lk(connectionsMutex_);
+        connection = fetchConnection(address);
+    }
     CHECK_TRUE(connection, SYSTEM_ERROR_NONE); // Connection is not established
     CHECK(disconnect(connection->info.conn_handle));
     return SYSTEM_ERROR_NONE;
@@ -1870,11 +1870,18 @@ int BleGap::disconnect(hal_ble_conn_handle_t connHandle) {
 }
 
 int BleGap::disconnectAll() {
-    std::lock_guard<RecursiveMutex> lk(connectionsMutex_);
-    for (const auto& connection : connections_) {
-        // TODO: check the return value.
-        disconnect(connection.info.conn_handle);
-        // The corresponding connection in the Vector will be removed on the disconnected event.
+    while (1) {
+        BleConnection* connection = nullptr;
+        {
+            std::lock_guard<RecursiveMutex> lk(connectionsMutex_);
+            if (connections_.size() > 0) {
+                connection = &connections_[0];
+            }
+        }
+        if (!connection) {
+            break;
+        }
+        disconnect(connection->info.conn_handle);
     }
     return SYSTEM_ERROR_NONE;
 }
@@ -3441,6 +3448,11 @@ int hal_ble_stack_deinit(void* reserved) {
     return SYSTEM_ERROR_NONE;
 }
 
+bool hal_ble_is_initialized(void* reserved) {
+    BleLock lk;
+    return BleGap::getInstance().initialized();
+}
+
 int hal_ble_select_antenna(hal_ble_ant_type_t antenna, void* reserved) {
     CHECK(selectRadioAntenna((radio_antenna_type)antenna));
     return SYSTEM_ERROR_NONE;

hal/src/rtl872x/rtl_sdk_support.cpp

@@ -34,6 +34,12 @@ extern "C" {
 #include "interrupts_hal.h"
 #include "osdep_service.h"
 #include "concurrent_hal.h"
+#if MODULE_FUNCTION != MOD_FUNC_BOOTLOADER
+#include "delay_hal.h"
+#include "wifi_conf.h"
+#include "ble_hal.h"
+#include "spark_wiring_thread.h"
+#endif
 
 extern "C" {
 
@@ -54,6 +60,13 @@ struct pcoex_reveng {
     _mutex* mutex;
 };
 
+namespace {
+#if MODULE_FUNCTION != MOD_FUNC_BOOTLOADER
+uint8_t radioStatus = RTW_RADIO_NONE;
+RecursiveMutex radioMutex;
+#endif
+}
+
 extern "C" pcoex_reveng* pcoex[4];
 
 extern "C" int rtw_coex_wifi_enable(void* priv, uint32_t state);
@@ -119,6 +132,65 @@ void rtwCoexCleanup(int idx) {
     }
 }
 
+#if MODULE_FUNCTION != MOD_FUNC_BOOTLOADER
+void rtwRadioReset() {
+    std::lock_guard<RecursiveMutex> lk(radioMutex);
+    hal_ble_lock(nullptr);
+    bool bleInitialized = hal_ble_is_initialized(nullptr);
+    bool advertising = hal_ble_gap_is_advertising(nullptr) ||
+                       hal_ble_gap_is_connecting(nullptr, nullptr) ||
+                       hal_ble_gap_is_connected(nullptr, nullptr);
+    if (bleInitialized) {
+        hal_ble_stack_deinit(nullptr);
+    }
+
+    rtwRadioRelease(RTW_RADIO_WIFI);
+    rtwRadioAcquire(RTW_RADIO_WIFI);
+
+    if (bleInitialized) {
+        if (hal_ble_stack_init(nullptr) == 0 && advertising) {
+            hal_ble_gap_start_advertising(nullptr);
+        }
+    }
+    hal_ble_unlock(nullptr);
+}
+
+void rtwRadioAcquire(RtwRadio r) {
+    std::lock_guard<RecursiveMutex> lk(radioMutex);
+    LOG_DEBUG(INFO, "rtwRadioAcquire: %d", r);
+    auto preStatus = radioStatus;
+    radioStatus |= r;
+    if (preStatus != RTW_RADIO_NONE) {
+        LOG(INFO, "WiFi is already on");
+        return;
+    }
+    if (radioStatus != RTW_RADIO_NONE) {
+        RCC_PeriphClockCmd(APBPeriph_WL, APBPeriph_WL_CLOCK, ENABLE);
+        rtwCoexCleanup(0);
+        SPARK_ASSERT(wifi_on(RTW_MODE_STA) == 0);
+        LOG(INFO, "WiFi on");
+    }
+}
+
+void rtwRadioRelease(RtwRadio r) {
+    std::lock_guard<RecursiveMutex> lk(radioMutex);
+    LOG_DEBUG(INFO, "rtwRadioRelease: %d", r);
+    auto preStatus = radioStatus;
+    radioStatus &= ~r;
+    if (preStatus == RTW_RADIO_NONE) {
+        LOG(INFO, "WiFi is already off");
+        return;
+    }
+    if (radioStatus == RTW_RADIO_NONE) {
+        rtwCoexPreventCleanup(0);
+        HAL_Delay_Milliseconds(100);
+        wifi_off();
+        RCC_PeriphClockCmd(APBPeriph_WL, APBPeriph_WL_CLOCK, DISABLE);
+        LOG(INFO, "WiFi off");
+    }
+}
+#endif
+
 
 extern "C" u32 DiagPrintf(const char *fmt, ...);
 extern "C" int DiagVSprintf(char *buf, const char *fmt, const int *dp);

hal/src/rtl872x/rtl_sdk_support.h

@@ -22,6 +22,13 @@
 
 typedef void (*rtl_ipc_callback_t)(void *data, uint32_t irq_status, uint32_t channel);
 
+typedef enum {
+    RTW_RADIO_NONE = 0x00,
+    RTW_RADIO_WIFI = 0x01,
+    RTW_RADIO_BLE = 0x02,
+    RTW_RADIO_ALL = 0x03
+} RtwRadio;
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -41,6 +48,10 @@ void rtwCoexRunEnable(int idx);
 void rtwCoexCleanup(int idx);
 void rtwCoexCleanupMutex(int idx);
 
+void rtwRadioReset();
+void rtwRadioAcquire(RtwRadio r);
+void rtwRadioRelease(RtwRadio r);
+
 #ifdef __cplusplus
 }
 #endif

hal/src/rtl872x/sleep_hal.cpp

@@ -155,16 +155,19 @@ class SleepClass {
         CHECK_TRUE(config, SYSTEM_ERROR_INVALID_ARGUMENT);
         memcpy(&alignedConfig_.config, config, sizeof(hal_sleep_config_t));
 
+        bool bleInitialized = hal_ble_is_initialized(nullptr);
         bool advertising = hal_ble_gap_is_advertising(nullptr) ||
                            hal_ble_gap_is_connecting(nullptr, nullptr) ||
                            hal_ble_gap_is_connected(nullptr, nullptr);
-        hal_ble_stack_deinit(nullptr);
-        // The delay is essential to make sure the resources are successfully freed.
-        // FIXME: Is this still needed? hal_ble_stack_deinit() should wait
-        // for deinitialization to complete?
-        // Leaving as-is for now, but should be reassessed. We are postponing sleep
-        // by 2 seconds all the time.
-        HAL_Delay_Milliseconds(2000);
+        if (bleInitialized) {
+            hal_ble_stack_deinit(nullptr);
+            // The delay is essential to make sure the resources are successfully freed.
+            // FIXME: Is this still needed? hal_ble_stack_deinit() should wait
+            // for deinitialization to complete?
+            // Leaving as-is for now, but should be reassessed. We are postponing sleep
+            // by 2 seconds all the time.
+            HAL_Delay_Milliseconds(2000);
+        }
 
         HAL_USB_Detach();
 
@@ -332,8 +335,8 @@ class SleepClass {
             }
         }
 
-        if (hal_ble_stack_init(nullptr) == SYSTEM_ERROR_NONE) {
-            if (advertising) {
+        if (bleInitialized) {
+            if (hal_ble_stack_init(nullptr) == SYSTEM_ERROR_NONE && advertising) {
                 hal_ble_gap_start_advertising(nullptr);
             }
         }