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

USB high speed & full speed support #39

Open
conorpp opened this issue May 5, 2020 · 4 comments · May be fixed by #73
Open

USB high speed & full speed support #39

conorpp opened this issue May 5, 2020 · 4 comments · May be fixed by #73

Comments

@conorpp
Copy link
Contributor

conorpp commented May 5, 2020

I'm interested in implementing high speed support and can make a PR but am not sure how best how to do it. The way I'm thinking is to make these two changes.

  1. Add method to UsbBus trait to read the current speed of bus.
pub enum UsbSpeed{
    LowSpeed,
    FullSpeed,
    HighSpeed,
    SuperSpeed,
}

pub trait UsbBus: Sync + Sized {
    // ...
    fn get_speed(&mut self) -> UsbSpeed;
    // ...
}
  1. Change method in UsbClass trait to use the UsbSpeed when making the usb descriptors. UsbSpeed would then get read at runtime and passed for each enumeration.
pub trait UsbClass<B: UsbBus> {
    //...
    fn get_configuration_descriptors(&self, writer: &mut DescriptorWriter, speed: UsbSpeed) -> Result<()>;
    //...
}

Problem with (2) is it would be a breaking change for existing classes, although a pretty easy fix.

What do you think? Does this look okay?

@mvirkkunen
Copy link
Collaborator

High speed support would be pretty nice to have. From what I've understood from reading the spec, usb-device needs to be able to do a few extra things to make it happen:

  1. Detect the speed at hardware level (as you outlined)
  2. Tell classes about the speed (as you also outlined)
  3. Generate the "device qualifier" and "other speed configuration" descriptors
  4. Somehow support changing the max_packet_size of endpoints depending on the speed.

Do you think there's anything else that needs to be changed to be compliant?

What comes to SuperSpeed, I haven't really read that far. I know the basic framework of endpoints and transfers is essentially the same, but to really take advantage of the speed you are probably going to have to support things like background DMA, which the current API doesn't really support (and the whole embedded ecosystem seems to be a bit undecided as to how that should be modelled in generic drivers). I would probably leave SuperSpeed out until somebody has a concrete plan for implementing it in a way that actually has an advantage.

Even high speed data rates might be a bit hard to achieve without background DMA, although the lower polling delays can be an advantage even if the device can't keep up with the data rate.

What comes to breaking changes - there's already a big breaking change in the works that will require major reworking of drivers, and some reworking of classes (example). It also adds other missing features such as alternate settings, has smaller code size, and is more ergonomic to use. The way alternate settings already map endpoints of different configurations to the same resources and buffers could help with point "4" above.

That branch has existed for a while, and it's finally nearing a state where I might consider merging it. I already have one hardware driver updated to support it, and the rest should be possible. I'm just not sure if I can merge the thing until I get some feedback from other driver implementers as to whether the changes are feasible in practice.

@conorpp
Copy link
Contributor Author

conorpp commented May 6, 2020

Thanks for your quick feedback. Yeah for SuperSpeed I have no plans for, and probably best to use up to HighSpeed.

I've been looking a lot on how to implement (4) and I think the main challenge is the UsbBus object is frozen and can't be mutated by the time get_configuration_descriptors is called, yet it needs to change something on the UsbBus.

I think it'd be nice to allocate the "larger" endpoint version of the descriptor first (highspeed), and then the endpoint sizes can be reduced later by the get_configuration_descriptors class implementation later. But I'm unsure how to properly apply the change to UsbBus.

Looking forward to your merge. Looks like the new endpoint objects can be changed throughout runtime -- maybe solving this (4) problem would be easier there? Happy to try it out.

@mvirkkunen
Copy link
Collaborator

Yep, buffer-wise it should work so that memory should be allocated for the largest possible packet size and then only a part of it will potentially be used.

Do you think it's necessary to support devices that change their configuration entirely depending on speed (the different descriptor theoretically allows for that), or would it be enough to only allow buffer sizes to change depending on speed?

@conorpp
Copy link
Contributor Author

conorpp commented May 6, 2020

I haven't seen that anywhere, so I'm inclined to vote for only buffer-size + polling-interval changes.

I've looked at USB example applications from NXP and STM and they always have two separate sets of descriptors for fullspeed and highspeed. They are always the same, except with different packet sizes + intervals.

Right now I think it's pretty close to possible to just return different configurations, assuming max endpoints/sizes is allocated first. The class can choose which ones/combination to write out, while technically all are allocated.

@mriise mriise linked a pull request Aug 6, 2021 that will close this issue
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 a pull request may close this issue.

2 participants