-
Notifications
You must be signed in to change notification settings - Fork 158
Rework USB driver #760
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
Rework USB driver #760
Conversation
|
@piotrfila I will try take a look on this over the weekend. Going back to |
|
I plan to adress that in a separate PR, this one is already pretty big. I just wanted to get the descriptors out of the way first. Despite the large diffs in the core/usb.zig and rp2xxx/hal/usb.zig the logic is largely unchanged there. Do you mean the FIFO in the cdc driver? I think that could be retired entirely in favor of std reader/writer, but I have yet to write any code relating to that. |
arkadiuszwojcik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work! I put few comments and will have another look tomorrow. Did you test it on both RP2040 a RP2350?
|
Someone reported it in the discord as well. That bug is also present in the main branch (to be specific the main branch does not work at all - you have to go a few commits back). I'm not quite sure where the bug is, but I suspect the fifo implementation. I'd rather just rewrite the cdc driver and usb device code in a separate PR. |
|
Sorry, I haven’t checked the code from main for a long time. Yes, it looks like the code doesn’t work at all. @mattnite @Grazfather, what do you think about it? This code is not 100% correct yet, but it works better than the current code in master (some past PR had to break it). The solution is also much cleaner than the current USB driver code, so it’s a welcome improvement and a step in the right direction. |
|
@arkadiuszwojcik @piotrfila that works for me, I don't know enough about USB yet to review this properly. I'm happy to merge improvements that are imperfect. |

This PR is my (yet another) shot at a better USB driver implementation.
Key changes:
This is a succesor to #658, but this time I tried to change as little controller logic as possible to avoid bugs.
As last time, I'm open to suggestions.
There is still much work to be done in this area, but this PR is already big. Nice additions would include: