rust-embedded-community / usb-device

Experimental device-side USB framework for microcontrollers in Rust.
MIT License
425 stars 75 forks source link

USB high speed & full speed support #39

Open conorpp opened 4 years ago

conorpp commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.