Skip to content

drivers: usb: udc: dwc3: initial implementation#84544

Draft
josuah wants to merge 1 commit intozephyrproject-rtos:mainfrom
tinyvision-ai-inc:pr-dwc3
Draft

drivers: usb: udc: dwc3: initial implementation#84544
josuah wants to merge 1 commit intozephyrproject-rtos:mainfrom
tinyvision-ai-inc:pr-dwc3

Conversation

@josuah
Copy link
Copy Markdown
Contributor

@josuah josuah commented Jan 25, 2025

Add support for Synopsys DWC3 core.

There is no hardware implementing this core in this PR, and users are encouraged to include this commit it on downstream PRs, then I will periodically re-integrate the modifications they needed in this PR.

This can help synchronizing the work of everyone on a common basis.

Active branches using this:

Related issues/PRs:

@josuah josuah marked this pull request as draft January 25, 2025 04:46
@zephyrbot zephyrbot added the area: USB Universal Serial Bus label Jan 25, 2025
@josuah josuah added area: Drivers Experimental Experimental features not enabled by default labels Jan 25, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2025

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions Bot added the Stale label May 5, 2025
@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented May 5, 2025

Once USB3 support lands (I anticipate after device -> device_next migration?) and once we could test it with IRQ support, I will reopen this PR.

If anybody need this before that happen, do feel free to ping me about it.

@github-actions github-actions Bot removed the Stale label May 6, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 6, 2025

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions Bot added the Stale label Jul 6, 2025
@github-actions github-actions Bot closed this Jul 20, 2025
@natto1784
Copy link
Copy Markdown
Contributor

can this (and the dependency) be reopened? i feel like this should be a welcome addition

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Jan 28, 2026

@natto1784 yes absolutely. However it might need a platform with which this can be tested with, otherwise it is dead code.

Do you know of any available hardware that can be picked for testing it?

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Jan 28, 2026

It seems like you are working on AM243x?

Here is the current state of this driver:

  • It is still being developed in rare occasions internally at tinyVision.ai for the needs of the https://tinyclunx33.tinyvision.ai/ (which remains a downstream platform with no current plan of upstreaming, not my choice and could change some day maybe)
  • Isochronous endpoint support might be on the roadmap
  • It lacks essential features but works well for as long as we used it with control and bulk endpoints
  • It requires the USB 3 support to be merged, but it is also possible to upstream an USB2-only version in the meantime.

I am @josuah.demangeon on Discord if that helps.

@natto1784
Copy link
Copy Markdown
Contributor

natto1784 commented Feb 12, 2026

Hi, @josuah, after some efforts with minimal changes/fixes to your driver (+ a small glue driver of our own), I have managed to get the CDC ACM sample working on AM62x SK platform which is already in the Zephyr tree, so perhaps we can get this PR re-opened, or is there some other approach to compiling patches in one patchset that you would prefer? I am open to talking on discord as well if that is what you prefer, my nickname is @ amneesh on the Zephyr discord.

In the meantime can you tell me the rationale behind adding the ev_* DT regions? They do not seem to be a standard thing and I have also removed them in my code.

Note that I got it working with the next stack and have not tested with the legacy one, but it should work regardless.

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Feb 12, 2026

Thank you for trying and making it so far!

It looks like there are two parallel efforts to get DWC3 working:

https://github.com/piterzhang/zephyr/tree/dwc3-rk3588

Luckily, both platforms use the same basis for the driver, so the implementation should be relatively similar.

can get this PR re-opened

If it was, I would need to be the one pushing the commits, so best if you open a new one.

have not tested with the legacy one, but it should work regardless.

The legacy stack uses different driver APIs, so it will not work with the legacy stack, but this is not a bad thing as it is expected to be deprecated relatively soon.

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Feb 12, 2026

DT_INST_REG_ADDR_BY_NAME() "REG" here stands for "Register", as in reg = <0x.....>;.

ev_* is what LiteX uses, an FPGA-based SoC architecture, so not relevant to DWC3 driver.

It is best to ignore it.

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Feb 12, 2026

I will ping you in the "DWC3 Upstreaming" thread...

@jfischer-no jfischer-no reopened this Feb 12, 2026
@jfischer-no jfischer-no removed the Stale label Feb 12, 2026
@jfischer-no
Copy link
Copy Markdown
Contributor

can get this PR re-opened

If it was, I would need to be the one pushing the commits, so best if you open a new one.

Let's keep it open until there is a new one. Alternatively, you can allow to push to this branch.

