8000 esp32: Fix ESP32-C3 usb serial/jtag on IDF v5.0.4, clean up native USB-CDC by projectgus · Pull Request #15727 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

esp32: Fix ESP32-C3 usb serial/jtag on IDF v5.0.4, clean up native USB-CDC #15727

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 3, 2024
Merged

esp32: Fix ESP32-C3 usb serial/jtag on IDF v5.0.4, clean up native USB-CDC #15727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ad6297
esp32: Fix ESP32-C3 USB serial/jtag peripheral pre-IDF 5.1.
projectgus Aug 27, 2024
5e692d0
esp32: Add MICROPY_HW_USB_CDC macro for native USB-CDC serial.
projectgus Aug 27, 2024
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
2 changes: 1 addition & 1 deletion ports/esp32/machine_pin.c
View file
Open in desktop
10000
Original file line number Diff line numberDiff line change
Expand Up @@ -159,7 +159,7 @@ static mp_obj_t machine_pin_obj_init_helper(const machine_pin_obj_t *self, size_
}
}

#if CONFIG_IDF_TARGET_ESP32C3 && CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#if CONFIG_IDF_TARGET_ESP32C3 && MICROPY_HW_ESP_USB_SERIAL_JTAG
if (index == 18 || index == 19) {
CLEAR_PERI_REG_MASK(USB_SERIAL_JTAG_CONF0_REG, USB_SERIAL_JTAG_USB_PAD_ENABLE);
}
Expand Down
2 changes: 1 addition & 1 deletion ports/esp32/machine_pin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
#define MICROPY_HW_ENABLE_GPIO11 (1)
#define MICROPY_HW_ENABLE_GPIO12 (1)
#define MICROPY_HW_ENABLE_GPIO13 (1)
#if !CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#if !MICROPY_HW_ESP_USB_SERIAL_JTAG
#define MICROPY_HW_ENABLE_GPIO18 (1)
#define MICROPY_HW_ENABLE_GPIO19 (1)
#endif
Expand Down
4 changes: 2 additions & 2 deletions ports/esp32/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ void mp_task(void *pvParameter) {
#if MICROPY_PY_THREAD
mp_thread_init(pxTaskGetStackStart(NULL), MICROPY_TASK_STACK_SIZE / sizeof(uintptr_t));
#endif
#if CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#if MICROPY_HW_ESP_USB_SERIAL_JTAG
usb_serial_jtag_init();
#elif CONFIG_USB_OTG_SUPPORTED
#elif MICROPY_HW_USB_CDC
Copy link
Member

Choose a reason for hiding this comment

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

I like this, using MICROPY_HW_xxx config options exclusively makes it easier to follow.

But, it seems this will clash with #15108, which uses MICROPY_HW_ENABLE_USBDEV here instead of MICROPY_HW_USB_CDC. Maybe the former is better because you want USB to be initialised if any USB mode is used (eg just MSC)?

I guess a lot of this PR will clash with #15108, eg the changes to mphalport.c. Don't want to have our efforts here wasted if #15108 will need to change things anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, agree that eventually we want to split this into MICROPY_HW_ENABLE_USBDEV and MICROPY_HW_USB_CDC the same as the rp2 port. This did cross my mind when creating this, but I figured at the moment these two macros would be equivalent - so leave splitting them out for when introducing the functionality that needs to differentiate them. I can see the case for adding it now, though - a little less churn.

@andrewleech Is doing this one way or the other way going to be easier for you?

Copy link
Contributor Author
@projectgus projectgus Sep 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Looked at #15108 again and can see the point, so I've pushed commit efe88a2 that adds MICROPY_HW_ENABLE_USBDEV. It's a bit odd because if you build with #define MICROPY_HW_ENABLE_USBDEV (1) and #define MICROPY_HW_USB_CDC (0) then the port enumerates as a USB-CDC device (as the descriptor is configured in ESP-IDF) but the port doesn't do anything.

It might be better if we drop that commit from here, and @andrewleech adds it to his branch the next time he rebases #15108. Not sure.

(EDIT: Updated commit hash after rebase and reformat.)

Copy link
Member

Choose a reason for hiding this comment

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

Let's make the change to use MICROPY_HW_ENABLE_USBDEV in #15108 instead. That keeps this PR simple.

usb_init();
#endif
#if MICROPY_HW_ENABLE_UART_REPL
Expand Down
14 changes: 14 additions & 0 deletions ports/esp32/mpconfigport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,20 @@ typedef long mp_off_t;
// board specifics
#define MICROPY_PY_SYS_PLATFORM "esp32"

// Enable stdio over native USB peripheral CDC via TinyUSB
#ifndef MICROPY_HW_USB_CDC
#define MICROPY_HW_USB_CDC (SOC_USB_OTG_SUPPORTED)
#endif

// Enable stdio over USB Serial/JTAG peripheral
#ifndef MICROPY_HW_ESP_USB_SERIAL_JTAG
#define MICROPY_HW_ESP_USB_SERIAL_JTAG (SOC_USB_SERIAL_JTAG_SUPPORTED && !MICROPY_HW_USB_CDC)
#endif

#if MICROPY_HW_USB_CDC && MICROPY_HW_ESP_USB_SERIAL_JTAG
#error "Invalid build config: Can't enable both native USB and USB Serial/JTAG peripheral"
#endif

// ESP32-S3 extended IO for 47 & 48
#ifndef MICROPY_HW_ESP32S3_EXTENDED_IO
#define MICROPY_HW_ESP32S3_EXTENDED_IO (1)
Expand Down
8 changes: 4 additions & 4 deletions ports/esp32/mphalport.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void check_esp_err_(esp_err_t code, const char *func, const int line, const char

uintptr_t mp_hal_stdio_poll(uintptr_t poll_flags) {
uintptr_t ret = 0;
#if CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#if MICROPY_HW_ESP_USB_SERIAL_JTAG
usb_serial_jtag_poll_rx();
#endif
if ((poll_flags & MP_STREAM_POLL_RD) && stdin_ringbuf.iget != stdin_ringbuf.iput) {
Expand All @@ -118,7 +118,7 @@ uintptr_t mp_hal_stdio_poll(uintptr_t poll_flags) {

int mp_hal_stdin_rx_chr(void) {
for (;;) {
#if CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#if MICROPY_HW_ESP_USB_SERIAL_JTAG
usb_serial_jtag_poll_rx();
#endif
int c = ringbuf_get(&stdin_ringbuf);
Expand All @@ -143,10 +143,10 @@ mp_uint_t mp_hal_stdout_tx_strn(const char *str, size_t len) {
if (release_gil) {
MP_THREAD_GIL_EXIT();
}
#if CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#if MICROPY_HW_ESP_USB_SERIAL_JTAG
usb_serial_jtag_tx_strn(str, len);
did_write = true;
#elif CONFIG_USB_OTG_SUPPORTED
#elif MICROPY_HW_USB_CDC
usb_tx_strn(str, len);
did_write = true;
#endif
Expand Down
2 changes: 1 addition & 1 deletion ports/esp32/uart.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

// Whether to enable the REPL on a UART.
#ifndef MICROPY_HW_ENABLE_UART_REPL
#define MICROPY_HW_ENABLE_UART_REPL (!CONFIG_USB_OTG_SUPPORTED && !CONFIG_ESP_CONSOLE_USB_CDC && !CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED)
#define MICROPY_HW_ENABLE_UART_REPL (!MICROPY_HW_USB_CDC && !MICROPY_HW_ESP_USB_SERIAL_JTAG)
#endif

#if MICROPY_HW_ENABLE_UART_REPL
Expand Down
4 changes: 2 additions & 2 deletions ports/esp32/usb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#include "py/mphal.h"
#include "usb.h"

#if CONFIG_USB_OTG_SUPPORTED && !CONFIG_ESP_CONSOLE_USB_CDC && !CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#if MICROPY_HW_USB_CDC

#include "esp_timer.h"
#ifndef NO_QSTR
Expand Down Expand Up @@ -100,4 +100,4 @@ void usb_tx_strn(const char *str, size_t len) {
}
}

#endif // CONFIG_USB_OTG_SUPPORTED && !CONFIG_ESP_CONSOLE_USB_CDC && !CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#endif // MICROPY_HW_USB_CDC
4 changes: 2 additions & 2 deletions ports/esp32/usb_serial_jtag.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#include "py/mphal.h"
#include "usb_serial_jtag.h"

#if CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#if MICROPY_HW_ESP_USB_SERIAL_JTAG

#include "hal/usb_serial_jtag_ll.h"
#include "esp_intr_alloc.h"
Expand Down Expand Up @@ -117,4 +117,4 @@ void usb_serial_jtag_tx_strn(const char *str, size_t len) {
}
}

#endif // CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG_ENABLED
#endif // MICROPY_HW_ESP_USB_SERIAL_JTAG
Loading
0