-
Notifications
You must be signed in to change notification settings - Fork 9.1k
drivers: usb: uhc_dwc2: add initial support #102967
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,8 +3,10 @@ | |||||
| # SPDX-License-Identifier: Apache-2.0 | ||||||
|
|
||||||
| zephyr_library() | ||||||
| zephyr_library_include_directories(${ZEPHYR_BASE}/drivers/usb/common/) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this should be inside
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation seems to match UDC. This probably matches the maintainer/collaborators preferred organization. zephyr/drivers/usb/udc/CMakeLists.txt Lines 6 to 7 in 7edd883
On my side no preference.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are some functions, that might be merged with their implementation in For example:
So, after the uhc will be merged, I can check the possibility to merge them and propose the During this, it will be possible to consider adding the Otherwise, I do not mind to keep it similar in udc and uhc for now. |
||||||
|
|
||||||
| zephyr_library_sources(uhc_common.c) | ||||||
| zephyr_library_sources_ifdef(CONFIG_UHC_DWC2 uhc_dwc2.c) | ||||||
| zephyr_library_sources_ifdef(CONFIG_UHC_MAX3421E uhc_max3421e.c) | ||||||
| zephyr_library_sources_ifdef(CONFIG_UHC_VIRTUAL uhc_virtual.c) | ||||||
| zephyr_library_sources_ifdef(CONFIG_UHC_NXP_EHCI uhc_mcux_common.c uhc_mcux_ehci.c) | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| # Copyright (c) 2026 Roman Leonov <jam_roma@yahoo.com> | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| config UHC_DWC2 | ||
| bool "DWC2 USB host controller driver" | ||
| depends on DT_HAS_SNPS_DWC2_ENABLED | ||
| default y | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In PR description, you wrote that building needs
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about simply add
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the uhc driver implementation, there is not strict dependency from UDC and it is possible to have both at the same time. I added the Right now I know a bit more about the environment so I'll double check the conflict, maybe it is possible to make it right. But if it is a DT- conflict in utilizing the same peripheral (usb_otg), then let me know. UPD: OK, seems that it is technically impossible. So seems that right now it gives us only two options: make them mutually exclusive or assign the node to UDC/UHC via menuconfig. I'll think about it a bit more, but let me know if there are any preferences.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might only have been a problem before this was merged (which required manually turning off the device mode): #105005 As enabling host/device is still happening manually so it might work to enable both by default, and the user would then be responsible to enable host or device stack.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we enable both at the same time - we get the multiple definition of the node during the build. I don't think (please, correct me if I am missing something) that changing the device tree description to having the assign of the role there something like this does make sense:
So, I think that simple mutual exclusive like above is the best option at the moment.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The devicetree is there to describe what exists in the hardware, and things like "role" is how this hardware is used by drivers, which I think is a different concern. If a device needs multiple drivers (i.e. host and device mode), indeed it needs multiple devicetree nodes, such as having child nodes, one for device and one for host, both always enabled and Kconfig used to select what to leverage, as usual? Though this seems a long-term issue, not needing a solution on this PR? Still glad to discuss it. |
||
| select EVENTS | ||
| help | ||
| DWC2 USB host controller driver. | ||
|
|
||
| if UHC_DWC2 | ||
|
|
||
| config UHC_DWC2_MAX_CHANNELS | ||
|
roma-jam marked this conversation as resolved.
|
||
| int "UHC DWC2 driver maximum supported number of channels" | ||
| default 16 | ||
| range 1 16 | ||
| help | ||
| Number of channels driver can support. | ||
| Reduce this value to save memory. | ||
| Increase to support more concurrent transfers. | ||
|
|
||
| config UHC_DRIVER_HIGH_SPEED_SUPPORT_ENABLED | ||
| bool "UHC DWC2 High speed" | ||
| default y if SOC_SERIES_NRF54LX | ||
| help | ||
| Enables HS support code if the SoC/board can support it. | ||
|
|
||
| config UHC_DWC2_STACK_SIZE | ||
| int "UHC DWC2 driver thread stack size" | ||
| default 512 | ||
| help | ||
| DWC2 driver thread stack size. | ||
|
|
||
| config UHC_DWC2_THREAD_PRIORITY | ||
| int "UHC DWC2 driver thread priority" | ||
| default 8 | ||
| help | ||
| DWC2 driver thread priority. | ||
|
|
||
| config UHC_DWC2_DEBOUNCE_DELAY_MS | ||
| int "Debounce delay in ms" | ||
| default 250 | ||
| help | ||
| On connection of a USB device, the USB 2.0 specification requires | ||
| a "debounce interval with a minimum duration of 100ms" to allow the connection to stabilize | ||
| (see USB 2.0 chapter 7.1.7.3 for more details). | ||
| During the debounce interval, no new connection/disconnection events are registered. | ||
|
|
||
| The default value is set to 250 ms to be safe. | ||
|
|
||
| config UHC_DWC2_RESET_HOLD_MS | ||
| int "Reset hold in ms" | ||
| default 50 | ||
| help | ||
| Root-port reset hold time in milliseconds. | ||
|
|
||
| USB 2.0 requires ordinary reset signaling to be asserted for at least | ||
| 10 ms, but reset issued from a root port must last at least 50 ms.. | ||
|
|
||
| The default value is set to 50 ms. | ||
|
|
||
| config UHC_DWC2_RESET_RECOVERY_MS | ||
| int "Reset recovery delay in ms" | ||
| default 30 | ||
| help | ||
| Post-reset recovery delay in milliseconds. | ||
|
|
||
| USB 2.0 requires system software to allow at least 10 ms for reset | ||
| recovery after the port stops driving reset, before the attached | ||
| device is expected to respond to data transfers. | ||
|
|
||
| The attached device may ignore data transfers during this recovery | ||
| interval. | ||
|
|
||
| The default value is set to 30 ms. | ||
|
|
||
| config UHC_DWC2_SET_ADDR_DELAY_MS | ||
| int "Delay after set device address" | ||
| default 10 | ||
| help | ||
| "After successful completion of the Status stage, the device is allowed a SetAddress() | ||
| recovery interval of 2 ms. At the end of this interval, the device must be able to accept | ||
| Setup packets addressed to the new address. Also, at the end of the recovery interval, the | ||
| device must not respond to tokens sent to the old address (unless, of course, the old and new | ||
| address is the same)." See USB 2.0 chapter 9.2.6.3 for more details. | ||
|
|
||
| The default value is set to 10 ms to be safe. | ||
|
|
||
| endif # UHC_DWC2 | ||
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.
This should go into separate commit (same PR is fine, just do it in a first commit).
Uh oh!
There was an error while loading. Please reload this page.
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.
Done, now this PR contains two commits.
Just for me to the future: what is the reason for that and which question should I ask myself to understand, do I need to split changes in several commits or can keep them in one commit?
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.
The reason is to make things cherry-pickable and also to make things easily revertable. The register header is used by both UDC and UHC. If there were some changes to be backported to UDC (especially if there were some that also touched the register definitions), then keeping the commits separate makes it possible to easily omit the UHC part.