From c5fe1bad9c54ed8b25baf73b92e5ed72aba8698a Mon Sep 17 00:00:00 2001 From: Armin Brauns Date: Wed, 27 Mar 2024 13:52:08 +0000 Subject: [PATCH 1/2] usb: stm32: fix calculation of TX FIFO sizes The RX FIFO size is in words, so needs to be subtracted from the total memory size *after* it's divided by 4. Fixes #70789. Signed-off-by: Armin Brauns --- drivers/usb/device/usb_dc_stm32.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/device/usb_dc_stm32.c b/drivers/usb/device/usb_dc_stm32.c index 13131061610064..7be05d95ee3ed0 100644 --- a/drivers/usb/device/usb_dc_stm32.c +++ b/drivers/usb/device/usb_dc_stm32.c @@ -131,10 +131,10 @@ static const struct gpio_dt_spec ulpi_reset = #define TX_FIFO_NUM USB_NUM_BIDIR_ENDPOINTS /* We need a minimum size for RX FIFO */ -#define USB_FIFO_RX_MIN 160 +#define RX_FIFO_EP_WORDS 160 /* 4-byte words TX FIFO */ -#define TX_FIFO_WORDS ((USB_RAM_SIZE - USB_FIFO_RX_MIN - 64) / 4) +#define TX_FIFO_WORDS ((USB_RAM_SIZE - 64) / 4 - RX_FIFO_EP_WORDS) /* Allocate FIFO memory evenly between the TX FIFOs */ /* except the first TX endpoint need only 64 bytes */ @@ -446,7 +446,7 @@ static int usb_dc_stm32_init(void) #else /* USB_OTG_FS */ /* TODO: make this dynamic (depending usage) */ - HAL_PCDEx_SetRxFiFo(&usb_dc_stm32_state.pcd, USB_FIFO_RX_MIN); + HAL_PCDEx_SetRxFiFo(&usb_dc_stm32_state.pcd, RX_FIFO_EP_WORDS); for (i = 0U; i < USB_NUM_BIDIR_ENDPOINTS; i++) { if (i == 0) { /* first endpoint need only 64 byte for EP_TYPE_CTRL */ From 2e15d392b03eec87993ee12fa5a2544e1511a7b8 Mon Sep 17 00:00:00 2001 From: Armin Brauns Date: Wed, 27 Mar 2024 13:52:42 +0000 Subject: [PATCH 2/2] usb: stm32: clarify calculation of FIFO sizes Magic constants throughout the code made this difficult to reason about, especially with two different units of measurement (bytes and words) at play. Signed-off-by: Armin Brauns --- drivers/usb/device/usb_dc_stm32.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/usb/device/usb_dc_stm32.c b/drivers/usb/device/usb_dc_stm32.c index 7be05d95ee3ed0..75ce9442f8e822 100644 --- a/drivers/usb/device/usb_dc_stm32.c +++ b/drivers/usb/device/usb_dc_stm32.c @@ -130,14 +130,14 @@ static const struct gpio_dt_spec ulpi_reset = /* We need n TX IN FIFOs */ #define TX_FIFO_NUM USB_NUM_BIDIR_ENDPOINTS -/* We need a minimum size for RX FIFO */ +/* We need a minimum size for RX FIFO - exact number seemingly determined through trial and error */ #define RX_FIFO_EP_WORDS 160 -/* 4-byte words TX FIFO */ -#define TX_FIFO_WORDS ((USB_RAM_SIZE - 64) / 4 - RX_FIFO_EP_WORDS) - /* Allocate FIFO memory evenly between the TX FIFOs */ /* except the first TX endpoint need only 64 bytes */ +#define TX_FIFO_EP_0_WORDS 16 +#define TX_FIFO_WORDS (USB_RAM_SIZE / 4 - RX_FIFO_EP_WORDS - TX_FIFO_EP_0_WORDS) +/* Number of words for each remaining TX endpoint FIFO */ #define TX_FIFO_EP_WORDS (TX_FIFO_WORDS / (TX_FIFO_NUM - 1)) #endif /* USB */ @@ -450,7 +450,8 @@ static int usb_dc_stm32_init(void) for (i = 0U; i < USB_NUM_BIDIR_ENDPOINTS; i++) { if (i == 0) { /* first endpoint need only 64 byte for EP_TYPE_CTRL */ - HAL_PCDEx_SetTxFiFo(&usb_dc_stm32_state.pcd, i, 16); + HAL_PCDEx_SetTxFiFo(&usb_dc_stm32_state.pcd, i, + TX_FIFO_EP_0_WORDS); } else { HAL_PCDEx_SetTxFiFo(&usb_dc_stm32_state.pcd, i, TX_FIFO_EP_WORDS);