Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rtl872x] some more fixes #2755

Merged
merged 5 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 11 additions & 31 deletions hal/src/rtl872x/ble_hal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ extern "C" {
#include "scope_guard.h"
#include "rtl_system_error.h"
#include "radio_common.h"
#include "freertos/wrapper.h"

#include "timer_hal.h"

Expand All @@ -92,17 +91,6 @@ using spark::Vector;
using namespace particle;
using namespace particle::ble;

struct pcoex_reveng {
uint32_t state;
uint32_t unknown;
_mutex* mutex;
};

extern "C" pcoex_reveng* pcoex[4];

extern "C" Rltk_wlan_t rltk_wlan_info[NET_IF_NUM];
extern "C" int rtw_coex_wifi_enable(void* priv, uint32_t state);
extern "C" int rtw_coex_bt_enable(void* priv, uint32_t state);
extern "C" void __real_bt_coex_handle_specific_evt(uint8_t* p, uint8_t len);
extern "C" void __wrap_bt_coex_handle_specific_evt(uint8_t* p, uint8_t len);
extern "C" int rltk_coex_set_wifi_slot(u8 wifi_slot);
Expand Down Expand Up @@ -1233,19 +1221,7 @@ int BleGap::stop(bool restore) {
gap_register_vendor_cb([](uint8_t cb_type, void *p_cb_data) -> void {
});
{
// This call makes rtw_coex_run_enable(123, false) not cleanup the mutex
// which might be still accessed by other coexistence tasks
// but still allows rtw_coex_bt_enable to deinitialize BT coexistence.
// Subsequently we restore the state with rtwCoexRunEnable() and rtw_coex_wifi_enable
// will call into rtw_coex_run_enable(123, false) which will finally cleanup the mutex
rtwCoexRunDisable(0);
rtw_coex_bt_enable(*(void**)rltk_wlan_info[0].dev->priv, 0);
HAL_Delay_Milliseconds(100);
rtwCoexRunEnable(0);
rtw_coex_wifi_enable(*(void**)rltk_wlan_info[0].dev->priv, 0);
rtwCoexRunDisable(0);
rtw_coex_wifi_enable(*(void**)rltk_wlan_info[0].dev->priv, 1);
rtwCoexCleanupMutex(0);
rtwCoexStop();
Copy link
Contributor

@eberseth eberseth Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrapped lines above into rtxCoexStop() and apply a mutex within.

}
}
bte_deinit();
Expand Down Expand Up @@ -1500,7 +1476,7 @@ int BleGap::startAdvertising(bool wait) {
// the next scan operation will fail with invalid state.
// Previous stopAdvertising() caused a race condition in the BT stack, try to recover
// Ignore error here
auto r = le_adv_stop();
auto r = WRAP_BLE_EVT_LOCK(le_adv_stop());
if (wait) {
LOCAL_DEBUG("wait GAP_ADV_STATE_IDLE");
if (r != GAP_CAUSE_SUCCESS || waitState(BleGapDevState().adv(GAP_ADV_STATE_IDLE))) {
Expand Down Expand Up @@ -1654,7 +1630,7 @@ int BleGap::startScanning(hal_ble_on_scan_result_cb_t callback, void* context) {

SCOPE_GUARD ({
if (isScanning_) {
const int LE_SCAN_STOP_RETRIES = 2;
const int LE_SCAN_STOP_RETRIES = 1;
for (int i = 0; i < LE_SCAN_STOP_RETRIES; i++) {
// This has seen failing a number of times at least
// with btgap logging enabled. Retry a few times just in case,
Expand All @@ -1667,12 +1643,14 @@ int BleGap::startScanning(hal_ble_on_scan_result_cb_t callback, void* context) {
}
break;
}
HAL_Delay_Milliseconds(100);
HAL_Delay_Milliseconds(10);
}
if (isScanning_) {
// Verified that if the scan state is GAP_SCAN_STATE_STOP, instead of GAP_SCAN_STATE_IDLE,
// the next scan operation will fail with invalid state.
LOG(TRACE, "Failed to stop scanning, resetting stack");
RtlGapDevState s;
le_get_gap_param(GAP_PARAM_DEV_STATE, &s.state);
LOG(TRACE, "Failed to stop scanning, resetting stack, scan state=%d", s.state.gap_scan_state);
resetStack = true;
}
}
Expand All @@ -1697,7 +1675,9 @@ int BleGap::startScanning(hal_ble_on_scan_result_cb_t callback, void* context) {
// GAP_SCAN_STATE_SCANNING may be propagated immediately following the GAP_SCAN_STATE_START
// IMPORTANT: have to poll here, for some reason we may not get notified
if (waitState(BleGapDevState().scan(GAP_SCAN_STATE_SCANNING), BLE_STATE_DEFAULT_TIMEOUT, true /* forcePoll */)) {
LOG(ERROR, "Failed to start scanning.");
RtlGapDevState s;
le_get_gap_param(GAP_PARAM_DEV_STATE, &s.state);
LOG(TRACE, "Failed to start scanning scan state=%d", s.state.gap_scan_state);
// The stack state is messed up, we need to reset the stack.
isScanning_ = false;
resetStack = true;
Expand Down Expand Up @@ -1945,7 +1925,7 @@ int BleGap::disconnect(hal_ble_conn_handle_t connHandle) {
disconnectingHandle_ = BLE_INVALID_CONN_HANDLE;
});
disconnectingHandle_ = connHandle;
CHECK_RTL(le_disconnect(connHandle));
CHECK_RTL(WRAP_BLE_EVT_LOCK(le_disconnect(connHandle)));
if (os_semaphore_take(disconnectSemaphore_, BLE_OPERATION_TIMEOUT_MS, false)) {
SPARK_ASSERT(false);
return SYSTEM_ERROR_TIMEOUT;
Expand Down
5 changes: 5 additions & 0 deletions hal/src/rtl872x/rtl_osdep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ extern "C" int _freertos_create_task(struct task_struct *ptask, const char *name
priority += OS_THREAD_PRIORITY_NETWORK;
}

if (!strcmp(name, "rtw_coex_mailbox_thread")) {
// Occasionally getting a stack overflow assertion
stack_size *= 2;
}

// Copy-paste from freertos_service.c
thread_func_t task_func = NULL;
void *task_ctx = NULL;
Expand Down
23 changes: 23 additions & 0 deletions hal/src/rtl872x/rtl_sdk_support.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ extern "C" {
#include "ble_hal.h"
#include "spark_wiring_thread.h"
#endif
#include "freertos/wrapper.h"

extern "C" {

Expand Down Expand Up @@ -72,6 +73,10 @@ extern "C" pcoex_reveng* pcoex[4];
extern "C" int rtw_coex_wifi_enable(void* priv, uint32_t state);
extern "C" int rtw_coex_bt_enable(void* priv, uint32_t state);

#if MODULE_FUNCTION != MOD_FUNC_BOOTLOADER
extern "C" Rltk_wlan_t rltk_wlan_info[NET_IF_NUM];
#endif // MODULE_FUNCTION != MOD_FUNC_BOOTLOADER

void rtwCoexRunDisable(int idx) {
os_thread_scheduling(false, nullptr);
auto p = pcoex[idx];
Expand Down Expand Up @@ -133,6 +138,24 @@ void rtwCoexCleanup(int idx) {
}

#if MODULE_FUNCTION != MOD_FUNC_BOOTLOADER

void rtwCoexStop() {
std::lock_guard<RecursiveMutex> lk(radioMutex);
// This call makes rtw_coex_run_enable(123, false) not cleanup the mutex
// which might be still accessed by other coexistence tasks
// but still allows rtw_coex_bt_enable to deinitialize BT coexistence.
// Subsequently we restore the state with rtwCoexRunEnable() and rtw_coex_wifi_enable
// will call into rtw_coex_run_enable(123, false) which will finally cleanup the mutex
rtwCoexRunDisable(0);
rtw_coex_bt_enable(*(void**)rltk_wlan_info[0].dev->priv, 0);
HAL_Delay_Milliseconds(100);
rtwCoexRunEnable(0);
rtw_coex_wifi_enable(*(void**)rltk_wlan_info[0].dev->priv, 0);
rtwCoexRunDisable(0);
rtw_coex_wifi_enable(*(void**)rltk_wlan_info[0].dev->priv, 1);
rtwCoexCleanupMutex(0);
}

void rtwRadioReset() {
std::lock_guard<RecursiveMutex> lk(radioMutex);
hal_ble_lock(nullptr);
Expand Down
1 change: 1 addition & 0 deletions hal/src/rtl872x/rtl_sdk_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ void rtwCoexCleanupMutex(int idx);
void rtwRadioReset();
void rtwRadioAcquire(RtwRadio r);
void rtwRadioRelease(RtwRadio r);
void rtwCoexStop();

#ifdef __cplusplus
}
Expand Down
4 changes: 2 additions & 2 deletions system/src/system_connection_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ ConnectionTester::ConnectionTester() {
for (const auto& i: getSupportedInterfaces()) {
struct ConnectionMetrics interfaceDiagnostics = {};
interfaceDiagnostics.interface = i;
interfaceDiagnostics.socketDescriptor = -1; // 0 is a valid socket fd number
metrics_.append(interfaceDiagnostics);
}
}
Expand All @@ -192,7 +193,7 @@ ConnectionTester::~ConnectionTester() {

ConnectionMetrics* ConnectionTester::metricsFromSocketDescriptor(int socketDescriptor) {
for (auto& i : metrics_) {
if (i.socketDescriptor == socketDescriptor) {
if (i.socketDescriptor == socketDescriptor && i.txBuffer && i.rxBuffer /* sanity check */) {
return &i;
}
}
Expand Down Expand Up @@ -376,7 +377,6 @@ int ConnectionTester::testConnections() {

// Step 2: Create, bind, and connect sockets for each network interface to test
for (struct addrinfo* a = info; a != nullptr; a = a->ai_next) {

// For each network interface to test, create + open a socket with the retrieved server address
// If any of the sockets fail to be created + opened with this server address, return an error
for (auto& connectionMetrics: metrics_) {
Expand Down