fix critical section wrong use

class/hub/usbh_hub.c

@@ -29,7 +29,7 @@ static uint32_t g_devinuse = 0;
 
 usb_slist_t hub_class_head = USB_SLIST_OBJECT_INIT(hub_class_head);
 
-USB_NOCACHE_RAM_SECTION uint8_t int_buffer[6][USBH_HUB_INTIN_BUFSIZE];
+USB_MEM_ALIGN32 uint8_t int_buffer[6][USBH_HUB_INTIN_BUFSIZE];
 extern void usbh_external_hport_connect(struct usbh_hubport *hport);
 extern void usbh_external_hport_disconnect(struct usbh_hubport *hport);
 extern void usbh_hport_activate(struct usbh_hubport *hport);
@@ -326,7 +326,6 @@ static void usbh_extern_hub_psc_event(void *arg)
     uint16_t change;
     uint16_t mask;
     uint16_t feat;
-    uint32_t flags;
     int ret;
 
     hub_class = (struct usbh_hub *)arg;
@@ -484,11 +483,10 @@ static void usbh_extern_hub_psc_event(void *arg)
         /* Hub status changed */
         USB_LOG_WRN("Hub status changed, not handled\n");
     }
-    flags = usb_osal_enter_critical_section();
+
     if (hub_class->parent->connected) {
         ret = usbh_ep_intr_async_transfer(hub_class->intin, hub_class->int_buffer, USBH_HUB_INTIN_BUFSIZE, usbh_external_hub_callback, hub_class);
     }
-    usb_osal_leave_critical_section(flags);
 }
 
 static void usbh_external_hub_callback(void *arg, int nbytes)

core/usbh_core.c

@@ -28,8 +28,6 @@
 static const char *speed_table[] = { "error speed", "low speed", "full speed", "high speed" };
 
 static const struct usbh_class_driver *usbh_find_class_driver(uint8_t class, uint8_t subcalss, uint8_t protocol, uint16_t vid, uint16_t pid);
-void usbh_hport_activate(struct usbh_hubport *hport);
-void usbh_hport_deactivate(struct usbh_hubport *hport);
 
 /* general descriptor field offsets */
 #define DESC_bLength         0 /** Length offset */
@@ -154,6 +152,44 @@ static int usbh_devaddr_destroy(struct usbh_hubport *hport, uint8_t dev_addr)
     return usbh_free_devaddr(&rhport->devgen, dev_addr);
 }
 
