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.

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

esp_tinyusb v1.4.3: ESP32P4 HS only support #292

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

roma-jam
Copy link
Collaborator

@roma-jam roma-jam commented Jan 4, 2024

Checklist

  • Component contains License
  • Component contains README.md
  • Component contains idf_component.yml file with url field defined
  • Component was added to upload job
  • Component was added to build job
  • Optional: Component contains unit tests
  • CI passing

Changes

  • Added USB OTG PHY HS configuration in KConfig for ESP32P4

Hint: For better experience and verification use TinyUSB v0.15.0~4

@roma-jam roma-jam self-assigned this Jan 4, 2024
Copy link

github-actions bot commented Jan 4, 2024

Unit Test Results

  11 files  ±0    11 suites  ±0   15m 1s ⏱️ -18s
  27 tests ±0    27 ✔️ ±0  0 💤 ±0  0 ±0 
252 runs  ±0  252 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 7cdc22e. ± Comparison against base commit 6906d9c.

♻️ This comment has been updated with latest results.

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.

@roma-jam Thanks for the update!

Overall LGTM, let me test on HW and we can go ahead with merging

@roma-jam roma-jam changed the title esp_tinyusb v1.4.3: added usb phy port selection for esp32p4 esp_tinyusb v1.4.3: added USB OTG peripheral selection for esp32p4 Jan 10, 2024
@roma-jam roma-jam changed the title esp_tinyusb v1.4.3: added USB OTG peripheral selection for esp32p4 esp_tinyusb v1.4.3: added USB OTG peripheral configuration for esp32p4 Jan 10, 2024
@roma-jam roma-jam changed the title esp_tinyusb v1.4.3: added USB OTG peripheral configuration for esp32p4 esp_tinyusb v1.4.3: Added USB OTG peripheral configuration for esp32p4 Jan 10, 2024
@roma-jam roma-jam changed the title esp_tinyusb v1.4.3: Added USB OTG peripheral configuration for esp32p4 esp_tinyusb v1.4.3: USB OTG peripheral configuration for esp32p4 Jan 10, 2024
@roma-jam roma-jam changed the title esp_tinyusb v1.4.3: USB OTG peripheral configuration for esp32p4 esp_tinyusb v1.4.3: ESP32P4 USB OTG peripheral configuration Jan 10, 2024
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_phy_portnum_config branch 4 times, most recently from f429112 to b6c93eb Compare January 10, 2024 22:03
@Dazza0 Dazza0 self-requested a review January 11, 2024 08:16
Copy link
Contributor

@Dazza0 Dazza0 left a comment

Choose a reason for hiding this comment

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

@roma-jam Left some nitpicks but LGTM otherwise

@roma-jam roma-jam force-pushed the feature/esp_tinyusb_phy_portnum_config branch from b6c93eb to dcdb8a9 Compare January 11, 2024 08:34
@roma-jam roma-jam changed the title esp_tinyusb v1.4.3: ESP32P4 USB OTG peripheral configuration esp_tinyusb v1.4.3: ESP32P4 HS support Jan 11, 2024
@roma-jam
Copy link
Collaborator Author

roma-jam commented Jan 11, 2024

Hey @tore-espressif @Dazza0,
I have resolved the situation with configuration descriptor. I don't thing we are going to add more possibilities of returning the configuration descriptor during the GetConfiguration() standard request, so I decided to add it the same way as it is implemented for current configuration descriptor (which right now can be called FS configuration descriptor).

The API doesn't changed (I left the name of configuration_descriptor in tinyusb_config_t structure the same). When the HS descriptor has not been provided by user, the default HS descriptor is using (if we have TUD_OPT_HIGH_SPEED)

I kept the commit history changes available, just for easier understanding of changes. (I will clean it right before merge)
I will test the changes a bit more, meanwhile looking forward for your reviews.

UPD: Changes reverted.

@tore-espressif
Copy link
Collaborator

@roma-jam Please also have a look tud_descriptor_other_speed_configuration_cb() API in TinyUSB https://github.com/espressif/tinyusb/blob/master/src/device/usbd.h#L136

And how it is used in tinyusb official examples.

@roma-jam
Copy link
Collaborator Author

@tore-espressif The tud_descriptor_other_speed_configuration_cb() is another thing, which is not related to the current changes.
Based on USB specs: a high-speed capable device supports the
device_qualifier descriptor to return information about the device for the speed at which it is not operating
(including wMaxPacketSize for the default endpoint and the number of configurations for the other speed).
The other_speed_configuration returns information in the same structure as a configuration descriptor, but
for a configuration if the device were operating at the other speed.
(usb_2.0.pdf, 9.4.3 Get Descriptor)

Anyway, to be able to return response to the GetConfiguration() request correctly in both cases we need to keep both FS and HS configuration descriptor somewhere in memory, to be able to return the const pointer in uint8_t const *tud_descriptor_configuration_cb(uint8_t index)

Otherwise, we need dynamically provide the correct descriptor.
And yes, that is true that we need to implement the tud_descriptor_other_speed_configuration_cb() for the case, when Host decided to change the speed of the device, because right now we don't have any.

@roma-jam roma-jam force-pushed the feature/esp_tinyusb_phy_portnum_config branch from 62b6af3 to 3e150ef Compare January 12, 2024 11:36
@roma-jam roma-jam changed the title esp_tinyusb v1.4.3: ESP32P4 HS support esp_tinyusb v1.4.3: ESP32P4 HS only support Jan 12, 2024
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_phy_portnum_config branch from 3e150ef to 7cdc22e Compare January 12, 2024 11:39
@tore-espressif tore-espressif merged commit c0d51e1 into master Jan 14, 2024
64 checks passed
@mahavirj mahavirj deleted the feature/esp_tinyusb_phy_portnum_config branch June 27, 2024 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants