Skip to content

Conversation

@roma-jam
Copy link
Collaborator

@roma-jam roma-jam commented Jan 7, 2025

Description

ESP32S2/S3 - USB OTG 1.1

Handled via USB PHY.

USB OTG signals are connected to the GPIO matrix and we can connect GPIO signal from VBUS monitoring pin directly to Bvalid session signal.

ESP32P4 - USB OTG 1.1

Handled via USB PHY.

USB OTG signals are connected to the GPIO matrix and we can connect GPIO signal from VBUS monitoring pin directly to Bvalid session signal.

ESP32P4 - USB OTG 2.0

USB OTG signals for DWC OTG 2.0 are not connected, so we need another mechanism to control the Bvalid session signal.

The controller provides an option to override Bvalid value via GOTGCTL register.

To workaround absence of signal mapping, the combination of GPIO level detection via IRQ and changing the Bvalid value in the tinyusb driver are being proposed.

When TinyUSB is configured on HS port, the VBUS monitoring logic can be enabled by setting the self_powered flag.
The logic detects the VBUS monitor pin state change and override the Bvalid value via register, causing the USB DWC controller to change the state of B-peripheral session.

Limitation

Overriding the Bvalid value via GOTGCTL register from the application works differently for USB OTG 1.1 and USB OTG 2.0.

On 2.0, when we override the value via GOTGCTL register, the device is staying connected to the USB Host and we need disable the D+ pull-up register additionally (via DCTL register).

On 1.1 the last step isn't necessary (test cases "Emulated" and "Controlled").

Refer to TODO: Verify, why device staying connected after setting value to 0 and why signaling works differently

Related

