Blocking USB driver API (#2009)

* invalidate memmgt thread dict
* Core: rollback memmgt thread dict invalidation
* Dialogs: move api lock to toolbox
* HAL: blocking usb API
* HAL usb: fix api return data
* HAL usb: api optimization
* api lock: test results
* Fix build errors
* DAP Link: fix imports
* Crash when malloc in ISR
* Fix dap-link copypaste error
* Moar memory management crashes.
* Crash when malloc in IRQ, not ISR
* USB-UART: Blocking VCP mode switch

Co-authored-by: nminaylov <nm29719@gmail.com>
Co-authored-by: Aleksandr Kutuzov <alleteam@gmail.com>

applications/main/gpio/gpio_app_i.h

@@ -29,6 +29,7 @@ struct GpioApp {
     GpioTest* gpio_test;
     GpioUsbUart* gpio_usb_uart;
     UsbUartBridge* usb_uart_bridge;
+    UsbUartConfig* usb_uart_cfg;
 };
 
 typedef enum {

applications/main/gpio/gpio_custom_event.h

@@ -9,4 +9,5 @@ typedef enum {
     GpioCustomEventErrorBack,
 
     GpioUsbUartEventConfig,
+    GpioUsbUartEventConfigSet,
 } GpioCustomEvent;

applications/main/gpio/scenes/gpio_scene_usb_uart_config.c

@@ -9,8 +9,6 @@ typedef enum {
     UsbUartLineIndexFlow,
 } LineIndex;
 
-static UsbUartConfig* cfg_set;
-
 static const char* vcp_ch[] = {"0 (CLI)", "1"};
 static const char* uart_ch[] = {"13,14", "15,16"};
 static const char* flow_pins[] = {"None", "2,3", "6,7", "16,15"};
@@ -28,8 +26,14 @@ static const uint32_t baudrate_list[] = {
 };
 
 bool gpio_scene_usb_uart_cfg_on_event(void* context, SceneManagerEvent event) {
-    UNUSED(context);
-    UNUSED(event);
+    GpioApp* app = context;
+    furi_assert(app);
+    if(event.type == SceneManagerEventTypeCustom) {
+        if(event.event == GpioUsbUartEventConfigSet) {
+            usb_uart_set_config(app->usb_uart_bridge, app->usb_uart_cfg);
+            return true;
+        }
+    }
     return false;
 }
 
@@ -38,55 +42,59 @@ void line_ensure_flow_invariant(GpioApp* app) {
     // selected. This function enforces that invariant by resetting flow_pins
     // to None if it is configured to 16,15 when LPUART is selected.
 
-    uint8_t available_flow_pins = cfg_set->uart_ch == FuriHalUartIdLPUART1 ? 3 : 4;
+    uint8_t available_flow_pins = app->usb_uart_cfg->uart_ch == FuriHalUartIdLPUART1 ? 3 : 4;
     VariableItem* item = app->var_item_flow;
     variable_item_set_values_count(item, available_flow_pins);
 
-    if(cfg_set->flow_pins >= available_flow_pins) {
-        cfg_set->flow_pins = 0;
-        usb_uart_set_config(app->usb_uart_bridge, cfg_set);
+    if(app->usb_uart_cfg->flow_pins >= available_flow_pins) {
+        app->usb_uart_cfg->flow_pins = 0;
 
-        variable_item_set_current_value_index(item, cfg_set->flow_pins);
-        variable_item_set_current_value_text(item, flow_pins[cfg_set->flow_pins]);
+        variable_item_set_current_value_index(item, app->usb_uart_cfg->flow_pins);
+        variable_item_set_current_value_text(item, flow_pins[app->usb_uart_cfg->flow_pins]);
     }
 }
 
 static void line_vcp_cb(VariableItem* item) {
     GpioApp* app = variable_item_get_context(item);
+    furi_assert(app);
     uint8_t index = variable_item_get_current_value_index(item);
 
     variable_item_set_current_value_text(item, vcp_ch[index]);
 
-    cfg_set->vcp_ch = index;
-    usb_uart_set_config(app->usb_uart_bridge, cfg_set);
+    app->usb_uart_cfg->vcp_ch = index;
+    view_dispatcher_send_custom_event(app->view_dispatcher, GpioUsbUartEventConfigSet);
 }
 
 static void line_port_cb(VariableItem* item) {
     GpioApp* app = variable_item_get_context(item);
+    furi_assert(app);
     uint8_t index = variable_item_get_current_value_index(item);
 
     variable_item_set_current_value_text(item, uart_ch[index]);
 
     if(index == 0)
-        cfg_set->uart_ch = FuriHalUartIdUSART1;
+        app->usb_uart_cfg->uart_ch = FuriHalUartIdUSART1;
     else if(index == 1)
-        cfg_set->uart_ch = FuriHalUartIdLPUART1;
-    usb_uart_set_config(app->usb_uart_bridge, cfg_set);
+        app->usb_uart_cfg->uart_ch = FuriHalUartIdLPUART1;
+
     line_ensure_flow_invariant(app);
+    view_dispatcher_send_custom_event(app->view_dispatcher, GpioUsbUartEventConfigSet);
 }
 
 static void line_flow_cb(VariableItem* item) {
     GpioApp* app = variable_item_get_context(item);
+    furi_assert(app);
     uint8_t index = variable_item_get_current_value_index(item);
 
     variable_item_set_current_value_text(item, flow_pins[index]);
 
-    cfg_set->flow_pins = index;
-    usb_uart_set_config(app->usb_uart_bridge, cfg_set);
+    app->usb_uart_cfg->flow_pins = index;
+    view_dispatcher_send_custom_event(app->view_dispatcher, GpioUsbUartEventConfigSet);
 }
 
 static void line_baudrate_cb(VariableItem* item) {
     GpioApp* app = variable_item_get_context(item);
+    furi_assert(app);
     uint8_t index = variable_item_get_current_value_index(item);
 
     char br_text[8];
@@ -94,28 +102,29 @@ static void line_baudrate_cb(VariableItem* item) {
     if(index > 0) {
         snprintf(br_text, 7, "%lu", baudrate_list[index - 1]);
         variable_item_set_current_value_text(item, br_text);
-        cfg_set->baudrate = baudrate_list[index - 1];
+        app->usb_uart_cfg->baudrate = baudrate_list[index - 1];
     } else {
         variable_item_set_current_value_text(item, baudrate_mode[index]);
-        cfg_set->baudrate = 0;
+        app->usb_uart_cfg->baudrate = 0;
     }
-    cfg_set->baudrate_mode = index;
-    usb_uart_set_config(app->usb_uart_bridge, cfg_set);
+    app->usb_uart_cfg->baudrate_mode = index;
+    view_dispatcher_send_custom_event(app->view_dispatcher, GpioUsbUartEventConfigSet);
 }
 
 void gpio_scene_usb_uart_cfg_on_enter(void* context) {
     GpioApp* app = context;
+    furi_assert(app);
     VariableItemList* var_item_list = app->var_item_list;
 
-    cfg_set = malloc(sizeof(UsbUartConfig));
-    usb_uart_get_config(app->usb_uart_bridge, cfg_set);
+    app->usb_uart_cfg = malloc(sizeof(UsbUartConfig));
+    usb_uart_get_config(app->usb_uart_bridge, app->usb_uart_cfg);
 
     VariableItem* item;
     char br_text[8];
 
     item = variable_item_list_add(var_item_list, "USB Channel", 2, line_vcp_cb, app);
-    variable_item_set_current_value_index(item, cfg_set->vcp_ch);
-    variable_item_set_current_value_text(item, vcp_ch[cfg_set->vcp_ch]);
+    variable_item_set_current_value_index(item, app->usb_uart_cfg->vcp_ch);
+    variable_item_set_current_value_text(item, vcp_ch[app->usb_uart_cfg->vcp_ch]);
 
     item = variable_item_list_add(
         var_item_list,
@@ -123,22 +132,23 @@ void gpio_scene_usb_uart_cfg_on_enter(void* context) {
         sizeof(baudrate_list) / sizeof(baudrate_list[0]) + 1,
         line_baudrate_cb,
         app);
-    variable_item_set_current_value_index(item, cfg_set->baudrate_mode);
-    if(cfg_set->baudrate_mode > 0) {
-        snprintf(br_text, 7, "%lu", baudrate_list[cfg_set->baudrate_mode - 1]);
+    variable_item_set_current_value_index(item, app->usb_uart_cfg->baudrate_mode);
+    if(app->usb_uart_cfg->baudrate_mode > 0) {
+        snprintf(br_text, 7, "%lu", baudrate_list[app->usb_uart_cfg->baudrate_mode - 1]);
         variable_item_set_current_value_text(item, br_text);
     } else {
-        variable_item_set_current_value_text(item, baudrate_mode[cfg_set->baudrate_mode]);
+        variable_item_set_current_value_text(
+            item, baudrate_mode[app->usb_uart_cfg->baudrate_mode]);
     }
 
     item = variable_item_list_add(var_item_list, "UART Pins", 2, line_port_cb, app);
-    variable_item_set_current_value_index(item, cfg_set->uart_ch);
-    variable_item_set_current_value_text(item, uart_ch[cfg_set->uart_ch]);
+    variable_item_set_current_value_index(item, app->usb_uart_cfg->uart_ch);
+    variable_item_set_current_value_text(item, uart_ch[app->usb_uart_cfg->uart_ch]);
 
     item = variable_item_list_add(
         var_item_list, "RTS/DTR Pins", COUNT_OF(flow_pins), line_flow_cb, app);
-    variable_item_set_current_value_index(item, cfg_set->flow_pins);
-    variable_item_set_current_value_text(item, flow_pins[cfg_set->flow_pins]);
+    variable_item_set_current_value_index(item, app->usb_uart_cfg->flow_pins);
+    variable_item_set_current_value_text(item, flow_pins[app->usb_uart_cfg->flow_pins]);
     app->var_item_flow = item;
     line_ensure_flow_invariant(app);
 
@@ -155,5 +165,5 @@ void gpio_scene_usb_uart_cfg_on_exit(void* context) {
         GpioAppViewUsbUartCfg,
         variable_item_list_get_selected_item_index(app->var_item_list));
     variable_item_list_reset(app->var_item_list);
-    free(cfg_set);
+    free(app->usb_uart_cfg);
 }

applications/main/gpio/usb_uart_bridge.c

@@ -3,6 +3,7 @@
 #include <furi_hal_usb_cdc.h>
 #include "usb_cdc.h"
 #include "cli/cli_vcp.h"
+#include <toolbox/api_lock.h>
 #include "cli/cli.h"
 
 #define USB_CDC_PKT_LEN CDC_DATA_SZ
@@ -51,6 +52,8 @@ struct UsbUartBridge {
 
     UsbUartState st;
 
+    FuriApiLock cfg_lock;
+
     uint8_t rx_buf[USB_CDC_PKT_LEN];
 };
 
@@ -244,6 +247,7 @@ static int32_t usb_uart_worker(void* context) {
                 usb_uart->cfg.flow_pins = usb_uart->cfg_new.flow_pins;
                 events |= WorkerEvtCtrlLineSet;
             }
+            api_lock_unlock(usb_uart->cfg_lock);
         }
         if(events & WorkerEvtLineCfgSet) {
             if(usb_uart->cfg.baudrate == 0)
@@ -352,8 +356,10 @@ void usb_uart_disable(UsbUartBridge* usb_uart) {
 void usb_uart_set_config(UsbUartBridge* usb_uart, UsbUartConfig* cfg) {
     furi_assert(usb_uart);
     furi_assert(cfg);
+    usb_uart->cfg_lock = api_lock_alloc_locked();
     memcpy(&(usb_uart->cfg_new), cfg, sizeof(UsbUartConfig));
     furi_thread_flags_set(furi_thread_get_id(usb_uart->thread), WorkerEvtCfgChange);
+    api_lock_wait_unlock_and_free(usb_uart->cfg_lock);
 }
 
 void usb_uart_get_config(UsbUartBridge* usb_uart, UsbUartConfig* cfg) {

applications/plugins/dap_link/dap_link.c

@@ -247,7 +247,6 @@ static int32_t dap_process(void* p) {
 
     // deinit usb
     furi_hal_usb_set_config(usb_config_prev, NULL);
-    dap_common_wait_for_deinit();
     dap_common_usb_free_name();
     dap_deinit_gpio(swd_pins_prev);
     return 0;

applications/plugins/dap_link/usb/dap_v2_usb.c

@@ -618,23 +618,12 @@ static void hid_init(usbd_device* dev, FuriHalUsbInterface* intf, void* ctx) {
     if(dap_state.semaphore_v2 == NULL) dap_state.semaphore_v2 = furi_semaphore_alloc(1, 1);
     if(dap_state.semaphore_cdc == NULL) dap_state.semaphore_cdc = furi_semaphore_alloc(1, 1);
 
-    usb_hid.dev_descr->idVendor = DAP_HID_VID;
-    usb_hid.dev_descr->idProduct = DAP_HID_PID;
-
     usbd_reg_config(dev, hid_ep_config);
     usbd_reg_control(dev, hid_control);
 
     usbd_connect(dev, true);
 }
 
-static bool deinit_flag = false;
-
-void dap_common_wait_for_deinit() {
-    while(!deinit_flag) {
-        furi_delay_ms(50);
-    }
-}
-
 static void hid_deinit(usbd_device* dev) {
     dap_state.usb_dev = NULL;
 
@@ -647,12 +636,6 @@ static void hid_deinit(usbd_device* dev) {
 
     usbd_reg_config(dev, NULL);
     usbd_reg_control(dev, NULL);
-
-    free(usb_hid.str_manuf_descr);
-    free(usb_hid.str_prod_descr);
-
-    FURI_SW_MEMBARRIER();
-    deinit_flag = true;
 }
 
 static void hid_on_wakeup(usbd_device* dev) {

applications/plugins/dap_link/usb/dap_v2_usb.h

@@ -51,5 +51,3 @@ void dap_common_usb_set_state_callback(DapStateCallback callback);
 void dap_common_usb_alloc_name(const char* name);
 
 void dap_common_usb_free_name();
-
-void dap_common_wait_for_deinit();

applications/services/dialogs/dialogs.c

@@ -1,6 +1,6 @@
 #include "dialogs/dialogs_message.h"
 #include "dialogs_i.h"
-#include "dialogs_api_lock.h"
+#include <toolbox/api_lock.h>
 #include "dialogs_module_file_browser.h"
 #include "dialogs_module_message.h"
 
@@ -35,7 +35,7 @@ static void dialogs_app_process_message(DialogsApp* app, DialogsAppMessage* mess
             dialogs_app_process_module_message(&message->data->dialog);
         break;
     }
-    API_LOCK_UNLOCK(message->lock);
+    api_lock_unlock(message->lock);
 }
 
 int32_t dialogs_srv(void* p) {

applications/services/dialogs/dialogs_api.c

@@ -1,6 +1,6 @@
 #include "dialogs/dialogs_message.h"
 #include "dialogs_i.h"
-#include "dialogs_api_lock.h"
+#include <toolbox/api_lock.h>
 #include <assets_icons.h>
 
 /****************** File browser ******************/
@@ -10,7 +10,7 @@ bool dialog_file_browser_show(
     FuriString* result_path,
     FuriString* path,
     const DialogsFileBrowserOptions* options) {
-    FuriApiLock lock = API_LOCK_INIT_LOCKED();
+    FuriApiLock lock = api_lock_alloc_locked();
     furi_check(lock != NULL);
 
     DialogsAppData data = {
@@ -35,7 +35,7 @@ bool dialog_file_browser_show(
 
     furi_check(
         furi_message_queue_put(context->message_queue, &message, FuriWaitForever) == FuriStatusOk);
-    API_LOCK_WAIT_UNTIL_UNLOCK_AND_FREE(lock);
+    api_lock_wait_unlock_and_free(lock);
 
     return return_data.bool_value;
 }
@@ -43,7 +43,7 @@ bool dialog_file_browser_show(
 /****************** Message ******************/
 
 DialogMessageButton dialog_message_show(DialogsApp* context, const DialogMessage* dialog_message) {
-    FuriApiLock lock = API_LOCK_INIT_LOCKED();
+    FuriApiLock lock = api_lock_alloc_locked();
     furi_check(lock != NULL);
 
     DialogsAppData data = {
@@ -61,7 +61,7 @@ DialogMessageButton dialog_message_show(DialogsApp* context, const DialogMessage
 
     furi_check(
         furi_message_queue_put(context->message_queue, &message, FuriWaitForever) == FuriStatusOk);
-    API_LOCK_WAIT_UNTIL_UNLOCK_AND_FREE(lock);
+    api_lock_wait_unlock_and_free(lock);
 
     return return_data.dialog_value;
 }

applications/services/dialogs/dialogs_api_lock.h

@@ -1,18 +0,0 @@
-#pragma once
-
-typedef FuriEventFlag* FuriApiLock;
-
-#define API_LOCK_EVENT (1U << 0)
-
-#define API_LOCK_INIT_LOCKED() furi_event_flag_alloc();
-
-#define API_LOCK_WAIT_UNTIL_UNLOCK(_lock) \
-    furi_event_flag_wait(_lock, API_LOCK_EVENT, FuriFlagWaitAny, FuriWaitForever);
-
-#define API_LOCK_FREE(_lock) furi_event_flag_free(_lock);
-
-#define API_LOCK_UNLOCK(_lock) furi_event_flag_set(_lock, API_LOCK_EVENT);
-
-#define API_LOCK_WAIT_UNTIL_UNLOCK_AND_FREE(_lock) \
-    API_LOCK_WAIT_UNTIL_UNLOCK(_lock);             \
-    API_LOCK_FREE(_lock);

applications/services/dialogs/dialogs_message.h

@@ -1,7 +1,7 @@
 #pragma once
 #include <furi.h>
 #include "dialogs_i.h"
-#include "dialogs_api_lock.h"
+#include <toolbox/api_lock.h>
 
 #ifdef __cplusplus
 extern "C" {

applications/services/dialogs/dialogs_module_file_browser.c

@@ -1,5 +1,5 @@
 #include "dialogs_i.h"
-#include "dialogs_api_lock.h"
+#include <toolbox/api_lock.h>
 #include "gui/modules/file_browser.h"
 
 typedef struct {
@@ -11,14 +11,14 @@ static void dialogs_app_file_browser_back_callback(void* context) {
     furi_assert(context);
     DialogsAppFileBrowserContext* file_browser_context = context;
     file_browser_context->result = false;
-    API_LOCK_UNLOCK(file_browser_context->lock);
+    api_lock_unlock(file_browser_context->lock);
 }
 
 static void dialogs_app_file_browser_callback(void* context) {
     furi_assert(context);
     DialogsAppFileBrowserContext* file_browser_context = context;
     file_browser_context->result = true;
-    API_LOCK_UNLOCK(file_browser_context->lock);
+    api_lock_unlock(file_browser_context->lock);
 }
 
 bool dialogs_app_process_module_file_browser(const DialogsAppMessageDataFileBrowser* data) {
@@ -27,7 +27,7 @@ bool dialogs_app_process_module_file_browser(const DialogsAppMessageDataFileBrow
 
     DialogsAppFileBrowserContext* file_browser_context =
         malloc(sizeof(DialogsAppFileBrowserContext));
-    file_browser_context->lock = API_LOCK_INIT_LOCKED();
+    file_browser_context->lock = api_lock_alloc_locked();
 
     ViewHolder* view_holder = view_holder_alloc();
     view_holder_attach_to_gui(view_holder, gui);
@@ -44,7 +44,7 @@ bool dialogs_app_process_module_file_browser(const DialogsAppMessageDataFileBrow
 
     view_holder_set_view(view_holder, file_browser_get_view(file_browser));
     view_holder_start(view_holder);
-    API_LOCK_WAIT_UNTIL_UNLOCK(file_browser_context->lock);
+    api_lock_wait_unlock(file_browser_context->lock);
 
     ret = file_browser_context->result;
 
@@ -52,7 +52,7 @@ bool dialogs_app_process_module_file_browser(const DialogsAppMessageDataFileBrow
     view_holder_free(view_holder);
     file_browser_stop(file_browser);
     file_browser_free(file_browser);
-    API_LOCK_FREE(file_browser_context->lock);
+    api_lock_free(file_browser_context->lock);
     free(file_browser_context);
     furi_record_close(RECORD_GUI);
 

applications/services/dialogs/dialogs_module_message.c

@@ -1,5 +1,5 @@
 #include "dialogs_i.h"
-#include "dialogs_api_lock.h"
+#include <toolbox/api_lock.h>
 #include <gui/modules/dialog_ex.h>
 
 typedef struct {
@@ -30,7 +30,7 @@ static void dialogs_app_message_back_callback(void* context) {
     furi_assert(context);
     DialogsAppMessageContext* message_context = context;
     message_context->result = DialogMessageButtonBack;
-    API_LOCK_UNLOCK(message_context->lock);
+    api_lock_unlock(message_context->lock);
 }
 
 static void dialogs_app_message_callback(DialogExResult result, void* context) {
@@ -49,7 +49,7 @@ static void dialogs_app_message_callback(DialogExResult result, void* context) {
     default:
         break;
     }
-    API_LOCK_UNLOCK(message_context->lock);
+    api_lock_unlock(message_context->lock);
 }
 
 DialogMessageButton dialogs_app_process_module_message(const DialogsAppMessageDataDialog* data) {
@@ -57,7 +57,7 @@ DialogMessageButton dialogs_app_process_module_message(const DialogsAppMessageDa
     Gui* gui = furi_record_open(RECORD_GUI);
     const DialogMessage* message = data->message;
     DialogsAppMessageContext* message_context = malloc(sizeof(DialogsAppMessageContext));
-    message_context->lock = API_LOCK_INIT_LOCKED();
+    message_context->lock = api_lock_alloc_locked();
 
     ViewHolder* view_holder = view_holder_alloc();
     view_holder_attach_to_gui(view_holder, gui);
@@ -87,14 +87,14 @@ DialogMessageButton dialogs_app_process_module_message(const DialogsAppMessageDa
 
     view_holder_set_view(view_holder, dialog_ex_get_view(dialog_ex));
     view_holder_start(view_holder);
-    API_LOCK_WAIT_UNTIL_UNLOCK(message_context->lock);
+    api_lock_wait_unlock(message_context->lock);
 
     ret = message_context->result;
 
     view_holder_stop(view_holder);
     view_holder_free(view_holder);
     dialog_ex_free(dialog_ex);
-    API_LOCK_FREE(message_context->lock);
+    api_lock_free(message_context->lock);
     free(message_context);
     furi_record_close(RECORD_GUI);