From 1632beaedee5171ab6c1dfc48092319c149b5ebf Mon Sep 17 00:00:00 2001 From: daniellizewski Date: Thu, 9 Apr 2026 13:49:50 -0400 Subject: [PATCH] arch/arm/src/stm32h5/stm32_usbdrdhost.c: Fixed Hub support Fixed a few bugs in the initial stm32_usbdrdhost.c implementation when using a USB hub. Fixed fault when freeing control endpoints. Fixed crash when disconnecting devices from hubs due ot use after free. Signed-off-by: daniellizewski --- arch/arm/src/stm32h5/stm32_usbdrdhost.c | 130 +++++++++++++----- .../nucleo-h563zi/configs/usbmsc/defconfig | 1 + .../arm/stm32h5/nucleo-h563zi/src/stm32_usb.c | 10 ++ drivers/usbhost/usbhost_hub.c | 21 +-- 4 files changed, 122 insertions(+), 40 deletions(-) diff --git a/arch/arm/src/stm32h5/stm32_usbdrdhost.c b/arch/arm/src/stm32h5/stm32_usbdrdhost.c index a562046dc1b..953fecd041b 100644 --- a/arch/arm/src/stm32h5/stm32_usbdrdhost.c +++ b/arch/arm/src/stm32h5/stm32_usbdrdhost.c @@ -217,6 +217,12 @@ struct stm32_usbhost_s sem_t pscsem; /* Port status change sem */ struct stm32_ctrlinfo_s ep0; /* EP0 description */ +#ifdef CONFIG_USBHOST_HUB + /* Used to pass external hub port events */ + + volatile struct usbhost_hubport_s *hport; +#endif + struct usbhost_devaddr_s devgen; /* Address generation data */ /* Host channels */ @@ -1452,13 +1458,19 @@ static void stm32_hc_in_irq(struct stm32_usbhost_s *priv, int chidx) if ((chepval & USB_CHEP_ERRRX) != 0) { - /* Clear error bit: write 0 to ERRRX to clear, - * write 1 to VTRX/VTTX to preserve + chepval = stm32_getreg(STM32H5_USB_CHEP(chidx)); + uerr("ERRRX chidx=%d chepval=0x%08x rx_status=%d nak=%d\n", + chidx, (unsigned int)chepval, + (int)((chepval & USB_CHEP_RX_STRX_MASK) >> + USB_CHEP_RX_STRX_SHIFT), + (int)((chepval & USB_CHEP_NAK) != 0)); + + /* Clear ERRRX (write 0) and VTRX (write 0) to acknowledge the CTR + * interrupt. Preserve VTTX by writing 1. */ - chepval = stm32_getreg(STM32H5_USB_CHEP(chidx)); - chepval = (chepval & USB_CHEP_REG_MASK & ~USB_CHEP_ERRRX) | - USB_CHEP_VTRX | USB_CHEP_VTTX; + chepval = (chepval & (0xffff7fff & USB_CHEP_REG_MASK) & + ~USB_CHEP_ERRRX) | USB_CHEP_VTTX; stm32_putreg(STM32H5_USB_CHEP(chidx), chepval); chan->result = EIO; @@ -1758,6 +1770,18 @@ static int stm32_usbdrd_interrupt(int irq, void *context, void *arg) } else { + /* STM32H562xx/563xx/573xx Errata: + * During OUT transfers, the correct transfer interrupt (CTR) is + * triggered a little before the last USB SRAM accesses have + * completed. If the software responds quickly to the interrupt, + * the full buffer contents may not be correct. + * Software should ensure that a small delay is included before + * accessing the SRAM contents. This delay should be 800 ns in + * Full Speed mode and 6.4 μs in Low Speed mode. + */ + + up_udelay(1); + stm32_hc_out_irq(priv, chidx); } } @@ -1833,9 +1857,31 @@ static int stm32_wait(struct usbhost_connection_s *conn, *hport = connport; leave_critical_section(flags); + + uinfo("RHport Connected: %s\n", + connport->connected ? "YES" : "NO"); return OK; } +#ifdef CONFIG_USBHOST_HUB + /* Is a device connected to an external hub? */ + + if (priv->hport) + { + /* Yes.. return the external hub port */ + + connport = (struct usbhost_hubport_s *)priv->hport; + priv->hport = NULL; + + *hport = connport; + leave_critical_section(flags); + + uinfo("Hub port Connected: %s\n", + connport->connected ? "YES" : "NO"); + return OK; + } +#endif + priv->pscwait = true; ret = nxsem_wait(&priv->pscsem); if (ret < 0) @@ -1859,6 +1905,7 @@ static int stm32_rh_enumerate(struct stm32_usbhost_s *priv, struct usbhost_hubport_s *hport) { uint32_t istr; + int ret; DEBUGASSERT(hport != NULL && hport->port == 0); @@ -1889,10 +1936,20 @@ static int stm32_rh_enumerate(struct stm32_usbhost_s *priv, hport->speed = USB_SPEED_FULL; } + /* Allocate control endpoint */ + + ret = stm32_ctrlchan_alloc(priv, 0, 0, hport->speed, &priv->ep0); + if (ret < 0) + { + uerr("ERROR: Failed to allocate EP0: %d\n", ret); + nxmutex_unlock(&priv->lock); + return ret; + } + uinfo("Enumerated device at %s speed\n", hport->speed == USB_SPEED_LOW ? "low" : "full"); - return OK; + return ret; } /**************************************************************************** @@ -1913,7 +1970,9 @@ static int stm32_enumerate(struct usbhost_connection_s *conn, /* If this is the root hub port */ - if (hport->port == 0) +#ifdef CONFIG_USBHOST_HUB + if (ROOTHUB(hport)) +#endif { ret = stm32_rh_enumerate(priv, conn, hport); if (ret < 0) @@ -1922,26 +1981,6 @@ static int stm32_enumerate(struct usbhost_connection_s *conn, } } - /* Get exclusive access */ - - ret = nxmutex_lock(&priv->lock); - if (ret < 0) - { - return ret; - } - - /* Allocate control endpoint */ - - ret = stm32_ctrlchan_alloc(priv, 0, 0, hport->speed, &priv->ep0); - if (ret < 0) - { - uerr("ERROR: Failed to allocate EP0: %d\n", ret); - nxmutex_unlock(&priv->lock); - return ret; - } - - nxmutex_unlock(&priv->lock); - /* Perform standard enumeration */ ret = usbhost_enumerate(hport, &hport->devclass); @@ -2066,9 +2105,31 @@ static int stm32_epfree(struct usbhost_driver_s *drvr, usbhost_ep_t ep) return ret; } - /* Free channel(s) */ + /* A single channel is represent by an index in the range of 0 to + * STM32H5_NHOST_CHANNELS. Otherwise, the ep must be a pointer to + * an allocated control endpoint structure. + */ - stm32_chan_free(priv, (int)ep); + if ((uintptr_t)ep < STM32H5_NHOST_CHANNELS) + { + /* Halt the channel and mark the channel available */ + + stm32_chan_free(priv, (int)ep); + } + else + { + /* Halt both control channel and mark the channels available */ + + struct stm32_ctrlinfo_s *ctrlep = + (struct stm32_ctrlinfo_s *)ep; + + stm32_chan_free(priv, ctrlep->inndx); + stm32_chan_free(priv, ctrlep->outndx); + + /* And free the control endpoint container */ + + kmm_free(ctrlep); + } nxmutex_unlock(&priv->lock); return OK; @@ -2452,9 +2513,16 @@ static int stm32_connect(struct usbhost_driver_s *drvr, DEBUGASSERT(priv != NULL && hport != NULL); - flags = enter_critical_section(); - priv->change = true; + /* Set the connected/disconnected flag */ + hport->connected = connected; + uinfo("Hub port %d connected: %s\n", + hport->port, connected ? "YES" : "NO"); + + /* Report the connection event */ + + flags = enter_critical_section(); + priv->hport = hport; if (priv->pscwait) { priv->pscwait = false; diff --git a/boards/arm/stm32h5/nucleo-h563zi/configs/usbmsc/defconfig b/boards/arm/stm32h5/nucleo-h563zi/configs/usbmsc/defconfig index 64ae9cfffd1..e0749c2aaa6 100644 --- a/boards/arm/stm32h5/nucleo-h563zi/configs/usbmsc/defconfig +++ b/boards/arm/stm32h5/nucleo-h563zi/configs/usbmsc/defconfig @@ -54,4 +54,5 @@ CONFIG_STM32H5_USE_HSE=y CONFIG_SYSTEM_NSH=y CONFIG_TASK_NAME_SIZE=0 CONFIG_USART3_SERIAL_CONSOLE=y +CONFIG_USBHOST_HUB=y CONFIG_USBHOST_MSC=y diff --git a/boards/arm/stm32h5/nucleo-h563zi/src/stm32_usb.c b/boards/arm/stm32h5/nucleo-h563zi/src/stm32_usb.c index bed293d1a06..64e2e6eb06f 100644 --- a/boards/arm/stm32h5/nucleo-h563zi/src/stm32_usb.c +++ b/boards/arm/stm32h5/nucleo-h563zi/src/stm32_usb.c @@ -151,6 +151,16 @@ int stm32_usbhost_initialize(void) uinfo("Register class drivers\n"); +#ifdef CONFIG_USBHOST_HUB + /* Initialize USB hub class support */ + + ret = usbhost_hub_initialize(); + if (ret < 0) + { + uerr("ERROR: usbhost_hub_initialize failed: %d\n", ret); + } +#endif + #ifdef CONFIG_USBHOST_MSC /* Register the USB mass storage class class */ diff --git a/drivers/usbhost/usbhost_hub.c b/drivers/usbhost/usbhost_hub.c index 8269dcaf97f..52032ee87dd 100644 --- a/drivers/usbhost/usbhost_hub.c +++ b/drivers/usbhost/usbhost_hub.c @@ -1001,18 +1001,21 @@ static void usbhost_hub_event(FAR void *arg) connport = &priv->hport[PORT_INDX(port)]; if (connport->devclass != NULL) { + /* For hubs, the usbhost_disconnect_event function + * (triggered by the CLASS_DISCONNECTED call below) + * will call usbhost_hport_deactivate for us. We + * prevent a crash when a hub is unplugged by skipping + * the second unnecessary usbhost_hport_deactivated + * call here. Check before disconnecting because the class + * will be cleared out after disconnected. + */ + + bool ishubclass = + (connport->devclass->connect == usbhost_connect); CLASS_DISCONNECTED(connport->devclass); - if (connport->devclass->connect == usbhost_connect) + if (ishubclass) { - /* For hubs, the usbhost_disconnect_event function - * (triggered by the CLASS_DISCONNECTED call above) - * will call usbhost_hport_deactivate for us. We - * prevent a crash when a hub is unplugged by skipping - * the second unnecessary usbhost_hport_deactivated - * call here. - */ - connport->devclass = NULL; } else