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

drivers/usbdev_synopsys_dwc2: fix and re-enable DMA mode #19397

Merged

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Mar 16, 2023

Contribution description

This PR fixes the DMA mode for all STM32 USB OTG HS cores (including that for STM32F4xx CID 1.xxx) and reenables it. It fixes remaining problems in issue #19359.

This PR includes also includes some changes that are needed to use the DMA mode:

  • EP number is used as defined in CMSIS (if defined) for STM32
  • periph_usbdev_hs feature is added in Kconfig
  • periph_usbdev_hs feature is added in board definition of stm32f429i-disc1
  • largest number of available EPs is used for STM32 instead of the smallest number (to be able to use all EPs of HS peripheral)
  • stm32f429i-disco is removed from blacklist in tests/usbus_cdc_ecm since it uses the HS peripheral

Testing procedure

The following tests should work

USEMODULE=stdio_cdc_acm BOARD=stm32f429i-disc1 make -j8 -C tests/usbus_cdc_ecm flash
Test results
[526755.875691] usb 1-2.2: new full-speed USB device number 106 using xhci_hcd
[526755.977853] usb 1-2.2: config 1 interface 3 altsetting 1 endpoint 0x84 has invalid maxpacket 512, setting to 64
[526755.977856] usb 1-2.2: config 1 interface 3 altsetting 1 endpoint 0x2 has invalid maxpacket 512, setting to 64
[526755.978762] usb 1-2.2: New USB device found, idVendor=1209, idProduct=7d01, bcdDevice= 1.00
[526755.978764] usb 1-2.2: New USB device strings: Mfr=3, Product=2, SerialNumber=4
[526755.978766] usb 1-2.2: Product: stm32f429i-disc1
[526755.978768] usb 1-2.2: Manufacturer: RIOT-os.org
[526755.978769] usb 1-2.2: SerialNumber: 7C156425A950A8EB
[526755.991190] cdc_acm 1-2.2:1.0: ttyACM1: USB ACM device
[526755.998131] cdc_ether 1-2.2:1.2 usb0: register 'cdc_ether' at usb-0000:00:14.0-2.2, CDC Ethernet Device, a6:f6:4a:85:1d:c9
[526756.044150] cdc_ether 1-2.2:1.2 enp0s20f0u2u2i2: renamed from usb0
USEMODULE='stdio_cdc_acm periph_usbdev_hs_utmi' BOARD=stm32f723e-disco make -j8 -C tests/usbus_cdc_ecm flash
Test results
[528733.480207] usb 1-4.3.4: reset high-speed USB device number 32 using xhci_hcd
[528733.707800] usb 1-4.4: new high-speed USB device number 111 using xhci_hcd
[528733.808257] usb 1-4.4: config 1 interface 0 altsetting 0 endpoint 0x81 has an invalid bInterval 255, changing to 11
[528733.808260] usb 1-4.4: config 1 interface 1 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 64
[528733.808263] usb 1-4.4: config 1 interface 1 altsetting 0 bulk endpoint 0x82 has invalid maxpacket 64
[528733.808642] usb 1-4.4: New USB device found, idVendor=1209, idProduct=7d01, bcdDevice= 1.00
[528733.808645] usb 1-4.4: New USB device strings: Mfr=3, Product=2, SerialNumber=4
[528733.808647] usb 1-4.4: Product: stm32f723e-disco
[528733.808649] usb 1-4.4: Manufacturer: RIOT-os.org
[528733.808651] usb 1-4.4: SerialNumber: A6BAC4E1B1E0806B
[528733.811988] cdc_acm 1-4.4:1.0: ttyACM1: USB ACM device
[528733.814456] cdc_ether 1-4.4:1.2 usb0: register 'cdc_ether' at usb-0000:00:14.0-4.4, CDC Ethernet Device, e6:75:97:3a:74:ba
[528733.854371] cdc_ether 1-4.4:1.2 enp0s20f0u4u4i2: renamed from usb0
USEMODULE='stdio_cdc_acm periph_usbdev_hs_ulpi' BOARD=stm32f746g-disco make -j8 -C tests/usbus_cdc_ecm flash
Test results
[529000.944482] usb 1-4.3.4: reset high-speed USB device number 32 using xhci_hcd
[529003.728260] usb 1-4.4: new high-speed USB device number 114 using xhci_hcd
[529003.833107] usb 1-4.4: config 1 interface 0 altsetting 0 endpoint 0x81 has an invalid bInterval 255, changing to 11
[529003.833111] usb 1-4.4: config 1 interface 1 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 64
[529003.833113] usb 1-4.4: config 1 interface 1 altsetting 0 bulk endpoint 0x82 has invalid maxpacket 64
[529003.833743] usb 1-4.4: New USB device found, idVendor=1209, idProduct=7d00, bcdDevice= 1.00
[529003.833747] usb 1-4.4: New USB device strings: Mfr=3, Product=2, SerialNumber=4
[529003.833749] usb 1-4.4: Product: stm32f746g-disco
[529003.833751] usb 1-4.4: Manufacturer: RIOT-os.org
[529003.833753] usb 1-4.4: SerialNumber: 66FE8934D1A363E0
[529003.837143] cdc_acm 1-4.4:1.0: ttyACM1: USB ACM device
[529003.839755] cdc_ether 1-4.4:1.2 usb0: register 'cdc_ether' at usb-0000:00:14.0-4.4, CDC Ethernet Device, 6a:88:1f:1f:b1:f0
[529003.879025] cdc_ether 1-4.4:1.2 enp0s20f0u4u4i2: renamed from usb0```

Issues/PRs references

Fixes #19359

Use the largest instead of the smallest number of available EPs for this definition. This became necessary to be able to use all EPs of a USB OTG HS peripheral if enabled.
Since the stm32f429i-disco uses the USB OTG HS instead of USB OTG FS peripheral, the number of available EPs is sufficient for this application. With the change of defining the largest number of available EPs for USBUS instead of the smallest number, the board can use all EPs of the USB OTG HS peripheral.
@github-actions github-actions bot added Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Mar 16, 2023
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 16, 2023
@gschorcht gschorcht requested a review from bergzand March 16, 2023 07:15
@riot-ci
Copy link

riot-ci commented Mar 16, 2023

Murdock results

✔️ PASSED

1cd128b cpu/stm32: reenable DMA for periph_usbdev

Success Failures Total Runtime
6882 0 6882 09m:49s

Artifacts

@gschorcht gschorcht force-pushed the drivers/usbdev_synopsys_dwc2/fix_dma_mode branch from f8d79d9 to 1cd128b Compare March 16, 2023 07:47
@benpicco benpicco requested a review from dylad March 16, 2023 11:11
@dylad
Copy link
Member

dylad commented Mar 16, 2023

I can have a look at it next week.

@benpicco benpicco added this to the Release 2023.04 milestone Mar 22, 2023
* valid for all devices, the smallest number of EPs must be used for
* multiple USB devices.
* @note USBUS allows only one definition of the number of available EPs, which
* is then used for all devices. To be able to use all EPs for devices
Copy link
Contributor

Choose a reason for hiding this comment

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

What is a device in this context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

device means in this context one USB device instance of type usbdev_t.

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Code looks good, I also tested on same54-xpro for good measure, even though it's not affected by the change.

The new doc comment is a bit confusing to me though. Since this fixes a regression, I think it should be in the release.

@MrKevinWeiss
Copy link
Contributor

Since this fixes a regression, I think it should be in the release.

Fixes are always welcome!

@benpicco
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 24, 2023

Build succeeded:

@bors bors bot merged commit 50cd32f into RIOT-OS:master Mar 24, 2023
@gschorcht gschorcht deleted the drivers/usbdev_synopsys_dwc2/fix_dma_mode branch April 12, 2023 14:11
@MrKevinWeiss
Copy link
Contributor

Does this need a backport?

@gschorcht
Copy link
Contributor Author

Hm, indeed. Good catch. I suspect that backporting PR #19472 makes no sense, or at least has no effect without backporting that PR, if it doesn't actually impact the function.

So yes, we should also backport this one.

@MrKevinWeiss
Copy link
Contributor

I think this is actually in already in I think... my bad.

@benpicco benpicco changed the title drivers/usbdev_synopsys_dwc2: fix and reenable DMA mode drivers/usbdev_synopsys_dwc2: fix and re-enable DMA mode Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sys/usbus: usbus_cdc_ecm does not work together with usbus_cdc_acm for STM32
5 participants