feat(USB-OTG): Add USB-OTG CDC RX/TX support

[radimkarnis]

• Add the support for the USB-OTG peripheral data TX/RX on ESP32-S2/S3/P4
• Continuation and refactoring of feat: add support for the USB-CDC feature #36
  • Reduced code repetition
  • Follow naming conventions
  • Adopted the structure set by UART and USB-Serial/JTAG implementations
• Related esp-flasher-stub PR: feat(USB-OTG): Add support for USB-OTG RX/TX esp-flasher-stub#32

[github-actions]

	Messages
📖	This PR seems to be quite large (total lines of code: 31757), you might consider splitting it into smaller PRs

👋 Hello radimkarnis, we appreciate your contribution to this project!

---

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against b50a3ce

[Copilot]

Pull request overview

This PR adds USB-OTG peripheral support for ESP32-S2, ESP32-S3, and ESP32-P4 chips, enabling data transmission and reception through USB-OTG CDC-ACM interface. The implementation follows the library's three-layer architecture with public APIs, implementation layer, and target-specific overrides.

Key Changes:

• Added USB-OTG CDC-ACM communication support with TX buffering and RX callbacks
• Implemented reset handling via RTS line state detection
• Added target-specific implementations for ESP32-S2/S3/P4 with appropriate interrupt routing

Reviewed changes

Copilot reviewed 19 out of 23 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/usb_otg.c	Main USB-OTG implementation with TX buffering, RX callbacks, and reset detection
src/target/esp32s3/src/usb_otg.c	ESP32-S3 specific USB-OTG interrupt setup and reset handling
src/target/esp32s2/src/usb_otg.c	ESP32-S2 specific USB-OTG implementation with interrupt matrix routing
src/target/esp32p4/src/usb_otg.c	ESP32-P4 specific implementation with CLIC interrupt handling
src/target/common/src/usb_otg.c	Weak default implementations for unsupported chips
src/target/base/include/target/usb_otg.h	Internal API header for target layer
include/esp-stub-lib/usb_otg.h	Public API header for library users
src/target/esp32s3/include/soc/rtc_cntl_reg.h	RTC control register definitions for ESP32-S3
src/target/esp32s3/ld/esp32s3.rom.api.ld	Added esp_rom_isr_mask ROM API mapping
src/target/esp32s2/ld/esp32s2.rom.api.ld	Added esp_rom_isr_mask ROM API mapping
src/target/esp32p4/ld/esp32p4.rom.api.ld	Added esp_rom_isr_mask ROM API mapping
CMakeLists.txt	Added usb_otg.c to main library sources
Various target CMakeLists.txt	Added usb_otg.c to target-specific sources

[erhankur]

@radimkarnis Before diving into the review, I wanted to discuss the approach to conditional compilation in this PR. I noticed a large #if block in src/usb_otg.c, which differs from the pattern we’ve followed so far. (a good example is medium sized uart). Our current architecture uses a three layer approach: src/ (public interface) → common/ (weak defaults) → target/esp32*/ (strong overrides), which helps us avoid #ifdefs.

I think now is the right time to discuss this, as I'm seeing an increase in #if block usage across the codebase. I’d love to hear your thoughts. Was there a specific challenge that made the weak/strong pattern hard to apply here, or would you be open to refactoring this to match the UART approach?

[radimkarnis]

@erhankur yes, thank you for your concern! That's also my everlasting question: how to do this correctly.

Putting most of the code into src/ (public interface) and not defining the functions in common/ (as weak defaults) introduces the need for an #ifdef, but reduces the need to repeat all of the implementations when a new target will have to be introduced.

• With this approach, if a new target with USB-OTG is being added, you just add three new strong overrides. And if the target doesn't support it, you don't have to add anything.
• With the approach you describe, we either have to provide new empty strong implementations for all of the functions (if USB-OTG is not supported) or three new strong implementations (if it is supported).

Adding new features to the stub-lib is now showing us with @Dzarda7 how much work really has to be done to support a target, and we are starting to really consider how many files we add and how many things will have to be re-implemented. This is a real trade-off between architectural purity (no #ifdefs) and practical maintainability (less boilerplate).

I am definitely open to refactoring this the way we agree on, but I also feel like now that we are "in the trenches", we start to feel new challenges and the real scale of the project.

Let's decide if we really want to avoid #ifdefs or if we agree to at least localize them to a certain layer.

[erhankur]

@radimkarnis Thanks for the explanation. I agree this is a real trade-off, and I’m open to balancing architectural cleanliness with practical maintainability. OK. Let's try to keep such conditionals (SOC_CAPS only) localized and limited. We can revisit the structure if this pattern starts to grow.

[erhankur]

LGTM

[Dzarda7]

I know we discussed the #ifdefs in usb-jtag-serial already where we kept it. But I thought about it again and I think it might be really hard to track what is going on in the code now as target specific stuff is now not just in target folder, but also in src folder. I would either put it into target/common or use the per target folders. I am a bit pro architectural cleanliness, but no strong opinion. Both has some tradeoffs.

• Using #ifdefs makes it harder to track what is going on, but avoids code duplication. This approach might fall apart when some target needs specific implementation in which case per target definition will be necessary.
• Using per-target definitions is easy to track, but causes a lot of code duplication (can be improved compared to current state, but anyway) and when we need to modify or fix something for one, it needs to be done in many places.

Feel free to keep the #ifdefs but I believe we should keep them at least on target level if possible.

[erhankur]

Using #ifdefs makes it harder to track what is going on, but avoids code duplication. This approach might fall apart when some target needs specific implementation in which case per target definition will be necessary.

We should avoid per target ifdefs. I am fine with soc capabilities under src/ . We should not propogate to the bottom.

Using per-target definitions is easy to track, but causes a lot of code duplication (can be improved compared to current state, but anyway) and when we need to modify or fix something for one, it needs to be done in many places.

Couldn't we simplify/divide functions as common+target specific? I believe recent targets have same pattern for most of the peripherals.

[Dzarda7]

We should avoid per target ifdefs. I am fine with soc capabilities under src/ . We should not propogate to the bottom.

I did not mean per target, I meant soc caps, but to do it here, as I believe top-level c files in main src folder should be target agnostic, but I might be wrong here.

Couldn't we simplify/divide functions as common+target specific? I believe recent targets have same pattern for most of the peripherals.

Yes, no issue for now as we mostly have RISC-V targets in common and xtensa in per target. I meant for the future where there might be some changes like when comming from xtensa to RISC-V and then we need per target. There is definitely a way to solve this again, just something to be aware of for the future.

[radimkarnis]

Thank you for all opinions. After our discussion, I will now merge this, but I am open to changing the architecture. We will soon see how much workload it is to add a new target; we can refactor based on this info afterwards.

[dobairoland]

LGTM