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

sys/usbus/cdc/ecm: fix High-Speed mode #19358

Merged
merged 6 commits into from
Mar 7, 2023

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR provides some changes to fix the USBUS CDC ECM interface in High-Speed mode.

In High-Speed mode, the EP data size has to be at least 512 bytes instead of 64 Byte in Full-Speed mode. To be able to define configurations like EP data sizes depending on whether Full-Speed or High-Speed USB device peripherals are used, the feature periph_usbdev_hs/HAD_PERIPH_USBDEV_HS is introduced.

Testing procedure

Use tests/usbus_cdc_ecm and any board with USB HS connector, for example:

USEMODULE=periph_usbdev_hs_utmi BOARD=stm32f723e-disco make -j8 -C tests/usbus_cdc_ecm flash

ping command works with this PR but doesn't work without this PR.

Issues/PRs references

To be able to define configurations like EP data sizes depending on whether full-speed or high-speed USB device peripherals are used, the feature `periph_usbdev_hs` is introduced.
To be able to define configurations like EP data sizes depending on whether full-speed or high-speed USB device peripherals are used, the feature `HAS_PERIPH_USBDEV_HS` is introduced.
CDC ECM driver/netdev is only working in High-Speed mode if the EP data size is at least 512 byte.
@github-actions github-actions bot added Area: boards Area: Board ports Area: build system Area: Build system Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: USB Area: Universal Serial Bus labels Mar 7, 2023
@gschorcht gschorcht requested a review from bergzand March 7, 2023 10:39
@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 7, 2023
@benpicco benpicco requested a review from dylad March 7, 2023 10:44
@riot-ci
Copy link

riot-ci commented Mar 7, 2023

Murdock results

✔️ PASSED

4f49fc8 usbus/cdc/ecm: increase EP data size in HS mode

Success Failures Total Runtime
6919 0 6919 13m:06s

Artifacts

@benpicco benpicco requested a review from maribu March 7, 2023 16:23
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.

Looks good to me.
I don't own these boards but it should be pretty straightforward.

@dylad
Copy link
Member

dylad commented Mar 7, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 7, 2023

Build succeeded:

@bors bors bot merged commit 6c1de71 into RIOT-OS:master Mar 7, 2023
@gschorcht
Copy link
Contributor Author

@dylad Thanks for reviewing.

bors bot added a commit that referenced this pull request Mar 31, 2023
19437: boards: fix USB configuration for stm32f429i-disco r=bergzand a=gschorcht

### Contribution description

This PR fixes the problem that CDC ECM is not working for `stm32f429i-disco`.

With PR #19358, the EP data size for CDC ECM bulk endpoints was increased to 512 byte to fix the problem that CDC ECM was not working for boards with USB High-Speed peripherals. Module `periph_usbdev_hs` was introduced to indicate whether a high-speed peripheral is used.

However, the `stm32f429i-disco` board uses a mixture of USB OTG HS peripheral together with the on-hip FS PHY. Using the USB OTG HS peripheral together with a FS-PHY and increasing the EP data size for the CDC ECM bulk endpoint to 512 bytes doesn't work. Therefore, module `periph_usbdev_hs` is not used for now to workaround the problem. Thus, the maximum number of EPs used is that of the USB OTG FS peripheral, which is only 4. This is not sufficient for this test application since the board uses `stdio_cdc_acm`. The board is therefore blacklisted.

### Testing procedure

Use a `stm32f429i-disc1` or a `stm32f429i-disco` board and flash the test app:
```
USEMODULE=stdio_uart BOARD=stm32f429i-disco make -j8 -C tests/usbus_cdc_ecm flash
```
With this PR it should be possible to ping the board.

### Issues/PRs references

Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
bors bot added a commit that referenced this pull request Mar 31, 2023
19438: usbus: Add support for full speed with high speed phy r=miri64 a=bergzand

### Contribution description

This adds infrastructure around usbus and usbdev to query the speed of the USB link after enumeration. This as the maximum speed of the link might be slower than the maximum speed of the peripheral. This is the case with the stm32f429i-disco board that has a full speed phy coupled with the high speed peripheral.

This also adds the necessary code to the cdc_ecm code to use the correct packet size. The allocated buffer size is not modified with this PR unfortunately.

### Testing procedure

The `cdc_ecm` handler should work with a HS peripheral coupled with a FS phy.

### Issues/PRs references

Fixes an issue caused by #19358


Co-authored-by: Koen Zandberg <koen@bergzand.net>
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
@gschorcht gschorcht deleted the sys/usbus/cdc/ecm/fix_hs_mode branch September 10, 2023 11:29
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: build system Area: Build system Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: sys Area: System 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 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.

4 participants