-
Notifications
You must be signed in to change notification settings - Fork 137
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
add controller register interface and support TinyUSB #22
add controller register interface and support TinyUSB #22
Conversation
* add root port suspended bit, more with hw root port * move host and device specific code to pio_usb_host/device.c * support endpoint stalled, nak * merge send in/out/setup into send_token() * add PIO_USB_USE_TINYUSB to select tinyusb
thank you for your PR, I will review changes within a few days. |
no problems, take your time :) |
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.
I think this PR can be merged after minor revisions.
I confirmed that we can use PIO USB port with TinyUSB stack, and maintainability is improved by separating files. Great work!!
Please refer following review comments.
- I'm palnning move all sources/headers to src/ folder. Because this may affect tinyusb makefiles, could you do that in this PR?
- Could you add description about TinyUSB's hid_to_cdc example to README.md?
- There are also some codes that need to be confirmed.
Following comments are not necessary to accept this PR, but I want to hear your opinion: How do you think about This is also not related to this PR: TinyUSB's hid_to_cdc example are not work on Windows because ITF_NUM_TOTAL is wrong. This works by comment out line 84 of usb_descriptors.c |
Great thank you very much 👍 💯
Will do this in the next push
Sure will do, maybe I will add it to examples of this repo as well. Since the tinyusb example is a bit more generic and include too many cross-platform #ifdef that complicate the rp2040 user.
Ah you are right on the multiple host ports scenario. I will try to fix those and do a full revise as well.
Yeah, I am glad you ask, I think about this previously, and intend to make follow up PRs for this very purpose. IMHO, there should be an application config file such as
Ah right, just fixed it, it is copy-paste mistake, Linux is apparently more forgiving on this. |
Hey Dudes, The work that you are doing here is simply amazing. Thank you so much. |
@sekigon-gonnoc 2 new commits doesn't change the code yet, since we are moving codes around (to https://github.com/hathach/Pico-PIO-USB/actions/runs/2283033358 Will push more update in upcoming days. |
TODO: try out multiple host ports, will push/post when there is more update. PS: Let me know if you have any plan to change or wrap macro in pio_usb_configuratio.h . I could make the changes as well, or we can wait and make the change afterward (follow up PR) if you want to spend time to think more |
I still haven't tried out multiple ports, since TinyUSB currently only support max 2 roothub ports (1 for native device, 1 for pio-usb host). Adding more rhport is possible, but required a bit of changing in macro naming etc ... I will try that later on. But the changes of c12e4c5 should be sufficient enough. For PIO_USB_DEFAULT_CONFIG, I wrap Let me know if update look good and/or any request changes you would like to be done. PS: the examples/host/hid_to_cdc is renamed to examples/dual/host_hid_to_device_cdc in tinyusb |
Great, thank you for testing out the multiple ports changes. I have no other TODO, please merge it when you have time. PS: Since the current ci use an tinyusb branch |
Thank you for your corporation. |
it is my pleasure |
FYI, I made an twitter video about this PR with mentioned to your github: |
Whoop, after 2-3 weeks of works, I finally wrap up this PR. This is more difficult than my initial thought. Your repo is going uber low (bit banging) into USB, I learn quite a lot more from your code despite working with USB for quite some time. I could only imagine amount of time and effort you spend on oscilloscope to get this working. Really brilliant work !!
This PR implement #14 and add an register interface for pio-usb port (host/device) similar to native usb controller on rp2040 e.g event interrupt status, ep complete/error/stalled and irq handler. This allow pio-usb "low-level controller part" can be used with tinyusb.
Following is the notable changes:
pio_usb_ll.h
for controller register interface (along with other internal common function of host & device)This is no way perfect and not optimized, but I did quite a bit of testing with both host and device. Let me know what you think and would change, I hope we could merge this soon.
Note: For testing with TinyUSB
host/hid_to_cdc
, which is configured to use native usb as cdc device, and pio usb as host to get hid input.@tannewt @ladyada