From a605ace2d5e90f6da40dcf7f71326ddf96f8dad5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Fri, 7 Feb 2025 08:35:02 +0100 Subject: [PATCH 01/12] [nrf fromtree] drivers: udc: Do not allow lock/unlock to fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit USB stack does not check api->lock() and api->unlock() return value and all UDC drivers block without timeout in its lock() and unlock() api implementations. There is no realistic way to handle lock() and unlock() errors without making USB device stack API unnecessarily complex. Remove the return type from lock() and unlock() to make it clear that the functions must not fail. Signed-off-by: Tomasz Moń (cherry picked from commit 23232e6bb23fea1a1271ad58cbf58e877ac103ed) --- drivers/usb/udc/udc_ambiq.c | 8 ++++---- drivers/usb/udc/udc_dwc2.c | 8 ++++---- drivers/usb/udc/udc_it82xx2.c | 8 ++++---- drivers/usb/udc/udc_kinetis.c | 8 ++++---- drivers/usb/udc/udc_mcux_ehci.c | 8 ++++---- drivers/usb/udc/udc_mcux_ip3511.c | 8 ++++---- drivers/usb/udc/udc_nrf.c | 8 ++++---- drivers/usb/udc/udc_numaker.c | 8 ++++---- drivers/usb/udc/udc_renesas_ra.c | 8 ++++---- drivers/usb/udc/udc_rpi_pico.c | 8 ++++---- drivers/usb/udc/udc_skeleton.c | 8 ++++---- drivers/usb/udc/udc_smartbond.c | 8 ++++---- drivers/usb/udc/udc_stm32.c | 8 ++++---- drivers/usb/udc/udc_virtual.c | 8 ++++---- include/zephyr/drivers/usb/udc.h | 4 ++-- 15 files changed, 58 insertions(+), 58 deletions(-) diff --git a/drivers/usb/udc/udc_ambiq.c b/drivers/usb/udc/udc_ambiq.c index ffc84c0413d8..f6f342fc21cd 100644 --- a/drivers/usb/udc/udc_ambiq.c +++ b/drivers/usb/udc/udc_ambiq.c @@ -594,14 +594,14 @@ static int udc_ambiq_shutdown(const struct device *dev) return 0; } -static int udc_ambiq_lock(const struct device *dev) +static void udc_ambiq_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_ambiq_unlock(const struct device *dev) +static void udc_ambiq_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static void ambiq_handle_evt_setup(const struct device *dev) diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index d1fc555d8393..13aa6c1738a1 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -2316,14 +2316,14 @@ static int dwc2_driver_preinit(const struct device *dev) return 0; } -static int udc_dwc2_lock(const struct device *dev) +static void udc_dwc2_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_dwc2_unlock(const struct device *dev) +static void udc_dwc2_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static void dwc2_on_bus_reset(const struct device *dev) diff --git a/drivers/usb/udc/udc_it82xx2.c b/drivers/usb/udc/udc_it82xx2.c index 6affd349e722..0c61b7b17db8 100644 --- a/drivers/usb/udc/udc_it82xx2.c +++ b/drivers/usb/udc/udc_it82xx2.c @@ -1495,14 +1495,14 @@ static int it82xx2_shutdown(const struct device *dev) return 0; } -static int it82xx2_lock(const struct device *dev) +static void it82xx2_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int it82xx2_unlock(const struct device *dev) +static void it82xx2_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static const struct udc_api it82xx2_api = { diff --git a/drivers/usb/udc/udc_kinetis.c b/drivers/usb/udc/udc_kinetis.c index c471fb15ebda..70fae41b4871 100644 --- a/drivers/usb/udc/udc_kinetis.c +++ b/drivers/usb/udc/udc_kinetis.c @@ -1069,14 +1069,14 @@ static int usbfsotg_shutdown(const struct device *dev) return 0; } -static int usbfsotg_lock(const struct device *dev) +static void usbfsotg_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int usbfsotg_unlock(const struct device *dev) +static void usbfsotg_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static int usbfsotg_driver_preinit(const struct device *dev) diff --git a/drivers/usb/udc/udc_mcux_ehci.c b/drivers/usb/udc/udc_mcux_ehci.c index 179e9282b18f..489540a9d159 100644 --- a/drivers/usb/udc/udc_mcux_ehci.c +++ b/drivers/usb/udc/udc_mcux_ehci.c @@ -64,14 +64,14 @@ struct udc_mcux_event { K_MEM_SLAB_DEFINE(udc_event_slab, sizeof(struct udc_mcux_event), CONFIG_UDC_NXP_EVENT_COUNT, sizeof(void *)); -static int udc_mcux_lock(const struct device *dev) +static void udc_mcux_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_mcux_unlock(const struct device *dev) +static void udc_mcux_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static int udc_mcux_control(const struct device *dev, usb_device_control_type_t command, diff --git a/drivers/usb/udc/udc_mcux_ip3511.c b/drivers/usb/udc/udc_mcux_ip3511.c index 7aa751a31133..f8db1221e074 100644 --- a/drivers/usb/udc/udc_mcux_ip3511.c +++ b/drivers/usb/udc/udc_mcux_ip3511.c @@ -64,14 +64,14 @@ struct udc_mcux_event { K_MEM_SLAB_DEFINE(udc_event_slab, sizeof(struct udc_mcux_event), CONFIG_UDC_NXP_EVENT_COUNT, sizeof(void *)); -static int udc_mcux_lock(const struct device *dev) +static void udc_mcux_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_mcux_unlock(const struct device *dev) +static void udc_mcux_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static int udc_mcux_control(const struct device *dev, usb_device_control_type_t command, diff --git a/drivers/usb/udc/udc_nrf.c b/drivers/usb/udc/udc_nrf.c index 255895f23d8f..cafe3fbd19fb 100644 --- a/drivers/usb/udc/udc_nrf.c +++ b/drivers/usb/udc/udc_nrf.c @@ -909,14 +909,14 @@ static int udc_nrf_driver_init(const struct device *dev) return 0; } -static int udc_nrf_lock(const struct device *dev) +static void udc_nrf_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_nrf_unlock(const struct device *dev) +static void udc_nrf_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static const struct udc_nrf_config udc_nrf_cfg = { diff --git a/drivers/usb/udc/udc_numaker.c b/drivers/usb/udc/udc_numaker.c index 2fba252371f4..5545132944d2 100644 --- a/drivers/usb/udc/udc_numaker.c +++ b/drivers/usb/udc/udc_numaker.c @@ -1622,14 +1622,14 @@ static int udc_numaker_shutdown(const struct device *dev) return 0; } -static int udc_numaker_lock(const struct device *dev) +static void udc_numaker_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_numaker_unlock(const struct device *dev) +static void udc_numaker_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static int udc_numaker_driver_preinit(const struct device *dev) diff --git a/drivers/usb/udc/udc_renesas_ra.c b/drivers/usb/udc/udc_renesas_ra.c index a8b127a35106..66f3ae308404 100644 --- a/drivers/usb/udc/udc_renesas_ra.c +++ b/drivers/usb/udc/udc_renesas_ra.c @@ -641,14 +641,14 @@ static int udc_renesas_ra_driver_preinit(const struct device *dev) return 0; } -static int udc_renesas_ra_lock(const struct device *dev) +static void udc_renesas_ra_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_renesas_ra_unlock(const struct device *dev) +static void udc_renesas_ra_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static const struct udc_api udc_renesas_ra_api = { diff --git a/drivers/usb/udc/udc_rpi_pico.c b/drivers/usb/udc/udc_rpi_pico.c index f109418baa98..4065f4555b15 100644 --- a/drivers/usb/udc/udc_rpi_pico.c +++ b/drivers/usb/udc/udc_rpi_pico.c @@ -1050,14 +1050,14 @@ static int udc_rpi_pico_driver_preinit(const struct device *dev) return 0; } -static int udc_rpi_pico_lock(const struct device *dev) +static void udc_rpi_pico_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_rpi_pico_unlock(const struct device *dev) +static void udc_rpi_pico_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static const struct udc_api udc_rpi_pico_api = { diff --git a/drivers/usb/udc/udc_skeleton.c b/drivers/usb/udc/udc_skeleton.c index f8870c87f58b..cba2016572ca 100644 --- a/drivers/usb/udc/udc_skeleton.c +++ b/drivers/usb/udc/udc_skeleton.c @@ -323,14 +323,14 @@ static int udc_skeleton_driver_preinit(const struct device *dev) return 0; } -static int udc_skeleton_lock(const struct device *dev) +static void udc_skeleton_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_skeleton_unlock(const struct device *dev) +static void udc_skeleton_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } /* diff --git a/drivers/usb/udc/udc_smartbond.c b/drivers/usb/udc/udc_smartbond.c index 3a214e1b60f2..dd5a2aa782a5 100644 --- a/drivers/usb/udc/udc_smartbond.c +++ b/drivers/usb/udc/udc_smartbond.c @@ -1695,14 +1695,14 @@ static int udc_smartbond_init(const struct device *dev) return 0; } -static int udc_smartbond_lock(const struct device *dev) +static void udc_smartbond_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_smartbond_unlock(const struct device *dev) +static void udc_smartbond_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static const struct udc_api udc_smartbond_api = { diff --git a/drivers/usb/udc/udc_stm32.c b/drivers/usb/udc/udc_stm32.c index 627d22997560..d95c6fb0afd5 100644 --- a/drivers/usb/udc/udc_stm32.c +++ b/drivers/usb/udc/udc_stm32.c @@ -60,14 +60,14 @@ struct udc_stm32_config { uint16_t ep_mps; }; -static int udc_stm32_lock(const struct device *dev) +static void udc_stm32_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_stm32_unlock(const struct device *dev) +static void udc_stm32_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } #define hpcd2data(hpcd) CONTAINER_OF(hpcd, struct udc_stm32_data, pcd); diff --git a/drivers/usb/udc/udc_virtual.c b/drivers/usb/udc/udc_virtual.c index cc21649ad672..c729aa08cd75 100644 --- a/drivers/usb/udc/udc_virtual.c +++ b/drivers/usb/udc/udc_virtual.c @@ -612,14 +612,14 @@ static int udc_vrt_driver_preinit(const struct device *dev) return 0; } -static int udc_vrt_lock(const struct device *dev) +static void udc_vrt_lock(const struct device *dev) { - return udc_lock_internal(dev, K_FOREVER); + udc_lock_internal(dev, K_FOREVER); } -static int udc_vrt_unlock(const struct device *dev) +static void udc_vrt_unlock(const struct device *dev) { - return udc_unlock_internal(dev); + udc_unlock_internal(dev); } static const struct udc_api udc_vrt_api = { diff --git a/include/zephyr/drivers/usb/udc.h b/include/zephyr/drivers/usb/udc.h index cd1e03851332..7e9d66bc638d 100644 --- a/include/zephyr/drivers/usb/udc.h +++ b/include/zephyr/drivers/usb/udc.h @@ -249,8 +249,8 @@ struct udc_api { int (*disable)(const struct device *dev); int (*init)(const struct device *dev); int (*shutdown)(const struct device *dev); - int (*lock)(const struct device *dev); - int (*unlock)(const struct device *dev); + void (*lock)(const struct device *dev); + void (*unlock)(const struct device *dev); }; /** From bf3a346aa746076ade4fceb9ed041565625cfaa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Thu, 6 Feb 2025 15:20:52 +0100 Subject: [PATCH 02/12] [nrf fromtree] drivers: udc_dwc2: Avoid unnecessary context switches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DWC2 thread must acquire UDC mutex before accessing shared resources (peripheral registers and software data structures). Whenever software enqueues a buffer, the caller first obtains mutex, adds the buffer to the list, posts event to wake up DWC2 thread and releases mutex. If DWC2 thread has higher priority than the task currently holding a mutex, there will be two completely unnecessary task switches: DWC2 will switch in, try to obtain mutex, and then the control will be returned to the mutex holder. Avoid the unnecessary task switches by locking scheduler prior to obtaining the mutex and unlocking scheduler after releasing the mutex. Signed-off-by: Tomasz Moń (cherry picked from commit 962a53ef8eb7c6cf32bf8e25b74075eec680b575) --- drivers/usb/udc/udc_dwc2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index 13aa6c1738a1..0b014d13cb0e 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -2318,12 +2318,14 @@ static int dwc2_driver_preinit(const struct device *dev) static void udc_dwc2_lock(const struct device *dev) { + k_sched_lock(); udc_lock_internal(dev, K_FOREVER); } static void udc_dwc2_unlock(const struct device *dev) { udc_unlock_internal(dev); + k_sched_unlock(); } static void dwc2_on_bus_reset(const struct device *dev) From 181b7aee5635136cb0f039d749eadcecf7cd8af4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Thu, 13 Mar 2025 14:28:31 +0100 Subject: [PATCH 03/12] [nrf fromtree] drivers: udc_dwc2: Disable endpoint while hibernated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the endpoint is disabled while the core is hibernated, there are timeouts waiting for interrupts. It is not clear how the stack should behave when class and/or application wants to disable the endpoint while device is suspended. The problem was originally observed when the endpoints were disabled as a result of usbd_disable() call. Avoid the timeouts by modifying the backup values instead of the real registers (which are not accessible when hibernated). Split the 32-bit txf_set variable into two 16-bit variables (txf_set and pending_tx_flush) because maximum number of TxFIFO instances is 16. The txf_set variable is used as-is, while the pending_tx_flush is used to keep track of TxFIFOs that have to be flushed on hibernation exit. Signed-off-by: Tomasz Moń (cherry picked from commit 2be960ad2b33ce1e5af50a2337de80600ab5eea5) --- drivers/usb/udc/udc_dwc2.c | 46 +++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index 0b014d13cb0e..72d2323d579d 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -110,11 +110,12 @@ struct udc_dwc2_data { struct k_event xfer_finished; struct dwc2_reg_backup backup; uint32_t ghwcfg1; - uint32_t txf_set; uint32_t max_xfersize; uint32_t max_pktcnt; uint32_t tx_len[16]; uint32_t rx_siz[16]; + uint16_t txf_set; + uint16_t pending_tx_flush; uint16_t dfifodepth; uint16_t rxfifo_depth; uint16_t max_txfifo_depth[16]; @@ -1484,6 +1485,7 @@ static int dwc2_unset_dedicated_fifo(const struct device *dev, static void udc_dwc2_ep_disable(const struct device *dev, struct udc_ep_config *const cfg, bool stall) { + struct udc_dwc2_data *const priv = udc_get_private(dev); struct usb_dwc2_reg *const base = dwc2_get_base(dev); uint8_t ep_idx = USB_EP_GET_IDX(cfg->addr); mem_addr_t dxepctl_reg; @@ -1493,6 +1495,40 @@ static void udc_dwc2_ep_disable(const struct device *dev, dxepctl_reg = dwc2_get_dxepctl_reg(dev, cfg->addr); dxepctl = sys_read32(dxepctl_reg); + if (priv->hibernated) { + /* Currently USB stack calls this function while hibernated, + * for example if usbd_disable() is called. We cannot access the + * real registers when hibernated, so just modify backup values + * that will be restored on hibernation exit. + */ + if (USB_EP_DIR_IS_OUT(cfg->addr)) { + dxepctl = priv->backup.doepctl[ep_idx]; + + dxepctl &= ~USB_DWC2_DEPCTL_EPENA; + if (stall) { + dxepctl |= USB_DWC2_DEPCTL_STALL; + } else { + dxepctl |= USB_DWC2_DEPCTL_SNAK; + } + + priv->backup.doepctl[ep_idx] = dxepctl; + } else { + dxepctl = priv->backup.diepctl[ep_idx]; + + dxepctl &= ~USB_DWC2_DEPCTL_EPENA; + dxepctl |= USB_DWC2_DEPCTL_SNAK; + if (stall) { + dxepctl |= USB_DWC2_DEPCTL_STALL; + } + + priv->backup.diepctl[ep_idx] = dxepctl; + + priv->pending_tx_flush |= BIT(usb_dwc2_get_depctl_txfnum(dxepctl)); + } + + return; + } + if (!is_iso && (dxepctl & USB_DWC2_DEPCTL_NAKSTS)) { /* Endpoint already sends forced NAKs. STALL if necessary. */ if (stall) { @@ -2162,6 +2198,7 @@ static int udc_dwc2_disable(const struct device *dev) if (priv->hibernated) { dwc2_exit_hibernation(dev, false, true); priv->hibernated = 0; + priv->pending_tx_flush = 0; } sys_clear_bits((mem_addr_t)&base->gahbcfg, USB_DWC2_GAHBCFG_GLBINTRMASK); @@ -2926,6 +2963,13 @@ static void dwc2_handle_hibernation_exit(const struct device *dev, k_event_post(&priv->drv_evt, BIT(DWC2_DRV_EVT_EP_FINISHED)); } } + + while (priv->pending_tx_flush) { + unsigned int fifo = find_lsb_set(priv->pending_tx_flush) - 1; + + priv->pending_tx_flush &= ~BIT(fifo); + dwc2_flush_tx_fifo(dev, fifo); + } } static uint8_t pull_next_ep_from_bitmap(uint32_t *bitmap) From ff8c6aa7a296dd1afe7b9d2afeeab6a41dd5e1c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Thu, 13 Mar 2025 14:31:14 +0100 Subject: [PATCH 04/12] [nrf fromtree] drivers: udc_dwc2: Ensure overlapping USBHS reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wait for PHY clock before triggering START task to ensure overlapped reset for PHY and DWC2 core. Signed-off-by: Tomasz Moń (cherry picked from commit 437983e2c957541f052facc2795e03b0071c738f) --- drivers/usb/udc/udc_dwc2_vendor_quirks.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/usb/udc/udc_dwc2_vendor_quirks.h b/drivers/usb/udc/udc_dwc2_vendor_quirks.h index f45404c2b847..0a1021881399 100644 --- a/drivers/usb/udc/udc_dwc2_vendor_quirks.h +++ b/drivers/usb/udc/udc_dwc2_vendor_quirks.h @@ -199,6 +199,11 @@ static inline int usbhs_enable_core(const struct device *dev) } wrapper->ENABLE = USBHS_ENABLE_PHY_Msk | USBHS_ENABLE_CORE_Msk; + + /* Wait for PHY clock to start */ + k_busy_wait(45); + + /* Release DWC2 reset */ wrapper->TASKS_START = 1UL; /* Wait for clock to start to avoid hang on too early register read */ @@ -218,7 +223,6 @@ static inline int usbhs_disable_core(const struct device *dev) wrapper->INTENCLR = 1UL; wrapper->ENABLE = 0UL; - wrapper->TASKS_START = 1UL; return 0; } From c135c2f7521951476183fc3313905ff47fc3031b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Mon, 10 Mar 2025 10:09:39 +0100 Subject: [PATCH 05/12] [nrf fromtree] drivers: udc_dwc2: Reset core on shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OUT endpoint 0 cannot be disabled and therefore the only way to forcibly reclaim the buffer is to reset the core. The reset does not finish if PHY clock is not running, but just triggering the reset seems to be enough to be able to reclaim the buffer. Signed-off-by: Tomasz Moń (cherry picked from commit 2395452656099afee86d642c063b582df6b0d6c9) --- drivers/usb/udc/udc_dwc2.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index 72d2323d579d..bc5d9881dbe9 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -1661,15 +1661,6 @@ static int udc_dwc2_ep_deactivate(const struct device *dev, sys_write32(dxepctl, dxepctl_reg); dwc2_set_epint(dev, cfg, false); - if (cfg->addr == USB_CONTROL_EP_OUT) { - struct net_buf *buf = udc_buf_get_all(dev, cfg->addr); - - /* Release the buffer allocated in dwc2_ctrl_feed_dout() */ - if (buf) { - net_buf_unref(buf); - } - } - return 0; } @@ -1875,10 +1866,16 @@ static int dwc2_core_soft_reset(const struct device *dev) } k_busy_wait(1); + + if (dwc2_quirk_is_phy_clk_off(dev)) { + /* Software reset won't finish without PHY clock */ + return -EIO; + } } while (sys_read32(grstctl_reg) & USB_DWC2_GRSTCTL_CSFTRST && !(sys_read32(grstctl_reg) & USB_DWC2_GRSTCTL_CSFTRSTDONE)); - sys_clear_bits(grstctl_reg, USB_DWC2_GRSTCTL_CSFTRST | USB_DWC2_GRSTCTL_CSFTRSTDONE); + /* CSFTRSTDONE is W1C so the write must have the bit set to clear it */ + sys_clear_bits(grstctl_reg, USB_DWC2_GRSTCTL_CSFTRST); return 0; } @@ -2177,10 +2174,9 @@ static int udc_dwc2_disable(const struct device *dev) struct udc_dwc2_data *const priv = udc_get_private(dev); struct usb_dwc2_reg *const base = dwc2_get_base(dev); mem_addr_t dctl_reg = (mem_addr_t)&base->dctl; + struct net_buf *buf; int err; - /* Enable soft disconnect */ - sys_set_bits(dctl_reg, USB_DWC2_DCTL_SFTDISCON); LOG_DBG("Disable device %p", dev); if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { @@ -2203,6 +2199,22 @@ static int udc_dwc2_disable(const struct device *dev) sys_clear_bits((mem_addr_t)&base->gahbcfg, USB_DWC2_GAHBCFG_GLBINTRMASK); + /* Enable soft disconnect */ + sys_set_bits(dctl_reg, USB_DWC2_DCTL_SFTDISCON); + + /* OUT endpoint 0 cannot be disabled by software. The buffer allocated + * in dwc2_ctrl_feed_dout() can only be freed after core reset if the + * core was in Buffer DMA mode. + * + * Soft Reset does timeout if PHY clock is not running. However, just + * triggering Soft Reset seems to be enough on shutdown clean up. + */ + dwc2_core_soft_reset(dev); + buf = udc_buf_get_all(dev, USB_CONTROL_EP_OUT); + if (buf) { + net_buf_unref(buf); + } + err = dwc2_quirk_disable(dev); if (err) { LOG_ERR("Quirk disable failed %d", err); From 4ed641c8cc3ddd1fe087022a6b12bfa83e6f6900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Thu, 20 Mar 2025 11:00:12 +0100 Subject: [PATCH 06/12] [nrf fromtree] drivers: udc_dwc2: Optimize endpoint event clear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is no point in calling k_event_test() to determine what events are posted and then passing that value to k_event_clear(). Simply pass UINT32_MAX to k_event_clear() and use the return value to slightly reduce overhead. Signed-off-by: Tomasz Moń (cherry picked from commit 2aa26ad2615297c3c69649cfd89b345838413c75) --- drivers/usb/udc/udc_dwc2.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index bc5d9881dbe9..8860f69ee367 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -3026,8 +3026,7 @@ static ALWAYS_INLINE void dwc2_thread_handler(void *const arg) if (!priv->hibernated) { LOG_DBG("New transfer(s) in the queue"); - eps = k_event_test(&priv->xfer_new, UINT32_MAX); - k_event_clear(&priv->xfer_new, eps); + eps = k_event_clear(&priv->xfer_new, UINT32_MAX); } else { /* Events will be handled after hibernation exit */ eps = 0; @@ -3049,8 +3048,7 @@ static ALWAYS_INLINE void dwc2_thread_handler(void *const arg) k_event_clear(&priv->drv_evt, BIT(DWC2_DRV_EVT_EP_FINISHED)); if (!priv->hibernated) { - eps = k_event_test(&priv->xfer_finished, UINT32_MAX); - k_event_clear(&priv->xfer_finished, eps); + eps = k_event_clear(&priv->xfer_finished, UINT32_MAX); } else { /* Events will be handled after hibernation exit */ eps = 0; From e0dca6c694e6293627b602977e1eab7f71f9138c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Mon, 10 Mar 2025 14:59:22 +0100 Subject: [PATCH 07/12] [nrf fromtree] drivers: udc_dwc2: Set control endpoint CNAK only when necessary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SETUP data is unconditionally ACKed by the controller. Other DATA packets sent to OUT control endpoint 0 (i.e. OUT Data Stage packets and OUT Status Stage packet) are ACKed by the device only if the endpoint was enabled with CNAK bit set. In Buffer DMA mode controller will lock up in following scenario: * OUT EP0 is not enabled, e.g. OUT Status Stage has finished * Host starts Control Write transfer, i.e. sends SETUP DATA0 and device ACKs (regardless if endpoint is enabled or not) * host sends OUT Data Stage (OUT DATA1) - software enables endpoint to be able to receive next SETUP data while host is transmitting the OUT token. If CNAK bit is set alongside the EPENA bit, the device will ACK the OUT Data Stage. If CNAK bit is not set, the device will NAK the OUT Data Stage. When the lockup occurs, from host perspective the OUT Data Stage packet was successfully transmitted. This can result in host starting IN Status Stage if there was only one OUT Data Stage packet. This in turn results in device never getting the DOEPTINT0 SetUp interrupt. Besides just not getting the SetUp interrupt, any subsequent control transfer won't be noticed by device at all. The lockup was first observed while stress testing. The host was issuing endless sequence of Control Write, Control Read, Control Write, Control Read, ... commands. When the controller did lock up in Buffer DMA mode, from host perspective the device was timing out all control transfers. Avoid the Buffer DMA lockup by setting CNAK bit only when necessary. Signed-off-by: Tomasz Moń (cherry picked from commit 6e3f8d4967e1e55257ba82ffd889604746d8facf) --- drivers/usb/udc/udc_dwc2.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index 8860f69ee367..feccd3d8662b 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -669,7 +669,32 @@ static void dwc2_prep_rx(const struct device *dev, struct net_buf *buf, /* Clear NAK and set endpoint enable */ doepctl = sys_read32(doepctl_reg); - doepctl |= USB_DWC2_DEPCTL_EPENA | USB_DWC2_DEPCTL_CNAK; + doepctl |= USB_DWC2_DEPCTL_EPENA; + if (cfg->addr == USB_CONTROL_EP_OUT) { + struct udc_data *data = dev->data; + + /* During OUT Data Stage every packet has to have CNAK set. + * In Buffer DMA mode the OUT endpoint is armed during IN Data + * Stage to accept either Status stage or new SETUP token. The + * Status stage (premature or not) can only be received if CNAK + * was set. In Completer mode the OUT endpoint armed during OUT + * Status stage needs CNAK. + * + * Setting CNAK in other cases opens up possibility for Buffer + * DMA controller to lock up completely if the endpoint is + * enabled (to receive SETUP data) when the host is transmitting + * subsequent control transfer OUT Data Stage packet (SETUP DATA + * is unconditionally ACKed regardless of software state). + */ + if (data->stage == CTRL_PIPE_STAGE_DATA_OUT || + data->stage == CTRL_PIPE_STAGE_DATA_IN || + data->stage == CTRL_PIPE_STAGE_STATUS_OUT) { + doepctl |= USB_DWC2_DEPCTL_CNAK; + } + } else { + /* Non-control endpoint, set CNAK for all transfers */ + doepctl |= USB_DWC2_DEPCTL_CNAK; + } if (dwc2_ep_is_iso(cfg)) { xfersize = USB_MPS_TO_TPL(cfg->mps); From 33005f724d7d1230bb1e2638a125e3047e34cb62 Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Wed, 20 Nov 2024 23:56:10 +0100 Subject: [PATCH 08/12] [nrf fromtree] usb: device_next: fix cdc_acm_send_notification() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix "warning: 'cdc_acm_send_notification' defined but not used" when Kconfig option UART_USE_RUNTIME_CONFIGURE is not used and properly handle enqueue error. Signed-off-by: Johann Fischer (cherry picked from commit 7b344567be8dbb10d53e8177b7f28c97b6fee1dd) Signed-off-by: Tomasz Moń --- subsys/usb/device_next/class/usbd_cdc_acm.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/subsys/usb/device_next/class/usbd_cdc_acm.c b/subsys/usb/device_next/class/usbd_cdc_acm.c index 3327db43d52f..514883864c9b 100644 --- a/subsys/usb/device_next/class/usbd_cdc_acm.c +++ b/subsys/usb/device_next/class/usbd_cdc_acm.c @@ -493,8 +493,8 @@ static int usbd_cdc_acm_init(struct usbd_class_data *const c_data) return 0; } -static int cdc_acm_send_notification(const struct device *dev, - const uint16_t serial_state) +static inline int cdc_acm_send_notification(const struct device *dev, + const uint16_t serial_state) { struct cdc_acm_notification notification = { .bmRequestType = 0xA1, @@ -528,7 +528,11 @@ static int cdc_acm_send_notification(const struct device *dev, net_buf_add_mem(buf, ¬ification, sizeof(struct cdc_acm_notification)); ret = usbd_ep_enqueue(c_data, buf); - /* FIXME: support for sync transfers */ + if (ret) { + net_buf_unref(buf); + return ret; + } + k_sem_take(&data->notif_sem, K_FOREVER); return ret; From 3fa5544a53f8ec1d3ceac335080dbe069342e158 Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Mon, 18 Nov 2024 16:58:22 +0100 Subject: [PATCH 09/12] [nrf fromtree] usb: device_next: allow CDC ACM UART to be initialized and enabled at boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are some boards and samples in the tree that use the CDC ACM UART as the default serial backend, just like a real UART controller would. The new device stack requires more detailed configuration than the old one. In order to use the CDC ACM UART as the serial backend with the new device stack in the same way as with the legacy stack, we need to provide a solution to initialise and enable the CDC ACM UART at boot time. We cannot use snippets as they do not support Kconfig files or source code. Shields would be an option, but they cannot be used for virtual devices such as the CDC ACM UART. The remaining solution is to put the code and Kconfig file for it in the subsys directory. Allow CDC ACM UART instance and USB device stack to be initialized and enabled at boot time and to use it as serial backend for logging or shell. Signed-off-by: Johann Fischer (cherry picked from commit 7ffa6203e1154cbd8d407ee53516914ad504b7eb) Signed-off-by: Tomasz Moń --- subsys/usb/device_next/CMakeLists.txt | 2 + subsys/usb/device_next/Kconfig | 1 + subsys/usb/device_next/app/CMakeLists.txt | 4 + .../device_next/app/Kconfig.cdc_acm_serial | 57 ++++++++ subsys/usb/device_next/app/cdc_acm_serial.c | 131 ++++++++++++++++++ subsys/usb/device_next/class/Kconfig.cdc_acm | 1 + 6 files changed, 196 insertions(+) create mode 100644 subsys/usb/device_next/app/CMakeLists.txt create mode 100644 subsys/usb/device_next/app/Kconfig.cdc_acm_serial create mode 100644 subsys/usb/device_next/app/cdc_acm_serial.c diff --git a/subsys/usb/device_next/CMakeLists.txt b/subsys/usb/device_next/CMakeLists.txt index 7ef32c4dcdb7..ffd60042fa28 100644 --- a/subsys/usb/device_next/CMakeLists.txt +++ b/subsys/usb/device_next/CMakeLists.txt @@ -17,6 +17,8 @@ zephyr_library_sources( usbd_msg.c ) +add_subdirectory(app) + zephyr_library_sources_ifdef( CONFIG_USBD_SHELL usbd_shell.c diff --git a/subsys/usb/device_next/Kconfig b/subsys/usb/device_next/Kconfig index a38823db71ad..52ce9d990858 100644 --- a/subsys/usb/device_next/Kconfig +++ b/subsys/usb/device_next/Kconfig @@ -56,5 +56,6 @@ config USBD_MSG_WORK_DELAY yet ready to publish the message. The delay unit is milliseconds. rsource "class/Kconfig" +rsource "app/Kconfig.cdc_acm_serial" endif # USB_DEVICE_STACK_NEXT diff --git a/subsys/usb/device_next/app/CMakeLists.txt b/subsys/usb/device_next/app/CMakeLists.txt new file mode 100644 index 000000000000..116d422def3e --- /dev/null +++ b/subsys/usb/device_next/app/CMakeLists.txt @@ -0,0 +1,4 @@ +# Copyright (c) 2024 Nordic Semiconductor ASA +# SPDX-License-Identifier: Apache-2.0 + +zephyr_sources_ifdef(CONFIG_CDC_ACM_SERIAL_INITIALIZE_AT_BOOT cdc_acm_serial.c) diff --git a/subsys/usb/device_next/app/Kconfig.cdc_acm_serial b/subsys/usb/device_next/app/Kconfig.cdc_acm_serial new file mode 100644 index 000000000000..dba212ba9baf --- /dev/null +++ b/subsys/usb/device_next/app/Kconfig.cdc_acm_serial @@ -0,0 +1,57 @@ +# Copyright (c) 2024 Nordic Semiconductor ASA +# SPDX-License-Identifier: Apache-2.0 + +# This file contains Kconfig options and defaults for configuring the USB +# device stack and CDC ACM instance to be used as the default serial backend +# for logging or shell. + + +menuconfig CDC_ACM_SERIAL_INITIALIZE_AT_BOOT + bool "Initialize USB device and CDC ACM UART at boot" + depends on USBD_CDC_ACM_CLASS + help + This is intended for use with cdc-acm-snippet or as a default serial + backend only in applications where no other USB features are + required, configured, and enabled. + +if CDC_ACM_SERIAL_INITIALIZE_AT_BOOT + +config CDC_ACM_SERIAL_MANUFACTURER_STRING + string "USB device manufacturer string descriptor" + default "Zephyr Project" + help + USB device manufacturer string descriptor. + +config CDC_ACM_SERIAL_PRODUCT_STRING + string "USB device product string descriptor" + default "CDC ACM serial backend" + help + USB device product string descriptor. + +config CDC_ACM_SERIAL_VID + hex "USB device Vendor ID" + default 0x2fe3 + help + You must use your own VID for samples and applications outside of + Zephyr Project. + +config CDC_ACM_SERIAL_PID + hex "USB device Product ID" + default 0x0004 + help + You must use your own PID for samples and applications outside of + Zephyr Project. + +config CDC_ACM_SERIAL_SELF_POWERED + bool "USB device Self-powered attribute" + help + Set the Self-powered attribute in the configuration. + +config CDC_ACM_SERIAL_MAX_POWER + int "USB device bMaxPower value" + default 125 + range 0 250 + help + bMaxPower value in the configuration in 2 mA units. + +endif #CDC_ACM_SERIAL_INITIALIZE_AT_BOOT diff --git a/subsys/usb/device_next/app/cdc_acm_serial.c b/subsys/usb/device_next/app/cdc_acm_serial.c new file mode 100644 index 000000000000..9402722c6b0d --- /dev/null +++ b/subsys/usb/device_next/app/cdc_acm_serial.c @@ -0,0 +1,131 @@ +/* + * Copyright (c) 2024 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include + +#include +#include +#include +#include + +#include +LOG_MODULE_REGISTER(cdc_acm_serial, LOG_LEVEL_DBG); + +/* + * This is intended for use with cdc-acm-snippet or as a default serial backend + * only in applications where no other USB features are required, configured, + * and enabled. This code only registers the first CDC-ACM instance. + */ + +USBD_DEVICE_DEFINE(cdc_acm_serial, + DEVICE_DT_GET(DT_NODELABEL(zephyr_udc0)), + CONFIG_CDC_ACM_SERIAL_VID, CONFIG_CDC_ACM_SERIAL_PID); + +USBD_DESC_LANG_DEFINE(cdc_acm_serial_lang); +USBD_DESC_MANUFACTURER_DEFINE(cdc_acm_serial_mfr, CONFIG_CDC_ACM_SERIAL_MANUFACTURER_STRING); +USBD_DESC_PRODUCT_DEFINE(cdc_acm_serial_product, CONFIG_CDC_ACM_SERIAL_PRODUCT_STRING); +USBD_DESC_SERIAL_NUMBER_DEFINE(cdc_acm_serial_sn); + +USBD_DESC_CONFIG_DEFINE(fs_cfg_desc, "FS Configuration"); +USBD_DESC_CONFIG_DEFINE(hs_cfg_desc, "HS Configuration"); + +static const uint8_t attributes = IS_ENABLED(CONFIG_CDC_ACM_SERIAL_SELF_POWERED) ? + USB_SCD_SELF_POWERED : 0; + +USBD_CONFIGURATION_DEFINE(cdc_acm_serial_fs_config, + attributes, + CONFIG_CDC_ACM_SERIAL_MAX_POWER, &fs_cfg_desc); + +USBD_CONFIGURATION_DEFINE(cdc_acm_serial_hs_config, + attributes, + CONFIG_CDC_ACM_SERIAL_MAX_POWER, &hs_cfg_desc); + + +static int register_cdc_acm_0(struct usbd_context *const uds_ctx, + const enum usbd_speed speed) +{ + struct usbd_config_node *cfg_nd; + int err; + + if (speed == USBD_SPEED_HS) { + cfg_nd = &cdc_acm_serial_hs_config; + } else { + cfg_nd = &cdc_acm_serial_fs_config; + } + + err = usbd_add_configuration(uds_ctx, speed, cfg_nd); + if (err) { + LOG_ERR("Failed to add configuration"); + return err; + } + + err = usbd_register_class(&cdc_acm_serial, "cdc_acm_0", speed, 1); + if (err) { + LOG_ERR("Failed to register classes"); + return err; + } + + return usbd_device_set_code_triple(uds_ctx, speed, + USB_BCC_MISCELLANEOUS, 0x02, 0x01); +} + + +static int cdc_acm_serial_init_device(void) +{ + int err; + + err = usbd_add_descriptor(&cdc_acm_serial, &cdc_acm_serial_lang); + if (err) { + LOG_ERR("Failed to initialize language descriptor (%d)", err); + return err; + } + + err = usbd_add_descriptor(&cdc_acm_serial, &cdc_acm_serial_mfr); + if (err) { + LOG_ERR("Failed to initialize manufacturer descriptor (%d)", err); + return err; + } + + err = usbd_add_descriptor(&cdc_acm_serial, &cdc_acm_serial_product); + if (err) { + LOG_ERR("Failed to initialize product descriptor (%d)", err); + return err; + } + + err = usbd_add_descriptor(&cdc_acm_serial, &cdc_acm_serial_sn); + if (err) { + LOG_ERR("Failed to initialize SN descriptor (%d)", err); + return err; + } + + if (usbd_caps_speed(&cdc_acm_serial) == USBD_SPEED_HS) { + err = register_cdc_acm_0(&cdc_acm_serial, USBD_SPEED_HS); + if (err) { + return err; + } + } + + err = register_cdc_acm_0(&cdc_acm_serial, USBD_SPEED_FS); + if (err) { + return err; + } + + err = usbd_init(&cdc_acm_serial); + if (err) { + LOG_ERR("Failed to initialize device support"); + return err; + } + + err = usbd_enable(&cdc_acm_serial); + if (err) { + LOG_ERR("Failed to enable device support"); + return err; + } + + return 0; +} + +SYS_INIT(cdc_acm_serial_init_device, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY); diff --git a/subsys/usb/device_next/class/Kconfig.cdc_acm b/subsys/usb/device_next/class/Kconfig.cdc_acm index 997ded105f4e..3d282560b1da 100644 --- a/subsys/usb/device_next/class/Kconfig.cdc_acm +++ b/subsys/usb/device_next/class/Kconfig.cdc_acm @@ -10,6 +10,7 @@ config USBD_CDC_ACM_CLASS select SERIAL_SUPPORT_INTERRUPT select RING_BUFFER select UART_INTERRUPT_DRIVEN + default y help USB device CDC ACM class implementation. From d38da30ac97a4c33b8d359ccaa722989247a5fbc Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Thu, 6 Feb 2025 12:16:23 +0100 Subject: [PATCH 10/12] [nrf fromtree] samples: usb: add console sample for the new USB device stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add sample to shows how to use the CDC ACM UART provided by the new experimental USB device stack as a serial backend for the console. Signed-off-by: Johann Fischer (cherry picked from commit be5e666eb3b8dcfe1eb5d4865e6946d906c44087) Signed-off-by: Tomasz Moń --- .../subsys/usb/console-next/CMakeLists.txt | 8 ++++ samples/subsys/usb/console-next/README.rst | 39 +++++++++++++++++++ samples/subsys/usb/console-next/app.overlay | 17 ++++++++ samples/subsys/usb/console-next/prj.conf | 10 +++++ samples/subsys/usb/console-next/sample.yaml | 10 +++++ samples/subsys/usb/console-next/src/main.c | 30 ++++++++++++++ 6 files changed, 114 insertions(+) create mode 100644 samples/subsys/usb/console-next/CMakeLists.txt create mode 100644 samples/subsys/usb/console-next/README.rst create mode 100644 samples/subsys/usb/console-next/app.overlay create mode 100644 samples/subsys/usb/console-next/prj.conf create mode 100644 samples/subsys/usb/console-next/sample.yaml create mode 100644 samples/subsys/usb/console-next/src/main.c diff --git a/samples/subsys/usb/console-next/CMakeLists.txt b/samples/subsys/usb/console-next/CMakeLists.txt new file mode 100644 index 000000000000..d8de1e746aff --- /dev/null +++ b/samples/subsys/usb/console-next/CMakeLists.txt @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(console-next) + +FILE(GLOB app_sources src/*.c) +target_sources(app PRIVATE ${app_sources}) diff --git a/samples/subsys/usb/console-next/README.rst b/samples/subsys/usb/console-next/README.rst new file mode 100644 index 000000000000..1ca2063640b6 --- /dev/null +++ b/samples/subsys/usb/console-next/README.rst @@ -0,0 +1,39 @@ +.. zephyr:code-sample:: usbd-cdc-acm-console + :name: Console over USB CDC ACM + :relevant-api: usbd_api uart_interface + + Output "Hello World!" to the console over USB CDC ACM. + +Overview +******** + +This example application shows how to use the CDC ACM UART provided by the new +experimental USB device stack as a serial backend for the console. + +Requirements +************ + +This project requires an experimental USB device driver (UDC API). + +Building and Running +******************** + +This sample can be built for multiple boards, in this example we will build it +for the reel_board board: + +.. zephyr-app-commands:: + :zephyr-app: samples/subsys/usb/console-next + :board: reel_board + :goals: flash + :compact: + +Plug the board into a host device, for sample, a PC running Linux OS. +The board will be detected as a CDC ACM serial device. To see the console output +from the sample, use a command similar to :command:`minicom -D /dev/ttyACM1`. + +.. code-block:: console + + Hello World! arm + Hello World! arm + Hello World! arm + Hello World! arm diff --git a/samples/subsys/usb/console-next/app.overlay b/samples/subsys/usb/console-next/app.overlay new file mode 100644 index 000000000000..47b52ce7d6d0 --- /dev/null +++ b/samples/subsys/usb/console-next/app.overlay @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2021 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/ { + chosen { + zephyr,console = &cdc_acm_uart0; + }; +}; + +&zephyr_udc0 { + cdc_acm_uart0: cdc_acm_uart0 { + compatible = "zephyr,cdc-acm-uart"; + }; +}; diff --git a/samples/subsys/usb/console-next/prj.conf b/samples/subsys/usb/console-next/prj.conf new file mode 100644 index 000000000000..fe0b2be7270c --- /dev/null +++ b/samples/subsys/usb/console-next/prj.conf @@ -0,0 +1,10 @@ +CONFIG_SERIAL=y +CONFIG_CONSOLE=y +CONFIG_UART_CONSOLE=y +CONFIG_STDOUT_CONSOLE=y +CONFIG_UART_LINE_CTRL=y + +CONFIG_USB_DEVICE_STACK_NEXT=y +CONFIG_CDC_ACM_SERIAL_INITIALIZE_AT_BOOT=y +CONFIG_CDC_ACM_SERIAL_PID=0x0004 +CONFIG_CDC_ACM_SERIAL_PRODUCT_STRING="USBD console sample" diff --git a/samples/subsys/usb/console-next/sample.yaml b/samples/subsys/usb/console-next/sample.yaml new file mode 100644 index 000000000000..f12969623c2b --- /dev/null +++ b/samples/subsys/usb/console-next/sample.yaml @@ -0,0 +1,10 @@ +sample: + name: Console over CDC ACM serial +tests: + sample.usbd.console: + depends_on: + - usbd + tags: usb + harness: console + harness_config: + fixture: fixture_usb_cdc diff --git a/samples/subsys/usb/console-next/src/main.c b/samples/subsys/usb/console-next/src/main.c new file mode 100644 index 000000000000..7bb050598186 --- /dev/null +++ b/samples/subsys/usb/console-next/src/main.c @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2016 Intel Corporation. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include + +BUILD_ASSERT(DT_NODE_HAS_COMPAT(DT_CHOSEN(zephyr_console), zephyr_cdc_acm_uart), + "Console device is not ACM CDC UART device"); + +int main(void) +{ + const struct device *const dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_console)); + uint32_t dtr = 0; + + /* Poll if the DTR flag was set */ + while (!dtr) { + uart_line_ctrl_get(dev, UART_LINE_CTRL_DTR, &dtr); + /* Give CPU resources to low priority threads. */ + k_sleep(K_MSEC(100)); + } + + while (1) { + printk("Hello World! %s\n", CONFIG_ARCH); + k_sleep(K_SECONDS(1)); + } +} From e82c56b4d8a38108d8270b71e19791e8d3c904d8 Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Thu, 6 Feb 2025 12:23:28 +0100 Subject: [PATCH 11/12] [nrf fromtree] samples: usb: clean up legacy USB CDC ACM console sample MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the configuration for the new USB device stack, as there is now a separate sample that demonstrates how to use it with Kconfig option CONFIG_CDC_ACM_SERIAL_INITIALIZE_AT_BOOT. Signed-off-by: Johann Fischer (cherry picked from commit 591413185276f28f02af241b1fe6f9bd74d161d1) Signed-off-by: Tomasz Moń --- samples/subsys/usb/console/Kconfig | 9 ------ samples/subsys/usb/console/src/main.c | 30 ------------------- samples/subsys/usb/console/usbd_next_prj.conf | 13 -------- 3 files changed, 52 deletions(-) delete mode 100644 samples/subsys/usb/console/Kconfig delete mode 100644 samples/subsys/usb/console/usbd_next_prj.conf diff --git a/samples/subsys/usb/console/Kconfig b/samples/subsys/usb/console/Kconfig deleted file mode 100644 index 96c545589480..000000000000 --- a/samples/subsys/usb/console/Kconfig +++ /dev/null @@ -1,9 +0,0 @@ -# Copyright (c) 2023 Nordic Semiconductor ASA -# SPDX-License-Identifier: Apache-2.0 - -# Source common USB sample options used to initialize new experimental USB -# device stack. The scope of these options is limited to USB samples in project -# tree, you cannot use them in your own application. -source "samples/subsys/usb/common/Kconfig.sample_usbd" - -source "Kconfig.zephyr" diff --git a/samples/subsys/usb/console/src/main.c b/samples/subsys/usb/console/src/main.c index bf2230687e1c..59296591ac5f 100644 --- a/samples/subsys/usb/console/src/main.c +++ b/samples/subsys/usb/console/src/main.c @@ -4,52 +4,22 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include - #include #include #include -#include #include BUILD_ASSERT(DT_NODE_HAS_COMPAT(DT_CHOSEN(zephyr_console), zephyr_cdc_acm_uart), "Console device is not ACM CDC UART device"); -#if defined(CONFIG_USB_DEVICE_STACK_NEXT) -static struct usbd_context *sample_usbd; - -static int enable_usb_device_next(void) -{ - int err; - - sample_usbd = sample_usbd_init_device(NULL); - if (sample_usbd == NULL) { - return -ENODEV; - } - - err = usbd_enable(sample_usbd); - if (err) { - return err; - } - - return 0; -} -#endif /* defined(CONFIG_USB_DEVICE_STACK_NEXT) */ - int main(void) { const struct device *const dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_console)); uint32_t dtr = 0; -#if defined(CONFIG_USB_DEVICE_STACK_NEXT) - if (enable_usb_device_next()) { - return 0; - } -#else if (usb_enable(NULL)) { return 0; } -#endif /* Poll if the DTR flag was set */ while (!dtr) { diff --git a/samples/subsys/usb/console/usbd_next_prj.conf b/samples/subsys/usb/console/usbd_next_prj.conf deleted file mode 100644 index 841bffbf012b..000000000000 --- a/samples/subsys/usb/console/usbd_next_prj.conf +++ /dev/null @@ -1,13 +0,0 @@ -CONFIG_USB_DEVICE_STACK_NEXT=y - -CONFIG_STDOUT_CONSOLE=y -CONFIG_SERIAL=y -CONFIG_UART_LINE_CTRL=y -CONFIG_USBD_CDC_ACM_CLASS=y - -CONFIG_LOG=y -CONFIG_USBD_LOG_LEVEL_WRN=y -CONFIG_UDC_DRIVER_LOG_LEVEL_WRN=y - -CONFIG_SAMPLE_USBD_PID=0x0004 -CONFIG_SAMPLE_USBD_PRODUCT="USBD console sample" From 2319abdacb3ee9da7b53147cba5311ba61cc5d28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Fri, 28 Feb 2025 08:34:07 +0100 Subject: [PATCH 12/12] [nrf fromtree] usb: device_next: Do not add serial number without HWINFO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For many devices iSerialNumber is not required. The only device class currently supported in Zephyr that requires iSerialNumber is MSC. If the serial number is not available then iSerialNumber must be 0. Failure to read the string descriptor for non-zero iSerialNumber fails USB Command Verifier Chapter 9 tests. When USBD_DESC_SERIAL_NUMBER_DEFINE() was used without CONFIG_HWINFO then it did lead to runtime failure when requesting the string (thus failing certification). Fail USBD_DESC_SERIAL_NUMBER_DEFINE() at build-time if CONFIG_HWINFO is not set to prevent the surprise at runtime. Only define serial number descriptors if CONFIG_HWINFO is enabled. Signed-off-by: Tomasz Moń (cherry picked from commit 75109707e3e43034d49b647f7ae1afff646cc3c2) --- include/zephyr/usb/usbd.h | 1 + samples/subsys/usb/common/sample_usbd_init.c | 7 +++++-- subsys/usb/device_next/app/cdc_acm_serial.c | 6 ++++-- subsys/usb/device_next/usbd_shell.c | 6 ++++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/include/zephyr/usb/usbd.h b/include/zephyr/usb/usbd.h index b588d7087d28..7d125dfdf232 100644 --- a/include/zephyr/usb/usbd.h +++ b/include/zephyr/usb/usbd.h @@ -631,6 +631,7 @@ static inline void *usbd_class_get_private(const struct usbd_class_data *const c * @param d_name String descriptor node identifier. */ #define USBD_DESC_SERIAL_NUMBER_DEFINE(d_name) \ + BUILD_ASSERT(IS_ENABLED(CONFIG_HWINFO), "HWINFO not enabled"); \ static struct usbd_desc_node d_name = { \ .str = { \ .utype = USBD_DUT_STRING_SERIAL_NUMBER, \ diff --git a/samples/subsys/usb/common/sample_usbd_init.c b/samples/subsys/usb/common/sample_usbd_init.c index fb5b8e0dc324..25e3a4555465 100644 --- a/samples/subsys/usb/common/sample_usbd_init.c +++ b/samples/subsys/usb/common/sample_usbd_init.c @@ -30,7 +30,8 @@ USBD_DEVICE_DEFINE(sample_usbd, USBD_DESC_LANG_DEFINE(sample_lang); USBD_DESC_MANUFACTURER_DEFINE(sample_mfr, CONFIG_SAMPLE_USBD_MANUFACTURER); USBD_DESC_PRODUCT_DEFINE(sample_product, CONFIG_SAMPLE_USBD_PRODUCT); -USBD_DESC_SERIAL_NUMBER_DEFINE(sample_sn); +IF_ENABLED(CONFIG_HWINFO, (USBD_DESC_SERIAL_NUMBER_DEFINE(sample_sn))); + /* doc string instantiation end */ USBD_DESC_CONFIG_DEFINE(fs_cfg_desc, "FS Configuration"); @@ -109,7 +110,9 @@ struct usbd_context *sample_usbd_setup_device(usbd_msg_cb_t msg_cb) return NULL; } - err = usbd_add_descriptor(&sample_usbd, &sample_sn); + IF_ENABLED(CONFIG_HWINFO, ( + err = usbd_add_descriptor(&sample_usbd, &sample_sn); + )) if (err) { LOG_ERR("Failed to initialize SN descriptor (%d)", err); return NULL; diff --git a/subsys/usb/device_next/app/cdc_acm_serial.c b/subsys/usb/device_next/app/cdc_acm_serial.c index 9402722c6b0d..6f8e64b3c8cf 100644 --- a/subsys/usb/device_next/app/cdc_acm_serial.c +++ b/subsys/usb/device_next/app/cdc_acm_serial.c @@ -27,7 +27,7 @@ USBD_DEVICE_DEFINE(cdc_acm_serial, USBD_DESC_LANG_DEFINE(cdc_acm_serial_lang); USBD_DESC_MANUFACTURER_DEFINE(cdc_acm_serial_mfr, CONFIG_CDC_ACM_SERIAL_MANUFACTURER_STRING); USBD_DESC_PRODUCT_DEFINE(cdc_acm_serial_product, CONFIG_CDC_ACM_SERIAL_PRODUCT_STRING); -USBD_DESC_SERIAL_NUMBER_DEFINE(cdc_acm_serial_sn); +IF_ENABLED(CONFIG_HWINFO, (USBD_DESC_SERIAL_NUMBER_DEFINE(cdc_acm_serial_sn))); USBD_DESC_CONFIG_DEFINE(fs_cfg_desc, "FS Configuration"); USBD_DESC_CONFIG_DEFINE(hs_cfg_desc, "HS Configuration"); @@ -95,7 +95,9 @@ static int cdc_acm_serial_init_device(void) return err; } - err = usbd_add_descriptor(&cdc_acm_serial, &cdc_acm_serial_sn); + IF_ENABLED(CONFIG_HWINFO, ( + err = usbd_add_descriptor(&cdc_acm_serial, &cdc_acm_serial_sn); + )) if (err) { LOG_ERR("Failed to initialize SN descriptor (%d)", err); return err; diff --git a/subsys/usb/device_next/usbd_shell.c b/subsys/usb/device_next/usbd_shell.c index 758094c7407f..6b321204a24a 100644 --- a/subsys/usb/device_next/usbd_shell.c +++ b/subsys/usb/device_next/usbd_shell.c @@ -43,7 +43,7 @@ static struct usbd_shell_speed { USBD_DESC_LANG_DEFINE(lang); USBD_DESC_MANUFACTURER_DEFINE(mfr, "ZEPHYR"); USBD_DESC_PRODUCT_DEFINE(product, "Zephyr USBD foobaz"); -USBD_DESC_SERIAL_NUMBER_DEFINE(sn); +IF_ENABLED(CONFIG_HWINFO, (USBD_DESC_SERIAL_NUMBER_DEFINE(sn))); /* Default device descriptors and context used in the shell. */ USBD_DEVICE_DEFINE(sh_uds_ctx, DEVICE_DT_GET(DT_NODELABEL(zephyr_udc0)), @@ -117,7 +117,9 @@ static int cmd_usbd_default_strings(const struct shell *sh, err = usbd_add_descriptor(my_uds_ctx, &lang); err |= usbd_add_descriptor(my_uds_ctx, &mfr); err |= usbd_add_descriptor(my_uds_ctx, &product); - err |= usbd_add_descriptor(my_uds_ctx, &sn); + IF_ENABLED(CONFIG_HWINFO, ( + err |= usbd_add_descriptor(my_uds_ctx, &sn); + )) if (err) { shell_error(sh, "dev: Failed to add default string descriptors, %d", err);