Comment thread drivers/usb/udc/udc_dwc3.c Outdated
@@ -0,0 +1,1593 @@
/*
* SPDX-License-Identifier: MIT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josuah Why is license MIT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It used to be the fallback license that tinyVision.ai used for everything, I can request it to be changed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@natto1784
Copy link
Copy Markdown
Contributor

natto1784 commented Feb 12, 2026 via email

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Feb 12, 2026

get rid of it as it's vendor specific code

Yes, totally, it better not be there and was planned to be removed for something more generic.

@josuah josuah force-pushed the pr-dwc3 branch 2 times, most recently from eb1629d to 3ee1194 Compare February 12, 2026 19:08
@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Mar 10, 2026

Force-push: from https://github.com/piterzhang/zephyr/tree/dwc3-rk3588-debug

Comment on lines +1627 to +1635
void udc_dwc3_irq_handler(void *ptr)
{
const struct device *const dev = ptr;
struct udc_dwc3_data *priv = udc_get_private(dev);

/* TODO: IRQs convert work handler to non-delayed */
k_work_reschedule(&priv->dwork, K_NO_WAIT);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During IRQ mode testing on rk3588, it was found that the &priv->dwork in k_work_reschedule(&priv->dwork, K_NO_WAIT); is not executed. There is no interrupt clear operation in udc_dwc3_irq_handler, so the handler is continuously triggered, preventing priv->dwork from ever being scheduled.

Copy link
Copy Markdown

@piterzhang piterzhang Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied the content of the &priv->dwork function into udc_dwc3_irq_handler, enabling it to run in IRQ mode. However, this change restricts it to IRQ mode only and makes it incompatible with the current polling mode. Additionally, this implementation is far from elegant—I only did it to verify feasibility. Adopting the dwc2 approach, where a thread is spawned to receive events and events are sent in the interrupt service routine, might be a good idea.
https://github.com/piterzhang/zephyr/blob/dwc3-rk3588-debug/drivers/usb/udc/udc_dwc3.c

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, can you check if this helps: c0a5c64 ? I used it a lot while testing, it prevents the thread from getting blocked.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems useful outside of debugging as IRQs would be no-op when processing events, so I added it too.

Comment thread drivers/usb/udc/udc_dwc3.c Outdated
Comment thread drivers/usb/udc/udc_dwc3.c Outdated
Comment thread drivers/usb/udc/udc_dwc3.c Outdated
Comment thread drivers/usb/udc/udc_dwc3.c Outdated
Comment thread drivers/usb/udc/udc_dwc3.c Outdated
Comment thread drivers/usb/udc/udc_dwc3.c
Comment thread drivers/usb/udc/udc_dwc3.c Outdated
@josuah josuah force-pushed the pr-dwc3 branch 4 times, most recently from 907e1c5 to c7fbe3e Compare March 11, 2026 15:01
@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Mar 11, 2026

@piterzhang whenever this makes sense, you might be able to reintegrate the latest version of the driver, to check that it still works with what you have.

Comment on lines +1900 to +1901
sys_set_bits(base + UDC_DWC3_U2PHYCTRL1, UDC_DWC3_U2PHYCTRL1_SEL_INTERNALCLK);
sys_set_bits(base + UDC_DWC3_U2PHYCTRL2, UDC_DWC3_U2PHYCTRL2_REFCLK_SEL);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UDC_DWC3_U2PHYCTRL1 UDC_DWC3_U2PHYCTRL2 are vendor register should delete

Comment on lines +698 to +701
trb->ctrl = ctrl;
trb->addr_lo = LO32((uintptr_t)buf->data);
trb->addr_hi = HI32((uintptr_t)buf->data);
trb->status = ep_data->cfg.caps.in ? buf->len : buf->size;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
trb->ctrl = ctrl;
trb->addr_lo = LO32((uintptr_t)buf->data);
trb->addr_hi = HI32((uintptr_t)buf->data);
trb->status = ep_data->cfg.caps.in ? buf->len : buf->size;
trb->addr_lo = LO32((uintptr_t)buf->data);
trb->addr_hi = HI32((uintptr_t)buf->data);
trb->status = ep_data->cfg.caps.in ? buf->len : buf->size;
trb->ctrl = ctrl;

there may have risk, if trb->ctrl set HWO IP do DMA, but this time adress are not set

config UDC_DWC3
bool "Synopsys USB device controller driver"
default y
select NOCACHE_MEMORY if ARCH_HAS_NOCACHE_MEMORY_SUPPORT
Copy link
Copy Markdown

@piterzhang piterzhang Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
select NOCACHE_MEMORY if ARCH_HAS_NOCACHE_MEMORY_SUPPORT
select NOCACHE_MEMORY if ARCH_HAS_NOCACHE_MEMORY_SUPPORT
select UDC_DRIVER_HAS_HIGH_SPEED_SUPPORT

UDC_DRIVER_HAS_HIGH_SPEED_SUPPORT ;USB APP use it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo, this should be set by the application itself. either this or superspeed

Copy link
Copy Markdown

