From 6959a41fbe37e5fbaf52b6d5e4c3c07dbe9d3325 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 08:34:24 -0300 Subject: [PATCH 01/11] fix: HWCDC pin number Fixes HW Serial pin setup. The pins were set up to the wrong value and it could not be correctly configured and used. --- cores/esp32/HWCDC.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 6657af79fd4..b4c5d935849 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -246,7 +246,7 @@ void HWCDC::begin(unsigned long baud) return; } // Setting USB D+ D- pins - uint8_t pin = ESP32_BUS_TYPE_USB_DM; + uint8_t pin = USB_DM_GPIO_NUM; if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ if(!perimanClearPinBus(pin)){ goto err; @@ -255,7 +255,7 @@ void HWCDC::begin(unsigned long baud) if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DM, (void *) this, -1, -1)){ goto err; } - pin = ESP32_BUS_TYPE_USB_DP; + pin = USB_DP_GPIO_NUM; if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ if(!perimanClearPinBus(pin)){ goto err; From d2166654efc8027736cffd1bf48919290dd5b4d2 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 11:56:55 -0300 Subject: [PATCH 02/11] fix: PHY initialization Fixes the PHY initialization. After detaching the pin and ending the HW Serial, a new begin() wouldn't start the CDC because it lacked the proper PHY initialization. --- cores/esp32/HWCDC.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index b4c5d935849..795d96c9741 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -26,6 +26,7 @@ #include "soc/io_mux_reg.h" #pragma GCC diagnostic ignored "-Wvolatile" #include "hal/usb_serial_jtag_ll.h" +#include "hal/usb_phy_ll.h" #pragma GCC diagnostic warning "-Wvolatile" #include "rom/ets_sys.h" #include "driver/usb_serial_jtag.h" @@ -238,13 +239,6 @@ void HWCDC::begin(unsigned long baud) log_e("HW CDC TX Buffer error"); } } - usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_LL_INTR_MASK); - usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET); - if(!intr_handle && esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, hw_cdc_isr_handler, NULL, &intr_handle) != ESP_OK){ - isr_log_e("HW USB CDC failed to init interrupts"); - end(); - return; - } // Setting USB D+ D- pins uint8_t pin = USB_DM_GPIO_NUM; if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ @@ -264,6 +258,15 @@ void HWCDC::begin(unsigned long baud) if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DP, (void *) this, -1, -1)){ goto err; } + // Configure PHY + usb_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); + usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_LL_INTR_MASK); + usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET); + if(!intr_handle && esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, hw_cdc_isr_handler, NULL, &intr_handle) != ESP_OK){ + isr_log_e("HW USB CDC failed to init interrupts"); + end(); + return; + } return; err: From c443868abc860998c7ed74c9339b469cbc050951 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 12:04:59 -0300 Subject: [PATCH 03/11] fix: crashing end() Fixes a crash when calling end() end() treminanates the `tx_ring_buf` but it was not tested for NULL in the ISR and in the cdc0_write_char() causing a crash if those are used. This depends on events and happens eventually. --- cores/esp32/HWCDC.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 795d96c9741..3265f7e5df3 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -87,7 +87,7 @@ static void hw_cdc_isr_handler(void *arg) { } else { connected = true; } - if (usb_serial_jtag_ll_txfifo_writable() == 1) { + if (tx_ring_buf != NULL && usb_serial_jtag_ll_txfifo_writable() == 1) { // We disable the interrupt here so that the interrupt won't be triggered if there is no data to send. usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); size_t queued_size; @@ -165,6 +165,9 @@ bool HWCDC::isCDC_Connected() } static void ARDUINO_ISR_ATTR cdc0_write_char(char c) { + if(tx_ring_buf == NULL) { + return; + } uint32_t tx_timeout_ms = 0; if(HWCDC::isConnected()) { tx_timeout_ms = requested_tx_timeout_ms; @@ -292,6 +295,7 @@ void HWCDC::end() arduino_hw_cdc_event_loop_handle = NULL; } HWCDC::deinit(this); + setDebugOutput(false); connected = false; } From 6c4fbf273091932fb75e0dd5b057c08199a6033d Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 13:08:00 -0300 Subject: [PATCH 04/11] reduces number of debug messages --- cores/esp32/HWCDC.cpp | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 3265f7e5df3..554769107e3 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -242,24 +242,41 @@ void HWCDC::begin(unsigned long baud) log_e("HW CDC TX Buffer error"); } } +<<<<<<< Updated upstream // Setting USB D+ D- pins +======= + // Setting USB D+ D- pins || reduces number of debug messages +>>>>>>> Stashed changes uint8_t pin = USB_DM_GPIO_NUM; - if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ - if(!perimanClearPinBus(pin)){ + if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_USB_DM){ + if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ + if(!perimanClearPinBus(pin)){ + goto err; + } + } + if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DM, (void *) this, -1, -1)){ goto err; } } - if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DM, (void *) this, -1, -1)){ - goto err; - } pin = USB_DP_GPIO_NUM; - if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ - if(!perimanClearPinBus(pin)){ + if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_USB_DP){ + if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ + if(!perimanClearPinBus(pin)){ + goto err; + } + } + if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DP, (void *) this, -1, -1)){ goto err; } } - if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DP, (void *) this, -1, -1)){ - goto err; + // Configure PHY + usb_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); + usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_LL_INTR_MASK); + usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET); + if(!intr_handle && esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, hw_cdc_isr_handler, NULL, &intr_handle) != ESP_OK){ + isr_log_e("HW USB CDC failed to init interrupts"); + end(); + return; } // Configure PHY usb_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); From 0daae595a1c8c19e28760ba9e1c6b6950872c40a Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 13:14:20 -0300 Subject: [PATCH 05/11] fix git stash/commit added lines --- cores/esp32/HWCDC.cpp | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 554769107e3..8d8c71838ae 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -242,11 +242,7 @@ void HWCDC::begin(unsigned long baud) log_e("HW CDC TX Buffer error"); } } -<<<<<<< Updated upstream - // Setting USB D+ D- pins -======= // Setting USB D+ D- pins || reduces number of debug messages ->>>>>>> Stashed changes uint8_t pin = USB_DM_GPIO_NUM; if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_USB_DM){ if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ @@ -278,15 +274,6 @@ void HWCDC::begin(unsigned long baud) end(); return; } - // Configure PHY - usb_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); - usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_LL_INTR_MASK); - usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET); - if(!intr_handle && esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, hw_cdc_isr_handler, NULL, &intr_handle) != ESP_OK){ - isr_log_e("HW USB CDC failed to init interrupts"); - end(); - return; - } return; err: From 94a5b5bd0779cb8712eca06a2d62b74d87d01bfd Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 15:13:03 -0300 Subject: [PATCH 06/11] fixes usb_phy_ll include and call --- cores/esp32/HWCDC.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 8d8c71838ae..1a44c78d2e0 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -26,7 +26,11 @@ #include "soc/io_mux_reg.h" #pragma GCC diagnostic ignored "-Wvolatile" #include "hal/usb_serial_jtag_ll.h" +#if defined __has_include && __has_include ("hal/usb_phy_ll.h") #include "hal/usb_phy_ll.h" +#else +#include "hal/usb_fsls_phy_ll.h" +#endif #pragma GCC diagnostic warning "-Wvolatile" #include "rom/ets_sys.h" #include "driver/usb_serial_jtag.h" @@ -266,7 +270,11 @@ void HWCDC::begin(unsigned long baud) } } // Configure PHY - usb_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); + #if defined __has_include && __has_include ("hal/usb_phy_ll.h") + usb_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); + #else + usb_fsls_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); + #endif usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_LL_INTR_MASK); usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET); if(!intr_handle && esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, hw_cdc_isr_handler, NULL, &intr_handle) != ESP_OK){ From f3887c286e138cdd1f7b4b95a0ab996c8c1332ec Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 16:48:05 -0300 Subject: [PATCH 07/11] roll back --- cores/esp32/HWCDC.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 1a44c78d2e0..3b23b6e81f4 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -26,11 +26,7 @@ #include "soc/io_mux_reg.h" #pragma GCC diagnostic ignored "-Wvolatile" #include "hal/usb_serial_jtag_ll.h" -#if defined __has_include && __has_include ("hal/usb_phy_ll.h") #include "hal/usb_phy_ll.h" -#else -#include "hal/usb_fsls_phy_ll.h" -#endif #pragma GCC diagnostic warning "-Wvolatile" #include "rom/ets_sys.h" #include "driver/usb_serial_jtag.h" @@ -270,11 +266,7 @@ void HWCDC::begin(unsigned long baud) } } // Configure PHY - #if defined __has_include && __has_include ("hal/usb_phy_ll.h") usb_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); - #else - usb_fsls_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); - #endif usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_LL_INTR_MASK); usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET); if(!intr_handle && esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, hw_cdc_isr_handler, NULL, &intr_handle) != ESP_OK){ From d9e5a6f60385c0ffa785aa301cd2b1574af5c629 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 17:40:47 -0300 Subject: [PATCH 08/11] solves HWSerial initialization --- cores/esp32/HWCDC.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 3b23b6e81f4..9814aa5ec77 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -24,9 +24,9 @@ #include "esp_intr_alloc.h" #include "soc/periph_defs.h" #include "soc/io_mux_reg.h" +#include "soc/usb_serial_jtag_struct.h" #pragma GCC diagnostic ignored "-Wvolatile" #include "hal/usb_serial_jtag_ll.h" -#include "hal/usb_phy_ll.h" #pragma GCC diagnostic warning "-Wvolatile" #include "rom/ets_sys.h" #include "driver/usb_serial_jtag.h" @@ -266,7 +266,14 @@ void HWCDC::begin(unsigned long baud) } } // Configure PHY - usb_phy_ll_int_jtag_enable(&USB_SERIAL_JTAG); + // USB_Serial_JTAG use internal PHY + USB_SERIAL_JTAG.conf0.phy_sel = 0; + // Disable software control USB D+ D- pullup pulldown (Device FS: dp_pullup = 1) + USB_SERIAL_JTAG.conf0.pad_pull_override = 0; + // Enable USB D+ pullup + USB_SERIAL_JTAG.conf0.dp_pullup = 1; + // Enable USB pad function + USB_SERIAL_JTAG.conf0.usb_pad_enable = 1; usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_LL_INTR_MASK); usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET); if(!intr_handle && esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, hw_cdc_isr_handler, NULL, &intr_handle) != ESP_OK){ From d40f66ed500de0b7bd4c6104893d76ff2727cbff Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 21:05:15 -0300 Subject: [PATCH 09/11] fixes C6|H2 issue issue with `if(Serial)` not working always --- cores/esp32/HWCDC.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 9814aa5ec77..36d574562c5 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -242,6 +242,12 @@ void HWCDC::begin(unsigned long baud) log_e("HW CDC TX Buffer error"); } } + +#if CONFIG_IDF_TARGET_ESP32C6 || CONFIG_IDF_TARGET_ESP32H2 +// it needs the HW Serial pins to be first deinited in order to allow `if(Serial)` to work :-( + deinit(NULL); + delay(10); +#endif // Setting USB D+ D- pins || reduces number of debug messages uint8_t pin = USB_DM_GPIO_NUM; if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_USB_DM){ From 13a83cecb91c847b5c997b76ae82b701e68c0f8b Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 21:17:33 -0300 Subject: [PATCH 10/11] github commit problem --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 36d574562c5..95021cf88cc 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -244,7 +244,7 @@ void HWCDC::begin(unsigned long baud) } #if CONFIG_IDF_TARGET_ESP32C6 || CONFIG_IDF_TARGET_ESP32H2 -// it needs the HW Serial pins to be first deinited in order to allow `if(Serial)` to work :-( +// the HW Serial pins needs to be first deinited in order to allow `if(Serial)` to work :-( deinit(NULL); delay(10); #endif From e731329366497507e4e14efbd99e4b4b3f7d3f29 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 12 Mar 2024 21:42:42 -0300 Subject: [PATCH 11/11] fixes --- cores/esp32/HWCDC.cpp | 32 +++++++------------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 95021cf88cc..e222cc9c165 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -243,34 +243,16 @@ void HWCDC::begin(unsigned long baud) } } -#if CONFIG_IDF_TARGET_ESP32C6 || CONFIG_IDF_TARGET_ESP32H2 -// the HW Serial pins needs to be first deinited in order to allow `if(Serial)` to work :-( + // the HW Serial pins needs to be first deinited in order to allow `if(Serial)` to work :-( deinit(NULL); - delay(10); -#endif - // Setting USB D+ D- pins || reduces number of debug messages + delay(10); // USB Host has to enumerate it again + + // Peripheral Manager setting for USB D+ D- pins uint8_t pin = USB_DM_GPIO_NUM; - if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_USB_DM){ - if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ - if(!perimanClearPinBus(pin)){ - goto err; - } - } - if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DM, (void *) this, -1, -1)){ - goto err; - } - } + if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DM, (void *) this, -1, -1)) goto err; pin = USB_DP_GPIO_NUM; - if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_USB_DP){ - if(perimanGetPinBusType(pin) != ESP32_BUS_TYPE_INIT){ - if(!perimanClearPinBus(pin)){ - goto err; - } - } - if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DP, (void *) this, -1, -1)){ - goto err; - } - } + if(!perimanSetPinBus(pin, ESP32_BUS_TYPE_USB_DP, (void *) this, -1, -1)) goto err; + // Configure PHY // USB_Serial_JTAG use internal PHY USB_SERIAL_JTAG.conf0.phy_sel = 0;