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.

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

Jump to bottom

boards: add esp32s3-pros3 support #19088

Merged
merged 3 commits into from
Feb 6, 2023
Merged

boards: add esp32s3-pros3 support #19088

merged 3 commits into from
Feb 6, 2023

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Jan 4, 2023 •
edited
Loading

Contribution description

This PR provides the support for ESP32 ProS3 board from Unexpected Maker.

This board doesn't have a USB-to-Serial chip on board. Therefore, USB Serial/JTAG is used for STDIO and the board is flashed via the USB Seral/JTAG interface by default.

Testing procedure

Flashing tests/shell should work.

Issues/PRs references

@github-actions github-actions bot added Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Jan 4, 2023
@gschorcht gschorcht added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jan 4, 2023
@gschorcht gschorcht mentioned this pull request Jan 5, 2023
Merged
12 tasks
@gschorcht gschorcht force-pushed the boards/esp32s3-pros3 branch 2 times, most recently from f3b3d26 to 8ae127d Compare January 5, 2023 07:41
@benpicco
Copy link
Contributor

Does stdio_usb_serial_jtag work on this one?

@gschorcht
Copy link
Contributor Author

Does stdio_usb_serial_jtag work on this one?

Yes. So it seems to be better to use stdio_usb_serial_jtag by default to let the application the option to use USBUS or tinyUSB. Was that the background of your question?

@benpicco
Copy link
Contributor

benpicco commented Jan 15, 2023 •
edited
Loading

Yes because it should only require a single USB cable then, right? Because Serial/JTAG is a dedicated USB port and device USB would be on another one.

So if the user only has to connect to a single port for flashing and the shell, that would be more convenient.

edit oh I see the board only has a single USB.
How does that work? Can Serial/JTAG and device USB be switched at run-time?

@gschorcht
Copy link
Contributor Author

gschorcht commented Jan 15, 2023 •
edited
Loading

edit oh I see the board only has a single USB.
How does that work? Can Serial/JTAG and device USB be switched at run-time?

Yeah, that's a bit tricky 😉 The ESP32S3 has two different controllers but a logic which one is routed to the D+/D- pin, see ESP32S3 TRM page 1204. On reset, the USB Serial/JTAG interface is selected by default so that you can flash and use the CDC ACM interface. If the USB OTG is initialized, the USB OTG signals are selected and the USB Serial/JTAG interface is not available any longer.

@benpicco
Copy link
Contributor

benpicco commented Jan 18, 2023 •
edited
Loading

That's interesting.
Yea I think stdio_usb_serial_jtag would be a good default since it should be more robust and light-weight as it acts more like a USB-UART adapter than a full USB stack.

But I'm mainly asking if it works on ESP32-S3 at all, I only received one in the mail today.

Btw: What's still WIP here?

@github-actions github-actions bot removed Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Feb 3, 2023
@gschorcht gschorcht removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Feb 3, 2023
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Feb 3, 2023
@gschorcht
Copy link
Contributor Author

Work on this board definition has been finished.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 4, 2023
@riot-ci
Copy link

riot-ci commented Feb 4, 2023 •
edited
Loading

Murdock results

✔️ PASSED

90dc95e boards/common/stdio_tinyusb_cdc_acm: let tests wait for serial port

Success Failures Total Runtime
6851 0 6851 09m:20s

Artifacts

@github-actions github-actions bot added Area: build system Area: Build system Area: sys Area: System labels Feb 4, 2023
@gschorcht
Copy link
Contributor Author