Follow-up

  • Make debounce delay for VBUS state change detection runtime configurable via tinyusb_config_t (now default 500ms, not critical)
  • Add dependency for ATTR_SELF_POWERED in device descriptor (right now, there are now verification if the config is set with self_powered = true but device descriptor doesn't have the attribute, not critical).

Testing

ESP32S2/S3

(1)	"Emulated VBUS, verify attach/detach events callback (via Bvalid signal)" [ci][dconn]
(2)	"Controlled VBUS, verify attach/detach events callback" [dconn]
(3)	"Real VBUS, verify attach/detach events callback (requires manual handling)" [dconn]

ESP32P4

(1)	"Emulated VBUS USB OTG 1.1, verify attach/detach events callback (via Bvalid signal)" [dconn]
(2)	"Emulated VBUS USB OTG 1.1, verify attach/detach events callback (via GOTGCTL register)" [dconn]
(3)	"Emulated VBUS USB OTG 2.0, verify attach/detach events callback (via GOTGCTL register)" [ci][dconn]
(4)	"Controlled VBUS USB OTG 1.1, verify attach/detach events callback" [dconn]
(5)	"Controlled VBUS USB OTG 2.0, verify attach/detach events callback" [dconn]
(6)	"Real VBUS USB OTG 1.1, verify attach/detach events callback (requires manual handling)" [dconn]
(7)	"Real VBUS USB OTG 2.0, verify attach/detach events callback (requires manual handling)" [dconn]

Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

@roma-jam roma-jam self-assigned this Jan 7, 2025
@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch from 0efeabb to 80b30e0 Compare October 2, 2025 11:13
@roma-jam roma-jam changed the title change(dconn_test): Added esp32p4 to CI change(dconn_test): VBUS monitoring feature Oct 2, 2025
@roma-jam roma-jam changed the title change(dconn_test): VBUS monitoring feature change(dconn_test): VBUS monitoring feature [WIP] Oct 2, 2025
@roma-jam roma-jam changed the title change(dconn_test): VBUS monitoring feature [WIP] feature(tinyusb): Software VBUS monitoring feature [WIP] Oct 6, 2025
@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch from c223b78 to 9be412d Compare October 7, 2025 10:35
@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch 4 times, most recently from 8eecced to 635e40e Compare October 8, 2025 12:25
@roma-jam roma-jam changed the title feature(tinyusb): Software VBUS monitoring feature [WIP] feature(tinyusb): Software VBUS monitoring feature Oct 8, 2025
@roma-jam roma-jam marked this pull request as ready for review October 8, 2025 12:26
@roma-jam roma-jam requested a review from Copilot October 8, 2025 12:26
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements software VBUS monitoring functionality for TinyUSB across different ESP32 variants. The feature addresses hardware limitations in ESP32P4's USB OTG 2.0 controller where USB OTG signals are not connected to the GPIO matrix.

  • Added VBUS monitoring logic with GPIO-based detection and debouncing
  • Introduced BVALID signal override mechanism for ESP32P4 USB OTG 2.0
  • Created comprehensive test suite covering different scenarios and target platforms

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tinyusb_vbus_monitor.c Core VBUS monitoring implementation with GPIO interrupt handling and debouncing
tinyusb.c Integration of VBUS monitoring into driver install/uninstall flows
tinyusb_task.c Task-level enablement of VBUS monitoring
test files Comprehensive test coverage for VBUS monitoring functionality
CMakeLists.txt Build configuration updates for new components
Comments suppressed due to low confidence (1)

device/esp_tinyusb/tinyusb_vbus_monitor.c:1

  • Duplicate comment on lines 156 and 158. One should be removed or made more specific.
/*

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


const static char *TAG = "VBUS mon";

#define DEBOUNCE_DELAY_MS 500 /* TODO: make configurable */
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

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

TODO comment indicates incomplete implementation. The hardcoded debounce delay should be made configurable.

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Might be added to tinyusb_config_t struct , as a runtime configurable parameter.

@roma-jam roma-jam added this to the esp_tinyusb v2.0.2 milestone Oct 8, 2025
@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch from 635e40e to 828766c Compare October 9, 2025 09:59
@roma-jam roma-jam requested a review from Copilot October 9, 2025 10:06
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch from 828766c to b87ad35 Compare October 9, 2025 10:14
@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch 2 times, most recently from 4bda625 to e154363 Compare October 9, 2025 10:54
// Set Bvalid signal to 1
dwc_otg->gotgctl_reg.bvalidovval = 1;

/* TODO: Verify, why device staying connected after setting value to 0 and why signaling works differently */
Copy link
Collaborator Author

@roma-jam roma-jam Oct 9, 2025

Choose a reason for hiding this comment

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

This one I marked as limitation in the description.

When we connect the ZERO signal to Bvalid signal, the device disappears from the USB Host.
When we do it via GOTGCTL register, by overriding the Bvalid signal with 0, the device does not disappears and showed as connected to the USB Host.

Right now I have the idea confirm the same on the USB OTG 1.1 controller (because it is possible to use the same register, not GPIO matrix) to understand, that I don't miss anything.

UPD:
I verified it on USB OTG 1.1 and with mapping signal or overriding by register it works the same.
So, it doesn't work the same way on USB OTG 2.0 and we need additionally disable the pull-up resistor to make the device disconnect from the Host.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will try to find any way to make it work without breaking change, otherwise I will add the additional events TINYUSB_EVENT_VBUS_ENABLED/TINYUSB_EVENT_VBUS_DISABLED to call the tud_connect() / tud_disconnect() from the application.

@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch from e154363 to b8b5b7a Compare October 9, 2025 11:23
@roma-jam roma-jam marked this pull request as draft October 9, 2025 11:30
@roma-jam roma-jam removed the request for review from tore-espressif October 9, 2025 11:30
@roma-jam roma-jam requested review from Copilot and removed request for igi540 and peter-marcisovsky October 9, 2025 11:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch 3 times, most recently from 2ae1828 to a7655a0 Compare October 10, 2025 07:09
@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch from a7655a0 to 3dd47bf Compare October 10, 2025 07:10
…ed flag

- Added test cases for USB OTG 1.1 (all targets)
- Added test cases for USB OTG 2.0 (esp32p4)
- Enabled the ci test for emulation of VBUS swithing on ESP32P4 USB OTG 2.0
@roma-jam roma-jam force-pushed the feature/dconn_esp32p4 branch from 3dd47bf to 317fcf9 Compare October 10, 2025 08:32
@roma-jam roma-jam marked this pull request as ready for review October 10, 2025 09:35
@roma-jam
Copy link
Collaborator Author

Hi @leeebo,

I would be happy, if you or someone from your team could check this changes as well.

Please, note the Limitation section in the PR description and my comment to the test_vbus_monitor.c file.

Copy link
Collaborator

@tore-espressif tore-espressif left a comment

Choose a reason for hiding this comment

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

we need disable the D+ pull-up register additionally (via DCTL register).

Is this already implemented in the code?

It would be great if we could avoid using the esp_timer for debouncing. Could you check if P4's glitch filter could help us here? Chapter 8.4.4 https://www.espressif.com/sites/default/files/documentation/esp32-p4_technical_reference_manual_en.pdf

I'll check with others whether there are other ways of doing this

"tinyusb.c"
"usb_descriptors.c"
"tinyusb_task.c"
"tinyusb_vbus_monitor.c"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only for P4?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, make sense.

Technically, it might be used for S2/S3 as well, but is used only for P4 USB OTG 2.0 now.

@roma-jam
Copy link
Collaborator Author

roma-jam commented Oct 14, 2025

@tore-espressif

Is this already implemented in the code?

Yes, I added these lines to the test application for behavior to be the same on S2/S3 and P4.
But I didn't do it in the driver, so there is no breaking change.

It would be great if we could avoid using the esp_timer for debouncing.
It might be replaced by the FreeRTOS basic timer. And we can omit the dependency from esp_timer with this.

Anyway, we will have one in the USB Host library (for suspend/wakeup), why not to have it in the device stack as well (for software VBUS detection).

UPD:

Could you check if P4's glitch filter could help us here?

Based on the info from ref manual, it might be the option. I'll check.

@roma-jam roma-jam marked this pull request as draft October 14, 2025 11:37
Copy link
Collaborator

@leeebo leeebo left a comment

Choose a reason for hiding this comment

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

Hi @roma-jam , this detection logic LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants