From 700e9fbe086f15690e0b2bf604d6fb01bf1e2d02 Mon Sep 17 00:00:00 2001 From: Tobias Pisani Date: Sun, 3 Nov 2024 14:13:21 +0100 Subject: [PATCH 1/3] drivers: udc: stm32: Offload data callbacks to thread All HAL callback handling is offloaded to a separate thread, as they involve non isr-compatible operations such as mutexes. Fixes #61464 Signed-off-by: Tobias Pisani --- drivers/usb/udc/Kconfig.stm32 | 23 ++++ drivers/usb/udc/udc_stm32.c | 196 ++++++++++++++++++++++++---------- 2 files changed, 164 insertions(+), 55 deletions(-) diff --git a/drivers/usb/udc/Kconfig.stm32 b/drivers/usb/udc/Kconfig.stm32 index 3afe6b15e3f27..050c162282a16 100644 --- a/drivers/usb/udc/Kconfig.stm32 +++ b/drivers/usb/udc/Kconfig.stm32 @@ -13,3 +13,26 @@ config UDC_STM32 default y help STM32 USB device controller driver. + +if UDC_STM32 + +config UDC_STM32_STACK_SIZE + int "UDC controller driver internal thread stack size" + default 512 + help + STM32 USB device controller driver internal thread stack size. + +config UDC_STM32_THREAD_PRIORITY + int "STM32 USB controller driver thread priority" + default 8 + help + STM32 USB device controller driver thread priority. + +config UDC_STM32_MAX_QMESSAGES + int "STM32 UDC driver maximum number of ISR event messages" + range 4 64 + default 8 + help + Maximum number of messages for handling of STM32 USBD ISR events. + +endif diff --git a/drivers/usb/udc/udc_stm32.c b/drivers/usb/udc/udc_stm32.c index 5f53a28bc814e..7bb828c209188 100644 --- a/drivers/usb/udc/udc_stm32.c +++ b/drivers/usb/udc/udc_stm32.c @@ -23,8 +23,6 @@ #include "udc_common.h" -#include "stm32_hsem.h" - #include LOG_MODULE_REGISTER(udc_stm32, CONFIG_UDC_DRIVER_LOG_LEVEL); @@ -50,6 +48,8 @@ struct udc_stm32_data { void (*pcd_prepare)(const struct device *dev); int (*clk_enable)(void); int (*clk_disable)(void); + struct k_thread thread_data; + struct k_msgq msgq_data; }; struct udc_stm32_config { @@ -60,6 +60,18 @@ struct udc_stm32_config { uint16_t ep_mps; }; +enum udc_stm32_msg_type { + UDC_STM32_MSG_SETUP, + UDC_STM32_MSG_DATA_OUT, + UDC_STM32_MSG_DATA_IN, +}; + +struct udc_stm32_msg { + uint8_t type; + uint8_t ep; + uint16_t rx_count; +}; + static int udc_stm32_lock(const struct device *dev) { return udc_lock_internal(dev, K_FOREVER); @@ -125,6 +137,26 @@ void HAL_PCD_ResumeCallback(PCD_HandleTypeDef *hpcd) udc_submit_event(priv->dev, UDC_EVT_RESUME, 0); } +void HAL_PCD_SetupStageCallback(PCD_HandleTypeDef *hpcd) +{ + struct udc_stm32_data *priv = hpcd2data(hpcd); + struct udc_stm32_msg msg = {.type = UDC_STM32_MSG_SETUP}; + int err; + + err = k_msgq_put(&priv->msgq_data, &msg, K_NO_WAIT); + + if (err < 0) { + LOG_ERR("UDC Message queue overrun"); + } +} + +void HAL_PCD_SOFCallback(PCD_HandleTypeDef *hpcd) +{ + struct udc_stm32_data *priv = hpcd2data(hpcd); + + udc_submit_event(priv->dev, UDC_EVT_SOF, 0); +} + static int usbd_ctrl_feed_dout(const struct device *dev, const size_t length) { struct udc_stm32_data *priv = udc_get_private(dev); @@ -143,57 +175,6 @@ static int usbd_ctrl_feed_dout(const struct device *dev, const size_t length) return 0; } -void HAL_PCD_SetupStageCallback(PCD_HandleTypeDef *hpcd) -{ - struct udc_stm32_data *priv = hpcd2data(hpcd); - struct usb_setup_packet *setup = (void *)priv->pcd.Setup; - const struct device *dev = priv->dev; - struct net_buf *buf; - int err; - - buf = udc_ctrl_alloc(dev, USB_CONTROL_EP_OUT, - sizeof(struct usb_setup_packet)); - if (buf == NULL) { - LOG_ERR("Failed to allocate for setup"); - return; - } - - udc_ep_buf_set_setup(buf); - memcpy(buf->data, setup, 8); - net_buf_add(buf, 8); - - udc_ctrl_update_stage(dev, buf); - - if (!buf->len) { - return; - } - - if ((setup->bmRequestType == 0) && - (setup->bRequest == USB_SREQ_SET_ADDRESS)) { - /* HAL requires we set the address before submitting status */ - HAL_PCD_SetAddress(&priv->pcd, setup->wValue); - } - - if (udc_ctrl_stage_is_data_out(dev)) { - /* Allocate and feed buffer for data OUT stage */ - err = usbd_ctrl_feed_dout(dev, udc_data_stage_length(buf)); - if (err == -ENOMEM) { - udc_submit_ep_event(dev, buf, err); - } - } else if (udc_ctrl_stage_is_data_in(dev)) { - udc_ctrl_submit_s_in_status(dev); - } else { - udc_ctrl_submit_s_status(dev); - } -} - -void HAL_PCD_SOFCallback(PCD_HandleTypeDef *hpcd) -{ - struct udc_stm32_data *priv = hpcd2data(hpcd); - - udc_submit_event(priv->dev, UDC_EVT_SOF, 0); -} - static void udc_stm32_flush_tx_fifo(const struct device *dev) { struct udc_stm32_data *priv = udc_get_private(dev); @@ -275,6 +256,36 @@ void HAL_PCD_DataOutStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum) { uint32_t rx_count = HAL_PCD_EP_GetRxCount(hpcd, epnum); struct udc_stm32_data *priv = hpcd2data(hpcd); + struct udc_stm32_msg msg = { + .type = UDC_STM32_MSG_DATA_OUT, + .ep = epnum, + .rx_count = rx_count, + }; + int err; + + err = k_msgq_put(&priv->msgq_data, &msg, K_NO_WAIT); + if (err != 0) { + LOG_ERR("UDC Message queue overrun"); + } +} + +void HAL_PCD_DataInStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum) +{ + struct udc_stm32_data *priv = hpcd2data(hpcd); + struct udc_stm32_msg msg = { + .type = UDC_STM32_MSG_DATA_IN, + .ep = epnum, + }; + int err; + + err = k_msgq_put(&priv->msgq_data, &msg, K_NO_WAIT); + if (err != 0) { + LOG_ERR("UDC Message queue overrun"); + } +} + +static void handle_msg_data_out(struct udc_stm32_data *priv, uint8_t epnum, uint16_t rx_count) +{ const struct device *dev = priv->dev; uint8_t ep = epnum | USB_EP_DIR_OUT; struct net_buf *buf; @@ -312,9 +323,8 @@ void HAL_PCD_DataOutStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum) } } -void HAL_PCD_DataInStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum) +static void handle_msg_data_in(struct udc_stm32_data *priv, uint8_t epnum) { - struct udc_stm32_data *priv = hpcd2data(hpcd); const struct device *dev = priv->dev; uint8_t ep = epnum | USB_EP_DIR_IN; struct net_buf *buf; @@ -378,6 +388,69 @@ void HAL_PCD_DataInStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum) } } +static void handle_msg_setup(struct udc_stm32_data *priv) +{ + struct usb_setup_packet *setup = (void *)priv->pcd.Setup; + const struct device *dev = priv->dev; + struct net_buf *buf; + int err; + + buf = udc_ctrl_alloc(dev, USB_CONTROL_EP_OUT, sizeof(struct usb_setup_packet)); + if (buf == NULL) { + LOG_ERR("Failed to allocate for setup"); + return; + } + + udc_ep_buf_set_setup(buf); + memcpy(buf->data, setup, 8); + net_buf_add(buf, 8); + + udc_ctrl_update_stage(dev, buf); + + if (!buf->len) { + return; + } + + if ((setup->bmRequestType == 0) && (setup->bRequest == USB_SREQ_SET_ADDRESS)) { + /* HAL requires we set the address before submitting status */ + HAL_PCD_SetAddress(&priv->pcd, setup->wValue); + } + + if (udc_ctrl_stage_is_data_out(dev)) { + /* Allocate and feed buffer for data OUT stage */ + err = usbd_ctrl_feed_dout(dev, udc_data_stage_length(buf)); + if (err == -ENOMEM) { + udc_submit_ep_event(dev, buf, err); + } + } else if (udc_ctrl_stage_is_data_in(dev)) { + udc_ctrl_submit_s_in_status(dev); + } else { + udc_ctrl_submit_s_status(dev); + } +} + +static void udc_stm32_thread_handler(void *arg1, void *arg2, void *arg3) +{ + const struct device *dev = arg1; + struct udc_stm32_data *priv = udc_get_private(dev); + struct udc_stm32_msg msg; + + while (true) { + k_msgq_get(&priv->msgq_data, &msg, K_FOREVER); + switch (msg.type) { + case UDC_STM32_MSG_SETUP: + handle_msg_setup(priv); + break; + case UDC_STM32_MSG_DATA_IN: + handle_msg_data_in(priv, msg.ep); + break; + case UDC_STM32_MSG_DATA_OUT: + handle_msg_data_out(priv, msg.ep, msg.rx_count); + break; + } + } +} + #if DT_INST_NODE_HAS_PROP(0, disconnect_gpios) void HAL_PCDEx_SetConnectionState(PCD_HandleTypeDef *hpcd, uint8_t state) { @@ -1128,6 +1201,10 @@ static const struct gpio_dt_spec ulpi_reset = GPIO_DT_SPEC_GET_OR(DT_PHANDLE(DT_INST(0, st_stm32_otghs), phys), reset_gpios, {0}); #endif +static char udc_msgq_buf_0[CONFIG_UDC_STM32_MAX_QMESSAGES * sizeof(struct udc_stm32_msg)]; + +K_THREAD_STACK_DEFINE(udc_stm32_stack_0, CONFIG_UDC_STM32_STACK_SIZE); + static int udc_stm32_driver_init0(const struct device *dev) { struct udc_stm32_data *priv = udc_get_private(dev); @@ -1185,6 +1262,15 @@ static int udc_stm32_driver_init0(const struct device *dev) priv->clk_disable = priv_clock_disable; priv->pcd_prepare = priv_pcd_prepare; + k_msgq_init(&priv->msgq_data, udc_msgq_buf_0, sizeof(struct udc_stm32_msg), + CONFIG_UDC_STM32_MAX_QMESSAGES); + + k_thread_create(&priv->thread_data, udc_stm32_stack_0, + K_THREAD_STACK_SIZEOF(udc_stm32_stack_0), udc_stm32_thread_handler, + (void *)dev, NULL, NULL, K_PRIO_COOP(CONFIG_UDC_STM32_THREAD_PRIORITY), + K_ESSENTIAL, K_NO_WAIT); + k_thread_name_set(&priv->thread_data, dev->name); + IRQ_CONNECT(UDC_STM32_IRQ, UDC_STM32_IRQ_PRI, udc_stm32_irq, DEVICE_DT_INST_GET(0), 0); From bd9d7a7bc120b5f621f4e1bdb802a8f2f2c76d0e Mon Sep 17 00:00:00 2001 From: Tobias Pisani Date: Wed, 20 Nov 2024 13:56:58 +0100 Subject: [PATCH 2/3] drivers: udc: stm32: set the hs capability if high speed is available This fixes usbd_caps_speed(), which is used in sample_usbd_init, and the documentation. Without it, no high speed configuration would be added. Signed-off-by: Tobias Pisani --- drivers/usb/udc/udc_stm32.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/usb/udc/udc_stm32.c b/drivers/usb/udc/udc_stm32.c index 7bb828c209188..726b8b232305d 100644 --- a/drivers/usb/udc/udc_stm32.c +++ b/drivers/usb/udc/udc_stm32.c @@ -58,6 +58,7 @@ struct udc_stm32_config { uint32_t dram_size; uint16_t ep0_mps; uint16_t ep_mps; + int speed_idx; }; enum udc_stm32_msg_type { @@ -960,6 +961,7 @@ static const struct udc_stm32_config udc0_cfg = { .pma_offset = USB_BTABLE_SIZE, .ep0_mps = EP0_MPS, .ep_mps = EP_MPS, + .speed_idx = DT_ENUM_IDX_OR(DT_DRV_INST(0), maximum_speed, 1), }; #if defined(USB_OTG_FS) || defined(USB_OTG_HS) @@ -1255,6 +1257,9 @@ static int udc_stm32_driver_init0(const struct device *dev) data->caps.rwup = true; data->caps.out_ack = false; data->caps.mps0 = UDC_MPS0_64; + if (cfg->speed_idx == 2) { + data->caps.hs = true; + } priv->dev = dev; priv->irq = UDC_STM32_IRQ; From cedc8c6542dcf9811013542210b01676a634c8d9 Mon Sep 17 00:00:00 2001 From: Tobias Pisani Date: Fri, 6 Dec 2024 12:46:44 +0100 Subject: [PATCH 3/3] drivers: udc: stm32: Fix RX FIFO min size On USB HS, the previous lower limit of 64 is too small, the value 160 has been chosen apparently through trial and error. See 1204aa25 for the original implementation in the old device driver. Signed-off-by: Tobias Pisani --- drivers/usb/udc/udc_stm32.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/udc/udc_stm32.c b/drivers/usb/udc/udc_stm32.c index 726b8b232305d..003f7e1b018db 100644 --- a/drivers/usb/udc/udc_stm32.c +++ b/drivers/usb/udc/udc_stm32.c @@ -546,9 +546,9 @@ static void udc_stm32_mem_init(const struct device *dev) } /* The documentation is not clear at all about RX FiFo size requirement, - * Allocate a minimum of 0x40 words, which seems to work reliably. + * 160 has been selected through trial and error. */ - words = MAX(0x40, cfg->ep_mps / 4); + words = MAX(160, cfg->ep_mps / 4); HAL_PCDEx_SetRxFiFo(&priv->pcd, words); priv->occupied_mem = words * 4;