Skip to content

drivers: usb: udc: stm32: driver cleanup and EP MPS fixes #96926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+82 −52
Open

drivers: usb: udc: stm32: driver cleanup and EP MPS fixes #96926

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d3adb48
drivers: usb: udc: stm32: turn EP0 max packet size into a constant
mathieuchopstm Oct 2, 2025
6c349f0
drivers: usb: udc: stm32: clean up handling of USB buffer table
mathieuchopstm Oct 2, 2025
88aab2b
drivers: usb: udc: stm32: accept non-word-aligned MaxPacketSize or FIFOs
mathieuchopstm Oct 2, 2025
8940344
drivers: usb: udc: stm32: allow EP max packet size up to HW capabilities
mathieuchopstm Oct 2, 2025
00d8ee0
drivers: usb: udc: stm32: configure OTGFS/HS RxFIFO size using Kconfig
mathieuchopstm Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 45 additions & 46 deletions drivers/usb/udc/udc_stm32.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,17 @@ LOG_MODULE_REGISTER(udc_stm32, CONFIG_UDC_DRIVER_LOG_LEVEL);
(PCD_SPEED_HIGH_IN_FULL), \
(PCD_SPEED_HIGH))))

/*
* Returns max packet size allowed for endpoints of 'usb_node'
*
* Hardware always supports the maximal value allowed
* by the USB Specification at a given operating speed:
* 1024 bytes in High-Speed, 1023 bytes in Full-Speed
*/
#define UDC_STM32_NODE_EP_MPS(node_id) \
((UDC_STM32_NODE_SPEED(node_id) == PCD_SPEED_HIGH) ? 1024U : 1023U)


Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpicking: could you remove this extra empty line?

#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32n6_otghs)
#define USB_USBPHYC_CR_FSEL_24MHZ USB_USBPHYC_CR_FSEL_1
#endif
Expand All @@ -136,6 +147,13 @@ static const int syscfg_otg_hs_phy_clk[] = {
};
#endif

/*
* Hardcode EP0 max packet size (bMaxPacketSize0) to 64,
* which is the maximum allowed by the USB Specification
* and supported by all STM32 USB controllers.
*/
#define UDC_STM32_EP0_MAX_PACKET_SIZE 64U