@benpicco I had to provide some fixes due to compilation error in Murdock:

  • The module usb_board_reset is also enabled when stdio_usb_serial_jtag is used as STDIO backend. Even if another STDIO backend like stdio_uart is flashed, the USB Serial/JTAG CDC ACM and thus a ttyACMx should be available. Although usb_board_reset doesn't affect the USB Serial/JTAG CDC ACM, it is necessary to be able to reset the board in download mode for flashing if stdio_cdc_acm or stdio_tinyusb_cdc_acm was previously flashed and is still active. Otherwise flashing via the USB Serial/JTAG CDC ACM interface is not possible. Commit 9f8b1e0 allows to enable usb_board_reset in Kconfig also if only stdio_usb_serial_jtag is used.
  • In PR boards: common: stdio_cdc_acm: let tests wait a bit for serial port #19128, a delay for tests was defined in stdio_cdc_acm.dep.mk to take into account the fact that it may take a few seconds before the CDC ACM serial device ttyACM becomes available. Commit 9810617 adds this change to stdio_tinyusb_cdc_acm.dep.mk.
  • Commit 97373d8 adds a comment for usb_board_reset and enables it in Kconfig.

benpicco
benpicco approved these changes Feb 4, 2023
View reviewed changes
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Looks good to me - please squash!

@benpicco
Copy link
Contributor

benpicco commented Feb 5, 2023

bors merge

bors bot added a commit that referenced this pull request Feb 5, 2023
@bors @gschorcht @benpicco
Merge #19088 #19216
9284658 
19088: boards: add esp32s3-pros3 support r=benpicco a=gschorcht

### Contribution description

This PR provides the support for [ESP32 ProS3](https://esp32s3.com/pros3.html#home) board from Unexpected Maker.

This board doesn't have a USB-to-Serial chip on board. Therefore, USB Serial/JTAG is used for STDIO and the board is flashed via the USB Seral/JTAG interface by default.

### Testing procedure

Flashing `tests/shell` should work.

### Issues/PRs references

19216: drivers/mtd_sdcard: add mtd_sdcard_default module r=benpicco a=benpicco



Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
Co-authored-by: Benjamin Valentin <benpicco@beuth-hochschule.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
@bors
Copy link
Contributor

bors bot commented Feb 5, 2023

Build failed (retrying...):

bors bot added a commit that referenced this pull request Feb 5, 2023
@bors @gschorcht
Merge #19088
2c301af 
19088: boards: add esp32s3-pros3 support r=benpicco a=gschorcht

### Contribution description

This PR provides the support for [ESP32 ProS3](https://esp32s3.com/pros3.html#home) board from Unexpected Maker.

This board doesn't have a USB-to-Serial chip on board. Therefore, USB Serial/JTAG is used for STDIO and the board is flashed via the USB Seral/JTAG interface by default.

### Testing procedure

Flashing `tests/shell` should work.

### Issues/PRs references

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
@bors
Copy link
Contributor

bors bot commented Feb 5, 2023

Build failed:

@gschorcht
boards: add support for ESP32 ProS3 board
df46c45
@gschorcht
sys/usb_board_reset: allow to enable it also for stdio_usb_serial_jtag
e3b1c81 
Even if only `stdio_usb_serial_jtag` is enabled as STDIO, `usb_board_reset` is enabled since there should be a CDC ACM interface in any case. This is necessary, for example, to reset the board into bootloader if `stdio_cdc_acm` or `stdio_tinyusb_cdc_acm` was previously used.
@gschorcht
boards/common/stdio_tinyusb_cdc_acm: let tests wait for serial port
90dc95e 
After a reset, it can take several seconds before the CDC ACM interface becomes available as serial device `ttyACMx`. This was a change that was already made for `stdio_cdc_acm` in RIOT-OS#19128.
@benpicco
Copy link
Contributor

benpicco commented Feb 6, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 6, 2023

Build succeeded:

@bors bors bot merged commit fcc07a5 into RIOT-OS:master Feb 6, 2023
@gschorcht
Copy link
Contributor Author

Thanks for reviewing and merging.

@gschorcht gschorcht deleted the boards/esp32s3-pros3 branch February 6, 2023 18:47
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

Assignees
No one assigned
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2023.04
Development

Successfully merging this pull request may close these issues.
4 participants
@gschorcht @benpicco @riot-ci @MrKevinWeiss