Implement usbh replug on pico #1193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hathach
force-pushed
the
implement_usbh_replug_on_pico
branch
from
fcaf272
to
3e3fe1e
Compare
hathach
approved these changes
Nov 30, 2021
There was a problem hiding this comment.
Thank you very much for the PR. I have been busy with other works, and only have time to review this now. It is a great fix. host stack apparently needs more dev time. Though I found the while() with find_first loop is not sufficient. We are better with just 1 for loop to close endpoint.
Therefore I have update the PR (also rebased since master has several updates).
PS: confirmed that it fix #1192, probably fix issue raspberrypi/pico-sdk#483 as well.
Comment on lines
437
to
435
| while (ep) | ||
| { | ||
| // in case it is an interrupt endpoint, disable it | ||
| usb_hw_clear->int_ep_ctrl = (1 << (ep->interrupt_num + 1)); | ||
| usb_hw->int_ep_addr_ctrl[ep->interrupt_num] = 0; | ||
| // unconfigure the endpoint | ||
| ep->configured = false; | ||
| *ep->endpoint_control = 0; | ||
| *ep->buffer_control = 0; | ||
| hw_endpoint_reset_transfer(ep); | ||
| ep->dev_addr = ADDR_INVALID; // don't find this one again | ||
| ep = _hw_find_first_endpoint(dev_addr); | ||
| } |
There was a problem hiding this comment.
this loop is not sufficient, we should do only 1 for loop to close endpoint with matched address
PS: updated with latest push
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request resolves issue #1192 in my testing.
When I was debugging the issue, I found that the host could not send report requests because the endpoint was claimed already. The first commit implements hcd_device_close() more completely so the hardware does not think the endpoints are doing anything and the second commit clears the claimed and busy flags for the device endpoints.
I am very new to this project. Is directly clearing the claimed and busy flags safe?