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

stm32_common: Add USB OTG FS/HS usbdev peripheral driver #12556

Merged
merged 16 commits into from
Feb 11, 2020

Conversation

bergzand
Copy link
Member

@bergzand bergzand commented Oct 23, 2019

Contribution description

This PR adds an USBdev driver for the ST (synopsys). It is used with the STM32F2, STM32F4, STM32F7 and STM32H7 devices. The STM32F1 and STM32F3 devices unfortunately use a different device type.

The goal is to support the both the full speed and the high speed version in a single driver, the differences between the two are minimal and drivers from other project are able to combine the two instances too.

The peripheral itself has a 'Core ID' register containing a version number-like value. So far I've identified 2 different iterations for both the FS and the HS devices. They have slightly different register flags and behave slightly different in some cases. The crude method to detect the version is to check if certain register flag definitions are defined.
It is assumed that when both the FS and the HS peripheral is available on a single device, both are of the same version.

  • CID 0x1x00: 4 FS endpoints, 6 HS endpoints (f401, f411…)
  • CID 0x2x00: 6 FS endpoints, 9 HS endpoints (f446, f7xx…)

Testing procedure

Issues/PRs references

One of the remaining issues with the driver is that it requires xtimer (or a different solution) to sleep between one of the register modifications.

depends on #12534, #12533, #12536 and #12579

TODO:

  • Add xtimer dependency when periph_usbdev is used.
  • Add usbdev feature to non-nucleo boards
  • Test with the HS peripheral in FS mode
  • Add DMA support for the HS peripheral
  • Add support for the non-nucleo boards
  • Migrate DMA enable/disable to a #define
  • Test isochronous transfers

Note

@chrysn, @basilfx: FYI The EFM32 devices seem to use an identical IP block from Synopsys, only the serialization phy seems to be different.

@bergzand bergzand added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers Area: USB Area: Universal Serial Bus labels Oct 23, 2019
@bergzand bergzand added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 27, 2019
@bergzand bergzand force-pushed the wip/stusbdev branch 3 times, most recently from 11ced48 to ddccb4a Compare October 28, 2019 17:06
@bergzand
Copy link
Member Author

Almost done here, I've tested this myself on the nucleo-f401re and the nucleo-f446re with a jumper wired USB cable. Both CID versions should work and both the FS and the HS peripheral work (in FS mode with the built-in phy). With the HS phy, both DMA and non-DMA is functional.

I have not yet tested isochronous transfers.

@bergzand bergzand added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 28, 2019
@bergzand bergzand added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 28, 2019
@bergzand bergzand force-pushed the wip/stusbdev branch 2 times, most recently from 77a8556 to 0155807 Compare October 28, 2019 20:04
@bergzand bergzand added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 28, 2019
@bergzand bergzand force-pushed the wip/stusbdev branch 2 times, most recently from 47630d8 to 69dc829 Compare October 29, 2019 16:52
@bergzand bergzand changed the title [WIP] stm32_common: Add USB OTG FS/HS usbdev peripheral driver stm32_common: Add USB OTG FS/HS usbdev peripheral driver Oct 29, 2019
@bergzand
Copy link
Member Author

Ready for review! I won't be force pushing anymore to this branch unless instructed otherwise.

I'm going to test isochronous transfers at some point, but I won't consider it blocking for this PR as it is not used within RIOT-provided USB functionality at the moment.

Still waiting for #12579 though. :)

@bergzand bergzand requested a review from aabadie October 29, 2019 17:03
@bergzand
Copy link
Member Author

I'm carefully going to claim that isochronous transfers work. I have a small test setup where I can at least get the events from the transfer and receive events for the transfers. I also receive correct transfer lengths, I'm only haven't tested the content of the transfer yet.

@bergzand bergzand added State: waiting for other PR State: The PR requires another PR to be merged first and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 29, 2019
@bergzand bergzand removed the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 2, 2019
@bergzand
Copy link
Member Author

bergzand commented Nov 2, 2019

Rebased now that #12579 is merged, no longer waiting for other PRs

@bergzand
Copy link
Member Author

Squashed after offline approval from @aabadie.

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit remaining.

Please squash! @dylad, do you still have comments ?

@@ -3,3 +3,7 @@ USEMODULE += pm_layered

# include stm32 common functions and stm32 common periph drivers
USEMODULE += stm32_common stm32_common_periph

ifneq (,$(filter periph_usbdev,$(FEATURES_USED)))
USEMODULE += xtimer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
USEMODULE += xtimer
USEMODULE += xtimer

Looks like there's a tab here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😲 Fixed!

Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good on my side.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 11, 2020
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK! Now let's wait for Murdock (which is quite busy)

@aabadie
Copy link
Contributor

aabadie commented Feb 11, 2020

All green, let's go !

@aabadie aabadie merged commit 3ac25c3 into RIOT-OS:master Feb 11, 2020
@bergzand bergzand deleted the wip/stusbdev branch February 11, 2020 19:55
@bergzand
Copy link
Member Author

Thanks!

@dylad
Copy link
Member

dylad commented Feb 11, 2020

Good job with this PR @bergzand !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: USB Area: Universal Serial Bus CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 3-testing The PR was tested according to the maintainer guidelines Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants