Skip to content

cdcecm: only activate OUT endpoint after interface selection #12533

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 29, 2019
Merged

cdcecm: only activate OUT endpoint after interface selection #12533

merged 1 commit into from
Oct 29, 2019

Conversation

bergzand
Copy link
Member

Contribution description

The OUT endpoint of the cdc ecm data endpoint is only expected to receive data when the alternative interface is activated. Signalling ready in the init function can cause issues as the endpoints are not yet enabled in the low level USB peripheral driver.

The out endpoint ready functions are now only called when either the alternative interface is selected or when netdev signals that the frame is copied by the receive function.

Before this commit the ready function was called on initialization and on reset, in both situations, the endpoint is not yet initialized.

Testing procedure

Verify that tests/usbus_cdc_ecm is still functional on any operating system. Testing a single operating system (Linux/MacOS) should be enough

Issues/PRs references

None (so far)

@bergzand
cdcecm: only activate OUT endpoint after interface selection
fdcadb6 
The OUT endpoint of the cdc ecm data endpoint is only expected to
receive data when the alternative interface is activated. Signalling
ready in the init function can cause issues as the endpoints are not yet
enabled in the low level USB peripheral driver.
@bergzand bergzand added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: USB Area: Universal Serial Bus labels Oct 21, 2019
@bergzand bergzand requested review from benpicco and dylad October 21, 2019 17:48
@dylad dylad added this to the Release 2020.01 milestone Oct 21, 2019
@dylad dylad self-assigned this Oct 21, 2019
@bergzand bergzand mentioned this pull request Oct 21, 2019
Merged
@dylad dylad added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines labels Oct 22, 2019
@bergzand bergzand mentioned this pull request Oct 23, 2019
Merged
7 tasks
@benpicco benpicco added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Oct 28, 2019
@benpicco
Copy link
Contributor

Still works on Linux with same54-xpro.

This is unrelated to this PR, but when using RIOT's USB network interface, my dmesg gets flooded with

[ 8934.779935] usb 2-1.2: cannot submit urb 0, error -28: not enough bandwidth
[ 8934.780185] usb 2-1.2: cannot submit urb 0, error -28: not enough bandwidth
[ 8934.780560] usb 2-1.2: cannot submit urb 0, error -28: not enough bandwidth
[ 8934.780809] usb 2-1.2: cannot submit urb 0, error -28: not enough bandwidth

benpicco
benpicco approved these changes Oct 28, 2019
View reviewed changes
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 makes sense and still works as before.

@bergzand bergzand merged commit 727263c into RIOT-OS:master Oct 29, 2019
@bergzand bergzand deleted the pr/usbus/cdcecm_ready_on_iface branch October 29, 2019 06:55
@bergzand
Copy link
Member Author

Thanks for the review!

@bergzand
Copy link
Member Author

This is unrelated to this PR, but when using RIOT's USB network interface, my dmesg gets flooded with

[ 8934.779935] usb 2-1.2: cannot submit urb 0, error -28: not enough bandwidth
[ 8934.780185] usb 2-1.2: cannot submit urb 0, error -28: not enough bandwidth
[ 8934.780560] usb 2-1.2: cannot submit urb 0, error -28: not enough bandwidth
[ 8934.780809] usb 2-1.2: cannot submit urb 0, error -28: not enough bandwidth

Could you please open an issue for this? I've never seen this message and I wonder if it is related to @dylad's issue with the same54-xpro

@benpicco
Copy link
Contributor

Uh never mind - that was actually my USB audio amplifier.
Now how it does make any sense for this to happen when my music streaming in Clementine gets interrupted because Network Manager decides to disable my 'real' Ethernet interface when selecting the RIOT one, I don't know.
I choose to rather not worry too much about it…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

@dylad dylad Awaiting requested review from dylad dylad is a code owner

Assignees

@dylad dylad

Labels
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: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Milestone
Release 2020.01
Development

Successfully merging this pull request may close these issues.
3 participants
@bergzand @benpicco @dylad