struct udc_stm32_data {
PCD_HandleTypeDef pcd;
const struct device *dev;
Expand All @@ -150,14 +168,13 @@ struct udc_stm32_data {

struct udc_stm32_config {
uint32_t num_endpoints;
uint32_t pma_offset;
uint32_t dram_size;
uint16_t ep0_mps;
uint16_t ep_mps;
/* PHY selected for use by instance */
uint32_t selected_phy;
/* Speed selected for use by instance */
uint32_t selected_speed;
/* Maximal packet size allowed for endpoints */
uint16_t ep_mps;
};

enum udc_stm32_msg_type {
Expand Down Expand Up @@ -188,19 +205,20 @@ void HAL_PCD_ResetCallback(PCD_HandleTypeDef *hpcd)
{
struct udc_stm32_data *priv = hpcd2data(hpcd);
const struct device *dev = priv->dev;
const struct udc_stm32_config *cfg = dev->config;
struct udc_ep_config *ep;

/* Re-Enable control endpoints */
ep = udc_get_ep_cfg(dev, USB_CONTROL_EP_OUT);
if (ep && ep->stat.enabled) {
HAL_PCD_EP_Open(&priv->pcd, USB_CONTROL_EP_OUT, cfg->ep0_mps,
HAL_PCD_EP_Open(&priv->pcd, USB_CONTROL_EP_OUT,
UDC_STM32_EP0_MAX_PACKET_SIZE,
EP_TYPE_CTRL);
}

ep = udc_get_ep_cfg(dev, USB_CONTROL_EP_IN);
if (ep && ep->stat.enabled) {
HAL_PCD_EP_Open(&priv->pcd, USB_CONTROL_EP_IN, cfg->ep0_mps,
HAL_PCD_EP_Open(&priv->pcd, USB_CONTROL_EP_IN,
UDC_STM32_EP0_MAX_PACKET_SIZE,
EP_TYPE_CTRL);
}

Expand Down Expand Up @@ -288,7 +306,6 @@ static int udc_stm32_tx(const struct device *dev, struct udc_ep_config *epcfg,
struct net_buf *buf)
{
struct udc_stm32_data *priv = udc_get_private(dev);
const struct udc_stm32_config *cfg = dev->config;
uint8_t *data; uint32_t len;
HAL_StatusTypeDef status;

Expand All @@ -302,7 +319,7 @@ static int udc_stm32_tx(const struct device *dev, struct udc_ep_config *epcfg,
len = buf->len;

if (epcfg->addr == USB_CONTROL_EP_IN) {
len = MIN(cfg->ep0_mps, buf->len);
len = MIN(UDC_STM32_EP0_MAX_PACKET_SIZE, buf->len);
}

buf->data += len;
Expand Down Expand Up @@ -444,8 +461,7 @@ static void handle_msg_data_in(struct udc_stm32_data *priv, uint8_t epnum)
}

if (ep == USB_CONTROL_EP_IN && buf->len) {
const struct udc_stm32_config *cfg = dev->config;
uint32_t len = MIN(cfg->ep0_mps, buf->len);
uint32_t len = MIN(UDC_STM32_EP0_MAX_PACKET_SIZE, buf->len);

HAL_PCD_EP_Transmit(&priv->pcd, ep, buf->data, len);

Expand Down Expand Up @@ -603,7 +619,12 @@ static inline void udc_stm32_mem_init(const struct device *dev)
struct udc_stm32_data *priv = udc_get_private(dev);
const struct udc_stm32_config *cfg = dev->config;

priv->occupied_mem = cfg->pma_offset;
/**
* Endpoint configuration table is placed at the
* beginning of Private Memory Area and consumes
* 8 bytes for each endpoint.
*/
priv->occupied_mem = (8 * cfg->num_endpoints);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpicking: could drop parentheses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, copy-paste artifact from expanding USB_BTABLE_SIZE = (8 * USB_NUM_BIDIR_ENDPOINTS).

}

static int udc_stm32_ep_mem_config(const struct device *dev,
Expand Down Expand Up @@ -643,22 +664,16 @@ static void udc_stm32_mem_init(const struct device *dev)

LOG_DBG("DRAM size: %ub", cfg->dram_size);

if (cfg->ep_mps % 4 || cfg->ep0_mps % 4) {
LOG_ERR("Not a 32-bit word multiple: ep0(%u)|ep(%u)",
cfg->ep0_mps, cfg->ep_mps);
return;
}

/* The documentation is not clear at all about RX FiFo size requirement,
* 160 has been selected through trial and error.
*/
words = MAX(160, cfg->ep_mps / 4);
words = MAX(160, DIV_ROUND_UP(cfg->ep_mps, 4U));
HAL_PCDEx_SetRxFiFo(&priv->pcd, words);
priv->occupied_mem = words * 4;

/* For EP0 TX, reserve only one MPS */
HAL_PCDEx_SetTxFiFo(&priv->pcd, 0, cfg->ep0_mps / 4);
priv->occupied_mem += cfg->ep0_mps;
HAL_PCDEx_SetTxFiFo(&priv->pcd, 0, DIV_ROUND_UP(UDC_STM32_EP0_MAX_PACKET_SIZE, 4U));
priv->occupied_mem += UDC_STM32_EP0_MAX_PACKET_SIZE;

/* Reset TX allocs */
for (unsigned int i = 1U; i < cfg->num_endpoints; i++) {
Expand All @@ -678,7 +693,7 @@ static int udc_stm32_ep_mem_config(const struct device *dev,
return 0;
}

words = MIN(udc_mps_ep_size(ep), cfg->ep_mps) / 4;
words = DIV_ROUND_UP(MIN(udc_mps_ep_size(ep), cfg->ep_mps), 4U);
words = (words <= 64) ? words * 2 : words;

if (!enable) {
Expand All @@ -705,7 +720,6 @@ static int udc_stm32_ep_mem_config(const struct device *dev,
static int udc_stm32_enable(const struct device *dev)
{
struct udc_stm32_data *priv = udc_get_private(dev);
const struct udc_stm32_config *cfg = dev->config;
HAL_StatusTypeDef status;
int ret;

Expand All @@ -720,14 +734,16 @@ static int udc_stm32_enable(const struct device *dev)
}

ret = udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT,
USB_EP_TYPE_CONTROL, cfg->ep0_mps, 0);
USB_EP_TYPE_CONTROL,
UDC_STM32_EP0_MAX_PACKET_SIZE, 0);
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);
USB_EP_TYPE_CONTROL,
UDC_STM32_EP0_MAX_PACKET_SIZE, 0);
if (ret) {
LOG_ERR("Failed enabling ep 0x%02x", USB_CONTROL_EP_IN);
return ret;
Expand Down Expand Up @@ -1066,22 +1082,7 @@ static const struct udc_api udc_stm32_api = {
* Kconfig system.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these inline comment still useful? (non-blocking, liekly a bit outside the scope of this P-R)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO the comment still applies because there is some usage remaining of the Cube macros (see L1115 in new file). But eventually it should be removed yes.

#define USB_NUM_BIDIR_ENDPOINTS DT_INST_PROP(0, num_bidir_endpoints)

#if defined(USB) || defined(USB_DRD_FS)
#define EP0_MPS 64U
#define EP_MPS 64U
#define USB_BTABLE_SIZE (8 * USB_NUM_BIDIR_ENDPOINTS)
#define USB_RAM_SIZE DT_INST_PROP(0, ram_size)
#else /* USB_OTG_FS */
#define EP0_MPS USB_OTG_MAX_EP0_SIZE
#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32_otghs)
#define EP_MPS USB_OTG_HS_MAX_PACKET_SIZE
#elif DT_HAS_COMPAT_STATUS_OKAY(st_stm32_otgfs) || DT_HAS_COMPAT_STATUS_OKAY(st_stm32_usb)
#define EP_MPS USB_OTG_FS_MAX_PACKET_SIZE
#endif
#define USB_RAM_SIZE DT_INST_PROP(0, ram_size)
#define USB_BTABLE_SIZE 0
#endif /* USB */
#define USB_RAM_SIZE DT_INST_PROP(0, ram_size)

static struct udc_stm32_data udc0_priv;

Expand All @@ -1093,9 +1094,7 @@ static struct udc_data udc0_data = {
static const struct udc_stm32_config udc0_cfg = {
.num_endpoints = USB_NUM_BIDIR_ENDPOINTS,
.dram_size = USB_RAM_SIZE,
.pma_offset = USB_BTABLE_SIZE,
.ep0_mps = EP0_MPS,
.ep_mps = EP_MPS,
.ep_mps = UDC_STM32_NODE_EP_MPS(DT_DRV_INST(0)),
.selected_phy = UDC_STM32_NODE_PHY_ITFACE(DT_DRV_INST(0)),
.selected_speed = UDC_STM32_NODE_SPEED(DT_DRV_INST(0)),
};
Expand All @@ -1109,7 +1108,7 @@ static void priv_pcd_prepare(const struct device *dev)

/* Default values */
priv->pcd.Init.dev_endpoints = cfg->num_endpoints;
priv->pcd.Init.ep0_mps = cfg->ep0_mps;
priv->pcd.Init.ep0_mps = UDC_STM32_EP0_MAX_PACKET_SIZE;
priv->pcd.Init.speed = cfg->selected_speed;

/* Per controller/Phy values */
Expand Down Expand Up @@ -1332,7 +1331,7 @@ static int udc_stm32_driver_init0(const struct device *dev)
ep_cfg_out[i].caps.out = 1;
if (i == 0) {
ep_cfg_out[i].caps.control = 1;
ep_cfg_out[i].caps.mps = cfg->ep0_mps;
ep_cfg_out[i].caps.mps = UDC_STM32_EP0_MAX_PACKET_SIZE;
} else {
ep_cfg_out[i].caps.bulk = 1;
ep_cfg_out[i].caps.interrupt = 1;
Expand All @@ -1352,7 +1351,7 @@ static int udc_stm32_driver_init0(const struct device *dev)
ep_cfg_in[i].caps.in = 1;
if (i == 0) {
ep_cfg_in[i].caps.control = 1;
ep_cfg_in[i].caps.mps = cfg->ep0_mps;
ep_cfg_in[i].caps.mps = UDC_STM32_EP0_MAX_PACKET_SIZE;
} else {
ep_cfg_in[i].caps.bulk = 1;
ep_cfg_in[i].caps.interrupt = 1;
Expand Down
Loading