search

espressif / idf-extra-components

Additional components for ESP-IDF, maintained by Espressif
143 stars 86 forks source link

esp_tinyusb v1.4.3: ESP32P4 HS only support #292

Closed roma-jam closed 7 months ago

roma-jam commented 7 months ago

Checklist

Changes

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

github-actions[bot] commented 7 months ago

Unit Test Results

  11 files  ±0    11 suites  ±0   15m 1s :stopwatch: -18s   27 tests ±0    27 :heavy_check_mark: ±0  0 :zzz: ±0  0 :x: ±0  252 runs  ±0  252 :heavy_check_mark: ±0  0 :zzz: ±0  0 :x: ±0 

Results for commit 7cdc22e6. ± Comparison against base commit 6906d9c2.

:recycle: This comment has been updated with latest results.

roma-jam commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

@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_speedconfiguration 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.