search

hathach / tinyusb

An open source cross-platform USB stack for embedded system
https://www.tinyusb.org
MIT License
5.03k stars 1.06k forks source link

TUD_OPT_HIGH_SPEED incorrectly being set (on RP2040) #1582

Closed czietz closed 2 years ago

czietz commented 2 years ago

Operating System

Linux

Board

Raspberry Pi Pico

Firmware

https://github.com/czietz/pico-usb-speed-check built with current TinyUSB (commit fd5bb6e5d).

What happened ?

I wanted to test my USB speed checker (see above) with the current TinyUSB instead of the version 0.12.0 provided in the Pico SDK. It did not work because an incorrect endpoint size was determined here: https://github.com/czietz/pico-usb-speed-check/blob/0524ce5e430261d9852ee842e3d5cf67ca310512/usb_descriptors.c#L88 ... since TUD_OPT_HIGH_SPEED was erroneously set, even though the RP2040 only supports full-speed USB.

git bisecting it, lead to this commit: https://github.com/hathach/tinyusb/commit/8b9cf152a0dedda573fb3374d8c873dce4b883d1#diff-055197dcf69fbff20e8e8364119495cef6e4a3a9be97fd39e02a29115eaa5302L228.

Imho, the new logic for setting TUD_OPT_HIGH_SPEED is wrong. Prior to the commit, it was set to TUD_RHPORT_MODE & OPT_MODE_HIGH_SPEED, i.e., only when the high speed mode was indicated in TUD_RHPORT_MODE. Now, it is set to TUD_RHPORT_MODE & OPT_MODE_SPEED_MASK, which can be non-zero even for full-speed devices.

How to reproduce ?

Build https://github.com/czietz/pico-usb-speed-check with latest TinyUSB, try to run provided speedcheck.py.

Debug Log as txt file

No response

Screenshots

No response

hathach commented 2 years ago

thank you for raising the issue, it is indeed my bad. Will be fixed by #1586 once ci passed.

PS: Merged, please test again to see if that works for you.