tock / tock

A secure embedded operating system for microcontrollers
https://www.tockos.org
Other
5.34k stars 682 forks source link

USB stack in Tock #4011

Open Ioan-Cristian opened 3 months ago

Ioan-Cristian commented 3 months ago

Hello!

I'm writing a USB driver for a board. I have a hard time due to the USB stack design. The issues that I have:

  1. ClientCtrl does not enable the IN interface of the endpoint 0. Is the peripheral expected to do this?
  2. The current USB HIL is written a bit in C-style: CtrlSetupResult could be rewritten as a Result.
  3. Peripheral implementations seem unnecessary complicated. For instance, why should there be a state machine for each endpoint? Wouldn't it be better if the upper layers take care of the USB state machine, especially that the upper layers already do it? The complexity seems to steem from the USB HIL design.
  4. The documentation for the HIL is lacking. I could guess the purpose of each method from the current implementations. Documentation should be added.
  5. There is minimal support for link events: only bus reset is supported. Other link events are useful: host lost, link suspended, link resumed etc.
  6. No support for errors: what happens if an invalid endpoint is passed to endpoint_set_in_buffer() method?

I would like to rework the USB HIL to ressemble the flash HIL. The reworked USB HIL will have one goal: eliminate any logic from peripheral implementations. The only purpose of the peripheral driver (USB Bus interface layer) is to notify the upper layers of packets received, sent and link state events.

What do you think?

bradjc commented 3 months ago
  1. Unclear, but https://github.com/tock/tock/blob/79cc869fd8e76facae2dcb88f28e185c5ad4ddee/capsules/extra/src/usb/usbc_client_ctrl.rs#L212 suggests there is no intent to use it, so maybe it was skipped or missed?
  2. I think that was probably written before it was clear Result would be ubiquitous and could be replaced.
  3. This seems like the most pertinent question and worth considering. I've worked on the nrf52 USB driver, but I'm far from having a good sense of what a better USB stack would look like.
  4. Agreed.
  5. Makes sense.
  6. Makes sense.

Speaking just for myself, updating the USB HIL would be welcome, and the current stack is confusing. However, we have multiple implementations for different chips, and updating all of those correctly might be hard.

Ioan-Cristian commented 3 months ago

Related to the IN interface for the default endpoint:

I'm writing a new USB HIL right now. When I port it to my board, I'll open a draft pull request. I think it would be helpful if other proposals for different USB controllers are made and then compare them.