@piterzhang piterzhang Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@natto1784
My understanding is that if the controller layer does not support high speed, the application layer cannot enable this macro. I tried enabling it in the webusb application but couldn't find a way to do so. Without configuring this option, the webusb application only compiles the Full Speed (FS) configuration into the ELF, but dwc3 runs in High Speed (HS), causing a mismatch error. Later, I saw this option used in Kconfig.dwc2, and after attempting to add it to dwc3's Kconfig, it passed.

The macro used by the application layer is USBD_SUPPORTS_HIGH_SPEED, and it is used as follows in include/zephyr/usb/usbd.h:

#define USBD_DEFINE_CLASS(class_name, class_api, class_priv, class_v_reqs)	\
	static struct usbd_class_data class_name = {				\
		.name = STRINGIFY(class_name),					\
		.api = class_api,						\
		.v_reqs = class_v_reqs,						\
		.priv = class_priv,						\
	};									\
	static STRUCT_SECTION_ITERABLE_ALTERNATE(				\
		usbd_class_fs, usbd_class_node, class_name##_fs) = {		\
		.c_data = &class_name,						\
	};									\
	IF_ENABLED(USBD_SUPPORTS_HIGH_SPEED, (					\
	static STRUCT_SECTION_ITERABLE_ALTERNATE(				\
		usbd_class_hs, usbd_class_node, class_name##_hs) = {		\
		.c_data = &class_name,						\
	};									\
	))									\
	IF_ENABLED(USBD_SUPPORTS_SUPER_SPEED, (					\
	static STRUCT_SECTION_ITERABLE_ALTERNATE(				\
		usbd_class_ss, usbd_class_node, class_name##_ss) = {		\
		.c_data = &class_name,						\
	};									\
	))

Additionally: If it's convenient for you, you can configure your board to HS, compile the webusb application, and try it. This should reproduce my error.

Copy link
Copy Markdown
Contributor Author

@josuah josuah Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch about UDC_DRIVER_HAS_HIGH_SPEED_SUPPORT. Adding it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be set by the application itself

Maybe CONFIG_UDC_DRIVER_HIGH_SPEED_SUPPORT_ENABLED?

int udc_dwc3_ep_enqueue(const struct device *const dev, struct udc_ep_config *const ep_cfg,
struct net_buf *buf)
{
struct udc_dwc3_ep_data *const ep_data = (void *)ep_cfg;
Copy link
Copy Markdown

@piterzhang piterzhang Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use struct udc_dwc3_ep_data *const ep_data =CONTAINER_OF(ep_cfg,struct udc_dwc3_ep_data,cfg);

easier to understand . udc_dwc3_ep_enable, udc_dwc3_ep_clear_halt, udc_dwc3_ep_disable ,udc_dwc3_ep_enqueue ,

@piterzhang
Copy link
Copy Markdown

piterzhang commented Mar 14, 2026

@piterzhang whenever this makes sense, you might be able to reintegrate the latest version of the driver, to check that it still works with what you have.

After making the following changes in the latest PR, the WebUSB application can run successfully on the RK3588 SoC:

  1. Add RK3588-specific PHY initialization and quirk configuration.

  2. Add select UDC_DRIVER_HAS_HIGH_SPEED_SUPPORT to drivers/usb/udc/Kconfig.dwc3.

  3. Move udc_submit_event(dev, UDC_EVT_RESET, 0); from udc_dwc3_on_usb_reset to udc_dwc3_on_connect_done, for the reason discussed earlier on Discord.

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Mar 17, 2026

I will soon try to refactor some small part for control packets according to this:

And will integrate @piterzhang modifications at the same time.

I will let you know when I could find time for it, but feel free to ping me for progress!

Add support for Synopsys DWC3 core, with only BULK endpoints supported at
the moment, as well as an initial vendor quirks system.

Signed-off-by: Josuah Demangeon <me@josuah.net>
@sonarqubecloud
Copy link
Copy Markdown

@jfischer-no
Copy link
Copy Markdown
Contributor

@josuah Do you want to have USB3 support in the stack, or will you limit this driver to support high-speed only and add USB3 support in the stack later?

@josuah
Copy link
Copy Markdown
Contributor Author

josuah commented Apr 1, 2026

@jfischer-no I was thinking of keeping it USB2, and have a separate USB3 PR based on top of this DWC3 PR.

This way all the noise about DWC3 hardware can be kept away from USB3 protocol discussions.

If anything else is preferred let me know!

@natto1784
Copy link
Copy Markdown
Contributor

natto1784 commented Apr 6, 2026

I for one, do not mind HS-only DWC3 driver first before adding SS support. Also I think we can open this for review soon, would be great if we could get this merged for 4.5.

@natto1784
Copy link
Copy Markdown
Contributor

Hi, has there been some progress on using udc_setup_received yet? I can volunteer to make the change if you're bit tied up with stuff.

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

Labels

area: Drivers area: USB Universal Serial Bus Experimental Experimental features not enabled by default

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants