From 4c308597a43968ee0a954903b02e464875be0c3a Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Mon, 7 Jul 2025 13:59:54 +0200 Subject: [PATCH 1/3] drivers: udc: add helper to enable/disable control endpoint Add helper function to enable/disable control endpoint. Signed-off-by: Johann Fischer --- drivers/usb/udc/udc_ambiq.c | 14 +++------ drivers/usb/udc/udc_common.h | 48 +++++++++++++++++++++++++++++++ drivers/usb/udc/udc_dwc2.c | 16 ++--------- drivers/usb/udc/udc_it82xx2.c | 18 ++---------- drivers/usb/udc/udc_kinetis.c | 18 ++---------- drivers/usb/udc/udc_max32.c | 14 ++------- drivers/usb/udc/udc_mcux_ehci.c | 20 ++----------- drivers/usb/udc/udc_mcux_ip3511.c | 20 ++----------- drivers/usb/udc/udc_nrf.c | 16 ++--------- drivers/usb/udc/udc_numaker.c | 14 ++------- drivers/usb/udc/udc_renesas_ra.c | 14 ++------- drivers/usb/udc/udc_rpi_pico.c | 16 ++--------- drivers/usb/udc/udc_sam0.c | 16 ++--------- drivers/usb/udc/udc_skeleton.c | 16 ++--------- drivers/usb/udc/udc_smartbond.c | 14 ++------- drivers/usb/udc/udc_stm32.c | 19 ++---------- drivers/usb/udc/udc_virtual.c | 16 ++--------- 17 files changed, 84 insertions(+), 225 deletions(-) diff --git a/drivers/usb/udc/udc_ambiq.c b/drivers/usb/udc/udc_ambiq.c index 6c6ed46495ce6..7f58daabe5576 100644 --- a/drivers/usb/udc/udc_ambiq.c +++ b/drivers/usb/udc/udc_ambiq.c @@ -640,14 +640,11 @@ static int udc_ambiq_init(const struct device *dev) #endif /* Enable Control Endpoints */ - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL, EP0_MPS, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL, EP0_MPS, 0)) { + if (udc_ep_enable_control(dev, EP0_MPS)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } + /* Connect and enable USB interrupt */ cfg->irq_enable_func(dev); @@ -663,14 +660,11 @@ static int udc_ambiq_shutdown(const struct device *dev) LOG_INF("shutdown"); /* Disable Control Endpoints */ - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } + /* Disable USB interrupt */ cfg->irq_disable_func(dev); /* Assert USB PHY reset */ diff --git a/drivers/usb/udc/udc_common.h b/drivers/usb/udc/udc_common.h index ed6712e95ae89..5f996ad423c24 100644 --- a/drivers/usb/udc/udc_common.h +++ b/drivers/usb/udc/udc_common.h @@ -200,6 +200,33 @@ int udc_ep_enable_internal(const struct device *dev, const uint16_t mps, const uint8_t interval); +/** + * @brief Helper function to enable control endpoint. + * + * This function can be used by the driver to enable control endpoint. + * + * @param[in] dev Pointer to device struct of the driver instance + * @param[in] mps Maximum packet size (same as wMaxPacketSize) + * + * @return 0 on success, all other values should be treated as error. + * @retval -ENODEV endpoint is not assigned or no configuration found + * @retval -EALREADY endpoint is already enabled + */ +static inline int udc_ep_enable_control(const struct device *dev, + const uint16_t mps) +{ + int err; + + err = udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, + USB_EP_TYPE_CONTROL, mps, 0); + if (err) { + return err; + } + + return udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, + USB_EP_TYPE_CONTROL, mps, 0); +} + /** * @brief Helper function to disable endpoint. * @@ -215,6 +242,27 @@ int udc_ep_enable_internal(const struct device *dev, int udc_ep_disable_internal(const struct device *dev, const uint8_t ep); +/** + * @brief Helper function to disable control endpoint. + * + * This function can be used by the driver to disable control endpoint. + * + * @param[in] dev Pointer to device struct of the driver instance + * + * @return 0 on success, all other values should be treated as error. + * @retval -ENODEV endpoint is not assigned or no configuration found + * @retval -EALREADY endpoint is already enabled + */ +static inline int udc_ep_disable_control(const struct device *dev) +{ + int err1, err2; + + err1 = udc_ep_disable_internal(dev, USB_CONTROL_EP_IN); + err2 = udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT); + + return err1 ? err1 : err2; +} + /** * @brief Helper function to register endpoint configuration. * diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index 5a47a23294c82..6bf70e883b5da 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -2136,14 +2136,7 @@ static int udc_dwc2_init_controller(const struct device *dev) i, priv->max_txfifo_depth[i], dwc2_get_txfaddr(dev, i)); } - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, 64, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, 64, 0)) { + if (udc_ep_enable_control(dev, 64)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -2206,12 +2199,7 @@ static int udc_dwc2_disable(const struct device *dev) LOG_DBG("Disable device %p", dev); - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_DBG("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_DBG("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_it82xx2.c b/drivers/usb/udc/udc_it82xx2.c index 6b41cb162d49c..4cf23c13b0de0 100644 --- a/drivers/usb/udc/udc_it82xx2.c +++ b/drivers/usb/udc/udc_it82xx2.c @@ -1469,30 +1469,18 @@ static int it82xx2_init(const struct device *dev) it82xx2_usb_dc_ip_init(dev); - ret = udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL, - config->ep_cfg_out[0].caps.mps, 0); + ret = udc_ep_enable_control(dev, config->ep_cfg_out[0].caps.mps); if (ret) { - LOG_ERR("Failed to enable ep 0x%02x", USB_CONTROL_EP_OUT); + LOG_ERR("Failed to enable control endpoint"); return ret; } - ret = udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL, - config->ep_cfg_in[0].caps.mps, 0); - if (ret) { - LOG_ERR("Failed to enable ep 0x%02x", USB_CONTROL_EP_IN); - return ret; - } return 0; } static int it82xx2_shutdown(const struct device *dev) { - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_kinetis.c b/drivers/usb/udc/udc_kinetis.c index 7d22730160396..496eb114feba3 100644 --- a/drivers/usb/udc/udc_kinetis.c +++ b/drivers/usb/udc/udc_kinetis.c @@ -1020,16 +1020,7 @@ static int usbfsotg_init(const struct device *dev) USB_INTEN_ERROREN_MASK | USB_INTEN_USBRSTEN_MASK); - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, - USBFSOTG_EP0_SIZE, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, - USBFSOTG_EP0_SIZE, 0)) { + if (udc_ep_enable_control(dev, USBFSOTG_EP0_SIZE)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -1048,12 +1039,7 @@ static int usbfsotg_shutdown(const struct device *dev) config->irq_disable_func(dev); - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_max32.c b/drivers/usb/udc/udc_max32.c index 526a49b8ba917..6041b3e35d381 100644 --- a/drivers/usb/udc/udc_max32.c +++ b/drivers/usb/udc/udc_max32.c @@ -650,12 +650,7 @@ static int udc_max32_init(const struct device *dev) return ret; } - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL, 64, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL, 64, 0)) { + if (udc_ep_enable_control(dev, 64)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -670,12 +665,7 @@ static int udc_max32_shutdown(const struct device *dev) MXC_USB_Shutdown(); irq_disable(DT_INST_IRQN(0)); - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_mcux_ehci.c b/drivers/usb/udc/udc_mcux_ehci.c index 041f9b759b199..f1d7530c6560f 100644 --- a/drivers/usb/udc/udc_mcux_ehci.c +++ b/drivers/usb/udc/udc_mcux_ehci.c @@ -442,28 +442,12 @@ static void udc_mcux_work_handler(struct k_work *item) mcux_msg = &ev->mcux_msg; if (mcux_msg->code == kUSB_DeviceNotifyBusReset) { - struct udc_ep_config *cfg; - udc_mcux_control(ev->dev, kUSB_DeviceControlSetDefaultStatus, NULL); - cfg = udc_get_ep_cfg(ev->dev, USB_CONTROL_EP_OUT); - if (cfg->stat.enabled) { - udc_ep_disable_internal(ev->dev, USB_CONTROL_EP_OUT); - } - cfg = udc_get_ep_cfg(ev->dev, USB_CONTROL_EP_IN); - if (cfg->stat.enabled) { - udc_ep_disable_internal(ev->dev, USB_CONTROL_EP_IN); - } - if (udc_ep_enable_internal(ev->dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, - USB_MCUX_EP0_SIZE, 0)) { + (void)udc_ep_disable_control(ev->dev); + if (udc_ep_enable_control(ev->dev, USB_MCUX_EP0_SIZE)) { LOG_ERR("Failed to enable control endpoint"); } - if (udc_ep_enable_internal(ev->dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, - USB_MCUX_EP0_SIZE, 0)) { - LOG_ERR("Failed to enable control endpoint"); - } udc_submit_event(ev->dev, UDC_EVT_RESET, 0); } else { ep = mcux_msg->code; diff --git a/drivers/usb/udc/udc_mcux_ip3511.c b/drivers/usb/udc/udc_mcux_ip3511.c index c6aab0d0bb1b1..cf1f39dc0333f 100644 --- a/drivers/usb/udc/udc_mcux_ip3511.c +++ b/drivers/usb/udc/udc_mcux_ip3511.c @@ -442,28 +442,12 @@ static void udc_mcux_work_handler(struct k_work *item) mcux_msg = &ev->mcux_msg; if (mcux_msg->code == kUSB_DeviceNotifyBusReset) { - struct udc_ep_config *cfg; - udc_mcux_control(ev->dev, kUSB_DeviceControlSetDefaultStatus, NULL); - cfg = udc_get_ep_cfg(ev->dev, USB_CONTROL_EP_OUT); - if (cfg->stat.enabled) { - udc_ep_disable_internal(ev->dev, USB_CONTROL_EP_OUT); - } - cfg = udc_get_ep_cfg(ev->dev, USB_CONTROL_EP_IN); - if (cfg->stat.enabled) { - udc_ep_disable_internal(ev->dev, USB_CONTROL_EP_IN); - } - if (udc_ep_enable_internal(ev->dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, - USB_MCUX_EP0_SIZE, 0)) { + (void)udc_ep_disable_control(ev->dev); + if (udc_ep_enable_control(ev->dev, USB_MCUX_EP0_SIZE)) { LOG_ERR("Failed to enable control endpoint"); } - if (udc_ep_enable_internal(ev->dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, - USB_MCUX_EP0_SIZE, 0)) { - LOG_ERR("Failed to enable control endpoint"); - } udc_submit_event(ev->dev, UDC_EVT_RESET, 0); } else { ep = mcux_msg->code; diff --git a/drivers/usb/udc/udc_nrf.c b/drivers/usb/udc/udc_nrf.c index 7b18862fa5966..0f81b2e022622 100644 --- a/drivers/usb/udc/udc_nrf.c +++ b/drivers/usb/udc/udc_nrf.c @@ -1705,14 +1705,7 @@ static int udc_nrf_enable(const struct device *dev) unsigned int key; int ret; - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, UDC_NRF_EP0_SIZE, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, UDC_NRF_EP0_SIZE, 0)) { + if (udc_ep_enable_control(dev, UDC_NRF_EP0_SIZE)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -1738,12 +1731,7 @@ static int udc_nrf_disable(const struct device *dev) nrf_usbd_legacy_disable(); - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_numaker.c b/drivers/usb/udc/udc_numaker.c index 43630010caa28..6134f1830ef11 100644 --- a/drivers/usb/udc/udc_numaker.c +++ b/drivers/usb/udc/udc_numaker.c @@ -1608,12 +1608,7 @@ static int udc_numaker_init(const struct device *dev) /* Initialize all EP H/W contexts */ numaker_usbd_ep_mgmt_init(dev); - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL, 64, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL, 64, 0)) { + if (udc_ep_enable_control(dev, 64)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -1625,12 +1620,7 @@ static int udc_numaker_shutdown(const struct device *dev) { struct udc_numaker_data *priv = udc_get_private(dev); - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_renesas_ra.c b/drivers/usb/udc/udc_renesas_ra.c index 4a3123ec2b9c8..dc640ae266c2e 100644 --- a/drivers/usb/udc/udc_renesas_ra.c +++ b/drivers/usb/udc/udc_renesas_ra.c @@ -506,12 +506,7 @@ static int udc_renesas_ra_init(const struct device *dev) return -EIO; } - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL, 64, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL, 64, 0)) { + if (udc_ep_enable_control(dev, 64)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -537,12 +532,7 @@ static int udc_renesas_ra_shutdown(const struct device *dev) { struct udc_renesas_ra_data *data = udc_get_private(dev); - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_rpi_pico.c b/drivers/usb/udc/udc_rpi_pico.c index 70ebf047399dc..7df7c535b7d28 100644 --- a/drivers/usb/udc/udc_rpi_pico.c +++ b/drivers/usb/udc/udc_rpi_pico.c @@ -1094,14 +1094,7 @@ static int udc_rpi_pico_init(const struct device *dev) const struct pinctrl_dev_config *const pcfg = config->pcfg; int err; - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, 64, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, 64, 0)) { + if (udc_ep_enable_control(dev, 64)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -1121,12 +1114,7 @@ static int udc_rpi_pico_shutdown(const struct device *dev) { const struct rpi_pico_config *config = dev->config; - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_sam0.c b/drivers/usb/udc/udc_sam0.c index e351c98e284bc..2bd52d0f4298e 100644 --- a/drivers/usb/udc/udc_sam0.c +++ b/drivers/usb/udc/udc_sam0.c @@ -981,14 +981,7 @@ static int udc_sam0_enable(const struct device *dev) base->DESCADD.reg = (uintptr_t)config->bdt; - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, 64, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, 64, 0)) { + if (udc_ep_enable_control(dev, 64)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -1017,12 +1010,7 @@ static int udc_sam0_disable(const struct device *dev) base->CTRLA.bit.ENABLE = 0; sam0_wait_syncbusy(dev); - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_skeleton.c b/drivers/usb/udc/udc_skeleton.c index 2c9be371dec29..f324d07e00751 100644 --- a/drivers/usb/udc/udc_skeleton.c +++ b/drivers/usb/udc/udc_skeleton.c @@ -221,14 +221,7 @@ static int udc_skeleton_disable(const struct device *dev) */ static int udc_skeleton_init(const struct device *dev) { - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, 64, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, 64, 0)) { + if (udc_ep_enable_control(dev, 64)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -243,12 +236,7 @@ static int udc_skeleton_init(const struct device *dev) /* Shut down the controller completely */ static int udc_skeleton_shutdown(const struct device *dev) { - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_smartbond.c b/drivers/usb/udc/udc_smartbond.c index 718efa3205c54..66c2d2ca9382c 100644 --- a/drivers/usb/udc/udc_smartbond.c +++ b/drivers/usb/udc/udc_smartbond.c @@ -817,12 +817,7 @@ static enum udc_bus_speed udc_smartbond_device_speed(const struct device *dev) static int udc_smartbond_shutdown(const struct device *dev) { - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } @@ -1597,12 +1592,7 @@ static int udc_smartbond_enable(const struct device *dev) usb_change_state(data, true, data->vbus_present); - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL, 8, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL, 8, 0)) { + if (udc_ep_enable_control(dev, 8)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_stm32.c b/drivers/usb/udc/udc_stm32.c index 8926c52d047be..06a1d3a34f333 100644 --- a/drivers/usb/udc/udc_stm32.c +++ b/drivers/usb/udc/udc_stm32.c @@ -646,17 +646,9 @@ static int udc_stm32_enable(const struct device *dev) return -EIO; } - ret = udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, cfg->ep0_mps, 0); + ret = udc_ep_enable_control(dev, cfg->ep0_mps); if (ret) { - LOG_ERR("Failed enabling ep 0x%02x", USB_CONTROL_EP_OUT); - return ret; - } - - ret |= udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, cfg->ep0_mps, 0); - if (ret) { - LOG_ERR("Failed enabling ep 0x%02x", USB_CONTROL_EP_IN); + LOG_ERR("Failed enabling control endpoint"); return ret; } @@ -672,12 +664,7 @@ static int udc_stm32_disable(const struct device *dev) irq_disable(UDC_STM32_IRQ); - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } diff --git a/drivers/usb/udc/udc_virtual.c b/drivers/usb/udc/udc_virtual.c index db4f56b11b06b..6a8385340df48 100644 --- a/drivers/usb/udc/udc_virtual.c +++ b/drivers/usb/udc/udc_virtual.c @@ -531,14 +531,7 @@ static int udc_vrt_init(const struct device *dev) { const struct udc_vrt_config *config = dev->config; - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, 64, 0)) { - LOG_ERR("Failed to enable control endpoint"); - return -EIO; - } - - if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, 64, 0)) { + if (udc_ep_enable_control(dev, 64)) { LOG_ERR("Failed to enable control endpoint"); return -EIO; } @@ -550,12 +543,7 @@ static int udc_vrt_shutdown(const struct device *dev) { const struct udc_vrt_config *config = dev->config; - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) { - LOG_ERR("Failed to disable control endpoint"); - return -EIO; - } - - if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) { + if (udc_ep_disable_control(dev)) { LOG_ERR("Failed to disable control endpoint"); return -EIO; } From 55bfaba5b33f37556ec9d79e9179b0dc2fd20ae7 Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Mon, 7 Jul 2025 13:31:31 +0200 Subject: [PATCH 2/3] usb: device_next: allow to pass maximum possible MPS value to drivers The wMaxPacketSize (MPS) value of an endpoint may vary depending on the interface's alternate settings. Device controller, which uses dedicated memory for the endpoint FIFO, may end up in a locked state during endpoint reconfiguration if the allocated memory is too small but the higher memory window is still used. Although the driver could allocate a new, larger memory window, the last used window would likely remain unused, wasting a bit of dedicated RAM. Allow the stack to pass the maximum possible MPS value within alternate settings. For a controller with endpoint FIFOs and dedicated RAM, the driver should use this value for the FIFO configuration. Signed-off-by: Johann Fischer --- drivers/usb/udc/udc_common.c | 5 ++- drivers/usb/udc/udc_common.h | 5 ++- include/zephyr/drivers/usb/udc.h | 4 ++ subsys/usb/device_next/usbd_endpoint.c | 3 +- subsys/usb/device_next/usbd_endpoint.h | 2 + subsys/usb/device_next/usbd_interface.c | 56 ++++++++++++++++++++++++- tests/drivers/udc/src/main.c | 4 ++ 7 files changed, 73 insertions(+), 6 deletions(-) diff --git a/drivers/usb/udc/udc_common.c b/drivers/usb/udc/udc_common.c index c81f8a2dd376e..85cd429983beb 100644 --- a/drivers/usb/udc/udc_common.c +++ b/drivers/usb/udc/udc_common.c @@ -330,6 +330,7 @@ int udc_ep_enable_internal(const struct device *dev, const uint8_t ep, const uint8_t attributes, const uint16_t mps, + const uint16_t m_mps, const uint8_t interval) { const struct udc_api *api = dev->api; @@ -353,6 +354,7 @@ int udc_ep_enable_internal(const struct device *dev, cfg->attributes = attributes; cfg->mps = mps; + cfg->m_mps = m_mps; cfg->interval = interval; cfg->stat.odd = 0; @@ -368,6 +370,7 @@ int udc_ep_enable(const struct device *dev, const uint8_t ep, const uint8_t attributes, const uint16_t mps, + const uint16_t m_mps, const uint8_t interval) { const struct udc_api *api = dev->api; @@ -384,7 +387,7 @@ int udc_ep_enable(const struct device *dev, goto ep_enable_error; } - ret = udc_ep_enable_internal(dev, ep, attributes, mps, interval); + ret = udc_ep_enable_internal(dev, ep, attributes, mps, m_mps, interval); ep_enable_error: api->unlock(dev); diff --git a/drivers/usb/udc/udc_common.h b/drivers/usb/udc/udc_common.h index 5f996ad423c24..4d02ac44b4922 100644 --- a/drivers/usb/udc/udc_common.h +++ b/drivers/usb/udc/udc_common.h @@ -198,6 +198,7 @@ int udc_ep_enable_internal(const struct device *dev, const uint8_t ep, const uint8_t attributes, const uint16_t mps, + const uint16_t m_mps, const uint8_t interval); /** @@ -218,13 +219,13 @@ static inline int udc_ep_enable_control(const struct device *dev, int err; err = udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, - USB_EP_TYPE_CONTROL, mps, 0); + USB_EP_TYPE_CONTROL, mps, mps, 0); if (err) { return err; } return udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, - USB_EP_TYPE_CONTROL, mps, 0); + USB_EP_TYPE_CONTROL, mps, mps, 0); } /** diff --git a/include/zephyr/drivers/usb/udc.h b/include/zephyr/drivers/usb/udc.h index 197cfe60e5575..618fb96f80cef 100644 --- a/include/zephyr/drivers/usb/udc.h +++ b/include/zephyr/drivers/usb/udc.h @@ -120,6 +120,8 @@ struct udc_ep_config { uint8_t attributes; /** Maximum packet size */ uint16_t mps; + /** Maximum possible MPS within all interface settings */ + uint16_t m_mps; /** Polling interval */ uint8_t interval; }; @@ -558,6 +560,7 @@ int udc_ep_try_config(const struct device *dev, * @param[in] ep Endpoint address (same as bEndpointAddress) * @param[in] attributes Endpoint attributes (same as bmAttributes) * @param[in] mps Maximum packet size (same as wMaxPacketSize) + * @param[in] m_mps Maximum possible wMaxPacketSize within interface settings * @param[in] interval Polling interval (same as bInterval) * * @return 0 on success, all other values should be treated as error. @@ -570,6 +573,7 @@ int udc_ep_enable(const struct device *dev, const uint8_t ep, const uint8_t attributes, const uint16_t mps, + const uint16_t m_mps, const uint8_t interval); /** diff --git a/subsys/usb/device_next/usbd_endpoint.c b/subsys/usb/device_next/usbd_endpoint.c index 1e0ef658d35ca..4c90a9bcf20be 100644 --- a/subsys/usb/device_next/usbd_endpoint.c +++ b/subsys/usb/device_next/usbd_endpoint.c @@ -19,12 +19,13 @@ LOG_MODULE_REGISTER(usbd_ep, CONFIG_USBD_LOG_LEVEL); int usbd_ep_enable(const struct device *dev, const struct usb_ep_descriptor *const ed, + const uint16_t m_mps, uint32_t *const ep_bm) { int ret; ret = udc_ep_enable(dev, ed->bEndpointAddress, ed->bmAttributes, - sys_le16_to_cpu(ed->wMaxPacketSize), ed->bInterval); + sys_le16_to_cpu(ed->wMaxPacketSize), m_mps, ed->bInterval); if (ret == 0) { usbd_ep_bm_set(ep_bm, ed->bEndpointAddress); } diff --git a/subsys/usb/device_next/usbd_endpoint.h b/subsys/usb/device_next/usbd_endpoint.h index 50ac99804283f..3ddb3ebce5e03 100644 --- a/subsys/usb/device_next/usbd_endpoint.h +++ b/subsys/usb/device_next/usbd_endpoint.h @@ -73,12 +73,14 @@ static inline bool usbd_ep_bm_is_set(const uint32_t *const ep_bm, const uint8_t * * @param[in] dev Pointer to UDC device * @param[in] ed Pointer to endpoint descriptor + * @param[in] m_mps Maximum possible wMaxPacketSize within interface settings * @param[in] ep_bm Pointer to endpoint bitmap * * @return 0 on success, other values on fail. */ int usbd_ep_enable(const struct device *dev, const struct usb_ep_descriptor *const ed, + const uint16_t m_mps, uint32_t *const ep_bm); /** diff --git a/subsys/usb/device_next/usbd_interface.c b/subsys/usb/device_next/usbd_interface.c index 9e94b9eb66106..e685299d63148 100644 --- a/subsys/usb/device_next/usbd_interface.c +++ b/subsys/usb/device_next/usbd_interface.c @@ -25,6 +25,7 @@ enum ep_op { static int handle_ep_op(struct usbd_context *const uds_ctx, const enum ep_op op, struct usb_ep_descriptor *const ed, + const uint16_t m_mps, uint32_t *const ep_bm) { const uint8_t ep = ed->bEndpointAddress; @@ -35,7 +36,7 @@ static int handle_ep_op(struct usbd_context *const uds_ctx, ret = 0; break; case EP_OP_UP: - ret = usbd_ep_enable(uds_ctx->dev, ed, ep_bm); + ret = usbd_ep_enable(uds_ctx->dev, ed, m_mps, ep_bm); break; case EP_OP_DOWN: ret = usbd_ep_disable(uds_ctx->dev, ep, ep_bm); @@ -50,6 +51,50 @@ static int handle_ep_op(struct usbd_context *const uds_ctx, return ret; } +/* + * This function is intended to be called from the interface descriptor + * position, where it can iterate over the endpoint descriptor and any + * alternate settings. It returns wMaxPacketSize for the largest possible total + * data payload for a specific endpoint within an interface descriptor. + */ +static uint16_t interface_find_mps(struct usbd_context *const uds_ctx, + struct usb_desc_header **const dhp, + const uint8_t iface, + const uint8_t ep) +{ + struct usb_desc_header **tmp = dhp; + uint16_t m_mps = 0; + + while (*tmp != NULL && (*tmp)->bLength != 0) { + struct usb_if_descriptor *ifd; + struct usb_ep_descriptor *ed; + uint16_t mps; + + if ((*tmp)->bDescriptorType == USB_DESC_INTERFACE) { + ifd = (struct usb_if_descriptor *)(*tmp); + + if (ifd->bInterfaceNumber != iface) { + break; + } + } + + if ((*tmp)->bDescriptorType == USB_DESC_ENDPOINT) { + ed = (struct usb_ep_descriptor *)(*tmp); + mps = sys_le16_to_cpu(ed->wMaxPacketSize); + + if (ep == ed->bEndpointAddress && + USB_MPS_TO_TPL(mps) > USB_MPS_TO_TPL(m_mps)) { + m_mps = mps; + } + } + + tmp++; + } + + + return m_mps; +} + static int usbd_interface_modify(struct usbd_context *const uds_ctx, struct usbd_class_node *const c_nd, const enum ep_op op, @@ -68,6 +113,7 @@ static int usbd_interface_modify(struct usbd_context *const uds_ctx, while (*dhp != NULL && (*dhp)->bLength != 0) { struct usb_if_descriptor *ifd; struct usb_ep_descriptor *ed; + uint16_t m_mps = 0; if ((*dhp)->bDescriptorType == USB_DESC_INTERFACE) { ifd = (struct usb_if_descriptor *)(*dhp); @@ -89,7 +135,13 @@ static int usbd_interface_modify(struct usbd_context *const uds_ctx, if ((*dhp)->bDescriptorType == USB_DESC_ENDPOINT && found_iface) { ed = (struct usb_ep_descriptor *)(*dhp); - ret = handle_ep_op(uds_ctx, op, ed, &c_nd->ep_active); + + if (op == EP_OP_UP) { + m_mps = interface_find_mps(uds_ctx, dhp, + iface, ed->bEndpointAddress); + } + + ret = handle_ep_op(uds_ctx, op, ed, m_mps, &c_nd->ep_active); if (ret) { return ret; } diff --git a/tests/drivers/udc/src/main.c b/tests/drivers/udc/src/main.c index e08a8f615a15b..90dec5f52ca08 100644 --- a/tests/drivers/udc/src/main.c +++ b/tests/drivers/udc/src/main.c @@ -132,12 +132,15 @@ static void test_udc_ep_enable(const struct device *dev, int err1, err2, err3; err1 = udc_ep_enable(dev, ed->bEndpointAddress, ed->bmAttributes, + sys_le16_to_cpu(ed->wMaxPacketSize), sys_le16_to_cpu(ed->wMaxPacketSize), ed->bInterval); err2 = udc_ep_enable(dev, ed->bEndpointAddress, ed->bmAttributes, + sys_le16_to_cpu(ed->wMaxPacketSize), sys_le16_to_cpu(ed->wMaxPacketSize), ed->bInterval); err3 = udc_ep_enable(dev, ctrl_ep, ed->bmAttributes, + sys_le16_to_cpu(ed->wMaxPacketSize), sys_le16_to_cpu(ed->wMaxPacketSize), ed->bInterval); @@ -320,6 +323,7 @@ static void test_udc_ep_api(const struct device *dev, for (int i = 0; i < num_of_iterations; i++) { err = udc_ep_enable(dev, ed->bEndpointAddress, ed->bmAttributes, + sys_le16_to_cpu(ed->wMaxPacketSize), sys_le16_to_cpu(ed->wMaxPacketSize), ed->bInterval); zassert_ok(err, "Failed to enable endpoint"); From 9da6d9012f8cf6a5acc89149e7145fbcecf34d83 Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Mon, 7 Jul 2025 17:22:43 +0200 Subject: [PATCH 3/3] drivers: udc_dwc2: allocate FIFO memory based on maximum MPS value To ensure that the alternate endpoint setting does not result in a memory window that is too small being allocated and locked because a higher FIFO is still in use, use the maximum MPS value for FIFO memory allocation. Signed-off-by: Johann Fischer --- drivers/usb/udc/udc_dwc2.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/usb/udc/udc_dwc2.c b/drivers/usb/udc/udc_dwc2.c index 6bf70e883b5da..983e972c1f17e 100644 --- a/drivers/usb/udc/udc_dwc2.c +++ b/drivers/usb/udc/udc_dwc2.c @@ -1315,7 +1315,11 @@ static int dwc2_set_dedicated_fifo(const struct device *dev, /* Keep everything but FIFO number */ tmp = *diepctl & ~USB_DWC2_DEPCTL_TXFNUM_MASK; - reqdep = DIV_ROUND_UP(udc_mps_ep_size(cfg), 4U); + /* Use the maximum possible MPS value to ensure that the alternate + * setting does not result in too small memory window being allocated + * and locked because a higher FIFO is still in use. + */ + reqdep = DIV_ROUND_UP(USB_MPS_EP_SIZE(cfg->m_mps), 4U); if (dwc2_in_buffer_dma_mode(dev)) { /* In DMA mode, TxFIFO capable of holding 2 packets is enough */ reqdep *= MIN(2, (1 + addnl));