Forráskód Böngészése

esp_system: usb_console: fix restart when Wi-Fi is working

Previously, reset over USB CDC was done by calling esp_restart from
an interrupt handler. This works only until some restart hook function
is registered using esp_register_shutdown_handler, and the hook
function tries to do something that isn’t allowed in an interrupt
handler. One such case is with Wi-Fi. When Wi-Fi driver is installed,
it registers esp_wifi_stop as a shutdown handler function. However
esp_wifi_stop cannot be called from an ISR, and hence we shouldn’t
call esp_restart from an ISR either.

This commit modifies USB CDC driver to call esp_restart by posting it
to esp_timer task.

Closes https://github.com/espressif/esp-idf/issues/7404
Ivan Grokhotkov 4 éve
szülő
commit
3254f8deae

+ 1 - 0
components/esp_system/linker.lf

@@ -22,6 +22,7 @@ entries:
         usb_console:esp_usb_console_cdc_acm_cb (noflash)
         usb_console:esp_usb_console_dfu_detach_cb (noflash)
         usb_console:esp_usb_console_before_restart (noflash)
+        usb_console:esp_usb_console_on_restart_timeout (noflash)
 
 [mapping:vfs_cdcacm]
 archive: libvfs.a

+ 54 - 1
components/esp_system/port/soc/esp32s2/usb_console.c

@@ -13,8 +13,13 @@
 #include "freertos/task.h"
 #include "freertos/semphr.h"
 #include "esp_system.h"
+#include "esp_log.h"
+#include "esp_timer.h"
+#include "esp_check.h"
 #include "esp_intr_alloc.h"
 #include "esp_private/usb_console.h"
+#include "esp_private/system_internal.h"
+#include "esp_private/startup_internal.h"
 #include "soc/periph_defs.h"
 #include "soc/rtc_cntl_reg.h"
 #include "soc/usb_struct.h"
@@ -51,6 +56,9 @@ static uint8_t cdcmem[CDC_WORK_BUF_SIZE];
 static esp_usb_console_cb_t s_rx_cb;
 static esp_usb_console_cb_t s_tx_cb;
 static void *s_cb_arg;
+static esp_timer_handle_t s_restart_timer;
+
+static const char* TAG = "usb_console";
 
 #ifdef CONFIG_ESP_CONSOLE_USB_CDC_SUPPORT_ETS_PRINTF
 static portMUX_TYPE s_write_lock = portMUX_INITIALIZER_UNLOCKED;
@@ -68,6 +76,9 @@ static inline void write_lock_release(void);
 /* The two functions below need to be revisited in the multicore case */
 _Static_assert(SOC_CPU_CORES_NUM == 1, "usb_osglue_*_int is not multicore capable");
 
+/* Other forward declarations */
+void esp_usb_console_before_restart(void);
+
 /* Called by ROM to disable the interrupts
  * Non-static to allow placement into IRAM by ldgen.
  */
@@ -150,10 +161,37 @@ void esp_usb_console_interrupt(void *arg)
     usb_dc_check_poll_for_interrupts();
     /* Restart can be requested from esp_usb_console_cdc_acm_cb or esp_usb_console_dfu_detach_cb */
     if (s_queue_reboot != REBOOT_NONE) {
-        esp_restart();
+        /* We can't call esp_restart here directly, since this function is called from an ISR.
+         * Instead, start an esp_timer and call esp_restart from the callback.
+         */
+        esp_err_t err = ESP_FAIL;
+        if (s_restart_timer) {
+            /* In case the timer is already running, stop it. No error check since this will fail if
+             * the timer is not running.
+             */
+            esp_timer_stop(s_restart_timer);
+            /* Start the timer again. 50ms seems to be not too long for the user to notice, but
+             * enough for the USB console output to be flushed.
+             */
+            const int restart_timeout_us = 50 * 1000;
+            err = esp_timer_start_once(s_restart_timer, restart_timeout_us);
+        }
+        if (err != ESP_OK) {
+            /* Can't schedule a restart for some reason? Call the "no-OS" restart function directly. */
+            esp_usb_console_before_restart();
+            esp_restart_noos();
+        }
     }
 }
 
+/* Called as esp_timer callback when the restart timeout expires.
+ * Non-static to allow placement into IRAM by ldgen.
+ */
+void esp_usb_console_on_restart_timeout(void *arg)
+{
+    esp_restart();
+}
+
 /* Call the USB interrupt handler while any interrupts are pending,
  * but not more than a few times.
  * Non-static to allow placement into IRAM by ldgen.
@@ -256,6 +294,21 @@ esp_err_t esp_usb_console_init(void)
     return ESP_OK;
 }
 
+/* This function runs as part of the startup code to initialize the restart timer.
+ * This is not done as part of esp_usb_console_init since that function is called
+ * too early, before esp_timer is fully initialized.
+ * This gets called a bit later in the process when we can already register a timer.
+ */
+ESP_SYSTEM_INIT_FN(esp_usb_console_init_restart_timer, BIT(0), 220)
+{
+    esp_timer_create_args_t timer_create_args = {
+        .callback = &esp_usb_console_on_restart_timeout,
+        .name = "usb_console_restart"
+    };
+    ESP_RETURN_ON_ERROR(esp_timer_create(&timer_create_args, &s_restart_timer), TAG, "failed to create the restart timer");
+    return ESP_OK;
+}
+
 /* Non-static to allow placement into IRAM by ldgen.
  * Must be called with the write lock held.
  */

+ 4 - 0
components/esp_system/system_init_fn.txt

@@ -26,3 +26,7 @@
 # the rest of the components which are initialized from startup.c
 # [refactor-todo]: move init calls into respective components
 200: init_components0 in components/esp_system/startup.c on BIT(0)
+
+# usb_console needs to create an esp_timer at startup.
+# This can be done only after esp_timer initialization, which is now in init_components0.
+220: esp_usb_console_init_restart_timer in components/esp_system/port/soc/esp32s2/usb_console.c on BIT(0)