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

Support for USB 1.0 and 1.0 in UsbRev enum #155

Merged
merged 1 commit into from
Oct 13, 2024

Conversation

sourcebox
Copy link
Contributor

The UsbRev enum only has support for specification 2.0 and 2.1. Unless there's a reason not to add older revisions, 1.0 and 1.1 should also be supported.

@ryan-summers
Copy link
Member

What were the diffs between 1.0 and 2.0? Does this stack actually conform with USB 1.0 and USB 1.1?

@sourcebox
Copy link
Contributor Author

What were the diffs between 1.0 and 2.0? Does this stack actually conform with USB 1.0 and USB 1.1?

Good question. I can't tell you the differences in detail, and it's debatable if anyone will ever do a USB 1.0 device these days. But USB 1.1 is what many full speed devices still use. And most MCUs only support FS without external PHY. So 1.1 should be an option and if something is not conform with it in the stack, then it has to be fixed.

From whar I see on various sources, the difference between 1.x and 2.x in terms of compatibility is primarily the electrical specification.

@jannic
Copy link
Contributor

jannic commented Oct 11, 2024

I don't know if there's a practical difference between setting bcdUSB to 1.1 or 2.0 for a full-speed device. But I'd give the firmware author the choice. Even if setting it to 2.0 is fully spec conformant, as a firmware author you may need to be compatible with a broken USB driver that requires 1.1.

@jannic
Copy link
Contributor

jannic commented Oct 11, 2024

But beware: The enum is not marked as non_exhaustive, so adding a new variant is a breaking change.

@ianrrees
Copy link
Contributor

To me, the usb-device API feels a bit unstable at the moment, and AFAIK it hasn't been confirmed to conform to specs. So, I'm in favour of adding useful variants to the enum, without marking non_exhaustive, but maybe we try to lump the change in with other breaking changes (eg #153 is one I'd like to move on)

@jannic
Copy link
Contributor

jannic commented Oct 12, 2024

Indeed, there's already been a breaking change since the last release: #144

So now it's probably a good time to apply this breaking change es well.

@ryan-summers
Copy link
Member

Sounds good, thanks for the discussion. Agreed in that if users choose to use this version, it's at their own design. If it causes problems, we can always remove it later :)

@ryan-summers ryan-summers merged commit 6cf2c47 into rust-embedded-community:master Oct 13, 2024
3 checks passed
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.

4 participants