Skip to content

Commit

Permalink
drivers: usb: udc_stm32: Offload data callbacks to thread
Browse files Browse the repository at this point in the history 
All HAL callback handling is offloaded to a separate thread,
fixing possible assertion errors.
Tested on STM32H747 (arduino portenta) with the high speed controller.

Fixes #61464

Signed-off-by: Tobias Pisani <[email protected]>
  • Loading branch information
@topisani
topisani committed Dec 6, 2024
1 parent aa03ade commit 480c9c5
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 53 deletions.
22 changes: 22 additions & 0 deletions drivers/usb/udc/Kconfig.stm32
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,25 @@ config UDC_STM32
default y
help
STM32 USB device controller driver.


config UDC_STM32_STACK_SIZE
int "UDC controller driver internal thread stack size"
depends on UDC_STM32
default 512
help
STM32 USB device controller driver internal thread stack size.

config UDC_STM32_THREAD_PRIORITY
int "STM32 USB controller driver thread priority"
depends on UDC_STM32
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.
185 changes: 132 additions & 53 deletions drivers/usb/udc/udc_stm32.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "udc_common.h"

#include "stm32_hsem.h"
#include "zephyr/kernel.h"

#include <zephyr/logging/log.h>
LOG_MODULE_REGISTER(udc_stm32, CONFIG_UDC_DRIVER_LOG_LEVEL);
Expand All @@ -50,6 +51,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 {
Expand All @@ -61,6 +64,18 @@ struct udc_stm32_config {
int speed_idx;
};

enum udc_stm32_msg_kind {
UDC_STM32_MSG_SETUP,
UDC_STM32_MSG_DATA_OUT,
UDC_STM32_MSG_DATA_IN,
};

struct udc_stm32_msg {
uint8_t kind;
uint8_t ep;
uint16_t rx_count;
};

static int udc_stm32_lock(const struct device *dev)
{
return udc_lock_internal(dev, K_FOREVER);
Expand Down Expand Up @@ -124,6 +139,24 @@ 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 = {.kind = UDC_STM32_MSG_SETUP};
int 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);
Expand All @@ -142,56 +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 int udc_stm32_tx(const struct device *dev, uint8_t ep, struct net_buf *buf)
{
struct udc_stm32_data *priv = udc_get_private(dev);
Expand Down Expand Up @@ -258,8 +241,29 @@ static int udc_stm32_rx(const struct device *dev, uint8_t ep, struct net_buf *bu

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);
uint32_t rx_count = HAL_PCD_EP_GetRxCount(&priv->pcd, epnum);
struct udc_stm32_msg msg = {.kind = UDC_STM32_MSG_DATA_OUT, .ep = epnum, .rx_count = rx_count };

Check warning on line 246 in drivers/usb/udc/udc_stm32.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE 

drivers/usb/udc/udc_stm32.c:246  line length of 104 exceeds 100 columns
int 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 = {.kind = UDC_STM32_MSG_DATA_IN, .ep = epnum};
int err = k_msgq_put(&priv->msgq_data, &msg, K_NO_WAIT);

if (err < 0) {
LOG_ERR("UDC Message queue overrun");
}
}

static void priv_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;
Expand Down Expand Up @@ -297,9 +301,8 @@ void HAL_PCD_DataOutStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum)
}
}

void HAL_PCD_DataInStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum)
static void priv_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;
Expand Down Expand Up @@ -362,6 +365,69 @@ void HAL_PCD_DataInStageCallback(PCD_HandleTypeDef *hpcd, uint8_t epnum)
}
}

static void priv_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.kind) {
case UDC_STM32_MSG_SETUP:
priv_handle_msg_setup(priv);
break;
case UDC_STM32_MSG_DATA_IN:
priv_handle_msg_data_in(priv, msg.ep);
break;
case UDC_STM32_MSG_DATA_OUT:
priv_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)
{
Expand Down Expand Up @@ -1087,6 +1153,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);
Expand Down Expand Up @@ -1147,6 +1217,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);

err = pinctrl_apply_state(usb_pcfg, PINCTRL_STATE_DEFAULT);
Expand Down

0 comments on commit 480c9c5

Please sign in to comment.