+void usbh_hport_activate(struct usbh_hubport *hport)
+{
+    struct usbh_endpoint_cfg ep0_cfg;
+
+    memset(&ep0_cfg, 0, sizeof(struct usbh_endpoint_cfg));
+
+    ep0_cfg.ep_addr = 0x00;
+    ep0_cfg.ep_interval = 0x00;
+    ep0_cfg.ep_mps = 0x08;
+    ep0_cfg.ep_type = USB_ENDPOINT_TYPE_CONTROL;
+    ep0_cfg.hport = hport;
+    /* Allocate memory for roothub port control endpoint */
+    usbh_ep_alloc(&hport->ep0, &ep0_cfg);
+}
+
+void usbh_hport_deactivate(struct usbh_hubport *hport)
+{
+    uint32_t flags;
+
+    /* Don't free the control pipe of root hub ports! */
+    if (hport->parent != NULL && hport->ep0 != NULL) {
+        usb_ep_cancel(hport->ep0);
+        usbh_ep_free(hport->ep0);
+        hport->ep0 = NULL;
+    }
+
+    flags = usb_osal_enter_critical_section();
+    /* Free the device address if one has been assigned */
+    usbh_devaddr_destroy(hport, hport->dev_addr);
+    hport->dev_addr = 0;
+    usb_osal_leave_critical_section(flags);
+
+    if (hport->setup)
+        usb_iofree(hport->setup);
+
+    hport->setup = NULL;
+}
+
 static int parse_device_descriptor(struct usbh_hubport *hport, struct usb_device_descriptor *desc, uint16_t length)
 {
     if (desc->bLength != USB_SIZEOF_DEVICE_DESC) {
@@ -673,17 +709,14 @@ static int usbh_portchange_wait(struct usbh_hubport **hport)
 static void usbh_portchange_detect_thread(void *argument)
 {
     struct usbh_hubport *hport = NULL;
-    uint32_t flags;
-
-    flags = usb_osal_enter_critical_section();
-    usb_hc_init();
 
     for (uint8_t port = USBH_HUB_PORT_START_INDEX; port <= CONFIG_USBHOST_RHPORTS; port++) {
         usbh_core_cfg.rhport[port - 1].hport.port = port;
         usbh_core_cfg.rhport[port - 1].devgen.next = 1;
         usbh_hport_activate(&usbh_core_cfg.rhport[port - 1].hport);
     }
-    usb_osal_leave_critical_section(flags);
+
+    usb_hc_init();
 
     while (1) {
         usbh_portchange_wait(&hport);
@@ -699,9 +732,7 @@ static void usbh_portchange_detect_thread(void *argument)
             } else {
                 USB_LOG_INFO("Hub %u, Port %u connected, %s\r\n", hport->parent->index, hport->port, speed_table[hport->speed]);
             }
-            usb_osal_thread_suspend(g_lpworkq.thread);
             usbh_enumerate(hport);
-            usb_osal_thread_resume(g_lpworkq.thread);
         } else {
             usbh_hport_deactivate(hport);
             for (uint8_t i = 0; i < hport->config.config_desc.bNumInterfaces; i++) {
@@ -734,7 +765,9 @@ void usbh_external_hport_connect(struct usbh_hubport *hport)
 
     if (usbh_core_cfg.pscwait) {
         usbh_core_cfg.pscwait = false;
+        usb_osal_leave_critical_section(flags);
         usb_osal_sem_give(usbh_core_cfg.pscsem);
+        return;
     }
 
     usb_osal_leave_critical_section(flags);
@@ -750,54 +783,14 @@ void usbh_external_hport_disconnect(struct usbh_hubport *hport)
 
     if (usbh_core_cfg.pscwait) {
         usbh_core_cfg.pscwait = false;
+        usb_osal_leave_critical_section(flags);
         usb_osal_sem_give(usbh_core_cfg.pscsem);
+        return;
     }
 
     usb_osal_leave_critical_section(flags);
 }
 
-void usbh_hport_activate(struct usbh_hubport *hport)
-{
-    struct usbh_endpoint_cfg ep0_cfg;
-    uint32_t flags;
-
-    flags = usb_osal_enter_critical_section();
-    memset(&ep0_cfg, 0, sizeof(struct usbh_endpoint_cfg));
-
-    ep0_cfg.ep_addr = 0x00;
-    ep0_cfg.ep_interval = 0x00;
-    ep0_cfg.ep_mps = 0x08;
-    ep0_cfg.ep_type = USB_ENDPOINT_TYPE_CONTROL;
-    ep0_cfg.hport = hport;
-    /* Allocate memory for roothub port control endpoint */
-    usbh_ep_alloc(&hport->ep0, &ep0_cfg);
-
-    usb_osal_leave_critical_section(flags);
-}
-
-void usbh_hport_deactivate(struct usbh_hubport *hport)
-{
-    uint32_t flags;
-
-    flags = usb_osal_enter_critical_section();
-    /* Don't free the control pipe of root hub ports! */
-    if (hport->parent != NULL && hport->ep0 != NULL) {
-        usb_ep_cancel(hport->ep0);
-        usbh_ep_free(hport->ep0);
-        hport->ep0 = NULL;
-    }
-    /* Free the device address if one has been assigned */
-    usbh_devaddr_destroy(hport, hport->dev_addr);
-
-    if (hport->setup)
-        usb_iofree(hport->setup);
-
-    hport->setup = NULL;
-    hport->dev_addr = 0;
-
-    usb_osal_leave_critical_section(flags);
-}
-
 void usbh_event_notify_handler(uint8_t event, uint8_t rhport)
 {
     switch (event) {

core/usbh_core.h

@@ -119,8 +119,6 @@ typedef struct usbh_hub {
 void usbh_event_notify_handler(uint8_t event, uint8_t rhport);
 
 int usbh_initialize(void);
-int usbh_lock(void);
-void usbh_unlock(void);
 int lsusb(int argc, char **argv);
 struct usbh_hubport *usbh_find_hubport(uint8_t dev_addr);
 void *usbh_find_class_instance(const char *devname);

port/ehci/usb_ehci.c

@@ -750,7 +750,7 @@ static struct usb_ehci_qh_s *usb_ehci_qh_create(struct usb_ehci_epinfo_s *epinfo
 
 #ifndef CONFIG_USBHOST_INT_DISABLE
     if (epinfo->xfrtype == USB_ENDPOINT_TYPE_INTERRUPT) {
-        regval |= ((uint32_t)epinfo->interval << QH_EPCAPS_SSMASK_SHIFT);
+        regval |= ((uint32_t)1 << QH_EPCAPS_SSMASK_SHIFT);
     }
 #endif
 
@@ -1831,8 +1831,11 @@ static inline void usb_ehci_portsc_bottomhalf(void)
 
         /* Handle port connection status change (CSC) events */
         if ((portsc & EHCI_PORTSC_CSC) != 0) {
-            /* Check current connect status */
-            if ((portsc & (EHCI_PORTSC_CCS | EHCI_PORTSC_PE)) != 0) {
+            /* Debounce */
+            usb_osal_msleep(25);
+            /* Check current connect status*/
+            portsc = usb_ehci_getreg(&HCOR->portsc[rhpndx]);
+            if ((portsc & EHCI_PORTSC_CCS) == EHCI_PORTSC_CCS) {
                 /* Connected ... Did we just become connected? */
                 if (!g_ehci.connected) {
                     g_ehci.connected = 1;
@@ -2032,7 +2035,12 @@ int usb_hc_init(void)
     uintptr_t physaddr1;
     uintptr_t physaddr2;
 
-    memset(&g_ehci, 0, sizeof(struct usb_ehci_s));
+    g_ehci.connected = 0;
+    g_ehci.qhfree = NULL;
+    g_ehci.qtdfree = NULL;
+
+    usb_slist_init(&g_ehci.epinfo_list);
+
     /* Initialize the list of free Queue Head (QH) structures */
 
     for (uint8_t i = 0; i < CONFIG_USB_EHCI_QH_NUM; i++) {
@@ -2257,21 +2265,16 @@ int usbh_ep0_reconfigure(usbh_epinfo_t ep, uint8_t dev_addr, uint8_t ep_mps, uin
 
 int usbh_ep_alloc(usbh_epinfo_t *ep, const struct usbh_endpoint_cfg *ep_cfg)
 {
-    int ret;
     struct usb_ehci_epinfo_s *epinfo;
     struct usbh_hubport *hport;
 
     DEBUGASSERT(ep_cfg != NULL && ep_cfg->hport != NULL);
 
-    ret = usb_osal_mutex_take(g_ehci.exclsem);
-    if (ret < 0) {
-        return ret;
-    }
-
     hport = ep_cfg->hport;
 
     /* new roothub ep info */
     if (((ep_cfg->ep_type & USB_ENDPOINT_TYPE_MASK) == USB_ENDPOINT_TYPE_CONTROL) && (hport->parent == NULL)) {
+        memset(&g_ehci.ep0, 0, sizeof(struct usb_ehci_epinfo_s));
         epinfo = &g_ehci.ep0[hport->port - 1];
     } else {
         /* new exteranl hub ep info */
@@ -2294,28 +2297,21 @@ int usbh_ep_alloc(usbh_epinfo_t *ep, const struct usbh_endpoint_cfg *ep_cfg)
     epinfo->hport = hport;
 
     epinfo->iocsem = usb_osal_sem_create(0);
+    usb_slist_add_tail(&g_ehci.epinfo_list, &epinfo->list);
 
     *ep = epinfo;
-    usb_slist_add_tail(&g_ehci.epinfo_list, &epinfo->list);
-    usb_osal_mutex_give(g_ehci.exclsem);
+
     return 0;
 }
 
 int usbh_ep_free(usbh_epinfo_t ep)
 {
-    int ret;
     struct usb_ehci_epinfo_s *epinfo = (struct usb_ehci_epinfo_s *)ep;
 
-    ret = usb_osal_mutex_take(g_ehci.exclsem);
-    if (ret < 0) {
-        return ret;
-    }
-
     usb_osal_sem_delete(epinfo->iocsem);
     usb_slist_remove(&g_ehci.epinfo_list, &epinfo->list);
     usb_free(epinfo);
 
-    usb_osal_mutex_give(g_ehci.exclsem);
     return 0;
 }
 

port/synopsys/usb_hc_synopsys.c

@@ -73,7 +73,6 @@ struct usb_synopsys_priv {
  *   Allocate a channel.
  *
  ****************************************************************************/
-
 static int usb_synopsys_chan_alloc(struct usb_synopsys_priv *priv)
 {
     int chidx;
@@ -119,7 +118,6 @@ static void usb_synopsys_chan_free(struct usb_synopsys_priv *priv, int chidx)
  *   Free all channels.
  *
  ****************************************************************************/
-
 static inline void usb_synopsys_chan_freeall(struct usb_synopsys_priv *priv)
 {
     uint8_t chidx;
@@ -145,7 +143,6 @@ static inline void usb_synopsys_chan_freeall(struct usb_synopsys_priv *priv)
  *   started.
  *
  ****************************************************************************/
-
 static int usb_synopsys_chan_waitsetup(struct usb_synopsys_priv *priv,
                                        struct usb_synopsys_chan *chan)
 {
@@ -189,7 +186,6 @@ static int usb_synopsys_chan_waitsetup(struct usb_synopsys_priv *priv,
  *   Might be called from the level of an interrupt handler
  *
  ****************************************************************************/
-
 #ifdef CONFIG_USBHOST_ASYNCH
 static int usb_synopsys_chan_asynchsetup(struct usb_synopsys_priv *priv,
                                          struct usb_synopsys_chan *chan,
@@ -221,7 +217,7 @@ static int usb_synopsys_chan_asynchsetup(struct usb_synopsys_priv *priv,
 #endif
 
 /****************************************************************************
- * Name: stm32_chan_wait
+ * Name: usb_synopsys_chan_wait
  *
  * Description:
  *   Wait for a transfer on a channel to complete.
@@ -230,7 +226,6 @@ static int usb_synopsys_chan_asynchsetup(struct usb_synopsys_priv *priv,
  *   Called from a normal thread context
  *
  ****************************************************************************/
-
 static int usb_synopsys_chan_wait(struct usb_synopsys_priv *priv, struct usb_synopsys_chan *chan)
 {
     int ret;
@@ -254,7 +249,7 @@ static int usb_synopsys_chan_wait(struct usb_synopsys_priv *priv, struct usb_syn
 }
 
 /****************************************************************************
- * Name: stm32_chan_wakeup
+ * Name: usb_synopsys_chan_wakeup
  *
  * Description:
  *   A channel transfer has completed... wakeup any threads waiting for the
@@ -265,7 +260,6 @@ static int usb_synopsys_chan_wait(struct usb_synopsys_priv *priv, struct usb_syn
  *   the channel.  Interrupts are disabled.
  *
  ****************************************************************************/
-
 static void usb_synopsys_chan_wakeup(struct usb_synopsys_priv *priv, struct usb_synopsys_chan *chan)
 {
     usbh_asynch_callback_t callback;
@@ -323,7 +317,9 @@ __WEAK void usb_hc_low_level_init(void)
 
 int usb_hc_init(void)
 {
-    memset(&g_usbhost, 0, sizeof(struct usb_synopsys_priv));
+    g_usbhost.sof_timer = 0;
+    g_usbhost.connected = 0;
+    g_usbhost.pscwait = 0;
 #if defined(CONFIG_USB_HS) || defined(CONFIG_USB_HS_IN_FULL)
     g_usbhost.handle = &hhcd_USB_OTG_HS;
     g_usbhost.handle->Instance = USB_OTG_HS;
@@ -334,15 +330,6 @@ int usb_hc_init(void)
 
     g_usbhost.exclsem = usb_osal_mutex_create();
 
-    for (uint8_t i = 0; i < CONFIG_USBHOST_CHANNELS; i++) {
-        struct usb_synopsys_chan *chan = &g_usbhost.chan[i];
-
-        /* The waitsem semaphore is used for signaling and, hence, should not
-       * have priority inheritance enabled.
-       */
-        chan->waitsem = usb_osal_sem_create(0);
-    }
-
     g_usbhost.handle->Init.Host_channels = CONFIG_USBHOST_CHANNELS;
     g_usbhost.handle->Init.speed = HCD_SPEED_FULL;
     g_usbhost.handle->Init.dma_enable = DISABLE;
@@ -405,32 +392,32 @@ int usbh_ep_alloc(usbh_epinfo_t *ep, const struct usbh_endpoint_cfg *ep_cfg)
     struct usb_synopsys_ctrlinfo *ep0;
     struct usbh_hubport *hport;
     int chidx;
-    int ret;
     uint8_t speed;
 
-    ret = usb_osal_mutex_take(g_usbhost.exclsem);
-    if (ret < 0) {
-        return ret;
-    }
-
     hport = ep_cfg->hport;
 
     if (hport->speed == USB_SPEED_FULL) {
         speed = 1;
     } else if (hport->speed == USB_SPEED_LOW) {
         speed = 2;
+    } else if (hport->speed == USB_SPEED_HIGH) {
+        speed = 0;
     }
 
     if (ep_cfg->ep_type == USB_ENDPOINT_TYPE_CONTROL) {
         ep0 = usb_malloc(sizeof(struct usb_synopsys_ctrlinfo));
+        memset(ep0, 0, sizeof(struct usb_synopsys_ctrlinfo));
 
         ep0->outndx = usb_synopsys_chan_alloc(&g_usbhost);
         ep0->inndx = usb_synopsys_chan_alloc(&g_usbhost);
 
         chan = &priv->chan[ep0->outndx];
-        chan->interval = 0;
+        memset(chan, 0, sizeof(struct usb_synopsys_chan));
+        chan->waitsem = usb_osal_sem_create(0);
+
         chan = &priv->chan[ep0->inndx];
-        chan->interval = 0;
+        memset(chan, 0, sizeof(struct usb_synopsys_chan));
+        chan->waitsem = usb_osal_sem_create(0);
 
         HAL_HCD_HC_Init(g_usbhost.handle, ep0->outndx, 0x00, hport->dev_addr, speed, USB_ENDPOINT_TYPE_CONTROL, ep_cfg->ep_mps);
         HAL_HCD_HC_Init(g_usbhost.handle, ep0->inndx, 0x80, hport->dev_addr, speed, USB_ENDPOINT_TYPE_CONTROL, ep_cfg->ep_mps);
@@ -441,7 +428,9 @@ int usbh_ep_alloc(usbh_epinfo_t *ep, const struct usbh_endpoint_cfg *ep_cfg)
         chidx = usb_synopsys_chan_alloc(&g_usbhost);
 
         chan = &priv->chan[chidx];
+        memset(chan, 0, sizeof(struct usb_synopsys_chan));
         chan->interval = ep_cfg->ep_interval;
+        chan->waitsem = usb_osal_sem_create(0);
 
         HAL_HCD_HC_Init(g_usbhost.handle, chidx, ep_cfg->ep_addr, hport->dev_addr, speed, ep_cfg->ep_type, ep_cfg->ep_mps);
 
@@ -450,27 +439,22 @@ int usbh_ep_alloc(usbh_epinfo_t *ep, const struct usbh_endpoint_cfg *ep_cfg)
 
         *ep = (usbh_epinfo_t)chidx;
     }
-    usb_osal_mutex_give(g_usbhost.exclsem);
     return 0;
 }
 
 int usbh_ep_free(usbh_epinfo_t ep)
 {
-    int ret;
-
-    ret = usb_osal_mutex_take(g_usbhost.exclsem);
-    if (ret < 0) {
-        return ret;
-    }
     if ((uintptr_t)ep < CONFIG_USBHOST_CHANNELS) {
         usb_synopsys_chan_free(&g_usbhost, (int)ep);
+        usb_osal_sem_delete(g_usbhost.chan[ep].waitsem);
     } else {
         struct usb_synopsys_ctrlinfo *ep0 = (struct usb_synopsys_ctrlinfo *)ep;
         usb_synopsys_chan_free(&g_usbhost, ep0->inndx);
         usb_synopsys_chan_free(&g_usbhost, ep0->outndx);
+        usb_osal_sem_delete(g_usbhost.chan[ep0->inndx].waitsem);
+        usb_osal_sem_delete(g_usbhost.chan[ep0->outndx].waitsem);
     }
 
-    usb_osal_mutex_give(g_usbhost.exclsem);
     return 0;
 }
 
@@ -770,6 +754,10 @@ int usb_ep_cancel(usbh_epinfo_t ep)
     uint32_t flags;
     struct usb_synopsys_chan *chan;
     struct usb_synopsys_priv *priv = &g_usbhost;
+#ifdef CONFIG_USBHOST_ASYNCH
+    usbh_asynch_callback_t callback;
+    void *arg;
+#endif
 
     uint8_t chidx = (uint8_t)ep;
 
@@ -778,6 +766,16 @@ int usb_ep_cancel(usbh_epinfo_t ep)
     flags = usb_osal_enter_critical_section();
 
     chan->result = -ESHUTDOWN;
+#ifdef CONFIG_USBHOST_ASYNCH
+    /* Extract the callback information */
+    callback = chan->callback;
+    arg = chan->arg;
+    chan->callback = NULL;
+    chan->arg = NULL;
+    chan->xfrd = 0;
+#endif
+    usb_osal_leave_critical_section(flags);
+
     /* Is there a thread waiting for this transfer to complete? */
 
     if (chan->waiter) {
@@ -786,29 +784,12 @@ int usb_ep_cancel(usbh_epinfo_t ep)
         usb_osal_sem_give(chan->waitsem);
     }
 #ifdef CONFIG_USBHOST_ASYNCH
-    /* No.. is an asynchronous callback expected when the transfer
-   * completes?
-   */
-
-    else if (chan->callback) {
-        usbh_asynch_callback_t callback;
-        void *arg;
-
-        /* Extract the callback information */
-
-        callback = chan->callback;
-        arg = chan->arg;
-
-        chan->callback = NULL;
-        chan->arg = NULL;
-        chan->xfrd = 0;
-
+    /* No.. is an asynchronous callback expected when the transfer completes? */
+    else if (callback) {
         /* Then perform the callback */
-
         callback(arg, -ESHUTDOWN);
     }
 #endif
-    usb_osal_leave_critical_section(flags);
     return 0;
 }